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.

Sign up for GitHub

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

Jump to bottom

Calling declared(params) from child namespace fails to include parent namespace defined params #503

Closed
wants to merge 7 commits into from
Closed

Calling declared(params) from child namespace fails to include parent namespace defined params #503

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
673a751
Failing test to show problem
myitcv Nov 6, 2013
e843414
Move to @dblock preferred testing format
myitcv Nov 7, 2013
6281644
Ignore binstubs bin dir
myitcv Nov 7, 2013
cfe5eba
Fix rubocop whitespace violation
myitcv Nov 7, 2013
609212b
Minor fix to specify format of endpoint
myitcv Nov 7, 2013
beb25bb
Merge branch 'master' into nested_declared_usage
myitcv Dec 5, 2013
739c131
Proposed fix for #503
myitcv Dec 5, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Next Release
* [#495](https://github.com/intridea/grape/pull/495): Fix `ParamsScope#params` for parameters nested inside arrays - [@asross](https://github.com/asross).
* [#498](https://github.com/intridea/grape/pull/498): Dry up options and headers logic, allow headers to be passed to OPTIONS requests - [@karlfreeman](https://github.com/karlfreeman).
* [#500](https://github.com/intridea/grape/pull/500): Skip entity auto-detection when explicitely passed - [@yaneq](https://github.com/yaneq).
* [#503](https://github.com/intridea/grape/pull/503): Calling declared(params) from child namespace fails to include parent namespace defined params - [@myitcv](https://github.com/myitcv).
* [#512](https://github.com/intridea/grape/pull/512): Don't create `Grape::Request` multiple times - [@dblock](https://github.com/dblock).
* Your contribution here.

Expand Down
2 changes: 1 addition & 1 deletion lib/grape/endpoint.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ def call!(env)
#
# @param params [Hash] The initial hash to filter. Usually this will just be `params`
# @param options [Hash] Can pass `:include_missing` and `:stringify` options.
def declared(params, options = {}, declared_params = settings[:declared_params])
def declared(params, options = {}, declared_params = settings.gather(:declared_params))
options[:include_missing] = true unless options.key?(:include_missing)

unless declared_params
Expand Down
31 changes: 31 additions & 0 deletions spec/grape/endpoint_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,37 @@ def app
end
end

describe '#declared; call from child namespace' do
before do
subject.format :json
subject.namespace :something do
params do
requires :id, type: Integer
end
resource ':id' do
params do
requires :foo
optional :bar
end
get do
{
params: params,
declared_params: declared(params)
}
end
end
end
end

it 'should include params defined in the parent namespace' do
get '/something/123', foo: 'test', extra: 'hello'
expect(last_response.status).to eq 200
json = JSON.parse(last_response.body, symbolize_names: true)
expect(json[:params][:id]).to eq 123
expect(json[:declared_params].keys).to include :foo, :bar, :id
end
end

Copy link
Member

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.

describe '#params' do
it 'is available to the caller' do
subject.get('/hey') do
Expand Down