[HOLD for payment 2023-06-02] [$1000] Logging in and quickly navigating to the Send Money causes app to crash

[mountiny]

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:

Break down in numbered steps

1. Log into the app while using network throttling (Slow 3G)
2. Quickly navigate to Send Money page

Expected Result:

Describe what you think should've happened

1. The app loads fine albeit it might take longer

Actual Result:

Describe what actually happened

1. The app crashed

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

N/A

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: v1.3.16-6
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
Notes/Photos/Videos: Any additional supporting documentation
https://expensify.enterprise.slack.com/files/U0533DXLD50/F058N5YB05U/screen_recording_2023-05-19_at_14.10.58.mov
Expensify/Expensify Issue URL:
Issue reported by: @jczekalski
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1684749433399059

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~016f6a8a8cad9b792d
• Upwork Job ID: 1660954516625797120
• Last Price Increase: 2023-05-23

[melvin-bot]

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

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~016f6a8a8cad9b792d

[melvin-bot]

Current assignee @jliexpensify is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[melvin-bot]

Current assignee @mountiny is eligible for the External assigner, not assigning anyone new.

[jczekalski]

@mountiny

[melvin-bot]

📣 @jczekalski You have been assigned to this job by @mountiny!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[mountiny]

This is blocking SWM from working on other issues so assigning Jan to figure this one out.

Please post a proposal once you have it ready

[alitoshmatov]

@mountiny Is it open to external contributors?

[mountiny]

@alitoshmatov sorry now, we decided to expedite this quickly to SWM as its blocking their other work

[jczekalski]

@mountiny Update: The issue affects all 3 IOU screens (Send money, Request money, Split bill).

[alitoshmatov]

I see.
Just want share what I found
The issue originating here:

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

Lines 46 to 50 in 97a5b38

	const defaultProps = {
	iou: {
	selectedCurrencyCode: CONST.CURRENCY.USD,
	},
	};

Setting default here is not effective since props.iou has value but not iou.selectedCurrencyCode, and it is preventing default value from being applied. I think you should move default value to initializing state here

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

Line 69 in 97a5b38

selectedCurrencyCode: props.iou.selectedCurrencyCode,

[YousefKhawaga]

Proposal

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

app crash when quickly navigates to IOU screens

What is the root cause of that problem?

the selected currency isn't passed because on the slow internet connection IOU is partially exist without the selectedCurrencyCode

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

Line 328 in 6946beb

selectedCurrencyCode={this.state.selectedCurrencyCode}

while it's required so it's failed

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

pass the default there

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

Line 328 in 6946beb

selectedCurrencyCode={this.state.selectedCurrencyCode}

What alternative solutions did you explore? (Optional)

we could add a default state till selectedCurrencyCode is available
or make sure that iou isn't built till selectedCurrencyCode is available
or built iou with the default selectedCurrencyCode

[jczekalski]

I see. Just want share what I found The issue originating here:

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

Lines 46 to 50 in 97a5b38

	const defaultProps = {
	iou: {
	selectedCurrencyCode: CONST.CURRENCY.USD,
	},
	};

Setting default here is not effective since props.iou has value but not iou.selectedCurrencyCode, and it is preventing default value from being applied. I think you should move default value to initializing state here

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

Line 69 in 97a5b38

selectedCurrencyCode: props.iou.selectedCurrencyCode,

I agree that this is the issue causing the crash. The proposal sounds good, though we need make sure the default value is replaced with the data from Onyx once it is ready. @mountiny can you reassign the issue to @alitoshmatov since I'm working on something else at the moment?

[melvin-bot]

📣 @alitoshmatov You have been assigned to this job by @mountiny!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[mountiny]

@alitoshmatov Lets go with your proposal, then but lets make sure we also update the value with what comes from onyx once ready.

[mountiny]

@fedirjh Sorry for the ping, I will let Jan handle the C+ role on this one.

[melvin-bot]

🎯 ⚡️ Woah @alitoshmatov, great job pushing this forwards! ⚡️

The pull request got merged within 2 working days of assignment, so this job is eligible for a 50% #urgency bonus 🎉

• when @alitoshmatov got assigned: 2023-05-23 10:35:50 Z
• when the PR got merged: 2023-05-24 13:28:47 UTC

On to the next one 🚀

[jliexpensify]

@mountiny can I confirm: do we pay @jczekalski ?

[jliexpensify]

Hmm @alitoshmatov - I can't see you on Upworks, can you share your profile?

[alitoshmatov]

@jliexpensify Sorry, forgot to apply to the job. Applied. And my upwork profile - https://www.upwork.com/freelancers/~015e75d0a3ca4b17ca

[jliexpensify]

@alitoshmatov cheers! Do you mind changing the name in your GH profile to match the one in your Upworks profile? I think this will make it a lot easier for us to find and pay you in the future.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.18-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Updated state initialization in MoneyRequestAmount page #19441

If no regressions arise, payment will be issued on 2023-06-02. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

• Merged PR within 3 business days of assignment - 50% bonus
• Merged PR more than 9 business days after assignment - 50% penalty

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

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

[alitoshmatov]

Looks like this one is ready for payment

[jliexpensify]

Apologies, paying now! @jczekalski and @alitoshmatov could you complete the checklist?

EDIT: Paid Alisher, awaiting Vit's response on something before closing contract.

[jliexpensify]

Job closed - @jczekalski @alitoshmatov : could you complete the checklist please? Thanks!

[alitoshmatov]

Regression test

• Logout from the app
• Simulate slow network
• Enter email
• enter verification code
• Imidiately, while report list is loading, press FAB and click send money
• Make sure that the app doesn't crash and everything is working
• Do we agree 👍 or 👎

[alitoshmatov]

@jliexpensify Sorry for late response

[jczekalski]

@jliexpensify I'm not sure how to complete the checklist. The instructions link points to a resource I can't access (@mountiny assigned me as C+, but I'm an external contributor from an agency).

I think @alitoshmatov's regression test looks good. I'd also add logging out and then deep linking into the "Send money" screen as an alternative method of testing. This approach might be better/safer since there's no need to simulate a slow network and then quickly tap to open the screen before the data is ready.

[jliexpensify]

Ah no worries @jczekalski - I think we're still working out some kinks here with external agencies!

So with the checklist, usually the C+ will propose solutions/tests. In this case, it looks like @alitoshmatov has proposed a regression test and @mountiny needs to link any PR's (if applicable) - you likely don't need to do anything now.

[mountiny]

Alright, I dont think there is a specific PR for this bug, it is an edge case which is hard to test and requires special conditions. Hence I would say we dont need a new regression test. The bug stems from a delay on receiving data from onyx in case user acts too fast on slow connection.

Since we are all paid up, I will close this out, let me know if there is anything else. Thanks everyone!