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

Support class reloading for Event Subscribers #3232

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions core/app/subscribers/spree/mailer_subscriber.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# frozen_string_literal: true

module Spree
module MailerSubscriber
include Spree::Event::Subscriber

event_action :order_finalized
event_action :send_reimbursement_email, event_name: :reimbursement_reimbursed

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

def send_reimbursement_email(event)
reimbursement = event.payload[:reimbursement]
Spree::Config.reimbursement_mailer_class.reimbursement_email(reimbursement.id).deliver_later
end
end
end
12 changes: 9 additions & 3 deletions core/lib/spree/core/engine.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# frozen_string_literal: true

require 'spree/config'
require 'spree/event/processors/mailer_processor'

module Spree
module Core
Expand Down Expand Up @@ -45,8 +44,15 @@ class Engine < ::Rails::Engine
Migrations.new(config, engine_name).check
end

initializer 'spree.core.subscribe_event_mailer_processor' do
Spree::Event::Processors::MailerProcessor.subscribe!
# Setup Event Subscribers
initializer 'spree.core.initialize_subscribers' do |app|
app.reloader.to_prepare do
Spree::Event.subscribers.each(&:subscribe!)
end

app.reloader.before_class_unload do
Spree::Event.subscribers.each(&:unsubscribe!)
end
end

# Load in mailer previews for apps to use in development.
Expand Down
8 changes: 7 additions & 1 deletion core/lib/spree/event.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ module Event
# @order.finalize!
# end
def fire(event_name, opts = {})
adapter.fire name_with_suffix(event_name), opts do
adapter.fire name_with_suffix(event_name.to_s), opts do
yield opts if block_given?
end
end
Expand Down Expand Up @@ -98,6 +98,12 @@ def suffix
Spree::Config.events.suffix
end

# @!attribute [r] subscribers
# @return [Array<Spree::Event::Subscriber>] A list of subscribers used to support class reloading for Spree::Event::Subscriber instances
def subscribers
Spree::Config.events.subscribers
end

private

def name_with_suffix(name)
Expand Down
8 changes: 8 additions & 0 deletions core/lib/spree/event/configuration.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,16 @@
# frozen_string_literal: true

require 'spree/core/class_constantizer'

module Spree
module Event
class Configuration
def subscribers
@subscribers ||= ::Spree::Core::ClassConstantizer::Set.new.tap do |set|
set << 'Spree::MailerSubscriber'
end
end

attr_writer :adapter, :suffix

def adapter
Expand Down
27 changes: 0 additions & 27 deletions core/lib/spree/event/processors/mailer_processor.rb

This file was deleted.

2 changes: 1 addition & 1 deletion core/spec/lib/spree/event/subscriber_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
require 'spec_helper'
require 'spree/event'

RSpec.describe Spree::Event do
RSpec.describe Spree::Event::Subscriber do
module M
include Spree::Event::Subscriber

Expand Down
52 changes: 39 additions & 13 deletions core/spec/lib/spree/event_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,21 +14,21 @@
expect(subject.adapter).to eql Spree::Event::Adapters::ActiveSupportNotifications
end

before do
# ActiveSupport::Notifications does not provide an interface to clean all
# subscribers at once, so some low level brittle code is required
@old_subscribers = notifier.instance_variable_get('@subscribers').dup
@old_listeners = notifier.instance_variable_get('@listeners_for').dup
notifier.instance_variable_get('@subscribers').clear
notifier.instance_variable_get('@listeners_for').clear
end
context 'with the default adapter' do
before do
# ActiveSupport::Notifications does not provide an interface to clean all
# subscribers at once, so some low level brittle code is required
@old_subscribers = notifier.instance_variable_get('@subscribers').dup
@old_listeners = notifier.instance_variable_get('@listeners_for').dup
notifier.instance_variable_get('@subscribers').clear
notifier.instance_variable_get('@listeners_for').clear
end

after do
notifier.instance_variable_set '@subscribers', @old_subscribers
notifier.instance_variable_set '@listeners_for', @old_listeners
end
after do
notifier.instance_variable_set '@subscribers', @old_subscribers
notifier.instance_variable_set '@listeners_for', @old_listeners
end

context 'with the default adapter' do
describe '#listeners' do
context 'when there is no subscription' do
it { expect(subject.listeners).to be_empty }
Expand Down Expand Up @@ -89,4 +89,30 @@
end
end
end

describe '.subscribers' do
let(:subscriber) { instance_double(Module, 'Subscriber') }
let(:subscriber_name) { instance_double(String, 'Subscriber name') }

before do
described_class.subscribers.clear
allow(subscriber).to receive(:name).and_return(subscriber_name)
allow(subscriber_name).to receive(:constantize).and_return(subscriber)
allow(subscriber_name).to receive(:to_s).and_return(subscriber_name)
end

it 'accepts the names of constants' do
Spree::Config.events.subscribers << subscriber_name

expect(described_class.subscribers.to_a).to eq([subscriber])
end

it 'discards duplicates' do
described_class.subscribers << subscriber_name
described_class.subscribers << subscriber_name
described_class.subscribers << subscriber_name

expect(described_class.subscribers.to_a).to eq([subscriber])
end
end
end
10 changes: 5 additions & 5 deletions core/spec/models/spree/order_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,14 @@
end

# These specs show how notifications can be removed, one at a time or
# all the ones set by MailerProcessor module
# all the ones set by MailerSubscriber module
context 'when removing the default email notification subscription' do
before do
Spree::Event.unsubscribe Spree::Event::Processors::MailerProcessor.order_finalized_handler
Spree::Event.unsubscribe Spree::MailerSubscriber.order_finalized_handler
end

after do
Spree::Event::Processors::MailerProcessor.subscribe!
Spree::MailerSubscriber.subscribe!
end

it 'does not send the email' do
Expand All @@ -47,11 +47,11 @@

context 'when removing all the email notification subscriptions' do
before do
Spree::Event::Processors::MailerProcessor.unsubscribe!
Spree::MailerSubscriber.unsubscribe!
end

after do
Spree::Event::Processors::MailerProcessor.subscribe!
Spree::MailerSubscriber.subscribe!
end

it 'does not send the email' do
Expand Down
2 changes: 1 addition & 1 deletion guides/source/developers/events/overview.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ Spree::Config.events.adapter = "Spree::EventBus.new"

## Subscribing to events

`Spree::Event.subscribe` allows to subscribe to a certain event. The event name is mandatory, the optional block will be executed everytime the event is fired:
`Spree::Event.subscribe` allows to subscribe to a certain event. The event name is mandatory, the optional block will be executed every time the event is fired:

```ruby
Spree::Event.subscribe 'order_finalized' do |event|
Expand Down