Web - Workspace - IOU moves in the LHN every time the merchant is changed

[kbecciv]

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.53.1
Reproducible in staging?: y
Reproducible in production?: n
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4428435&group_by=cases:section_id&group_id=294998&group_order=asc
Issue reported by: Applause - Internal Team

Action Performed:

1. Admin: Log in with a new Gmail account
2. Admin: Create a workspace
3. Admin: Invite a member to the workspace
4. Member: Create a scan IOU in the workspace chat
5. Member: Open the IOU and add the "Amount" and "Merchant" fields manually
6. Admin: Open the IOU and edit the "Merchant" field

Expected Result:

It shouldn't move. If an order change is needed due to the unread status, it should only happen once.

Actual Result:

IOU moves in the LHN every time the merchant is changed.

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

Bug6415247_1710525981908.bandicam_2024-03-15_18-47-10-675.mp4

View all open jobs on GitHub

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Triggered auto assignment to @hayata-suenaga (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[kbecciv]

We think that this bug might be related to #wave-collect - Release 1

[kbecciv]

Issue is not reproduce in Prod

bandicam.2024-03-15.22-26-02-304.mp4

[hayata-suenaga]

taking a look at this issue

[hayata-suenaga]

This is reproducible
https://github.com/Expensify/App/assets/98560306/6c2e7584-ce99-44be-ae27-cf17595cbdc2

[hayata-suenaga]

asking for help in Slack

[paultsimura]

This might be considered a regression from #38027 – now, every time we update the MoneyRequest field, we call OpenReport because of this hook:

App/src/pages/home/ReportScreen.tsx

Lines 365 to 374 in d78c0f1

	// If a user has chosen to leave a thread, and then returns to it (e.g. with the back button), we need to call `openReport` again in order to allow the user to rejoin and to receive real-time updates
	useEffect(() => {
	if (!isFocused || prevIsFocused || !ReportUtils.isChatThread(report) || report.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN) {
	return;
	}
	Report.openReport(report.reportID);
	// We don't want to run this useEffect every time `report` is changed
	// eslint-disable-next-line react-hooks/exhaustive-deps
	}, [prevIsFocused, report.notificationPreference, isFocused]);

The reason why the issue happens is that on updating the money request, we set the hasOutstandingChildRequest: false for the chat report in the optimistic data – that's why the chat report moves below the money request in LHN – it has to RBR for a moment. But after we call OpenReport, the chat report gets updated with hasOutstandingChildRequest: true, and the RBR appears again, moving the chat report above the open Money Request.

However, I would blame this PR: #34866 – it changed the way we calculate the optimistic hasOutstandingChildRequest, so now we are setting it to false, when we shouldn't.

[paultsimura]

Proposal

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

The chat report's GBR disappears for a moment when modifying the money request data.

Note: this shouldn't really be a blocker, because the version on Staging only allowed this bug to be more visible – it exists in Prod as well.

What is the root cause of that problem?

The reason why the issue happens is that on updating the money request, we set the hasOutstandingChildRequest: false for the chat report in the optimistic data – that's why the chat report moves below the money request in LHN – it has no GBR for a moment.

In this PR, we've added the logic for calculating the optimistic hasOutstandingChildRequest:

App/src/libs/actions/IOU.ts

Lines 1261 to 1262 in 2979e3c

	hasOutstandingChildRequest:
	iouReport && needsToBeManuallySubmitted(iouReport) && updatedMoneyRequestReport.managerID === userAccountID && updatedMoneyRequestReport.total !== 0,

However, this code doesn't work as expected. e.g. it strongly checks needsToBeManuallySubmitted(iouReport) for every iouReport, but I should have the GBR if I'm the admin of the workspace even if the request doesn't need to be manually submitted. Also, the mentioned PR broke the existing logic: #33030 (the first test).

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

First, we should modify the getOutstandingChildRequest to correctly calculate the GBR for different scenarios (it may require some further alterations, need to test different cases deeper):

function getOutstandingChildRequest(policy: OnyxEntry<Policy> | EmptyObject, iouReport: OnyxEntry<Report> | EmptyObject): OutstandingChildRequest {
    if (!iouReport || isEmptyObject(iouReport)) {
        return {};
    }

    if (!isPolicyExpenseChat(iouReport)) {
        return {
            hasOutstandingChildRequest: iouReport.managerID === currentUserAccountID && iouReport.total !== 0,
        };
    }

    if (!needsToBeManuallySubmitted(iouReport)) {
        return {
            hasOutstandingChildRequest: false,
        };
    }

    if (PolicyUtils.isPolicyAdmin(policy)) {
        return {
            hasOutstandingChildRequest: true,
        };
    }

    // We don't need to update hasOutstandingChildRequest in this case
    return {};
}

App/src/libs/actions/IOU.ts

Lines 419 to 438 in 5770e23

	/**
	* Return the object to update hasOutstandingChildRequest
	*/
	function getOutstandingChildRequest(policy: OnyxEntry<OnyxTypes.Policy> | EmptyObject, iouReport: OnyxTypes.Report): OutstandingChildRequest {
	if (!needsToBeManuallySubmitted(iouReport)) {
	return {
	hasOutstandingChildRequest: false,
	};
	}
	if (PolicyUtils.isPolicyAdmin(policy)) {
	return {
	hasOutstandingChildRequest: true,
	};
	}
	return {
	hasOutstandingChildRequest: iouReport.managerID === userAccountID && iouReport.total !== 0,
	};
	}

Second, use this function instead of the incorrect custom checks:

        {
            onyxMethod: Onyx.METHOD.MERGE,
            key: `${ONYXKEYS.COLLECTION.REPORT}${iouReport?.parentReportID}`,
            value: ReportUtils.getOutstandingChildRequest(policy, updatedMoneyRequestReport),
        },

App/src/libs/actions/IOU.ts

Lines 1257 to 1264 in 5770e23

	{
	onyxMethod: Onyx.METHOD.MERGE,
	key: `${ONYXKEYS.COLLECTION.REPORT}${iouReport?.parentReportID}`,
	value: {
	hasOutstandingChildRequest:
	iouReport && needsToBeManuallySubmitted(iouReport) && updatedMoneyRequestReport.managerID === userAccountID && updatedMoneyRequestReport.total !== 0,
	},
	},

App/src/libs/actions/IOU.ts

Lines 3029 to 3035 in 5770e23

	{
	onyxMethod: Onyx.METHOD.MERGE,
	key: `${ONYXKEYS.COLLECTION.REPORT}${chatReport?.reportID}`,
	value: {
	hasOutstandingChildRequest: iouReport && needsToBeManuallySubmitted(iouReport) && updatedIOUReport?.managerID === userAccountID && updatedIOUReport.total !== 0,
	},
	},

What alternative solutions did you explore? (Optional)