-
Notifications
You must be signed in to change notification settings - Fork 214
fix(auth): Update SubscriptionUpgrade and SubscriptionSubsequentInvoice emails to handle credits #18851
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
Conversation
041c78c
to
796f28b
Compare
subscriptionUpgrade-content-charge-prorated = | ||
You have been charged a one-time fee of { $paymentProrated } to reflect your subscription’s higher price for the remainder of { $productPaymentCycleOldPronoun -> | ||
[masculine] this | ||
[feminine] this | ||
*[other] this | ||
} { $productPaymentCycleOld -> | ||
[day] day | ||
[month] month | ||
[6-month] 6-month | ||
[year] year | ||
*[other] billing period | ||
}. |
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.
There's no way for Fluent to determine whether something is masculine/feminine in a certain language (and we can't feed that variable from the backend as different languages would have different values) so this approach won't work.
Also only a-zA-Z characters are valid for non-plural variants so [6-month]
causes a Fluent error.
We could potentially consider something like:
subscriptionUpgrade-content-charge-prorated = | |
You have been charged a one-time fee of { $paymentProrated } to reflect your subscription’s higher price for the remainder of { $productPaymentCycleOldPronoun -> | |
[masculine] this | |
[feminine] this | |
*[other] this | |
} { $productPaymentCycleOld -> | |
[day] day | |
[month] month | |
[6-month] 6-month | |
[year] year | |
*[other] billing period | |
}. | |
subscriptionUpgrade-content-charge-prorated = | |
{ $productPaymentCycleOld -> | |
[day] You have been charged a one-time fee of { $paymentProrated } to reflect your subscription’s higher price for the remainder of this day. | |
[month] You have been charged a one-time fee of { $paymentProrated } to reflect your subscription’s higher price for the remainder of this month. | |
[sixMonth] You have been charged a one-time fee of { $paymentProrated } to reflect your subscription’s higher price for the remainder of this 6-month period. | |
[year] You have been charged a one-time fee of { $paymentProrated } to reflect your subscription’s higher price for the remainder of this year. | |
*[other] You have been charged a one-time fee of { $paymentProrated } to reflect your subscription’s higher price for the remainder of this billing period. | |
} |
But it might be simpler to make each of these separate strings.
*[other] billing period | ||
}. | ||
subscriptionUpgrade-content-charge-credit = You have received an account credit in the amount of { $paymentProrated }. | ||
subscriptionUpgrade-content-starting = |
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 also has an issue with [6-month]
causing a Fluent error.
Other than that, this is technically valid Fluent. I'm not sure if we've used a construction like this before so I'm checking with my team to see if Pontoon can handle a string like this.
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.
Checked with the team. Pontoon can not in fact handle this anymore (this was changed a couple years ago to only to be able handle full sentence variations).
So this will need to be adjusted.
We could potentially change this to two strings (if we want to include an unspecified billing period
option):
subscriptionUpgrade-content-starting = Starting with your next bill, your charge will change from { $paymentAmountOld } per billing period ({ $productPaymentCycleOld }) to { $paymentAmountNew } per billing period ({ $productPaymentCycleNew}).
subscriptionUpgrade-content-starting-no-period = Starting with your next bill, your charge will change from { $paymentAmountOld } to { $paymentAmountNew } per billing period.
However, we'd also need to create strings for day / month / ... etc. so we could fill in the placeables with localized text instead of the raw English string which is what we are doing now.
subscriptionUpgrade-content-charge-prorated = | ||
{ $productPaymentCycleOld -> | ||
[day] You have been charged a one-time fee of { $paymentProrated } to reflect your subscription’s higher price for the remainder of this day. | ||
[month] You have been charged a one-time fee of { $paymentProrated } to reflect your subscription’s higher price for the remainder of this month. | ||
[sixMonth] You have been charged a one-time fee of { $paymentProrated } to reflect your subscription’s higher price for the remainder of this 6-month period. | ||
[year] You have been charged a one-time fee of { $paymentProrated } to reflect your subscription’s higher price for the remainder of this year. | ||
*[other] You have been charged a one-time fee of { $paymentProrated } to reflect your subscription’s higher price for the remainder of this billing period. | ||
} |
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.
After checking with some other l10n members, I've now learned this approach won't work.
We can't guarantee that all variants would be translated/exist for locales (it's possible some could be missed/ignored but still be considered valid by Pontoon the system, it would just end up showing the wrong text).
Would it be possible to split these into separate strings and have the back end select the appropriate one?
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 appreciate the insight, Bryan! As this pull request is for a ticket that was only scoped as a quick patch for credit amounts, I'll file a new ticket to improve localization for billing period intervals - as well as localizing prices, date formats, etc in other subscription emails, which also all include backend revisions.
Ticket filed - FXA-11662
*[other] billing period | ||
}. | ||
subscriptionUpgrade-content-charge-credit = You have received an account credit in the amount of { $paymentProrated }. | ||
subscriptionUpgrade-content-starting = |
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.
Checked with the team. Pontoon can not in fact handle this anymore (this was changed a couple years ago to only to be able handle full sentence variations).
So this will need to be adjusted.
We could potentially change this to two strings (if we want to include an unspecified billing period
option):
subscriptionUpgrade-content-starting = Starting with your next bill, your charge will change from { $paymentAmountOld } per billing period ({ $productPaymentCycleOld }) to { $paymentAmountNew } per billing period ({ $productPaymentCycleNew}).
subscriptionUpgrade-content-starting-no-period = Starting with your next bill, your charge will change from { $paymentAmountOld } to { $paymentAmountNew } per billing period.
However, we'd also need to create strings for day / month / ... etc. so we could fill in the placeables with localized text instead of the raw English string which is what we are doing now.
01b5fc3
to
29d49d8
Compare
## $productPaymentCycleNew (String) - The interval of time from the end of one payment statement date to the next payment statement date of the new subscription, e.g. month | ||
## $productPaymentCycleOld (String) - The interval of time from the end of one payment statement date to the next payment statement date of the old subscription, e.g. month | ||
## $paymentProrated (String) - The one time fee to reflect the higher charge for the remainder of the payment cycle, including currency, e.g. $10.00 | ||
subscriptionUpgrade-content-charge-prorated = You have been charged a one-time fee of { $paymentProrated } to reflect your subscription’s higher price for the remainder of this { $productPaymentCycleOld }. |
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.
subscriptionUpgrade-content-charge-prorated = You have been charged a one-time fee of { $paymentProrated } to reflect your subscription’s higher price for the remainder of this { $productPaymentCycleOld }. | |
subscriptionUpgrade-content-charge-prorated = You have been charged a one-time fee of { $paymentProrated } to reflect your subscription’s higher price for the remainder of this billing period ({ $productPaymentCycleOld }). |
Ok, then let's at least adjust this string so that it avoids the issue with gendered articles.
…ce emails to handle credits - updates the subscriptionUpgrade and subscriptionSubsequentInvoice email templates to handle credit amounts Closes: FXA-11636
This pull request
Issue that this pull request solves
Closes: FXA-11636
Checklist
Put an
x
in the boxes that applyScreenshots (Optional)
Credit - Upgrade from "123Done Pro monthly - $5/mo" to "123Foxkeh monthly $1/mo"
SubscriptionUpgrade

SubscriptionSubsequentInvoice

Prorated - Upgrade from "123Done Pro monthly - $5/mo" to "123Foxkeh monthly $35/mo"
SubscriptionUpgrade

SubscriptionSubsequentInvoice

Zero - Upgrade from "123Done Pro monthly - $5/mo" to "123Foxkeh monthly $5/mo"
SubscriptionUpgrade

SubscriptionSubsequentInvoice
