From feaaa7576a0774471e36dc59730a886f623712e6 Mon Sep 17 00:00:00 2001
From: Genadi Samokovarov <gsamokovarov@gmail.com>
Date: Fri, 19 Apr 2019 13:49:56 +0900
Subject: Refactor after the most recent code review

---
 .../action_dispatch/middleware/actionable_exceptions.rb  |  4 ++--
 actionpack/test/dispatch/actionable_exceptions_test.rb   | 10 ++++++++++
 activesupport/lib/active_support.rb                      |  2 +-
 activesupport/lib/active_support/actionable_error.rb     | 16 ++++++----------
 activesupport/test/actionable_error_test.rb              |  6 ------
 5 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/actionpack/lib/action_dispatch/middleware/actionable_exceptions.rb b/actionpack/lib/action_dispatch/middleware/actionable_exceptions.rb
index 059fcdcfd4..e94cc46603 100644
--- a/actionpack/lib/action_dispatch/middleware/actionable_exceptions.rb
+++ b/actionpack/lib/action_dispatch/middleware/actionable_exceptions.rb
@@ -16,9 +16,9 @@ module ActionDispatch
       request = ActionDispatch::Request.new(env)
       return @app.call(env) unless actionable_request?(request)
 
-      ActiveSupport::ActionableError.dispatch(request.params["error"], request.params["action"])
+      ActiveSupport::ActionableError.dispatch(request.params[:error].to_s.safe_constantize, request.params[:action])
 
-      redirect_to request.params["location"]
+      redirect_to request.params[:location]
     end
 
     private
diff --git a/actionpack/test/dispatch/actionable_exceptions_test.rb b/actionpack/test/dispatch/actionable_exceptions_test.rb
index ae382e201e..9215a91e9c 100644
--- a/actionpack/test/dispatch/actionable_exceptions_test.rb
+++ b/actionpack/test/dispatch/actionable_exceptions_test.rb
@@ -67,4 +67,14 @@ class ActionableExceptionsTest < ActionDispatch::IntegrationTest
       }
     end
   end
+
+  test "cannot dispatch Inexistent errors" do
+    assert_raise ActiveSupport::ActionableError::NonActionable do
+      post ActionDispatch::ActionableExceptions.endpoint, params: {
+        error: "",
+        action: "Inexistent action",
+        location: "/",
+      }
+    end
+  end
 end
diff --git a/activesupport/lib/active_support.rb b/activesupport/lib/active_support.rb
index 40a9181493..9e242ddeaa 100644
--- a/activesupport/lib/active_support.rb
+++ b/activesupport/lib/active_support.rb
@@ -33,8 +33,8 @@ require "active_support/core_ext/date_and_time/compatibility"
 module ActiveSupport
   extend ActiveSupport::Autoload
 
-  autoload :ActionableError
   autoload :Concern
+  autoload :ActionableError
   autoload :CurrentAttributes
   autoload :Dependencies
   autoload :DescendantsTracker
diff --git a/activesupport/lib/active_support/actionable_error.rb b/activesupport/lib/active_support/actionable_error.rb
index 8adf40cc3d..7db14cd178 100644
--- a/activesupport/lib/active_support/actionable_error.rb
+++ b/activesupport/lib/active_support/actionable_error.rb
@@ -1,7 +1,5 @@
 # frozen_string_literal: true
 
-require "active_support/concern"
-
 module ActiveSupport
   # Actionable errors let's you define actions to resolve an error.
   #
@@ -11,14 +9,10 @@ module ActiveSupport
   module ActionableError
     extend Concern
 
-    NonActionable = Class.new(StandardError)
-
-    NoActions = Hash.new do |_, name| # :nodoc:
-      raise NonActionable, "Cannot find action \"#{name}\""
-    end
+    class NonActionable < StandardError; end
 
     included do
-      class_attribute :_actions, default: NoActions.dup
+      class_attribute :_actions, default: {}
     end
 
     def self.actions(error) # :nodoc:
@@ -26,12 +20,14 @@ module ActiveSupport
       when ActionableError, -> it { Class === it && it < ActionableError }
         error._actions
       else
-        NoActions
+        {}
       end
     end
 
     def self.dispatch(error, name) # :nodoc:
-      actions(error.is_a?(String) ? error.constantize : error)[name].call
+      actions(error).fetch(name).call
+    rescue KeyError
+      raise NonActionable, "Cannot find action \"#{name}\""
     end
 
     module ClassMethods
diff --git a/activesupport/test/actionable_error_test.rb b/activesupport/test/actionable_error_test.rb
index fb2abf2bcf..63046b937c 100644
--- a/activesupport/test/actionable_error_test.rb
+++ b/activesupport/test/actionable_error_test.rb
@@ -37,12 +37,6 @@ class ActionableErrorTest < ActiveSupport::TestCase
     end
   end
 
-  test "dispatches actions from error class as string and name" do
-    assert_changes "DispatchableError.flip2", from: false, to: true do
-      ActiveSupport::ActionableError.dispatch DispatchableError.name, "Flip 2"
-    end
-  end
-
   test "cannot dispatch missing actions" do
     err = assert_raises ActiveSupport::ActionableError::NonActionable do
       ActiveSupport::ActionableError.dispatch NonActionableError, "action"
-- 
cgit v1.2.3