diff --git a/app/services/email_digest_article_collector.rb b/app/services/email_digest_article_collector.rb index 254319698..ea6693228 100644 --- a/app/services/email_digest_article_collector.rb +++ b/app/services/email_digest_article_collector.rb @@ -4,6 +4,7 @@ class EmailDigestArticleCollector ARTICLES_TO_SEND = "EmailDigestArticleCollector#articles_to_send".freeze RESULTS_COUNT = 7 # Winner of digest_count_03_18 field test + CLICK_LOOKBACK = 30 def initialize(user) @user = user @@ -48,12 +49,22 @@ class EmailDigestArticleCollector # rubocop:enable Metrics/BlockLength end - private - def should_receive_email? return true unless last_email_sent - last_email_sent.before? Settings::General.periodic_email_digest.days.ago + email_sent_within_lookback_period = last_email_sent >= Settings::General.periodic_email_digest.days.ago + return false if email_sent_within_lookback_period && !recent_tracked_click? + + true + end + + private + + def recent_tracked_click? + @user.email_messages + .where(mailer: "DigestMailer#digest_email") + .where("sent_at > ?", CLICK_LOOKBACK.days.ago) + .where.not(clicked_at: nil).any? end def last_email_sent diff --git a/spec/services/email_digest_article_collector_spec.rb b/spec/services/email_digest_article_collector_spec.rb index 9a3f641cc..2be7ee521 100644 --- a/spec/services/email_digest_article_collector_spec.rb +++ b/spec/services/email_digest_article_collector_spec.rb @@ -78,4 +78,67 @@ RSpec.describe EmailDigestArticleCollector, type: :service do end end end + + describe "#should_receive_email?" do + let(:user) { create(:user) } + let(:collector) { described_class.new(user) } + + before do + Settings::General.periodic_email_digest = 3 + end + + context "when the user clicked the last email within the lookback period" do + it "returns true" do + Ahoy::Message.create(mailer: "DigestMailer#digest_email", + user_id: user.id, sent_at: 2.days.ago, clicked_at: 1.day.ago) + expect(collector.should_receive_email?).to be true + end + end + + context "when the user has not received any emails" do + it "returns true" do + expect(collector.should_receive_email?).to be true + end + end + + context "when the last email was received outside the periodic email digest days" do + it "returns true" do + Ahoy::Message.create(mailer: "DigestMailer#digest_email", + user_id: user.id, sent_at: 4.days.ago) + expect(collector.should_receive_email?).to be true + end + end + + context "when the last email was received within the periodic email digest days without click" do + it "returns false" do + Ahoy::Message.create(mailer: "DigestMailer#digest_email", + user_id: user.id, sent_at: 2.days.ago) + expect(collector.should_receive_email?).to be false + end + end + + context "when the last email was received just before the periodic email digest days limit" do + it "returns true" do + Ahoy::Message.create(mailer: "DigestMailer#digest_email", + user_id: user.id, sent_at: 3.days.ago) + expect(collector.should_receive_email?).to be true + end + end + + context "when the last email was clicked but outside the lookback period" do + it "returns true" do + Ahoy::Message.create(mailer: "DigestMailer#digest_email", + user_id: user.id, sent_at: 33.days.ago, clicked_at: 32.days.ago) + expect(collector.should_receive_email?).to be true + end + + it "returns false when there is a sent at within the thredhold" do + Ahoy::Message.create(mailer: "DigestMailer#digest_email", + user_id: user.id, sent_at: 33.days.ago, clicked_at: 32.days.ago) + Ahoy::Message.create(mailer: "DigestMailer#digest_email", + user_id: user.id, sent_at: 2.days.ago) + expect(collector.should_receive_email?).to be false + end + end + end end