From 1dd15fb0aa62a44aac7c615ba0399243c1f24969 Mon Sep 17 00:00:00 2001 From: Seth Call Date: Fri, 17 Oct 2025 08:15:46 -0500 Subject: [PATCH] Add tests for all PLG emails --- ...trail_expires_reminder_columns_to_users.rb | 9 ++- .../jam_ruby/lib/email_profile_reminder.rb | 19 ++--- ruby/lib/jam_ruby/lib/gear_setup_reminder.rb | 6 +- .../jam_ruby/lib/group_session_reminder.rb | 4 +- ruby/lib/jam_ruby/lib/test_gear_reminder.rb | 8 +-- .../jam_ruby/lib/trial_expires_reminder.rb | 16 +++-- .../lib/trail_expires_reminder_spec.rb | 71 ++++++++++++------- 7 files changed, 84 insertions(+), 49 deletions(-) diff --git a/ruby/db/migrate/20250817162004_add_trail_expires_reminder_columns_to_users.rb b/ruby/db/migrate/20250817162004_add_trail_expires_reminder_columns_to_users.rb index 869fa84a2..6fb120530 100644 --- a/ruby/db/migrate/20250817162004_add_trail_expires_reminder_columns_to_users.rb +++ b/ruby/db/migrate/20250817162004_add_trail_expires_reminder_columns_to_users.rb @@ -3,7 +3,14 @@ execute "ALTER TABLE users ADD COLUMN trial_expires_reminder1_sent_at TIMESTAMP" execute "ALTER TABLE users ADD COLUMN trial_expires_reminder2_sent_at TIMESTAMP" execute "ALTER TABLE users ADD COLUMN trial_expires_reminder3_sent_at TIMESTAMP" - end + + # slide in some more production indexes + execute "CREATE INDEX index_users_on_first_music_session_at ON users USING btree (first_music_session_at)" + # subscription_sync_code + execute "CREATE INDEX index_users_on_subscription_sync_code ON users USING btree (subscription_sync_code)" + # first_certified_gear_at + execute "CREATE INDEX index_users_on_first_certified_gear_at ON users USING btree (first_certified_gear_at)" + end def self.down execute "ALTER TABLE users DROP COLUMN trial_expires_reminder1_sent_at" execute "ALTER TABLE users DROP COLUMN trial_expires_reminder2_sent_at" diff --git a/ruby/lib/jam_ruby/lib/email_profile_reminder.rb b/ruby/lib/jam_ruby/lib/email_profile_reminder.rb index ae928da0b..8c4f923e1 100644 --- a/ruby/lib/jam_ruby/lib/email_profile_reminder.rb +++ b/ruby/lib/jam_ruby/lib/email_profile_reminder.rb @@ -6,19 +6,20 @@ module JamRuby begin cutoff_date = Date.parse(Rails.application.config.profile_complete_reminders_effective_from_date) # Define a cutoff date for the profile completion emails #If the user has not updated their profile 1 day after signup, then send reminder email1 - reminder1_users.find_each do |user| + reminder1_users(cutoff_date).find_each do |user| + puts "reminder1_users user: #{user.id}" UserMailer.profile_complete_reminder1(user).deliver_now User.where(id: user.id).update_all(profile_complete_reminder1_sent_at: Time.now) end #If the user has not updated their profile 3 days after signup, then send reminder email2 - reminder2_users.find_each do |user| + reminder2_users(cutoff_date).find_each do |user| UserMailer.profile_complete_reminder2(user).deliver_now User.where(id: user.id).update_all(profile_complete_reminder2_sent_at: Time.now) end #If the user has not updated their profile 5 days after signup, then send reminder email3 - reminder3_users.find_each do |user| + reminder3_users(cutoff_date).find_each do |user| UserMailer.profile_complete_reminder3(user).deliver_now User.where(id: user.id).update_all(profile_complete_reminder3_sent_at: Time.now) end @@ -33,17 +34,19 @@ module JamRuby end def self.reminder1_users(cutoff_date) - EmailProfileReminder.prospect_users.where("users.created_at > ? AND users.created_at::date <= ? AND users.profile_complete_reminder1_sent_at IS NULL", cutoff_date, 1.day.ago.to_date) + # ensure that the user has had the account for at least one day + puts "reminder1_users cutoff_date: #{cutoff_date}" + EmailProfileReminder.prospect_users.where("users.created_at > ? AND users.profile_complete_reminder1_sent_at IS NULL AND (NOW() - users.created_at) > INTERVAL '1 day'", cutoff_date) end def self.reminder2_users(cutoff_date) - EmailProfileReminder.prospect_users.where("users.created_at > ? AND GREATEST(users.created_at::date, users.profile_complete_reminder1_sent_at::date) <= ? AND users.profile_complete_reminder1_sent_at IS NOT NULL AND users.profile_complete_reminder2_sent_at IS NULL", cutoff_date, 3.days.ago.to_date) + # ensure that the user has had the account for at least three days and guard against rapid back-to-back emails + EmailProfileReminder.prospect_users.where("users.created_at > ? AND users.profile_complete_reminder1_sent_at < ? AND users.profile_complete_reminder1_sent_at IS NOT NULL AND users.profile_complete_reminder2_sent_at IS NULL", cutoff_date, 2.days.ago) end def self.reminder3_users(cutoff_date) - EmailProfileReminder.prospect_users.where("users.created_at > ? AND GREATEST(users.created_at::date, users.profile_complete_reminder2_sent_at::date) <= ? AND users.profile_complete_reminder2_sent_at IS NOT NULL AND users.profile_complete_reminder3_sent_at IS NULL", cutoff_date, 5.days.ago.to_date) + # ensure that user has had the profile for 5 days and guard against rapid back-to-back emails + EmailProfileReminder.prospect_users.where("users.created_at > ? AND users.profile_complete_reminder2_sent_at < ? AND users.profile_complete_reminder2_sent_at IS NOT NULL AND users.profile_complete_reminder3_sent_at IS NULL", cutoff_date, 2.days.ago) end - - end end \ No newline at end of file diff --git a/ruby/lib/jam_ruby/lib/gear_setup_reminder.rb b/ruby/lib/jam_ruby/lib/gear_setup_reminder.rb index ff70db17e..4bdc2a0d8 100644 --- a/ruby/lib/jam_ruby/lib/gear_setup_reminder.rb +++ b/ruby/lib/jam_ruby/lib/gear_setup_reminder.rb @@ -32,15 +32,15 @@ module JamRuby end def self.reminder1_users(cutoff_date) - GearSetupReminder.prospect_users.where("users.created_at::date = ? AND users.created_at > ? AND users.gear_setup_reminder1_sent_at IS NULL", 1.day.ago.to_date, cutoff_date) + GearSetupReminder.prospect_users.where("users.created_at > ? AND users.gear_setup_reminder1_sent_at IS NULL AND (NOW() - users.created_at) > INTERVAL '1 day'", cutoff_date) end def self.reminder2_users(cutoff_date) - GearSetupReminder.prospect_users.where("users.created_at::date = ? AND users.created_at > ? AND users.gear_setup_reminder1_sent_at IS NOT NULL AND users.gear_setup_reminder2_sent_at IS NULL", 3.days.ago.to_date, cutoff_date) + GearSetupReminder.prospect_users.where("users.created_at > ? AND users.gear_setup_reminder1_sent_at IS NOT NULL AND users.gear_setup_reminder2_sent_at IS NULL AND users.gear_setup_reminder1_sent_at < ?", cutoff_date, 2.days.ago) end def self.reminder3_users(cutoff_date) - GearSetupReminder.prospect_users.where("users.created_at::date = ? AND users.created_at > ? AND users.gear_setup_reminder2_sent_at IS NOT NULL AND users.gear_setup_reminder3_sent_at IS NULL", 5.days.ago.to_date, cutoff_date) + GearSetupReminder.prospect_users.where("users.created_at > ? AND users.gear_setup_reminder2_sent_at IS NOT NULL AND users.gear_setup_reminder3_sent_at IS NULL AND users.gear_setup_reminder2_sent_at < ?", cutoff_date, 2.days.ago) end end diff --git a/ruby/lib/jam_ruby/lib/group_session_reminder.rb b/ruby/lib/jam_ruby/lib/group_session_reminder.rb index 12ef1ac9d..6910ea490 100644 --- a/ruby/lib/jam_ruby/lib/group_session_reminder.rb +++ b/ruby/lib/jam_ruby/lib/group_session_reminder.rb @@ -36,11 +36,11 @@ module JamRuby end def self.reminder2_users(cutoff_date) - GroupSessionReminder.prospect_users.where("users.created_at > ? AND users.group_session_reminder1_sent_at IS NOT NULL AND users.group_session_reminder2_sent_at IS NULL AND users.first_music_session_at::date = ?", cutoff_date, 3.days.ago.to_date) + GroupSessionReminder.prospect_users.where("users.created_at > ? AND users.group_session_reminder1_sent_at IS NOT NULL AND users.group_session_reminder2_sent_at IS NULL AND users.first_music_session_at < ? AND group_session_reminder1_sent_at < ?", cutoff_date, 3.days.ago, 1.day.ago) end def self.reminder3_users(cutoff_date) - GroupSessionReminder.prospect_users.where("users.created_at > ? AND users.group_session_reminder2_sent_at IS NOT NULL AND users.group_session_reminder3_sent_at IS NULL AND users.first_music_session_at::date = ?", cutoff_date, 5.days.ago.to_date) + GroupSessionReminder.prospect_users.where("users.created_at > ? AND users.group_session_reminder2_sent_at IS NOT NULL AND users.group_session_reminder3_sent_at IS NULL AND users.first_music_session_at < ? AND group_session_reminder2_sent_at < ?", cutoff_date, 5.days.ago, 1.day.ago) end end diff --git a/ruby/lib/jam_ruby/lib/test_gear_reminder.rb b/ruby/lib/jam_ruby/lib/test_gear_reminder.rb index 09b1737a9..0681e4c2f 100644 --- a/ruby/lib/jam_ruby/lib/test_gear_reminder.rb +++ b/ruby/lib/jam_ruby/lib/test_gear_reminder.rb @@ -27,19 +27,19 @@ module JamRuby end def self.prospect_users - User.where("users.first_music_session_at IS NULL") + User.where("users.first_music_session_at IS NULL AND first_certified_gear_at IS NOT NULL") end def self.reminder1_users(cutoff_date) - TestGearReminder.prospect_users.where("users.first_certified_gear_at::date = ? AND users.created_at >= ? AND users.test_gear_reminder1_sent_at IS NULL", 1.day.ago.to_date, cutoff_date) + TestGearReminder.prospect_users.where("users.created_at > ? AND users.test_gear_reminder1_sent_at IS NULL AND (NOW() - users.first_certified_gear_at) > INTERVAL '1 day'", cutoff_date) end def self.reminder2_users(cutoff_date) - TestGearReminder.prospect_users.where("users.first_certified_gear_at::date = ? AND users.created_at >= ? AND users.test_gear_reminder1_sent_at IS NOT NULL AND users.test_gear_reminder2_sent_at IS NULL", 3.days.ago.to_date, cutoff_date) + TestGearReminder.prospect_users.where("users.created_at > ? AND users.test_gear_reminder1_sent_at IS NOT NULL AND users.test_gear_reminder2_sent_at IS NULL AND users.test_gear_reminder1_sent_at < ?", cutoff_date, 2.days.ago) end def self.reminder3_users(cutoff_date) - TestGearReminder.prospect_users.where("users.first_certified_gear_at::date = ? AND users.created_at > ? AND users.test_gear_reminder2_sent_at IS NOT NULL AND users.test_gear_reminder3_sent_at IS NULL", 5.days.ago.to_date, cutoff_date) + TestGearReminder.prospect_users.where("users.created_at > ? AND users.test_gear_reminder2_sent_at IS NOT NULL AND users.test_gear_reminder3_sent_at IS NULL AND users.test_gear_reminder2_sent_at < ?", cutoff_date, 2.days.ago) end end end \ No newline at end of file diff --git a/ruby/lib/jam_ruby/lib/trial_expires_reminder.rb b/ruby/lib/jam_ruby/lib/trial_expires_reminder.rb index 2e9a43df3..6abe44dfc 100644 --- a/ruby/lib/jam_ruby/lib/trial_expires_reminder.rb +++ b/ruby/lib/jam_ruby/lib/trial_expires_reminder.rb @@ -8,24 +8,26 @@ module JamRuby class TrialExpiresReminder @@log = Logging.logger[TrialExpiresReminder] - FIRST_NOTIFICATION_CHECK = 1.day.ago.to_date - SECOND_NOTIFICATION_CHECK = 5.days.ago.to_date - THIRD_NOTIFICATION_CHECK = 9.days.ago.to_date + FIRST_NOTIFICATION_CHECK = 1.day.ago + # unused here, but just as a form of simple documentation... + # SECOND_NOTIFICATION_CHECK = 4.days (from the previous reminder) + # THIRD_NOTIFICATION_CHECK = 4.days (from the previous reminder) def self.prospect_users(cutoff_date) - User.where("(users.subscription_trial_ends_at IS NOT NULL AND users.subscription_trial_ends_at > ?)", cutoff_date) + User.where("(users.subscription_trial_ends_at IS NOT NULL AND users.created_at > ?)", cutoff_date) end + # trial_ended | in_trial def self.reminder1_users(cutoff_date) - prospect_users(cutoff_date).where("users.subscription_last_checked_at < ? AND users.subscription_trial_ends_at::date = ? AND users.trial_expires_reminder1_sent_at IS NULL", 1.day.ago, FIRST_NOTIFICATION_CHECK) + prospect_users(cutoff_date).where("users.subscription_sync_code = 'trial_ended' AND users.subscription_trial_ends_at < ? AND users.trial_expires_reminder1_sent_at IS NULL", FIRST_NOTIFICATION_CHECK) end def self.reminder2_users(cutoff_date) - prospect_users(cutoff_date).where("users.subscription_last_checked_at < ? AND users.subscription_trial_ends_at::date = ? AND trial_expires_reminder1_sent_at IS NOT NULL AND users.trial_expires_reminder2_sent_at IS NULL", 1.day.ago, SECOND_NOTIFICATION_CHECK) + prospect_users(cutoff_date).where("users.subscription_sync_code = 'trial_ended' AND trial_expires_reminder1_sent_at IS NOT NULL AND users.trial_expires_reminder2_sent_at IS NULL AND (NOW() - trial_expires_reminder1_sent_at) > INTERVAL '4 days'") end def self.reminder3_users(cutoff_date) - prospect_users(cutoff_date).where("users.subscription_last_checked_at < ? AND users.subscription_trial_ends_at::date = ? AND trial_expires_reminder2_sent_at IS NOT NULL AND users.trial_expires_reminder3_sent_at IS NULL", 1.day.ago, THIRD_NOTIFICATION_CHECK) + prospect_users(cutoff_date).where("users.subscription_sync_code = 'trial_ended' AND trial_expires_reminder2_sent_at IS NOT NULL AND users.trial_expires_reminder3_sent_at IS NULL AND (NOW() - trial_expires_reminder2_sent_at) > INTERVAL '4 days'") end def self.send_reminders diff --git a/ruby/spec/jam_ruby/lib/trail_expires_reminder_spec.rb b/ruby/spec/jam_ruby/lib/trail_expires_reminder_spec.rb index 9943e6368..27d395606 100644 --- a/ruby/spec/jam_ruby/lib/trail_expires_reminder_spec.rb +++ b/ruby/spec/jam_ruby/lib/trail_expires_reminder_spec.rb @@ -19,13 +19,19 @@ describe TrialExpiresReminder do ActionMailer::Base.deliveries.clear end + def no_more_emails_sent + UserMailer.deliveries.clear + TrialExpiresReminder.send_reminders + expect(ActionMailer::Base.deliveries.count).to eq(0) + end + it "sends reminder emails to users whose trials are about to expire" do user1.subscription_trial_ends_at = 1.days.from_now - user1.subscription_last_checked_at = 2.days.ago + user1.subscription_sync_code = 'trial_ended' user1.save! - user2.subscription_trial_ends_at = 1.days.ago - user2.subscription_last_checked_at = 2.days.ago + user2.subscription_trial_ends_at = 2.days.ago + user2.subscription_sync_code = 'trial_ended' user2.save! TrialExpiresReminder.send_reminders @@ -36,11 +42,14 @@ describe TrialExpiresReminder do expect(ActionMailer::Base.deliveries.last.subject).to include("Your free gold trial has expired, but you have great options to keep playing!") expect(user2.reload.trial_expires_reminder1_sent_at).not_to be_nil + + no_more_emails_sent end it "does not send reminder emails to users who have already received them" do - user1.subscription_trial_ends_at = 1.days.ago - user1.subscription_last_checked_at = 2.days.ago + user1.reload + user1.subscription_trial_ends_at = 1.days.ago + 1.hour + user1.subscription_sync_code = 'trial_ended' user1.trial_expires_reminder1_sent_at = Time.now user1.save! @@ -50,31 +59,38 @@ describe TrialExpiresReminder do end it "sends the second reminder email to users whose trials are about to expire" do - user1.subscription_trial_ends_at = 4.days.ago - user1.subscription_last_checked_at = 1.days.ago - user1.trial_expires_reminder1_sent_at = Time.now + user1.reload + user2.reload + + # pretend that the first reminder email was sent 2 days ago + user1.subscription_trial_ends_at = 4.days.ago + 1.hour + user1.subscription_sync_code = 'trial_ended' + user1.trial_expires_reminder1_sent_at = 5.days.ago user1.save! - user2.subscription_trial_ends_at = 5.days.ago - user2.subscription_last_checked_at = 1.days.ago + user2.subscription_trial_ends_at = 4.days.ago + 1.hour + user2.subscription_sync_code = 'trial_ended' user2.trial_expires_reminder1_sent_at = Time.now user2.save! TrialExpiresReminder.send_reminders expect(ActionMailer::Base.deliveries.count).to eq(1) - expect(ActionMailer::Base.deliveries.map(&:to).flatten).to include(user2.email) + expect(ActionMailer::Base.deliveries.map(&:to).flatten).to include(user1.email) # Check if the second reminder email was sent by verifying the subject expect(ActionMailer::Base.deliveries.last.subject).to include("Don’t forget to check your options to keep playing") - expect(user2.reload.trial_expires_reminder2_sent_at).not_to be_nil + expect(user1.reload.trial_expires_reminder2_sent_at).not_to be_nil + + no_more_emails_sent end it "sends the third reminder email to users whose trials are about to expire" do - user1.subscription_trial_ends_at = 10.days.ago - user1.subscription_last_checked_at = 1.days.ago - user1.trial_expires_reminder1_sent_at = 6.days.ago - user1.trial_expires_reminder2_sent_at = 4.days.ago + user1.reload + user1.subscription_trial_ends_at = 10.days.ago + 1.hour + user1.subscription_sync_code = 'trial_ended' + user1.trial_expires_reminder1_sent_at = 8.days.ago + user1.trial_expires_reminder2_sent_at = 9.days.ago user1.save! TrialExpiresReminder.send_reminders @@ -85,22 +101,28 @@ describe TrialExpiresReminder do expect(ActionMailer::Base.deliveries.last.subject).to include("One last reminder!") expect(user1.reload.trial_expires_reminder3_sent_at).not_to be_nil + + no_more_emails_sent end it "sends first and second and third reminder emails to users whose trials are about to expire" do + user1.reload + user2.reload + user3.reload + user1.subscription_trial_ends_at = 2.days.ago - user1.subscription_last_checked_at = 1.days.ago + user1.subscription_sync_code = 'trial_ended' user1.save! - user2.subscription_trial_ends_at = 6.days.ago - user2.subscription_last_checked_at = 1.days.ago - user2.trial_expires_reminder1_sent_at = 4.days.ago + user2.subscription_trial_ends_at = 2.days.ago + user2.trial_expires_reminder1_sent_at = 5.days.ago + user2.subscription_sync_code = 'trial_ended' user2.save! - user3.subscription_trial_ends_at = 10.days.ago - user3.subscription_last_checked_at = 2.days.ago - user3.trial_expires_reminder1_sent_at = 6.days.ago - user3.trial_expires_reminder2_sent_at = 4.days.ago + user3.subscription_trial_ends_at = 2.days.ago + user3.trial_expires_reminder1_sent_at = 8.days.ago + user3.trial_expires_reminder2_sent_at = 9.days.ago + user3.subscription_sync_code = 'trial_ended' user3.save! TrialExpiresReminder.send_reminders @@ -112,5 +134,6 @@ describe TrialExpiresReminder do expect(ActionMailer::Base.deliveries.count).to eq(3) expect(ActionMailer::Base.deliveries.map(&:to).flatten).to include(user1.email, user2.email, user3.email) + no_more_emails_sent end end \ No newline at end of file