Skip to content

Commit

Permalink
Merge pull request #333 from orgsync/gh-87-symbolic
Browse files Browse the repository at this point in the history
Copy symbolic errors when composing
  • Loading branch information
tfausak committed Jan 10, 2016
2 parents 4485767 + f0431a8 commit 40e0b9f
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 15 deletions.
40 changes: 40 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
8 changes: 1 addition & 7 deletions lib/active_interaction/concerns/runnable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
22 changes: 21 additions & 1 deletion lib/active_interaction/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
14 changes: 7 additions & 7 deletions spec/active_interaction/base_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -337,19 +337,19 @@ 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
expect(outcome).to be_valid
end

it 'returns the sum' do
expect(result).to eql x + y
expect(result).to eql x + z
end
end

Expand All @@ -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
Expand Down

0 comments on commit 40e0b9f

Please sign in to comment.