From 9b15329110eb652d20bd8f48b2b2b19c40e6d99f Mon Sep 17 00:00:00 2001 From: Ben Halpern Date: Wed, 10 Apr 2024 10:39:15 -0400 Subject: [PATCH] Adjust digest rate if recent clicks tracked (#20838) * Adjust digest rate if recent clicks tracked * Fix issues * Fix issues * Fix issues * Fiddle with tests * Fix syntax * Fiddle with tests * Fiddle with tests * Fiddle with tests * Fiddle with tests * Fiddle with tests * Fiddle with tests * Fiddle with tests * Remove .env.test symlink (#20839) * Update the mdx vscode extension (#20840) * Update @cypress/code-coverage to version 3.12.30 (#20803) Co-authored-by: depfu[bot] <23717796+depfu[bot]@users.noreply.github.com> Co-authored-by: Mac Siri * Update ruby VSCode extensions for debugging (#20816) The 'rebornix.Ruby' extension is deprecated. I replaced with the 'Shopify.ruby-lsp' extension since VSCode's Ruby docs pointed users to ruby-lsp, see https://code.visualstudio.com/docs/languages/ruby. * Update esbuild to version 0.19.12 (#20841) Co-authored-by: depfu[bot] <23717796+depfu[bot]@users.noreply.github.com> * Update eslint to version 8.57.0 (#20853) Co-authored-by: depfu[bot] <23717796+depfu[bot]@users.noreply.github.com> * fix: remove listings option from views/dashboards/_actions_mobile.html.erb (#20849) Co-authored-by: Mac Siri * Update eslint-import-resolver-webpack to version 0.13.8 (#20854) Co-authored-by: depfu[bot] <23717796+depfu[bot]@users.noreply.github.com> * add timestamp to users roles table (#20844) * Update eslint-plugin-import to version 2.29.1 (#20855) Co-authored-by: depfu[bot] <23717796+depfu[bot]@users.noreply.github.com> * Adjust tests to account for clicks * Adjust tests to account for clicks --------- Co-authored-by: Mac Siri Co-authored-by: Meredith Co-authored-by: depfu[bot] <23717796+depfu[bot]@users.noreply.github.com> Co-authored-by: Gabriel Quaresma Co-authored-by: Philip How --- .../email_digest_article_collector.rb | 17 ++++- .../email_digest_article_collector_spec.rb | 63 +++++++++++++++++++ 2 files changed, 77 insertions(+), 3 deletions(-) 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