[Held requests] [$250] Duplicate expense report displayed when employee submit expense

[IuliiaHerets]

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.21-0 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4863120&group_by=cases:section_id&group_order=asc&group_id=309128 Email or phone of affected tester (no customers): gocemate+workspaceemployee159@gmail.com Issue reported by: Applause Internal Team

Action Performed:

Precondition:

1. Login as the owner of the workspace
2. Create a workspace
3. Invite the approver and employee
4. Navigate to more features
5. Enable "workflows"
6. On the "Workflow" editor - enable "Add Approvals"
7. Set the Approver account as the Approver

Steps:

1. As employee submit 2 expenses to the workspace chat
2. As Approver Hold one of the expenses and partially approve the report
3. As employee submit another expense to the workspace chat

Expected Result:

There should be only one new expense present on workspace chat, along with expense that was put on hold

Actual Result:

Duplicate expense report displayed when employee submit another expense while there is a expense with hold status Workspace avatar turn to offline image

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

https://github.com/user-attachments/assets/4c96e2a5-3a89-4393-8858-32394214b60e

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01da56a170fe42f065
• Upwork Job ID: 1824490735723577485
• Last Price Increase: 2024-09-04

Issue Owner

Current Issue Owner: @

[melvin-bot[bot]]

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

[IuliiaHerets]

We think that this bug might be related to #wave-collect - Release 1

[IuliiaHerets]

@sakluger FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[excile1]

Proposal

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

After holding one of the submitted expenses and partially approving it, submitting a subsequent expense creates a duplicate report in the chat.

What is the root cause of that problem?

The root cause of this problem, unfortunately, seems to be the data sent from the API through Pusher. I explored the request that is being sent from the application, and it seems to be correct. The API response, though, includes both the correct data (the data that was submitted to the API) and also seems to include some extra "ghost" reportAction_{chatRoomID} and report_{chatRoomID} rows, that don't include the iouReportId and say that the workspace owes €0.00. And then this broken extra data gets pushed to the employee's side through Pusher, which is what causes the bug, because the subsequently submitted expenses can't link to the previous one without iouReportId.

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

It is possible to fix it through a workaround, like forcing it to ReportActions.openReport (or any other means to force it to reload the latest data), but it looks like the fix for the root cause must come from the API side.

What alternative solutions did you explore? (Optional)

(this is not an alternative solution, but just another thing I had to fix before being able to exactly reproduce this issue) When I tried to reproduce the bug first, I noticed that the behavior was a bit different from the one in the video. For some reason, on the build I compiled from the latest version on this repo, this very same issue (extra report appearing in chat) appears even when expenses are supposed to be grouped, without holding them or doing anything extra. For whatever reason, this doesn't seem to happen on the staging build, but I'll include the fix I had to make in here in case the Contributor+ or other Contributors run into the same issue.

This seems to be caused by the canAddOrDeleteTransactions function. Currently, if the report is an expense report and the current user account isn't a manager (which an employee in a workspace isn't), then it returns false and causes any new expenses to duplicate on the employee's side. It can be fixed by altering this if statement here: https://github.com/Expensify/App/blob/67ad466251078a3b54558a2ed84876ec2fe18608/src/libs/ReportUtils.ts#L1674

[melvin-bot[bot]]

@sakluger, @aimane-chnaif Whoops! This issue is 2 days overdue. Let's get this updated quick!

[aimane-chnaif]

@excile1 thanks for the proposal. We're not looking for workaround but fix the root cause.

[excile1]

@excile1 thanks for the proposal. We're not looking for workaround but fix the root cause.

Wouldn't the root cause have to be fixed on the backend side though?

So, in the approveMoneyRequest the optimisticData generated in the frontend looks good (with the correct owed amount in report preview and also iouReportID in the report)

The problem seems to appear in the request response's onyxData first, where it returns both the correct and expected response, but also returns the incorrect owes €0.00 report preview (which is the one that gets sent to the other user, which is why the issue seems to appear)

[aimane-chnaif]

Agree this is backend bug. 🎀👀🎀

[melvin-bot[bot]]

Triggered auto assignment to @Gonals, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[sakluger]

@Gonals can you check and confirm that this is a BE bug?

[trjExpensify]

CC: @robertjchen for eyes.

[robertjchen]

We had a fix go out earlier today that stops the inaccurate Onyx response- can we please re-test to see if this is still reproducible?

[Gonals]

@Expensify/applause, could we get a retest on this?

[kavimuru]

Bug is still reproducible.

https://github.com/user-attachments/assets/40d573b6-e767-452c-af66-90b8432a9df4

[sakluger]

@robertjchen what should our next step be on this one?

[melvin-bot[bot]]

@sakluger @Gonals @aimane-chnaif 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!

[robertjchen]

@Gonals let me know if you have any questions when investigating this 👍 unfortunately, the other fix did not help resovle this

[Gonals]

I'll take a closer look tomorrow!

[melvin-bot[bot]]

Current assignee @aimane-chnaif is eligible for the External assigner, not assigning anyone new.

[Gonals]

After further investigation, I believe this to be an external issue.

