Merge pull request #19060 from iainbeeston/deprecate-skip-action-callback

Deprecate `AbstractController::Callbacks#skip_action_callback`

3 files changed, 28 insertions, 2 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 8f36af94b4..8685cd6495 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