Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Event Subscribers: Can't subscribe the same method to two events #4340

Closed
mamhoff opened this issue Apr 14, 2022 · 3 comments
Closed

Event Subscribers: Can't subscribe the same method to two events #4340

mamhoff opened this issue Apr 14, 2022 · 3 comments

Comments

@mamhoff
Copy link
Contributor

mamhoff commented Apr 14, 2022

I have an event subscriber that I would like to subscribe to two events: order_finalized and order_recalculated.

Based on the documentation, I set out to create a subscriber like so:

module MySubscriber
  include Spree::Event::Subscriber

  event_action :my_method, event_name: :order_recalculated
  event_action :my_method, event_name: :order_finalized

  def my_method(event)
    puts "You've recalculated or finalized your order!
  end
end

However, my new subscriber will only run when the order_finalized event is fired. I think this has to do with the event_actions hash in the Spree::Subscriber module. Its keys are method names, so in the above example the key would be :my_method, and the second invocation of event_action will overwrite the previously existing contents of Spree::Subscriber.event_actions[:my_method]. See here and here.

Solidus Version:
Reproducible on 2.11 and master branch.

To Reproduce
See above.

Current behavior

MySubscriber#my_method is only called upon firing order_finalized, because that is the last invocation of event_action.

Expected behavior

MySubscriber#my_method is called upon firing order_recalculated and order_finalized.

@waiting-for-dev
Copy link
Contributor

I'll propose replacing the current event system with Omnes (which is an extraction of the former, with better subscriber objects) very soon. That is fixed there.

@waiting-for-dev
Copy link
Contributor

I'll propose replacing the current event system with Omnes (which is an extraction of the former, with better subscriber objects) very soon.

Please, see #4342

@waiting-for-dev
Copy link
Contributor

Closed by #4342

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants