Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed 404 handling in app generator. #11

Merged
merged 2 commits into from
Aug 27, 2014

Conversation

magneland
Copy link
Contributor

Return 404 instead of 500 for show action on unknown resource id.

Signed-off-by: Magne Land [email protected]

@@ -198,6 +198,9 @@ RUBY
# media_type media_type
# end
Praxis::ApiDefinition.define do
response_template :not_found do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's no need to define the not_found template, as it is given by the framework.
But! for such a simple app, the autoloader is being too lazy so by the time the resource definition is read the template is not yet registered (i.e. this is a bug).

I've pushed a change that solve the issue. So if you don't mind pulling the latest code and resubmitting the PR without this template block?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, done.

Return 404 instead of 500 for show action on unknown resource id.

Signed-off-by: Magne Land <[email protected]>
@magneland magneland force-pushed the 10-fix_app_generator branch from b1ac6da to ed0748f Compare August 27, 2014 01:31
@blanquer
Copy link
Contributor

Thank you Magne!
LGTM. Fixes #10

blanquer added a commit that referenced this pull request Aug 27, 2014
Fixed 404 handling in app generator.
@blanquer blanquer merged commit 0210019 into praxis:master Aug 27, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants