Jmax/LG-9565 fix GPO reminder query

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])

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|

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: ''

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

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

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

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

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')