Interpolate "Login.gov" everywhere (LG-4886)

[zachmargolis]

Before: Sometimes we hardcode "Login.gov" and sometimes we use our APP_NAME constant

Now: Use APP_NAME/%{app_name} everywhere

I have a few notes, will make them inline

[zachmargolis]

Open question: which file should this live in? Maybe we wait for #5496 to land and then just move it inside there

[aduth]

Open question: which file should this live in? Maybe we wait for #5496 to land and then just move it inside there

For me, I think the ideal implementation would be one where we bootstrap this with data attributes and make it available through a context provider, very similar to what we do already with service provider.

[zachmargolis]

gave it a shot: fdf066d

[aduth]

Was this previously escaping the link? 🤔 (noting difference of the _html suffix)

[aduth]

Quick search reveals a couple other suspect ones, maybe out of scope to deal with here:

identity-idp/app/views/user_mailer/account_reset_cancel.html.erb

Line 2 in 5f37004

<%= t('user_mailer.account_reset_cancel.intro', app_name: link_to(APP_NAME, IdentityConfig.store.mailer_domain_name, class: 'gray')) %>

identity-idp/app/views/user_mailer/sps_over_quota_limit.html.erb

Line 2 in 5f37004

<%= t('.info', link: link_to(APP_NAME, root_url)) %>

[zachmargolis]

this one got on my radar because the specs for it broke... will leave the others for the time being

[zachmargolis]

I wanted to spot-check these, so I implemented #5502 and you were right... account_reset_cancel definitely needed a fix 1bbee5e

[zachmargolis]

So for sps_over_quota_limit ... something is very wrong and I'm gonna declare it out of scope of this... none of the translation strings have interpolation values... it's a very blank email:

identity-idp/config/locales/user_mailer/en.yml

Lines 195 to 198 in 0544a36

	sps_over_quota_limit:
	help: ''
	info: There are SPs that have gone over their quota limit.
	subject: Some SPs have reached their quota limit

[aduth]

Admittedly a bit tough to review in great detail, but overall looks good from what I could tell!

Of my comments, only blocker is adding app_name to the other 'idv.messages.sessions.review_message' string.

[aduth]

Seems like it was busted before, but this spec should be passing in the argument too:

identity-idp/spec/views/idv/phone_errors/_warning.html.erb_spec.rb

Lines 30 to 33 in cbe59e4

	expect(rendered).not_to have_link(
	t('idv.troubleshooting.options.contact_support'),
	href: MarketingSite.contact_url,
	)

[aduth]

Optional: Looks like the string doesn't use app_name ?

identity-idp/config/locales/mailer/en.yml

Line 7 in cbe59e4

help: If you need help, visit %{link}

(Kinda wonder how hard it'd be to make a test that verifies that all strings use all the variables we pass to it)

[zachmargolis]

we could monkeypatch i18n.t and compare the given interp arguments with a string scan in tests?

[zachmargolis]

anyways, removed in c601b7c

[zachmargolis]

took a stab at monkeypatching in d9aa159, fixed some errors in 1138ff0, will see what else pops up in CI

[zachmargolis]

ok so that spec was too sensitive, but probably worth returning to later

• the checks for missing i18n caught a bunch but there were some false positives for FormPasswordValidator#strong_password I couldn't sort out
• the checks for extra i18n args were tricky because the i18n interface has a bunch of extra keys (scope, default, count) that get mixed in with the args for translating

[aduth]

Thanks for trying it, I liked the idea!