From f1168169d1001064e03cdd59219b4155443fbe96 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Wed, 2 Aug 2023 17:44:51 -0400 Subject: [PATCH 01/36] Created mailer method, preview, and placeholder for content. --- app/mailers/user_mailer.rb | 6 +++++ app/views/user_mailer/gpo_reminder.html.erb | 25 ++++++++++++++++++++ spec/mailers/previews/user_mailer_preview.rb | 7 ++++++ 3 files changed, 38 insertions(+) create mode 100644 app/views/user_mailer/gpo_reminder.html.erb diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 437e3e29414..6e4e2aa3eb1 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -391,6 +391,12 @@ def suspended_reset_password end end + def gpo_reminder + with_user_locale(user) do + mail(to: email_address.email, subject: "GPO reminder") + end + end + private def email_should_receive_nonessential_notifications?(email) diff --git a/app/views/user_mailer/gpo_reminder.html.erb b/app/views/user_mailer/gpo_reminder.html.erb new file mode 100644 index 00000000000..d4d37955902 --- /dev/null +++ b/app/views/user_mailer/gpo_reminder.html.erb @@ -0,0 +1,25 @@ +

+ This is actually the shiny new GPO reminder email. +

+ +

+ <%= t('user_mailer.account_rejected.intro', app_name: APP_NAME) %> +

+ + + + + + + +
+   +
+ + + + + +
+   +
diff --git a/spec/mailers/previews/user_mailer_preview.rb b/spec/mailers/previews/user_mailer_preview.rb index 72006555ea6..c7ef6e15ce4 100644 --- a/spec/mailers/previews/user_mailer_preview.rb +++ b/spec/mailers/previews/user_mailer_preview.rb @@ -179,6 +179,13 @@ def suspended_reset_password ).suspended_reset_password end + def gpo_reminder + UserMailer.with( + user: user, + email_address: email_address_record, + ).gpo_reminder + end + private def user From 615abf4b4e9f07f8756d6a0f4e3a454f25375660 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Thu, 3 Aug 2023 10:43:28 -0400 Subject: [PATCH 02/36] Email formatting --- app/mailers/user_mailer.rb | 6 +++- app/views/user_mailer/gpo_reminder.html.erb | 39 +++++++++++++-------- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 6e4e2aa3eb1..7778619226b 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -393,7 +393,11 @@ def suspended_reset_password def gpo_reminder with_user_locale(user) do - mail(to: email_address.email, subject: "GPO reminder") + # raw_date_letter_was_sent = user&.pending_profile&.gpo_verification_pending_at + raw_date_letter_was_sent = Time.zone.now - 2.weeks + @date_letter_was_sent = raw_date_letter_was_sent.strftime(I18n.t('time.formats.event_date')) + + mail(to: email_address.email, subject: "Finish verifying your identity") end end diff --git a/app/views/user_mailer/gpo_reminder.html.erb b/app/views/user_mailer/gpo_reminder.html.erb index d4d37955902..e331c4ddb90 100644 --- a/app/views/user_mailer/gpo_reminder.html.erb +++ b/app/views/user_mailer/gpo_reminder.html.erb @@ -1,25 +1,34 @@

- This is actually the shiny new GPO reminder email. + You requested a letter to verify your identity + on <%= @date_letter_was_sent %>. You'll need to + enter the code from the letter to finish verifying your identity. + Learn more about verifying your address by mail.

-

- <%= t('user_mailer.account_rejected.intro', app_name: APP_NAME) %> -

- - +
- +
-   + + + + + + + +
+ <%= link_to "Finish verifying your identity", + new_user_password_url, + target: '_blank', + class: 'float-center', + align: 'center', + rel: 'noopener' %> +
- - - - -
-   -
+

+ If you didn't get this letter, sign in to request another letter. +

From 7784bf7bb8cb4edec914778f3c5130b18f8babc4 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Fri, 4 Aug 2023 11:31:50 -0400 Subject: [PATCH 03/36] Moved strings out to i18m Yaml --- app/mailers/user_mailer.rb | 2 +- app/views/user_mailer/gpo_reminder.html.erb | 28 ++++++++++++++++----- config/locales/idv/en.yml | 10 ++++++++ 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 7778619226b..6f532283488 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -397,7 +397,7 @@ def gpo_reminder raw_date_letter_was_sent = Time.zone.now - 2.weeks @date_letter_was_sent = raw_date_letter_was_sent.strftime(I18n.t('time.formats.event_date')) - mail(to: email_address.email, subject: "Finish verifying your identity") + mail(to: email_address.email, subject: t('idv.messages.gpo_reminder.subject')) end end diff --git a/app/views/user_mailer/gpo_reminder.html.erb b/app/views/user_mailer/gpo_reminder.html.erb index e331c4ddb90..ab0fcbc674d 100644 --- a/app/views/user_mailer/gpo_reminder.html.erb +++ b/app/views/user_mailer/gpo_reminder.html.erb @@ -1,8 +1,17 @@

- You requested a letter to verify your identity - on <%= @date_letter_was_sent %>. You'll need to - enter the code from the letter to finish verifying your identity. - Learn more about verifying your address by mail. + <%= + t( + 'idv.messages.gpo_reminder.body_html', + date_letter_was_sent: @date_letter_was_sent, + ) + %> + <%= + link_to( + t('idv.troubleshooting.options.learn_more_verify_by_mail'), + "https://www.google.com", + {style: "text-decoration: 'underline'"}, + ) + %>

@@ -13,7 +22,7 @@
- <%= link_to "Finish verifying your identity", + <%= link_to t('idv.messages.gpo_reminder.finish'), new_user_password_url, target: '_blank', class: 'float-center', @@ -30,5 +39,12 @@

- If you didn't get this letter, sign in to request another letter. + <%= t('idv.messages.gpo_reminder.did_not_get_a_letter') %> + <%= + link_to( + t('idv.messages.gpo_reminder.request_another_letter'), + 'https://www.google.com', + {style: "text-decoration: 'underline'"} + ) + %>

diff --git a/config/locales/idv/en.yml b/config/locales/idv/en.yml index 53349bd78e9..fb68b980a1e 100644 --- a/config/locales/idv/en.yml +++ b/config/locales/idv/en.yml @@ -233,6 +233,16 @@ en: resend_timeframe: Letters typically take 3 to 7 business days to arrive. timeframe_html: Letters are sent the next business day via USPS First Class Mail and typically take 3 to 7 business days to arrive. + gpo_reminder: + body_html: You requested a letter to verify your identity on + %{date_letter_was_sent}. You'll need to + enter the code from the letter to finish verifying your + identity. Sign back in to Login.gov to finish verifying your + identity. + did_not_get_a_letter: "If you didn't get this letter, " + finish: Finish verifying your identity + request_another_letter: sign in to request another letter + subject: Finish verifying your identity otp_delivery_method_description: If you entered a landline above, please select “Phone call” below. personal_key: This is your new personal key. Write it down and keep it in a safe place. You will need it if you ever lose your password. From 1f86b769b1571b7eace1c11873f972b4e7810db5 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Mon, 7 Aug 2023 11:53:22 -0400 Subject: [PATCH 04/36] Lint nits --- app/views/user_mailer/gpo_reminder.html.erb | 48 ++++++++++----------- 1 file changed, 22 insertions(+), 26 deletions(-) diff --git a/app/views/user_mailer/gpo_reminder.html.erb b/app/views/user_mailer/gpo_reminder.html.erb index ab0fcbc674d..48649c3e680 100644 --- a/app/views/user_mailer/gpo_reminder.html.erb +++ b/app/views/user_mailer/gpo_reminder.html.erb @@ -1,17 +1,13 @@

- <%= - t( - 'idv.messages.gpo_reminder.body_html', - date_letter_was_sent: @date_letter_was_sent, - ) - %> - <%= - link_to( - t('idv.troubleshooting.options.learn_more_verify_by_mail'), - "https://www.google.com", - {style: "text-decoration: 'underline'"}, - ) - %> + <%= t( + 'idv.messages.gpo_reminder.body_html', + date_letter_was_sent: @date_letter_was_sent, + ) %> + <%= link_to( + t('idv.troubleshooting.options.learn_more_verify_by_mail'), + 'https://www.google.com', + { style: "text-decoration: 'underline'" }, + ) %>

@@ -23,11 +19,11 @@ @@ -39,12 +35,12 @@
<%= link_to t('idv.messages.gpo_reminder.finish'), - new_user_password_url, - target: '_blank', - class: 'float-center', - align: 'center', - rel: 'noopener' %> + new_user_password_url, + target: '_blank', + class: 'float-center', + align: 'center', + rel: 'noopener' %>

- <%= t('idv.messages.gpo_reminder.did_not_get_a_letter') %> - <%= - link_to( - t('idv.messages.gpo_reminder.request_another_letter'), - 'https://www.google.com', - {style: "text-decoration: 'underline'"} - ) - %> + <%= t( + 'idv.messages.gpo_reminder.did_not_get_a_letter_html', + another_letter_link: link_to( + t('idv.messages.gpo_reminder.sign_in_and_request_another_letter'), + 'https://www.google.com', + { style: "text-decoration: 'underline'" }, + ), + ) %>

