Skip to content

LG-14308 Add an account verified but not connected template#11203

Closed
jmhooper wants to merge 4 commits intomainfrom
jmhooper-account-verified-but-not-connected-email
Closed

LG-14308 Add an account verified but not connected template#11203
jmhooper wants to merge 4 commits intomainfrom
jmhooper-account-verified-but-not-connected-email

Conversation

@jmhooper
Copy link
Copy Markdown
Contributor

@jmhooper jmhooper commented Sep 5, 2024

This commit adds a new account_verified_but_not_connected email to the user mailer. This email is intended to be sent to users who verify their identity but do not continue to the service provider. The content is intended to give them a link to the service provider so they can sign in there and connect their account.

This commit also includes some tweaks to the existing account_verified email.

This commit adds a new `account_verified_but_not_connected` email to the user mailer. This email is intended to be sent to users who verify their identity but do not continue to the service provider. The content is intended to give them a link to the service provider so they can sign in there and connect their account.

This commit also includes some tweaks to the existing `account_verified` email.

[skip changelog]
@jmhooper jmhooper requested a review from a team September 5, 2024 14:47
@jmhooper
Copy link
Copy Markdown
Contributor Author

jmhooper commented Sep 5, 2024

Here are screenshots of the new template:

localhost_3000_rails_mailers_user_mailer_account_verified_but_not_connected html_locale=en
localhost_3000_rails_mailers_user_mailer_account_verified_but_not_connected html_locale=es
localhost_3000_rails_mailers_user_mailer_account_verified_but_not_connected html_locale=fr
localhost_3000_rails_mailers_user_mailer_account_verified_but_not_connected html_locale=zh

@jmhooper
Copy link
Copy Markdown
Contributor Author

jmhooper commented Sep 5, 2024

Here are screenshots of the tweaked existing template:

localhost_3000_rails_mailers_user_mailer_account_verified html_locale=en
localhost_3000_rails_mailers_user_mailer_account_verified html_locale=es
localhost_3000_rails_mailers_user_mailer_account_verified html_locale=fr
localhost_3000_rails_mailers_user_mailer_account_verified html_locale=zh

@jmhooper
Copy link
Copy Markdown
Contributor Author

jmhooper commented Sep 5, 2024

Apparently I am cursed. This is going to require me to re-generate fonts to add the [ and ] glyphs in the subject line.

@aduth
Copy link
Copy Markdown
Contributor

aduth commented Sep 5, 2024

Apparently I am cursed. This is going to require me to re-generate fonts to add the [ and ] glyphs in the subject line.

I dunno if there's some way we can distinguish, but since the font wouldn't be used for emails anyways, it'd be nice if we could exclude the characters from the check / font build.

… this I plan to take a coffee break and stare into the sun for 15 minutes.
@jmhooper
Copy link
Copy Markdown
Contributor Author

jmhooper commented Sep 5, 2024

Hmm, that is an interesting thought. It seems too easy to use a translation that was originally only intended for emails elsewhere.

@jmhooper
Copy link
Copy Markdown
Contributor Author

jmhooper commented Sep 5, 2024

My best idea would be a separate translation file for email translations and similar to how we do for translations that go in SMS and voice messages.

user_mailer.account_verified_but_not_connected.change_password_link: change your password
user_mailer.account_verified_but_not_connected.contact_link: contact us
user_mailer.account_verified_but_not_connected.help_html: If you did not perform this action, please %{contact_link_html} and sign in to %{change_password_link_html}.
user_mailer.account_verified_but_not_connected.instructions_html: Sign back in at the <strong>%{sp_name}</strong> website to connect your verified information and access services.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

my vote is to move the strong tag into the ERB

Suggested change
user_mailer.account_verified_but_not_connected.instructions_html: Sign back in at the <strong>%{sp_name}</strong> website to connect your verified information and access services.
user_mailer.account_verified_but_not_connected.instructions_html: Sign back in at the %{sp_name_html} website to connect your verified information and access services.

user_mailer.account_verified.intro_html: You successfully verified your identity with %{sp_name} on %{date} using %{app_name}. If you did not perform this action, please %{contact_link_html} and sign in to %{change_password_link_html}.
user_mailer.account_verified.subject: You verified your identity with %{sp_name}.
user_mailer.account_verified.help_html: If you did not perform this action, please %{contact_link_html} and sign in to %{change_password_link_html}.
user_mailer.account_verified.intro_html: On %{date}, you used %{app_name} to:<ul><li>Verify your identity</li><li>Connect your verified information to %{sp_name}</li></ul>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

in the spirit of moving HTML into ERBs, I think it would be worth considering moving each sentence/bullet into its own key, and then leave the HTML markup in the template

@@ -1 +1 @@
!"#$%&'()+,-./0123456789:;>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz~ «»¿ÀÁÈÉÊÎÓÚàáâãçèéêëíîïñóôùúû ‑—‘’“”…‹中体文简
!"#$%&'()+,-./0123456789:;>?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[]_abcdefghijklmnopqrstuvwxyz~ «»¿ÀÁÈÉÊÎÓÚàáâãçèéêëíîïñóôùúû ‑—‘’“”…‹中体文简
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Definitely not germane to your PR directly, but now that we've had to regenerate fonts twice within the past month or so, I'm left wondering how much we're really saving users. Semi-regular users would have had to re-download them on each update. It's probably still a net win, but with a narrowing margin.

(My point is that I think glyphindor or whatnot isn't hugely valuable and could be phased out, not that you should try to work around the fact that the linter made you regenerate fonts here.)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The previous update was due to a bug. Similarly, as commented earlier, I don't think we'd really need to update these fonts here.

I think there's also an interesting discussion to be had whether to question why we're introducing new punctuation that we don't use anywhere else, and whether that's something we should want to avoid.

It's hard to argue in favor of end-user performance vs. developer ergonomics when we personally encounter the latter more often, but I still feel quite favorable to this optimization, and think we can continue to smooth out some of the rough edges.

@jmhooper
Copy link
Copy Markdown
Contributor Author

jmhooper commented Sep 5, 2024

I'm closing this since @artfulaction and I are going back to the drawing board for this approach

@jmhooper jmhooper closed this Sep 5, 2024
@aduth aduth deleted the jmhooper-account-verified-but-not-connected-email branch September 5, 2024 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants