Skip to content
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

[HOLD] [$500] Chat - 'task for' task copy words not translated on copy #29082

Closed
6 tasks done
kbecciv opened this issue Oct 9, 2023 · 18 comments
Closed
6 tasks done

[HOLD] [$500] Chat - 'task for' task copy words not translated on copy #29082

kbecciv opened this issue Oct 9, 2023 · 18 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Monthly KSv2

Comments

@kbecciv
Copy link

kbecciv commented Oct 9, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Action Performed:

  1. Open the app
  2. Keep language as spanish
  3. Open report which has task message in it
  4. Hover or long press on task message and copy to clipboard
  5. Paste and observe that it is in english
  6. Change language and again copy the message
  7. Paste and observe that 'task for ' part of the message is not translated with language change

Expected Result:

App should translate client side set text on change in language

Actual Result:

App does not translate 'task for' text set client side for task message on copy to clipboard of task message

Workaround:

Unknown

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.3.79.3
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

android.native.task.for.text.not.translated.mov
Recording.4893.mp4
mac.chrome.desktop.task.copy.not.translated.mov
android.chrome.task.copy.not.translated.mp4
ios.safari.native.task.copy.not.translated.mp4
windows.chrome.task.message.not.translated.on.copy.mp4

Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1696669199917159

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01d104553451987e6f
  • Upwork Job ID: 1711345218665820160
  • Last Price Increase: 2023-10-16
@esh-g
Copy link
Contributor

esh-g commented Oct 9, 2023

Proposal

Please re-state the problem we are trying to solve:

'task for' task copy words not translated on copy

What is the root cause of this problem?

The root cause is that the task message is hard coded here:

return `task for ${report.reportName}`;

Moreover, currently how we handle the copying of the right text for task report action is completely different from the way we handle the copying of actions like an IOU or reportPreview. For task, we are directly manipulating the messageHTML here:

const messageHtml = isTaskAction || isCreateTaskAction ? Task.getTaskReportActionMessage(reportAction.actionName, reportID, isCreateTaskAction) : lodashGet(message, 'html', '');

But for things like IOU and reportPreview we are using the if-else ladder with their util functions.

This is actually related to #29035 and a proposal for that issue will interfere with this one. According to the expected changes in that issue, mentioned in this proposal, a TaskUtils.js will be created with a getTaskTitle function and the logic for getting the message got created task action will be decoupled from the Closed/Re-opened/Cancelled task action.

What changes should be made to fix this?

We should first put this issue on hold for #29035 and then if the expected changes are to be made in the that issue, the following changes can be made here:

First
We need to obviously add translations to en.ts and es.ts here:

App/src/languages/en.ts

Lines 1562 to 1564 in e0ab038

messages: {
completed: 'marked as complete',
canceled: 'deleted task',

under task.messages we can create a new created field:

created: ({taskTitle}: TaskCreatedActionParams) => `task for ${taskTitle}`,

same can be done for spanish tarea para ${taskTitle}

Second
We then need to use those translations in ContextMenuActions.js where the hard coded task for ${taskTitle} will end up after fixing the fore-mentioned issue to be something like this:

} else if (ReportActionsUtils.isCreatedTaskReportAction(reportAction)) {
   const taskTitle = TaskUtils.getTaskTitle(reportAction.childReportID, reportAction.childReportName);
   const translatedMessage = Localize.translateLocal('task.messages.created', {taskTitle});
   Clipboard.setString(translatedMessage);
}

Third
We should also add the translation for the task created message here:

const optimisticAddCommentReport = ReportUtils.buildOptimisticTaskCommentReportAction(taskReportID, title, assigneeEmail, assigneeAccountID, `task for ${title}`, parentReportID);

@kbecciv kbecciv added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 9, 2023
@c3024
Copy link
Contributor

c3024 commented Oct 9, 2023

Proposal

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

The phrase task to is not translated when language is changed to Spanish

What is the root cause of that problem?

We hardcoded the English phrase here

return `task for ${report.reportName}`;

so changing language still returns the same English phrase

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

