FIX: prevent sending multiple summary emails due to Sidekiq delays

FIX: prevent sending multiple summary emails due to Sidekiq delays

diff --git a/app/jobs/regular/user_email.rb b/app/jobs/regular/user_email.rb
index 5821123..7e7d8f5 100644
--- a/app/jobs/regular/user_email.rb
+++ b/app/jobs/regular/user_email.rb
@@ -97,7 +97,12 @@ module Jobs
         return skip_message(SkippedEmailLog.reason_types[:user_email_user_suspended_not_pm])
       end
 
-      return if user.staged && type.to_s == "digest"
+      if type.to_s == "digest"
+        return if user.staged
+        return if user.last_emailed_at &&
+          user.last_emailed_at >
+            (user.user_option&.digest_after_minutes || SiteSetting.default_email_digest_frequency.to_i).minutes.ago
+      end
 
       seen_recently = (user.last_seen_at.present? && user.last_seen_at > SiteSetting.email_time_window_mins.minutes.ago)
       seen_recently = false if always_email_regular?(user, type) || always_email_private_message?(user, type) || user.staged
diff --git a/spec/jobs/user_email_spec.rb b/spec/jobs/user_email_spec.rb
index 6218719..16d0e65 100644
--- a/spec/jobs/user_email_spec.rb
+++ b/spec/jobs/user_email_spec.rb
@@ -24,16 +24,43 @@ describe Jobs::UserEmail do
     expect { Jobs::UserEmail.new.execute(type: :no_method, user_id: user.id) }.to raise_error(Discourse::InvalidParameters)
   end
 
-  it "doesn't call the mailer when the user is missing" do
-    Jobs::UserEmail.new.execute(type: :digest, user_id: 1234)
+  context 'digest can be generated' do
+    let(:user) { Fabricate(:user, last_seen_at: 8.days.ago, last_emailed_at: 8.days.ago) }
+    let!(:popular_topic) { Fabricate(:topic, user: Fabricate(:admin), created_at: 1.hour.ago) }
 
-    expect(ActionMailer::Base.deliveries).to eq([])
-  end
+    it "doesn't call the mailer when the user is missing" do
+      Jobs::UserEmail.new.execute(type: :digest, user_id: 1234)
+      expect(ActionMailer::Base.deliveries).to eq([])
+    end
+
+    it "doesn't call the mailer when the user is staged" do
+      staged.update_attributes!(last_seen_at: 8.days.ago, last_emailed_at: 8.days.ago)
+      Jobs::UserEmail.new.execute(type: :digest, user_id: staged.id)
+      expect(ActionMailer::Base.deliveries).to eq([])
+    end
+
+    context 'not emailed recently' do
+      before do
+        user.update_attributes!(last_emailed_at: 8.days.ago)
+      end
+
+      it "calls the mailer when the user exists" do
+        Jobs::UserEmail.new.execute(type: :digest, user_id: user.id)
+        expect(ActionMailer::Base.deliveries).to_not be_empty
+      end
+    end
 
-  it "doesn't call the mailer when the user is staged" do
-    Jobs::UserEmail.new.execute(type: :digest, user_id: staged.id)
+    context 'recently emailed' do
+      before do
+        user.update_attributes!(last_emailed_at: 2.hours.ago)
+        user.user_option.update_attributes!(digest_after_minutes: 1.day.to_i / 60)
+      end
 
-    expect(ActionMailer::Base.deliveries).to eq([])
+      it 'skips sending digest email' do
+        Jobs::UserEmail.new.execute(type: :digest, user_id: user.id)
+        expect(ActionMailer::Base.deliveries).to eq([])
+      end
+    end
   end
 
   context "bounce score" do
@@ -602,5 +629,4 @@ describe Jobs::UserEmail do
     end
 
   end
-
 end

GitHub sha: 399e937a

1 Like

This commit has been mentioned on Discourse Meta. There might be relevant details there:

This commit has been mentioned on Discourse Meta. There might be relevant details there: