From 6a924d6b1f5afc6349fd8507cc3e59b752113eab Mon Sep 17 00:00:00 2001 From: Dathan Bennett Date: Wed, 12 Oct 2016 13:21:11 -0700 Subject: [PATCH 1/2] Allow multiple transitions in a single event with the same to and from states As reported in https://github.com/aasm/aasm/issues/372 and https://github.com/aasm/aasm/issues/362, if two transitions in the same event have the same `to` and `from` states but differ in their guards, if any transition is valid, the first transition is the one that's always executed. This attempts to resolve that issue. --- lib/aasm/core/event.rb | 7 +++-- ...e_transitions_that_differ_only_by_guard.rb | 31 +++++++++++++++++++ ...nsitions_that_differ_only_by_guard_spec.rb | 9 ++++++ 3 files changed, 44 insertions(+), 3 deletions(-) create mode 100644 spec/models/multiple_transitions_that_differ_only_by_guard.rb create mode 100644 spec/unit/multiple_transitions_that_differ_only_by_guard_spec.rb diff --git a/lib/aasm/core/event.rb b/lib/aasm/core/event.rb index 3b1c8a5..889c4ca 100644 --- a/lib/aasm/core/event.rb +++ b/lib/aasm/core/event.rb @@ -128,12 +128,13 @@ module AASM::Core transitions.each do |transition| next if to_state and !Array(transition.to).include?(to_state) - if (options.key?(:may_fire) && Array(transition.to).include?(options[:may_fire])) || + if (options.key?(:may_fire) && transition.eql?(options[:may_fire])) || (!options.key?(:may_fire) && transition.allowed?(obj, *args)) - result = to_state || Array(transition.to).first + if options[:test_only] - # result = true + result = transition else + result = to_state || Array(transition.to).first Array(transition.to).each {|to| @valid_transitions[to] = transition } transition.execute(obj, *args) end diff --git a/spec/models/multiple_transitions_that_differ_only_by_guard.rb b/spec/models/multiple_transitions_that_differ_only_by_guard.rb new file mode 100644 index 0000000..4924408 --- /dev/null +++ b/spec/models/multiple_transitions_that_differ_only_by_guard.rb @@ -0,0 +1,31 @@ +class MultipleTransitionsThatDifferOnlyByGuard + include AASM + + attr_accessor :executed_correctly + + aasm do + state :start, :initial => true + state :gone + + event :go do + transitions :from => :start, :to => :gone, :guard => :returns_false, :after => :this_should_not_execute + transitions :from => :start, :to => :gone, :guard => :returns_true, :after => :this_should_execute + end + end + + def returns_false + false + end + + def returns_true + true + end + + def this_should_not_execute + raise RuntimeError, "This should not execute" + end + + def this_should_execute + self.executed_correctly = true + end +end \ No newline at end of file diff --git a/spec/unit/multiple_transitions_that_differ_only_by_guard_spec.rb b/spec/unit/multiple_transitions_that_differ_only_by_guard_spec.rb new file mode 100644 index 0000000..266373c --- /dev/null +++ b/spec/unit/multiple_transitions_that_differ_only_by_guard_spec.rb @@ -0,0 +1,9 @@ +require 'spec_helper' + +describe "multiple transitions that differ only by guard" do + let(:job) { MultipleTransitionsThatDifferOnlyByGuard.new } + + it 'does not follow the first transition if its guard fails' do + expect{job.go}.not_to raise_error + end +end From ce8c620a460da27431a5fd13ad84f72b42e26fd2 Mon Sep 17 00:00:00 2001 From: Dathan Bennett Date: Wed, 12 Oct 2016 13:56:00 -0700 Subject: [PATCH 2/2] Add test to verify the correct transition's callbacks actually run --- .../models/multiple_transitions_that_differ_only_by_guard.rb | 4 ++-- .../multiple_transitions_that_differ_only_by_guard_spec.rb | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/spec/models/multiple_transitions_that_differ_only_by_guard.rb b/spec/models/multiple_transitions_that_differ_only_by_guard.rb index 4924408..159e9e7 100644 --- a/spec/models/multiple_transitions_that_differ_only_by_guard.rb +++ b/spec/models/multiple_transitions_that_differ_only_by_guard.rb @@ -1,7 +1,7 @@ class MultipleTransitionsThatDifferOnlyByGuard include AASM - attr_accessor :executed_correctly + attr_accessor :executed_second aasm do state :start, :initial => true @@ -26,6 +26,6 @@ class MultipleTransitionsThatDifferOnlyByGuard end def this_should_execute - self.executed_correctly = true + self.executed_second = true end end \ No newline at end of file diff --git a/spec/unit/multiple_transitions_that_differ_only_by_guard_spec.rb b/spec/unit/multiple_transitions_that_differ_only_by_guard_spec.rb index 266373c..1f7477f 100644 --- a/spec/unit/multiple_transitions_that_differ_only_by_guard_spec.rb +++ b/spec/unit/multiple_transitions_that_differ_only_by_guard_spec.rb @@ -6,4 +6,9 @@ describe "multiple transitions that differ only by guard" do it 'does not follow the first transition if its guard fails' do expect{job.go}.not_to raise_error end + + it 'executes the second transition\'s callbacks' do + job.go + expect(job.executed_second).to be_truthy + end end