From 0dc14bcde968482a811ce83c63e6f0ef752231a5 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Mon, 7 Aug 2023 11:53:30 -0400 Subject: [PATCH 05/36] French and Spanish strings --- config/locales/idv/en.yml | 11 +++++------ config/locales/idv/es.yml | 9 +++++++++ config/locales/idv/fr.yml | 10 ++++++++++ 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/config/locales/idv/en.yml b/config/locales/idv/en.yml index fb68b980a1e..8788c32ddf6 100644 --- a/config/locales/idv/en.yml +++ b/config/locales/idv/en.yml @@ -235,13 +235,12 @@ en: and typically take 3 to 7 business days to arrive. gpo_reminder: body_html: You requested a letter to verify your identity on - %{date_letter_was_sent}. You'll need to - enter the code from the letter to finish verifying your - identity. Sign back in to Login.gov to finish verifying your - identity. - did_not_get_a_letter: "If you didn't get this letter, " + %{date_letter_was_sent}. You’ll need to enter the + code from the letter to finish verifying your identity. Sign back in + to Login.gov to finish verifying your identity. + did_not_get_a_letter_html: If you didn’t get this letter, %{another_letter_link} finish: Finish verifying your identity - request_another_letter: sign in to request another letter + sign_in_and_request_another_letter: sign in to request another letter subject: Finish verifying your identity otp_delivery_method_description: If you entered a landline above, please select “Phone call” below. personal_key: This is your new personal key. Write it down and keep it in a safe diff --git a/config/locales/idv/es.yml b/config/locales/idv/es.yml index 2f6cdba337f..fd765012cb8 100644 --- a/config/locales/idv/es.yml +++ b/config/locales/idv/es.yml @@ -242,6 +242,15 @@ es: timeframe_html: Las cartas se envían al día siguiente por First Class Mail de USPS y suelen tardar entre 3 y 7 días hábiles en llegar. + gpo_reminder: + body_html: El %{date_letter_was_sent} solicitaste una carta + para verificar tu identidad. Deberás ingresar el código que contiene + esa carta para completar la verificación por correo. Vuelve a iniciar + sesión en Login.gov para terminar de verificar tu identidad. + did_not_get_a_letter_html: Si no recibiste dicha carta, %{another_letter_link} + finish: Termina de verificar tu identidad + sign_in_and_request_another_letter: inicia sesión para solicitar otra + subject: Termina de verificar tu identidad otp_delivery_method_description: Si ha introducido un teléfono fijo más arriba, seleccione “Llamada telefónica” más abajo. personal_key: Esta es su nueva clave personal. Escríbala y guárdela en un lugar diff --git a/config/locales/idv/fr.yml b/config/locales/idv/fr.yml index 47316f25155..f60671baba7 100644 --- a/config/locales/idv/fr.yml +++ b/config/locales/idv/fr.yml @@ -258,6 +258,16 @@ fr: timeframe_html: Les lettres sont envoyées les jours ouvrables par courriel de première classe de USPS et prennent généralement entre trois à sept jours ouvrables pour être reçues. + gpo_reminder: + body_html: Vous avez demandé une lettre pour vérifier votre identité le + %{date_letter_was_sent}. Vous devrez saisir le code + figurant dans la lettre pour terminer la vérification par courrier. + Reconnectez-vous à Login.gov pour terminer la vérification de votre + identité. + did_not_get_a_letter_html: Si vous n’avez pas reçu cette lettre, %{another_letter_link} + finish: Terminer la vérification de votre identité + sign_in_and_request_another_letter: connectez-vous pour en demander une autre. + subject: Terminer la vérification de votre identité otp_delivery_method_description: Si vous avez saisi une ligne fixe ci-dessus, veuillez sélectionner « Appel téléphonique » ci-dessous. personal_key: Il s’agit de votre nouvelle clé personnelle. Notez-la et From 72caa22db2c24325837b20c90fa0f652a41fcb56 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Mon, 7 Aug 2023 12:13:30 -0400 Subject: [PATCH 06/36] Lint and i18n nits --- app/views/user_mailer/gpo_reminder.html.erb | 1 + config/locales/idv/en.yml | 4 ++-- config/locales/idv/es.yml | 4 ++-- config/locales/idv/fr.yml | 4 ++-- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/app/views/user_mailer/gpo_reminder.html.erb b/app/views/user_mailer/gpo_reminder.html.erb index 48649c3e680..0cdad9c4b0d 100644 --- a/app/views/user_mailer/gpo_reminder.html.erb +++ b/app/views/user_mailer/gpo_reminder.html.erb @@ -2,6 +2,7 @@ <%= t( 'idv.messages.gpo_reminder.body_html', date_letter_was_sent: @date_letter_was_sent, + app_name: APP_NAME, ) %> <%= link_to( t('idv.troubleshooting.options.learn_more_verify_by_mail'), diff --git a/config/locales/idv/en.yml b/config/locales/idv/en.yml index 8788c32ddf6..c8a2982f9e1 100644 --- a/config/locales/idv/en.yml +++ b/config/locales/idv/en.yml @@ -237,8 +237,8 @@ en: body_html: You requested a letter to verify your identity on %{date_letter_was_sent}. You’ll need to enter the code from the letter to finish verifying your identity. Sign back in - to Login.gov to finish verifying your identity. - did_not_get_a_letter_html: If you didn’t get this letter, %{another_letter_link} + to %{app_name} to finish verifying your identity. + did_not_get_a_letter_html: If you didn’t get this letter, %{another_letter_link} finish: Finish verifying your identity sign_in_and_request_another_letter: sign in to request another letter subject: Finish verifying your identity diff --git a/config/locales/idv/es.yml b/config/locales/idv/es.yml index fd765012cb8..c32241524ca 100644 --- a/config/locales/idv/es.yml +++ b/config/locales/idv/es.yml @@ -246,8 +246,8 @@ es: body_html: El %{date_letter_was_sent} solicitaste una carta para verificar tu identidad. Deberás ingresar el código que contiene esa carta para completar la verificación por correo. Vuelve a iniciar - sesión en Login.gov para terminar de verificar tu identidad. - did_not_get_a_letter_html: Si no recibiste dicha carta, %{another_letter_link} + sesión en %{app_name} para terminar de verificar tu identidad. + did_not_get_a_letter_html: Si no recibiste dicha carta, %{another_letter_link} finish: Termina de verificar tu identidad sign_in_and_request_another_letter: inicia sesión para solicitar otra subject: Termina de verificar tu identidad diff --git a/config/locales/idv/fr.yml b/config/locales/idv/fr.yml index f60671baba7..923304577fe 100644 --- a/config/locales/idv/fr.yml +++ b/config/locales/idv/fr.yml @@ -262,9 +262,9 @@ fr: body_html: Vous avez demandé une lettre pour vérifier votre identité le %{date_letter_was_sent}. Vous devrez saisir le code figurant dans la lettre pour terminer la vérification par courrier. - Reconnectez-vous à Login.gov pour terminer la vérification de votre + Reconnectez-vous à %{app_name} pour terminer la vérification de votre identité. - did_not_get_a_letter_html: Si vous n’avez pas reçu cette lettre, %{another_letter_link} + did_not_get_a_letter_html: Si vous n’avez pas reçu cette lettre, %{another_letter_link} finish: Terminer la vérification de votre identité sign_in_and_request_another_letter: connectez-vous pour en demander une autre. subject: Terminer la vérification de votre identité From ab2a75aba447dd681392ad8ddc2dcb7f681ecada Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Tue, 8 Aug 2023 11:33:30 -0400 Subject: [PATCH 07/36] Point email links at correct targets --- app/views/user_mailer/gpo_reminder.html.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/user_mailer/gpo_reminder.html.erb b/app/views/user_mailer/gpo_reminder.html.erb index 0cdad9c4b0d..ab3c3d7b50f 100644 --- a/app/views/user_mailer/gpo_reminder.html.erb +++ b/app/views/user_mailer/gpo_reminder.html.erb @@ -20,7 +20,7 @@ <%= link_to t('idv.messages.gpo_reminder.finish'), - new_user_password_url, + idv_gpo_verify_url, target: '_blank', class: 'float-center', align: 'center', @@ -40,7 +40,7 @@ 'idv.messages.gpo_reminder.did_not_get_a_letter_html', another_letter_link: link_to( t('idv.messages.gpo_reminder.sign_in_and_request_another_letter'), - 'https://www.google.com', + idv_gpo_url, { style: "text-decoration: 'underline'" }, ), ) %> From 41c1f65ab7a489bf6d69ef1f1ad358336e2b4585 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Wed, 9 Aug 2023 16:00:32 -0400 Subject: [PATCH 08/36] Added reminder_sent_at to usps_confirmations table --- app/services/gpo_reminder_sender.rb | 5 +++ ...nder_sent_at_to_usps_confirmation_codes.rb | 8 ++++ db/schema.rb | 2 + spec/services/gpo_reminder_sender_spec.rb | 39 +++++++++++++++++++ 4 files changed, 54 insertions(+) create mode 100644 app/services/gpo_reminder_sender.rb create mode 100644 db/primary_migrate/20230809194211_add_reminder_sent_at_to_usps_confirmation_codes.rb create mode 100644 spec/services/gpo_reminder_sender_spec.rb diff --git a/app/services/gpo_reminder_sender.rb b/app/services/gpo_reminder_sender.rb new file mode 100644 index 00000000000..2aa09e72b7a --- /dev/null +++ b/app/services/gpo_reminder_sender.rb @@ -0,0 +1,5 @@ +class GpoReminderSender + def send_emails + + end +end diff --git a/db/primary_migrate/20230809194211_add_reminder_sent_at_to_usps_confirmation_codes.rb b/db/primary_migrate/20230809194211_add_reminder_sent_at_to_usps_confirmation_codes.rb new file mode 100644 index 00000000000..79030dc87b5 --- /dev/null +++ b/db/primary_migrate/20230809194211_add_reminder_sent_at_to_usps_confirmation_codes.rb @@ -0,0 +1,8 @@ +class AddReminderSentAtToUspsConfirmationCodes < ActiveRecord::Migration[7.0] + disable_ddl_transaction! + + def change + add_column :usps_confirmation_codes, :reminder_sent_at, :datetime, precision: nil + add_index :usps_confirmation_codes, :reminder_sent_at, algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index 7dd15040377..7df4d6dbe7f 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -619,8 +619,10 @@ t.datetime "created_at", precision: nil, null: false t.datetime "updated_at", precision: nil, null: false t.datetime "bounced_at", precision: nil + t.datetime "reminder_sent_at", precision: nil t.index ["otp_fingerprint"], name: "index_usps_confirmation_codes_on_otp_fingerprint" t.index ["profile_id"], name: "index_usps_confirmation_codes_on_profile_id" + t.index ["reminder_sent_at"], name: "index_usps_confirmation_codes_on_reminder_sent_at" end create_table "usps_confirmations", id: :serial, force: :cascade do |t| diff --git a/spec/services/gpo_reminder_sender_spec.rb b/spec/services/gpo_reminder_sender_spec.rb new file mode 100644 index 00000000000..af853ce5d06 --- /dev/null +++ b/spec/services/gpo_reminder_sender_spec.rb @@ -0,0 +1,39 @@ +require 'rails_helper' + +RSpec.describe GpoReminderSender do + describe '#send_emails' do + let(:user) { create(:user, :with_pending_gpo_profile) } + + context 'when no users need a reminder' do + before do + user.gpo_verification_pending_profile.gpo_verification_pending_at = Time.zone.now - 13.days + user.save + end + + it 'sends no emails' do + expect{subject.send_emails}.to change{ActionMailer::Base.deliveries.size}.by(0) + end + end + + context 'when a user is due for a reminder' do + before do + user.gpo_verification_pending_profile.gpo_verification_pending_at = Time.zone.now - 14.days + user.save + end + + it 'sends that user an email' do + expect{subject.send_emails}.to change{ActionMailer::Base.deliveries.size}.by(1) + end + + context 'but a reminder has already been sent' do + before do + user.gpo_verification_pending_profile.gpo_reminder_sent_at = Time.zone.now - 1.day + end + + it 'does not send that user an email' do + expect{subject.send_emails}.to change{ActionMailer::Base.deliveries.size}.by(0) + end + end + end + end +end From 4736e902837551205022e66b6c4f6a83c34b6284 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Thu, 10 Aug 2023 10:58:10 -0400 Subject: [PATCH 09/36] Point links at correct targets, take 2. --- app/views/user_mailer/gpo_reminder.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/user_mailer/gpo_reminder.html.erb b/app/views/user_mailer/gpo_reminder.html.erb index ab3c3d7b50f..d07ca172136 100644 --- a/app/views/user_mailer/gpo_reminder.html.erb +++ b/app/views/user_mailer/gpo_reminder.html.erb @@ -40,7 +40,7 @@ 'idv.messages.gpo_reminder.did_not_get_a_letter_html', another_letter_link: link_to( t('idv.messages.gpo_reminder.sign_in_and_request_another_letter'), - idv_gpo_url, + idv_gpo_verify_url, { style: "text-decoration: 'underline'" }, ), ) %> From b72eada85a80983d9ed2ee9d976ce119333287d6 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Thu, 10 Aug 2023 16:40:47 -0400 Subject: [PATCH 10/36] Specs for basic cases. --- app/services/gpo_reminder_sender.rb | 4 +++- spec/services/gpo_reminder_sender_spec.rb | 24 ++++++++++++----------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/app/services/gpo_reminder_sender.rb b/app/services/gpo_reminder_sender.rb index 2aa09e72b7a..c5a8da471a1 100644 --- a/app/services/gpo_reminder_sender.rb +++ b/app/services/gpo_reminder_sender.rb @@ -1,5 +1,7 @@ class GpoReminderSender def send_emails - + Profile.where.not(gpo_verification_pending_at: nil).each do |profile| + profile.user.send_email_to_all_addresses(:gpo_reminder) + end end end diff --git a/spec/services/gpo_reminder_sender_spec.rb b/spec/services/gpo_reminder_sender_spec.rb index af853ce5d06..6229fd9253b 100644 --- a/spec/services/gpo_reminder_sender_spec.rb +++ b/spec/services/gpo_reminder_sender_spec.rb @@ -4,11 +4,18 @@ describe '#send_emails' do let(:user) { create(:user, :with_pending_gpo_profile) } + def set_gpo_verification_pending_at(to_time) + user.gpo_verification_pending_profile.update(gpo_verification_pending_at: to_time) + end + + def set_reminder_sent_at(to_time) + gpo_confirmation_code = user.gpo_verification_pending_profile.gpo_confirmation_codes.first + gpo_confirmation_code.reminder_sent_at = Time.zone.now - 1.day + gpo_confirmation_code.save + end + context 'when no users need a reminder' do - before do - user.gpo_verification_pending_profile.gpo_verification_pending_at = Time.zone.now - 13.days - user.save - end + before { set_gpo_verification_pending_at(Time.zone.now - 13.days) } it 'sends no emails' do expect{subject.send_emails}.to change{ActionMailer::Base.deliveries.size}.by(0) @@ -16,19 +23,14 @@ end context 'when a user is due for a reminder' do - before do - user.gpo_verification_pending_profile.gpo_verification_pending_at = Time.zone.now - 14.days - user.save - end + before { set_gpo_verification_pending_at(Time.zone.now - 14.days) } it 'sends that user an email' do expect{subject.send_emails}.to change{ActionMailer::Base.deliveries.size}.by(1) end context 'but a reminder has already been sent' do - before do - user.gpo_verification_pending_profile.gpo_reminder_sent_at = Time.zone.now - 1.day - end + before { set_reminder_sent_at(Time.zone.now - 1.day) } it 'does not send that user an email' do expect{subject.send_emails}.to change{ActionMailer::Base.deliveries.size}.by(0) From db0d9938b15455f99ef90453227973bb85d1dba0 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Thu, 10 Aug 2023 17:21:20 -0400 Subject: [PATCH 11/36] Add time range to profile select. --- app/services/gpo_reminder_sender.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/services/gpo_reminder_sender.rb b/app/services/gpo_reminder_sender.rb index c5a8da471a1..9a74ec3aff0 100644 --- a/app/services/gpo_reminder_sender.rb +++ b/app/services/gpo_reminder_sender.rb @@ -1,6 +1,10 @@ class GpoReminderSender def send_emails - Profile.where.not(gpo_verification_pending_at: nil).each do |profile| + profiles = Profile# joins(:usps_confirmation_codes) + .where(gpo_verification_pending_at: ..(Time.zone.now - 14.days)) + # .where(reminder_sent_at: nil) + + profiles.each do |profile| profile.user.send_email_to_all_addresses(:gpo_reminder) end end From 6878977f7e2defc2797ff9d34464fca01630abc6 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Fri, 11 Aug 2023 11:15:32 -0400 Subject: [PATCH 12/36] Add checks for time and reminder already sent --- app/services/gpo_reminder_sender.rb | 11 ++++++----- spec/services/gpo_reminder_sender_spec.rb | 8 ++++---- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/app/services/gpo_reminder_sender.rb b/app/services/gpo_reminder_sender.rb index 9a74ec3aff0..d7f48f93561 100644 --- a/app/services/gpo_reminder_sender.rb +++ b/app/services/gpo_reminder_sender.rb @@ -1,10 +1,11 @@ class GpoReminderSender def send_emails - profiles = Profile# joins(:usps_confirmation_codes) - .where(gpo_verification_pending_at: ..(Time.zone.now - 14.days)) - # .where(reminder_sent_at: nil) - - profiles.each do |profile| + profiles_due_for_reminder = Profile.joins(:gpo_confirmation_codes). + where( + gpo_verification_pending_at: ..(Time.zone.now - 14.days), + gpo_confirmation_codes: { reminder_sent_at: nil }, + ) + profiles_due_for_reminder.each do |profile| profile.user.send_email_to_all_addresses(:gpo_reminder) end end diff --git a/spec/services/gpo_reminder_sender_spec.rb b/spec/services/gpo_reminder_sender_spec.rb index 6229fd9253b..e62936cc0b1 100644 --- a/spec/services/gpo_reminder_sender_spec.rb +++ b/spec/services/gpo_reminder_sender_spec.rb @@ -10,7 +10,7 @@ def set_gpo_verification_pending_at(to_time) def set_reminder_sent_at(to_time) gpo_confirmation_code = user.gpo_verification_pending_profile.gpo_confirmation_codes.first - gpo_confirmation_code.reminder_sent_at = Time.zone.now - 1.day + gpo_confirmation_code.reminder_sent_at = to_time gpo_confirmation_code.save end @@ -18,7 +18,7 @@ def set_reminder_sent_at(to_time) before { set_gpo_verification_pending_at(Time.zone.now - 13.days) } it 'sends no emails' do - expect{subject.send_emails}.to change{ActionMailer::Base.deliveries.size}.by(0) + expect { subject.send_emails }.to change { ActionMailer::Base.deliveries.size }.by(0) end end @@ -26,14 +26,14 @@ def set_reminder_sent_at(to_time) before { set_gpo_verification_pending_at(Time.zone.now - 14.days) } it 'sends that user an email' do - expect{subject.send_emails}.to change{ActionMailer::Base.deliveries.size}.by(1) + expect { subject.send_emails }.to change { ActionMailer::Base.deliveries.size }.by(1) end context 'but a reminder has already been sent' do before { set_reminder_sent_at(Time.zone.now - 1.day) } it 'does not send that user an email' do - expect{subject.send_emails}.to change{ActionMailer::Base.deliveries.size}.by(0) + expect { subject.send_emails }.to change { ActionMailer::Base.deliveries.size }.by(0) end end end From 5a52cceda3748fa72513476938b8c65f3610a928 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Fri, 11 Aug 2023 13:01:38 -0400 Subject: [PATCH 13/36] Hande users with multiple emails --- spec/services/gpo_reminder_sender_spec.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spec/services/gpo_reminder_sender_spec.rb b/spec/services/gpo_reminder_sender_spec.rb index e62936cc0b1..87f46775d44 100644 --- a/spec/services/gpo_reminder_sender_spec.rb +++ b/spec/services/gpo_reminder_sender_spec.rb @@ -29,6 +29,14 @@ def set_reminder_sent_at(to_time) expect { subject.send_emails }.to change { ActionMailer::Base.deliveries.size }.by(1) 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 }.to change { ActionMailer::Base.deliveries.size }.by(2) + end + end + context 'but a reminder has already been sent' do before { set_reminder_sent_at(Time.zone.now - 1.day) } From b403690bb86f622919d20ca97531163394a965a1 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Fri, 11 Aug 2023 14:05:53 -0400 Subject: [PATCH 14/36] Added mailer spec for gpo_reminder --- app/mailers/user_mailer.rb | 7 +++--- spec/mailers/user_mailer_spec.rb | 37 ++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 6f532283488..4995b14278b 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -393,9 +393,10 @@ def suspended_reset_password def gpo_reminder with_user_locale(user) do - # raw_date_letter_was_sent = user&.pending_profile&.gpo_verification_pending_at - raw_date_letter_was_sent = Time.zone.now - 2.weeks - @date_letter_was_sent = raw_date_letter_was_sent.strftime(I18n.t('time.formats.event_date')) + @date_letter_was_sent = user. + pending_profile. + gpo_verification_pending_at. + strftime(I18n.t('time.formats.event_date')) mail(to: email_address.email, subject: t('idv.messages.gpo_reminder.subject')) end diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index 54cdee31305..d160592a83b 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -872,4 +872,41 @@ def expect_email_body_to_have_help_and_contact_links ) end end + + describe '#gpo_reminder' do + let(:date_letter_was_sent) { Date.new(1969, 7, 20) } + + let(:user) do + user = create(:user, :with_pending_gpo_profile) + user.pending_profile.update(gpo_verification_pending_at: date_letter_was_sent) + user + end + + let(:mail) do + UserMailer.with(user: user, email_address: email_address).gpo_reminder + end + + it_behaves_like 'a system email' + it_behaves_like 'an email that respects user email locale preference' + + it 'sends to the specified email' do + expect(mail.to).to eq [email_address.email] + end + + it 'renders the subject' do + expect(mail.subject).to eq t('idv.messages.gpo_reminder.subject') + end + + it 'renders the body' do + expected_body = strip_tags( + t( + 'idv.messages.gpo_reminder.body_html', + date_letter_was_sent: date_letter_was_sent.strftime(t('time.formats.event_date')), + app_name: APP_NAME, + ), + ) + + expect(mail.html_part.body).to have_content(expected_body) + end + end end From 922be1ea2096e5d7001cddc5ceb282039eca3435 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Fri, 11 Aug 2023 16:32:46 -0400 Subject: [PATCH 15/36] Actually set up the job. --- app/jobs/gpo_reminder_job.rb | 9 +++++++++ app/services/gpo_reminder_sender.rb | 5 +++-- config/initializers/job_configurations.rb | 6 ++++++ spec/jobs/gpo_reminder_job_spec.rb | 19 +++++++++++++++++++ spec/services/gpo_reminder_sender_spec.rb | 23 ++++++++++++++++------- 5 files changed, 53 insertions(+), 9 deletions(-) create mode 100644 app/jobs/gpo_reminder_job.rb create mode 100644 spec/jobs/gpo_reminder_job_spec.rb diff --git a/app/jobs/gpo_reminder_job.rb b/app/jobs/gpo_reminder_job.rb new file mode 100644 index 00000000000..0d51c201251 --- /dev/null +++ b/app/jobs/gpo_reminder_job.rb @@ -0,0 +1,9 @@ +class GpoReminderJob < ApplicationJob + queue_as :low + + # Send email reminders to people with USPS proofing letters whose + # letters were sent a while ago, and haven't yet entered their code + def perform(cutoff_time_for_sending_reminders) + GpoReminderSender.new.send_emails(cutoff_time_for_sending_reminders) + end +end diff --git a/app/services/gpo_reminder_sender.rb b/app/services/gpo_reminder_sender.rb index d7f48f93561..c0f275d6c0c 100644 --- a/app/services/gpo_reminder_sender.rb +++ b/app/services/gpo_reminder_sender.rb @@ -1,10 +1,11 @@ class GpoReminderSender - def send_emails + def send_emails(for_letters_sent_before) profiles_due_for_reminder = Profile.joins(:gpo_confirmation_codes). where( - gpo_verification_pending_at: ..(Time.zone.now - 14.days), + gpo_verification_pending_at: ..for_letters_sent_before, gpo_confirmation_codes: { reminder_sent_at: nil }, ) + profiles_due_for_reminder.each do |profile| profile.user.send_email_to_all_addresses(:gpo_reminder) end diff --git a/config/initializers/job_configurations.rb b/config/initializers/job_configurations.rb index 8cf01a96956..aac6b35d4b9 100644 --- a/config/initializers/job_configurations.rb +++ b/config/initializers/job_configurations.rb @@ -182,6 +182,12 @@ cron: cron_24h, args: -> { [Time.zone.today] }, }, + # Send reminder letters for old, outstanding GPO verification codes + send_gpo_code_reminders: { + class: 'GpoReminderJob', + cron: cron_24h, + args: -> { [Time.zone.now - 14.days] }, + }, }.compact end # rubocop:enable Metrics/BlockLength diff --git a/spec/jobs/gpo_reminder_job_spec.rb b/spec/jobs/gpo_reminder_job_spec.rb new file mode 100644 index 00000000000..7998d47663d --- /dev/null +++ b/spec/jobs/gpo_reminder_job_spec.rb @@ -0,0 +1,19 @@ +require 'rails_helper' + +RSpec.describe GpoReminderJob do + describe '#perform' do + subject(:perform) { GpoReminderJob.new.perform(Time.zone.now - 14.days) } + + before do + create(:user, :with_pending_gpo_profile). + pending_profile. + update( + gpo_verification_pending_at: Time.zone.now - 14.days, + ) + end + + it 'sends reminder emails' do + expect { perform }.to change { ActionMailer::Base.deliveries.count }.by(1) + end + end +end diff --git a/spec/services/gpo_reminder_sender_spec.rb b/spec/services/gpo_reminder_sender_spec.rb index 87f46775d44..a39052c89ce 100644 --- a/spec/services/gpo_reminder_sender_spec.rb +++ b/spec/services/gpo_reminder_sender_spec.rb @@ -2,6 +2,11 @@ RSpec.describe GpoReminderSender do describe '#send_emails' do + WAIT_BEFORE_SENDING_REMINDER = 14.days + TIME_DUE_FOR_REMINDER = Time.zone.now - WAIT_BEFORE_SENDING_REMINDER + TIME_NOT_YET_DUE = TIME_DUE_FOR_REMINDER + 1.day + TIME_YESTERDAY = Time.zone.now - 1.day + let(:user) { create(:user, :with_pending_gpo_profile) } def set_gpo_verification_pending_at(to_time) @@ -15,33 +20,37 @@ def set_reminder_sent_at(to_time) end context 'when no users need a reminder' do - before { set_gpo_verification_pending_at(Time.zone.now - 13.days) } + before { set_gpo_verification_pending_at(TIME_NOT_YET_DUE) } it 'sends no emails' do - expect { subject.send_emails }.to change { ActionMailer::Base.deliveries.size }.by(0) + expect { subject.send_emails(TIME_DUE_FOR_REMINDER) }. + to change { ActionMailer::Base.deliveries.size }.by(0) end end context 'when a user is due for a reminder' do - before { set_gpo_verification_pending_at(Time.zone.now - 14.days) } + before { set_gpo_verification_pending_at(TIME_DUE_FOR_REMINDER) } it 'sends that user an email' do - expect { subject.send_emails }.to change { ActionMailer::Base.deliveries.size }.by(1) + expect { subject.send_emails(TIME_DUE_FOR_REMINDER) }. + to change { ActionMailer::Base.deliveries.size }.by(1) 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 }.to change { ActionMailer::Base.deliveries.size }.by(2) + expect { subject.send_emails(TIME_DUE_FOR_REMINDER) }. + to change { ActionMailer::Base.deliveries.size }.by(2) end end context 'but a reminder has already been sent' do - before { set_reminder_sent_at(Time.zone.now - 1.day) } + before { set_reminder_sent_at(TIME_YESTERDAY) } it 'does not send that user an email' do - expect { subject.send_emails }.to change { ActionMailer::Base.deliveries.size }.by(0) + expect { subject.send_emails(TIME_DUE_FOR_REMINDER) }. + to change { ActionMailer::Base.deliveries.size }.by(0) end end end From 3450849431a178295f5342be8f0b085761f6d555 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Mon, 14 Aug 2023 12:38:11 -0400 Subject: [PATCH 16/36] Added analytics event for reminder email sent. --- app/jobs/gpo_reminder_job.rb | 9 +++++++- app/services/analytics_events.rb | 6 +++++ app/services/gpo_reminder_sender.rb | 5 +++++ spec/jobs/gpo_reminder_job_spec.rb | 21 +++++++++++++----- spec/services/gpo_reminder_sender_spec.rb | 27 ++++++++++++++++++++--- 5 files changed, 58 insertions(+), 10 deletions(-) diff --git a/app/jobs/gpo_reminder_job.rb b/app/jobs/gpo_reminder_job.rb index 0d51c201251..9985f980aa6 100644 --- a/app/jobs/gpo_reminder_job.rb +++ b/app/jobs/gpo_reminder_job.rb @@ -4,6 +4,13 @@ class GpoReminderJob < ApplicationJob # Send email reminders to people with USPS proofing letters whose # letters were sent a while ago, and haven't yet entered their code def perform(cutoff_time_for_sending_reminders) - GpoReminderSender.new.send_emails(cutoff_time_for_sending_reminders) + GpoReminderSender.new(analytics). + send_emails(cutoff_time_for_sending_reminders) + end + + private + + def analytics(user: AnonymousUser.new) + Analytics.new(user: user, request: nil, session: {}, sp: nil) end end diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 0f7f37c2a90..505910d60e7 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -1035,6 +1035,12 @@ def idv_gpo_confirm_start_over_visited track_event('IdV: gpo confirm start over visited') end + # A GPO reminder email was sent to the user + # @param [String] to_user UUID of user who we sent a reminder to + def idv_gpo_reminder_email_sent(to_user: ) + track_event('IdV: gpo reminder email sent', to_user: to_user) + end + # @identity.idp.previous_event_name Account verification submitted # @param [Boolean] success # @param [Hash] errors diff --git a/app/services/gpo_reminder_sender.rb b/app/services/gpo_reminder_sender.rb index c0f275d6c0c..eccf8fe8ab3 100644 --- a/app/services/gpo_reminder_sender.rb +++ b/app/services/gpo_reminder_sender.rb @@ -1,4 +1,8 @@ class GpoReminderSender + def initialize(analytics) + @analytics = analytics + end + def send_emails(for_letters_sent_before) profiles_due_for_reminder = Profile.joins(:gpo_confirmation_codes). where( @@ -8,6 +12,7 @@ def send_emails(for_letters_sent_before) profiles_due_for_reminder.each do |profile| profile.user.send_email_to_all_addresses(:gpo_reminder) + @analytics.idv_gpo_reminder_email_sent(to_user: profile.user.uuid) end end end diff --git a/spec/jobs/gpo_reminder_job_spec.rb b/spec/jobs/gpo_reminder_job_spec.rb index 7998d47663d..fb5bc33c4e9 100644 --- a/spec/jobs/gpo_reminder_job_spec.rb +++ b/spec/jobs/gpo_reminder_job_spec.rb @@ -1,19 +1,28 @@ require 'rails_helper' RSpec.describe GpoReminderJob do + WAIT_BEFORE_SENDING_REMINDER = 14.days + describe '#perform' do - subject(:perform) { GpoReminderJob.new.perform(Time.zone.now - 14.days) } + subject(:perform) { job.perform(Time.zone.now - WAIT_BEFORE_SENDING_REMINDER) } + + let(:job) { GpoReminderJob.new } + let(:user) { create(:user, :with_pending_gpo_profile) } + let(:pending_profile) { user.pending_profile } + let(:job_analytics) { FakeAnalytics.new } before do - create(:user, :with_pending_gpo_profile). - pending_profile. - update( - gpo_verification_pending_at: Time.zone.now - 14.days, - ) + pending_profile.update( + gpo_verification_pending_at: Time.zone.now - WAIT_BEFORE_SENDING_REMINDER, + ) + allow(job).to receive(:analytics).and_return(job_analytics) end it 'sends reminder emails' do expect { perform }.to change { ActionMailer::Base.deliveries.count }.by(1) + expect(job_analytics).to have_logged_event( + 'IdV: gpo reminder email sent', + ) end end end diff --git a/spec/services/gpo_reminder_sender_spec.rb b/spec/services/gpo_reminder_sender_spec.rb index a39052c89ce..a7f9fb32b47 100644 --- a/spec/services/gpo_reminder_sender_spec.rb +++ b/spec/services/gpo_reminder_sender_spec.rb @@ -7,16 +7,21 @@ TIME_NOT_YET_DUE = TIME_DUE_FOR_REMINDER + 1.day TIME_YESTERDAY = Time.zone.now - 1.day + subject(:sender) { GpoReminderSender.new(fake_analytics) } + let(:user) { create(:user, :with_pending_gpo_profile) } + let(:fake_analytics) { FakeAnalytics.new } def set_gpo_verification_pending_at(to_time) user.gpo_verification_pending_profile.update(gpo_verification_pending_at: to_time) end def set_reminder_sent_at(to_time) - gpo_confirmation_code = user.gpo_verification_pending_profile.gpo_confirmation_codes.first - gpo_confirmation_code.reminder_sent_at = to_time - gpo_confirmation_code.save + user. + gpo_verification_pending_profile. + gpo_confirmation_codes. + first. + update(reminder_sent_at: to_time) end context 'when no users need a reminder' do @@ -26,6 +31,11 @@ def set_reminder_sent_at(to_time) 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 end context 'when a user is due for a reminder' do @@ -36,6 +46,12 @@ def set_reminder_sent_at(to_time) to change { ActionMailer::Base.deliveries.size }.by(1) end + it 'logs an event' do + subject.send_emails(TIME_DUE_FOR_REMINDER) + + expect(fake_analytics).to have_logged_event('IdV: gpo reminder email sent') + end + context 'and the user has multiple emails' do let(:user) { create(:user, :with_pending_gpo_profile, :with_multiple_emails) } @@ -52,6 +68,11 @@ def set_reminder_sent_at(to_time) 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 end end end From ef01c755d2b214bcc0d9c416f6ed45bfdc6bb1f7 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Mon, 14 Aug 2023 13:39:52 -0400 Subject: [PATCH 17/36] Lint nit and changelog changelog: User-Facing Improvement, Identity Verification, Added GPO reminder email --- app/services/analytics_events.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 505910d60e7..d23a0ff2a90 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -1037,7 +1037,7 @@ def idv_gpo_confirm_start_over_visited # A GPO reminder email was sent to the user # @param [String] to_user UUID of user who we sent a reminder to - def idv_gpo_reminder_email_sent(to_user: ) + def idv_gpo_reminder_email_sent(to_user:) track_event('IdV: gpo reminder email sent', to_user: to_user) end From 44766b42963709cccbee40ceca5647f41ccee9e4 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Tue, 15 Aug 2023 11:43:05 -0400 Subject: [PATCH 18/36] Added spec for user canceling GPO verification --- spec/services/gpo_reminder_sender_spec.rb | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/spec/services/gpo_reminder_sender_spec.rb b/spec/services/gpo_reminder_sender_spec.rb index a7f9fb32b47..9c535fd666c 100644 --- a/spec/services/gpo_reminder_sender_spec.rb +++ b/spec/services/gpo_reminder_sender_spec.rb @@ -2,8 +2,8 @@ RSpec.describe GpoReminderSender do describe '#send_emails' do - WAIT_BEFORE_SENDING_REMINDER = 14.days - TIME_DUE_FOR_REMINDER = Time.zone.now - WAIT_BEFORE_SENDING_REMINDER + WAIT = 14.days + TIME_DUE_FOR_REMINDER = Time.zone.now - WAIT TIME_NOT_YET_DUE = TIME_DUE_FOR_REMINDER + 1.day TIME_YESTERDAY = Time.zone.now - 1.day @@ -61,6 +61,22 @@ def set_reminder_sent_at(to_time) end end + context 'but the user has cancelled gpo verification' do + before do + Idv::CancelVerificationAttempt.new(user: user).call + 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 + + it 'logs no events' do + expect { subject.send_emails(TIME_DUE_FOR_REMINDER) }. + not_to change { fake_analytics.events.count } + end + end + context 'but a reminder has already been sent' do before { set_reminder_sent_at(TIME_YESTERDAY) } From 0de4ca5409386efc02647cb7ff577f7add209205 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Tue, 15 Aug 2023 12:16:12 -0400 Subject: [PATCH 19/36] Pointed at new 'request another letter' page. --- app/views/user_mailer/gpo_reminder.html.erb | 2 +- spec/mailers/user_mailer_spec.rb | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/views/user_mailer/gpo_reminder.html.erb b/app/views/user_mailer/gpo_reminder.html.erb index d07ca172136..95482bd9afa 100644 --- a/app/views/user_mailer/gpo_reminder.html.erb +++ b/app/views/user_mailer/gpo_reminder.html.erb @@ -40,7 +40,7 @@ 'idv.messages.gpo_reminder.did_not_get_a_letter_html', another_letter_link: link_to( t('idv.messages.gpo_reminder.sign_in_and_request_another_letter'), - idv_gpo_verify_url, + idv_gpo_verify_url(did_not_receive_letter: 1), { style: "text-decoration: 'underline'" }, ), ) %> diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index d160592a83b..be10f01334b 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -908,5 +908,19 @@ def expect_email_body_to_have_help_and_contact_links expect(mail.html_part.body).to have_content(expected_body) end + + it 'renders the finish link' do + expect(mail.html_part.body).to have_link( + t('idv.messages.gpo_reminder.finish'), + href: idv_gpo_verify_url, + ) + end + + it 'renders the did not get it link' do + expect(mail.html_part.body).to have_link( + t('idv.messages.gpo_reminder.sign_in_and_request_another_letter'), + href: idv_gpo_verify_url(did_not_receive_letter: 1), + ) + end end end From 7fbdb71fdea5aec76f5a7118dc698ea3b82315de Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Tue, 15 Aug 2023 12:33:34 -0400 Subject: [PATCH 20/36] Review comments --- app/mailers/user_mailer.rb | 9 +++---- app/services/analytics_events.rb | 4 +-- app/services/gpo_reminder_sender.rb | 2 +- config/initializers/job_configurations.rb | 2 +- spec/services/gpo_reminder_sender_spec.rb | 33 +++++++++++------------ 5 files changed, 24 insertions(+), 26 deletions(-) diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 4995b14278b..70c0580713a 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -393,11 +393,10 @@ def suspended_reset_password def gpo_reminder with_user_locale(user) do - @date_letter_was_sent = user. - pending_profile. - gpo_verification_pending_at. - strftime(I18n.t('time.formats.event_date')) - + @date_letter_was_sent = I18n.l( + user.gpo_verification_pending_profile.gpo_verification_pending_at, + format: :event_date, + ) mail(to: email_address.email, subject: t('idv.messages.gpo_reminder.subject')) end end diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index d23a0ff2a90..48bfd16c4ea 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -1037,8 +1037,8 @@ def idv_gpo_confirm_start_over_visited # A GPO reminder email was sent to the user # @param [String] to_user UUID of user who we sent a reminder to - def idv_gpo_reminder_email_sent(to_user:) - track_event('IdV: gpo reminder email sent', to_user: to_user) + def idv_gpo_reminder_email_sent(user_id:) + track_event('IdV: gpo reminder email sent', user_id: user_id) end # @identity.idp.previous_event_name Account verification submitted diff --git a/app/services/gpo_reminder_sender.rb b/app/services/gpo_reminder_sender.rb index eccf8fe8ab3..905357d8845 100644 --- a/app/services/gpo_reminder_sender.rb +++ b/app/services/gpo_reminder_sender.rb @@ -12,7 +12,7 @@ def send_emails(for_letters_sent_before) profiles_due_for_reminder.each do |profile| profile.user.send_email_to_all_addresses(:gpo_reminder) - @analytics.idv_gpo_reminder_email_sent(to_user: profile.user.uuid) + @analytics.idv_gpo_reminder_email_sent(user_id: profile.user.uuid) end end end diff --git a/config/initializers/job_configurations.rb b/config/initializers/job_configurations.rb index aac6b35d4b9..1845e0dc250 100644 --- a/config/initializers/job_configurations.rb +++ b/config/initializers/job_configurations.rb @@ -186,7 +186,7 @@ send_gpo_code_reminders: { class: 'GpoReminderJob', cron: cron_24h, - args: -> { [Time.zone.now - 14.days] }, + args: -> { [14.days.ago] }, }, }.compact end diff --git a/spec/services/gpo_reminder_sender_spec.rb b/spec/services/gpo_reminder_sender_spec.rb index 9c535fd666c..0aca9cec48e 100644 --- a/spec/services/gpo_reminder_sender_spec.rb +++ b/spec/services/gpo_reminder_sender_spec.rb @@ -2,15 +2,14 @@ RSpec.describe GpoReminderSender do describe '#send_emails' do - WAIT = 14.days - TIME_DUE_FOR_REMINDER = Time.zone.now - WAIT - TIME_NOT_YET_DUE = TIME_DUE_FOR_REMINDER + 1.day - TIME_YESTERDAY = Time.zone.now - 1.day - subject(:sender) { GpoReminderSender.new(fake_analytics) } let(:user) { create(:user, :with_pending_gpo_profile) } let(:fake_analytics) { FakeAnalytics.new } + let(:wait_for_reminder) { 14.days } + let(:time_due_for_reminder) { Time.zone.now - wait_for_reminder } + 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) user.gpo_verification_pending_profile.update(gpo_verification_pending_at: to_time) @@ -25,29 +24,29 @@ def set_reminder_sent_at(to_time) end context 'when no users need a reminder' do - before { set_gpo_verification_pending_at(TIME_NOT_YET_DUE) } + before { set_gpo_verification_pending_at(time_not_yet_due) } it 'sends no emails' do - expect { subject.send_emails(TIME_DUE_FOR_REMINDER) }. + 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) }. + expect { subject.send_emails(time_due_for_reminder) }. not_to change { fake_analytics.events.count } end end context 'when a user is due for a reminder' do - before { set_gpo_verification_pending_at(TIME_DUE_FOR_REMINDER) } + 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) }. + expect { subject.send_emails(time_due_for_reminder) }. to change { ActionMailer::Base.deliveries.size }.by(1) end it 'logs an event' do - subject.send_emails(TIME_DUE_FOR_REMINDER) + subject.send_emails(time_due_for_reminder) expect(fake_analytics).to have_logged_event('IdV: gpo reminder email sent') end @@ -56,7 +55,7 @@ def set_reminder_sent_at(to_time) 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) }. + expect { subject.send_emails(time_due_for_reminder) }. to change { ActionMailer::Base.deliveries.size }.by(2) end end @@ -67,26 +66,26 @@ def set_reminder_sent_at(to_time) end it 'does not send that user an email' do - expect { subject.send_emails(TIME_DUE_FOR_REMINDER) }. + 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) }. + expect { subject.send_emails(time_due_for_reminder) }. not_to change { fake_analytics.events.count } end end context 'but a reminder has already been sent' do - before { set_reminder_sent_at(TIME_YESTERDAY) } + before { set_reminder_sent_at(time_yesterday) } it 'does not send that user an email' do - expect { subject.send_emails(TIME_DUE_FOR_REMINDER) }. + 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) }. + expect { subject.send_emails(time_due_for_reminder) }. not_to change { fake_analytics.events.count } end end From 13341b9bdf821e0c442f126cfbc1609f69e277d3 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Tue, 15 Aug 2023 13:14:57 -0400 Subject: [PATCH 21/36] Review comment --- app/services/analytics_events.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 48bfd16c4ea..85dba267330 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -1036,8 +1036,8 @@ def idv_gpo_confirm_start_over_visited end # A GPO reminder email was sent to the user - # @param [String] to_user UUID of user who we sent a reminder to - def idv_gpo_reminder_email_sent(user_id:) + # @param [String] user_id UUID of user who we sent a reminder to + def idv_gpo_reminder_email_sent(user_id:, **_extra) track_event('IdV: gpo reminder email sent', user_id: user_id) end From eb3f24d75a7c002a944d423910bd9191e377d376 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Tue, 15 Aug 2023 13:35:34 -0400 Subject: [PATCH 22/36] Normalize YAML --- config/locales/idv/en.yml | 3 ++- config/locales/idv/es.yml | 3 ++- config/locales/idv/fr.yml | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/config/locales/idv/en.yml b/config/locales/idv/en.yml index c8a2982f9e1..22b9a8beea9 100644 --- a/config/locales/idv/en.yml +++ b/config/locales/idv/en.yml @@ -238,7 +238,8 @@ en: %{date_letter_was_sent}. You’ll need to enter the code from the letter to finish verifying your identity. Sign back in to %{app_name} to finish verifying your identity. - did_not_get_a_letter_html: If you didn’t get this letter, %{another_letter_link} + did_not_get_a_letter_html: If you didn’t get this letter, %{another_letter_link} finish: Finish verifying your identity sign_in_and_request_another_letter: sign in to request another letter subject: Finish verifying your identity diff --git a/config/locales/idv/es.yml b/config/locales/idv/es.yml index c32241524ca..5df5ea0c35b 100644 --- a/config/locales/idv/es.yml +++ b/config/locales/idv/es.yml @@ -247,7 +247,8 @@ es: para verificar tu identidad. Deberás ingresar el código que contiene esa carta para completar la verificación por correo. Vuelve a iniciar sesión en %{app_name} para terminar de verificar tu identidad. - did_not_get_a_letter_html: Si no recibiste dicha carta, %{another_letter_link} + did_not_get_a_letter_html: Si no recibiste dicha carta, %{another_letter_link} finish: Termina de verificar tu identidad sign_in_and_request_another_letter: inicia sesión para solicitar otra subject: Termina de verificar tu identidad diff --git a/config/locales/idv/fr.yml b/config/locales/idv/fr.yml index 923304577fe..b40fca2aed5 100644 --- a/config/locales/idv/fr.yml +++ b/config/locales/idv/fr.yml @@ -264,7 +264,8 @@ fr: figurant dans la lettre pour terminer la vérification par courrier. Reconnectez-vous à %{app_name} pour terminer la vérification de votre identité. - did_not_get_a_letter_html: Si vous n’avez pas reçu cette lettre, %{another_letter_link} + did_not_get_a_letter_html: Si vous n’avez pas reçu cette lettre, %{another_letter_link} finish: Terminer la vérification de votre identité sign_in_and_request_another_letter: connectez-vous pour en demander une autre. subject: Terminer la vérification de votre identité From 0a0a36df23c3cecbb3bd9c3edcd5a0d3583a826d Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Tue, 15 Aug 2023 16:38:52 -0400 Subject: [PATCH 23/36] Review comment --- app/services/analytics_events.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/services/analytics_events.rb b/app/services/analytics_events.rb index 85dba267330..92a666a1584 100644 --- a/app/services/analytics_events.rb +++ b/app/services/analytics_events.rb @@ -1037,8 +1037,8 @@ def idv_gpo_confirm_start_over_visited # A GPO reminder email was sent to the user # @param [String] user_id UUID of user who we sent a reminder to - def idv_gpo_reminder_email_sent(user_id:, **_extra) - track_event('IdV: gpo reminder email sent', user_id: user_id) + def idv_gpo_reminder_email_sent(user_id:, **extra) + track_event('IdV: gpo reminder email sent', user_id: user_id, **extra) end # @identity.idp.previous_event_name Account verification submitted From d64357a3d7b8f9edefb5a447141ccdd7d4af9d42 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Tue, 15 Aug 2023 16:43:20 -0400 Subject: [PATCH 24/36] Review comment Fixed incorrect help link. --- app/views/user_mailer/gpo_reminder.html.erb | 7 ++++++- spec/mailers/user_mailer_spec.rb | 12 ++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/app/views/user_mailer/gpo_reminder.html.erb b/app/views/user_mailer/gpo_reminder.html.erb index 95482bd9afa..f9356c7873d 100644 --- a/app/views/user_mailer/gpo_reminder.html.erb +++ b/app/views/user_mailer/gpo_reminder.html.erb @@ -6,7 +6,12 @@ ) %> <%= link_to( t('idv.troubleshooting.options.learn_more_verify_by_mail'), - 'https://www.google.com', + help_center_redirect_url( + category: 'verify-your-identity', + article: 'verify-your-address-by-mail', + flow: :idv, + step: :gpo_send_letter, + ), { style: "text-decoration: 'underline'" }, ) %>

diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index be10f01334b..6a8770965c4 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -922,5 +922,17 @@ def expect_email_body_to_have_help_and_contact_links href: idv_gpo_verify_url(did_not_receive_letter: 1), ) end + + it 'renders the help link' do + expect(mail.html_part.body).to have_link( + t('idv.troubleshooting.options.learn_more_verify_by_mail'), + href: help_center_redirect_url( + category: 'verify-your-identity', + article: 'verify-your-address-by-mail', + flow: :idv, + step: :gpo_send_letter, + ), + ) + end end end From affdbf772785e0098cd8d74a670aef06e5c31574 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Tue, 15 Aug 2023 17:08:04 -0400 Subject: [PATCH 25/36] Lint --- app/views/user_mailer/gpo_reminder.html.erb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/views/user_mailer/gpo_reminder.html.erb b/app/views/user_mailer/gpo_reminder.html.erb index f9356c7873d..17d16ecc87b 100644 --- a/app/views/user_mailer/gpo_reminder.html.erb +++ b/app/views/user_mailer/gpo_reminder.html.erb @@ -6,12 +6,12 @@ ) %> <%= link_to( t('idv.troubleshooting.options.learn_more_verify_by_mail'), - help_center_redirect_url( - category: 'verify-your-identity', - article: 'verify-your-address-by-mail', - flow: :idv, - step: :gpo_send_letter, - ), + help_center_redirect_url( + category: 'verify-your-identity', + article: 'verify-your-address-by-mail', + flow: :idv, + step: :gpo_send_letter, + ), { style: "text-decoration: 'underline'" }, ) %>

