LG-14624: Display visited location in in_person_verified, in_person_failed, and in_person_failed_fraud emails

[eileen-nava]

🎫 Ticket

LG-14624: Fix bug where verified email shows selected post office location instead of visited post office location

🛠 Summary of changes

I realized that this bug existed in the below emails:

• in_person_verified
• in_person_failed
• in_person_failed_fraud

Additionally, the in_person_deadline_passed email template included variables it didn't need, such as the formatted_verified_date from the presenter. I removed the superfluous variables.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

• Inspect the in_person_please_call email via http://localhost:3000/rails/mailers/user_mailer/in_person_please_call. Confirm it looks the same on this branch as it does on main, and that no regressions have been introduced by this PR.
• Inspect the in_person_deadline_passed email via http://localhost:3000/rails/mailers/user_mailer/in_person_deadline_passed. Confirm it looks the same on this branch as it does on main, and that no regressions have been introduced by this PR.
• Inspect the in_person_verified email via http://localhost:3000/rails/mailers/user_mailer/in_person_verified. Confirm that the displayed location name is no longer the same as the location name displayed in the in_person_ready_to_verify email.
• Inspect the in_person_failed email via http://localhost:3000/rails/mailers/user_mailer/in_person_failed. Confirm that the displayed location name is no longer the same as the location name displayed in the in_person_ready_to_verify email.
• Inspect the in_person_failed_fraud email via http://localhost:3000/rails/mailers/user_mailer/in_person_failed_fraud. Confirm that the displayed location name is no longer the same as the location name displayed in the in_person_ready_to_verify email.

👀 Screenshots

I only included English-language screenshots because we didn't change translations. The change in post office name should apply to the emails in all languages. If design would like, I can also upload screenshots of all supported languages.

in_person_please_call_email Before:

in_person_please_call_email After:

in_person_deadline_passed email Before:

in_person_deadline_passed email After:

in_person_verified email Before:

in_person_verified email After:

in_person_failed email Before:

in_person_failed email After:

in_person_failed_fraud email Before:

in_person_failed_fraud email After:

[eileen-nava]

In the scenario where the user does not attempt to proof at a post office before the deadline passes, there is no visited location name because there is no visited location. Hence, 'none.'

I also contemplated using inheritance and creating a child of VerificationResultsEmailPresenter, DeadlinePassedEmailPresenter, that wouldn't take visited_location_name as an argument for its initialize method. Curious what other engineers think!

[gina-yamada]

I had mixed feeling about this. We are passing in 'none' to send_deadlin_passed_email and then again in the mailer. I thought maybe good to not pass it in- and just set the default to none in the mailer. I think I like this better because we stay consistent with passing in 3 ordered args. I think when we refactor this (usps job), I'd like to see us passing around hashes. I think it is easier when there are 3+ args.) I would not do that here in this PR- that requires a lot of changes so blows up scope here. All this to say- this is fine for me.

[eileen-nava]

I also went back and forth about this internally. I decided to refactor to use keyword arguments, in part because of points made here.

[gina-yamada]

✅ I walked through the usps job. You found and handled all IPP result emails that have the name of the visited PO in the email.
✅ Specs are updated
✅ I can see the visited location name on verified, failed, and fraud emails. There was no regression in the expired email (visited location name does not appear on this email) using the mailer.
✅ I did one sanity check for passed- I wanted to confirm this locally without the mailer. I followed instructions for Viewing email messages. When the email gets delivered- I can see WILKES BARRE displayed- this is the proofing post office in the mocked response for passing proofing results. (When I did this on main it was 'ANYPLACE' which comes from mocked ipp facilities). If you test this out- maybe try for failed or failed fraud

[aduth]

• I think positional arguments only make sense when it's clear from the callsite what the argument's purpose is. I don't think it's particularly clear what a 'none' third argument is meant to do from looking at it in isolation, for example. Refactoring to use keyword arguments would be my preference.
• I see that we pass user as enrollment.user in all cases, but since we're already passing enrollment, it seems unnecessary to have it as an argument if the method could reference the user directly from enrollment.user.
• On a related note about collapsing arguments to use enrollment, I'd wonder if visited_location_name is something we'd want to store on the enrollment, if we're not already?

[gina-yamada]

We save the selected location but not the visited location. I had this same thought but wondered outside of these emails is it needed. Maybe nice in analytics so would make that easier.

[eileen-nava]

• I agree about removing user as an argument method. I will make that change.
• Storing the visited_location_name on the in_person_enrollment in the database is a bigger change. I'd like to hear what the team thinks and will follow up in Joy eng huddle.

[rutvigupta-design]

@eileen-nava this LGTM. As you pointed out, the Post Office name will not be different in other languages so I'm okay approving this with the English screenshots!

[KeithNava]

[non-blocking] just curious when there is no location will the email display 'none' as the post office name? Meaning, would the sentence read something like You successfully verified your identity at the none Post Office? I don't think this is a scenario that would happen realistically but just curious if it could happen.

[eileen-nava]

The email won't display 'none' as the post office name. We don't have 'none' set as a default argument for visited_location_name. I did test out this scenario with the below code change in the user mailer preview

  def in_person_verified
    UserMailer.with(user: user, email_address: email_address_record).in_person_verified(
      enrollment: in_person_enrollment_id_ipp,
      visited_location_name: nil,
    )
  end

and here is what's displayed:

in_person_verified email with a nil visited_location_name: