[$500] Request Money - Request money screen opens again on click of Next button

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

---

Action Performed:

1. Click on FAB icon
2. Select Request money option
3. Enter amount click on Next button
4. Select user
5. Click Add to Split
6. Click on back icon and go to Request money page
7. Edit amount and click on Next button

Expected Result:

It should navigate to user selection screen

Actual Result:

It navigates to Request money page with a 0 value

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.74.0
Reproducible in staging?: y
Reproducible in production?: n
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

Screen.Recording.2023-09-25.at.2.41.20.PM.mov

Recording.4746.mp4

Expensify/Expensify Issue URL:
Issue reported by: @tewodrosGirmaA
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1695407645834009

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~013bcacd63e5609ce5
• Upwork Job ID: 1706561388369063936
• Last Price Increase: 2023-12-05

[OSBotify]

👋 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 @cristipaval (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[Pluto0104]

This isn't a deploy blocker anymore. I can reproduce it on NewDot. It's a regression of #27797.

[cristipaval]

This is also reproducible in production

[melvin-bot]

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

[peterdbarkerUK]

Initial report here

Subsequent report (generating this issue) here

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~013bcacd63e5609ce5

[melvin-bot]

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

[dukenv0307]

Proposal

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

It navigates to Request money page with a 0 value

What is the root cause of that problem?

Here, we're only calling goBack with optional fallback. So if we go from the "participants" page with iouType = request (with split selected) to "confirm" page with iouType = split. When going back it will go back to the iouType = request participants page, which will trigger resetting the iou.

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

In here we need to force navigate to fallback so it will work correctly for the case iouType changes in the middle of the flow

Navigation.goBack(fallback, true);

This force fallback appraoch is already used in many cases in the participants page, for example here or here

The same thing needs to be done when going back in the participants page here.

onBackButtonPress={() => navigateBack(true)}

What alternative solutions did you explore? (Optional)

In participants page, if the user is selecting the split option (likely to go to split flow when press Next), we can update the page params to iouType = split (then back to request if the split is deselected), but this will create URL change when the user is viewing it.

Another way is to manipulate the navigation history to iouType = split if the iouType changes in the middle of the flow. So when it goes back it will go back properly.

[cristipaval]

@0xmiroslav are we still waiting for other proposals?

[0xmiros]

@dukenv0307 this is recent regression. are you able to find offending PR?

[dukenv0307]

@dukenv0307 this is recent regression. are you able to find offending PR?

@0xmiroslav sorry I'm not sure which PR causes this (or whether this is a regression)

[peterdbarkerUK]

I think #26538 is now fully merged.

[paultsimura]

Proposal

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

Coming back to the "Money request" page from the confirmation page reopens the screen on "Next" button.

What is the root cause of that problem?

When navigating from the MoneyRequestParticipantsPage to MoneyRequestConfirmPage, we change the IOU.id using IOU.setMoneyRequestId here:

App/src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsPage.js

Lines 88 to 93 in 87c1967

	const navigateToConfirmationStep = (moneyRequestType) => {
	IOU.setMoneyRequestId(moneyRequestType);
	IOU.resetMoneyRequestCategory();
	IOU.resetMoneyRequestTag();
	Navigation.navigate(ROUTES.MONEY_REQUEST_CONFIRMATION.getRoute(moneyRequestType, reportID));
	};

In case of a split operation, the iou.id will become split. Then, coming back, we do not reset the iou.id back to request.

When navigating back to the NewRequestAmountPage and proceeding to MoneyRequestParticipantsPage, we use this function for navigation:

App/src/pages/iou/steps/NewRequestAmountPage.js

Lines 131 to 142 in d521dd6

	const navigateToNextPage = (currentAmount) => {
	const amountInSmallestCurrencyUnits = CurrencyUtils.convertToBackendAmount(Number.parseFloat(currentAmount));
	IOU.setMoneyRequestAmount(amountInSmallestCurrencyUnits);
	IOU.setMoneyRequestCurrency(currency);
	if (isEditing) {
	Navigation.goBack(ROUTES.MONEY_REQUEST_CONFIRMATION.getRoute(iouType, reportID));
	return;
	}
	IOU.navigateToNextPage(iou, iouType, report);
	};

As a result, when calculating const shouldReset = iou.id !== moneyRequestID, we compare request and split, so we force the IOU reset and navigate back to the initial page:

App/src/libs/actions/IOU.js

Lines 2934 to 2942 in 296fc65

	function navigateToNextPage(iou, iouType, report, path = '') {
	const moneyRequestID = `${iouType}${report.reportID || ''}`;
	const shouldReset = iou.id !== moneyRequestID;
	// If the money request ID in Onyx does not match the ID from params, we want to start a new request
	// with the ID from params. We need to clear the participants in case the new request is initiated from FAB.
	if (shouldReset) {
	resetMoneyRequestInfo(moneyRequestID);
	}

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

First of all, I would like to point out that we shouldn't use the forceCallback approach as suggested above because it'll generate other issues like this:

Issue with forceCallback

If we go to: split confirmation -> back to participants -> uncheck the split users -> click on a user, – we should be able to create a 1:1 money request. The forceCallback will result in the creation of a split request, which is incorrect:

Screen.Recording.2023-11-28.at.21.22.28-compressed.mp4

My suggestion:

First, we should not rely on the iou.id here anymore, instead use the recently introduced in Onyx iou.isSplitRequest:

App/src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsPage.js

Line 64 in 87c1967

const isSplitRequest = iou.id === CONST.IOU.TYPE.SPLIT;

Second, I suggest that we should not update the iou.id on navigating to the confirmation step, as it was initially designed to handle both manual & split requests on one page here: #25564, but now we can use the iou.isSplitRequest for this.

So my suggestion is to remove IOU.setMoneyRequestId(moneyRequestType) from here:

App/src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsPage.js

Lines 88 to 93 in 87c1967

	const navigateToConfirmationStep = (moneyRequestType) => {
	IOU.setMoneyRequestId(moneyRequestType);
	IOU.resetMoneyRequestCategory();
	IOU.resetMoneyRequestTag();
	Navigation.navigate(ROUTES.MONEY_REQUEST_CONFIRMATION.getRoute(moneyRequestType, reportID));
	};

Also, since we won't rely on iou.id on the split requests, modify the shouldReset logic here:

const shouldReset = iouType === CONST.IOU.TYPE.SPLIT ? !props.iou.isSplitRequest : !isDistanceRequest && props.iou.id !== moneyRequestId;

App/src/pages/iou/steps/MoneyRequestConfirmPage.js

Line 126 in 87c1967

const shouldReset = !isDistanceRequest && props.iou.id !== moneyRequestId;

Optional:
Also, I think we should add an extra check on the MoneyRequestConfirmPage to make sure the hook that enforces navigation is working only when we are actually on the MoneyRequestConfirmPage because upon navigation between steps, the steps components are not unmounted – only on the modal dismiss – and the hook will be triggered even when the page is not in sight. This might cause unexpected behaviors. We should probably add these checks on the other RequestMoney steps where the navigation is forced on hooks:

+   const isFocused = useIsFocused();

    useEffect(() => {
+       if (!isFocused) {
+           return;
+       }

        // ID in Onyx could change by initiating a new request in a separate browser tab or completing a request
        if (!isDistanceRequest && prevMoneyRequestId.current !== props.iou.id) {
        ...

App/src/pages/iou/steps/MoneyRequestConfirmPage.js

Lines 114 to 138 in 87c1967

	useEffect(() => {
	// ID in Onyx could change by initiating a new request in a separate browser tab or completing a request
	if (!isDistanceRequest && prevMoneyRequestId.current !== props.iou.id) {
	// The ID is cleared on completing a request. In that case, we will do nothing.
	if (props.iou.id) {
	Navigation.goBack(ROUTES.MONEY_REQUEST.getRoute(iouType, reportID), true);
	}
	return;
	}
	// Reset the money request Onyx if the ID in Onyx does not match the ID from params
	const moneyRequestId = `${iouType}${reportID}`;
	const shouldReset = !isDistanceRequest && props.iou.id !== moneyRequestId;
	if (shouldReset) {
	IOU.resetMoneyRequestInfo(moneyRequestId);
	}
	if (_.isEmpty(props.iou.participants) || (props.iou.amount === 0 && !props.iou.receiptPath && !isDistanceRequest) || shouldReset || ReportUtils.isArchivedRoom(props.report)) {
	Navigation.goBack(ROUTES.MONEY_REQUEST.getRoute(iouType, reportID), true);
	}
	return () => {
	prevMoneyRequestId.current = props.iou.id;
	};
	}, [props.iou.participants, props.iou.amount, props.iou.id, props.iou.receiptPath, isDistanceRequest, props.report, iouType, reportID]);

Result (I tried to test different cases: split, manual request, send money):

Screen.Recording.2023-11-25.at.12.40.49-compressed.mp4

What alternative solutions did you explore? (Optional)

Alternatively, if we still want to reset the iou.id on navigation, we can do the following instead of my second point in the main proposal:

Reset the iou.id to match the iouType (the URL param) when we navigate to the MoneyRequestParticipantsPage. For this, add a hook in MoneyRequestParticipantsPage:

    useFocusEffect(
        useCallback(() => {
            IOU.setMoneyRequestId(iouType);
        }, [iouType])
    )

This hook will trigger once we navigate to the page. The simple useEffect with empty dependencies won't work since the component isn't unmounted on navigation between steps, only on the modal dismiss.

[paultsimura]

@peterdbarkerUK indeed, the PR we were holding for is now merged. The issue still remains, just with a slightly different root cause.

[dukenv0307]

@0xmiroslav My proposal will still work fine for this issue.

[0xmiros]

@peterdbarkerUK should we remove hold then?

[melvin-bot]

⚠️ This issue has had its price increased by 4x or more. Please review the issue and ensure the price is correct.

[melvin-bot]

Upwork job price has been updated to $500

[FitseTLT]

Isn't this duplicate??

[paultsimura]

Isn't this duplicate??

Seems like. But that issue was opened on Oct 10, while this – on Sep 25. According to the rules, that one should be closed in favor of this.

[cristipaval]

This one needs C+ review, correct?

[cristipaval]

friendly bump @0xmiroslav

[melvin-bot]

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

[cristipaval]

bumped @0xmiroslav on Slack here

[0xmiros]

#28618 is merged.
I am not able to reproduce

[cristipaval]

alright, thank you @0xmiroslav!