-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Calling declared(params) from child namespace fails to include parent namespace defined params #503
Conversation
last_response.status.should == 200 | ||
end | ||
end | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is certainly a good way to write a test, but generally I'm trying to put test conditions in the test itself. Mind rewriting it with say something like this:
get do
{
params: params,
declared_params: declared(params)
}.as_json
end
...
get ...
json = JSON.parse(last_response)
json[:params][:id].should == 123
json[:declared_params].keys.should include :foo # rather than == true, or at least use be_true
Thanks.
And yes, this totally makes sense. |
Done. Will also write some documentation for |
Test looks good. There's a rubocop violation (trailing space), so Travis didn't run this (failing) test, fyi. All you have to do now is fix this behavior :) |
Sorry, that's fixed now too. Will take a look at fixing |
Btw, if I were to nitpick on the test I would change it to be in JSON format, therefore avoiding having to explicitly call before do
subject.format :json
subject.namespace :something do
resource ':id' do
...
get do
{
params: params,
declared_params: declared(params)
}
end
end
end
end |
No, that's fair enough. Worth going to the effort of ensuring new tests set and follow best practice! |
@dblock - this looks to be the correct way to fix the bug, especially given this piece of code that gathers the validations in the same way. Thoughts? |
Looks good, I'll merge squashed. |
Merged via 535fd7d. Thanks. |
Changes Unknown when pulling 739c131 on myitcv:nested_declared_usage into * on intridea:master*. |
The test in this PR highlights the problem as described in the subject.
The docs seem to suggest that a params block defined against
resource
in this way should apply to every method within the resource(/namespace)Hence the expectation that a call to
declared(params)
should include the param defined against the resource.Does this make sense? Hopefully the test does!