Fix observer callbacks being run incorrectly when using nil states in…

… ActiveModel-based integrations

Add tests proving that translations work for nil states

CHANGELOG.md

@@ -1,5 +1,6 @@
 # master
 
+* Fix observer callbacks being run incorrectly when using nil states in ActiveModel-based integrations
 * Remove ActiveModel Observer method chains in order to better ensure compatibility
 * Update DataMapper integration for 1.2.0+ support [Markus Schirp]
 * Provide access to the human state name in invalid_transition translations

lib/state_machine/integrations/active_model.rb

@@ -556,8 +556,8 @@ def add_events(new_events)
         def notify(type, object, transition)
           name = self.name
           event = transition.qualified_event
-          from = transition.from_name
-          to = transition.to_name
+          from = transition.from_name || 'nil'
+          to = transition.to_name || 'nil'
 
           # Machine-specific updates
           ["#{type}_#{event}", "#{type}_transition_#{name}"].each do |event_segment|

lib/state_machine/state.rb

@@ -179,9 +179,7 @@ def matches?(other_value)
     # This can be called multiple times.  Each time a new context is created,
     # a new module will be included in the owner class.
     def context(&block)
-      owner_class = machine.owner_class
       machine_name = machine.name
-      name = self.name
 
       # Evaluate the method definitions
       context = StateContext.new(self)
@@ -199,7 +197,7 @@ def #{method}(*args, &block)
 
       # Include the context so that it can be bound to the owner class (the
       # context is considered an ancestor, so it's allowed to be bound)
-      owner_class.class_eval { include context }
+      machine.owner_class.class_eval { include context }
 
       context
     end

test/unit/integrations/active_model_test.rb

@@ -798,6 +798,54 @@ def before_ignite(*args)
       @transition.perform
       assert_equal [instance], instance.notifications
     end
+
+    def test_should_support_nil_from_states
+      callbacks = [
+        :before_ignite_from_nil_to_idling,
+        :before_ignite_from_nil,
+        :before_transition_state_from_nil_to_idling,
+        :before_transition_state_from_nil
+      ]
+
+      notified = false
+      observer = new_observer(@model) do
+        callbacks.each do |callback|
+          define_method(callback) do |*args|
+            notifications << callback
+          end
+        end
+      end
+
+      instance = observer.instance
+
+      transition = StateMachine::Transition.new(@record, @machine, :ignite, nil, :idling)
+      transition.perform
+      assert_equal callbacks, instance.notifications
+    end
+
+    def test_should_support_nil_to_states
+      callbacks = [
+        :before_ignite_from_parked_to_nil,
+        :before_ignite_to_nil,
+        :before_transition_state_from_parked_to_nil,
+        :before_transition_state_to_nil
+      ]
+
+      notified = false
+      observer = new_observer(@model) do
+        callbacks.each do |callback|
+          define_method(callback) do |*args|
+            notifications << callback
+          end
+        end
+      end
+
+      instance = observer.instance
+
+      transition = StateMachine::Transition.new(@record, @machine, :ignite, :parked, nil)
+      transition.perform
+      assert_equal callbacks, instance.notifications
+    end
   end
 
   class MachineWithNamespacedObserversTest < BaseTestCase
@@ -1027,6 +1075,16 @@ def test_should_allow_customized_state_key_unscoped
       assert_equal 'shutdown', machine.state(:parked).human_name
     end
 