The new expense is part of the same report the HELD expense is, so the backend returns this correctly. The issue is that the frontend initially adds is separately to the chat, instead of adding it to the right report.

cc @aimane-chnaif

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[aimane-chnaif]

Awaiting new proposals

[hungvu193]

So I spent some time investigating this issue, here's some info that I found: After the approver partially approved the expense, ApproveMoneyRequest returned Onyx data to update iouReportID, which is the reportID of the current expense that the approver just approved.

Approving the full case is correct. However, with partial approval, that's wrong iouReportID. Why? Because that expense is marked as approved. Meanwhile, if that expense is approved partially, we should return the reportID of the remaining expense, aka the expense that's being held.

Next time, when the employee creates a new expense, we will get the report data from this iouReport here: https://github.com/Expensify/App/blob/4af620b439636f714ff72a26320ed57f9d2faac3/src/libs/actions/IOU.ts#L2042

And shouldCreateNewMoneyRequestReport here will return true, because that iouReport was already approved: https://github.com/Expensify/App/blob/4af620b439636f714ff72a26320ed57f9d2faac3/src/libs/actions/IOU.ts#L2045

And because shouldCreateNewMoneyRequestReport is true, we will create a new expense report instead of updating iouReport. https://github.com/Expensify/App/blob/4af620b439636f714ff72a26320ed57f9d2faac3/src/libs/actions/IOU.ts#L2047-L2050

Later when Pusher event arrived which lead to the duplicate bug that we saw in this issue.

So I think we can fix this one from our BE. If the Approver approves an expense partially, the iouReportID should be the expense that's being held.

https://github.com/user-attachments/assets/be6fa4e6-2713-4fb3-a1ca-d4af817e3a5e

[sakluger]

@hungvu193 thanks for the thorough investigation, I really appreciate it!

@Gonals - I know you said that you thought this was an external issue, but what do you think of @hungvu193's investigation and theory that this is a BE issue?

[robertjchen]

Appreciate the in-depth analysis, this definitely looks like a BE issue- it appears we're returning an Onyx update for the wrong report after the ApproveMoneyRequest involving a partial approval

[Gonals]

That ws a great investigation, @hungvu193. It seems we could fix this in the backend. I'm going to take a look at it!

[Gonals]

So... funny thing @hungvu193. I can't reproduce the issue in dev anymore, and the iouReport seems to be the correct one. Can you still reproduce the issue?

[hungvu193]

I still can reproduce it, make sure that the employee needs to submit the expense (not the owner) and the approver need to hold one of these expense before partial approve. IF you can't reproduce, try to repeat the steps 2,3 times.

https://github.com/user-attachments/assets/ab3f59d7-44f9-44c9-b229-8e885aabb548

[Gonals]

Hmmm. I still can't seem to reproduce it.

I gotta focus on some travel-related stuff for a day or two, but I'll come back to this and try again.

[Gonals]

I'll get back to this as soon as I can!

[sakluger]

@Gonals any updates? Have you been able to reproduce the issue?

[Gonals]

I haven't been able to get to it yet. Travel is taking priority (and my life). I'll get back to it as soon as I have any bandwidth

[Gonals]

Blocking some time for this tomorrow morning

[Gonals]

Managed to reproduce this in Staging, but not in Dev, which makes testing a bit hard. I DO have a theory on how to fix it, so I'll give that route a shot and see what happens

[trjExpensify]

Encouraging news!

[sakluger]

Looks like the PR is merged and deployed! Hopefully it fixes the bug.

[sakluger]

@IuliiaHerets would you mind giving this one a retest?

@aimane-chnaif do you think that the work you did reviewing proposals on this one was enough to warrant payment? Thanks!

[IuliiaHerets]

@sakluger This issue is not repro anymore, build - v9.0.41-2

https://github.com/user-attachments/assets/43b7ad2f-58b1-4178-96ea-7b91372f2964

[aimane-chnaif]

I am fine with no payment here. Let's close

[robertjchen]

Thank you! 🙇

[robertjchen]

Apparently still reproducible: https://expensify.slack.com/archives/C036QM0SLJK/p1727771195051329?thread_ts=1727720815.151679&cid=C036QM0SLJK

[sakluger]

@Gonals any ideas what might still be causing this?

[Gonals]

Dammit -.-

Well, I used my ideas in the previous fix. I'll try to reproduce in dev again (which I have not been able to so far). With that, I should be able to figure out what's up.

[Gonals]

Can reproduce on Staging. Still not in Dev, which makes it difficult to debug. I have a couple of ides that may help. I'll keep you posted

[Gonals]

Ok. This is getting overwritten by a "newer" Onyx Update, which wrongly sets the iouReportID to the oldOne. I'm digging out the origin.

[Gonals]

Still chasing this. It is NOT coming from this flow, but from a concurrent event, so it has become a way harder thing to chase. I think I have it surrounded, though

[Gonals]

Finally got a solution! Buuuut... it has unearthed another (existing) bug. I'll create an issue for it.

[Gonals]

New issue: https://github.com/Expensify/Auth/pull/12721

Fix for it already in review