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

Creating a Spree::Event::Subscriber removes/cancels other subscribers to same event #3687

Closed
brchristian opened this issue Jun 30, 2020 · 3 comments

Comments

@brchristian
Copy link
Contributor

brchristian commented Jun 30, 2020

Solidus Version:
2.10.0

To Reproduce

Create a new event subscriber at app/subscribers/spree/new_subscriber.rb like so:

require 'spree/event/subscriber'

module Spree
  module NewSubscriber
    include Spree::Event::Subscriber

    event_action :send_new_email, event_name: :order_finalized

    def send_new_email(event)
      order = event.payload[:order]
      Spree::NewMailer.new_email(order).deliver_later
    end

  end
end

Also add an initializer at config/initializers/new_subscribe.rb:

Spree::NewSubscriber.subscribe!

Proceed through checkout. The new_email triggers. But the default order_finalized/confirm_email does not trigger!

Now, for good measure, add the following at app/subscribers/spree/mailer_subscriber_decorator.rb

Spree::MailerSubscriber.class_eval do
  def order_finalized(event)
    order = event.payload[:order]
    unless order.confirmation_delivered?
      Spree::Config.order_mailer_class.confirm_email(order).deliver_later
      order.update_column(:confirmation_delivered, true)
    end
  end
end

All we are doing is re-adding back the exact same code that is in Solidus core, but doing it as a decorator.

Proceed through checkout. Theorder_finalized/confirm_email triggers. But the new_email does not!

Desktop:

  • OS: MacOS Catalina 10.15.5
  • Browser: Safari
  • Version: 13.1.1
@spaghetticode
Copy link
Member

@brchristian thanks for reporting the issue. I tried with Solidus master and this does not happen anymore. I did not check thoroughly the reason, but I suppose the "fix" is a side effect of this PR.

@kennyadsl
Copy link
Member

@brchristian do you have any update on this one? Do you think the latest code fixes the issue or it's still something we should look into?

@brchristian
Copy link
Contributor Author

Hi @kennyadsl, as far as I can tell, this is fixed in master, and I suspect it was exactly the PR that @spaghetticode describes.

It's still an issue in 2.10, but if it's fixed going forward, that's great! I think we can close this issue. Cheers!

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

3 participants