Deprecate passing hash as first parameter into ActionController::Head

5 files changed, 47 insertions, 12 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 05cbe472e0..cb5e7516fb 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,7 @@
+*   Deprecate passing first parameter as `Hash` and default status code for `head` method.
+
+    *Mehmet Emin İNAÇ*
+
 *   Adds`Rack::Utils::ParameterTypeError` and `Rack::Utils::InvalidParameterError`
     to the rescue_responses hash in `ExceptionWrapper` (Rack recommends
     integrators serve 400s for both of these).
diff --git a/actionpack/lib/action_controller/metal/head.rb b/actionpack/lib/action_controller/metal/head.rb
index 70f42bf565..f445094bdc 100644
--- a/actionpack/lib/action_controller/metal/head.rb
+++ b/actionpack/lib/action_controller/metal/head.rb
@@ -17,8 +17,18 @@ module ActionController
     #
     # See Rack::Utils::SYMBOL_TO_STATUS_CODE for a full list of valid +status+ symbols.
     def head(status, options = {})
-      options, status = status, nil if status.is_a?(Hash)
-      status ||= options.delete(:status) || :ok
+      if status.is_a?(Hash)
+        msg = status[:status] ? 'The :status option' : 'The implicit :ok status'
+        options, status = status, status.delete(:status)
+
+        ActiveSupport::Deprecation.warn(<<-MSG.squish)
+          #{msg} on `head` has been deprecated and will be removed in Rails 5.1.
+          Please pass the status as a separate parameter before the options, instead.
+        MSG
+      end
+
+      status ||= :ok
+      
       location = options.delete(:location)
       content_type = options.delete(:content_type)
 
diff --git a/actionpack/test/controller/log_subscriber_test.rb b/actionpack/test/controller/log_subscriber_test.rb
index ccbf336acf..7835d2768a 100644
--- a/actionpack/test/controller/log_subscriber_test.rb
+++ b/actionpack/test/controller/log_subscriber_test.rb
@@ -10,7 +10,7 @@ module Another
     end
 
     rescue_from SpecialException do
-      head :status => 406
+      head 406
     end
 
     before_action :redirector, only: :never_executed
diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb
index c9c43de37d..43e992d432 100644
--- a/actionpack/test/controller/render_test.rb
+++ b/actionpack/test/controller/render_test.rb
@@ -138,6 +138,14 @@ class TestController < ActionController::Base
     fresh_when(:last_modified => Time.now.utc.beginning_of_day, :etag => [ :foo, 123 ])
   end
 
+  def head_with_status_hash
+    head status: :created
+  end
+
+  def head_with_hash_does_not_include_status
+    head warning: :deprecated
+  end
+
   def head_created
     head :created
   end
@@ -151,31 +159,31 @@ class TestController < ActionController::Base
   end
 
   def head_with_location_header
-    head :location => "/foo"
+    head :ok, :location => "/foo"
   end
 
   def head_with_location_object
-    head :location => Customer.new("david", 1)
+    head :ok, :location => Customer.new("david", 1)
   end
 
   def head_with_symbolic_status
-    head :status => params[:status].intern
+    head params[:status].intern
   end
 
   def head_with_integer_status
-    head :status => params[:status].to_i
+    head params[:status].to_i
   end
 
   def head_with_string_status
-    head :status => params[:status]
+    head params[:status]
   end
 
   def head_with_custom_header
-    head :x_custom_header => "something"
+    head :ok, :x_custom_header => "something"
   end
 
   def head_with_www_authenticate_header
-    head 'WWW-Authenticate' => 'something'
+    head :ok, 'WWW-Authenticate' => 'something'
   end
 
   def head_with_status_code_first
@@ -490,6 +498,19 @@ class HeadRenderTest < ActionController::TestCase
     assert_response :created
   end
 
+  def test_passing_hash_to_head_as_first_parameter_deprecated
+    assert_deprecated do
+      get :head_with_status_hash
+    end
+  end
+
+  def test_head_with_default_value_is_deprecated
+    assert_deprecated do
+      get :head_with_hash_does_not_include_status
+      assert_response :ok
+    end
+  end
+
   def test_head_created_with_application_json_content_type
     post :head_created_with_application_json_content_type
     assert @response.body.blank?
diff --git a/actionpack/test/controller/rescue_test.rb b/actionpack/test/controller/rescue_test.rb
index 4898b0c57f..2cfd0b8023 100644
--- a/actionpack/test/controller/rescue_test.rb
+++ b/actionpack/test/controller/rescue_test.rb
@@ -47,10 +47,10 @@ class RescueController < ActionController::Base
   rescue_from 'InvalidRequestToRescueAsString', :with => proc { |exception| render :text => exception.message }
 
   rescue_from BadGateway do
-    head :status => 502
+    head 502
   end
   rescue_from 'BadGatewayToRescueAsString' do
-    head :status => 502
+    head 502
   end
 
   rescue_from ResourceUnavailable do |exception|