diff --git a/app/jobs/follows/send_email_notification_job.rb b/app/jobs/follows/send_email_notification_job.rb index 5ab1b462e..eb8900d52 100644 --- a/app/jobs/follows/send_email_notification_job.rb +++ b/app/jobs/follows/send_email_notification_job.rb @@ -1,3 +1,4 @@ +# @TODO: to be removed in favor of `app/workers/follows/send_email_notification_worker.rb` module Follows class SendEmailNotificationJob < ApplicationJob queue_as :send_follow_email_notification diff --git a/app/mailers/notify_mailer.rb b/app/mailers/notify_mailer.rb index 227b38faf..272e25f10 100644 --- a/app/mailers/notify_mailer.rb +++ b/app/mailers/notify_mailer.rb @@ -1,4 +1,8 @@ class NotifyMailer < ApplicationMailer + SUBJECTS = { + new_follower_email: "just followed you on dev.to".freeze + }.freeze + def new_reply_email(comment) @user = comment.parent_user return if RateLimitChecker.new.limit_by_email_recipient_address(@user.email) @@ -15,7 +19,7 @@ class NotifyMailer < ApplicationMailer @follower = follow.follower @unsubscribe = generate_unsubscribe_token(@user.id, :email_follower_notifications) - mail(to: @user.email, subject: "#{@follower.name} just followed you on dev.to") + mail(to: @user.email, subject: "#{@follower.name} #{SUBJECTS[__method__]}") end def new_mention_email(mention) diff --git a/app/models/follow.rb b/app/models/follow.rb index 3623f6db6..9b490442c 100644 --- a/app/models/follow.rb +++ b/app/models/follow.rb @@ -46,7 +46,7 @@ class Follow < ApplicationRecord def send_email_notification return unless followable.class.name == "User" && followable.email? - Follows::SendEmailNotificationJob.perform_later(id) + Follows::SendEmailNotificationWorker.perform_async(id) end def modify_chat_channel_status diff --git a/app/models/user.rb b/app/models/user.rb index 163b9542a..cb3005c5d 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -451,6 +451,11 @@ class User < ApplicationRecord credits.unspent.size >= num_credits_needed end + def receives_follower_email_notifications? + email.present? && + email_follower_notifications + end + private def index_id diff --git a/app/workers/follows/send_email_notification_worker.rb b/app/workers/follows/send_email_notification_worker.rb new file mode 100644 index 000000000..351f07c96 --- /dev/null +++ b/app/workers/follows/send_email_notification_worker.rb @@ -0,0 +1,17 @@ +module Follows + class SendEmailNotificationWorker + include Sidekiq::Worker + sidekiq_options queue: :mailers, retry: 10 + + def perform(follow_id, mailer = NotifyMailer.name) + follow = Follow.find_by(id: follow_id, followable_type: "User") + return unless follow&.followable.present? && follow.followable.receives_follower_email_notifications? + + return if EmailMessage.where(user_id: follow.followable_id). + where("sent_at > ?", rand(15..35).hours.ago). + where("subject LIKE ?", "%#{NotifyMailer::SUBJECTS[:new_follower_email]}").exists? + + mailer.constantize.new_follower_email(follow).deliver + end + end +end diff --git a/spec/models/follow_spec.rb b/spec/models/follow_spec.rb index 4f85b4213..554b2ffcc 100644 --- a/spec/models/follow_spec.rb +++ b/spec/models/follow_spec.rb @@ -26,10 +26,10 @@ RSpec.describe Follow, type: :model do end.to have_enqueued_job(Follows::CreateChatChannelJob) end - it "enqueues send notification job" do + it "enqueues send notification worker" do expect do described_class.create(follower: user, followable: user_2) - end.to have_enqueued_job(Follows::SendEmailNotificationJob) + end.to change(Follows::SendEmailNotificationWorker.jobs, :size).by(1) end end @@ -73,7 +73,7 @@ RSpec.describe Follow, type: :model do it "sends an email notification" do user_2.update_column(:email_follower_notifications, true) expect do - perform_enqueued_jobs do + Sidekiq::Testing.inline! do described_class.create!(follower: user, followable: user_2) end end.to change(EmailMessage, :count).by(1) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index ccfa4a52e..25df102d5 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -727,4 +727,21 @@ RSpec.describe User, type: :model do expect(user.enough_credits?(1)).to be(true) end end + + describe "#receives_follower_email_notifications?" do + it "returns false if user has no email" do + user.assign_attributes(email: nil) + expect(user.receives_follower_email_notifications?).to be(false) + end + + it "returns false if user opted out from follower notifications" do + user.assign_attributes(email_follower_notifications: false) + expect(user.receives_follower_email_notifications?).to be(false) + end + + it "returns true if user opted in from follower notifications and has an email" do + user.assign_attributes(email_follower_notifications: true) + expect(user.receives_follower_email_notifications?).to be(true) + end + end end diff --git a/spec/workers/follows/send_email_notification_worker_spec.rb b/spec/workers/follows/send_email_notification_worker_spec.rb new file mode 100644 index 000000000..1736334c8 --- /dev/null +++ b/spec/workers/follows/send_email_notification_worker_spec.rb @@ -0,0 +1,56 @@ +require "rails_helper" + +class MockMailer + def self.deliver; end +end + +RSpec.describe Follows::SendEmailNotificationWorker, type: :worker do + subject(:worker) { described_class } + + let_it_be(:user) { create(:user) } + let_it_be(:user2) { create(:user) } + let_it_be(:follow) { create(:follow, follower: user, followable: user2) } + + describe "#perform" do + before do + allow(NotifyMailer).to receive(:new_follower_email).and_return(MockMailer) + allow(MockMailer).to receive(:deliver) + worker.perform_async(follow_id) + end + + context "with follow" do + let(:follow_id) { follow.id } + + it "sends a new_follower_email" do + user2.update_column(:email_follower_notifications, true) + worker.drain + + expect(MockMailer).to have_received(:deliver).once + end + + it "doesn't send an email if user has disabled notifications" do + user2.update_column(:email_follower_notifications, false) + worker.drain + + expect(MockMailer).not_to have_received(:deliver) + end + + it "doesn't create an EmailMessage if it already exists" do + subject = "#{user.username} just followed you on dev.to" + EmailMessage.create!(user_id: user2.id, sent_at: Time.current, subject: subject) + + worker.drain + + expect(MockMailer).not_to have_received(:deliver) + end + end + + context "without follow" do + let(:follow_id) { nil } + + it "does not break" do + expect { worker.drain }.not_to raise_error + end + end + end +end