an event's :error callback now retrieves all arguments passed to the event #196

CHANGELOG.md

@@ -5,6 +5,10 @@
  * `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.5 (not yet released)
+
+ * an event's `:error` callback now retrieves all arguments passed to the event (see [issue #196](https://github.com/aasm/aasm/issues/196) for details)
+
 ## 4.0.5
 
  * bugfix: initialize the aasm state column after initialization of the _ActiveRecord_ instance only if the attribute has been loaded (see [issue #193](https://github.com/aasm/aasm/issues/193) for details)

lib/aasm/aasm.rb

@@ -81,7 +81,7 @@ private
         aasm_failed(event_name, old_state)
       end
     rescue StandardError => e
-      event.fire_callbacks(:error, self, e) || raise(e)
+      event.fire_callbacks(:error, self, e, *process_args(event, aasm.current_state, *args)) || raise(e)
     end
   end
 

lib/aasm/core/event.rb

@@ -124,14 +124,22 @@ module AASM::Core
     def invoke_callbacks(code, record, args)
       case code
         when Symbol, String
-          record.send(code, *args)
+          unless record.respond_to?(code)
+            raise NoMethodError.new("NoMethodError: undefined method `#{code}' for #{self.inspect}:#{self.class}")
+          end
+          arity = record.send(:method, code.to_sym).arity
+          record.send(code, *(arity < 0 ? args : args[0...arity]))
           true
+
         when Proc
-          record.instance_exec(*args, &code)
+          arity = code.arity
+          record.instance_exec(*(arity < 0 ? args : args[0...arity]), &code)
           true
+
         when Array
           code.each {|a| invoke_callbacks(a, record, args)}
           true
+
         else
           false
       end

lib/aasm/core/transition.rb

@@ -50,11 +50,9 @@ module AASM::Core
 
       case code
       when Symbol, String
-        # QUESTION : record.send(code, *args) ?
         arity = record.send(:method, code.to_sym).arity
         arity == 0 ? record.send(code) : record.send(code, *args)
       when Proc
-        # QUESTION : record.instance_exec(*args, &code) ?
         code.arity == 0 ? record.instance_exec(&code) : record.instance_exec(*args, &code)
       when Array
         if options[:guard]

spec/models/callbacks/basic.rb

@@ -6,6 +6,15 @@ module Callbacks
       @fail_event_guard = options[:fail_event_guard]
       @fail_transition_guard = options[:fail_transition_guard]
       @log = options[:log]
+      reset_data
+    end
+
+    def reset_data
+      @data = []
+    end
+
+    def data
+      @data.join(' ')
     end
 
     aasm do
@@ -35,28 +44,32 @@ module Callbacks
     end
 
     def log(text)
+      @data << 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 aasm_write_state(*args); log('aasm_write_state'); true; 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 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 event_guard; log('event_guard'); !@fail_event_guard; end
-    def transition_guard; log('transition_guard'); !@fail_transition_guard; end
-    def after_transition; log('after_transition'); 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 before_event; log('before_event'); end
-    def after_event; log('after_event'); end
+    def event_guard;          log('event_guard');         !@fail_event_guard;      end
+    def transition_guard;     log('transition_guard');    !@fail_transition_guard; end
+
+    def after_transition;     log('after_transition');    end
+
+    def before_event;         log('before_event');        end
+    def after_event;          log('after_event');         end
   end
 end

spec/models/callbacks/with_args.rb

@@ -2,6 +2,19 @@ module Callbacks
   class WithArgs
     include AASM
 
+    def initialize(options={})
+      @log = options[:log]
+      reset_data
+    end
+
+    def reset_data
+      @data = []
+    end
+
+    def data
+      @data.join(' ')
+    end
+
     aasm do
       state :open, :initial => true,
         :before_enter => :before_enter_open,
@@ -25,9 +38,12 @@ module Callbacks
     end
 
     def log(text)
-      # puts text
+      @data << text
+      puts text if @log
     end
 
+    def aasm_write_state(*args); log('aasm_write_state'); true; end
+
     def before_enter_open; log('before_enter_open'); end
     def before_exit_open; log('before_exit_open'); end
     def after_enter_open; log('after_enter_open'); end
