[HOLD for payment 2023-10-30] [HOLD for payment 2023-09-29] [$500] Dev: Web - My note not translated in Spanish

kbecciv commented 1 year ago

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:

Go to Settings > Preferences >Language
Change Language to Spanish
Create workspace if not created
Go to #admins
Click on header
Go to Private notes

Expected Result:

My note should be translated in Spanish

Actual Result:

My note not translated in Spanish

Workaround:

Unknown

Platforms:

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

[ ] Android / native
[ ] Android / Chrome
[ ] iOS / native
[ ] iOS / Safari
[x] MacOS / Chrome / Safari
[ ] MacOS / Desktop

Version Number: Dev 1.3.70.5 Reproducible in staging?: n Reproducible in production?: n 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

https://github.com/Expensify/App/assets/93399543/90f18795-c638-4a0f-aebd-904d79d4b485

Expensify/Expensify Issue URL: Issue reported by: @gadhiyamanan Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1694784868918699

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01a01358f997cc9909
Upwork Job ID: 1703115340291780608
Last Price Increase: 2023-09-16
Automatic offers:

namhihi237 | Contributor | 26746126
gadhiyamanan | Reporter | 26746127

namhihi237 commented 1 year ago

Proposal

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

"My note" should be translated when changing language to Spanish

What is the root cause of that problem?

We have some places hard-coding "My note". Therefore, when changing the language to Spanish, they will not be translated. https://github.com/Expensify/App/blob/76c1559fb3fbee1d07c8f210b522b63edd03920e/src/pages/PrivateNotes/PrivateNotesEditPage.js#L90

https://github.com/Expensify/App/blob/76c1559fb3fbee1d07c8f210b522b63edd03920e/src/pages/PrivateNotes/PrivateNotesListPage.js#L110

https://github.com/Expensify/App/blob/76c1559fb3fbee1d07c8f210b522b63edd03920e/src/pages/PrivateNotes/PrivateNotesViewPage.js#L67

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

We should make use of translate() and create translations for them.

translate('privateNotes.myNote')

What alternative solutions did you explore? (Optional)

melvin-bot[bot] commented 1 year ago

Job added to Upwork: https://www.upwork.com/jobs/~01a01358f997cc9909

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

neonbhai commented 1 year ago

Proposal

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

My note not translated in spanish

What is the root cause of that problem?

We are missing Localization logic for PrivateNotesFeature in PrivateNotesEditPage.js, PrivateNotesListPage.js and PrivateNotesViewPage.js

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

The messages in this feature should refer to en.ts and spanish translation to es.ts

Then we will be using the useLocalize() hook

const {translate} = useLocalize();

to add Localization feature:

subtitle={translate('privateNotes.myNote')}

