From 9aa50f9c11fcafec10644f36833be6a1052349a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thorsten=20B=C3=B6ttger?= Date: Tue, 25 Nov 2014 22:23:23 +0100 Subject: [PATCH] bugfix: fire guards only once per transition, part 2 #187 --- CHANGELOG.md | 3 +- lib/aasm/event.rb | 3 +- .../multiple_transitions_transition_guard.rb | 65 +++++++++++++++++++ spec/unit/callbacks_spec.rb | 57 ++++++++++++---- 4 files changed, 113 insertions(+), 15 deletions(-) create mode 100644 spec/models/callbacks/multiple_transitions_transition_guard.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 31125ae..f129330 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,8 +5,9 @@ * `aasm_column` has been removed. Use `aasm.attribute_name` instead * `aasm_human_event_name` has been removed. Use `aasm.human_event_name` instead -## 4.0.x (not yet released) +## 4.0.3 + * bugfix: fire guards only once per transition, part 2 (see [issue #187](https://github.com/aasm/aasm/issues/187) for details) * `aasm_column` is deprecated. Use `aasm.attribute_name` instead ## 4.0.2 diff --git a/lib/aasm/event.rb b/lib/aasm/event.rb index 054751e..de5e4ac 100644 --- a/lib/aasm/event.rb +++ b/lib/aasm/event.rb @@ -106,7 +106,8 @@ module AASM transitions.each do |transition| next if to_state and !Array(transition.to).include?(to_state) - if Array(transition.to).include?(options[:may_fire]) || transition.allowed?(obj, *args) + if (options.key?(:may_fire) && Array(transition.to).include?(options[:may_fire])) || + (!options.key?(:may_fire) && transition.allowed?(obj, *args)) result = to_state || Array(transition.to).first if options[:test_only] # result = true diff --git a/spec/models/callbacks/multiple_transitions_transition_guard.rb b/spec/models/callbacks/multiple_transitions_transition_guard.rb new file mode 100644 index 0000000..1d67510 --- /dev/null +++ b/spec/models/callbacks/multiple_transitions_transition_guard.rb @@ -0,0 +1,65 @@ +module Callbacks + class MultipleTransitionsTransitionGuard + include AASM + + def initialize(options={}) + @fail_event_guard = options[:fail_event_guard] + @fail_transition_guard = options[:fail_transition_guard] + @log = options[:log] + end + + aasm do + state :open, :initial => true, + :before_enter => :before_enter_open, + :enter => :enter_open, + :after_enter => :after_enter_open, + :before_exit => :before_exit_open, + :exit => :exit_open, + :after_exit => :after_exit_open + + state :closed, + :before_enter => :before_enter_closed, + :enter => :enter_closed, + :after_enter => :after_enter_closed, + :before_exit => :before_exit_closed, + :exit => :exit_closed, + :after_exit => :after_exit_closed + + state :failed + + event :close, :before => :before, :after => :after, :guard => :event_guard do + transitions :to => :closed, :from => [:open], :guard => :transition_guard, :after => :transitioning + transitions :to => :failed, :from => [:open] + end + + event :open, :before => :before, :after => :after do + transitions :to => :open, :from => :closed + end + end + + def log(text) + puts text if @log + end + + def before_enter_open; log('before_enter_open'); end + def enter_open; log('enter_open'); end + def before_exit_open; log('before_exit_open'); end + def after_enter_open; log('after_enter_open'); end + def exit_open; log('exit_open'); end + def after_exit_open; log('after_exit_open'); end + + def before_enter_closed; log('before_enter_closed'); end + def enter_closed; log('enter_closed'); end + def before_exit_closed; log('before_exit_closed'); end + def exit_closed; log('exit_closed'); end + def after_enter_closed; log('after_enter_closed'); end + def after_exit_closed; log('after_exit_closed'); end + + def event_guard; log('event_guard'); !@fail_event_guard; end + def transition_guard; log('transition_guard'); !@fail_transition_guard; end + def transitioning; log('transitioning'); end + + def before; log('before'); end + def after; log('after'); end + end +end diff --git a/spec/unit/callbacks_spec.rb b/spec/unit/callbacks_spec.rb index 816547c..8533b9e 100644 --- a/spec/unit/callbacks_spec.rb +++ b/spec/unit/callbacks_spec.rb @@ -49,27 +49,58 @@ describe 'callbacks for the new DSL' do context "if the transition guard fails" do it "does not run any state callback if guard is defined inline" do - callback = CallbackNewDsl.new + show_debug_log = false + callback = CallbackNewDsl.new(:log => show_debug_log, :fail_transition_guard => true) callback.aasm.current_state - expect(callback).to receive(:before).once.ordered - expect(callback).to receive(:event_guard).once.ordered.and_return(true) - expect(callback).to receive(:transition_guard).once.ordered.and_return(false) - expect(callback).to_not receive(:before_exit_open) - expect(callback).to_not receive(:exit_open) - expect(callback).to_not receive(:transitioning) - expect(callback).to_not receive(:before_enter_closed) - expect(callback).to_not receive(:enter_closed) - expect(callback).to_not receive(:aasm_write_state) - expect(callback).to_not receive(:after_exit_open) - expect(callback).to_not receive(:after_enter_closed) - expect(callback).to_not receive(:after) + unless show_debug_log + expect(callback).to receive(:before).once.ordered + expect(callback).to receive(:event_guard).once.ordered.and_return(true) + expect(callback).to receive(:transition_guard).once.ordered.and_return(false) + expect(callback).to_not receive(:before_exit_open) + expect(callback).to_not receive(:exit_open) + expect(callback).to_not receive(:transitioning) + expect(callback).to_not receive(:before_enter_closed) + expect(callback).to_not receive(:enter_closed) + expect(callback).to_not receive(:aasm_write_state) + expect(callback).to_not receive(:after_exit_open) + expect(callback).to_not receive(:after_enter_closed) + expect(callback).to_not receive(:after) + end expect { callback.close! }.to raise_error(AASM::InvalidTransition) end + it "does not run transition_guard twice for multiple permitted transitions" do + require 'models/callbacks/multiple_transitions_transition_guard' + + show_debug_log = false + callback = Callbacks::MultipleTransitionsTransitionGuard.new(:log => show_debug_log, :fail_transition_guard => true) + callback.aasm.current_state + + unless show_debug_log + expect(callback).to receive(:before).once.ordered + expect(callback).to receive(:event_guard).once.ordered.and_return(true) + expect(callback).to receive(:transition_guard).once.ordered.and_return(false) + expect(callback).to receive(:event_guard).once.ordered.and_return(true) + expect(callback).to receive(:before_exit_open).once.ordered + expect(callback).to receive(:exit_open).once.ordered + expect(callback).to receive(:aasm_write_state).once.ordered.and_return(true) # this is when the state changes + expect(callback).to receive(:after_exit_open).once.ordered + expect(callback).to receive(:after).once.ordered + + expect(callback).to_not receive(:transitioning) + expect(callback).to_not receive(:before_enter_closed) + expect(callback).to_not receive(:enter_closed) + expect(callback).to_not receive(:after_enter_closed) + end + + callback.close! + expect(callback.aasm.current_state).to eql :failed + end + it "does not run any state callback if guard is defined with block" do callback = GuardWithinBlock.new #(:log => true, :fail_transition_guard => true) callback.aasm.current_state