[HOLD for payment 2023-05-04] [HOLD for payment 2023-04-26] [$1000] ‘Link has been resent’ Spanish message is not dynamically translated into English when a user changes langue preference to English

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Open the app on the Sign In screen.
2. Verify if the App's translations are in English.
3. Set your device / browser language to Spanish.
4. Open the app again on the Sign In screen.
5. Verify if the App's translations are in Spanish
6. Enter email address and click on continue button
7. Click on ‘forgot password’ link
8. Click on ‘Resend link’ button
9. Reset your device / browser language to English.
10. Navigate back to the sign-in page and click refresh button

Expected Result:

System should translate all sing-in page content to English

Actual Result:

System translates all sign-in page content to English, except ‘Link has been resent’ Spanish text

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.2.97-2
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

2023-04-08.23.21.00.mp4

Recording.188.mp4

Expensify/Expensify Issue URL:
Issue reported by: @Natnael-Guchima
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1680986148035829

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~010987a0fa7cf27346
• Upwork Job ID: 1645488016025690112
• Last Price Increase: 2023-04-10

[MelvinBot]

Triggered auto assignment to @CortneyOfstad (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[MelvinBot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[CortneyOfstad]

I was able to recreate this (forgot to get a video) so getting eyes on this 👍

[MelvinBot]

Job added to Upwork: https://www.upwork.com/jobs/~010987a0fa7cf27346

[MelvinBot]

Current assignee @CortneyOfstad is eligible for the External assigner, not assigning anyone new.

[MelvinBot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @mollfpr (External)

[MelvinBot]

Triggered auto assignment to @tgolen (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[allroundexperts]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Link has been resent spanish message is not dynamically translated.

What is the root cause of that problem?

The root cause of this issue is that we're saving translated text in Onyx instead of saving the key. This can be seen here.

What changes do you think we should make in order to solve the problem?

We need to save the key of the text instead of the translated version of text. Here, we need to add the following:

message: 'resendValidationForm.linkHasBeenResent',

Then, wherever the message key is being used, we need to use Localize.translateLocal method to convert it to the correct translated text.

What alternative solutions did you explore? (Optional)

None

[Pujan92]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Success message of link has been reset not converted to selected language

What is the root cause of that problem?

For the ResendRequestPasswordReset and RequestAccountValidationLink api call we are storing the translated message in the onyx store.

App/src/libs/actions/Session/index.js

Line 476 in 5bad1bb

message: Localize.translateLocal('resendValidationForm.linkHasBeenResent'),

App/src/libs/actions/Session/index.js

Line 95 in 5bad1bb

message: Localize.translateLocal('resendValidationForm.linkHasBeenResent'),

What changes do you think we should make in order to solve the problem?

We should store the translation key in both the places for onyx success and use props.translate for those keys in the component.

message: 'resendValidationForm.linkHasBeenResent',

App/src/pages/signin/ResendValidationForm.js

Line 76 in 5bad1bb

<DotIndicatorMessage style={[styles.mb5, styles.flex0]} type="success" messages={{0: props.account.message}} />

props.translate(props.account.message)

We also need to do the same for the errors but I am not seeing right now we are setting any errors in case of failure.

Result

Screen.Recording.2023-04-10.at.11.55.43.PM.mov

[mollfpr]

Thank you guys for the proposals!

Both proposals got the point of the issue correct, but @Pujan92's proposal was more accurate about the place that caused the problem, either Session.resendResetPassword() or Session.resendValidationLink().

---

Then, wherever the message key is being used, we need to use Localize.translateLocal method to convert it to the correct translated text.

@allroundexperts your proposal only mentions Session.resendValidationLink() is the place that caused the problem, and the suggesting changes part is unclear where we should update the code to read the key translation.

I know it's high competition to propose a solution, but taking your time to analyze it and having the best proposal with all the information we need to review it is better. I'll be happy to review your proposal in the other issue! 👍

---

@tgolen I like @Pujan92 proposal because it has the complete root cause and a clear place for suggesting the changes.

In addition, we need to update the BaseValidateCodeForm to translate the message key.

App/src/pages/signin/ValidateCodeForm/BaseValidateCodeForm.js

Line 231 in d196a12

{this.props.account.message}

🎀 👀 🎀 C+ reviewed!

[allroundexperts]

Thank you guys for the proposals!

Both proposals got the point of the issue correct, but @Pujan92's proposal was more accurate about the place that caused the problem, either Session.resendResetPassword() or Session.resendValidationLink().

---

Then, wherever the message key is being used, we need to use Localize.translateLocal method to convert it to the correct translated text.

@allroundexperts your proposal only mentions Session.resendValidationLink() is the place that caused the problem, and the suggesting changes part is unclear where we should update the code to read the key translation.

I know it's high competition to propose a solution, but taking your time to analyze it and having the best proposal with all the information we need to review it is better. I'll be happy to review your proposal in the other issue! 👍

---

@tgolen I like @Pujan92 proposal because it has the complete root cause and a clear place for suggesting the changes.

In addition, we need to update the BaseValidateCodeForm to translate the message key.

App/src/pages/signin/ValidateCodeForm/BaseValidateCodeForm.js

Line 231 in d196a12

{this.props.account.message}

🎀 👀 🎀 C+ reviewed!

Thanks for the review. I agree with your decision!

[tgolen]

Sounds good, thanks! @CortneyOfstad 🟢 to hire @Pujan92

[gianni92-carb]

Contributor details Your Expensify account email:giannicarboni@hotmail.it
Upwork Profile Link: https://www.upwork.com/freelancers/~019cd25d87a71f705e

[MelvinBot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[Pujan92]

I am not assigned for this issue yet but based on @tgolen comment/approval I just raised a PR which is ready for review!

[tgolen]

bump @CortneyOfstad to hire and assign @Pujan92

[CortneyOfstad]

Invite sent!

[MelvinBot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[MelvinBot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.6-0 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• translation added for codeRequestedMessage #17895

If no regressions arise, payment will be issued on 2023-05-04. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

[MelvinBot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@mollfpr] The PR that introduced the bug has been identified. Link to the PR:
• [@mollfpr] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@mollfpr] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@mollfpr] Determine if we should create a regression test for this bug.
• [@mollfpr] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@isabelastisser] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[isabelastisser]

Just confirming that we are holding for payment on May 4, correct?

[mollfpr]

@isabelastisser Yes!

[tgolen]

Now that it's May 8, what's the next step here?

[isabelastisser]

I was OOO and the payments and checklist tasks are delayed. I will review this now.

[mollfpr]

[@mollfpr] The PR that introduced the bug has been identified. Link to the PR:

#10556
#10783

[@mollfpr] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:

https://github.com/Expensify/App/pull/10556/files#r1187620912
https://github.com/Expensify/App/pull/10783/files#r1187621034

[@mollfpr] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:

We could probably add a checklist whenever a localized text is saved in Onyx. Make sure that we save the key instead the translated text.

[isabelastisser]

All hired in Upwork:

Issue reported by: @Natnael-Guchima
C+ review: @mollfpr
Assigned Contributor: @Pujan92

[mollfpr]

Regression Test Proposal

1. In magic link/forgot password screen click on the Resend button
2. It shows the success text Link has been re-sent if the current language in english
3. Switch language to spanish
4. Verify the success text translated to spanish

Do we agree 👍 or 👎

[isabelastisser]

@tgolen the contributor was assigned this issue on April 13, and the first PR was merged on April 14. Based on this, the contributors would qualify for a 50% bonus, however, I found this additional PR that was later created to fix a regression issue and only merged on April 25. Can you please clarify if this issue qualifies for a bonus? Thanks!

[priya-zha]

@isabelastisser For the regression issue, I've already been paid by @puneetlath . Reference here. Thanks

[isabelastisser]

@priya-zha I understand, but I still need clarification on this:

#17216 (comment)

[tgolen]

Our guidelines state:

If the PR causes a regression at any point within the regression period (starting when the code is merged and ending 7 days after being deployed to production), contributors are not eligible for the 50% bonus.

So, I do not think they are eligible for a 50% bonus on the original PR.

[Natnael-Guchima]

All hired in Upwork:

Issue reported by: @Natnael-Guchima
C+ review: @mollfpr
Assigned Contributor: @Pujan92

Accepted! Thanks🙏

[isabelastisser]

Thanks for confirming, @tgolen!

I have processed the payments in Upwork and will follow up on the task below now:

[@isabelastisser] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

https://github.com/Expensify/Expensify/issues/282237