[$250] Group - App crash when removing a member on the process splitting expense

[izarutskaya]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: v9.0.16-4 Reproducible in staging?: Y Reproducible in production?: N Email or phone of affected tester (no customers): dave0123seife@gmail.com Logs: https://stackoverflow.com/c/expensify/questions/4856 Issue reported by: Applause-Internal team

Action Performed:

1. As User A Create a group chat with User B While viewing both accounts
2. go to user B > navigate to the group chat created
3. Click on the + button on the composer box > Split Expense
4. Enter amount > Click next. Don't finish the flow
5. As User A,While User B is the confirmation page, remove user B from the group.

Expected Result:

Error is shown

Actual Result:

App Crash

Workaround:

Unknown

Platforms:

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

• [ ] Android: Native
• [x] Android: mWeb Chrome
• [ ] iOS: Native
• [x] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/d950437c-dbd0-44bd-9b6c-42531415d657

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~013d4e761b19949e81
• Upwork Job ID: 1820614678720926546
• Last Price Increase: 2024-08-06
• Automatic offers:
• ishpaul777 | Contributor | 103413818

Issue Owner

Current Issue Owner: @RachCHopkins

[melvin-bot[bot]]

Triggered auto assignment to @cristipaval (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[melvin-bot[bot]]

Triggered auto assignment to @RachCHopkins (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[github-actions[bot]]

:wave: 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[bot]]

⚠️ 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.

[daledah]

Edited by proposal-police: This proposal was edited at 2024-08-06 04:08:22 UTC.

Proposal

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

App Crash

What is the root cause of that problem?

we use useOnyx to get policy In

https://github.com/Expensify/App/blob/02d0d618a882a51e0e9fd489dfbfd3654b318580/src/pages/iou/request/IOURequestStartPage.tsx#L44

When user A add user B from group, the report?.policyId is _FAKE_ -> we get policy__FALE_ when A remove B, report?.report will be undefined -> we get policy_-1

In we have the logic to detect if policy__FALE_ and policy_-1 have the same collection key

https://github.com/Expensify/react-native-onyx/blob/115542b601349d13114bf6784f645e44c02816d1/lib/useOnyx.ts#L112

But the logic to extract the collection key (splitCollectionMemberKey) is wrong

https://github.com/Expensify/react-native-onyx/blob/115542b601349d13114bf6784f645e44c02816d1/lib/OnyxUtils.ts#L409

we're using lastIndexOf to get the index of _ -> the collection key of policy__FALE_ is policy__FALE_ the collection key of policy_-1 is policy_

-> the condition is failed

-> the error is throw here

https://github.com/Expensify/react-native-onyx/blob/115542b601349d13114bf6784f645e44c02816d1/lib/useOnyx.ts#L121-L123

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

Create usePolicy hook to handle the policyID-defaulting logic

 const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${report.policyID === CONST.POLICY.OWNER_EMAIL_FAKE ? '-1' : report.policyID ?? '-1'}`); 

What alternative solutions did you explore? (Optional)

[daledah]

@cristipaval I can raise the PR soon if you agree with the solution. Thanks

[ishpaul777]

We should use indexOf instead of lastIndexOf

@daledah Wouldn't that restrict the collection key to not use _ in between ? for ex. a_collection_key (i dont think this is relevant for our App right now, but that's for sure a restriction if we do this)

[roryabraham]

@daledah as @ishpaul777 pointed out, your proposal would not work in cases when there is an underscore in the collection key base, such as for the POLICY_RECENTLY_USED_TAGS key: https://github.com/Expensify/App/blob/edb81c1df2b31103aa13a13394be88bf9ec0068b/src/ONYXKEYS.ts#L410

[roryabraham]

I think more simply we should just audit all uses of useOnyx with the policy collection key and do something like this:

https://github.com/Expensify/App/blob/a680ecc261cc8f6846172f2c93e75ff7f2e26d2b/src/pages/home/report/ReportActionItemContentCreated.tsx#L55

Maybe we can create a utility hook like usePolicy that handles that policyID-defaulting logic.

[marcaaron]

Gonna demote this one since it's kind of an edge case. Sounds like @ishpaul777 is on the right track though.

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~013d4e761b19949e81

[melvin-bot[bot]]

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

[daledah]

@marcaaron @roryabraham, seems like we're using the RCA from my proposal. I checked the collection key, most of them don't have _ between (except POLICY_RECENTLY_USED_TAGS), that leads my solution incorrect.

Thanks to @roryabraham's suggestion, we have the right way to do. I don't know why @ishpaul777 create the draft PR. Can I update the proposal to use @roryabraham's solution?

[daledah]

Updated proposal

[ishpaul777]

Sorry i was just working locally to resolve it, tests were taking too long on my machine, so i created a draft PR to run checks, i'll close it 🙇

[melvin-bot[bot]]

📣 @ishpaul777 🎉 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 📖

[cristipaval]

I assigned @ishpaul777 as the C+ since he has context on this.

[ishpaul777]

Thank you @cristipaval!

Proposal from @daledah Sounds good to me! Feel free to branch off from https://github.com/ishpaul777/App/tree/use-usePolicy if you want .

🎀 👀 🎀 C+ reviewed

[melvin-bot[bot]]

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

[melvin-bot[bot]]

📣 @daledah You have been assigned to this job! Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing 📖

[roryabraham]

Only other thing is that we might want to use || '-1' rather than ?? '-1' in case policyID is an empty string

[daledah]

@ishpaul777 this PR is ready for review.

[melvin-bot[bot]]

This issue has not been updated in over 15 days. @cristipaval, @RachCHopkins, @ishpaul777, @daledah eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[ishpaul777]

This is ready for payment 🎉

[roryabraham]

confirmed, this has been on prod for more than a week

[RachCHopkins]

Sorry @daledah did you apply for the upwork job per this comment? I do see one shortlisted proposal but I'm not 100% sure it's you.

[RachCHopkins]

Ok, no it's not, I found your upwork profile and that is not you. I will set up the job again and send you another offer @daledah

@ishpaul777 forgive me if this is a silly question - did you get paid for this offer?

[ishpaul777]

@ishpaul777 forgive me if this is a silly question - did you get paid for this offer?

Yes thats the offer i accepted, here's the contract https://www.upwork.com/nx/wm/workroom/38035610

[RachCHopkins]

What I mean is, did you get paid money for it @ishpaul777 ?

[ishpaul777]

What I mean is, did you get paid money for it @ishpaul777 ?

No, on automatic offers A BZ member need to release payment manually when the payment is due, i.e. 7 days after PR deplyed to Prod.

[RachCHopkins]

Thanks for confirmation @ishpaul777 - I was looking at totally the wrong page! I've paid that out.

@daledah I'm just waiting on you to accept the proposal (and two others) so I can pay you.

[RachCHopkins]

Payment Summary:

• Contributor: @daledah to be paid $250 via Upwork - Update: Paid 16 Sept
• Contributor+: @ishpaul777 paid $250 via Upwork

Upwork job here

[RachCHopkins]

Contributor has been paid, the contract has been completed, and the Upwork post has been closed.