[$250] Language - Message of added or invited user to "#admins" chat, doesn't get translated

izarutskaya commented 1 month 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!

Version Number: 9.0.17-0 Reproducible in staging?: Y Reproducible in production?: Y Email or phone of affected tester (no customers): bellicoignacio+30@hotmail.com Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: Applause-Internal team

Action Performed:

Open the https://staging.new.expensify.com/ website.
Tap on the avatar on the bottom of the screen.
Tap on "Preferences"
Tap on "Language"
Tap on "Spanish"
Tap on the arrow on the top left corner.
Tap on the chats icon on the bottom of the screen.
Check in the "#admins" chat preview, the language in which the invitation message is displayed.
Open the "#admins" chat.
Check the "New user added" message and the language in which is displayed.

Expected Result:

When choosing Spanish as language, all the sections and chats information should be changed to that language.

Things that need to be changed: Invited 1 user to Chat Report

We already have a translation of the invited user to english into spanish but that doesn't match the screenshot (no number) so seems like we might have this phrase somewhere else.

[ ] update the LHN to use this English copy. Tbh neither I nor Ioni cannot find the copy in the screenshot in the database, maybe it was already updated?
[ ] update English copy to invited member ${to}
[ ] update Spanish copy to miembro invitado ${to}

added <email> as a user

[ ] English should be updated to added <email> as a member
[ ] Spanish translation should be set to agregó a <email> como miembro

Actual Result:

When choosing Spanish as language, the "#admins" chat, still displays information in English. For example, the message "Invited 1 user to chat report", in the preview of the chat, is not translated as "Invitó a un usuario al chat." And when opening the chat, the message "Added ignaciodbellico@gmail.com as a user" is also missing translation. The correct translation of this last sentence, would be "Agregó a ignaciodbellico@gmail.com como usuario."

Workaround:

Unknown

Platforms:

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

[ ] Android: Native
[x] Android: mWeb Chrome
[ ] iOS: Native
[x] iOS: mWeb Safari
[ ] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos

Bug6554265_1722044595083!34c88d80-f5b7-4c49-97d9-d846bf71d22b

https://github.com/user-attachments/assets/76544b0d-f8c0-4cc8-9fcb-ffbe811c46c2

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~010023a595aa275ac1
Upwork Job ID: 1823685163547735883
Last Price Increase: 2024-09-05
Automatic offers:

ZhenjaHorbach | Reviewer | 103526301
nkdengineer | Contributor | 103526302

Issue Owner

Current Issue Owner: @ZhenjaHorbach

melvin-bot[bot] commented 1 month ago