From f3489a06ea435f29f2a94caf58ac70cf75ec583b Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Wed, 16 Aug 2023 12:31:10 -0400 Subject: [PATCH 26/36] Update spec/jobs/gpo_reminder_job_spec.rb Co-authored-by: Zach Margolis --- spec/jobs/gpo_reminder_job_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/jobs/gpo_reminder_job_spec.rb b/spec/jobs/gpo_reminder_job_spec.rb index fb5bc33c4e9..686f7e2c695 100644 --- a/spec/jobs/gpo_reminder_job_spec.rb +++ b/spec/jobs/gpo_reminder_job_spec.rb @@ -13,7 +13,7 @@ before do pending_profile.update( - gpo_verification_pending_at: Time.zone.now - WAIT_BEFORE_SENDING_REMINDER, + gpo_verification_pending_at: wait_before_sending_reminder.ago, ) allow(job).to receive(:analytics).and_return(job_analytics) end From 74b9dd62be2f43f67f2cd6250e723947cc83f883 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Wed, 16 Aug 2023 12:31:32 -0400 Subject: [PATCH 27/36] Update spec/jobs/gpo_reminder_job_spec.rb Co-authored-by: Zach Margolis --- spec/jobs/gpo_reminder_job_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/jobs/gpo_reminder_job_spec.rb b/spec/jobs/gpo_reminder_job_spec.rb index 686f7e2c695..134bf4a4a33 100644 --- a/spec/jobs/gpo_reminder_job_spec.rb +++ b/spec/jobs/gpo_reminder_job_spec.rb @@ -1,10 +1,10 @@ require 'rails_helper' RSpec.describe GpoReminderJob do - WAIT_BEFORE_SENDING_REMINDER = 14.days + let(:wait_before_sending_reminder) { 14.days } describe '#perform' do - subject(:perform) { job.perform(Time.zone.now - WAIT_BEFORE_SENDING_REMINDER) } + subject(:perform) { job.perform(wait_before_sending_reminder.ago) } let(:job) { GpoReminderJob.new } let(:user) { create(:user, :with_pending_gpo_profile) } From b719f1cdd4f24bf72878ff761cc18256da360953 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Wed, 16 Aug 2023 12:33:45 -0400 Subject: [PATCH 28/36] Fixed preview --- spec/mailers/previews/user_mailer_preview.rb | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/spec/mailers/previews/user_mailer_preview.rb b/spec/mailers/previews/user_mailer_preview.rb index c7ef6e15ce4..c94c93819db 100644 --- a/spec/mailers/previews/user_mailer_preview.rb +++ b/spec/mailers/previews/user_mailer_preview.rb @@ -181,7 +181,7 @@ def suspended_reset_password def gpo_reminder UserMailer.with( - user: user, + user: user_with_pending_gpo_letter, email_address: email_address_record, ).gpo_reminder end @@ -192,6 +192,19 @@ def user unsaveable(User.new(email_addresses: [email_address_record])) end + def user_with_pending_gpo_letter + raw_user = user + gpo_pending_profile = unsaveable( + Profile.new( + user: raw_user, + active: false, + gpo_verification_pending_at: Time.zone.now, + ), + ) + raw_user.send(:instance_variable_set, :@pending_profile, gpo_pending_profile) + raw_user + end + def email_address 'email@example.com' end From d5a5304ca4375282746ff5dc3dcd28e96fd95f7c Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Wed, 16 Aug 2023 12:49:21 -0400 Subject: [PATCH 29/36] Review comments Cleaned up I18n HTML handling per Zach. --- app/views/user_mailer/gpo_reminder.html.erb | 2 +- config/locales/idv/en.yml | 3 +-- config/locales/idv/es.yml | 3 +-- config/locales/idv/fr.yml | 3 +-- 4 files changed, 4 insertions(+), 7 deletions(-) diff --git a/app/views/user_mailer/gpo_reminder.html.erb b/app/views/user_mailer/gpo_reminder.html.erb index 17d16ecc87b..369a42ba835 100644 --- a/app/views/user_mailer/gpo_reminder.html.erb +++ b/app/views/user_mailer/gpo_reminder.html.erb @@ -43,7 +43,7 @@

