Skip to content

Commit 5b5ac38

Browse files
committed
Add rudimentary protection against double-send Welcome
Adds a check for prior welcome notifications before delivering the welcome email. Checking for the presence of a notification record at may prevent any welcome email being sent if multiple notification records are created prior to processing the notification event job for the first.
1 parent 4a73cf0 commit 5b5ac38

File tree

3 files changed

+36
-2
lines changed

3 files changed

+36
-2
lines changed

app/models/notification_event.rb

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,22 @@ def self.inherited(notifier)
2929
notifier.const_set :Notification, Class.new(Notification)
3030
end
3131

32+
def deliver_to?(recipient)
33+
true
34+
end
35+
3236
# CommentNotifier.deliver(User.all)
3337
# CommentNotifier.deliver(User.all, priority: 10)
3438
# CommentNotifier.deliver(User.all, queue: :low_priority)
3539
# CommentNotifier.deliver(User.all, wait: 5.minutes)
3640
# CommentNotifier.deliver(User.all, wait_until: 1.hour.from_now)
37-
def deliver(recipients = nil, enqueue_job: true, **options)
41+
def deliver(given_recipients = nil, enqueue_job: true, **options)
3842
validate!
3943

44+
recipients = Array.wrap(given_recipients)
45+
4046
transaction do
41-
recipients_attributes = Array.wrap(recipients).map do |recipient|
47+
recipients_attributes = recipients.map do |recipient|
4248
recipient_attributes_for(recipient)
4349
end
4450

app/notifiers/welcome_notifier.rb

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,16 @@ def self.deliver_to(user, **)
66
def deliver_notification(notification)
77
user = notification.recipient
88

9+
if !deliver_to?(user)
10+
Rails.logger.info "#[#{self.class}] Skipping delivery for: #{user.class.name}##{user.id}"
11+
Honeybadger.event("empty_notification", notifier: self.class.name, recipient: "#{user.class.name}##{user.id}")
12+
return false
13+
end
14+
915
Emails::UserMailer.welcome(user).deliver_later
1016
end
17+
18+
def deliver_to?(recipient)
19+
Notification.joins(:notification_event).where(recipient: recipient, notification_event: {type: self.class.name}).count < 2
20+
end
1121
end
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
require "rails_helper"
2+
3+
RSpec.describe WelcomeNotifier do
4+
describe ".deliver_to" do
5+
it "sends at most once per user" do
6+
allow(Emails::UserMailer).to receive(:welcome).and_return(double(deliver_later: nil))
7+
8+
user = FactoryBot.create(:user, :confirmed)
9+
10+
WelcomeNotifier.deliver_to(user)
11+
perform_enqueued_jobs
12+
WelcomeNotifier.deliver_to(user)
13+
perform_enqueued_jobs
14+
15+
expect(Emails::UserMailer).to have_received(:welcome).with(user).once
16+
end
17+
end
18+
end

0 commit comments

Comments
 (0)