Split Bill - Page Freezes After Hitting the Split bill Button

kbecciv commented 1 year ago

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:

Open NewDot on mweb or web.
Go to any chat and click the "+" FAB.
Click on split bill and type amount.
Click on Next button and click on split button.

Expected Result:

Split bill amount and continue without freezing.

Actual Result:

Hitting the split bill button causes the page to freeze.

Workaround:

Unknown

Platforms:

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

[ ] Android / native
[x] Android / Chrome
[ ] iOS / native
[x] iOS / Safari
[x] MacOS / Chrome / Safari
[ ] MacOS / Desktop

Version Number: 1.3.40-4 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

https://github.com/Expensify/App/assets/93399543/61641581-93a5-4735-a97b-cc05dfa0ec99

https://github.com/Expensify/App/assets/93399543/34a26ee3-d385-4ad2-82e8-620fbbb78df0

Expensify/Expensify Issue URL: Issue reported by: @jo-ui Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1689354587381659

View all open jobs on GitHub

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

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

hungvu193 commented 1 year ago

Proposal

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

Split Bill - Page Freezes After Hitting the Split bill Button

What is the root cause of that problem?

When splitting bill, we collected user email, mapped them and put into the params, the problem is since we migrated to use accountID, we won't show userLogin (email), if we didn't have any interactions with them, so their emails are undefined.

That's why our app will be crashed, because we're trying to access undefined value in here and here

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

Since our backend still validate the email, we can add a validation on FE, if we couldn't find user email then we should show error text and prevent user from splitting.
Incase we still allow user to split money with the user that they've never interacted with, we should update our BE to remove email validation (since we used accountID), and replace email with accountID in here and here For example:
```
    if (email === currentUserEmail) {
        return;
    }
```
Should be changed to:
```
    if (accountID === currentUserAccountID) {
        return;
    }
```
What alternative solutions did you explore? (Optional)

N/A

melvin-bot[bot] commented 1 year ago

@JmillsExpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

JmillsExpensify commented 1 year ago

I'm not able to reproduce this one.

melvin-bot[bot] commented 1 year ago

📣 @jo1290! 📣 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>
```

jo-ui commented 1 year ago

@JmillsExpensify When splitting bill among many users this issue occurs, try it in large group with many users.

melvin-bot[bot] commented 1 year ago

📣 @jo-ui! 📣 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>
```

jo-ui commented 1 year ago

Contributor details Your Expensify account email: jhon.benex@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~01fe71dd923c718f93

melvin-bot[bot] commented 1 year ago

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

melvin-bot[bot] commented 1 year ago

@JmillsExpensify Huh... This is 4 days overdue. Who can take care of this?

JmillsExpensify commented 1 year ago

Still not able to reproduce this. Most recently tried on iOS / Safari.

melvin-bot[bot] commented 1 year ago

@JmillsExpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

melvin-bot[bot] commented 1 year ago

@JmillsExpensify this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

melvin-bot[bot] commented 1 year ago

@JmillsExpensify 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

JmillsExpensify commented 1 year ago

Testing again. Wasn't reproducible in Mac OS / Safari. Closing.

BhuvaneshPatil commented 1 year ago

Proposal

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

Split bill - App crashing when make a split bill in a group chat with a new users

What is the root cause of that problem?

In the method - createSplitsAndOnyxData in IOU.js we are using participant.login value at few places.

https://github.com/Expensify/App/blob/ed2c287daf6a1d59f4f43a9bfc84128597575730/src/libs/actions/IOU.js#L476

https://github.com/Expensify/App/blob/ed2c287daf6a1d59f4f43a9bfc84128597575730/src/libs/actions/IOU.js#L598

https://github.com/Expensify/App/blob/ed2c287daf6a1d59f4f43a9bfc84128597575730/src/libs/actions/IOU.js#L667

When we create a new user using email while creating group, the value for login property is undefined.

And the error is saying that we can't have undefined as a value for that.

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

When we create a new user, the value for text in participants (in createSplitsAndOnyxData method) has same value as the email/phone that was used to create the account.

We can use that on the places mentioned above -

participant.login || participant.text

In this way we will always prefer the login value and in case new user (where login is undefined) we will take text as a value.

What alternative solutions did you explore? (Optional)

jo-ui commented 1 year ago

@JmillsExpensify This issue is still reproducible. Please consider it.

Expensify / App