From 3fbc6328439abf14f009ae4b6fbd6ea7157d4fda Mon Sep 17 00:00:00 2001
From: Iain Beeston <iain.beeston@gmail.com>
Date: Mon, 23 Feb 2015 20:33:04 +0000
Subject: Deprecate `AbstractController::Callbacks#skip_action_callback`

As part of #19029, in future `skip_before_action`, `skip_after_action` and
`skip_around_action` will raise an ArgumentError if the specified
callback does not exist. `skip_action_callback` calls all three of these
methods and will almost certainly result in an ArgumentError. If anyone
wants to remove all three callbacks then they can still call the three
individual methods. Therefore let's deprecate `skip_action_callback` now
and remove it when #19029 is merged.
---
 actionpack/CHANGELOG.md                         |  5 +++++
 actionpack/lib/abstract_controller/callbacks.rb |  1 +
 actionpack/test/controller/filters_test.rb      | 24 ++++++++++++++++++++++--
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index c5b208f5b6..e38f7405e0 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,8 @@
+*   Deprecate AbstractController#skip_action_callback in favor of individual skip_callback methods
+    (which can be made to raise an error if no callback was removed).
+
+    *Iain Beeston*
+
 *   Alias the `ActionDispatch::Request#uuid` method to `ActionDispatch::Request#request_id`.
     Due to implementation, `config.log_tags = [:request_id]` also works in substitute
     for `config.log_tags = [:uuid]`.
diff --git a/actionpack/lib/abstract_controller/callbacks.rb b/actionpack/lib/abstract_controller/callbacks.rb
index 69f490b327..f4fd1db36c 100644
--- a/actionpack/lib/abstract_controller/callbacks.rb
+++ b/actionpack/lib/abstract_controller/callbacks.rb
@@ -63,6 +63,7 @@ module AbstractController
       #   impossible to skip a callback defined using an anonymous proc
       #   using #skip_action_callback
       def skip_action_callback(*names)
+        ActiveSupport::Deprecation.warn('`skip_action_callback` is deprecated and will be removed in the next major version of Rails. Please use skip_before_action, skip_after_action or skip_around_action instead.')
         skip_before_action(*names)
         skip_after_action(*names)
         skip_around_action(*names)
diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb
index b9fb6be4e3..94faaeec69 100644
--- a/actionpack/test/controller/filters_test.rb
+++ b/actionpack/test/controller/filters_test.rb
@@ -967,8 +967,15 @@ class ControllerWithAllTypesOfFilters < PostsController
 end
 
 class ControllerWithTwoLessFilters < ControllerWithAllTypesOfFilters
-  skip_action_callback :around_again
-  skip_action_callback :after
+  skip_around_action :around_again
+  skip_after_action :after
+end
+
+class SkipFilterUsingSkipActionCallback < ControllerWithAllTypesOfFilters
+  ActiveSupport::Deprecation.silence do
+    skip_action_callback :around_again
+    skip_action_callback :after
+  end
 end
 
 class YieldingAroundFiltersTest < ActionController::TestCase
@@ -1055,6 +1062,19 @@ class YieldingAroundFiltersTest < ActionController::TestCase
     assert_equal 3, controller.instance_variable_get(:@try)
   end
 
+  def test_skipping_with_skip_action_callback
+    test_process(SkipFilterUsingSkipActionCallback,'no_raise')
+    assert_equal 'before around (before yield) around (after yield)', assigns['ran_filter'].join(' ')
+  end
+
+  def test_deprecated_skip_action_callback
+    assert_deprecated do
+      Class.new(TestController) do
+        skip_action_callback :clean_up
+      end
+    end
+  end
+
   protected
     def test_process(controller, action = "show")
       @controller = controller.is_a?(Class) ? controller.new : controller
-- 
cgit v1.2.3