[$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 [email protected] 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): [email protected]
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
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6562221.1722857270352.Screen.Recording.2024-08-05.At.1.56.25.Pm.mp4

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]

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

[melvin-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]

👋 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]

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

App/src/pages/iou/request/IOURequestStartPage.tsx

Line 44 in 02d0d61

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

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:

App/src/ONYXKEYS.ts

Line 410 in edb81c1

POLICY_RECENTLY_USED_TAGS: 'nvp_recentlyUsedTags_',

[roryabraham]

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

App/src/pages/home/report/ReportActionItemContentCreated.tsx

Line 55 in a680ecc

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

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]

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

[daledah]

@ishpaul777 this PR is ready for review.

[melvin-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.