[HOLD for payment 2023-12-20] [$500] Task - When the receiver clicks on 'Copy to clipboard' for a task, the copied text shows incorrectly

[m-natarajan]

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:

1. Log in to User A's account.
2. Open User B's chat.
3. Go to the "Assign task" section.
4. Create a task by entering a title.
5. Open another browser and log in to User B's account.
6. Open User A's chat and click on “Copy to clipboard” of sent task
7. Paste it in the chat.
8. Notice that “Task for undefined” is copied instead of the actual task title

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?

• 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

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

• Upwork Job URL: https://www.upwork.com/jobs/~01adb76d099d1a9033
• Upwork Job ID: 1729954370591846400
• Last Price Increase: 2023-11-29
• Automatic offers:
• cubuspl42 | Reviewer | 27886951
• paultsimura | Contributor | 27886953

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0104b5bf9c69d8ee5f

[melvin-bot]

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

[paultsimura]

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 undefined for the recipient here:

App/src/libs/actions/Task.js

Lines 922 to 925 in 9945d3c

	const report = ReportUtils.getReport(reportID);
	if (isCreateTaskAction) {
	return `task for ${report.reportName}`;
	}

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 reportAction contains the field childReportName, 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.

App/src/components/ReportActionItem/TaskPreview.js

Line 74 in 7130361

const taskTitle = props.taskReport.reportName || props.action.childReportName;

After the recent changes, we use the Task.getTaskReportActionMessage in 2 places, I recommend making a broader refactoring:

Zero, make a Pusher BE change:

We set the childReportName optimistically here:

App/src/libs/ReportUtils.js

Line 2088 in 2f60c27

reportAction.reportAction.childReportName = taskTitle;

It also is updated on the OpenReport API call.

We need to also add an update of reportAction.childReportName to the EditTask API 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 the reportAction.childReportName updated 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.ts with 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 getTaskReportActionMessage as before the recent updates – only for building the message for actions that are isTaskAction == 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 of if/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 isCreateTaskAction check and using message.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

	function TaskAction(props) {
	return (
	<>
	<View style={[styles.flex1, styles.flexRow, styles.alignItemsCenter]}>
	<Text style={[styles.chatItemMessage, styles.colorMuted]}>{Task.getTaskReportActionMessage(props.actionName, props.taskReportID, false)}</Text>
	</View>
	</>
	);
	}

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.

App/src/components/ReportActionItem/TaskPreview.js

Line 74 in 7130361

const taskTitle = props.taskReport.reportName || props.action.childReportName;

-    const taskTitle = props.taskReport.reportName || props.action.childReportName;
+    const taskTitle = TaskUtils.getTaskTitle(props.taskReportID, props.action.childReportName);

What alternative solutions did you explore? (Optional)

[allroundexperts]

I would agree with @paultsimura's proposal here. I think I missed the fact that child report name is included in the action 😄

[redpanda-bit]

Proposal

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)

[cubuspl42]

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)

[melvin-bot]

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

[cubuspl42]

@NikkiWines Would you review the "Zero, make a Pusher BE change" section in this proposal?

We need to figure out a plan for the next steps. Something that could work...

1. An internal engineer could add the necessary change to the back-end
2. We would then re-test the proposed solution(s) and resume the external process

Let me know what you think

[melvin-bot]

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

[melvin-bot]

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

[NikkiWines]

Sorry for the delay here, will give the BE proposal a review tomorrow!!

[NikkiWines]

👋 Looks like the backend changes suggested here align with some proposed backend changes for another issue related to task name changes / editing tasks, which is already underway (being handled by @marcaaron).

Let's wait for those changes to be deployed and re-evaluate to determine what (if any) next steps are necessary to resolve this issue

[NikkiWines]

Ooh, although it looks like we've possibly decided to hold off on those changes 🤔 @thienlnam should we HOLD on this issue as well?

[melvin-bot]

@cubuspl42 @NikkiWines @sonialiap this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[cubuspl42]

@paultsimura Just the translation

[paultsimura]

Thank you. The PR is ready for review: #32249

In this PR I've fixed another issue – the report text in LHN (if the last message was a task) was not localized and did not get updated when the task title was modified: it always remained "task for ${originalTitle}".

[cubuspl42]

From the checklist:

I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method

If you modify some logic for displaying a message in a non-trivial way, this checkbox starts applying to you in the context of this message, in my opinion.

[paultsimura]

If you modify some logic for displaying a message in a non-trivial way, this checkbox starts applying to you in the context of this message, in my opinion.

Sorry, I don't think I got your point in the context of this task. I did use localization, and the copy was confirmed in Slack. Am I missing something?

[cubuspl42]

I've fixed another issue

I just meant that this isn't really "another issue" in my interpretation. Solving unrelated issues in a PR for a specific issue might be not considered appropriate. Everything is fine, we can move on

[melvin-bot]

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

[melvin-bot]

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:

• fix: Update task title without opening the report #32249

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)

[melvin-bot]

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:

[cubuspl42]

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

[sonialiap]

@cubuspl42 $500 - paid ✔️
@paultsimura $500 - paid ✔️
@ayazhussain79 report $50 - offer sent - paid ✔️

[ayazhussain79]

@sonialiap Offer accepted, Thank you

[melvin-bot]

@cubuspl42, @paultsimura, @sonialiap, @thienlnam Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[sonialiap]

Payment completed ✔️