[HOLD for Payment 2024-05-29][$250] IOU - Copied link URL includes "Undefined"

[izarutskaya]

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

---

Version Number: 1.4.66-2
Reproducible in staging?: Y
Reproducible in production?: Y
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

1. Navigate to staging.new.expensify.com
2. Send an IOU to a user
3. Open the expense report and send a comment
4. Reply in thread to the sent comment on step 3
5. Hover over the IOU preview component and click on copy link
6. Paste the copied link elsewhere

Expected Result:

The copied URL doesn't include "Undefined"

Actual Result:

The copied URL includes "Undefined" at the end

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6462644_1714123878533.bandicam_2024-04-26_12-26-46-976.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01a65eb92968b7e037
• Upwork Job ID: 1784516119574372352
• Last Price Increase: 2024-05-05

Issue Owner

Current Issue Owner: @gijoe0295

[melvin-bot]

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

[izarutskaya]

We think this issue might be related to the #vip-vsb.

[dragnoir]

Proposal

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

Copied link URL includes "Undefined"

What is the root cause of that problem?

on context menu, the copy link menu item const reportActionID = reportAction?.reportActionID; can be undefined

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

Lines 398 to 403 in 035adb8

	onPress: (closePopover, {reportAction, reportID}) => {
	Environment.getEnvironmentURL().then((environmentURL) => {
	const reportActionID = reportAction?.reportActionID;
	Clipboard.setString(`${environmentURL}/r/${reportID}/${reportActionID}`);
	});
	hideContextMenu(true, ReportActionComposeFocusManager.focus);

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

We need to check if the value reportAction?.reportActionID is available first.

const reportActionID = reportAction?.reportActionID || '';

POC:

20240426_184957.mp4

[gijoe0295]

Proposal

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

reportActionID in URL is undefined when reply an expense report's message in thread

What is the root cause of that problem?

originalReportID evaluated in ReportActionItem points to the transaction thread report, but should be the policy expense report instead:

App/src/libs/ReportUtils.ts

Lines 5277 to 5279 in 2d88758

	if (transactionThreadReportID !== null) {
	return Object.keys(currentReportAction ?? {}).length === 0 ? transactionThreadReportID : reportID;
	}

By the early return above, we skip the isThreadFirstCheck which should be used for the report preview action in this case.

Then when we retrieved the report actions in BaseReportActionContextMenu we couldn't find the correct action because it apparently belonged to another report:

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

Line 304 in d3eeff3

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

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

Line 134 in 2d88758

return reportActions[reportActionID] ?? null;

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

We should prioritize the isThreadFirstChat check:

App/src/libs/ReportUtils.ts

Lines 5277 to 5282 in 2d88758

	if (transactionThreadReportID !== null) {
	return Object.keys(currentReportAction ?? {}).length === 0 ? transactionThreadReportID : reportID;
	}
	return isThreadFirstChat(reportAction, reportID) && Object.keys(currentReportAction ?? {}).length === 0
	? allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]?.parentReportID
	: reportID;

if (Object.keys(currentReportAction ?? {}).length === 0) {
    return isThreadFirstChat(reportAction, reportID) ? parentReportID : transactionThreadReportID ?? reportID;
}
return reportID;

What alternative solutions did you explore? (Optional)

Or we can just show [Deleted report] in this case, by this user couldn't copy link. That means expense reports with no expenses are deleted. To do this, we can either:

1. Modify the isClosedExpenseReportWithNoExpenses to not include the statusNum condition. Now it should be isExpenseReportWithNoExpenses:

App/src/libs/ReportUtils.ts

Line 1146 in fa29a35

function isClosedExpenseReportWithNoExpenses(report: OnyxEntry<Report>): boolean {

2. Modify the backend to set the expense report's statusNum to CLOSED (i.e. 2) when there're no expenses.

[melvin-bot]

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

[melvin-bot]

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

[laurenreidexpensify]

This feels like low priority / polish for vip-vsb

[melvin-bot]

@rojiphil, @laurenreidexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[gijoe0295]

Hi @rojiphil did you have a chance to take a look?

[laurenreidexpensify]

@rojiphil bump for review ^^

[rojiphil]

@gijoe0295 As per your proposal, the copied link will have reportActionID as that of the Report Preview action and reportID as that of the Report Preview action’s Chat Report. Is my understanding correct?

[rojiphil]

And sorry for the delay in review here.

[gijoe0295]

@rojiphil Yes, correct. In other words, that will link to the report preview action in the policy expense report (not the expense report currently).

[rojiphil]

Yes, correct. In other words, that will link to the report preview action in the policy expense report (not the expense report currently).

@gijoe0295 And why do you think reportID should point to the policy expense report?

[gijoe0295]

@rojiphil because the report preview is in the policy expense report. If it didn't point to the correct report, it would cause errors like this one, or navigate to not found page.

[rojiphil]

@gijoe0295 Thanks for the clarifications.

[rojiphil]

@gijoe0295 proposal has the correct RCA and solution.
This will also ensure Copy to clipboard to work correctly which is otherwise broken currently.
🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

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

[laurenreidexpensify]

@bondydaa this one is ready for review 👍

[melvin-bot]

❌ There was an error making the offer to @rojiphil for the Reviewer role. The BZ member will need to manually hire the contributor.

[melvin-bot]

❌ There was an error making the offer to @gijoe0295 for the Contributor role. The BZ member will need to manually hire the contributor.

[bondydaa]

oh GH issues might have caused that https://www.githubstatus.com/incidents/tsrh60nkbchl

[laurenreidexpensify]

@gijoe0295 looks like we good to go - please let us know when you expect to have a draft PR up thanks

[laurenreidexpensify]

Payment Summary:

• C+ @rojiphil $250 offer sent in upwork
• Contributor @gijoe0295 $250 offer sent in upwork

[rojiphil]

• C+ @rojiphil $250 offer sent in upwork

Thanks @laurenreidexpensify. Accepted the offer

[laurenreidexpensify]

Payment Summary:

C+ @rojiphil $250 paid in upwork
Contributor @gijoe0295 $250 paid in upwork

@rojiphil do we need a regression test for this - please confirm steps if so, thanks

[rojiphil]

do we need a regression test for this - please confirm steps if so, thanks

A regression test is not required as this issue occurs only in certain cases and is not high-impact. We can skip this.