Reports - Hold educational modal is displayed when holding IOU expense on Reports

[mitarachim]

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: 9.2.84-0
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught during regression testing, add the test name, ID and link from BrowserStack: #77047
Email or phone of affected tester (no customers): jsodijsoijodsjiosjd@gmail.com
Issue reported by: Applause Internal Team
Bug source: Pull Request QA execution
Device used: Mac 26.1 / Chrome
App Component: Search

Action Performed:

1. Go to staging.new.expensify.com
2. Go to 1:1 DM with any user.
3. Create an expense.
4. Open expense report.
5. Click More > Hold.
   → Hold educational modal is not displayed for IOU expenses - Expected.
6. Go to Reports > Expenses.
7. Select the expense via checkbox.
8. Click dropdown button > Hold.

Expected Result:

Hold educational modal will not be displayed when holding IOU expense on Reports.

Actual Result:

Hold educational modal is displayed when holding IOU expense on Reports.

Workaround:

Unknown

Platforms:

• Android: App
• Android: mWeb Chrome
• iOS: App
• iOS: mWeb Safari
• iOS: mWeb Chrome
• Windows: Chrome
• MacOS: Chrome / Safari

Screenshots/Videos

Bug7033684_1766149388242.23.mp4

View all open jobs on GitHub

Issue Owner

Current Issue Owner: @truph01

[melvin-bot]

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

[samranahm]

Proposal

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

Hold educational modal is displayed when holding IOU expense on Reports

What is the root cause of that problem?

The Hold action in SearchPage wisas checking if transactions were from the submitter, but unlike MoneyReportHeader, it wasn't checking if the transactions were from IOU/DM reports. IOU expenses should skip the educational modal.

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

We should add areAllTransactionsFromIOUReports useMemo that checks if all selected transaction reports are from IOU/DM reports by

• Getting the IOU report for each selected transaction
• Checking if the parent chat report is a DM

const areAllTransactionsFromIOUReports = useMemo(() => {
    const searchData = currentSearchResults?.data;
    const reports: Report[] = searchData
        ? Object.keys(searchData)
                .filter((key) => key.startsWith(ONYXKEYS.COLLECTION.REPORT))
                .map((key) => searchData[key as keyof typeof searchData] as Report)
                .filter((report): report is Report => report != null && 'reportID' in report)
        : [];

    return (
        selectedTransactionReportIDs.length > 0 &&
        selectedTransactionReportIDs.every((id) => {
            const iouReport = getReportOrDraftReport(id, reports);
            if (!iouReport?.chatReportID) {
                return false;
            }
            const chatReport = getReportOrDraftReport(iouReport.chatReportID, reports);
            return isDM(chatReport);
        })
    );
}, [selectedTransactionReportIDs, currentSearchResults?.data]);

And update the Hold action logic to skip the educational modal when areAllTransactionsFromIOUReports is true, to match the behavior in MoneyReportHeader

    if (isDismissed || areAllTransactionsFromIOUReports) {

here

App/src/pages/Search/SearchPage.tsx

Lines 606 to 622 in 2157c12

	if (shouldShowHoldOption) {
	options.push({
	icon: expensifyIcons.Stopwatch,
	text: translate('search.bulkActions.hold'),
	value: CONST.SEARCH.BULK_ACTION_TYPES.HOLD,
	shouldCloseModalOnSelect: true,
	onSelected: () => {
	if (isOffline) {
	setIsOfflineModalVisible(true);
	return;
	}
	const isDismissed = areAllTransactionsFromSubmitter ? dismissedHoldUseExplanation : dismissedRejectUseExplanation;
	if (isDismissed) {
	Navigation.navigate(ROUTES.TRANSACTION_HOLD_REASON_RHP);
	} else if (areAllTransactionsFromSubmitter) {

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[etCoderDysto]

Proposal

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

Reports - Hold educational modal is displayed when holding IOU expense on Reports

What is the root cause of that problem?

We are not checking if all the transactions are from selfDM. And setIsHoldEducationalModalVisible is called since areAllTransactionsFromSubmitter is true

App/src/pages/Search/SearchPage.tsx

Lines 620 to 625 in 129f730

	if (isDismissed) {
	Navigation.navigate(ROUTES.TRANSACTION_HOLD_REASON_RHP);
	} else if (areAllTransactionsFromSubmitter) {
	setIsHoldEducationalModalVisible(true);
	} else {
	setRejectModalAction(CONST.REPORT.TRANSACTION_SECONDARY_ACTIONS.HOLD);

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

We should navigate to hold reason modal if all the selected transactions are from selfDM

1. Calculate if all the selected transactions are from self DM here. We can create a separate useMemo block that returns true if the all the transactions are from selfDM

    const [areAllTransactionsFromSubmitter, areAllTransactionFromSelfDM] = useMemo(() => {
        const searchData = currentSearchResults?.data;
        const reports: Report[] = searchData
            ? Object.keys(searchData)
                  .filter((key) => key.startsWith(ONYXKEYS.COLLECTION.REPORT))
                  .map((key) => searchData[key as keyof typeof searchData] as Report)
                  .filter((report): report is Report => report != null && 'reportID' in report)
            : [];

        const areAllSelfDm = reports.every((report) => {
            const parentReport = allReports[`${ONYXKEYS.COLLECTION.REPORT}${report?.chatReportID}`];
            return isDM(parentReport);
        });

        if (!currentUserPersonalDetails?.accountID) {
            return [false, areAllSelfDm];
        }

        const isSubmitter =
            selectedTransactionReportIDs.length > 0 &&
            selectedTransactionReportIDs.every((id) => {
                return isCurrentUserSubmitter(getReportOrDraftReport(id, reports));
            });

        return [isSubmitter, areAllSelfDm];
    }, [selectedTransactionReportIDs, currentUserPersonalDetails?.accountID, currentSearchResults?.data, allReports]);

2, Navigate to hold reason page if all the transactions are from selfDM here

                    if (isDismissed || areAllTransactionFromSelfDM) {
                        Navigation.navigate(ROUTES.TRANSACTION_HOLD_REASON_RHP);
                    } else if (areAllTransactionsFromSubmitter) {

What alternative solutions did you explore? (Optional)

Create a separate useMemo block that returns true if the all the transactions are from selfDM

const areAllTransactionFromSelfDM = useMemo(() => {
    const searchData = currentSearchResults?.data;
    const reports: Report[] = searchData
        ? Object.keys(searchData)
              .filter((key) => key.startsWith(ONYXKEYS.COLLECTION.REPORT))
              .map((key) => searchData[key as keyof typeof searchData] as Report)
              .filter((report): report is Report => report != null && 'reportID' in report)
        : [];

    return reports.every((report) => {
        const parentReport = allReports[`${ONYXKEYS.COLLECTION.REPORT}${report?.chatReportID}`];
        return isDM(parentReport);
    });
    }, [selectedTransactionReportIDs, currentUserPersonalDetails?.accountID, currentSearchResults?.data, allReports]);

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[truph01]

Discussing this here.

[bernhardoj]

Proposal

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

Hold educational modal is shown inconsistently.

What is the root cause of that problem?

The educational modal will show if the user is the report submitter.

App/src/components/MoneyReportHeader.tsx

Lines 1197 to 1198 in 251ab97

	} else if (isReportSubmitter) {
	setIsHoldEducationalModalVisible(true);

When we hold it from the report page, it checks whether the user is the report submitter based on the chat report. On a DM case, the chat report ownerAccountID is 0, so it always false.

App/src/libs/ReportUtils.ts

Lines 1580 to 1582 in 251ab97

	function isCurrentUserSubmitter(report: OnyxEntry<Report>): boolean {
	return !!report && report.ownerAccountID === currentUserAccountID;
	}

App/src/components/MoneyReportHeader.tsx

Line 483 in 251ab97

const isReportSubmitter = isCurrentUserSubmitter(chatIOUReport);

But on the search page, it checks the expense/IOU report itself instead of the chat report.

App/src/pages/Search/SearchPage.tsx

Lines 389 to 393 in 251ab97

	return (
	selectedTransactionReportIDs.length > 0 &&
	selectedTransactionReportIDs.every((id) => {
	return isCurrentUserSubmitter(getReportOrDraftReport(id, reports));
	})

So the educational modal is shown for DM too, which I think is the expected result.

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

We need to check the IOU/expense report instead of the chat report.

App/src/components/MoneyReportHeader.tsx

Line 483 in 251ab97

const isReportSubmitter = isCurrentUserSubmitter(chatIOUReport);

[garrettmknight]

Assigning @truph01 and @mananjadhav since they worked on a really similar update.

[RachCHopkins]

I don't get the same repro steps.

8. Click dropdown button > Hold.

I get "remove hold" and I don't get an educational modal at any stage in the repro, hold or remove hold.

I'm on Mac Chrome v9.2.85-1 - maybe we just need a retest?

[RachCHopkins]

Worked it out just as I was about to request the retest - the repro steps are wrong. They include irrelevant steps to illustrate the correct outcome.

Correct repro steps:

1. Go to staging.new.expensify.com
2. Go to 1:1 DM with any user.
3. Create an expense.
4. Go to Reports > Expenses.
5. Select the expense via checkbox.
6. Click dropdown button (1 Selected) > Hold.

Expected Result:
Hold educational modal will not be displayed when holding IOU expense on Reports.

Actual Result:
Hold educational modal is displayed when holding IOU expense on Reports.

[truph01]

As discussed here, we decided to reintroduce the Hold Education modal when an IOU is created from a DM. However, the copy in the education modal needs to be updated to better reflect this specific use case.

We’re currently waiting on the design doc from @Expensify/design.

@mananjadhav will take care of the PR once the designs are ready.

[melvin-bot]

@truph01 Whoops! This issue is 2 days overdue. Let's get this updated quick!