Skip to content

LG-7185 email reminders #7198

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
svalexander merged 26 commits into from
Oct 28, 2022
Merged

LG-7185 email reminders #7198

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
bdef9b4
translations
svalexander Oct 21, 2022
5d2a695
reminder email
svalexander Oct 21, 2022
8e3762b
move date methods to inpersonenrollment model
svalexander Oct 24, 2022
cb4b59d
update presenter and test
svalexander Oct 24, 2022
d26dbf7
update tests
svalexander Oct 24, 2022
d589e77
change variable name
svalexander Oct 24, 2022
7edc39c
user config value to calculate days
svalexander Oct 24, 2022
45a44ad
refactor days_remaining
svalexander Oct 24, 2022
2020506
Improvements, Email, reminder emails sent to users
svalexander Oct 24, 2022
1f4e906
lint fix
svalexander Oct 25, 2022
46fde8d
fix more lint issues
svalexander Oct 25, 2022
921b3c8
changelog: Improvements, Email, reminder emails sent to users
svalexander Oct 25, 2022
b7f890e
Improvements, Email, reminder emails sent to users
svalexander Oct 24, 2022
77bcd73
changelog: Improvements, Email, reminder emails sent to users
svalexander Oct 25, 2022
740e76e
change days to day remaining
svalexander Oct 25, 2022
cfcd2a1
Merge remote-tracking branch 'origin/main' into shannon/lg-7185-email…
Oct 25, 2022
1ff5290
working on changes
svalexander Oct 26, 2022
6732692
Create shared view for ready to verify email body
Oct 26, 2022
a2640c4
Remove pending trait from with_service_provider
Oct 26, 2022
b9d2513
using service provider instead of issuer
svalexander Oct 26, 2022
1107866
remove unused issuer
svalexander Oct 26, 2022
aee55c7
remove job
svalexander Oct 26, 2022
7bfb0e0
normalize yaml
svalexander Oct 26, 2022
31e9165
Merge branch 'main' into shannon/lg-7185-email-reminders
svalexander Oct 26, 2022
5a7357b
move heading to user mailer
svalexander Oct 28, 2022
e47c997
Empty-Commit
svalexander Oct 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions app/mailers/user_mailer.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,30 @@ def in_person_ready_to_verify(enrollment:)
end
end

def in_person_ready_to_verify_reminder(enrollment:)
attachments.inline['barcode.png'] = BarcodeOutputter.new(
code: enrollment.enrollment_code,
).image_data

with_user_locale(user) do
@presenter = Idv::InPerson::ReadyToVerifyPresenter.new(
enrollment: enrollment,
barcode_image_url: attachments['barcode.png'].url,
)
@header = t(
'user_mailer.in_person_ready_to_verify_reminder.heading',
days_remaining: @presenter.days_remaining,
)
mail(
to: email_address.email,
subject: t(
'user_mailer.in_person_ready_to_verify_reminder.subject',
days_remaining: @presenter.days_remaining,
),
)
end
end

def in_person_verified(enrollment:)
with_user_locale(user) do
@hide_title = true
Expand Down
10 changes: 10 additions & 0 deletions app/models/in_person_enrollment.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,16 @@ def self.generate_unique_id
SecureRandom.hex(9)
end

def due_date
Copy link
Contributor

@sheldon-b sheldon-b Oct 25, 2022

Choose a reason for hiding this comment

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

This is non-blocking, it's kind of beyond the scope of this ticket. But I feel like expiration_date is a slightly better name, it better describes what the consequence of this date is and relates to the expired status of enrollments. Would require updating a few other places in the code to make this change cohesive though 🤷

Suggested change
def due_date
def expiration_date

start_date = enrollment_established_at.presence || created_at
start_date + IdentityConfig.store.in_person_enrollment_validity_in_days.days
end
Comment on lines +68 to +71
Copy link
Contributor

@sheldon-b sheldon-b Oct 27, 2022

Choose a reason for hiding this comment

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

I think this would be misleading for enrollments that have status establishing. Some preference for just returning nil in that case

Suggested change
def due_date
start_date = enrollment_established_at.presence || created_at
start_date + IdentityConfig.store.in_person_enrollment_validity_in_days.days
end
def due_date
nil unless established_at
established_at + IdentityConfig.store.in_person_enrollment_validity_in_days.days
end

Copy link
Contributor Author

@svalexander svalexander Oct 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

