[$250] Send invoice - Invoice thread name can renamed when created offline

[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!

---

Version Number: 1.4.82-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go offline.
3. Go FAB > Send invoice.
4. Send invoice to a user.
5. Go to transaction thread.
6. Click report header > Settings.
7. Click Room name.
8. Go online.
9. Repeat Step 6 and 7.

Expected Result:

Invoice thread name cannot be renamed when created offline, because it can't be renamed online

Actual Result:

Invoice thread name can renamed when created offline

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

Add any screenshot/video evidence

Bug6510719_1718191819509.20240612_192647.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0189569ee60be7065e
• Upwork Job ID: 1803295776701776707
• Last Price Increase: 2024-06-26
• Automatic offers:
• shubham1206agra | Reviewer | 102975673
• cretadn22 | Contributor | 102975676

Issue Owner

Current Issue Owner: @

[melvin-bot]

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

[lanitochka17]

@JmillsExpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[Tony-MK]

Proposal

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

Send invoice - Invoice thread name can renamed when created offline

What is the root cause of that problem?

The problem is that the MenuItem in ReportSettingsPage is always enabled when offline for every type of report.

App/src/pages/settings/Report/ReportSettingsPage.tsx

Lines 78 to 86 in 79e619a

	{shouldDisableRename ? (
	<View style={[styles.ph5, styles.pv3]}>
	<Text
	style={[styles.textLabelSupporting, styles.lh16, styles.mb1]}
	numberOfLines={1}
	>
	{roomNameLabel}
	</Text>
	<DisplayNames

This is because the ReportUtils.shouldDisableRename function always does not check for invoice reports.

App/src/pages/settings/Report/ReportSettingsPage.tsx

Line 34 in 79e619a

const shouldDisableRename = useMemo(() => ReportUtils.shouldDisableRename(report, linkedWorkspace), [report, linkedWorkspace]);

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

We should edit using the shouldDisableRename boolean but we need to edit the ReportUtils.shouldDisableRename function to include invoice reports.

App/src/libs/ReportUtils.ts

Lines 6103 to 6104 in 79e619a

	if (isDefaultRoom(report) || isArchivedRoom(report) || isThread(report) || isMoneyRequestReport(report) || isPolicyExpenseChat(report)) {
	return true;

So, let's use the ReportUtils.isInvoiceReport function in the condition above.

[cretadn22]

Proposal

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

Send invoice - Invoice thread name can renamed when created offline

What is the root cause of that problem?

When operating online, the shouldDisableRename function will indicate true because the invoice report is a thread. However, in offline mode, since the invoice report lacks a 'parentReportActionID', the isThread function returns false, consequently causing shouldDisableRename to also return false.

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

When creating optimistic data for invoice report, we should include parentReportActionID

Currently, we're generating the invoice report prior to creating the optimistic report action

App/src/libs/actions/IOU.ts

Lines 1758 to 1759 in 79e619a

	// STEP 2: Create a new optimistic invoice report.
	const optimisticInvoiceReport = ReportUtils.buildOptimisticInvoiceReport(chatReport.reportID, senderWorkspaceID, receiverAccountID, receiver.displayName ?? '', amount, currency);

App/src/libs/actions/IOU.ts

Lines 1805 to 1806 in 79e619a

	// STEP 5: Build optimistic reportActions.
	const [optimisticCreatedActionForChat, optimisticCreatedActionForIOUReport, iouAction, optimisticTransactionThread, optimisticCreatedActionForTransactionThread] =

My suggestion is that we generate a random ID, similar to what we did when creating the optimistic report action

App/src/libs/ReportUtils.ts

Line 4624 in c82dafd

reportActionID: NumberUtils.rand64(),

Then we will use this reportActionID for both optimistic invoiceReport and optimistic reportAction

What alternative solutions did you explore? (Optional)

[Tony-MK]

Proposal

Updated

[melvin-bot]

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

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0189569ee60be7065e

[melvin-bot]

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

[JmillsExpensify]

Added to vip invoicing and went ahead and opened up to the community.

[melvin-bot]

@JmillsExpensify, @shubham1206agra Huh... This is 4 days overdue. Who can take care of this?

[JmillsExpensify]

Awaiting proposal review.

[melvin-bot]

Current assignee @shubham1206agra is eligible for the External assigner, not assigning anyone new.

[JmillsExpensify]

Not clear why this is overdue.

[shubham1206agra]

@cretadn22 Can you provide the test branch to test your proposal?

[Tony-MK]

@shubham1206agra, Can you please give me feedback on my proposal

[cretadn22]

@shubham1206agra Thanks for reviewing. I made a mistake in the previous test branch. You are right, this is the new draft PR, could you check again?

[shubham1206agra]

Nope, still wrong.

[cretadn22]

The idea still be my proposal here. About detail implementation:

1. When creating iouReport we generate a random reportActionID (And assign it to iouReport.parenReportActionID)
2. When creating reportAction for chat report (in these function: buildOptimisticCreatedReportAction buildOptimisticReportPreview) we will use above reportActionID

[cretadn22]

@shubham1206agra Could you point out what problem? I tested and it worked well

[shubham1206agra]

The idea still be my proposal here. About detail implementation:

1. When creating iouReport we generate a random reportActionID (And assign it to iouReport.parenReportActionID)

2. When creating reportAction for chat report (in these function: buildOptimisticCreatedReportAction buildOptimisticReportPreview) we will use above reportActionID

You are kind of right, but we do not generate a random reportActionID. See other flow (submit expense) for solution.

[melvin-bot]

📣 @shubham1206agra 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @cretadn22 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[cretadn22]

@shubham1206agra
I apologize if I've misunderstood anything. I checked the submitRequest process and saw that we generate a random reportActionID, which we then assign to iouReport.parentReportActionID.

App/src/libs/actions/IOU.ts

Lines 2039 to 2044 in 55546bb

	reportPreviewAction = ReportUtils.buildOptimisticReportPreview(chatReport, iouReport, comment, optimisticTransaction);
	chatReport.lastVisibleActionCreated = reportPreviewAction.created;
	// Generated ReportPreview action is a parent report action of the iou report.
	// We are setting the iou report's parentReportActionID to display subtitle correctly in IOU page when offline.
	iouReport.parentReportActionID = reportPreviewAction.reportActionID;

[cretadn22]

But we do not generate a random reportActionID

Could you help to detail this point? We create request offline, how we can get new reportActionID if we don't generate it

[shubham1206agra]

@shubham1206agra I apologize if I've misunderstood anything. I checked the submitRequest process and saw that we generate a random reportActionID, which we then assign to iouReport.parentReportActionID.

App/src/libs/actions/IOU.ts

Lines 2039 to 2044 in 55546bb

	reportPreviewAction = ReportUtils.buildOptimisticReportPreview(chatReport, iouReport, comment, optimisticTransaction);
	chatReport.lastVisibleActionCreated = reportPreviewAction.created;
	// Generated ReportPreview action is a parent report action of the iou report.
	// We are setting the iou report's parentReportActionID to display subtitle correctly in IOU page when offline.
	iouReport.parentReportActionID = reportPreviewAction.reportActionID;

@cretadn22 I want you to do a similar thing here. Do not change any function signature.

[melvin-bot]

This issue has not been updated in over 15 days. @JmillsExpensify, @youssef-lr, @shubham1206agra, @cretadn22 eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[shubham1206agra]

@JmillsExpensify It missed posting the payment date. It is August 1.

[cretadn22]

@JmillsExpensify Is the payment available?

[cretadn22]

@youssef-lr Could someone assist with processing the payment?

[JmillsExpensify]

Payment Summary:

• Contributor: @cretadn22 $250
• Reviewer: @shubham1206agra $250

[JmillsExpensify]

Contributor Upwork contract paid out.

[shubham1206agra]

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:

• [@shubham1206agra] The PR that introduced the bug has been identified. Link to the PR: Implement Send Invoice flow from Global Create #40015
• [@shubham1206agra] 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: https://github.com/Expensify/App/pull/40015/files#r1715194553
• [@shubham1206agra] 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: Not Required
• [@shubham1206agra] Determine if we should create a regression test for this bug. Yes
• [@shubham1206agra] 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.

1. Go offline.
2. Go FAB > Send invoice.
3. Send invoice to a user.
4. Go to transaction thread.
5. Click report header > Settings.
6. Verify that the Who can post option should be disabled.

• [@JmillsExpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[shubham1206agra]

@JmillsExpensify Please close this issue.

[shubham1206agra]

@JmillsExpensify Bump here

[cretadn22]

@youssef-lr Please close this issue.