Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Do not call callbacks when they are disabled #162

Closed
wants to merge 1 commit into from

2 participants

@joelind

Hi,

I've been using state_machine with ActiveRecord and in some instances am using ActiveRecord::Observers.disable to turn off observers (for instance, to isolate certain model behaviors during tests). state_machine's definition of Observer.update_with_transition does not check for disabled_for? before using the send method to call the callback.

I added test cases to the ActiveModel and ActiveRecord integration tests to test the behavior and changed the update_with_transition method to match the current implementation in ActiveModel itself.

Thanks!
-Joe

@obrie obrie was assigned
@obrie
Owner

Hey @joelind -

Thanks for reporting this and providing a fix! As it turns out, the problem is that ActiveModel just isn't properly detecting the disabled list of observers on the object -- state_machine doesn't need to actually implement those checks. I've gone with a different implementation that should resolve this issue in the same manner.

@obrie obrie closed this pull request from a commit
@obrie obrie Fix observer callbacks being run when disabled in ActiveModel / Activ…
…eRecord integrations. Closes #162
db84662
@obrie obrie closed this in db84662
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 21, 2012
  1. @joelind
This page is out of date. Refresh to see the latest.
View
5 lib/state_machine/integrations/active_model/observer.rb
@@ -21,7 +21,10 @@ module ActiveModel
module Observer
def update_with_transition(args)
observed_method, object, transition = args
- send(observed_method, object, transition) if respond_to?(observed_method)
+ return unless respond_to?(observed_method)
+ return if disabled_for?(object)
+
+ send(observed_method, object, transition)
end
end
end
View
31 test/unit/integrations/active_model_test.rb
@@ -764,6 +764,37 @@ def test_should_call_all_transition_callback_permutations
@transition.perform
assert_equal callbacks, instance.notifications
end
+
+ def test_should_call_no_transition_callbacks_when_observers_disabled
+ callbacks = [
+ :before_ignite_from_parked_to_idling,
+ :before_ignite_from_parked,
+ :before_ignite_to_idling,
+ :before_ignite,
+ :before_transition_state_from_parked_to_idling,
+ :before_transition_state_from_parked,
+ :before_transition_state_to_idling,
+ :before_transition_state,
+ :before_transition
+ ]
+
+ notified = false
+ 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
View
33 test/unit/integrations/active_record_test.rb
@@ -1585,7 +1585,38 @@ def test_should_call_all_transition_callback_permutations
@transition.perform
assert_equal callbacks, instance.notifications
end
-
+
+ def test_should_call_no_transition_callbacks_when_observers_disabled
+ callbacks = [
+ :before_ignite_from_parked_to_idling,
+ :before_ignite_from_parked,
+ :before_ignite_to_idling,
+ :before_ignite,
+ :before_transition_state_from_parked_to_idling,
+ :before_transition_state_from_parked,
+ :before_transition_state_to_idling,
+ :before_transition_state,
+ :before_transition
+ ]
+
+ notified = false
+ 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)
Something went wrong with that request. Please try again.