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

Using #fetch with #permit isn't fully tested #899

Closed
wants to merge 2 commits into from
Closed

Using #fetch with #permit isn't fully tested #899

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
33f5454
Proper test case for #495
Feb 8, 2016
7ff6f12
Fix permit #fetch test case
Feb 10, 2016
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
21 changes: 10 additions & 11 deletions spec/unit/shoulda/matchers/action_controller/permit_matcher_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,16 @@ def params_with_conditional_require(params, *filters)
for(:show, verb: :get, params: { slug: 'foo' })
end

it 'works when #fetch is used instead of #require (issue #495)' do
define_controller_with_strong_parameters(action: :create) do
params.fetch(:order, {}).permit(:eta, :diner_id)
end

expect(controller).
to permit(:eta, :diner_id).
for(:create, params: { order: { foo: '' } })
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for this -- this is definitely a more realistic way to test this. You're using foo as a key just so the order hash is non-empty, is that correct?

Copy link
Author

Choose a reason for hiding this comment

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

Yeah.. Ideally that shouldn't need to happen for this, but I'm not sure why it's needed. I get why it's needed when using require, but not fetch.

end

it 'works with #update specifically' do
define_controller_with_strong_parameters(action: :update) do
params.permit(:name)
Expand All @@ -191,17 +201,6 @@ def params_with_conditional_require(params, *filters)
end

describe '#matches?' do
it 'does not raise an error when #fetch was used instead of #require (issue #495)' do
matcher = permit(:eta, :diner_id).for(:create)
matching = -> { matcher.matches?(controller) }

define_controller_with_strong_parameters(action: :create) do
params.fetch(:order, {}).permit(:eta, :diner_id)
end

expect(&matching).not_to raise_error
end

context 'stubbing params on the controller' do
it 'still allows the original params hash to be modified and accessed prior to the call to #require' do
actual_user_params = nil
Expand Down