[HOLD App # 28618] Request & send money - User is redirected to Amount page from Amount page from deep link

[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.3.87-1

Reproducible in staging?: Yes

Reproducible in production?: Yes

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

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

Issue found when executing PR https://github.com/Expensify/App/pull/29655

Action Performed:

1. Go to staging.new.expensify.com
2. Paste Request money link (https://staging.new.expensify.com/request/new ) to any chat, 3, Click on the link above.
3. Enter amount and click Next.
4. Repeat Step 2-4 with Send money link (https://staging.new.expensify.com/send/new ).

Expected Result:

User will be redirected to the next step of the Request money/Send money process

Actual Result:

The process is looped. Instead of proceeding to the next step, user lands on the enter amount page again

Workaround:

Unknown

Platforms:

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

• [x] Android: Native
• [x] Android: mWeb Chrome
• [x] iOS: Native
• [x] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [x] MacOS: Desktop

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

https://github.com/Expensify/App/assets/78819774/39517ebe-aec8-4659-b056-7511003924ce

MacOS: Desktop

View all open jobs on GitHub

[melvin-bot[bot]]

Unable to auto-create job on Upwork. The BZ team member should create it manually for this issue.

[melvin-bot[bot]]

Triggered auto assignment to @joekaufmanexpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot[bot]]

Bug0 Triage Checklist (Main S/O)

• [x] This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• [x] 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
• [x] 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.
• [x] 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.
• [x] I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot[bot]]

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

[barros001]

Proposal

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

If a user clicks on a link to request money (eg: https://staging.new.expensify.com/request/new), enter an amount, click next and then proceed to click on a link to send money (eg: https://staging.new.expensify.com/send/new), the amount will be pre-populated but after clicking next, it will render the amount screen again. The same happens if you invert the order of the links (send money first and then request money)

What is the root cause of that problem?

The problem is that a single IOU object is used for both send and request money screens. When sending money, the id is send and when requesting money the id is request. When you are in, say, the money screen, enter an amount and click Next, the IOU will be persisted with id = "send". If you then click the request money link, that IOU will be loaded and will still have id = "send". When you click next, the following method will be executed:

IOU.navigateToNextPage(iou, iouType, report);

In this case, iou.id = 'send' and iouType = 'request'.

The first thing this method does is check if the ID of the stored IOU is different than the current IOU being created/edit, and if it is, it will reset the the stored IOU, triggering a screen re-render, which will re-render the amount screen:

    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?

This problem does not happen when you use the FAB to initiate Send or Receive money. That's because each of these buttons will first call resetMoneyRequestInfo before loading the screen.

function startMoneyRequest(iouType, reportID = '') {
    resetMoneyRequestInfo(`${iouType}${reportID}`);
    Navigation.navigate(ROUTES.MONEY_REQUEST.getRoute(iouType, reportID));
}

This will stop this issue from happening. When clicking on links, we don't have the opportunity to reset before it loads, causing this problem.

My proposal is to call resetMoneyRequestInfo when the screen is first mounted, on src/pages/iou/MoneyRequestSelectorPage.js, but ONLY if there's a divergence between iou currently in Onyx and the url parameters, as follows:

    // reset money request info when screen is first loaded
    // but only if there's a divergence between current iou and url parameters
    useEffect(() => {
        if (iou.id === `${iouType}${reportID}`) {
            return;
        }

        resetMoneyRequestInfo();
    }, []);

This will resolve the inconsistence while still allowing user to reload the screen and continue right where it was left off (we will not reset because the url parameters still match current IOU.

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.

[FitseTLT]

Proposal

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

User redirected from amount page to amount page (instead of participants page) when using deep link

What is the root cause of that problem?

The problem occurs when there is a mismatch between iouType (which is found from route params) and iou.id which is found from the IOU value saved in ONYX. So if the value of the iou was 'request' and we are creating 'send' iou or vice versa navigateToNextPage resets the iou value. Because of this line https://github.com/Expensify/App/blob/eb5fec2c258e74a69bbed9d93100db4b30b265ec/src/libs/actions/IOU.js#L2863-L2871 But the resetMoneyRequestInfo function resets all the iou properties including iou.amount (to 0). So in the MoneyRequestParticipantsPage this lines of code will navigate it back because iou.amount is 0. https://github.com/Expensify/App/blob/eb5fec2c258e74a69bbed9d93100db4b30b265ec/src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsPage.js#L97-L99

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

We need to update resetMoneyRequestInfo to have the option not to reset the amount if needed. Like this,

function resetMoneyRequestInfo(id = '', donResetAmount = false) { const created = currentDate || format(new Date(), CONST.DATE.FNS_FORMAT_STRING); Onyx.merge(ONYXKEYS.IOU, { id, ...(!donResetAmount && {amount: 0}), currency: lodashGet(currentUserPersonalDetails, 'localCurrencyCode', CONST.CURRENCY.USD), comment: '', participants: [], merchant: CONST.TRANSACTION.DEFAULT_MERCHANT, category: '', tag: '', created, receiptPath: '', receiptFilename: '', transactionID: '', billable: null, }); } and replace https://github.com/Expensify/App/blob/eb5fec2c258e74a69bbed9d93100db4b30b265ec/src/libs/actions/IOU.js#L2863-L2871

to

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, true);
}

` Result

https://github.com/Expensify/App/assets/38649957/6bfcf33e-35a8-4c27-b10c-c7e1d6de9689

What alternative solutions did you explore? (Optional)

[cooldev900]

Proposal

from: @cooldev900

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

If a user clicks on a link to request money (eg: https://staging.new.expensify.com/request/new), enter an amount, click next and then proceed to click on a link to send money (eg: https://staging.new.expensify.com/send/new), the amount will be pre-populated but after clicking next, it will render the amount screen again.

What is the root cause of that problem?

The problem is that a single IOU object is used for both send and request money screens, and the id of IOU object will become "" after new request is handled because both sendMoney(send) and createTransaction(request) in MoneyRequestConfirmPage calls resetMoneyRequestInfo function without id. https://github.com/Expensify/App/blob/8dba113c233c0a433e81a637ea6c28ca815b3f02/src/libs/actions/IOU.js#L124-L141

https://github.com/Expensify/App/blob/302cebf14c4e0fcd6570ac66e9d85a1ca0212ae6/src/pages/iou/steps/MoneyRequestConfirmPage.js#L295-L311

https://github.com/Expensify/App/blob/302cebf14c4e0fcd6570ac66e9d85a1ca0212ae6/src/pages/iou/steps/MoneyRequestConfirmPage.js#L295-L311

https://github.com/Expensify/App/blob/8dba113c233c0a433e81a637ea6c28ca815b3f02/src/libs/actions/IOU.js#L1259-L1279

https://github.com/Expensify/App/blob/8dba113c233c0a433e81a637ea6c28ca815b3f02/src/libs/actions/IOU.js#L2513-L2519

After clicking request button in MoneyRequestConfirmPage, the id of IOU object will be empty so shouldReset becomes true due to "" !== "request" or "send" and then !isDistanceRequest && ((iou.amount === 0 && !iou.receiptPath) || shouldReset) turns to true so that navigateBack function lets the user to get back to amount page again. And

if (shouldReset) {
      IOU.resetMoneyRequestInfo(moneyRequestId);
  }

could be another reason in next refresh.

https://github.com/Expensify/App/blob/302cebf14c4e0fcd6570ac66e9d85a1ca0212ae6/src/pages/iou/steps/MoneyRequstParticipantsPage/MoneyRequestParticipantsPage.js#L92-L99

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

Calling the resetMoneyRequestInfo in IOU.js let a user get back to report page automatically but when we visit request/new page directly, we need to take an account if the iou.id is set or not to avoid getting back to amount page again. in NewRequestAmountPage.js

useEffect(() => {
        if (!iou.id && (lodashGet(route, 'params.iouType', '') === CONST.IOU.TYPE.REQUEST || lodashGet(route, 'params.iouType', '') === CONST.IOU.TYPE.SEND)) {
            IOU.resetMoneyRequestInfo(lodashGet(route, 'params.iouType', ''));
        }
    }, [iou.id, route.params])

What alternative solutions did you explore? (Optional)

N/A

[barros001]

Proposal

Updated

[maxconnectAbhi]

Proposal

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

If a user clicks on a link to request money (eg: https://staging.new.expensify.com/request/new), enter an amount, click next and then proceed to click on a link to send money (eg: https://staging.new.expensify.com/send/new), the amount will be pre-populated but after clicking next, it will render the amount screen again.

What is the root cause of that problem?

The issue here is that the same IOU object

{
amount:0
category:""
comment:""
created:"2023-10-19"
currency:"INR"
id:"request"
merchant:"Request"
participants:[]
receiptFilename:""
receiptPath:""
tag:""
transactionID:"8311076577222983710"
}

is used for both sending and requesting money but with opposite id's i.e, When you send money, it's labeled with the "receive" ID, and when you request money, it's labeled "send".

Which results : iou.id = 'send' and iouType = 'request'. This is handled in navigateToNextPage function https://github.com/Expensify/App/blob/18acd82f40153fc1e5091aea3217c532142e500e/src/libs/actions/IOU.js#L2863-L2871 which creates the new request

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

We need to reset the IOU if id is not matched through resetMoneyRequestInfo(moneyRequestID) in else case here

https://github.com/Expensify/App/blob/18acd82f40153fc1e5091aea3217c532142e500e/src/pages/iou/steps/NewRequestAmountPage.js#L96-L121

like

else{
            const moneyRequestID = `${iouType}${reportID}`;
            const shouldReset = iou.id !== moneyRequestID;
            if (shouldReset) {
                IOU.resetMoneyRequestInfo(moneyRequestID);
            }

What alternative solutions did you explore? (Optional)

NA

POC

https://github.com/Expensify/App/assets/47495619/15c3c12c-1c9e-4624-8f1c-e85fb26938af

[joekaufmanexpensify]

Def seems like a bug. @Ollyws thoughts on the above proposals?

[Ollyws]

Will have a look at this one today.

[joekaufmanexpensify]

Great, ty!

[Ollyws]

Thanks for the proposals everyone.

@barros001, there was a previous PR that did exactly what you are suggesting but it was reverted due to causing multiple regressions, so I think we best avoid that solution.

@FitseTLT The problem is that we're calling resetMoneyRequestInfo when we navigate to the next page due to the fact that iou.id (Onyx data) and iouType (route param) don't match, better to make sure we have the correct values initially rather than applying a work-around.

@cooldev900 Still considering yours, will need to do a little more testing.

@maxconnectAbhi Correct me if I'm wrong but I'm not sure how your proposal would work as you're suggesting we modify code inside the isEditing block, which isn't being called here as far as I can see unless we're editing.

[barros001]

@Ollyws Got it. That PR does look pretty similar to what I tried locally, so yeah, let's avoid that. What about my alternative solution? The idea is to simply reset the IOU every time the screen is first loaded. It's similar to what startMoneyRequest does, but because the route is being loaded from a link, we don't have a chance to reset it before it loads. So instead we reset it on the component level, when the screen loads using, for example:

    useEffect(() => {
        resetMoneyRequestInfo();
    }, []);

Also, we would do it on MoneyRequestSelectorPage.js instead of NewRequestAmountPage.js as NewRequestAmountPage, NewDistanceRequestPage and ReceiptSelector all depend on IOU and not reseting them when on any of the other screens could have unexpected site effects just like this one we're dealing with.

[maxconnectAbhi]

@Ollyws No I am not modifying code inside isEditing block , Its in else condition.

[peterdbarkerUK]

@Ollyws - do you think the bug report here is derivative/has the same root cause as this issue?

[Ollyws]

They are related in that they're both due to a mismatch of the Onyx data and the route param, but I'm not sure the solutions would overlap.

This one seems to be because we never clear the Onyx data when navigating via deeplink and are therefore left with stale data which doesn't match the route param, so clearning it initially when we navigate via deeplink (to match what we do when we navigate via the menu) seems to be the correct solution here.

Whereas from what I can see https://github.com/Expensify/App/issues/30060 seems to be because we set the Onyx data for the request type when we navigate to the confirmation step, and it's retained when we navigate back.

[joekaufmanexpensify]

Ongoing discussion about solutions

[joekaufmanexpensify]

@Ollyws curious if you think we're close to being able to select any of these proposals?

[joekaufmanexpensify]

Still pending more discussion on proposals

[Ollyws]

@barros001 I like the idea of running resetMoneyRequestInfo() on mount but it will cause some inconsistencies with or current behaviour. For example:

1. Create a money request and navigate to the confirmation page
2. Refresh the page
3. Click the back button until you're back on the amount page

Currently the amount will be whatever we previously entered, but with your solution it will be reset to zero.

[barros001]

@Ollyws what if we reset on mount, but only if url type parameter does not match the current IOU type in Onyx? Downside is clicking on a link from chat would not reset unless type changes but at least it will behave better (this is current behavior anyways, so still a net positive).

Unless there’s a way to intercept the link click, determine that it’s opening either a send or request screen and only then reset (I’ll try to look into this option)

[barros001]

Proposal

Updated

[joekaufmanexpensify]

Pending more discussion on proposals

[joekaufmanexpensify]

@Ollyws are we close to selecting a proposal here?

[Ollyws]

I like @barros001's updated proposal. Only resetting the moneyRequestInfo on mount if the Onyx data and param are mismatched seems like the way to go here.

🎀👀🎀 C+ reviewed

[melvin-bot[bot]]

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

[melvin-bot[bot]]

@Ollyws @luacmartins @joekaufmanexpensify this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[barros001]

@Ollyws thanks for reviewing my proposal. Looks like Upwork job failed to post. I'll get a PR ready for review once the job is posted.

[barros001]

@Ollyws @luacmartins @joekaufmanexpensify just following up on the failed Upwork job post. I have a branch ready for a PR but waiting on the Upwork project before I submit it.

[joekaufmanexpensify]

@barros001 Hi! As a heads up, your proposal has not been officially selected until you've been assigned to the issue. Right now, we're waiting for @luacmartins to do a final review here. Once you're assigned, you'll be invited to the Upwork job.

LMK if you have any questions. Thanks!

[luacmartins]

Agree that @barros001 proposal looks good. Assigning @barros001 to the issue.

[barros001]

I found the Upwork job by searching Upwork (it was created but never linked here). Upwork decided that it needs to verify my account before I can apply for the job (it's my understanding the first time I have to apply, then after that I'd be invited). I started the process but might take a day or two.

[joekaufmanexpensify]

All good, and sounds good!

[barros001]

Well, it was a lot faster than I thought it would be, my account was verified already and I applied for the job via Upwork.

[joekaufmanexpensify]

@barros001 offer sent for $500! Please let us know once the PR is ready for review.

[barros001]

PR is ready for review. Just pending attaching video for Android native, will do as soon as I can get it to run on my dev machine.

[luacmartins]

I'll put this issue on hold for https://github.com/Expensify/App/pull/28618, since that refactor should fix this.

[Ollyws]

Just to note: According to section 19 of the process doc, in this situation payment is due for the C+ and contributor. Thanks.

[joekaufmanexpensify]

Yep, I agree! We'll wait and see if that PR fixes this issue. If it does, we'll still issue payment and close this out. If not, we'll proceed with the PR.

[joekaufmanexpensify]

Held

[joekaufmanexpensify]

Same

[joekaufmanexpensify]

Still held

[joekaufmanexpensify]

Other PR was deployed to staging today. Going to retest there.

[joekaufmanexpensify]

Restested, and now when I go to https://staging.new.expensify.com/request/new I am redirected to a generic error page.

I think this is because the request refactor changed the format of request URLs. @luacmartins thoughts on fixing this?

Perhaps that could be a good pivot for this issue. And instead of going to the above page (where there is no option to leave), we could go to https://staging.new.expensify.com/not-found, which let you go back to the home page.

[barros001]

Problem here is the following:

https://github.com/Expensify/App/blob/2d8523ae3fd8a5b692700a91efb90adac7f1175f/src/ROUTES.ts#L247-L298

https://github.com/Expensify/App/blob/2d8523ae3fd8a5b692700a91efb90adac7f1175f/src/libs/Navigation/linkingConfig.ts#L393-L410

https://github.com/Expensify/App/blob/2d8523ae3fd8a5b692700a91efb90adac7f1175f/src/pages/iou/MoneyRequestSelectorPage.js#L123-L139

The /request/new route has been replaced but it's still there because money request routes will catch ANYTHING/new and try to render MoneyRequestSelectorPage. If iouType is invalid, it will render a not-found screen, but request IS valid. The app will continue rendering the old request money screen and it will fail because CONST.TAB.DISTANCE, CONST.TAB.MANUAL, and CONST.TAB.SCAN no longer exist. The same will happen to split (not sure if split have been migrated or not).

To me looks like we're in this transition phase where this route/screen has to exist because send still uses it. Do we plan to transition send as well and eventually get rid of the old screens? If that's the case I suggest we do the following:

Update the should show check to this:

shouldShow={!IOUUtils.isValidMoneyRequestType(iouType) || !isAllowedToCreateRequest || [CONST.IOU.TYPE.REQUEST, CONST.IOU.TYPE.SPLIT].includes(iouType)}

And get rid of this if block: https://github.com/Expensify/App/blob/2d8523ae3fd8a5b692700a91efb90adac7f1175f/src/pages/iou/MoneyRequestSelectorPage.js#L111-L141

This will make it so request/new behaves the same way as invalidioutype/new until everything is migrated away and this routes/screens are completely removed.

[joekaufmanexpensify]

Ah, actually I think we just fixed that here.

[joekaufmanexpensify]

Chatted with @luacmartins and there's nothing else we need to fix here. I just need to pay $500 to each of @barros001 and @Ollyws and then close this out. Will take care of that tomorrow!

[joekaufmanexpensify]

@barros001 $500 sent and contract ended!

[joekaufmanexpensify]

Upwork job auto closed, so opened a new one to pay @Ollyws