diff --git a/README.md b/README.md index 69ef4e89..808f13fe 100644 --- a/README.md +++ b/README.md @@ -981,6 +981,9 @@ class AddAndDouble < ActiveInteraction::Base end ``` +Note that errors in composed interactions have a few tricky cases. See [the +errors section][] for more information about them. + ### Defaults The default value for an input can take on many different forms. Setting the @@ -1100,6 +1103,42 @@ class UpdateThing < ActiveInteraction::Base end ``` +When a composed interaction fails, its errors are merged onto the caller. This +generally produces good error messages, but there are a few cases to look out +for. + +``` rb +class Inner < ActiveInteraction::Base + boolean :x, :y +end + +class Outer < ActiveInteraction::Base + string :x + boolean :z, default: nil + + def execute + compose(Inner, x: x, y: z) + end +end + +outcome = Outer.run(x: 'yes') +outcome.errors.details +# => { :x => [{ :error => :invalid_type, :type => "boolean" }], +# :base => [{ :error => "Y is required" }] } +outcome.errors.full_messages.join(' and ') +# => "X is not a valid boolean and Y is required" +``` + +Since both interactions have an input called `x`, the inner error for that +input is moved to the `x` error on the outer interaction. This results in a +misleading error that claims the input `x` is not a valid boolean even though +it's a string on the outer interaction. + +Since only the inner interaction has an input called `y`, the inner error for +that input is moved to the `base` error on the outer interaction. This results +in a confusing error that claims the input `y` is required even though it's not +present on the outer interaction. + ### Forms The outcome returned by `.run` can be used in forms as though it were an @@ -1348,6 +1387,7 @@ ActiveInteraction is licensed under [the MIT License][]. [formtastic]: https://rubygems.org/gems/formtastic [simple_form]: https://rubygems.org/gems/simple_form [the filters section]: #filters +[the errors section]: #errors [the optional inputs section]: #optional-inputs [aire]: example [`with_options`]: http://api.rubyonrails.org/classes/Object.html#method-i-with_options diff --git a/lib/active_interaction/concerns/runnable.rb b/lib/active_interaction/concerns/runnable.rb index 04a3b475..fb457cd2 100644 --- a/lib/active_interaction/concerns/runnable.rb +++ b/lib/active_interaction/concerns/runnable.rb @@ -80,18 +80,12 @@ def run self.result = if result_or_errors.is_a?(ActiveInteraction::Errors) - merge_errors_onto_base(result_or_errors) + errors.merge!(result_or_errors) else result_or_errors end end - def merge_errors_onto_base(new_errors) - new_errors.full_messages.each do |message| - errors.add(:base, message) unless errors.added?(:base, message) - end - end - # @return [Object] # # @raise [InvalidInteractionError] If there are validation errors. diff --git a/lib/active_interaction/errors.rb b/lib/active_interaction/errors.rb index 150f476a..89889ae5 100644 --- a/lib/active_interaction/errors.rb +++ b/lib/active_interaction/errors.rb @@ -97,6 +97,10 @@ def merge!(other) def merge_messages!(other) other.messages.each do |attribute, messages| messages.each do |message| + unless attribute?(attribute) + message = full_message(attribute, message) + attribute = :base + end add(attribute, message) unless added?(attribute, message) end end @@ -107,9 +111,25 @@ def merge_details!(other) details.each do |detail| detail = detail.dup error = detail.delete(:error) - add(attribute, error, detail) unless added?(attribute, error, detail) + + merge_detail!(other, attribute, detail, error) end end end + + def merge_detail!(other, attribute, detail, error) + if attribute?(attribute) + add(attribute, error, detail) unless added?(attribute, error, detail) + else + message = full_message( + attribute, other.generate_message(attribute, error)) + attribute = :base + add(attribute, message) unless added?(attribute, message) + end + end + + def attribute?(attribute) + @base.respond_to?(attribute) + end end end diff --git a/spec/active_interaction/base_spec.rb b/spec/active_interaction/base_spec.rb index 1ffafe6e..c0ec9eed 100644 --- a/spec/active_interaction/base_spec.rb +++ b/spec/active_interaction/base_spec.rb @@ -19,12 +19,12 @@ def execute end InterruptInteraction = Class.new(TestInteraction) do - object :x, :y, + object :x, :z, class: Object, default: nil def execute - compose(AddInteraction, inputs) + compose(AddInteraction, x: x, y: z) end end @@ -337,11 +337,11 @@ def execute describe '#compose' do let(:described_class) { InterruptInteraction } let(:x) { rand } - let(:y) { rand } + let(:z) { rand } context 'with valid composition' do before do - inputs.merge!(x: x, y: y) + inputs.merge!(x: x, z: z) end it 'is valid' do @@ -349,7 +349,7 @@ def execute end it 'returns the sum' do - expect(result).to eql x + y + expect(result).to eql x + z end end @@ -359,8 +359,8 @@ def execute end it 'has the correct errors' do - expect(outcome.errors[:base]) - .to match_array ['X is required', 'Y is required'] + expect(outcome.errors.details) + .to eql(x: [{ error: :missing }], base: [{ error: 'Y is required' }]) end end end