diff options
15 files changed, 284 insertions, 2 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 79f6320a04..4109ae7006 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,12 @@ +* Introduce `ActionDispatch::ActionableExceptions`. + + The `ActionDispatch::ActionableExceptions` middleware dispatches actions + from `ActiveSupport::ActionableError` descendants. + + Actionable errors let's you dispatch actions from Rails' error pages. + + *Vipul A M*, *Yao Jie*, *Genadi Samokovarov* + * Raise an `ArgumentError` if a resource custom param contains a colon (`:`). After this change it's not possible anymore to configure routes like this: diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 8f39b88d56..6a4ba9af4a 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -53,6 +53,7 @@ module ActionDispatch autoload :RequestId autoload :Callbacks autoload :Cookies + autoload :ActionableExceptions autoload :DebugExceptions autoload :DebugLocks autoload :DebugView diff --git a/actionpack/lib/action_dispatch/middleware/actionable_exceptions.rb b/actionpack/lib/action_dispatch/middleware/actionable_exceptions.rb new file mode 100644 index 0000000000..f76ea7c2e2 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/actionable_exceptions.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require "erb" +require "action_dispatch/http/request" +require "active_support/actionable_error" + +module ActionDispatch + class ActionableExceptions # :nodoc: + cattr_accessor :endpoint, default: "/rails/actions" + + def initialize(app) + @app = app + end + + def call(env) + request = ActionDispatch::Request.new(env) + return @app.call(env) unless actionable_request?(request) + + ActiveSupport::ActionableError.dispatch(request.params["error"], request.params["action"]) + + redirect_to request.params["location"] + end + + private + def actionable_request?(request) + request.post? && request.path == endpoint + end + + def redirect_to(location) + body = "<html><body>You are being <a href=\"#{ERB::Util.unwrapped_html_escape(location)}\">redirected</a>.</body></html>" + + [302, { + "Content-Type" => "text/html; charset=#{Response.default_charset}", + "Content-Length" => body.bytesize.to_s, + "Location" => location, + }, [body]] + end + end +end diff --git a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb index 59113e13f4..a679f9bf01 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_exceptions.rb @@ -4,6 +4,8 @@ require "action_dispatch/http/request" require "action_dispatch/middleware/exception_wrapper" require "action_dispatch/routing/inspector" +require "active_support/actionable_error" + require "action_view" require "action_view/base" @@ -19,7 +21,7 @@ module ActionDispatch end def initialize(app, routes_app = nil, response_format = :default, interceptors = self.class.interceptors) - @app = app + @app = ActionableExceptions.new(app) @routes_app = routes_app @response_format = response_format @interceptors = interceptors diff --git a/actionpack/lib/action_dispatch/middleware/debug_view.rb b/actionpack/lib/action_dispatch/middleware/debug_view.rb index 43c0a84504..a03650254e 100644 --- a/actionpack/lib/action_dispatch/middleware/debug_view.rb +++ b/actionpack/lib/action_dispatch/middleware/debug_view.rb @@ -52,5 +52,9 @@ module ActionDispatch super end end + + def protect_against_forgery? + false + end end end diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_actions.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_actions.html.erb new file mode 100644 index 0000000000..e181b6c539 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_actions.html.erb @@ -0,0 +1,15 @@ +<% if ActiveSupport::ActionableError === exception %> + <% actions = ActiveSupport::ActionableError.actions(exception) %> + + <% if actions.any? %> + <div class="actions"> + <% actions.each do |action, _| %> + <%= button_to action, ActionDispatch::ActionableExceptions.endpoint, params: { + error: exception.class.name, + action: action, + location: request.path + } %> + <% end %> + </div> + <% end %> +<% end %> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/_actions.text.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/_actions.text.erb new file mode 100644 index 0000000000..e69de29bb2 --- /dev/null +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/_actions.text.erb diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb index bde26f46c2..999e84e4d6 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/diagnostics.html.erb @@ -8,7 +8,11 @@ </header> <div id="container"> - <h2><%= h @exception.message %></h2> + <h2> + <%= h @exception.message %> + + <%= render "rescues/actions", exception: @exception, request: @request %> + </h2> <%= render "rescues/source", source_extracts: @source_extracts, show_source_idx: @show_source_idx, error_index: 0 %> <%= render "rescues/trace", traces: @traces, trace_to_show: @trace_to_show, error_index: 0 %> diff --git a/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb index 39ea25bdfc..0f78e23b7f 100644 --- a/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb +++ b/actionpack/lib/action_dispatch/middleware/templates/rescues/layout.erb @@ -117,6 +117,10 @@ background-color: #FFCCCC; } + .button_to { + display: inline-block; + } + .hidden { display: none; } diff --git a/actionpack/test/dispatch/actionable_exceptions_test.rb b/actionpack/test/dispatch/actionable_exceptions_test.rb new file mode 100644 index 0000000000..46979ed878 --- /dev/null +++ b/actionpack/test/dispatch/actionable_exceptions_test.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require "abstract_unit" + +class ActionableExceptionsTest < ActionDispatch::IntegrationTest + Actions = [] + + class ActionError < StandardError + include ActiveSupport::ActionableError + + action "Successful action" do + Actions << "Action!" + end + + action "Failed action" do + raise "Inaction!" + end + end + + Noop = -> env { [200, {}, [""]] } + + setup do + @app = ActionDispatch::ActionableExceptions.new(Noop) + + Actions.clear + end + + test "dispatches an actionable error" do + post ActionDispatch::ActionableExceptions.endpoint, params: { + error: ActionError.name, + action: "Successful action", + location: "/", + } + + assert_equal ["Action!"], Actions + + assert_equal 302, response.status + assert_equal "/", response.headers["Location"] + end + + test "dispatched action can fail" do + assert_raise RuntimeError do + post ActionDispatch::ActionableExceptions.endpoint, params: { + error: ActionError.name, + action: "Failed action", + location: "/", + } + end + end + + test "cannot dispatch non-actionable errors" do + assert_raise ActiveSupport::ActionableError::NonActionable do + post ActionDispatch::ActionableExceptions.endpoint, params: { + error: RuntimeError.name, + action: "Inexistent action", + location: "/", + } + end + end +end diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index ed0c6d48b8..e27c8ba4c8 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -4,6 +4,7 @@ require "benchmark" require "set" require "zlib" require "active_support/core_ext/module/attribute_accessors" +require "active_support/actionable_error" module ActiveRecord class MigrationError < ActiveRecordError #:nodoc: diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 301b0c8822..4c7b134c35 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,35 @@ +* Introduce `ActiveSupport::ActionableError`. + + Actionable errors let's you dispatch actions from Rails' error pages. This + can help you save time if you have a clear action for the resolution of + common development errors. + + The de-facto example are pending migrations. Every time pending migrations + are found, a middleware raises an error. With actionable errors, you can + run the migrations right from the error page. Other examples include Rails + plugins that need to run a rake task to setup themselves. They can now + raise actionable errors to run the setup straight from the error pages. + + Here is how to define an actionable error: + + ```ruby + class PendingMigrationError < MigrationError #:nodoc: + include ActiveSupport::ActionableError + + action "Run pending migrations" do + ActiveRecord::Tasks::DatabaseTasks.migrate + end + end + ``` + + To make an error actionable, include the `ActiveSupport::ActionableError` + module and invoke the `action` class macro to define the action. An action + needs a name and a procedure to execute. The name is shown as the name of a + button on the error pages. Once clicked, it will invoke the given + procedure. + + *Vipul A M*, *Yao Jie*, *Genadi Samokovarov* + * Preserve `html_safe?` status on `ActiveSupport::SafeBuffer#*`. Before: diff --git a/activesupport/lib/active_support.rb b/activesupport/lib/active_support.rb index 5589c71281..40a9181493 100644 --- a/activesupport/lib/active_support.rb +++ b/activesupport/lib/active_support.rb @@ -33,6 +33,7 @@ require "active_support/core_ext/date_and_time/compatibility" module ActiveSupport extend ActiveSupport::Autoload + autoload :ActionableError autoload :Concern autoload :CurrentAttributes autoload :Dependencies diff --git a/activesupport/lib/active_support/actionable_error.rb b/activesupport/lib/active_support/actionable_error.rb new file mode 100644 index 0000000000..aeee2177aa --- /dev/null +++ b/activesupport/lib/active_support/actionable_error.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require "active_support/concern" + +module ActiveSupport + # Actionable errors let's you define actions to resolve an error. + # + # To make an error actionable, include the <tt>ActiveSupport::ActionableError</tt> + # module and invoke the +action+ class macro to define the action. + # + # An action needs a name and a procedure to execute. The name can be shown by + # the action dispatching mechanism. + module ActionableError + extend Concern + + NonActionable = Class.new(StandardError) + + included do + class_attribute :_actions, default: Hash.new do |_, label| + raise NonActionable, "Cannot find action \"#{label}\" for #{self}" + end + end + + def self.===(other) # :nodoc: + super || Module === other && other.ancestors.include?(self) + end + + def self.actions(error) # :nodoc: + error = error.constantize if String === error + raise NonActionable, "#{error.name} is non-actionable" unless self === error + error._actions + end + + def self.dispatch(error, label) # :nodoc: + actions(error)[label].call + end + + module ClassMethods + # Defines an action that can resolve the error. + # + # class PendingMigrationError < MigrationError + # include ActiveSupport::ActionableError + # + # action "Run pending migrations" do + # ActiveRecord::Tasks::DatabaseTasks.migrate + # end + # end + def action(label, &block) + _actions[label] = block + end + end + end +end diff --git a/activesupport/test/actionable_error_test.rb b/activesupport/test/actionable_error_test.rb new file mode 100644 index 0000000000..66ba94e0dd --- /dev/null +++ b/activesupport/test/actionable_error_test.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +require "abstract_unit" +require "active_support/actionable_error" + +class ActionableErrorTest < ActiveSupport::TestCase + class NonActionableError < StandardError + end + + class DispatchableError < StandardError + include ActiveSupport::ActionableError + + class_attribute :flip1, default: false + class_attribute :flip2, default: false + + action "Flip 1" do + self.flip1 = true + end + + action "Flip 2" do + self.flip2 = true + end + end + + test "can get all action of an actionable error" do + assert_equal ["Flip 1", "Flip 2"], ActiveSupport::ActionableError.actions(DispatchableError).keys + assert_equal ["Flip 1", "Flip 2"], ActiveSupport::ActionableError.actions(DispatchableError.new).keys + end + + test "cannot get actions from non-actionable errors" do + assert_raises ActiveSupport::ActionableError::NonActionable do + ActiveSupport::ActionableError.actions(NonActionableError) + end + end + + test "dispatches actions from class and a label" do + assert_changes "DispatchableError.flip1", from: false, to: true do + ActiveSupport::ActionableError.dispatch DispatchableError, "Flip 1" + end + end + + test "dispatches actions from class name and a label" do + assert_changes "DispatchableError.flip2", from: false, to: true do + ActiveSupport::ActionableError.dispatch DispatchableError.name, "Flip 2" + end + end + + test "cannot dispatch errors that do not include ActiveSupport::ActionableError" do + err = assert_raises ActiveSupport::ActionableError::NonActionable do + ActiveSupport::ActionableError.dispatch NonActionableError, "action" + end + + assert_equal <<~EXPECTED.chop, err.to_s + ActionableErrorTest::NonActionableError is non-actionable + EXPECTED + end +end |