-
Notifications
You must be signed in to change notification settings - Fork 213
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.
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
feat(emails): Add inactiveAccountFirstWarning email template + partials #18202
base: main
Are you sure you want to change the base?
Conversation
packages/fxa-auth-server/lib/senders/emails/templates/inactiveAccountFirstWarning/index.mjml
Show resolved
Hide resolved
packages/fxa-auth-server/lib/senders/emails/templates/inactiveAccountFirstWarning/index.mjml
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vpomerleau Thank you! On a side note, this email is pretty much identical to https://mozilla-hub.atlassian.net/browse/FXA-10637 minus subject, title and extra note in description . For what it is worth the duplication seems alright to me. I could extact to more partials? What do you think?
...es/fxa-auth-server/lib/senders/emails/templates/inactiveAccountFirstWarning/index.stories.ts
Show resolved
Hide resolved
I'd agree that the duplication seems fine to me, similar to our verification reminder emails. And I assume we'll have separate handlers for each so having 1:1 between handlers and templates seems fine. Keeping them separate makes it easier to tweak the copy of one of the emails if we need to in the future. |
@vbudhram I think it makes sense to document partials when they can be displayed in various ways, like the |
import { storyWithProps } from '../../storybook-email'; | ||
|
||
export default { | ||
title: 'Partials/automatedEmail/automatedEmailChangePassword', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we put this under Partials/footers/*
? I think it'll make it a lot more obvious for Product etc. when they're looking at storybook, that this is going to show our footers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure thing.
I think @vbudhram was asking if he should extract more content of these inactive account emails into partials (vs questioning if we should add storybooks for the partials :))
@@ -0,0 +1,4 @@ | |||
account-deletion-info-block-communications = If your account is deleted, you’ll still receive emails from { -brand-mozilla-corporation } and { -brand-mozilla-foundation }, unless you <a data-l10n-name="unsubscribeLink">ask to unsubscribe</a>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of creating terms here, let's hardcode them. This is what we do elsewhere like Firefox since using them are so uncommon.
Also, in these cases we don't want to use the existing Mozilla brand term either. (e.g. Mozilla Corporation and Mozilla Foundation
not { -brand-mozilla } Corporation
and { -brand-mozilla } Foundation`)
Apart from those reasons, the main reason we probably don't want to use terms is because of issues with fallbacking. If a locale weren't able to translate these emails in time before sending, I'd assume they'd fall back to English. However, terms / strings referenced by a placeholder won't fallback if the referenced term doesn't exist, instead it would just show the placeholder and { -brand-mozilla-corporation }
would appear as-is.
@@ -0,0 +1 @@ | |||
automated-email-inactive-account = This is an automated email. You are receiving it because you have a Mozilla account and it has been 2 years since your last sign-in. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use the -product-mozilla-account
term here however. (Unless there's a technical reason not to.)
@@ -0,0 +1,10 @@ | |||
inactiveAccountFirstWarning-subject = Remember to confirm your account | |||
inactiveAccountFirstWarning-title = Do you want to keep your { -brand-mozilla } account and data? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inactiveAccountFirstWarning-title = Do you want to keep your { -brand-mozilla } account and data? | |
inactiveAccountFirstWarning-title = Do you want to keep your { -product-mozilla-account } and data? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From my understanding Mozilla account
does mean an individual's account so should we use -product-mozilla-account
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one I wasn't sure about since Mozilla seemed to apply to both the account and the data, but maybe that's fine and localizers can work around it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I too was unsure. Yeah maybe what you have is better in terms of providing localizers the flexibility.
@@ -0,0 +1,10 @@ | |||
inactiveAccountFirstWarning-subject = Remember to confirm your account | |||
inactiveAccountFirstWarning-title = Do you want to keep your { -brand-mozilla } account and data? | |||
inactiveAccountFirstWarning-account-description = Your { -product-mozilla-account } is used to access free privacy and browsing products like Firefox Sync, Monitor, Relay, and MDN.</span> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use the brand terms that exist for Relay, MDN, etc.
Also, "Firefox Sync" we don't treat as a product anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll ask for content review on this in Figma. Our product terms are Mozilla Monitor and Firefox Relay, so I want to confirm what we should do in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just an update that content review is in progress for how to reference 'sync' in this sentence, and the other products will be updated to full names (Mozilla Monitor, Firefox Relay) so we can use the existing brand terms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because: * We will be sending out warnings before deleting inactive accounts This commit: * Adds new email template for inactiveAccountFirstWarning including l10n and storybook * Adds new partials for use in inactive account emails * Adds stories for automated email partials Closes #FXA-10636
Because
This pull request
Issue that this pull request solves
Closes: #FXA-10636
Checklist
Put an
x
in the boxes that applyScreenshots (Optional)
Please attach the screenshots of the changes made in case of change in user interface.
Other information (Optional)
Any other information that is important to this pull request.