bugfix: fire guards only once per transition, part 2 #187

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

lib/aasm/event.rb

@@ -106,7 +106,8 @@ def _fire(obj, options={}, to_state=nil, *args)
 
       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

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

spec/unit/callbacks_spec.rb

@@ -49,27 +49,58 @@
 
   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