Skip to content

Jmax/lg 9565 two week gpo letter reminder email#9001

Merged
jmax-gsa merged 36 commits intomainfrom
jmax/LG-9565-two-week-gpo-letter-reminder-email
Aug 21, 2023
Merged

Jmax/lg 9565 two week gpo letter reminder email#9001
jmax-gsa merged 36 commits intomainfrom
jmax/LG-9565-two-week-gpo-letter-reminder-email

Conversation

@jmax-gsa
Copy link
Contributor

@jmax-gsa jmax-gsa commented Aug 14, 2023

🎫 Ticket

LG-9565

🛠 Summary of changes

Added a new reminder email for users who requested a GPO letter two weeks ago.
Added task to send such letters daily.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

  • Visually validate the email's layout and copy using Rails' email preview mechanism
  • (In a sandbox or local dev environment) Manually create two users in GPO verification. Use the Rails console to back-date the gpo_verification_pending_at column for one user's pending profile. Run the GpoReminderJob from the rails console (GpoReminderJob.new(14.days.ago), and verify that the expected email was sent to the user that you backdated, and that no email is sent to the other user.
  • Run the GpoReminderJob again, and verify that no emails are sent.

👀 Screenshots

English:

Screenshot 2023-08-15 at 6 28 13 PM

Spanish:

Screenshot 2023-08-15 at 6 28 43 PM

French:

Screenshot 2023-08-15 at 6 29 09 PM

@jmax-gsa jmax-gsa force-pushed the jmax/LG-9565-two-week-gpo-letter-reminder-email branch 2 times, most recently from ad28efa to bc9636b Compare August 15, 2023 16:56
@jmax-gsa jmax-gsa marked this pull request as ready for review August 15, 2023 22:52
@jmax-gsa jmax-gsa requested review from a team, Kamal-Munshi and zachmargolis August 15, 2023 22:55
@jmax-gsa jmax-gsa force-pushed the jmax/LG-9565-two-week-gpo-letter-reminder-email branch from b4b8379 to a44baba Compare August 16, 2023 16:33
Comment on lines 245 to 253
Copy link
Contributor

Choose a reason for hiding this comment

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

The Spanish translation is using informal you, and the French translation below is using formal you. I'll add this to the Translations Quality Tracker doc.

@soniaconnolly
Copy link
Contributor

(you will need to upload the attached Yaml file during ID upload in order to trigger the GPO flow.)

Out of curiosity, where is the attached yaml file? I just click "verify by mail instead" on the Phone step. Usually yaml files are to trigger in person proofing.

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.

Tested this out, works as described. There weren't instructions for how to run the GpoReminderJob, so I ran GpoReminderJob.new.perform(14.days.ago).

Not approving because I ran the job again and got a second email. I thought we were limiting it to one reminder per user.

Should the 14 days be a config setting? Or maybe that's a future PR - make the job send emails for whatever intervals appear in a config setting.

@jmax-gsa jmax-gsa requested a review from soniaconnolly August 17, 2023 20:16
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.

LGTM, with a couple of comments.

Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering if we need an index here. Are we querying the usps_confirmation_codes table by when reminders were sent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we are querying that, although it's not immediately obvious. The index is for the benefit of the code in GpoReminderSender, where we do a join that this index helps.

jmax-gsa and others added 25 commits August 21, 2023 12:20
changelog: User-Facing Improvement, Identity Verification, Added GPO reminder email
Fixed incorrect help link.
Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
Co-authored-by: Zach Margolis <zachmargolis@users.noreply.github.com>
Cleaned up I18n HTML handling per Zach.
Stopped passing analytics in, because it's unfashionable.
Changed `@date_letter_was_sent` to `@gpo_verification_pending_at` in
gpo letter reminder email template. More clearly states the value
being dealt with.
@jmax-gsa jmax-gsa force-pushed the jmax/LG-9565-two-week-gpo-letter-reminder-email branch from 7ea4caa to e563453 Compare August 21, 2023 16:20
@jmax-gsa jmax-gsa merged commit 7b14135 into main Aug 21, 2023
@jmax-gsa jmax-gsa deleted the jmax/LG-9565-two-week-gpo-letter-reminder-email branch August 21, 2023 16:46
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