[$500] Task - Unable to copy task complete/reopen message

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Issue found when executing PR #27119

Action Performed:

1. Go to staging.new.expensify.com
2. Go to any task report
3. Complete the task and then reopen it
4. Right click on the task completion or reopening message > Copy to clipboard
5. Paste it in the composer

Expected Result:

The copied task message is pasted

Actual Result:

The copied task message is not pasted. The message is not copied at all

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.72-6

Reproducible in staging?: Yes

Reproducible in production?: Yes

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

Bug6207782_20230921_000923.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0181f1cb78999beeb1
• Upwork Job ID: 1706863200146464768
• Last Price Increase: 2023-09-27

[melvin-bot]

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

[lcsvcn]

Proposal

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

The copied task message is pasted

What is the root cause of that problem?

The cause of the problem is that we need to wrap the TaskAction is not using the CopyTextToClipboard, where other text are using it.

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

We need to wrap the text with the CopyTextToClipboard component. Similar on what is done here:

App/src/pages/iou/ReceiptSelector/index.js

Lines 118 to 121 in 4c207ec

	<CopyTextToClipboard
	text={CONST.EMAIL.RECEIPTS}
	textStyles={[styles.textBlue]}
	/>

So at this part, we need to wrap the report action message with the component above, that way it is know that needs to be copied:

App/src/components/ReportActionItem/TaskAction.js

Lines 23 to 25 in 4c207ec

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

What alternative solutions did you explore? (Optional)

[melvin-bot]

@anmurali Eep! 4 days overdue now. Issues have feelings too...

[anmurali]

Yep, can reproduce

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0181f1cb78999beeb1

[melvin-bot]

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

[BhuvaneshPatil]

Proposal

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

Unable to copy task complete/reopen message, when we try to copy the message of task complete/reopen we are not able to copy it, it does nothing.

What is the root cause of that problem?

The error for this is that when we copy the message using context menu action (that appears when we hover over it), ti calls onPress method here -

App/src/pages/home/report/ContextMenu/ContextMenuActions.js

Lines 184 to 189 in d7ad305

	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();

We are getting reportAction empty here. That's why it's not able to set the data.

The reason we are getting reportActions empty is how we are retrieving data from ONYX here -

App/src/pages/home/report/ContextMenu/BaseReportActionContextMenu.js