+    def test_should_support_nil_state_key
+      I18n.backend.store_translations(:en, {
+        :activemodel => {:state_machines => {:states => {:nil => 'empty'}}}
+      })
+
+      machine = StateMachine::Machine.new(@model)
+
+      assert_equal 'empty', machine.state(nil).human_name
+    end
+
     def test_should_allow_customized_event_key_scoped_to_class_and_machine
       I18n.backend.store_translations(:en, {
         :activemodel => {:state_machines => {:'active_model_test/foo' => {:state => {:events => {:park => 'stop'}}}}}

test/unit/integrations/active_record_test.rb

@@ -1592,6 +1592,54 @@ def before_ignite(*args)
       @transition.perform
       assert_equal [:before_ignite, [:before_save, @object]], instance.notifications
     end
+
+    def test_should_support_nil_from_states
+      callbacks = [
+        :before_ignite_from_nil_to_idling,
+        :before_ignite_from_nil,
+        :before_transition_state_from_nil_to_idling,
+        :before_transition_state_from_nil
+      ]
+
+      notified = false
+      observer = new_observer(@model) do
+        callbacks.each do |callback|
+          define_method(callback) do |*args|
+            notifications << callback
+          end
+        end
+      end
+
+      instance = observer.instance
+
+      transition = StateMachine::Transition.new(@record, @machine, :ignite, nil, :idling)
+      transition.perform
+      assert_equal callbacks, instance.notifications
+    end
+
+    def test_should_support_nil_to_states
+      callbacks = [
+        :before_ignite_from_parked_to_nil,
+        :before_ignite_to_nil,
+        :before_transition_state_from_parked_to_nil,
+        :before_transition_state_to_nil
+      ]
+
+      notified = false
+      observer = new_observer(@model) do
+        callbacks.each do |callback|
+          define_method(callback) do |*args|
+            notifications << callback
+          end
+        end
+      end
+
+      instance = observer.instance
+
+      transition = StateMachine::Transition.new(@record, @machine, :ignite, :parked, nil)
+      transition.perform
+      assert_equal callbacks, instance.notifications
+    end
   end
 
   class MachineWithNamespacedObserversTest < BaseTestCase
@@ -1988,6 +2036,16 @@ def test_should_allow_customized_state_key_unscoped
         assert_equal 'shutdown', machine.state(:parked).human_name
       end
 
+      def test_should_support_nil_state_key
+        I18n.backend.store_translations(:en, {
+          :activerecord => {:state_machines => {:states => {:nil => 'empty'}}}
+        })
+
+        machine = StateMachine::Machine.new(@model)
+
+        assert_equal 'empty', machine.state(nil).human_name
+      end
+
       def test_should_allow_customized_event_key_scoped_to_class_and_machine
         I18n.backend.store_translations(:en, {
           :activerecord => {:state_machines => {:'active_record_test/foo' => {:state => {:events => {:park => 'stop'}}}}}

test/unit/integrations/mongo_mapper_test.rb

@@ -1574,6 +1574,16 @@ def test_should_allow_customized_state_key_unscoped
         assert_equal 'shutdown', machine.state(:parked).human_name
       end
 
+      def test_should_support_nil_state_key
+        I18n.backend.store_translations(:en, {
+          :mongo_mapper => {:state_machines => {:states => {:nil => 'empty'}}}
+        })
+
+        machine = StateMachine::Machine.new(@model)
+
+        assert_equal 'empty', machine.state(nil).human_name
+      end
+
       def test_should_allow_customized_event_key_scoped_to_class_and_machine
         I18n.backend.store_translations(:en, {
           :mongo_mapper => {:state_machines => {:'mongo_mapper_test/foo' => {:state => {:events => {:park => 'stop'}}}}}

test/unit/integrations/mongoid_test.rb

@@ -1567,6 +1567,16 @@ def test_should_allow_customized_state_key_unscoped
       assert_equal 'shutdown', machine.state(:parked).human_name
     end
 
+    def test_should_support_nil_state_key
+      I18n.backend.store_translations(:en, {
+        :mongoid => {:state_machines => {:states => {:nil => 'empty'}}}
+      })
+
+      machine = StateMachine::Machine.new(@model)
+
+      assert_equal 'empty', machine.state(nil).human_name
+    end
+
     def test_should_allow_customized_event_key_scoped_to_class_and_machine
       I18n.backend.store_translations(:en, {
         :mongoid => {:state_machines => {:'mongoid_test/foo' => {:state => {:events => {:park => 'stop'}}}}}