Triggered auto assignment to @sonialiap (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

izarutskaya commented 1 month ago

We think this issue might be related to the #vip-vsb

nkdengineer commented 1 month ago

I don't think it's a bug because almost policy change log actions aren't translated.

sonialiap commented 1 month ago

@Expensify/design is this a feature or a bug?

shawnborton commented 1 month ago

Hmm @dylanexpensify is your system message project covering system message translations? Otherwise I'm not too sure what the current expected behavior is for this.

dylanexpensify commented 1 month ago

Ahh interesting, technically it was just for silencing them, but we did do some copy updates, and I wonder if maybe we missed this. @deetergp any ideas?

melvin-bot[bot] commented 1 month ago

@sonialiap Huh... This is 4 days overdue. Who can take care of this?

sonialiap commented 1 month ago

@deetergp bumping Dylan's question. We're wondering if this is expected or a bug

deetergp commented 1 month ago

That is not one of the messages we made translatable as a part of the Enhanced System Messages project, so this is not a bug, it is new functionality. For future reference, these were the actions we localized for the project: https://github.com/Expensify/App/blob/11af67b62a0a1ec905d7181ac82a183f1b03bd3b/src/languages/en.ts#L3770-L3799

sonialiap commented 1 month ago

Thanks Scott!

Ok, so this is not a "bug" but may be a new feature request. Asking in slack if we want to do something about this

sonialiap commented 1 month ago

Things that need to be changed:

English: change "user" to "member" in added <email> as a member
Spanish: add a translation of added <email> as a member as agregó a <email> como miembro
Spanish: add a translation of Invited 1 member to <chat name> as Invitó a 1 miembro al <chat name>

Confirmed translations in slack ✔️

nkdengineer commented 1 month ago

Edited by proposal-police: This proposal was edited at 2023-10-11 11:18:00.

Proposal

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

When choosing Spanish as language, the "#admins" chat, still displays information in English. For example, the message "Invited 1 user to chat report", in the preview of the chat, is not translated as "Invitó a un usuario al chat." And when opening the chat, the message "Added ignaciodbellico@gmail.com as a user" is also missing translation. The correct translation of this last sentence, would be "Agregó a ignaciodbellico@gmail.com como usuario."

What is the root cause of that problem?

This is a new feature request

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

For invited user to, we need to add a case for this action here then the message can be translated in LHN

else if (ReportActionsUtils.isOldDotReportAction(lastAction)) { 
     result.alternateText = ReportActionsUtils.getMessageOfOldDotReportAction(action);
}

https://github.com/Expensify/App/blob/f2dad3a524b408f45dd74ea2c49b2c70015b2f38/src/libs/SidebarUtils.ts#L427

For add employee action: we need to add a new translation key for this action as here and

We should have a util to get the translated message for POLICYCHANGELOG_ADD_EMPLOYEE action.

function isPolicyChangeLogAddEmployeeMessage(reportAction: OnyxInputOrEntry<ReportAction>): reportAction is ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.ADD_EMPLOYEE> {
    return isActionOfType(reportAction, CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.ADD_EMPLOYEE);
}

function getPolicyChangeLogAddEmployeeMessage(reportAction: OnyxInputOrEntry<ReportAction>): string {
    if (!isPolicyChangeLogAddEmployeeMessage(reportAction)) {
        return '';
    }

    const {email, role, name} = getOriginalMessage(reportAction);
    return Localize.translateLocal('report.actions.type.addEmployee', {email, name, role});
}

Then we will add a case in ReportActionItem to display the translated message for POLICYCHANGELOG_ADD_EMPLOYEE action

else if (action.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.ADD_EMPLOYEE) {
    children = <ReportActionItemBasicMessage message={ReportActionsUtils.getPolicyChangeLogAddEmployeeMessage(action)} />;
}

https://github.com/Expensify/App/blob/55f67f53ca8ed8596943ce930ec10a3bd6f13427/src/pages/home/report/ReportActionItem.tsx#L654-L655

Add a case in SidebarUtils to display the translated message in LHN

else if (lastAction?.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.ADD_EMPLOYEE) { 
     result.alternateText = ReportActionsUtils.getPolicyChangeLogAddEmployeeMessage(action);
}

https://github.com/Expensify/App/blob/55f67f53ca8ed8596943ce930ec10a3bd6f13427/src/libs/SidebarUtils.ts#L427-L428

Add a case in ContextMenuActions to copy the translated message to clipboard

else if (reportAction?.actionName === CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.ADD_EMPLOYEE) { 
     setClipboardMessage(ReportActionsUtils.getPolicyChangeLogAddEmployeeMessage(reportAction));
}

https://github.com/Expensify/App/blob/55f67f53ca8ed8596943ce930ec10a3bd6f13427/src/pages/home/report/ContextMenu/ContextMenuActions.tsx#L460-L461

We can do the same way for other system actions that we need

OPTIONAL: If the add employee is an old dot action, we can add this as a case in getMessageOfOldDotReportAction and isOldDotReportAction then we don't need to create a case in ReportActionItem, SidebarUtils and ContextMenuActions.

What alternative solutions did you explore? (Optional)

sonialiap commented 1 month ago

`Invited 1 user to Chat Report`

We already have a translation of the invited user to english into spanish but that doesn't match the screenshot (no number) so seems like we might have this phrase somewhere else.

[ ] update the LHN to use this English copy. Tbh neither I nor Ioni cannot find the copy in the screenshot in the database, maybe it was already updated?
[ ] update English copy to invited member ${to}
[ ] update Spanish copy to miembro invitado ${to}

`added <email> as a user`

[ ] English should be updated to added <email> as a member
[ ] Spanish translation should be set to agregó a <email> como miembro

melvin-bot[bot] commented 1 month ago

Job added to Upwork: https://www.upwork.com/jobs/~010023a595aa275ac1

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 1 month ago

Upwork job price has been updated to $125

sonialiap commented 1 month ago

Decreasing price since this is a simple copy change, making sure a translation exists, and a little bit of searching to confirm where the old translation is coming from. Details here

ZhenjaHorbach commented 1 month ago

@nkdengineer Thanks for your proposal Could you check this message and update your proposal ?

nkdengineer commented 1 month ago

@ZhenjaHorbach I updated my proposal.

ZhenjaHorbach commented 1 month ago

@nkdengineer Thanks ! This proposal looks good to me !

🎀👀🎀 C+ reviewed

melvin-bot[bot] commented 1 month ago

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

melvin-bot[bot] commented 4 weeks ago

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

Offer link Upwork job

melvin-bot[bot] commented 4 weeks ago

📣 @nkdengineer 🎉 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 📖

ZhenjaHorbach commented 4 weeks ago

@sonialiap

In fact, we have more issues related to translation At the moment I have already found 2 new issues (update role and remove participant ) I think we can cover it in one PR

sonialiap commented 4 weeks ago

@ZhenjaHorbach sounds good 👍

nkdengineer commented 4 weeks ago

@sonialiap please give me translations for 2 cases above

sonialiap commented 4 weeks ago

@nkdengineer English - updated the role of <email> from "<role>" to "<role>" Spanish - actualicé el rol de "<role>" a "<role>"

English - removed <role> <email> Spanish - eliminado <role> <email>

user - should be changed to member in English. miembro in Spanish admin - administrador in Spanish

melvin-bot[bot] commented 4 weeks ago

Triggered auto assignment to @zanyrenney (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

sonialiap commented 4 weeks ago

@zanyrenney I'm OOO Aug 19-30, adding leave buddy. Status: waiting for build

melvin-bot[bot] commented 2 weeks ago

@sonialiap, @srikarparsi, @zanyrenney, @ZhenjaHorbach, @nkdengineer Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

melvin-bot[bot] commented 2 weeks ago

@sonialiap, @srikarparsi, @zanyrenney, @ZhenjaHorbach, @nkdengineer Still overdue 6 days?! Let's take care of this!

ZhenjaHorbach commented 2 weeks ago

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:

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

New feature (Part of the Enhanced System Messages project)

[x] [@ZhenjaHorbach] 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:

NA

[x] [@ZhenjaHorbach] 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:

NA

[x] [@ZhenjaHorbach] Determine if we should create a regression test for this bug.

[x] [@ZhenjaHorbach] 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.

Regression Test Proposal

Open App
Tap on the Settings on the bottom of the screen.
Tap on "Preferences", open "Language" and choose "Spanish"
Tap on the arrow on the top left corner.
Tap on the chats icon on the bottom of the screen.
Check in the "#admins" chat preview, the language in which the invitation message is displayed.
Open the "#admins" chat.
Check the "New user added" message and the language in which is displayed.
Verify that system messages are translated into Spanish

Do we agree 👍 or 👎

nkdengineer commented 2 weeks ago

Decreasing price since this is a simple copy change, making sure a translation exists, and a little bit of searching to confirm where the old translation is coming from. Details https://github.com/Expensify/App/issues/46960#issuecomment-2288509737

@zanyrenney This issue not only did a simple change translation but also updated copy case, LHN case, and side bar util case and fixed for some other messages. See the PR for more details. Can you please update the price to the default price, thanks.

cc @sonialiap

ZhenjaHorbach commented 2 weeks ago

Decreasing price since this is a simple copy change, making sure a translation exists, and a little bit of searching to confirm where the old translation is coming from. Details #46960 (comment)

@zanyrenney This issue not only did a simple change translation but also updated copy case, LHN case, and side bar util case and fixed for some other messages. See the PR for more details. Can you please update the price to the default price, thanks.

cc @sonialiap

I just wanted to write about this 😅 I agree with this Since we covered several additional cases

zanyrenney commented 1 week ago

I'm unassigning as Sonia came back a few days ago, so you shouldn't need my help here! She has more context on the payment being changed so will leave that convo to her! :)

melvin-bot[bot] commented 1 week ago

Upwork job price has been updated to $250

sonialiap commented 1 week ago

Hi team, I agree that it makes sense to update the price to the default since this issue ended up growing 👍

Automation didn't run, but the deploy happened 3 weeks ago so I'm issuing payment!

Payment summary:

@ZhenjaHorbach $250 - paid in upwork ✔️
@nkdengineer $250 - paid in upwork ✔️

Expensify / App