Fix failing test on several methods on Parameter

* `each`
* `each_pair`
* `delete`
* `select!`

3 files changed, 36 insertions, 3 deletions

diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb
index beffe9172b..7bfe370da4 100644
--- a/actionpack/lib/action_controller/metal/strong_parameters.rb
+++ b/actionpack/lib/action_controller/metal/strong_parameters.rb
@@ -160,6 +160,18 @@ module ActionController
       end
     end
 
+    # Convert all hashes in values into parameters, then yield each pair like
+    # the same way as <tt>Hash#each_pair</tt>
+    def each_pair(&block)
+      super do |key, value|
+        convert_hashes_to_parameters(key, value)
+      end
+
+      super
+    end
+
+    alias_method :each, :each_pair
+
     # Attribute that keeps track of converted arrays, if any, to avoid double
     # looping in the common use case permit + mass-assignment. Defined in a
     # method to instantiate it only if needed.
@@ -195,7 +207,6 @@ module ActionController
     #   Person.new(params) # => #<Person id: nil, name: "Francesco">
     def permit!
       each_pair do |key, value|
-        value = convert_hashes_to_parameters(key, value)
         Array.wrap(value).each do |v|
           v.permit! if v.respond_to? :permit!
         end
@@ -387,6 +398,19 @@ module ActionController
       end
     end
 
+    # Deletes and returns a key-value pair from +Parameters+ whose key is equal
+    # 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, &block)
+      convert_hashes_to_parameters(key, super, false)
+    end
+
+    # Equivalent to Hash#keep_if, but returns nil if no changes were made.
+    def select!(&block)
+      convert_value_to_parameters(super)
+    end
+
     # Returns an exact copy of the <tt>ActionController::Parameters</tt>
     # instance. +permitted+ state is kept on the duped object.
     #
diff --git a/actionpack/test/controller/parameters/accessors_test.rb b/actionpack/test/controller/parameters/accessors_test.rb
index 7f4e595330..97875c3cbb 100644
--- a/actionpack/test/controller/parameters/accessors_test.rb
+++ b/actionpack/test/controller/parameters/accessors_test.rb
@@ -36,6 +36,15 @@ class ParametersAccessorsTest < ActiveSupport::TestCase
     @params.each { |key, value| assert_not(value.permitted?) if key == "person" }
   end
 
+  test "each_pair carries permitted status" do
+    @params.permit!
+    @params.each_pair { |key, value| assert(value.permitted?) if key == "person" }
+  end
+
+  test "each_pair carries unpermitted status" do
+    @params.each_pair { |key, value| assert_not(value.permitted?) if key == "person" }
+  end
+
   test "except retains permitted status" do
     @params.permit!
     assert @params.except(:person).permitted?
diff --git a/actionpack/test/controller/parameters/mutators_test.rb b/actionpack/test/controller/parameters/mutators_test.rb
index 300dbfa61c..744d8664be 100644
--- a/actionpack/test/controller/parameters/mutators_test.rb
+++ b/actionpack/test/controller/parameters/mutators_test.rb
@@ -63,11 +63,11 @@ class ParametersMutatorsTest < ActiveSupport::TestCase
 
   test "select! retains permitted status" do
     @params.permit!
-    assert @params.select! { |k| k == "person" }.permitted?
+    assert @params.select! { |k| k != "person" }.permitted?
   end
 
   test "select! retains unpermitted status" do
-    assert_not @params.select! { |k| k == "person" }.permitted?
+    assert_not @params.select! { |k| k != "person" }.permitted?
   end
 
   test "slice! retains permitted status" do