[$1000] Money request page crash after sign in #23420

kavimuru · 2023-07-23T17:02:27Z

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:

Sign out if you are signed in
Go to staging.new.expensify.com/request/new
Login

Expected Result:

App crash

Actual Result:

App should not crash

Workaround:

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

Platforms:

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

Version Number: 1.3.44-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
Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2023-07-22.at.17.04.45.mov

Recording.1320.mp4

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01c96f9a061c08188b
Upwork Job ID: 1683620832387874816
Automatic offers:

bernhardoj | Contributor | 25856547
bernhardoj | Reporter | 25856549

melvin-bot · 2023-07-23T17:02:32Z

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

kavimuru · 2023-07-23T17:02:34Z

Proposal

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

The app crash when opening the money request page after signing in

What is the root cause of that problem?

The crash happens because in TextInputWithCurrencySymbol, selectedCurrencyCode prop is marked as required, but it got undefined.

App/src/components/TextInputWithCurrencySymbol.js

Lines 23 to 24 in b7d17e0

    
           /** Currency code of user's selected currency */ 
        
           selectedCurrencyCode: PropTypes.string.isRequired,

The selectedCurrencyCode is passed down from MoneyRequestAmountPage with the value from props.iou.currency (Onyx IOU). We have a defaultProps that set the currency to USD.

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

Lines 67 to 72 in b7d17e0

    
           iou: { 
        
               id: '', 
        
               amount: 0, 
        
               currency: CONST.CURRENCY.USD, 
        
               participants: [], 
        
           },

defaultProps will be used if iou is undefined. The problem is, Onyx IOU itself has a default value.

App/src/setup/index.js