title: Number(lodashGet(session, 'accountID', null)) === Number(accountID) ? translate('privateNotes.myNote') : lodashGet(personalDetailsList, [accountID, 'login'],

subtitle={isCurrentUserNote ? translate('privateNotes.myNote') : `${lodashGet(personalDetailsList, [route.params.accountID, 'login'], '')} note`}

What alternative solutions did you explore? (Optional)

allroundexperts commented 1 year ago

@neonbhai How is your proposal different from what @namhihi237 proposed earlier?

neonbhai commented 1 year ago

Hi @allroundexperts, added clear implementation details and line changes.

allroundexperts commented 1 year ago

@neonbhai Adding implementation details with a duplicated approach won't lead to your proposal getting accepted. One can write pseudo code only as well as long as the approach is clear enough. I would suggest you to write proposals that are different from the already existing ones.

allroundexperts commented 1 year ago

@namhihi237's proposal looks good to me!

🎀 👀 🎀 C+ reviewed

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

📣 @allroundexperts Please request via NewDot manual requests for the Reviewer role ($500)

melvin-bot[bot] commented 1 year ago

📣 @namhihi237 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

melvin-bot[bot] commented 1 year ago

📣 @gadhiyamanan 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

melvin-bot[bot] commented 1 year ago

🎯 ⚡️ Woah @allroundexperts / @namhihi237, great job pushing this forwards! ⚡️

The pull request got merged within 3 working days of assignment, so this job is eligible for a 50% #urgency bonus 🎉

when @namhihi237 got assigned: 2023-09-19 04:11:43 Z
when the PR got merged: 2023-09-19 23:26:10 UTC

On to the next one 🚀

melvin-bot[bot] commented 12 months ago

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

melvin-bot[bot] commented 12 months ago

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

https://github.com/Expensify/App/pull/27761

If no regressions arise, payment will be issued on 2023-09-29. :confetti_ball:

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

For reference, here are some details about the assignees on this issue:

@allroundexperts does not require payment (Eligible for Manual Requests)
@namhihi237 requires payment offer (Contributor)
@gadhiyamanan requires payment offer (Reporter)

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

melvin-bot[bot] commented 11 months ago

@tgolen, @alexpensify, @allroundexperts, @namhihi237 Whoops! This issue is 2 days overdue. Let's get this updated quick!

tgolen commented 11 months ago

@alexpensify Can you issue payment for this, please?

alexpensify commented 11 months ago

Here is the payment summary:

External issue reporter: @gadhiyamanan $50
Contributor that fixed the issue: @namhihi237 $750
Contributor+ that helped on the issue and/or PR: @allroundexperts $750

Upwork Job: https://www.upwork.com/jobs/~01a01358f997cc9909

*If applicable, the bonuses will be applied on the final payment

Extra Notes regarding payment: There is an urgency bonus here and will be applied during the payment process.

alexpensify commented 11 months ago

@allroundexperts please complete the checklist and request the payment.

Right now, everyone who is paid in Upwork has been paid there and I closed the job.

allroundexperts commented 11 months ago

Checklist

We forgot to add Spanish translation for this piece of text when we implemented this issue in this PR.
https://github.com/Expensify/App/pull/25761/files#r1343268376
Slack discussion not needed as we already have translation related points in our check list.
A regression test would be helpful.

Regression test steps

Login and change the language to Spanish.
Create workspace if not created and Go to #admins room
Click on header and go to Private notes

Verify that My note is seen as translated into Spanish as Mi notas

Do we 👍 or 👎 ?

alexpensify commented 11 months ago

Closing - the regression test request has been created for this case. I'm going to close this GH.

JmillsExpensify commented 11 months ago

$750 payment approved for @allroundexperts based on BZ summary.

isagoico commented 11 months ago

Reopening this issue because the current translation in app is not correct. It's showing "Mi notas" which is incorrect in Spanish.

It should either be:

For plural: My notes = Mis notas
For singular: My note = Mi nota

tgolen commented 11 months ago

Thank you for spotting this, Isa! @namhihi237 Can you please fix that?

I looked and I can't find a record of either me nor @allroundexperts checking to make sure that was an official translation. That's something we can learn from and do better at next time.

namhihi237 commented 11 months ago

Sure I will raise PR next hours.

namhihi237 commented 11 months ago

@tgolen PR already for review

melvin-bot[bot] commented 11 months ago

Based on my calculations, the pull request did not get merged within 3 working days of assignment. Please, check out my computations here:

when @namhihi237 got assigned: 2023-09-19 04:11:43 Z
when the PR got merged: 2023-10-19 21:58:35 UTC
days elapsed: 22

On to the next one 🚀

melvin-bot[bot] commented 11 months ago

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

melvin-bot[bot] commented 11 months ago

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

https://github.com/Expensify/App/pull/29980

If no regressions arise, payment will be issued on 2023-10-30. :confetti_ball:

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

For reference, here are some details about the assignees on this issue:

@allroundexperts does not require payment (Eligible for Manual Requests)
@namhihi237 requires payment offer (Contributor)
@gadhiyamanan requires payment offer (Reporter)

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

alexpensify commented 10 months ago

@allroundexperts and @gadhiyamanan - I'm considering closing with no payment updates. This is not a regression but was a quick fix. Everyone has already paid out already with bonuses in September. Let me know if you disagree.

cc: @tgolen

allroundexperts commented 10 months ago

Absolutely @alexpensify. That was my expectation as well.

alexpensify commented 10 months ago

Thanks! Closing for now.

Expensify / App