[hold for payment 2024-08-26] [$250] Split - Unable to split expense with workspace via FAB

[lanitochka17]

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: 9.0.18-7 Reproducible in staging?: Y Reproducible in production?: N If this was caught during regression testing, add the test name, ID and link from TestRail: N/A Email or phone of affected tester (no customers): applausetester+kh050806@applause.expensifail.com Issue reported by: Applause - Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to FAB > Split expense > Manual
3. Enter amount > Next
4. Select a workspace > Next
5. Split the expense

Expected Result:

The expense is split in the workspace chat

Actual Result:

User is unable to split expense with workspace via FAB App opens a report with RBR on LHN and the report shows infinite skeleton after splitting expense with workspace via FAB

Workaround:

Unknown

Platforms:

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

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/40534103-b2ea-4b92-8445-f731727704af

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01fde995cf07525892
• Upwork Job ID: 1821964572979581542
• Last Price Increase: 2024-08-09
• Automatic offers:
• hoangzinh | Reviewer | 103497009

Issue Owner

Current Issue Owner: @hoangzinh

[melvin-bot[bot]]

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

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

[lanitochka17]

We think that this bug might be related to #vip-split

[bernhardoj]

Edited by proposal-police: This proposal was edited at {current_timestamp}.

Proposal

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

Can't split bill with workspace from FAB.

What is the root cause of that problem?

When we split bill with a workspace from FAB, we create a new workspace chat even though there is already a workspace chat. The reason we create a new workspace is because the existing chat report ID below is empty, so getOrCreateOptimisticSplitChatReport decide to create a new report. https://github.com/Expensify/App/blob/db669c4d45ec14c849fc5f84af06e3d8564e58b9/src/libs/actions/IOU.ts#L3812-L3814

Previously, the logic look like this:

const existingChatReportID = existingSplitChatReportID || participants[0].reportID

When we split bill from FAB, existingSplitChatReportID will be an empty string and if the participant is a workspace, then participants[0].reportID will contain the workspace reportID.

But now, we use ?? which won't fallback to participants[0].reportID because existingSplitChatReportID is an empty string. The logic is updated by this commit.

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

Based on the commit message, we don't want an undefined existingChatReportID which is possible if the participant is not a workspace.

const existingChatReportID = '' || undefined => undefined

That's why we fall back again to an empty string. But we shouldn't use ?? specifically for existingSplitChatReportID because it's an empty string and not undefined.

const existingChatReportID = '' ?? {reportID} ?? '' => ''

So, to fix this, we should change ?? to || back.

const existingChatReportID = existingSplitChatReportID || participants[0].reportID || '';

Btw, I think we should fall back to -1 or just let it be undefined again. If it's an empty string, then the report onyx key will be report_ which will fetch the whole report collection. https://github.com/Expensify/App/blob/db669c4d45ec14c849fc5f84af06e3d8564e58b9/src/libs/actions/IOU.ts#L3817

I think the reason we don't want the reportID to be undefined (report_undefined) is that sometimes the invalid report exists in the storage with error data, without a reportID.

report_undefined: {
    errors: ...,
}

And because the report object is not undefined, the logic thinks that it found an existing report, even though it doesn't. https://github.com/Expensify/App/blob/db669c4d45ec14c849fc5f84af06e3d8564e58b9/src/libs/actions/IOU.ts#L3820-L3828

To fix this, we can check for the reportID to make sure the report object is valid.

if (!existingSplitChatReport?.reportID) {
    existingSplitChatReport = ReportUtils.getChatByParticipants(allParticipantsAccountIDs, undefined, participantAccountIDs.length > 1);
}

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[grgia]

@hoangzinh could you assess the proposal, I think we can assign @bernhardoj

[thienlnam]

@grgia Did you mean to remove the App blocker? Looks like this is a FE issue

[melvin-bot[bot]]

Current assignee @grgia is eligible for the DeployBlockerCash assigner, not assigning anyone new.

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

[hoangzinh]

Btw, I think we should fall back to -1 or just let it be undefined again. If it's an empty string, then the report onyx key will be report_ which will fetch the whole report collection.

@bernhardoj because we're getting the report from an array, therefore if it's an empty string, it won't return any data, will it?

https://github.com/Expensify/App/blob/93dc1c91437ea8cb9bd48a2a65f395e6da3c678b/src/libs/actions/IOU.ts#L3816-L3817

Btw, I think it's safer if we check existingChatReportID exists before retrieving existingSplitChatReport from array. What do you think?

[bernhardoj]

because we're getting the report from an array

ReportConnection.getAllReports() doesn't return an array, it's an object.

Btw, I think it's safer if we check existingChatReportID exists before retrieving existingSplitChatReport from array. What do you think?

That's a good idea. I remember we did that before, but it was changed in this PR

[hoangzinh]

Oh my bad, it's an object.

I remember we did that before, but it was changed in this PR

It appears that there is a commit https://github.com/Expensify/App/commit/1443b2645869fbd3e30ad0856de9b710ece794db from that PR to fix disabling Eslint comment. Btw, because it's a DB, I'm happy with either way

@bernhardoj's proposal https://github.com/Expensify/App/issues/47159#issuecomment-2278021011 looks good to me

🎀👀🎀 C+ reviewed

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

Offer link Upwork job

[Beamanator]

@bernhardoj if you could please make this fix a priority, that would be great!

[Beamanator]

Actually if this was really caused by https://github.com/Expensify/App/pull/42302, this shouldn't be a blocker anymore since https://github.com/Expensify/App/pull/42302 is now in prod 😅

[melvin-bot[bot]]

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

[bernhardoj]

PR is ready

cc: @hoangzinh

[joekaufmanexpensify]

PR merged!

[joekaufmanexpensify]

Hmm, automation did not work here. This went out to prod over a week ago, so payment is due now. Will tackle payment in the next day or so!

[hoangzinh]

BugZero Checklist:

• [x] The PR that introduced the bug has been identified. Link to the PR: https://github.com/Expensify/App/pull/42302
• [x] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: https://github.com/Expensify/App/pull/42302/files#r1732660564
• [x] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
• [x] Determine if we should create a regression test for this bug: 🚫 No, because it was a DB, I think we already have a regression test for this bug.

[joekaufmanexpensify]

Sounds good. TY!

[joekaufmanexpensify]

All set to issue payment here. We need to pay:

• @hoangzinh - $250 for C+ via Upwork.
• @bernhardoj - $250 for PR via NewDot.

[joekaufmanexpensify]

@hoangzinh $250 sent and contract ended!

[joekaufmanexpensify]

Upwork job closed.

[joekaufmanexpensify]

Closing as only thing left here is for @bernhardoj to request payment via NewDot!

[bernhardoj]

Requested in ND.

[JmillsExpensify]

$250 approved for @bernhardoj