<%= t( 'idv.messages.gpo_reminder.did_not_get_a_letter_html', - another_letter_link: link_to( + another_letter_link_html: link_to( t('idv.messages.gpo_reminder.sign_in_and_request_another_letter'), idv_gpo_verify_url(did_not_receive_letter: 1), { style: "text-decoration: 'underline'" }, diff --git a/config/locales/idv/en.yml b/config/locales/idv/en.yml index 22b9a8beea9..cee6965d4f9 100644 --- a/config/locales/idv/en.yml +++ b/config/locales/idv/en.yml @@ -238,8 +238,7 @@ en: %{date_letter_was_sent}. You’ll need to enter the code from the letter to finish verifying your identity. Sign back in to %{app_name} to finish verifying your identity. - did_not_get_a_letter_html: If you didn’t get this letter, %{another_letter_link} + did_not_get_a_letter_html: If you didn’t get this letter, %{another_letter_link_html} finish: Finish verifying your identity sign_in_and_request_another_letter: sign in to request another letter subject: Finish verifying your identity diff --git a/config/locales/idv/es.yml b/config/locales/idv/es.yml index 5df5ea0c35b..fba142902e6 100644 --- a/config/locales/idv/es.yml +++ b/config/locales/idv/es.yml @@ -247,8 +247,7 @@ es: para verificar tu identidad. Deberás ingresar el código que contiene esa carta para completar la verificación por correo. Vuelve a iniciar sesión en %{app_name} para terminar de verificar tu identidad. - did_not_get_a_letter_html: Si no recibiste dicha carta, %{another_letter_link} + did_not_get_a_letter_html: Si no recibiste dicha carta, %{another_letter_link_html} finish: Termina de verificar tu identidad sign_in_and_request_another_letter: inicia sesión para solicitar otra subject: Termina de verificar tu identidad diff --git a/config/locales/idv/fr.yml b/config/locales/idv/fr.yml index b40fca2aed5..88b0323a292 100644 --- a/config/locales/idv/fr.yml +++ b/config/locales/idv/fr.yml @@ -264,8 +264,7 @@ fr: figurant dans la lettre pour terminer la vérification par courrier. Reconnectez-vous à %{app_name} pour terminer la vérification de votre identité. - did_not_get_a_letter_html: Si vous n’avez pas reçu cette lettre, %{another_letter_link} + did_not_get_a_letter_html: Si vous n’avez pas reçu cette lettre, %{another_letter_link_html} finish: Terminer la vérification de votre identité sign_in_and_request_another_letter: connectez-vous pour en demander une autre. subject: Terminer la vérification de votre identité From e36a6eedc9ac0b481bca46fefcfc2fd87761c97b Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Wed, 16 Aug 2023 16:04:28 -0400 Subject: [PATCH 30/36] Review comments. --- app/views/user_mailer/gpo_reminder.html.erb | 14 ++++++++------ config/locales/idv/en.yml | 4 ++-- config/locales/idv/es.yml | 4 ++-- config/locales/idv/fr.yml | 4 ++-- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/app/views/user_mailer/gpo_reminder.html.erb b/app/views/user_mailer/gpo_reminder.html.erb index 369a42ba835..b69a21dfd2b 100644 --- a/app/views/user_mailer/gpo_reminder.html.erb +++ b/app/views/user_mailer/gpo_reminder.html.erb @@ -1,10 +1,5 @@

- <%= t( - 'idv.messages.gpo_reminder.body_html', - date_letter_was_sent: @date_letter_was_sent, - app_name: APP_NAME, - ) %> - <%= link_to( + <%= help_link = link_to( t('idv.troubleshooting.options.learn_more_verify_by_mail'), help_center_redirect_url( category: 'verify-your-identity', @@ -13,6 +8,13 @@ step: :gpo_send_letter, ), { style: "text-decoration: 'underline'" }, + ) + + t( + 'idv.messages.gpo_reminder.body_html', + date_letter_was_sent: @date_letter_was_sent, + app_name: APP_NAME, + help_link: help_link, ) %>

diff --git a/config/locales/idv/en.yml b/config/locales/idv/en.yml index cee6965d4f9..4f4146cc122 100644 --- a/config/locales/idv/en.yml +++ b/config/locales/idv/en.yml @@ -237,8 +237,8 @@ en: body_html: You requested a letter to verify your identity on %{date_letter_was_sent}. You’ll need to enter the code from the letter to finish verifying your identity. Sign back in - to %{app_name} to finish verifying your identity. - did_not_get_a_letter_html: If you didn’t get this letter, %{another_letter_link_html} + to %{app_name} to finish verifying your identity. %{help_link}. + did_not_get_a_letter_html: If you didn’t get this letter, %{another_letter_link_html}. finish: Finish verifying your identity sign_in_and_request_another_letter: sign in to request another letter subject: Finish verifying your identity diff --git a/config/locales/idv/es.yml b/config/locales/idv/es.yml index fba142902e6..97c6c03de00 100644 --- a/config/locales/idv/es.yml +++ b/config/locales/idv/es.yml @@ -246,8 +246,8 @@ es: body_html: El %{date_letter_was_sent} solicitaste una carta para verificar tu identidad. Deberás ingresar el código que contiene esa carta para completar la verificación por correo. Vuelve a iniciar - sesión en %{app_name} para terminar de verificar tu identidad. - did_not_get_a_letter_html: Si no recibiste dicha carta, %{another_letter_link_html} + sesión en %{app_name} para terminar de verificar tu identidad. %{help_link}. + did_not_get_a_letter_html: Si no recibiste dicha carta, %{another_letter_link_html}. finish: Termina de verificar tu identidad sign_in_and_request_another_letter: inicia sesión para solicitar otra subject: Termina de verificar tu identidad diff --git a/config/locales/idv/fr.yml b/config/locales/idv/fr.yml index 88b0323a292..89989952b94 100644 --- a/config/locales/idv/fr.yml +++ b/config/locales/idv/fr.yml @@ -263,8 +263,8 @@ fr: %{date_letter_was_sent}. Vous devrez saisir le code figurant dans la lettre pour terminer la vérification par courrier. Reconnectez-vous à %{app_name} pour terminer la vérification de votre - identité. - did_not_get_a_letter_html: Si vous n’avez pas reçu cette lettre, %{another_letter_link_html} + identité. %{help_link}. + did_not_get_a_letter_html: Si vous n’avez pas reçu cette lettre, %{another_letter_link_html}. finish: Terminer la vérification de votre identité sign_in_and_request_another_letter: connectez-vous pour en demander une autre. subject: Terminer la vérification de votre identité From 7dbf07a805a44342a3dfa25f09b1a9fd3c097bc9 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Wed, 16 Aug 2023 17:20:36 -0400 Subject: [PATCH 31/36] Added spec for user who completes GPO proofing before their reminder --- spec/services/gpo_reminder_sender_spec.rb | 36 ++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/spec/services/gpo_reminder_sender_spec.rb b/spec/services/gpo_reminder_sender_spec.rb index 0aca9cec48e..4dcd201b3e3 100644 --- a/spec/services/gpo_reminder_sender_spec.rb +++ b/spec/services/gpo_reminder_sender_spec.rb @@ -12,7 +12,9 @@ let(:time_yesterday) { Time.zone.now - 1.day } def set_gpo_verification_pending_at(to_time) - user.gpo_verification_pending_profile.update(gpo_verification_pending_at: to_time) + user. + gpo_verification_pending_profile. + update(gpo_verification_pending_at: to_time) end def set_reminder_sent_at(to_time) @@ -89,6 +91,38 @@ def set_reminder_sent_at(to_time) not_to change { fake_analytics.events.count } end end + + context 'but the user has completed gpo verification' do + before do + otp = 'ABC123' + pending_profile = user.gpo_verification_pending_profile + + pending_profile.gpo_confirmation_codes = [ + create( + :gpo_confirmation_code, + otp_fingerprint: Pii::Fingerprinter.fingerprint(otp), + code_sent_at: Time.zone.now, + profile: pending_profile, + ), + ] + + GpoVerifyForm.new( + user: user, + pii: Idp::Constants::MOCK_IDV_APPLICANT_WITH_PHONE, + otp: otp, + ).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 + + it 'logs no events' do + expect { subject.send_emails(time_due_for_reminder) }. + not_to change { fake_analytics.events.count } + end + end end end end From f6edd1cecb17b812107835aea3f37f3d554f802d Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Wed, 16 Aug 2023 18:05:03 -0400 Subject: [PATCH 32/36] Fixed spec. --- spec/mailers/user_mailer_spec.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/spec/mailers/user_mailer_spec.rb b/spec/mailers/user_mailer_spec.rb index 6a8770965c4..7e2d1c0b7f7 100644 --- a/spec/mailers/user_mailer_spec.rb +++ b/spec/mailers/user_mailer_spec.rb @@ -898,11 +898,23 @@ def expect_email_body_to_have_help_and_contact_links end it 'renders the body' do + expected_help_link = ActionController::Base.helpers.link_to( + t('idv.troubleshooting.options.learn_more_verify_by_mail'), + help_center_redirect_url( + category: 'verify-your-identity', + article: 'verify-your-address-by-mail', + flow: :idv, + step: :gpo_send_letter, + ), + { style: "text-decoration: 'underline'" }, + ) + expected_body = strip_tags( t( 'idv.messages.gpo_reminder.body_html', date_letter_was_sent: date_letter_was_sent.strftime(t('time.formats.event_date')), app_name: APP_NAME, + help_link: expected_help_link, ), ) From 0227318cbf874045008f63a9c68b3394678a0d66 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Thu, 17 Aug 2023 13:45:05 -0400 Subject: [PATCH 33/36] Spec for updating sent_at time --- spec/services/gpo_reminder_sender_spec.rb | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/spec/services/gpo_reminder_sender_spec.rb b/spec/services/gpo_reminder_sender_spec.rb index 4dcd201b3e3..14175ee9554 100644 --- a/spec/services/gpo_reminder_sender_spec.rb +++ b/spec/services/gpo_reminder_sender_spec.rb @@ -5,6 +5,13 @@ subject(:sender) { GpoReminderSender.new(fake_analytics) } let(:user) { create(:user, :with_pending_gpo_profile) } + let(:gpo_confirmation_code) do + user. + gpo_verification_pending_profile. + gpo_confirmation_codes. + first + end + let(:fake_analytics) { FakeAnalytics.new } let(:wait_for_reminder) { 14.days } let(:time_due_for_reminder) { Time.zone.now - wait_for_reminder } @@ -18,11 +25,9 @@ def set_gpo_verification_pending_at(to_time) end def set_reminder_sent_at(to_time) - user. - gpo_verification_pending_profile. - gpo_confirmation_codes. - first. - update(reminder_sent_at: to_time) + gpo_confirmation_code.update( + reminder_sent_at: to_time, + ) end context 'when no users need a reminder' do @@ -53,6 +58,12 @@ def set_reminder_sent_at(to_time) expect(fake_analytics).to have_logged_event('IdV: gpo reminder email sent') end + 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) + end + context 'and the user has multiple emails' do let(:user) { create(:user, :with_pending_gpo_profile, :with_multiple_emails) } From 54f160f96c0ea6dce7c8d0589012b077a5449e51 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Thu, 17 Aug 2023 15:27:00 -0400 Subject: [PATCH 34/36] Review comments. Stopped passing analytics in, because it's unfashionable. --- app/jobs/gpo_reminder_job.rb | 8 +------- app/services/gpo_reminder_sender.rb | 13 ++++++++----- spec/jobs/gpo_reminder_job_spec.rb | 2 +- spec/services/gpo_reminder_sender_spec.rb | 4 +++- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/app/jobs/gpo_reminder_job.rb b/app/jobs/gpo_reminder_job.rb index 9985f980aa6..5345a0a8bc9 100644 --- a/app/jobs/gpo_reminder_job.rb +++ b/app/jobs/gpo_reminder_job.rb @@ -4,13 +4,7 @@ class GpoReminderJob < ApplicationJob # Send email reminders to people with USPS proofing letters whose # letters were sent a while ago, and haven't yet entered their code def perform(cutoff_time_for_sending_reminders) - GpoReminderSender.new(analytics). + GpoReminderSender.new. send_emails(cutoff_time_for_sending_reminders) end - - private - - def analytics(user: AnonymousUser.new) - Analytics.new(user: user, request: nil, session: {}, sp: nil) - end end diff --git a/app/services/gpo_reminder_sender.rb b/app/services/gpo_reminder_sender.rb index 905357d8845..3c1b1a72a91 100644 --- a/app/services/gpo_reminder_sender.rb +++ b/app/services/gpo_reminder_sender.rb @@ -1,8 +1,4 @@ class GpoReminderSender - def initialize(analytics) - @analytics = analytics - end - def send_emails(for_letters_sent_before) profiles_due_for_reminder = Profile.joins(:gpo_confirmation_codes). where( @@ -12,7 +8,14 @@ def send_emails(for_letters_sent_before) profiles_due_for_reminder.each do |profile| profile.user.send_email_to_all_addresses(:gpo_reminder) - @analytics.idv_gpo_reminder_email_sent(user_id: profile.user.uuid) + profile.gpo_confirmation_codes.first.update(reminder_sent_at: Time.zone.now) + analytics.idv_gpo_reminder_email_sent(user_id: profile.user.uuid) end end + + private + + def analytics + Analytics.new(user: AnonymousUser.new, request: nil, session: {}, sp: nil) + end end diff --git a/spec/jobs/gpo_reminder_job_spec.rb b/spec/jobs/gpo_reminder_job_spec.rb index 134bf4a4a33..59a2d6eb75f 100644 --- a/spec/jobs/gpo_reminder_job_spec.rb +++ b/spec/jobs/gpo_reminder_job_spec.rb @@ -15,7 +15,7 @@ pending_profile.update( gpo_verification_pending_at: wait_before_sending_reminder.ago, ) - allow(job).to receive(:analytics).and_return(job_analytics) + allow(Analytics).to receive(:new).and_return(job_analytics) end it 'sends reminder emails' do diff --git a/spec/services/gpo_reminder_sender_spec.rb b/spec/services/gpo_reminder_sender_spec.rb index 14175ee9554..1c5e328c7c9 100644 --- a/spec/services/gpo_reminder_sender_spec.rb +++ b/spec/services/gpo_reminder_sender_spec.rb @@ -2,7 +2,7 @@ RSpec.describe GpoReminderSender do describe '#send_emails' do - subject(:sender) { GpoReminderSender.new(fake_analytics) } + subject(:sender) { GpoReminderSender.new } let(:user) { create(:user, :with_pending_gpo_profile) } let(:gpo_confirmation_code) do @@ -30,6 +30,8 @@ def set_reminder_sent_at(to_time) ) end + 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) } From e563453b47b3edcf44406ce4f04d89bd5244c9a3 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Mon, 21 Aug 2023 12:18:42 -0400 Subject: [PATCH 35/36] Review comment Changed `@date_letter_was_sent` to `@gpo_verification_pending_at` in gpo letter reminder email template. More clearly states the value being dealt with. --- app/mailers/user_mailer.rb | 2 +- app/views/user_mailer/gpo_reminder.html.erb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 70c0580713a..43d2f59d80a 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -393,7 +393,7 @@ def suspended_reset_password def gpo_reminder with_user_locale(user) do - @date_letter_was_sent = I18n.l( + @gpo_verification_pending_at = I18n.l( user.gpo_verification_pending_profile.gpo_verification_pending_at, format: :event_date, ) diff --git a/app/views/user_mailer/gpo_reminder.html.erb b/app/views/user_mailer/gpo_reminder.html.erb index b69a21dfd2b..ef04ff809b6 100644 --- a/app/views/user_mailer/gpo_reminder.html.erb +++ b/app/views/user_mailer/gpo_reminder.html.erb @@ -12,7 +12,7 @@ t( 'idv.messages.gpo_reminder.body_html', - date_letter_was_sent: @date_letter_was_sent, + date_letter_was_sent: @gpo_verification_pending_at, app_name: APP_NAME, help_link: help_link, ) %> From b49d2d664e1b29cef53f0bb174ef7943a17a26b6 Mon Sep 17 00:00:00 2001 From: John Maxwell Date: Mon, 21 Aug 2023 12:26:24 -0400 Subject: [PATCH 36/36] Lint nits --- config/locales/idv/es.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/config/locales/idv/es.yml b/config/locales/idv/es.yml index 97c6c03de00..753352e6088 100644 --- a/config/locales/idv/es.yml +++ b/config/locales/idv/es.yml @@ -246,7 +246,8 @@ es: body_html: El %{date_letter_was_sent} solicitaste una carta para verificar tu identidad. Deberás ingresar el código que contiene esa carta para completar la verificación por correo. Vuelve a iniciar - sesión en %{app_name} para terminar de verificar tu identidad. %{help_link}. + sesión en %{app_name} para terminar de verificar tu identidad. + %{help_link}. did_not_get_a_letter_html: Si no recibiste dicha carta, %{another_letter_link_html}. finish: Termina de verificar tu identidad sign_in_and_request_another_letter: inicia sesión para solicitar otra