Lines 186 to 192 in d7ad305

	withOnyx({
	reportActions: {
	key: ({originalReportID}) => `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${originalReportID}`,
	canEvict: false,
	},
	}),
	)(

For the task completed and reopen action we don't have taskReportActions with originalReportID.

We need to use reportID for that.

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

• when we get reportActions from ONYX we shall get reportActions for both originalReportID and reportID. This will ensure that we have all the report actions and we don't break anything.
• We can use getReportAction() method here to get the reportAction in case we have empty reportAction
  

  App/src/pages/home/report/ContextMenu/BaseReportActionContextMenu.js

  Lines 57 to 62 in d7ad305

  	const reportAction = useMemo(() => {
  	if (_.isEmpty(props.reportActions) || props.reportActionID === '0') {
  	return {};
  	}
  	return props.reportActions[props.reportActionID] || {};
  	}, [props.reportActions, props.reportActionID]);

What alternative solutions did you explore? (Optional)

We can directly use reportID instead of originalReportID but I don't think that would be appropriate here.

[tienifr]

Proposal

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

The copied task message is not pasted. The message is not copied at all

What is the root cause of that problem?

This is because the originalReportID here is retrieved incorrectly for task.

That logic was added in this PR to make sure we get the correct original report id of the first thread report action for Chat Thread, to copy it correctly.

The problem is the condition in here matches Task report actions like complete, reopen as well, since those report actions also have childReportID equals to the Task thread reportID.

So here it retrieves incorrect originalReportId of the report the task was shared in, thus unable to get the report action for complete, reopen to copy.

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

We need to fix the condition here to align with its intended use case, which is to get the original report id for first thread report action of a Chat Thread. We need to make sure the report is a Chat Thread (not task thread, ...)

function getOriginalReportID(reportID, reportAction) {
    const report = lodashGet(allReports, [`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]);
    return isChatThread(report) && isThreadFirstChat(reportAction, reportID) ? lodashGet(report, ['parentReportID']) : reportID;
}

What alternative solutions did you explore? (Optional)

The condition isChatThread(report) can instead be added to the isThreadFirstChat util, but this will require additional back-end fix to not return childVisibleActionCount for the complete/reopen report actions of the task.

[s77rt]

@lcsvcn Thanks for the proposal. I don't think your RCA is correct. We have the MiniReportActionContextMenu inside ReportActionItem which handles the mini context menu actions including the copy action.

[s77rt]

@BhuvaneshPatil Thanks for the proposal. Your RCA makes sense but I don't think we got into the real root cause. As for the solutions: getting report actions for both originalReportID and reportID or when the report action is empty is a workaround. Using reportID instead of originalReportID will cause regressions (we will basically revert the use of originalReportID).

[s77rt]

@tienifr Thanks for the proposal. Your RCA is almost correct except I don't think the bug is on getOriginalReportID.

The problem is the condition in here matches Task report actions like complete, reopen as well, since those report actions also have childReportID equals to the Task thread reportID.

This seems to describe the real root cause and I think those report actions should not have childReportID in the first place since they are not the parent of that report id. This seems to be a backend issue.

🎀 👀 🎀 C+ reviewed
Need an engineer review for a backend bug: Task report actions such as TASKREOPENED and TASKCOMPLETED have childReportID set but they should not. Those report actions do not have a child.

[melvin-bot]

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

[s77rt]

Not overdue. I think this is waiting for @cead22's input ^

[melvin-bot]

@cead22 @anmurali @s77rt 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!

[s77rt]

@cead22 ^

[MariaHCD]

In any case, I think the childReportID is being returned in Report::GetChats specifically when building the report action list here. I see it's being returned in the response of OpenReport:

[MariaHCD]

@cristipaval could you clarify if we meant to set the childReportID for task report actions?

[MariaHCD]

@cristipaval mentioned he'd take a look here today 🙏🏼

[cristipaval]

This is not reproducible anymore. Could someone confirm?

Also, @MariaHCD, FWIW: the code that blames me was added last week, whereas this issue was opened three weeks ago. I just moved that logic that was the same in multiple places to a separate reusable function.

[cristipaval]

Let me know if the issue is still reproducible to you, and I'll do my best to spend some time trying to investigate deeper.

[s77rt]

Still reproducible. I can still see the childReportID added to the TASKREOPENED/TASKCOMPLETED report actions.

[melvin-bot]

@cead22 @MariaHCD @anmurali @s77rt this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[melvin-bot]

@cead22, @MariaHCD, @anmurali, @s77rt Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

@cead22 @MariaHCD @anmurali @s77rt this issue is now 4 weeks old and preventing us from maintaining WAQ, can you:

• Decide whether any proposals currently meet our guidelines and can be approved as-is today
• If no proposals meet that standard, please take this issue internal and treat it as one of your highest priorities
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[melvin-bot]

@cead22, @MariaHCD, @anmurali, @s77rt Still overdue 6 days?! Let's take care of this!

[melvin-bot]

@cead22, @MariaHCD, @anmurali, @s77rt 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

[MariaHCD]

cc: @thienlnam if you have any context around why the childReportID is included in the TASKREOPENED / TASKCOMPLETED report actions: #27884 (comment)

[thienlnam]

That shouldn't be happening, the code that adds those reportActions doesn't include a childReportID
https://github.com/Expensify/Auth/blob/53aea0415eddf80a82db5283ace0162feba26a19/auth/lib/Report.cpp#L5828-L5845

I wonder if it's happening via the FE? Or if this is still reproducible with a new task report

[MariaHCD]

Still seeing it on new task reports 🤔

It seems to me that we're deliberately setting the childReportID to the task reportID here?

[thienlnam]

Ahhh yes, so this was only meant to be added to the ADD_COMMENT reportActions not these other task actions. I can update it so that happens

Context: You can assign a task to an infinite number of people, and they might not have access to the parent report of the task so they get a reportAction in the DM between the owner and the assignee with a link to the task report. The childReportID is added to those to make sure they all link to the same task report.

[MariaHCD]

Thanks, @thienlnam! Approved the BE fix, tested locally and it worked as expected:

Screen.Recording.2023-10-26.at.11.30.24.AM.mov

We'll update this issue once the fix is deployed.

[MariaHCD]

Heads up, @s77rt, the backend fix was deployed to production. Can we retest? Thanks!

[s77rt]

@MariaHCD It's fixed! 🚀

[MariaHCD]

Nice, thanks! I don't think any payments are required or anything else is needed from engineering? So we should be good to close this one out.