LG-11666 Add Post Office search link to barcode page and emails

[jennyverdeyen]

🎫 Ticket

Link to the relevant ticket:
LG-11666

🛠 Summary of changes

Updates Ready to Verify barcode page and email/reminder email:

• Removes the text "You can visit any Participating Post Office location." from the body text under the "Post Office information" heading
• For EIPP only, adds a section with the heading "Need to change your Post Office location?" and add body text under it: "You don’t need to create a new barcode, you can bring this barcode to any participating Post Office location. Find other participating Post Office locations."
• Also refactored the specs for the show.html.erb content to focus on sections of content as contexts, for readability and consistency.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

• Enter through Sinatra and choose identity-verified (not Enhanced)
• Go through the flow to create an enrollment
• At the barcode page, observe that the "You can visit any Participating Post Office location" body text is removed, under the H2 "Post Office Information"
• Observe the added H3 "Need to change your Post Office location?" and the added body text under it: "You don’t need to create a new barcode, you can bring this barcode to any participating Post Office location. Find other participating Post Office locations."
• Verify that the link to "Find other participating Post Office locations" brings you to the help center PO search page.
• Verify that translations are present and consistent with the translations doc

Verify that the new H3 and body text are not added in the EIPP flow:

• Enter through Sinatra and choose Enhanced In-Person Proofing
• Go through the flow to create an enrollment
• At the barcode page, observe that the "You can visit any Participating Post Office location" body text is removed, under the H2 "Post Office Information"
• Observe the H3 "Need to change your Post Office location?" and any body text underneath are not added.

Verify the emails:

• Verify that the same content changes are present in the Ready to Verify email
• Verify that the same content changes are present in the Ready to Verify Reminder email

👀 Screenshots

Ready to Verify Barcode Page (ID-IPP):

Ready to Verify Barcode Page (EIPP):

e04b70">

Ready to Verify Email (ID-IPP):

Reminder Email (ID-IPP):

Read to Verify email (EIPP):

Reminder email (EIPP):

[aduth]

Instead of html_safe, I suggest using _html suffix convention for translated strings:

https://guides.rubyonrails.org/i18n.html#using-safe-html-translations

[jennyverdeyen]

Thanks!! Will make this update.

[aduth]

Should this be using the assigned let variable assignment, or at the very least have a matching before in "when enabled" block below? Assigning with the variable would let us define it once and have the context variable change for each spec.

Suggested change

	context 'when Enhanced IPP is not enabled' do
	let(:is_enhanced_ipp) { false }
	before do
	@is_enhanced_ipp = false
	end
	before do
	@is_enhanced_ipp = is_enhanced_ipp
	end
	context 'when Enhanced IPP is not enabled' do
	let(:is_enhanced_ipp) { false }

[jennyverdeyen]

Good catch! These new tests were actually not functioning properly at all, thanks for highlighting them.

[kellular]

I just had one comment about the Chinese translations, otherwise looks good from design.

[kellular]

Nit: We're missing a "。" at the end of the sentence. So the end of the sentence should look like:

... 点。

[eileen-nava]

I started to look at this, but didn't have time to do a full review. I can come back to it tomorrow. Good stuff!

[eileen-nava]

Ruby style guidelines vote for unless over if !, but I know unless is controversial. It be would nice if we could change this to if @is_id_ipp, but that would require introducing a new instance variable. I don't think the change is worth that.

What are your thoughts on if ! versus unless?

[jennyverdeyen]

Ooooh I was not hip to the unless hatred!

kill unless with fire

😱

I agree if @is_id_ipp would be the most readable but also agree it isn't particularly worth the squeeze here. I don't have strong feelings about unless vs if ! but I stuck with if ! here because it was being used that way earlier in the same file. I kind of like the idiomatic feel of unless but now I wonder if I should avoid it in the future so as not to upset reviewers 😛

[eileen-nava]

Eh. I slightly prefer unless because it's in the Ruby style guidelines, but I also am not going to die on this hill. I also found the PR that introduced the if ! syntax into this template and found that I approved it. 🙃 I guess my past self didn't feel strongly, either!

[eileen-nava]

The comment I left on L232 of app/views/idv/in_person/ready_to_verify/show.html.erb is relevant here, too.

[eileen-nava]

Approved. I left nonblocking feedback on test setup.

[eileen-nava]

I appreciate how readable these specs are.

[eileen-nava]

Non-blocking nit request: While you're in this file, could you please change line 819 to specify that it's Informed Delivery IPP (ID-IPP)?

[eileen-nava]

Non-blocking: I wonder if Andrew's comment on the user mailer spec also applies here?

[eileen-nava]

Non-blocking: Again, I wonder if Andrew's comment on the user mailer spec also applies to this set-up?

[eileen-nava]

Non-blocking: taps sign

[voidlily]

if you're still using this reviewapp (unlikely since the PR for this was merged), i cleaned it up manually since it was causing some problems in the reviewapps cluster