We should add a key like taskFor in maybetask,messages in en.ts and es and use the key and translate it similar to the completed, cancelled task messages there.

What alternative solutions did you explore? (Optional)

Result

@melvin-bot melvin-bot bot changed the title Chat - 'task for' task copy words not translated on copy [$500] Chat - 'task for' task copy words not translated on copy Oct 9, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 9, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Oct 9, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Oct 9, 2023

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 9, 2023
@saranshbalyan-1234
Copy link
Contributor

Proposal

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

'task for' task copy words not translated on copy

What is the root cause of that problem?

We have hardcoded 'task for' here

return `task for ${report.reportName}`;

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

In order to avoid this, move this to es.ts and en.ts here

App/src/languages/en.ts

Lines 1556 to 1557 in 389d7b0

task: {
task: 'Task',

and use the translation like this

return `${Localize.translateLocal('task.messages.taskFor')} ${report.reportName}`;

What alternative solutions did you explore? (Optional)

N/A

@melvin-bot
Copy link

melvin-bot bot commented Oct 9, 2023

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

@AmjedNazzal
Copy link
Contributor

same RC as this post which was decided not to be fixed, if it will be fixed I think it's fair to re-open that post.

@dhanashree-sawant
Copy link

dhanashree-sawant commented Oct 10, 2023

We have hardcoded text in this case, not sure if it was same in that issue too.

@AmjedNazzal
Copy link
Contributor

All text in the app except for errors and user input is hardcoded, It's the same thing in both posts, at the moment we are not supporting back-end localization for these and in the post I linked it was decided to put all related issues on hold until a back-end localization is introduced because they didn't want a front-end workaround.

@burczu
Copy link
Contributor

burczu commented Oct 11, 2023

I don't agree with @AmjedNazzal that this is related to the lack of localization on the backend side. The text we use here is indeed hardcoded and it shouldn't be. But I agree with the statement from the @aswin-s's proposal:

This is actually related to #29035 and a proposal for that issue will interfere with this one. According to the expected changes in that issue, mentioned in #29035 (comment), a TaskUtils.js will be created with a getTaskTitle function and the logic for getting the message got created task action will be decoupled from the Closed/Re-opened/Cancelled task action.

That's why I agree we should put this on hold until the mentioned issue is resolved and then check if the issue still occurs.

@AmjedNazzal
Copy link
Contributor

@burczu I get it that on the surfice this issue looks unrelated to the one I referenced because this is about a text being hardcoded directly inline but when you look at the bigger picture this is also about language changes and task messages, if you were to solve this without using Localize then you would still get only English text out of task.messages.taskFor, so for it to become Spanish you need Localize.translateLocal for task messages which in the post I've referenced they closed the post because they prefer waiting for back-end localization. Do you see what I'm saying?

@melvin-bot melvin-bot bot added the Overdue label Oct 13, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 16, 2023

@burczu, @isabelastisser Eep! 4 days overdue now. Issues have feelings too...

@melvin-bot
Copy link

melvin-bot bot commented Oct 16, 2023

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@isabelastisser
Copy link
Contributor

Hey @burczu, should we close/put this issue on hold based on this comment?

@melvin-bot melvin-bot bot removed the Overdue label Oct 16, 2023
@isabelastisser
Copy link
Contributor

Bump @burczu , can you please follow up? Thanks!

@burczu
Copy link
Contributor

burczu commented Oct 18, 2023

@isabelastisser I think we should put this one on hold, because this issue could fix it (it waits for some BE changes now).

@isabelastisser isabelastisser added Monthly KSv2 and removed Daily KSv2 labels Oct 18, 2023
@isabelastisser isabelastisser changed the title [$500] Chat - 'task for' task copy words not translated on copy [HOLD] [$500] Chat - 'task for' task copy words not translated on copy Oct 18, 2023
@melvin-bot melvin-bot bot added the Overdue label Nov 20, 2023
@isabelastisser
Copy link
Contributor

Waiting for this issue to be fixed. If this persists after, please reopen. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Monthly KSv2
Projects
None yet
Development

No branches or pull requests

8 participants