Skip to content

Loading…

nil param value that expects a hash should return :key => nil #20

Closed
wants to merge 4 commits into from

5 participants

@bjhess

When a nil param value arrives in place of an expected hash, return :key => nil, not simply nil.

Prior:

(rdb:1) params
{"book"=>nil}
(rdb:1) params.permit(book: { genre: :type })
nil

Now:

(rdb:1) params
{"book"=>nil}
(rdb:1) params.permit(book: { genre: :type })
{"book"=>nil}
bjhess added some commits
@bjhess bjhess When a nil param value arrives in place of an expected hash, return
:key => nil, not simply nil.

Prior:

(rdb:1) params
{"book"=>nil}
(rdb:1) params.permit(book: { genre: :type })
nil

Now:

(rdb:1) params
{"book"=>nil}
(rdb:1) params.permit(book: { genre: :type })
{"book"=>nil}
7e9c635
@bjhess bjhess When a nil param value arrives in place of an expected hash, return
:key => nil, not simply nil.

Prior:

(rdb:1) params
{"book"=>nil}
(rdb:1) params.permit(book: { genre: :type })
nil

Now:

(rdb:1) params
{"book"=>nil}
(rdb:1) params.permit(book: { genre: :type })
{"book"=>nil}
6d02002
@bjhess bjhess Merge branch 'master' of github.com:bjhess/strong_parameters d9e9ac7
@bjhess bjhess Classic Ruby hash syntax for max compatibility. 8368288
@dhh
Ruby on Rails member
dhh commented

Please update to master and I'll merge.

@chikamichi

Any updates on this issue? The proposed behaviour is really convenient.

@carlosantoniodasilva carlosantoniodasilva commented on the diff
Gemfile.lock
((30 lines not shown))
builder (~> 3.0.0)
- activesupport (3.2.2)
+ activesupport (3.2.3)
@carlosantoniodasilva Ruby on Rails member

There's no need to change the lock file, please revert this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@carlosantoniodasilva carlosantoniodasilva commented on the diff
test/nested_parameters_test.rb
@@ -77,4 +77,12 @@ class NestedParametersTest < ActiveSupport::TestCase
permitted = params.permit book: { genre: :type }
assert_nil permitted[:book][:genre]
end
+
+ test "nil param value that should be a hash" do
+ params = ActionController::Parameters.new({
+ :book => nil
+ })
+ permitted = params.permit :book => { :genre => :type}
@carlosantoniodasilva Ruby on Rails member

Missing space before }.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@carlosantoniodasilva carlosantoniodasilva commented on the diff
test/nested_parameters_test.rb
@@ -77,4 +77,12 @@ class NestedParametersTest < ActiveSupport::TestCase
permitted = params.permit book: { genre: :type }
assert_nil permitted[:book][:genre]
end
+
+ test "nil param value that should be a hash" do
+ params = ActionController::Parameters.new({
+ :book => nil
+ })
+ permitted = params.permit :book => { :genre => :type}
+ assert_nil permitted[:book]
@carlosantoniodasilva Ruby on Rails member

I think you should also assert that the key is there, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@carlosantoniodasilva
Ruby on Rails member

It also needs another rebase, github says it cannot be merged anymore.

@kiela

@bjhess could you finish this PR?
It would be nice to have this merged in master

@bjhess

Sorry, folks, I didn't even realize there was any discussion on this PR until @kiela commented. I've put together a new PR at #151. Closing this one.

@bjhess bjhess closed this
@bjhess

(Oh, and thanks for the feedback @carlosantoniodasilva. I made use of it in the updated PR.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 9, 2012
  1. @bjhess

    When a nil param value arrives in place of an expected hash, return

    bjhess committed
    :key => nil, not simply nil.
    
    Prior:
    
    (rdb:1) params
    {"book"=>nil}
    (rdb:1) params.permit(book: { genre: :type })
    nil
    
    Now:
    
    (rdb:1) params
    {"book"=>nil}
    (rdb:1) params.permit(book: { genre: :type })
    {"book"=>nil}
  2. @bjhess

    When a nil param value arrives in place of an expected hash, return

    bjhess committed
    :key => nil, not simply nil.
    
    Prior:
    
    (rdb:1) params
    {"book"=>nil}
    (rdb:1) params.permit(book: { genre: :type })
    nil
    
    Now:
    
    (rdb:1) params
    {"book"=>nil}
    (rdb:1) params.permit(book: { genre: :type })
    {"book"=>nil}
Commits on Apr 17, 2012
  1. @bjhess
Commits on May 14, 2012
  1. @bjhess
Showing with 16 additions and 10 deletions.
  1. +8 −8 Gemfile.lock
  2. +0 −2 lib/action_controller/parameters.rb
  3. +8 −0 test/nested_parameters_test.rb
View
16 Gemfile.lock
@@ -1,27 +1,27 @@
PATH
remote: .
specs:
- strong_parameters (0.1.2)
+ strong_parameters (0.1.3)
actionpack (>= 3.2.0)
activemodel (>= 3.2.0)
GEM
remote: http://rubygems.org/
specs:
- actionpack (3.2.2)
- activemodel (= 3.2.2)
- activesupport (= 3.2.2)
+ actionpack (3.2.3)
+ activemodel (= 3.2.3)
+ activesupport (= 3.2.3)
builder (~> 3.0.0)
erubis (~> 2.7.0)
journey (~> 1.0.1)
rack (~> 1.4.0)
- rack-cache (~> 1.1)
+ rack-cache (~> 1.2)
rack-test (~> 0.6.1)
sprockets (~> 2.1.2)
- activemodel (3.2.2)
- activesupport (= 3.2.2)
+ activemodel (3.2.3)
+ activesupport (= 3.2.3)
builder (~> 3.0.0)
- activesupport (3.2.2)
+ activesupport (3.2.3)
@carlosantoniodasilva Ruby on Rails member

There's no need to change the lock file, please revert this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
i18n (~> 0.6)
multi_json (~> 1.0)
builder (3.0.0)
View
2 lib/action_controller/parameters.rb
@@ -41,8 +41,6 @@ def permit(*filters)
params[filter] = self[filter] if has_key?(filter)
when Hash then
self.slice(*filter.keys).each do |key, value|
- return unless value
-
key = key.to_sym
params[key] = each_element(value) do |value|
View
8 test/nested_parameters_test.rb
@@ -77,4 +77,12 @@ class NestedParametersTest < ActiveSupport::TestCase
permitted = params.permit book: { genre: :type }
assert_nil permitted[:book][:genre]
end
+
+ test "nil param value that should be a hash" do
+ params = ActionController::Parameters.new({
+ :book => nil
+ })
+ permitted = params.permit :book => { :genre => :type}
@carlosantoniodasilva Ruby on Rails member

Missing space before }.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ assert_nil permitted[:book]
@carlosantoniodasilva Ruby on Rails member

I think you should also assert that the key is there, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ end
end
Something went wrong with that request. Please try again.