@@ -38,8 +54,8 @@ module Callbacks
     def after_enter_closed; log('after_enter_closed'); end
     def after_exit_closed; log('after_exit_closed'); end
 
-    def before(*args); log('before'); end
-    def transition_proc(arg1, arg2); log('transition_proc'); end
-    def after(*args); log('after'); end
+    def before(arg1, *args); log("before(#{arg1.inspect},#{args.map(&:inspect).join(',')})"); end
+    def transition_proc(arg1, arg2); log("transition_proc(#{arg1.inspect},#{arg2.inspect})"); end
+    def after(*args); log("after(#{args.map(&:inspect).join(',')})"); end
   end
 end

spec/unit/callbacks_spec.rb

@@ -128,19 +128,12 @@ describe 'callbacks for the new DSL' do
   end
 
   it "should properly pass arguments" do
-    cb = Callbacks::WithArgs.new
-
-    # TODO: use expect syntax here
-    cb.should_receive(:before).with(:arg1, :arg2).once.ordered
-    cb.should_receive(:before_exit_open).once.ordered                   # these should be before the state changes
-    cb.should_receive(:transition_proc).with(:arg1, :arg2).once.ordered
-    cb.should_receive(:before_enter_closed).once.ordered
-    cb.should_receive(:aasm_write_state).once.ordered.and_return(true)  # this is when the state changes
-    cb.should_receive(:after_exit_open).once.ordered                    # these should be after the state changes
-    cb.should_receive(:after_enter_closed).once.ordered
-    cb.should_receive(:after).with(:arg1, :arg2).once.ordered
+    cb = Callbacks::WithArgs.new(:log => false)
+    cb.aasm.current_state
 
+    cb.reset_data
     cb.close!(:arg1, :arg2)
+    expect(cb.data).to eql 'before(:arg1,:arg2) before_exit_open transition_proc(:arg1,:arg2) before_enter_closed aasm_write_state after_exit_open after_enter_closed after(:arg1,:arg2)'
   end
 
   it "should call the callbacks given the to-state as argument" do
@@ -181,6 +174,10 @@ describe 'event callbacks' do
   describe "with an error callback defined" do
     before do
       class Foo
+        # this hack is needed to allow testing of parameters, since RSpec
+        # destroys a method's arity when mocked
+        attr_accessor :data
+
         aasm do
           event :safe_close, :success => :success_callback, :error => :error_callback do
             transitions :to => :closed, :from => [:open]
@@ -191,16 +188,42 @@ describe 'event callbacks' do
       @foo = Foo.new
     end
 
-    it "should run error_callback if an exception is raised and error_callback defined" do
-      def @foo.error_callback(e); end
+    context "error_callback defined" do
+      it "should run error_callback if an exception is raised" do
+        def @foo.error_callback(e)
+          @data = [e]
+        end
 
-      allow(@foo).to receive(:before_enter).and_raise(e = StandardError.new)
-      expect(@foo).to receive(:error_callback).with(e)
+        allow(@foo).to receive(:before_enter).and_raise(e = StandardError.new)
 
-      @foo.safe_close!
+        @foo.safe_close!
+        expect(@foo.data).to eql [e]
+      end
+
+      it "should run error_callback without parameters if callback does not support any" do
+        def @foo.error_callback(e)
+          @data = []
+        end
+
+        allow(@foo).to receive(:before_enter).and_raise(e = StandardError.new)
+
+        @foo.safe_close!('arg1', 'arg2')
+        expect(@foo.data).to eql []
+      end
+
+      it "should run error_callback with parameters if callback supports them" do
+        def @foo.error_callback(e, arg1, arg2)
+          @data = [arg1, arg2]
+        end
+
+        allow(@foo).to receive(:before_enter).and_raise(e = StandardError.new)
+
+        @foo.safe_close!('arg1', 'arg2')
+        expect(@foo.data).to eql ['arg1', 'arg2']
+      end
     end
 
-    it "should raise NoMethodError if exceptionis raised and error_callback is declared but not defined" do
+    it "should raise NoMethodError if exception is raised and error_callback is declared but not defined" do
       allow(@foo).to receive(:before_enter).and_raise(StandardError)
       expect{@foo.safe_close!}.to raise_error(NoMethodError)
     end