Lines 31 to 39 in b7d17e0

    
           initialKeyStates: { 
        
               // Clear any loading and error messages so they do not appear on app startup 
        
               [ONYXKEYS.SESSION]: {loading: false}, 
        
               [ONYXKEYS.ACCOUNT]: CONST.DEFAULT_ACCOUNT_DATA, 
        
               [ONYXKEYS.NETWORK]: {isOffline: false}, 
        
               [ONYXKEYS.IOU]: { 
        
                   loading: false, 
        
                   error: false, 
        
               },

So, when we sign in, we have an IOU object of {loading, error} which will ignore the defaultProps in MoneyRequestAmountPage, thus the currency is undefined.
This issue is a regression of my PR here #17964. Previously, we use lodashGet to get the currency and default to USD. Here is the commit of using the lodashGet https://github.com/Expensify/App/pull/17964/commits/4fdf587c1fca3cc147f7184c1bf765bcd2cf5ee5#diff-879f243892ee617d3ea9b[…]91d2bec3eb12b90f09aac6a05a
but somehow I remove it in a later commit.

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

I think that's okay to not use lodashGet and instead we can remove the default value of Onyx IOU.
(remove this)

App/src/setup/index.js

Lines 36 to 39 in b7d17e0

    
           [ONYXKEYS.IOU]: { 
        
               loading: false, 
        
               error: false, 
        
           },

Both loading and error are not being used anywhere.

What alternative solutions did you explore? (Optional)

Make selectedCurrencyCode in TextInputWithCurrencySymbol optional and set USD as the default prop of selectedCurrencyCode.

melvin-bot · 2023-07-23T17:02:34Z

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

JmillsExpensify · 2023-07-24T23:29:19Z

Interestingly, I don't get a crash for this case though I do see this.

JmillsExpensify · 2023-07-24T23:30:45Z

In any case, I think that's largely a detail given that it's consistent with your video. I'm going to triage as a result.

melvin-bot · 2023-07-24T23:31:03Z

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

melvin-bot · 2023-07-24T23:31:06Z

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

melvin-bot · 2023-07-24T23:31:11Z

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

MuhammadASif011 · 2023-07-25T02:47:01Z

Hello, If you need my help to fix this task, so please let me know. Thank you
[$1000] Money request page crash after sign in #23420 - Expensify

Upwork profile: https://www.upwork.com/freelancers/~01ef98c02265da3b6f

melvin-bot · 2023-07-25T02:47:04Z

📣 @MuhammadASif011! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.

Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

kayowakmelese · 2023-07-25T04:00:40Z

Hello, we can solve the issue by modifying this code
//Check if props.iou.currency exists, otherwise, use the default currency (USD)
const selectedCurrencyCode = props.iou.currency || 'USD';

melvin-bot · 2023-07-25T04:00:43Z

📣 @kayowakmelese! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.

Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

kayowakmelese · 2023-07-25T04:03:36Z

Contributor details
Your Expensify account email: kayomelese4@gmail.com
Upwork Profile Link: https://www.upwork.com/fl/~01393ae9100dff9b33

melvin-bot · 2023-07-25T04:03:38Z

⚠️ Missing/invalid email or upwork profile link. Please make sure you add both your Expensify email and Upwork profile link in the format specified.

kayowakmelese · 2023-07-25T04:08:23Z

Contributor details
Your Expensify account email: kayomelese4@gmail.com
Upwork Profile Link: https://www.upwork.com/fl/~01393ae9100dff9b33

melvin-bot · 2023-07-25T04:08:25Z

⚠️ Missing/invalid email or upwork profile link. Please make sure you add both your Expensify email and Upwork profile link in the format specified.

kayowakmelese · 2023-07-25T04:09:41Z

Contributor details
Your Expensify account email: kayomelese4@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~01393ae9100dff9b33

melvin-bot · 2023-07-25T04:09:43Z

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

MuhammadASif011 · 2023-07-25T04:40:52Z

Contributor details
Expensify account email: asifsuperior43@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~01ef98c02265da3b6f

flyingmorrow · 2023-07-25T09:02:15Z

It is snap to fix it I think.
In this case error occurred due to variable of currency.
check currency variable and prevent case that can not be undefined then it can be fixed easily I think.
I can fix it rapidly.
Please kindly send me mail to handsome.sladar@gmail.com
then I can fix it right now.
Thank you.

melvin-bot · 2023-07-25T09:02:18Z

📣 @flyingmorrow! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.

Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

allroundexperts · 2023-07-25T23:02:13Z

The RCA and solution mentioned in @bernhardoj's proposal looks good to me.

🎀 👀 🎀 C+ reviewed

melvin-bot · 2023-07-31T12:47:46Z

📣 @allroundexperts Please request via NewDot manual requests for the Reviewer role ($1000)

melvin-bot · 2023-07-31T12:47:50Z

📣 @bernhardoj 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer 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 📖

melvin-bot · 2023-07-31T12:47:55Z

📣 @bernhardoj 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

bernhardoj · 2023-07-31T13:22:29Z

Can't log in to the app. I think there is a log in problem.

bernhardoj · 2023-07-31T16:51:27Z

PR is ready for review

cc: @allroundexperts

melvin-bot · 2023-07-31T20:12:56Z

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

melvin-bot · 2023-08-02T17:02:42Z

🎯 ⚡️ Woah @allroundexperts / @bernhardoj, great job pushing this forwards! ⚡️

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

when @bernhardoj got assigned: 2023-07-31 12:47:50 Z
when the PR got merged: 2023-08-02 17:02:41 UTC

On to the next one 🚀

rushatgabhane · 2023-08-08T04:16:10Z

I don't think this was the current fix

correct fix was done here - #23636 (comment)

we can revert the PR in this issue

bernhardoj · 2023-08-08T04:21:58Z

It is the correct fix.

Quoting from the linked issue:

However we need to make sure to not assign directly to props.IOU.currency in the useEffect as it is undefined at the first time.

The reason the currency is undefined is because the default props are not applied. That is because we initialized the IOU props to be {errors, loading}. The PR removes that because it is unused. The IOU currency props won't be undefined anymore.

allroundexperts · 2023-08-22T22:19:36Z

@JmillsExpensify I think this is ready for payment.

bernhardoj · 2023-08-28T14:40:26Z

@JmillsExpensify bump for payment

trjExpensify · 2023-08-30T17:45:31Z

👋 @JmillsExpensify can you reduce the payment for @bernhardoj by 50% when you pay this please?

We missed a regression here and I already settled up. Thanks!

allroundexperts · 2023-09-06T12:45:11Z

@JmillsExpensify Waiting for the payment summary so I can request payment in ND!

bernhardoj · 2023-09-18T18:03:44Z

@JmillsExpensify hi, is there any update on the payment?

bernhardoj · 2023-10-03T03:58:18Z

@JmillsExpensify waiting for the payment

JmillsExpensify · 2023-10-16T15:31:43Z

Apologies, circling back to this now.

JmillsExpensify · 2023-10-16T15:37:46Z

Payment summary:

Issue reporter: @bernhardoj $250
Contributor: @bernhardoj $1,000 (incl. urgency bonus; overpaid $500 here)
Contributor+: @allroundexperts $1,500 (incl. urgency bonus)

I'll issue an offer to @bernhardoj based on this amount. Please comment on this issue if you feel that's incorrect.

JmillsExpensify · 2023-10-16T15:38:17Z

Then @allroundexperts looks like you owe us a BZ checklist on this issue, including suggestion on regression test.

JmillsExpensify · 2023-10-16T15:40:59Z

Alright, Upwork offers are paid out. I'm going to close this issue but I'll still keep tabs on follow-up since I'll have to process a payment for C+ via NewDot.

allroundexperts · 2023-10-18T00:50:13Z

Checklist

As mentioned in the authors proposal, the issue came from another PR that he did.
N/A
N/A
A regression test would be helpful.

Regression test

Sign out if you are signed in
Go to /request/new
Login and verify that the app does not crash

Do we 👍 or 👎 ?

JmillsExpensify · 2023-10-23T16:43:14Z

That seems like a pretty basic regression test. I think it's already covered given that we are testing requesting before each deploy.

JmillsExpensify · 2023-10-23T16:43:31Z

$1,500 payment approved for @allroundexperts based on summary above.

kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jul 23, 2023

melvin-bot bot assigned JmillsExpensify Jul 23, 2023

JmillsExpensify added the External Added to denote the issue can be worked on by a contributor label Jul 24, 2023

melvin-bot bot changed the title ~~Money request page crash after sign in~~ [$1000] Money request page crash after sign in Jul 24, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 24, 2023

melvin-bot bot assigned allroundexperts Jul 24, 2023

melvin-bot bot assigned madmax330 Jul 25, 2023

bernhardoj mentioned this issue Jul 31, 2023

Fix crash on opening money request page after sign ing #23937

Merged

58 tasks

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Jul 31, 2023

ahmedGaber93 mentioned this issue Jul 31, 2023

[HOLD for payment 2023-08-10] App crashes when clicking 'Split bill' button for newly created account #23951

Closed

6 tasks

luacmartins mentioned this issue Aug 3, 2023

Fix expense report total when deleting requests #23822

Merged

58 tasks

bernhardoj mentioned this issue Aug 30, 2023

[HOLD for payment 2023-08-28] [$1000] When deleting a file, there is a space between the username and the '[Deleted message]' #24216

Closed

6 tasks

JmillsExpensify closed this as completed Oct 16, 2023

[$1000] Money request page crash after sign in #23420

[$1000] Money request page crash after sign in #23420

Comments

kavimuru commented Jul 23, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

melvin-bot bot commented Jul 23, 2023

kavimuru commented Jul 23, 2023

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

melvin-bot bot commented Jul 23, 2023 • edited by JmillsExpensify Loading

Bug0 Triage Checklist (Main S/O)

JmillsExpensify commented Jul 24, 2023

JmillsExpensify commented Jul 24, 2023

melvin-bot bot commented Jul 24, 2023

melvin-bot bot commented Jul 24, 2023

melvin-bot bot commented Jul 24, 2023

MuhammadASif011 commented Jul 25, 2023

melvin-bot bot commented Jul 25, 2023

kayowakmelese commented Jul 25, 2023

melvin-bot bot commented Jul 25, 2023

kayowakmelese commented Jul 25, 2023

melvin-bot bot commented Jul 25, 2023

kayowakmelese commented Jul 25, 2023

melvin-bot bot commented Jul 25, 2023

kayowakmelese commented Jul 25, 2023

melvin-bot bot commented Jul 25, 2023

MuhammadASif011 commented Jul 25, 2023

flyingmorrow commented Jul 25, 2023

melvin-bot bot commented Jul 25, 2023

allroundexperts commented Jul 25, 2023 • edited Loading

melvin-bot bot commented Jul 31, 2023

melvin-bot bot commented Jul 31, 2023

melvin-bot bot commented Jul 31, 2023

bernhardoj commented Jul 31, 2023

bernhardoj commented Jul 31, 2023

melvin-bot bot commented Jul 31, 2023

melvin-bot bot commented Aug 2, 2023

rushatgabhane commented Aug 8, 2023 • edited Loading

bernhardoj commented Aug 8, 2023 • edited Loading

allroundexperts commented Aug 22, 2023

bernhardoj commented Aug 28, 2023

trjExpensify commented Aug 30, 2023

allroundexperts commented Sep 6, 2023

bernhardoj commented Sep 18, 2023

bernhardoj commented Oct 3, 2023

JmillsExpensify commented Oct 16, 2023

JmillsExpensify commented Oct 16, 2023

JmillsExpensify commented Oct 16, 2023

JmillsExpensify commented Oct 16, 2023

allroundexperts commented Oct 18, 2023

Checklist

Regression test

JmillsExpensify commented Oct 23, 2023

JmillsExpensify commented Oct 23, 2023

kavimuru commented Jul 23, 2023 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Jul 23, 2023 •

edited by JmillsExpensify

Loading

allroundexperts commented Jul 25, 2023 •

edited

Loading

rushatgabhane commented Aug 8, 2023 •

edited

Loading

bernhardoj commented Aug 8, 2023 •

edited

Loading