we were using created_at here to account for some enrollments that only have created_at defined when the enrollment status changes to pending and the user becomes ready to verify. We want to default to created_at if there's no enrollment_established_at value. (ticket)

Copy link
Contributor

@sheldon-b sheldon-b Oct 28, 2022

Choose a reason for hiding this comment

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

An enrollment doesn't have a due date until it's pending though. If we use the created_at timestamp we're just making something up that isn't real. That date has no relevance to a user or USPS. Seems like the only reason to use created_at as a fallback is to return some sort of date if we ever execute that codepath, so that we're showing the user some sort of date. But if we ever execute that codepath with an enrollment that doesn't have enrollment_established_at then wouldn't it be a bug?

It's not a hill I want to die on but I am curious why we defined it that way in the ticket, I'm going to ask the ticket creator

Copy link
Contributor

@tomas-nava tomas-nava Oct 28, 2022

Choose a reason for hiding this comment

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

I'm fine with removing the fallback to created_at. Note though, that

  1. the variable should be enrollment_established_at in sheldon-b's suggested change, and
  2. the days_to_due_date method will have to be updated to return nil if due_date returns nil (otherwise, it'll return (DateTime.now...nil).count which is Inifinity).
  3. the days_remaining method on the ReadyToVerifyPresenter will have to be updated to anticipate a nil value from days_to_due_date (otherwise it'll raise a NoMethodError).

I don't think we have to be too rigorous about protecting against this case though, because when we write the job, we shouldn't be generating emails for non-pending enrollments, which means we should never be in the circumstance where we're getting a nil value for due date.


def days_to_due_date
Copy link
Contributor

@sheldon-b sheldon-b Oct 25, 2022

Choose a reason for hiding this comment

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

Same as above, non-blocking suggestion

Suggested change
def days_to_due_date
def days_to_expiration

today = DateTime.now
(today...due_date).count
end

private

def on_status_updated
Expand Down
24 changes: 17 additions & 7 deletions app/presenters/idv/in_person/ready_to_verify_presenter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,20 @@ class ReadyToVerifyPresenter

delegate :selected_location_details, :enrollment_code, to: :enrollment

def initialize(enrollment:, barcode_image_url: nil)
def initialize(enrollment:, barcode_image_url: nil, sp_name: nil)
@enrollment = enrollment
@barcode_image_url = barcode_image_url
@sp_name = sp_name
end

# Reminder is exclusive of the day the email is sent (1 less than days_to_due_date)
def days_remaining
enrollment.days_to_due_date - 1
end

def formatted_due_date
due_date.in_time_zone(USPS_SERVER_TIMEZONE).strftime(I18n.t('time.formats.event_date'))
enrollment.due_date.in_time_zone(USPS_SERVER_TIMEZONE).
strftime(I18n.t('time.formats.event_date'))
end

def selected_location_hours(prefix)
Expand All @@ -27,15 +34,18 @@ def needs_proof_of_address?
!enrollment.current_address_matches_id
end

def service_provider
enrollment.service_provider
end

def sp_name
service_provider ? service_provider.friendly_name : ''
Copy link
Contributor

@sheldon-b sheldon-b Oct 27, 2022

Choose a reason for hiding this comment

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

I have a question about what should happen if there is no service provider. We'll see that a lot in lower environments today. And after we finish the pilot I think there will be ways for production users to get into the IPP flow without an associated partner, such as by going through the password reset flow

I left a question on the Figma file about it

Copy link
Contributor Author

@svalexander svalexander Oct 28, 2022

Choose a reason for hiding this comment

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

that's a good question. i left it as an empty string and so in examples i looked at w/o one it looks like an unfinished sentence -it definitely is not ideal. would be good to adjust for sure.

Copy link
Contributor

@sheldon-b sheldon-b Oct 28, 2022

Choose a reason for hiding this comment

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

Lizzie suggested we discuss it at design critique on Monday so I'm going to plan to attend for that. Do you want to join too?

Copy link
Contributor

@sheldon-b sheldon-b Nov 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

We discussed during design critique and tentatively agreed to fall back to Login.gov when there's no service provider, similar to how we do in other places. Design is going to look into other places where this might be an issue in IPP and make suggestions for a fallback

Suggested change
service_provider ? service_provider.friendly_name : ''
service_provider&.friendly_name || APP_NAME

Copy link
Contributor

@tomas-nava tomas-nava Nov 1, 2022

Choose a reason for hiding this comment

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

this is being addressed in #7256

end

private

attr_reader :enrollment

def due_date
start_date = enrollment.enrollment_established_at.presence || enrollment.created_at
start_date + IdentityConfig.store.in_person_enrollment_validity_in_days.days
end

def localized_hours(hours)
case hours
when 'Closed'
Expand Down
97 changes: 1 addition & 96 deletions app/views/user_mailer/in_person_ready_to_verify.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,99 +3,4 @@
<%= t('user_mailer.in_person_ready_to_verify.intro') %>
</p>

<table class="info-alert margin-y-4">
<tr>
<td width="16">
<%= image_tag('email/info.png', width: 16, height: 16, alt: '') %>
</td>
<td>
<p class="margin-bottom-1"><strong><%= t('in_person_proofing.body.barcode.deadline', deadline: @presenter.formatted_due_date) %></strong></p>
<p class="margin-bottom-0"><%= t('in_person_proofing.body.barcode.deadline_restart') %></p>
</td>
</tr>
</table>

<div class="border-1px border-primary-light radius-lg padding-4">
<h2 class="margin-top-0 margin-bottom-2 font-heading-lg text-bold">
<%= t('in_person_proofing.body.barcode.items_to_bring') %>
</h2>
<table class="process-list">
<tr>
<td><div class="process-list__circle">1</div></td>
<td>
<h3 class="font-heading-md text-bold"><%= t('in_person_proofing.process.barcode.heading') %></h3>
<p><%= t('in_person_proofing.process.barcode.info') %></p>
<%= render BarcodeComponent.new(
barcode_data: @presenter.enrollment_code,
barcode_image_url: @presenter.barcode_image_url,
label: nil,
label_formatter: Idv::InPerson::EnrollmentCodeFormatter.method(:format),
) %>
</td>
</tr>
<tr>
<td><div class="process-list__circle">2</div></td>
<td>
<h3 class="font-heading-md text-bold"><%= t('in_person_proofing.process.state_id.heading') %></h3>
<p class="margin-bottom-105"><%= t('in_person_proofing.process.state_id.info') %></p>
<ul class="usa-list margin-y-105">
<% t('in_person_proofing.process.state_id.acceptable_documents').each do |document| %>
<li><%= document %></li>
<% end %>
</ul>
<p><%= t('in_person_proofing.process.state_id.no_other_documents') %></p>
</td>
</tr>
<% if @presenter.needs_proof_of_address? %>
<tr>
<td><div class="process-list__circle">3</div></td>
<td>
<h3 class="font-heading-md text-bold"><%= t('in_person_proofing.process.proof_of_address.heading') %></h3>
<p class="margin-bottom-105"><%= t('in_person_proofing.process.proof_of_address.info') %></p>
<ul class="usa-list margin-y-105">
<% t('in_person_proofing.process.proof_of_address.acceptable_proof').each do |proof| %>
<li><%= proof %></li>
<% end %>
</ul>
<p><%= t('in_person_proofing.process.proof_of_address.physical_or_digital_copy') %></p>
</td>
</tr>
<% end %>
</table>
<p class="margin-bottom-0">
<%= t('in_person_proofing.body.barcode.items_to_bring_questions') %>
<%= link_to(
t('in_person_proofing.body.barcode.learn_more'),
MarketingSite.help_center_article_url(
category: 'verify-your-identity',
article: 'verify-your-identity-in-person',
),
) %>
</p>
</div>

<% if @presenter.selected_location_details.present? %>
<div class="margin-y-4">
<h2 class="font-sans-md margin-bottom-1 text-bold"><%= @presenter.selected_location_details['name'] %></h2>
<div class="margin-bottom-1">
<%= @presenter.selected_location_details['street_address'] %><br>
<%= @presenter.selected_location_details['formatted_city_state_zip'] %>
</div>
<div><strong><%= t('in_person_proofing.body.barcode.retail_hours') %></strong></div>
<div class="margin-bottom-2">
<%= t('date.range', from: t('date.day_names')[0], to: t('date.day_names')[4]) %>: <%= @presenter.selected_location_hours(:weekday) %><br>
<%= t('date.day_names')[5] %>: <%= @presenter.selected_location_hours(:saturday) %><br>
<%= t('date.day_names')[6] %>: <%= @presenter.selected_location_hours(:sunday) %>
</div>
<div>
<%= @presenter.selected_location_details[:phone] %>
</div>
</div>
<% end %>

<p><%= t('in_person_proofing.body.barcode.speak_to_associate') %></p>

<p>
<strong><%= t('in_person_proofing.body.barcode.deadline', deadline: @presenter.formatted_due_date) %></strong>
<%= t('in_person_proofing.body.barcode.no_appointment_required') %>
</p>
<%= render 'user_mailer/shared/in_person_ready_to_verify' %>
6 changes: 6 additions & 0 deletions app/views/user_mailer/in_person_ready_to_verify_reminder.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<p>
<%= t('user_mailer.in_person_ready_to_verify_reminder.greeting') %><br>
<%= t('user_mailer.in_person_ready_to_verify_reminder.intro', sp_name: @presenter.sp_name) %>
</p>

<%= render 'user_mailer/shared/in_person_ready_to_verify' %>
96 changes: 96 additions & 0 deletions app/views/user_mailer/shared/_in_person_ready_to_verify.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
<table class="info-alert margin-y-4">
<tr>
<td width="16">
<%= image_tag('email/info.png', width: 16, height: 16, alt: '') %>
</td>
<td>
<p class="margin-bottom-1"><strong><%= t('in_person_proofing.body.barcode.deadline', deadline: @presenter.formatted_due_date) %></strong></p>
<p class="margin-bottom-0"><%= t('in_person_proofing.body.barcode.deadline_restart') %></p>
</td>
</tr>
</table>

<div class="border-1px border-primary-light radius-lg padding-4">
<h2 class="margin-top-0 margin-bottom-2 font-heading-lg text-bold">
<%= t('in_person_proofing.body.barcode.items_to_bring') %>
</h2>
<table class="process-list">
<tr>
<td><div class="process-list__circle">1</div></td>
<td>
<h3 class="font-heading-md text-bold"><%= t('in_person_proofing.process.barcode.heading') %></h3>
<p><%= t('in_person_proofing.process.barcode.info') %></p>
<%= render BarcodeComponent.new(
barcode_data: @presenter.enrollment_code,
barcode_image_url: @presenter.barcode_image_url,
label: nil,
label_formatter: Idv::InPerson::EnrollmentCodeFormatter.method(:format),
) %>
</td>
</tr>
<tr>
<td><div class="process-list__circle">2</div></td>
<td>
<h3 class="font-heading-md text-bold"><%= t('in_person_proofing.process.state_id.heading') %></h3>
<p class="margin-bottom-105"><%= t('in_person_proofing.process.state_id.info') %></p>
<ul class="usa-list margin-y-105">
<% t('in_person_proofing.process.state_id.acceptable_documents').each do |document| %>
<li><%= document %></li>
<% end %>
</ul>
<p><%= t('in_person_proofing.process.state_id.no_other_documents') %></p>
</td>
</tr>
<% if @presenter.needs_proof_of_address? %>
<tr>
<td><div class="process-list__circle">3</div></td>
<td>
<h3 class="font-heading-md text-bold"><%= t('in_person_proofing.process.proof_of_address.heading') %></h3>
<p class="margin-bottom-105"><%= t('in_person_proofing.process.proof_of_address.info') %></p>
<ul class="usa-list margin-y-105">
<% t('in_person_proofing.process.proof_of_address.acceptable_proof').each do |proof| %>
<li><%= proof %></li>
<% end %>
</ul>
<p><%= t('in_person_proofing.process.proof_of_address.physical_or_digital_copy') %></p>
</td>
</tr>
<% end %>
</table>
<p class="margin-bottom-0">
<%= t('in_person_proofing.body.barcode.items_to_bring_questions') %>
<%= link_to(
t('in_person_proofing.body.barcode.learn_more'),
MarketingSite.help_center_article_url(
category: 'verify-your-identity',
article: 'verify-your-identity-in-person',
),
) %>
</p>
</div>

<% if @presenter.selected_location_details.present? %>
<div class="margin-y-4">
<h2 class="font-sans-md margin-bottom-1 text-normal text-bold"><%= @presenter.selected_location_details['name'] %></h2>
<div class="margin-bottom-1">
<%= @presenter.selected_location_details['street_address'] %><br>
<%= @presenter.selected_location_details['formatted_city_state_zip'] %>
</div>
<div><strong><%= t('in_person_proofing.body.barcode.retail_hours') %></strong></div>
<div class="margin-bottom-2">
<%= t('date.range', from: t('date.day_names')[0], to: t('date.day_names')[4]) %>: <%= @presenter.selected_location_hours(:weekday) %><br>
<%= t('date.day_names')[5] %>: <%= @presenter.selected_location_hours(:saturday) %><br>
<%= t('date.day_names')[6] %>: <%= @presenter.selected_location_hours(:sunday) %>
</div>
<div>
<%= @presenter.selected_location_details[:phone] %>
</div>
</div>
<% end %>

<p><%= t('in_person_proofing.body.barcode.speak_to_associate') %></p>

<p>
<strong><%= t('in_person_proofing.body.barcode.deadline', deadline: @presenter.formatted_due_date) %></strong>
<%= t('in_person_proofing.body.barcode.no_appointment_required') %>
</p>
6 changes: 6 additions & 0 deletions config/locales/user_mailer/en.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,12 @@ en:
intro: Here are the details to verify your identity in person at a United States
Post Office near you.
subject: You’re ready to verify your identity with %{app_name} in person
in_person_ready_to_verify_reminder:
greeting: Hello,
heading: You have %{days_remaining} days left to verify your identity in person
intro: Don’t miss the chance to verify your identity at your local Post Office.
Complete this step to access %{sp_name}.
subject: Verify your identity at a Post Office in the next %{days_remaining} days
in_person_verified:
greeting: Hello,
intro: You successfully verified your identity at the %{location} Post Office on
Expand Down
7 changes: 7 additions & 0 deletions config/locales/user_mailer/es.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,13 @@ es:
intro: Estos son los detalles para verificar su identidad en persona en una
oficina de correos de los Estados Unidos cercana a usted.
subject: Está listo para verificar su identidad con %{app_name} en persona
in_person_ready_to_verify_reminder:
greeting: Hola,
heading: Tiene %{days_remaining} días para verificar su identidad en persona
intro: No pierda la oportunidad de verificar su identidad en su oficina de
correos local. Complete este paso para acceder %{sp_name}.
subject: Verifique su identidad en una oficina de correos en los próximos
%{days_remaining} días
in_person_verified:
greeting: Hola,
intro: El %{date}, verificó correctamente su identidad en la oficina de correos
Expand Down
8 changes: 8 additions & 0 deletions config/locales/user_mailer/fr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,14 @@ fr:
intro: Voici les détails pour vérifier votre identité en personne dans un bureau
de poste des États-Unis près de chez vous.
subject: Vous êtes prêt à vérifier votre identité avec %{app_name} en personne
in_person_ready_to_verify_reminder:
greeting: Bonjour,
heading: Il vous reste %{days_remaining} jours pour vérifier votre identité en
personne
intro: Ne manquez pas l’occasion de vérifier votre identité dans votre bureau de
poste de proximité. Complétez cette étape pour accéder à %{sp_name}.
subject: Vérifiez votre identité auprès d’un bureau de poste dans les
%{days_remaining} prochains jours.
in_person_verified:
greeting: Bonjour,
intro: Vous avez vérifié avec succès votre identité au bureau de poste de
Expand Down
4 changes: 4 additions & 0 deletions spec/factories/in_person_enrollments.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,9 @@
status_check_attempted_at { Time.zone.now }
status_updated_at { Time.zone.now }
end

trait :with_service_provider do
service_provider { association :service_provider }
end
end
end
14 changes: 13 additions & 1 deletion spec/mailers/previews/user_mailer_preview.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,15 @@ def in_person_ready_to_verify
)
end

def in_person_ready_to_verify_reminder
UserMailer.with(
user: user,
email_address: email_address_record,
).in_person_ready_to_verify_reminder(
enrollment: in_person_enrollment,
)
end

def in_person_verified
UserMailer.with(user: user, email_address: email_address_record).in_person_verified(
enrollment: in_person_enrollment,
Expand Down Expand Up @@ -166,6 +175,10 @@ def in_person_enrollment
profile: unsaveable(Profile.new(user: user)),
enrollment_code: '2048702198804358',
created_at: Time.zone.now - 2.hours,
service_provider: ServiceProvider.new(
friendly_name: 'Test Service Provider',
issuer: SecureRandom.uuid,
),
status_updated_at: Time.zone.now - 1.hour,
current_address_matches_id: params['current_address_matches_id'] == 'true',
selected_location_details: {
Expand All @@ -177,7 +190,6 @@ def in_person_enrollment
'saturday_hours' => '9:00 AM - 12:00 PM',
'sunday_hours' => 'Closed',
},
service_provider: params[:issuer] ? ServiceProvider.find_by(issuer: params[:issuer]) : nil,
),
)
end
Expand Down
Loading