Skip to content

LG-9564: Notify users when verify by mail codes expire 📣 #9475

Closed
matthinz wants to merge 33 commits intomainfrom
matthinz/9564-verify-by-mail-expiring
Closed

LG-9564: Notify users when verify by mail codes expire 📣 #9475
matthinz wants to merge 33 commits intomainfrom
matthinz/9564-verify-by-mail-expiring

Conversation

@matthinz
Copy link
Contributor

@matthinz matthinz commented Oct 27, 2023

🎫 Ticket

LG-9564

🛠 Summary of changes

This PR adds a new cron job, SendGpoCodeExpirationNoticesJob, that sends emails to users who have requested a Verify by mail code but never entered it within the 30 day validity window.

Ultimately, this job will run daily, but for now we're not enabling it so we can run it more intentionally at first.

There is a lot to consider when picking users to email! We don’t email anyone who:

  • Already has received an email for a particular expired code
  • Already managed to get an active profile in some other way in the intervening 30 days
  • Reset their password or otherwise deactivated their GPO pending profile
  • Requested a new code more recently (so if you still have a valid code out there, we don’t notify you that your prior one expired)
  • Is currently rate limited for requesting new GPO letters

✨ Date math ✨

The ticket says:

Users who have not completed verify by mail and whose most-recent verify by mail letter is >31 days old should be “expired”

Confirmation codes expire 30 days after the code_sent_at timestamp on GpoConfirmationCode.

When this job runs, it will look for any confirmation codes eligible for notification within a small window of about 31 days ago. This is to:

  • Avoid emailing everyone who’s ever had a code expire in the history of Login.gov when we initially deploy
  • Keep the working set small for the more advanced filtering we have to do (like checking if a code is the most recent for a profile)

To determine the window, we first take the timestamp when the job is running and remove the time component (in prod this will be midnight UTC, but your time zone may vary). Call this T.

The upper bound of the window (the highest value for code_sent_at for which we will consider sending a notification) is T - 30 days.

The lower bound of the window (the lowest value for code_sent_at for which we will consider sending a notification) is T - 32 days.

That is, the window is 48 hours, running from midnight UTC - midnight UTC.

The job is meant to run daily, meaning that there will likely be overlap of these windows on subsequent days. But since we update the expiration_notice_sent_at after sending, we can filter out codes for which we've sent notifications and avoid sending duplicate notifications.

📜 Testing Plan

  • Begin IdV, taking the GPO route and requesting a letter.
  • Once you have your GPO code, get in your time machine and move forward 15 days
  • Use the Rails console to run the job: SendGpoCodeExpirationNoticesJob.new.perform
  • Verify you do not receive an email
  • Request another GPO code
  • Once you have your GPO code, get in your time machine and move forward another 16 days (You should now be 31 days after your first request for a letter)
  • Use the Rails console to run the job: SendGpoCodeExpirationNoticesJob.new.perform
  • Verify that you do not receive an email
  • Use your time machine to move forward another 15 days
  • Use the Rails console to run the job: SendGpoCodeExpirationNoticesJob.new.perform
  • Verify you receive an email
  • Run the job again
  • Verify you do not receive a second email

(There are a number of edge cases that I tried to capture in the spec file as well.)

If you have not yet received your GFE time machine

It's not ideal, but you can simulate the required increase in entropy if you must. First you should probably tweak your application.yml to make your life easier:

  # Allow re-requesting GPO letters same-day
  minimum_wait_before_another_usps_letter_in_hours: 0

Then, in the Rails console:

# Clear out any codes you have sitting in your local db to reset to a baseline state
GpoConfirmationCode.delete_all

# STOP. Go to the IDP and request a letter

# Grab the code you requested
first_code = GpoConfirmationCode.order(code_sent_at: :desc).first

# Simulate 15 days passing
first_code.update(code_sent_at: 15.days.ago)

# Run the job (should not email)
SendGpoCodeExpirationNoticesJob.new.perform

# STOP. Go request another letter, then come back here and grab your new code
second_code = GpoConfirmationCode.order(code_sent_at: :desc).first

# Simulate 16 more days passing
# (first_code is now expired, but second_code is still active)
first_code.update(code_sent_at: 31.days.ago)
second_code.update(code_sent_at: 16.days.ago)

# Run the job (should not email)
SendGpoCodeExpirationNoticesJob.new.perform

# Simulate 14 more days passing 
# (second_code is now expired, but not within the notification window)
first_code.update(code_sent_at: 45.days.ago)
second_code.update(code_sent_at: 30.days.ago)

# Run the job again (should not email)
SendGpoCodeExpirationNoticesJob.new.perform

# Simulate 1 more day passing
# (both codes are expired, and now second_code is within the notification window)
first_code.update(code_sent_at: 46.days.ago)
second_code.update(code_sent_at: 31.days.ago)

# Run the job (should send 1 email with second_code.code_sent_at referenced in the body)
SendGpoCodeExpirationNoticesJob.new.perform

# Run the job again (should not email, since we already sent a notification for second_code)
SendGpoCodeExpirationNoticesJob.new.perform

👀 Screenshots

English Screenshot 2023-10-27 at 2 02 37 PM
French Screenshot 2023-10-27 at 2 02 51 PM
Spanish Screenshot 2023-10-27 at 2 02 43 PM

end

def send_email_to_all_addresses(user_mailer_template)
def send_email_to_all_addresses(user_mailer_template, **args)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor change here to support forwarding args to UserMailer

@matthinz matthinz changed the title LG-9564: Notify users when GPO access codes expire 📣 LG-9564: Notify users when verify by mail codes expire 📣 Oct 27, 2023
@matthinz matthinz requested a review from a team October 27, 2023 22:45
Comment on lines +5 to +6
add_column :usps_confirmation_codes, :expiration_notice_sent_at, :datetime, precision: nil
add_index :usps_confirmation_codes, :expiration_notice_sent_at, algorithm: :concurrently
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Migration to add an indexed column to usps_confirmation_codes to track the date/time we sent an expiration notice for a single code.

matthinz and others added 4 commits October 30, 2023 10:59
And name the methods like the thing they are doing!
Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
Copy link
Contributor

@soniaconnolly soniaconnolly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of positive comments, couple minor suggestions. Will run it locally according to your detailed test plan, and approve if all goes well.

codes_to_send_notifications_for.find_each do |code|
user = code.profile.user

user.send_email_to_all_addresses(:gpo_code_expired, code_sent_at: code.code_sent_at)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I commented on the ticket, we are assuming here that the config value max_mail_events_window_in_days, currently set to 30, will always be less than the code expiration, currently set to 31. I still think that we should check that assumption somewhere so we're not accidentally sending users emails inviting them to restart IdV if they're rate limited in the future when someone changes the config value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added an additional condition, user_is_not_rate_limited_for_gpo in 072f4f4 (and fixed a bug in it in ab6c876). The idea is that we just don't email folks who are currently rate limited for GPO.

Copy link
Contributor

@soniaconnolly soniaconnolly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might want to merge in main to sync up schemas. When I did a make update on this branch it left me with an edited schema.rb that looked like it deleted the index you had added. 🤷

Followed your test plan, and everything went as expected except that when I clicked on the email link to Verify my Identity, it took me to the Enter Code page. Even when I signed out and back in. Maybe because my user's gpo_verification_pending_at was not updated to match the edits to the GpoConfirmationCodes. Not sure if there's anything that needs to change for that.

Checked the email in all three languages, looks good.

@matthinz
Copy link
Contributor Author

matthinz commented Nov 2, 2023

Closing this for now, as there has been a little scope creep. I'm going to follow up with smaller PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants