[HOLD for payment 2023-11-22] [$500] Request Money - Error Does not disappear when returning back

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.3.85.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:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @daveSeife
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1697510382480569

Action Performed:

1. FAB > Request Money >Manual
2. Click next without entering a valid amount.
   Notice an error will appear.
3. Switch or Go to 'Scan' or 'Distance' in the header.
4. Return to 'Manual'
   Notice the error is still there

Expected Result:

Error disappears when returning back

Actual Result:

Error Does not disappear when returning back

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

Android: Native

XRecorder_17102023_053114.mp4

Android: mWeb Chrome

XRecorder_17102023_053023.mp4

iOS: Native

RPReplay_Final1697510044.MP4

iOS: mWeb Safari

RPReplay_Final1697510004.MP4

MacOS: Chrome / Safari

T159macchrome.Error.After.Going.Back-1.mp4

Recording.5033.mp4

MacOS: Desktop

T159macdesk.Error.When.Going.Back-1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01b128a1e4d22d1251
• Upwork Job ID: 1714280416375877632
• Last Price Increase: 2023-10-31
• Automatic offers:
• DylanDylann | Contributor | 27548886
• daveSeife | Reporter | 27548887

Issue Owner

Current Issue Owner: @JmillsExpensify

[DylanDylann]

Proposal

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

Error Does not disappear when returning back in request money

What is the root cause of that problem?

Currently when user switches between tabs in request money iou, component MoneyRequestAmountForm will not unmount and still has formError when user back to Manual tab

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

1. Pass selectedTab as a prop in

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

   Lines 153 to 160 in 91e2120

   	<MoneyRequestAmountForm
   	isEditing={isEditing}
   	currency={currency}
   	amount={iou.amount}
   	ref={(e) => (textInput.current = e)}
   	onCurrencyButtonPress={navigateToCurrencySelectionPage}
   	onSubmitButtonPress={navigateToNextPage}
   	/>

2. Every time A changes, we will reset formError

• We should add useEffect in MoneyRequestAmountForm

 useEffect(()=>{
        setFormError('')
    }, [selectedTab])

What alternative solutions did you explore? (Optional)

[Nathan-Mulugeta]

Proposal

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

Error message doesn't clear when changing tabs and coming back.

What is the root cause of that problem?

The root cause of this problem is here in the MoneyRequestAmountForm page, we can see here that typing an amount clears the error:

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

Lines 136 to 138 in be44c93

	if (!_.isEmpty(formError)) {
	setFormError('');
	}

But we are not clearing the error when the selected tab is changed.

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

We can pass a prop selectedTab:

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

Lines 153 to 160 in be44c93

	<MoneyRequestAmountForm
	isEditing={isEditing}
	currency={currency}
	amount={iou.amount}
	ref={(e) => (textInput.current = e)}
	onCurrencyButtonPress={navigateToCurrencySelectionPage}
	onSubmitButtonPress={navigateToNextPage}
	/>

like this:

<MoneyRequestAmountForm
            isEditing={isEditing}
            currency={currency}
            amount={iou.amount}
            ref={(e) => (textInput.current = e)}
            onCurrencyButtonPress={navigateToCurrencySelectionPage}
            onSubmitButtonPress={navigateToNextPage}
            selectedTab={selectedTab} //Add this line
        />

and add a useEffect on MoneyRequestAmountForm page to clear the error whenever we change tabs:

 useEffect(() => {
        setFormError('');
    }, [selectedTab]);

What alternative solutions did you explore? (Optional)

We can also follow the pattern we used on DistanceRequest component to remove the error

App/src/components/DistanceRequest/index.js

Lines 99 to 111 in 4c1ae82

	useEffect(() => {
	const transactionWaypoints = lodashGet(transaction, 'comment.waypoints', {});
	if (!lodashGet(transaction, 'transactionID') || !_.isEmpty(transactionWaypoints)) {
	return;
	}
	// Create the initial start and stop waypoints
	Transaction.createInitialWaypoints(transactionID);
	return () => {
	// Whenever we reset the transaction, we need to set errors as empty/false.
	setHasError(false);
	};
	}, [transaction, transactionID]);

Instead of passing the selectedTab to MoneyRequestAmountForm we can pass the transactionID

<MoneyRequestAmountForm
            isEditing={isEditing}
            currency={currency}
            amount={iou.amount}
            ref={(e) => (textInput.current = e)}
            onCurrencyButtonPress={navigateToCurrencySelectionPage}
            onSubmitButtonPress={navigateToNextPage}
            transactionID={iou.transactionID} //Add this line
        />

and add a useEffect on MoneyRequestAmountForm page to clear the error whenever the transactionID changes:

 useEffect(() => {
        setFormError('');
    }, [transactionID]);

[melvin-bot]

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

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01b128a1e4d22d1251

[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

[melvin-bot]

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

[strepanier03]

Easy to recreate:

error still visible

[Santhosh-Sellavel]

Will review proposals today

[Santhosh-Sellavel]

@strepanier03 Yes easy to recreate but is that expected behavior can you check and confirm with the Product team, please?

[melvin-bot]

@strepanier03, @Santhosh-Sellavel Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[strepanier03]

Raised it @Santhosh-Sellavel - My recommendation is for closing it because the error is correct, the amount is still invalid.

[melvin-bot]

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

[daveSeife]

@strepanier03 if you do the same thing in 'distance' section, the error is dismissed when going back.

Switch to distance > click next. Notice an error is shown
Now switch to scan and switch back to Distance. The error is gone.

[strepanier03]

I'll have to circle back to this next week.

[Santhosh-Sellavel]

Any update @strepanier03?

[strepanier03]

I didn't get any traction on the discussion raised. It makes sense to me we should be consistent in this behavior and in the example @daveSeife shared we do clear the error when switching. Let's move forward with being consistent with that.

[melvin-bot]

@strepanier03 @Santhosh-Sellavel 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!

[melvin-bot]

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:

• [@Santhosh-Sellavel] The PR that introduced the bug has been identified. Link to the PR:
• [@Santhosh-Sellavel] 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:
• [@Santhosh-Sellavel] 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:
• [@Santhosh-Sellavel] Determine if we should create a regression test for this bug.
• [@Santhosh-Sellavel] 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.
• [@strepanier03] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[melvin-bot]

@dangrous, @strepanier03, @Santhosh-Sellavel, @DylanDylann Eep! 4 days overdue now. Issues have feelings too...

[Santhosh-Sellavel]

_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:

• [@Santhosh-Sellavel] The PR that introduced the bug has been identified. Link to the PR: This is a minor inconsistency between request flows. Not able to point out a single PR that caused this, i.e it was missed.
• [@Santhosh-Sellavel] 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: N/A
• [@Santhosh-Sellavel] 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: N/A Should have been caught based checklist.
• [@Santhosh-Sellavel] Determine if we should create a regression test for this bug. Yes
• [@Santhosh-Sellavel] 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. [HOLD for payment 2023-11-22] [$500] Request Money - Error Does not disappear when returning back #29774 (comment)
• [@strepanier03] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:_ https://github.com/Expensify/Expensify/issues/340549

[strepanier03]

My apologies for the delay here everyone! I was out most of last week for the US holiday but I'm working on this now and will have an update shortly.

[strepanier03]

@daveSeife and @DylanDylann - I've paid you both via Upwork and ended the contracts, thanks again for being part of the community!

---

@Santhosh-Sellavel - Please post the checklist responses as soon as you have the time to do so and I'll move this forward. Once the checklist is done I'lll post the payment summary for your manual request.

Thanks!

[strepanier03]

@Santhosh-Sellavel - Sorry I missed your comment right before mine yesterday with the checklist. It appears empty to me though, was that a bad copy paste?

[Santhosh-Sellavel]

Regression Test Steps

Bug: Request Money - Error Does not disappear when returning back

Steps

1. FAB > Request Money >Manual
2. Click next without entering a valid amount.
3. Notice an error will appear.
4. Switch or Go to 'Scan' or 'Distance' in the header.
5. Return to 'Manual'
6. Verify that the error disappeared

[strepanier03]

Thanks @Santhosh-Sellavel - I'll work on the regression test now and then post the payment summary.

[strepanier03]

Payment summary

• External issue reporter - @daveSeife - $50 via Upwork
• Contributor that fixed the issue - @DylanDylann - $500 via Upwork
• Contributor+ that helped on the issue and/or PR - @Santhosh-Sellavel - $500 via Manual Requests

@JmillsExpensify @Santhosh-Sellavel - Please submit your Manual Request for payment.

[strepanier03]

This can be closed once your request is paid. Let me know or close if you can.

[strepanier03]

@Santhosh-Sellavel - Have you submitted your request for payment yet?

[melvin-bot]

@JmillsExpensify, @dangrous, @strepanier03, @Santhosh-Sellavel, @DylanDylann Huh... This is 4 days overdue. Who can take care of this?

[Santhosh-Sellavel]

Yes submitted last week, yet to receive payment.

[strepanier03]

@JmillsExpensify - Friendly bump on the payment for the C+. Santhosh says they submitted their request last week. Feel free to close this GH once you pay it or @mention me so I can do ti.

[JmillsExpensify]

@Santhosh-Sellavel Did you potentially submit on the old workspace? This is the chat room/workspace you should be using for report submissions. I announced in Slack, though no worries if you missed it.

[Santhosh-Sellavel]

Moved Request to new workspace

[strepanier03]

Thank you @Santhosh-Sellavel for moving that and @JmillsExpensify for checking in and helping us wrap this up.

[dangrous]

I think we're just waiting on payment of the (new) request, right? Anything else we need?

[JmillsExpensify]

$500 payment approved for @Santhosh-Sellavel based on this comment.

[JmillsExpensify]

P.S. @strepanier03 In the future, no need to hold issues open for NewDot payments.