Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Fix observer callbacks being run when disabled in ActiveModel / Activ…

…eRecord integrations. Closes #162
  • Loading branch information...
commit db846629e735b241852ded803049f84fafbf0b4c 1 parent c99b406
Aaron Pfeifer obrie authored
1  CHANGELOG.md
View
@@ -1,5 +1,6 @@
# master
+* Fix observer callbacks being run when disabled in ActiveModel / ActiveRecord integrations
* Add YARD integration for autogenerating documentation / embedding visualizations of state machines
* Allow states / events to be drawn with their human name instead of their internal name
5 lib/state_machine/integrations/active_model.rb
View
@@ -475,6 +475,7 @@ def load_locale
# Loads extensions to ActiveModel's Observers
def load_observer_extensions
require 'state_machine/integrations/active_model/observer'
+ require 'state_machine/integrations/active_model/observer_update'
end
# Adds a set of default callbacks that utilize the Observer extensions
@@ -568,14 +569,14 @@ def notify(type, object, transition)
["_from_#{from}", nil].each do |from_segment|
["_to_#{to}", nil].each do |to_segment|
object.class.changed if object.class.respond_to?(:changed)
- object.class.notify_observers('update_with_transition', [[event_segment, from_segment, to_segment].join, object, transition])
+ object.class.notify_observers('update_with_transition', ObserverUpdate.new([event_segment, from_segment, to_segment].join, object, transition))
end
end
end
# Generic updates
object.class.changed if object.class.respond_to?(:changed)
- object.class.notify_observers('update_with_transition', ["#{type}_transition", object, transition])
+ object.class.notify_observers('update_with_transition', ObserverUpdate.new("#{type}_transition", object, transition))
true
end
6 lib/state_machine/integrations/active_model/observer.rb
View
@@ -19,9 +19,9 @@ module ActiveModel
# end
# end
module Observer
- def update_with_transition(args)
- observed_method, object, transition = args
- send(observed_method, object, transition) if respond_to?(observed_method)
+ def update_with_transition(observer_update)
+ method = observer_update.method
+ send(method, *observer_update.args) if respond_to?(method)
end
end
end
42 lib/state_machine/integrations/active_model/observer_update.rb
View
@@ -0,0 +1,42 @@
+module StateMachine
+ module Integrations #:nodoc:
+ module ActiveModel
+ # Represents the encapsulation of all of the details to be included in an
+ # update to state machine observers. This allows multiple arguments to
+ # get passed to an observer method (instead of just a single +object+)
+ # while still respecting the way in which ActiveModel checks for the
+ # object's list of observers.
+ class ObserverUpdate
+ # The method to invoke on the observer
+ attr_reader :method
+
+ # The object being transitioned
+ attr_reader :object
+
+ # The transition being run
+ attr_reader :transition
+
+ def initialize(method, object, transition) #:nodoc:
+ @method, @object, @transition = method, object, transition
+ end
+
+ # The arguments to pass into the method
+ def args
+ [object, transition]
+ end
+
+ # The class of the object being transitioned. Normally the object
+ # getting passed into observer methods is the actual instance of the
+ # ActiveModel class. ActiveModel uses that instance's class to check
+ # for enabled / disabled observers.
+ #
+ # Since state_machine is passing an ObserverUpdate instance into observer
+ # methods, +class+ needs to be overridden so that ActiveModel can still
+ # get access to the enabled / disabled observers.
+ def class
+ object.class
+ end
+ end
+ end
+ end
+end
60 test/unit/integrations/active_model_test.rb
View
@@ -727,6 +727,40 @@ def test_should_be_valid_if_validation_succeeds_within_state_scope
end
end
+ class ObserverUpdateTest < BaseTestCase
+ def setup
+ @model = new_model { include ActiveModel::Observing }
+ @machine = StateMachine::Machine.new(@model)
+ @machine.state :parked, :idling
+ @machine.event :ignite
+
+ @record = @model.new(:state => 'parked')
+ @transition = StateMachine::Transition.new(@record, @machine, :ignite, :parked, :idling)
+
+ @observer_update = StateMachine::Integrations::ActiveModel::ObserverUpdate.new(:before_transition, @record, @transition)
+ end
+
+ def test_should_have_method
+ assert_equal :before_transition, @observer_update.method
+ end
+
+ def test_should_have_object
+ assert_equal @record, @observer_update.object
+ end
+
+ def test_should_have_transition
+ assert_equal @transition, @observer_update.transition
+ end
+
+ def test_should_include_object_and_transition_in_args
+ assert_equal [@record, @transition], @observer_update.args
+ end
+
+ def test_should_use_record_class_as_class
+ assert_equal @model, @observer_update.class
+ end
+ end
+
class MachineWithObserversTest < BaseTestCase
def setup
@model = new_model { include ActiveModel::Observing }
@@ -750,7 +784,6 @@ def test_should_call_all_transition_callback_permutations
:before_transition
]
- notified = false
observer = new_observer(@model) do
callbacks.each do |callback|
define_method(callback) do |*args|
@@ -765,6 +798,31 @@ def test_should_call_all_transition_callback_permutations
assert_equal callbacks, instance.notifications
end
+ def test_should_call_no_transition_callbacks_when_observers_disabled
+ return unless ::ActiveModel::VERSION::MAJOR >= 3 && ::ActiveModel::VERSION::MINOR >= 1
+
+ callbacks = [
+ :before_ignite,
+ :before_transition
+ ]
+
+ observer = new_observer(@model) do
+ callbacks.each do |callback|
+ define_method(callback) do |*args|
+ notifications << callback
+ end
+ end
+ end
+
+ instance = observer.instance
+
+ @model.observers.disable(observer) do
+ @transition.perform
+ end
+
+ assert_equal [], instance.notifications
+ end
+
def test_should_pass_record_and_transition_to_before_callbacks
observer = new_observer(@model) do
def before_transition(*args)
26 test/unit/integrations/active_record_test.rb
View
@@ -1571,7 +1571,6 @@ def test_should_call_all_transition_callback_permutations
:before_transition
]
- notified = false
observer = new_observer(@model) do
callbacks.each do |callback|
define_method(callback) do |*args|
@@ -1586,6 +1585,31 @@ def test_should_call_all_transition_callback_permutations
assert_equal callbacks, instance.notifications
end
+ def test_should_call_no_transition_callbacks_when_observers_disabled
+ return unless ::ActiveRecord::VERSION::MAJOR >= 3 && ::ActiveRecord::VERSION::MINOR >= 1
+
+ callbacks = [
+ :before_ignite,
+ :before_transition
+ ]
+
+ observer = new_observer(@model) do
+ callbacks.each do |callback|
+ define_method(callback) do |*args|
+ notifications << callback
+ end
+ end
+ end
+
+ instance = observer.instance
+
+ @model.observers.disable(observer) do
+ @transition.perform
+ end
+
+ assert_equal [], instance.notifications
+ end
+
def test_should_pass_record_and_transition_to_before_callbacks
observer = new_observer(@model) do
def before_transition(*args)
Please sign in to comment.
Something went wrong with that request. Please try again.