Repository navigation
[HOLD for payment 2023-12-20] [$500] Task - When the receiver clicks on 'Copy to clipboard' for a task, the copied text shows incorrectly #29035
Description
Activity
- changed the title
[-]Task - When the receiver clicks on "Copy to clipboard" for a task, the copied text shows as "Task for undefined"[/-][+][$500] Task - When the receiver clicks on "Copy to clipboard" for a task, the copied text shows as "Task for undefined"[/+]on Oct 6, 2023 Job added to Upwork: https://www.upwork.com/jobs/~0104b5bf9c69d8ee5f
Triggered auto assignment to Contributor-plus team member for initial proposal review - @cubuspl42 (
External)Proposal
Please re-state the problem that we are trying to solve in this issue.
When a new task is created, the recipient gets "task for undefined" copied to the clipboard.
What is the root cause of that problem?
When a new task is created, the task report behaves the same way as the thread report: it is not sent by Pusher to the recipient unless one of two happens:
- The recipient opens the task manually;
- The task was assigned to the recipient, not only shared with them;
Because of that, the task report is
undefinedfor the recipient here:
Lines 922 to 925 in 9945d3c
What changes do you think we should make in order to solve the problem?
Since the task report is under the hood a child report of the "Task created" report action, even though the report is not sent by Pusher to the recipient, the original
reportActioncontains the fieldchildReportName, which should be updated by Pusher for both Sender and Recipient on every task name change:
And we already use it as a fallback here, we can do the same approach in the copy-to-clipboard text.
After the recent changes, we use the
Task.getTaskReportActionMessagein 2 places, I recommend making a broader refactoring:Zero, make a Pusher BE change:
We set the childReportName optimistically here:
Line 2088 in 2f60c27
It also is updated on the OpenReport API call.
We need to also add an update of
reportAction.childReportNameto theEditTaskAPI response, so that in cases as this, when the sender is subscribed to the task report, and the recipient is not yet, both will still get thereportAction.childReportNameupdated on every task name change.This will also help to change the task name in the chat dynamically to avoid this situation:
First, refactor
Task.getTaskReportActionMessage:I'm not sure why it's in the actions/Task.js, since it is not making any updates to Onyx and is not handling any business logic. I suggest creating the following
TaskUtils.tswith 2 functions:const allReports: Record<string, Report> = {}; Onyx.connect({ key: ONYXKEYS.COLLECTION.REPORT, callback: (report, key) => { if (!key || !report) { return; } const reportID = CollectionUtils.extractCollectionItemID(key); allReports[reportID] = report; }, }); /** * Given the Task reportAction name, return the appropriate message to be displayed and copied to clipboard. */ function getTaskReportActionMessage(actionName: string): string { switch (actionName) { case CONST.REPORT.ACTIONS.TYPE.TASKCOMPLETED: return Localize.translateLocal('task.messages.completed'); case CONST.REPORT.ACTIONS.TYPE.TASKCANCELLED: return Localize.translateLocal('task.messages.canceled'); case CONST.REPORT.ACTIONS.TYPE.TASKREOPENED: return Localize.translateLocal('task.messages.reopened'); default: return Localize.translateLocal('task.task'); } } function getTaskTitle(taskReportID: string, fallbackTitle: string): string { let taskReport = allReports[taskReportID] ?? {}; return lodashHas(taskReport, 'reportID') && taskReport.reportName ? taskReport.reportName : fallbackTitle; }
We will use
getTaskReportActionMessageas before the recent updates – only for building the message for actions that areisTaskAction == true, and since it now builds only a static text, we don't need the 2nd and 3rd params.Will update call of this function:
onPress: (closePopover, {reportAction, selection}) => { const isTaskAction = ReportActionsUtils.isTaskAction(reportAction); - const isCreateTaskAction = ReportActionsUtils.isCreatedTaskReportAction(reportAction); const isReportPreviewAction = ReportActionsUtils.isReportPreviewAction(reportAction); const message = _.last(lodashGet(reportAction, 'message', [{}])); - const reportID = lodashGet(reportAction, 'originalMessage.taskReportID', '').toString(); - const messageHtml = isTaskAction || isCreateTaskAction ? Task.getTaskReportActionMessage(reportAction.actionName, reportID, isCreateTaskAction) : lodashGet(message, 'html', ''); + const messageHtml = isTaskAction ? Task.getTaskReportActionMessage(reportAction.actionName) : lodashGet(message, 'html', '');Second
For the actions that are
isCreateTaskAction == true, we will add a separate branch ofif/else.
We should also localize the "task for" message, but I omitted it here to be laconic:if (!isAttachment) { const content = selection || messageHtml; if (isReportPreviewAction) { const iouReport = ReportUtils.getReport(ReportActionsUtils.getIOUReportIDFromReportActionPreview(reportAction)); const displayMessage = ReportUtils.getReportPreviewMessage(iouReport, reportAction); Clipboard.setString(displayMessage); } else if (ReportActionsUtils.isModifiedExpenseAction(reportAction)) { const modifyExpenseMessage = ReportUtils.getModifiedExpenseMessage(reportAction); Clipboard.setString(modifyExpenseMessage); } else if (ReportActionsUtils.isMoneyRequestAction(reportAction)) { const displayMessage = ReportUtils.getIOUReportActionDisplayMessage(reportAction); Clipboard.setString(displayMessage); + } else if (ReportActionsUtils.isCreatedTaskReportAction(reportAction)) { + const taskTitle = TaskUtils.getTaskTitle(reportAction.childReportID, reportAction.childReportName); + Clipboard.setString(`task for ${taskTitle}`); } else if (content) { const parser = new ExpensiMark(); if (!Clipboard.canSetHtml()) { Clipboard.setString(parser.htmlToMarkdown(content)); } else { const plainText = parser.htmlToText(content); Clipboard.setHtml(content, plainText); } } }Compared to simply removing the
isCreateTaskActioncheck and usingmessage.html, this approach is responsive to the task name changes.To leverage this change, we also need to change the util function call here to just
TaskUtils.getTaskReportActionMessage(props.actionName):
App/src/components/ReportActionItem/TaskAction.js
Lines 20 to 28 in d8d98cd
We should also use the util function in
TaskPreview, similar to what I mentioned in my other proposal here – this current solution will fix both issues at a time.- const taskTitle = props.taskReport.reportName || props.action.childReportName; + const taskTitle = TaskUtils.getTaskTitle(props.taskReportID, props.action.childReportName);
What alternative solutions did you explore? (Optional)
I would agree with @paultsimura's proposal here. I think I missed the fact that child report name is included in the action 😄
Reacted by Pavlo TsimuraProposal
Please re-state the problem that we are trying to solve in this issue.
Copied text pastes with undefined in the string.
What is the root cause of that problem?
As mentioned in the other proposals, the function that copies the text relies on the reportAction to be in storage in order to copy the correct text.
What changes do you think we should make in order to solve the problem?
Given that we already have the content we need, we're showing it in the list item, we can move the logic we use in the list item (TaskPreview.js) into a new function and use that function to do both:
a) render the content in the list item
b) render the content to be used in the clipboard function, with "Task for " prepended, lowercase or uppercase.
Note: By retrieve I mean to just return, no api calls involved.
This approach copies and pastes "Task for " + <@mention if any> + taskTitle. Assuming the user wants to copy the entire text, including the mention.
What alternative solutions did you explore? (Optional)
We have a consensus that we need backend changes here before we can move forward with front-end fixes.
C+ reviewed 🎀 👀 🎀
(no proposal has been selected yet; we need help coordinating with the backend)
Triggered auto assignment to @NikkiWines, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
91 remaining items
- changed the title
[-][$500] Task - When the receiver clicks on 'Copy to clipboard' for a task, the copied text shows incorrectly[/-][+][HOLD for payment 2023-12-20] [$500] Task - When the receiver clicks on 'Copy to clipboard' for a task, the copied text shows incorrectly[/+]on Dec 13, 2023 Reviewinglabel has been removed, please complete the "BugZero Checklist".The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.11-25 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2023-12-20. 🎊
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:
- @cubuspl42 requires payment automatic offer (Reviewer)
- @paultsimura requires payment automatic offer (Contributor)
- @ayazhussain79 requires payment (Needs manual offer from BZ)
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:
- [@cubuspl42] The PR that introduced the bug has been identified. Link to the PR:
- [@cubuspl42] 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:
- [@cubuspl42] 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:
- [@cubuspl42] Determine if we should create a regression test for this bug.
- [@cubuspl42] 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.
- [@sonialiap] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:
- The PR that introduced the bug has been identified. Link to the PR:
- N/A (This was a problem involving both a frontend and backend, it's difficult or impossible to point a single frontend PR)
- 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:
- N/A
- 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:
- No need for additional discussion
- Determine if we should create a regression test for this bug.
- Up to the QA team
- 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.
- Log in as User A
- Share a task with User B
- Open another browser and log in as User B
- Open the chat with User A chat and click on “Copy to clipboard” of the sent task, but don't open the task itself
- Paste the copied text in the chat.
- Verify that the text contains "task for ${title}" with the correct task title
- Verify that the chat with User A has the "task for ${title}" with the correct task title in LHN
- As User A, modify the Task title
- As User B, click on “Copy to clipboard” of the task, and paste it in the chat.
- Verify that the text contains "task for ${title}" with the updated task title
- Verify that the chat with User A has the "task for ${title}" with the updated task title in LHN
- Change language to Spanish
- Verify that both LHN text and the one copied to the clipboard are translated into Spanish.
Reacted by Sonia Liapounova- The PR that introduced the bug has been identified. Link to the PR:
@cubuspl42 $500 - paid ✔️
@paultsimura $500 - paid ✔️
@ayazhussain79 report $50 - offer sent - paid ✔️@sonialiap Offer accepted, Thank you
@cubuspl42, @paultsimura, @sonialiap, @thienlnam Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
Payment completed ✔️
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:
Expected Result:
When the receiver clicks on “Copy to clipboard” for a task, the copied text should display the task title, not “undefined”
Actual Result:
When the receiver clicks on “Copy to clipboard,” the copied text shows as "Task for undefined," which is incorrect
Workaround:
unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
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
Screen.Recording.2023-10-06.at.10.48.11.PM.mov
Screen.Recording.2023-10-06.at.10.42.17.PM.mov
Screen.Recording.2023-10-06.at.10.40.39.PM.mov
20231006224627.mp4
screen-recording-2023-10-06-at-100240-pm_wtHhh4Fs.mp4
Recording.13.mp4
Expensify/Expensify Issue URL:
Issue reported by: @ayazhussain79
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1696612963386119
View all open jobs on GitHub
Upwork Automation - Do Not Edit