[$500] Web- Split bill - Unable to split the money with new users in a group

[kbecciv]

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

---

Action Performed:

1. Go to staging dot on web chrome
2. Create a group with non-existing users
3. Go to that group chat and click on + and click on split bill , enter the amount and click on split money button
4. Notice that the split button doesn't work

Expected Result:

Able to split the money with new users in a group

Actual Result:

Unable to split the money with new users in a group

Workaround:

Unknown

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

Reproducible in staging?: Yes

Reproducible in production?: No

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

green-2023-06-17_14.56.24.mp4

Recording.3138.mp4

Expensify/Expensify Issue URL:

Issue reported by: @priya-zha

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1686993012583369
https://expensify.slack.com/archives/C049HHMV9SM/p1686991867243109

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01a34b44e2ad9be2a6
• Upwork Job ID: 1714382223552339968
• Last Price Increase: 2023-10-17

[OSBotify]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[melvin-bot]

Triggered auto assignment to @stitesExpensify (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[luacmartins]

Related thread. We couldn't reproduce on staging. Closing.

[kbecciv]

Issue is reproducible on build 1.3.83.10

Recording.4982.mp4

[stitesExpensify]

@kbecciv is this only on staging? If so I'll re-add the blocker label

[stitesExpensify]

cc @mountiny since I know you were working on this

[mountiny]

I have just retested on 1.3.84 and could not repro with non-existing expensifail first time and then with non-existing gmail accounts and it worked fine.

Adding Needs Reproduction label

cc @luacmartins and @youssef-lr in case you would run into this

[luacmartins]

Should we close the issue if we cant reproduce it anymore?

[c3024]

It it still reproducible.

Video

toLowerCaseCrash.mp4

[c3024]

Proposal

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

Cannot split bill and gives a console error when splitting bill with new users or in a group with new users

What is the root cause of that problem?

login is undefined for participants who have not replied or otherwise interacted with a user. So this here

App/src/libs/actions/IOU.js

Line 1057 in a456ef9

const email = isOwnPolicyExpenseChat || isPolicyExpenseChat ? '' : OptionsListUtils.addSMSDomainIfPhoneNumber(participant.login).toLowerCase();

throws an error because participant.login is undefined and it does not have a toLowerCase() method.

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

We should add participant.text also in the above line similar to here

App/src/libs/actions/IOU.js

Line 1445 in a456ef9

const email = participant.isOwnPolicyExpenseChat ? '' : OptionsListUtils.addSMSDomainIfPhoneNumber(participant.login || participant.text).toLowerCase();

What alternative solutions did you explore? (Optional)

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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

[thesahindia]

@c3024's proposal looks good to me!

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Current assignee @stitesExpensify is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[youssef-lr]

We should add participant.text also in the above line similar to here

This won't work if the user has set a display name in their profile, as text will equal John Doe for example, and the backend will throw an error because we're sending an invalid email.

I thinnk we shouldn't allow adding a user to the split if their login is missing and their text prop is not a valid email. But probably the proper fix for this was to fix the backend to take accountIDs instead of emails..

@c3024 have you tried your solution by having one of the participant set their display name?

[c3024]

@youssef-lr
You are right. i did not check with one of the participants setting their display name. It will fail.

Disallowing splitting a bill with a user without login may also be not a good idea in my opinion. We can create a group with unknown users and composer menu has one of the options as split bill. Disabling it when one of the members has their login hidden from the user appears wacky to me.

Fixing it from the backend is ideal. I think there is no need to pass email to the backend and at the least the backend should not validate the email.

[youssef-lr]

I think we can do both:

1. For known users when we have login set, send the email
2. For unknown users with a display name, send the accountID
3. For unknown users without a display name, send the email from the text prop.

[c3024]

As far as I understand, here

App/src/libs/actions/IOU.js

Lines 1179 to 1181 in 0d11e0b

	const individualSplit = {
	email,
	accountID,

we send both email and accountID and backend validates both and throws an error if either of them is not sent.

For unknown users with a display name, send the accountID

So, I think we cannot do this (that is send accountID alone) unless we change the logic of backend to make email optional.

[youssef-lr]

So, I think we cannot do this (that is send accountID alone) unless we change the logic of backend to make email optional.

Yeah I can work on it.

[c3024]

Then we can change this

App/src/libs/actions/IOU.js

Line 1057 in 0d11e0b

const email = isOwnPolicyExpenseChat || isPolicyExpenseChat ? '' : OptionsListUtils.addSMSDomainIfPhoneNumber(participant.login).toLowerCase();

to send an empty string when login does not exist for a user with something like this

const email = isOwnPolicyExpenseChat || isPolicyExpenseChat || !participant.login ? '' : OptionsListUtils.addSMSDomainIfPhoneNumber(participant.login).toLowerCase();

or pass undefined for email.

[youssef-lr]

Closing this one in favor of #23470. Backend PR should be ready shortly, there's no changes required in App, but I'll create a small cleanup PR and add some comments.