Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.56k stars 2.9k forks source link

[HOLD for payment 2024-08-26][$250] Workspace – System message for changed workspace name is not translated to Spanish #46219

Closed lanitochka17 closed 2 months ago

lanitochka17 commented 3 months 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.12-0 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Email or phone of affected tester (no customers): gocemate+a706@gmail.com Issue reported by: Applause - Internal Team

Issue found when executing PR https://github.com/Expensify/App/pull/45970

Action Performed:

  1. On an account with multiple policies in NewDOt, create an expense on one.
  2. Log into OldDot and change the policy name
  3. Back in NewDot verify that there's a message in #Admin that says "updated the workspace name from X to Y"
  4. Go to Preferences> Change the language to Spanish
  5. From OldDot Cchange the workspace name again
  6. Go to admin room and check language on system message

Expected Result:

The system message should be translated to Spanish

Actual Result:

System message for changed workspace is not translated to Spanish. It appear on English even language is changed

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/36a24a0e-8f72-45f2-82db-0ea26e5feb6a

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01cff222f969263a52
  • Upwork Job ID: 1818044405838732218
  • Last Price Increase: 2024-07-29
  • Automatic offers:
    • suneox | Reviewer | 103338869
    • FitseTLT | Contributor | 103338870
melvin-bot[bot] commented 3 months ago

Triggered auto assignment to @johncschuster (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.

FitseTLT commented 3 months ago

Proposal

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

Workspace – System message for changed workspace name is not translated to Spanish

What is the root cause of that problem?

We are not displaying the action POLICYCHANGELOG_UPDATE_NAME translating the message but instead display the action with ReportActionItemMessage which only displays the message fragments without translation

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

We need to include a translation first for policy changed text that takes new and old policy name as params then we will display the action here by creating an else if for POLICYCHANGELOG_UPDATE_NAME type action and pass the translated text as the message of ReportActionItemBasicMessage https://github.com/Expensify/App/blob/b0f810df06c3df866b7fdbed4959f8f1fef803ed/src/pages/home/report/ReportActionItem.tsx#L655-L656

We can of course create a util that returns the translated text. And also we can apply the same for other type of actions too that do not support translation yet.

What alternative solutions did you explore? (Optional)

melvin-bot[bot] commented 3 months ago

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

melvin-bot[bot] commented 3 months ago

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

cretadn22 commented 3 months ago

Proposal

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

System message for changed workspace name is not translated to Spanish

What is the root cause of that problem?

For update log messages in admin room, we don't translate to Spanish

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

https://github.com/Expensify/App/blob/df69c80229acdc510064d2671e2fdde8095a93a7/src/libs/ReportActionsUtils.ts#L1298-L1315

We’ve added a function to translate system messages, but we haven't included the POLICYCHANGELOG_UPDATE_NAME action. We need to incorporate logic to handle the translation for the POLICYCHANGELOG_UPDATE_NAME action.

if (isActionOfType(action, CONST.REPORT.ACTIONS.TYPE.POLICY_CHANGE_LOG.UPDATE_NAME)) {
        const oldName = getOriginalMessage(action)?.oldName ?? '';
        const newName = getOriginalMessage(action)?.newName ?? '';
        const message = `${Localize.translateLocal('workspace.renamedPolicyAction.renamedPolicyAction', {oldName, newName})}`;   // NEED TO CREATE A NEW TRANSLATION KEY renamedPolicyAction
        return [{text: message, html: `<muted-text>${message}</muted-text>`, type: 'COMMENT'}];
    }

For example, en.ts (need to confirm Spanish translation in es.ts)

renamedPolicyAction: {
    renamedPolicyAction: ({oldName, newName}: RenamedRoomActionParams) => ` updated the name of this workspace from ${oldName} to ${newName}`,
},

I've observed that other update log messages, such as POLICYCHANGELOG_UPDATE_FIELD, also aren't translated into Spanish. We should apply the same approach to address these cases as well.

What alternative solutions did you explore? (Optional)

suneox commented 3 months ago

@FitseTLT @cretadn22 Thank you for your proposals.

The RCA from both solutions is correct, and both solutions include updating the new translation. In spite of the proposal from @cretadn22 being more detailed but since the policy name does not support markdown so there is no need to update the render as a fragment with TextCommentFragment.

The solution from @FitseTLT was provided first, and rendering the content as ReportActionItemBasicMessage also makes sense in this case.

So @FitseTLT proposal is LGTM, and we should also update alternateText for the sidebar.

🎀 👀 🎀 C+ reviewed

melvin-bot[bot] commented 3 months ago

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

robertjchen commented 3 months ago

Thanks for the proposals and review. Let's go with @FitseTLT 's 👍

melvin-bot[bot] commented 3 months ago

📣 @suneox 🎉 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 3 months ago

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

FitseTLT commented 3 months ago

@johncschuster I need a copy for updated the name of this workspace from ${oldName} to ${newName} Thx

FitseTLT commented 3 months ago

@johncschuster or @robertjchen Can you add the Need a Copy label? Thx

melvin-bot[bot] commented 3 months ago

Triggered auto assignment to @danielrvidal (Waiting for copy), see https://stackoverflow.com/c/expensify/questions/7025/ for more details.

melvin-bot[bot] commented 3 months ago

@danielrvidal, @robertjchen, @johncschuster, @suneox, @FitseTLT Whoops! This issue is 2 days overdue. Let's get this updated quick!

FitseTLT commented 3 months ago

@danielrvidal Can you pls give us the translation of this? Thx.

johncschuster commented 3 months ago

Just bumped @danielrvidal in DM 👍

danielrvidal commented 3 months ago

For our internal team, I'm confirming the translation here: https://expensify.slack.com/archives/C21FRDWCV/p1722978885798709

johncschuster commented 3 months ago

Thanks @danielrvidal!

danielrvidal commented 3 months ago

Here you are @FitseTLT

actualizó el nombre de este espacio de trabajo de “${oldName}” a “${newName}”

Let me know if you have any questions.

johncschuster commented 3 months ago

Awesome. Thanks, @danielrvidal!

robertjchen commented 3 months ago

Just some conflicts to be resolved on the PR and I can merge 👍

robertjchen commented 3 months ago

Merged and on the way out!

robertjchen commented 2 months ago

On prod

melvin-bot[bot] commented 2 months ago

Current assignee @johncschuster is eligible for the Bug assigner, not assigning anyone new.

robertjchen commented 2 months ago

@johncschuster could you please help with the next steps here? Thanks!

suneox commented 2 months ago

@johncschuster This issue has been deployed to production over a week, so I think we should add label await payment

melvin-bot[bot] commented 2 months ago

@danielrvidal, @robertjchen, @johncschuster, @suneox, @FitseTLT Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

suneox commented 2 months ago

This issue has been deployed to production 2 weeks ago. We're waiting for payment

johncschuster commented 2 months ago

Sorry about that! I process my "Hold for payment" issues every morning (by way of filtering specifically for those issue titles), and I missed this since the automation didn't seem to change the title. I'll get this sorted!

johncschuster commented 2 months ago

Payment Summary:

Contributor: @FitseTLT paid $250 via Upwork Contributor+: @suneox owed $250 via Upwork

johncschuster commented 2 months ago

(I've updated the title to make sure I see this when I action my payment issues in the morning)

suneox commented 2 months ago

@johncschuster I've accepted the offer

FitseTLT commented 2 months ago

@johncschuster There is already an offer that I have already accepted.

johncschuster commented 2 months ago

Thanks for clarifying that, @FitseTLT! I'll use the offer you've already accepted and will close the new one I made.

johncschuster commented 2 months ago

I've issued payment! Thank you both for your contributions! 🎉

robertjchen commented 2 months ago

thanks!