diff --git a/app/services/gpo_confirmation_exporter.rb b/app/services/gpo_confirmation_exporter.rb index 009a245ed67..3c7e3e5c9f8 100644 --- a/app/services/gpo_confirmation_exporter.rb +++ b/app/services/gpo_confirmation_exporter.rb @@ -5,7 +5,6 @@ class GpoConfirmationExporter LINE_ENDING = "\r\n".freeze HEADER_ROW_ID = '01'.freeze CONTENT_ROW_ID = '02'.freeze - OTP_MAX_VALID_DAYS = IdentityConfig.store.usps_confirmation_max_days def initialize(confirmations) @confirmations = confirmations @@ -32,7 +31,7 @@ def make_header_row(num_entries) def make_entry_row(confirmation) now = current_date - due = confirmation.created_at + OTP_MAX_VALID_DAYS.days + due = confirmation.created_at + IdentityConfig.store.usps_confirmation_max_days.days entry = confirmation.entry service_provider = ServiceProvider.find_by(issuer: entry[:issuer]) diff --git a/app/services/gpo_reminder_sender.rb b/app/services/gpo_reminder_sender.rb index 3c1b1a72a91..d07a8d2d263 100644 --- a/app/services/gpo_reminder_sender.rb +++ b/app/services/gpo_reminder_sender.rb @@ -1,9 +1,13 @@ class GpoReminderSender def send_emails(for_letters_sent_before) + letter_eligible_range = + IdentityConfig.store.usps_confirmation_max_days.days.ago..for_letters_sent_before + profiles_due_for_reminder = Profile.joins(:gpo_confirmation_codes). where( - gpo_verification_pending_at: ..for_letters_sent_before, + gpo_verification_pending_at: letter_eligible_range, gpo_confirmation_codes: { reminder_sent_at: nil }, + deactivation_reason: [nil, :in_person_verification_pending], ) profiles_due_for_reminder.each do |profile| diff --git a/config/application.yml.default b/config/application.yml.default index 71aeeb71819..891145e997e 100644 --- a/config/application.yml.default +++ b/config/application.yml.default @@ -330,7 +330,7 @@ version_headers_enabled: false use_dashboard_service_providers: false use_kms: false usps_auth_token_refresh_job_enabled: false -usps_confirmation_max_days: 10 +usps_confirmation_max_days: 30 usps_ipp_password: '' usps_ipp_client_id: '' usps_ipp_root_url: '' diff --git a/spec/features/users/verify_profile_spec.rb b/spec/features/users/verify_profile_spec.rb index 4e61eb95d7a..9788a2428e1 100644 --- a/spec/features/users/verify_profile_spec.rb +++ b/spec/features/users/verify_profile_spec.rb @@ -34,7 +34,9 @@ end scenario 'OTP has expired' do - GpoConfirmationCode.first.update(code_sent_at: 11.days.ago) + GpoConfirmationCode.first.update( + code_sent_at: (IdentityConfig.store.usps_confirmation_max_days + 1).days.ago, + ) sign_in_live_with_2fa(user) fill_in t('idv.gpo.form.otp_label'), with: otp diff --git a/spec/forms/gpo_verify_form_spec.rb b/spec/forms/gpo_verify_form_spec.rb index 5de76c9ca60..9cf641f3b77 100644 --- a/spec/forms/gpo_verify_form_spec.rb +++ b/spec/forms/gpo_verify_form_spec.rb @@ -86,7 +86,13 @@ end context 'when OTP is expired' do - let(:code_sent_at) { 11.days.ago } + let(:expiration_days) { 10 } + let(:code_sent_at) { (expiration_days + 1).days.ago } + + before do + allow(IdentityConfig.store).to receive(:usps_confirmation_max_days). + and_return(expiration_days) + end it 'is invalid' do result = subject.submit diff --git a/spec/jobs/gpo_reminder_job_spec.rb b/spec/jobs/gpo_reminder_job_spec.rb index 59a2d6eb75f..5b3a3007d98 100644 --- a/spec/jobs/gpo_reminder_job_spec.rb +++ b/spec/jobs/gpo_reminder_job_spec.rb @@ -1,27 +1,50 @@ require 'rails_helper' RSpec.describe GpoReminderJob do - let(:wait_before_sending_reminder) { 14.days } + let(:days_before_sending_reminder) { 12 } + let(:max_days_ago_to_send_letter) { 27 } describe '#perform' do - subject(:perform) { job.perform(wait_before_sending_reminder.ago) } + subject(:perform) { job.perform(days_before_sending_reminder.days.ago) } let(:job) { GpoReminderJob.new } - let(:user) { create(:user, :with_pending_gpo_profile) } - let(:pending_profile) { user.pending_profile } + + let(:gpo_expired_user) { create(:user, :with_pending_gpo_profile) } + let(:due_for_reminder_user) { create(:user, :with_pending_gpo_profile) } + let(:not_yet_due_for_reminder_user) { create(:user, :with_pending_gpo_profile) } + let(:user_with_invalid_profile) { create(:user, :with_pending_gpo_profile) } + let(:job_analytics) { FakeAnalytics.new } before do - pending_profile.update( - gpo_verification_pending_at: wait_before_sending_reminder.ago, - ) + allow(IdentityConfig.store).to receive(:usps_confirmation_max_days). + and_return(max_days_ago_to_send_letter) allow(Analytics).to receive(:new).and_return(job_analytics) + + gpo_expired_user.gpo_verification_pending_profile.update( + gpo_verification_pending_at: (max_days_ago_to_send_letter + 1).days.ago, + ) + + due_for_reminder_user.gpo_verification_pending_profile.update( + gpo_verification_pending_at: days_before_sending_reminder.days.ago, + ) + + not_yet_due_for_reminder_user.gpo_verification_pending_profile.update( + gpo_verification_pending_at: (days_before_sending_reminder - 1).days.ago, + ) + + user_with_invalid_profile.gpo_verification_pending_profile.update( + gpo_verification_pending_at: days_before_sending_reminder.days.ago, + ) + user_with_invalid_profile.gpo_verification_pending_profile.deactivate(:password_reset) end - it 'sends reminder emails' do + it 'sends only one reminder email, to the correct user' do expect { perform }.to change { ActionMailer::Base.deliveries.count }.by(1) + expect(ActionMailer::Base.deliveries.last.to.first).to eq(due_for_reminder_user.email) expect(job_analytics).to have_logged_event( 'IdV: gpo reminder email sent', + user_id: due_for_reminder_user.uuid, ) end end diff --git a/spec/services/gpo_reminder_sender_spec.rb b/spec/services/gpo_reminder_sender_spec.rb index 1c5e328c7c9..4d8fbd89d28 100644 --- a/spec/services/gpo_reminder_sender_spec.rb +++ b/spec/services/gpo_reminder_sender_spec.rb @@ -1,5 +1,37 @@ require 'rails_helper' +RSpec.shared_examples 'sends no emails' do + it 'sends no emails' do + expect { subject.send_emails(time_due_for_reminder) }. + not_to change { ActionMailer::Base.deliveries.size } + end + + it 'logs no events' do + expect { subject.send_emails(time_due_for_reminder) }. + not_to change { fake_analytics.events.count } + end +end + +RSpec.shared_examples 'sends emails' do |expected_number_of_emails:, + expected_number_of_analytics_events: + expected_number_of_emails| + it "sends that user #{expected_number_of_emails} email(s)" do + expect { subject.send_emails(time_due_for_reminder) }. + to change { ActionMailer::Base.deliveries.size }.by(expected_number_of_emails) + end + + it 'logs the email events' do + subject.send_emails(time_due_for_reminder) + + expect(fake_analytics.events['IdV: gpo reminder email sent']&.size).to( + eq(expected_number_of_analytics_events), + ) + expect(fake_analytics.events['Email Sent']&.size).to( + eq(expected_number_of_emails), + ) + end +end + RSpec.describe GpoReminderSender do describe '#send_emails' do subject(:sender) { GpoReminderSender.new } @@ -18,7 +50,7 @@ let(:time_not_yet_due) { time_due_for_reminder + 1.day } let(:time_yesterday) { Time.zone.now - 1.day } - def set_gpo_verification_pending_at(to_time) + def set_gpo_verification_pending_at(user, to_time) user. gpo_verification_pending_profile. update(gpo_verification_pending_at: to_time) @@ -33,76 +65,62 @@ def set_reminder_sent_at(to_time) before { allow(Analytics).to receive(:new).and_return(fake_analytics) } context 'when no users need a reminder' do - before { set_gpo_verification_pending_at(time_not_yet_due) } + before { set_gpo_verification_pending_at(user, time_not_yet_due) } - it 'sends no emails' do - expect { subject.send_emails(time_due_for_reminder) }. - to change { ActionMailer::Base.deliveries.size }.by(0) - end - - it 'logs no events' do - expect { subject.send_emails(time_due_for_reminder) }. - not_to change { fake_analytics.events.count } - end + include_examples 'sends no emails' end - context 'when a user is due for a reminder' do - before { set_gpo_verification_pending_at(time_due_for_reminder) } - - it 'sends that user an email' do - expect { subject.send_emails(time_due_for_reminder) }. - to change { ActionMailer::Base.deliveries.size }.by(1) + context 'when a user has requested two letters' do + before do + set_gpo_verification_pending_at(user, time_due_for_reminder - 2.days) + new_confirmation_code = create(:gpo_confirmation_code) + user.gpo_verification_pending_profile.gpo_confirmation_codes << new_confirmation_code end - it 'logs an event' do + include_examples 'sends emails', expected_number_of_emails: 2 + + it 'updates the GPO verification code `reminder_sent_at`' do subject.send_emails(time_due_for_reminder) - expect(fake_analytics).to have_logged_event('IdV: gpo reminder email sent') + expect(gpo_confirmation_code.reminder_sent_at).to be_within(1.second).of(Time.zone.now) end + end + + context 'when a user is due for a reminder' do + before { set_gpo_verification_pending_at(user, time_due_for_reminder) } + + include_examples 'sends emails', expected_number_of_emails: 1 it 'updates the GPO verification code `reminder_sent_at`' do subject.send_emails(time_due_for_reminder) - expect(gpo_confirmation_code.reminder_sent_at).to be_within(1).of(Time.zone.now) + expect(gpo_confirmation_code.reminder_sent_at).to be_within(1.second).of(Time.zone.now) end context 'and the user has multiple emails' do let(:user) { create(:user, :with_pending_gpo_profile, :with_multiple_emails) } - it 'sends an email to all of them' do - expect { subject.send_emails(time_due_for_reminder) }. - to change { ActionMailer::Base.deliveries.size }.by(2) + include_examples 'sends emails', + expected_number_of_emails: 2, + expected_number_of_analytics_events: 1 + + it 'updates the GPO verification code `reminder_sent_at`' do + subject.send_emails(time_due_for_reminder) + + expect(gpo_confirmation_code.reminder_sent_at).to be_within(1.second).of(Time.zone.now) end end context 'but the user has cancelled gpo verification' do - before do - Idv::CancelVerificationAttempt.new(user: user).call - end + before { Idv::CancelVerificationAttempt.new(user: user).call } - it 'does not send that user an email' do - expect { subject.send_emails(time_due_for_reminder) }. - to change { ActionMailer::Base.deliveries.size }.by(0) - end - - it 'logs no events' do - expect { subject.send_emails(time_due_for_reminder) }. - not_to change { fake_analytics.events.count } - end + include_examples 'sends no emails' end context 'but a reminder has already been sent' do before { set_reminder_sent_at(time_yesterday) } - it 'does not send that user an email' do - expect { subject.send_emails(time_due_for_reminder) }. - to change { ActionMailer::Base.deliveries.size }.by(0) - end - - it 'logs no events' do - expect { subject.send_emails(time_due_for_reminder) }. - not_to change { fake_analytics.events.count } - end + include_examples 'sends no emails' end context 'but the user has completed gpo verification' do @@ -126,16 +144,39 @@ def set_reminder_sent_at(to_time) ).submit end - it 'does not send that user an email' do - expect { subject.send_emails(time_due_for_reminder) }. - to change { ActionMailer::Base.deliveries.size }.by(0) - end + include_examples 'sends no emails' + end - it 'logs no events' do - expect { subject.send_emails(time_due_for_reminder) }. - not_to change { fake_analytics.events.count } - end + context 'but the user has changed their password' do + before { user.gpo_verification_pending_profile.deactivate(:password_reset) } + + include_examples 'sends no emails' + end + end + + context 'when a user is due for a reminder from too long ago' do + let(:max_age_to_send_letter_in_days) { 42 } + + before do + set_gpo_verification_pending_at(user, (max_age_to_send_letter_in_days + 1).days.ago) + allow(IdentityConfig.store).to receive(:usps_confirmation_max_days). + and_return(max_age_to_send_letter_in_days) end + + include_examples 'sends no emails' + end + + context 'a user in the in-person flow who also requested a GPO letter' do + let(:user) { create(:user, :with_pending_in_person_enrollment) } + + before do + gpo_code = create(:gpo_confirmation_code) + user.pending_profile.gpo_confirmation_codes << gpo_code + user.pending_profile.gpo_verification_pending_at = time_due_for_reminder + user.pending_profile.save + end + + include_examples 'sends emails', expected_number_of_emails: 1 end end end diff --git a/spec/support/idv_examples/gpo_otp_verification.rb b/spec/support/idv_examples/gpo_otp_verification.rb index bfe415a650f..60bce85ef35 100644 --- a/spec/support/idv_examples/gpo_otp_verification.rb +++ b/spec/support/idv_examples/gpo_otp_verification.rb @@ -29,7 +29,9 @@ it 'renders an error for an expired GPO OTP' do sign_in_live_with_2fa(user) - gpo_confirmation_code.update(code_sent_at: 11.days.ago) + gpo_confirmation_code.update( + code_sent_at: (IdentityConfig.store.usps_confirmation_max_days + 1).days.ago, + ) fill_in t('idv.gpo.form.otp_label'), with: otp click_button t('idv.gpo.form.submit')