From fb0fae9747ceec065e75867ea83afb713a5b449b Mon Sep 17 00:00:00 2001 From: Eugene Kenny Date: Wed, 10 May 2017 23:48:46 +0100 Subject: [PATCH] Pass block in ActionController::Parameters#delete In order to fully support the same interface as `Hash#delete`, we need to pass the block through to the underlying method, not just the key. This used to work correctly, but it regressed when `ActionController::Parameters` stopped inheriting from `Hash` in 5.0. --- .../metal/strong_parameters.rb | 4 ++-- .../controller/parameters/mutators_test.rb | 21 +++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 7864f9decd879..20330b5091e6c 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -666,8 +666,8 @@ def transform_keys!(&block) # to key. If the key is not found, returns the default value. If the # optional code block is given and the key is not found, pass in the key # and return the result of block. - def delete(key) - convert_value_to_parameters(@parameters.delete(key)) + def delete(key, &block) + convert_value_to_parameters(@parameters.delete(key, &block)) end # Returns a new instance of ActionController::Parameters with only diff --git a/actionpack/test/controller/parameters/mutators_test.rb b/actionpack/test/controller/parameters/mutators_test.rb index e61bbdbe137a4..2c36f488c68fd 100644 --- a/actionpack/test/controller/parameters/mutators_test.rb +++ b/actionpack/test/controller/parameters/mutators_test.rb @@ -25,6 +25,27 @@ class ParametersMutatorsTest < ActiveSupport::TestCase assert_not @params.delete(:person).permitted? end + test "delete returns the value when the key is present" do + assert_equal "32", @params[:person].delete(:age) + end + + test "delete removes the entry when the key present" do + @params[:person].delete(:age) + assert_not @params[:person].key?(:age) + end + + test "delete returns nil when the key is not present" do + assert_equal nil, @params[:person].delete(:first_name) + end + + test "delete returns the value of the given block when the key is not present" do + assert_equal "David", @params[:person].delete(:first_name) { "David" } + end + + test "delete yields the key to the given block when the key is not present" do + assert_equal "first_name: David", @params[:person].delete(:first_name) { |k| "#{k}: David" } + end + test "delete_if retains permitted status" do @params.permit! assert @params.delete_if { |k| k == "person" }.permitted?