[$250] Held expense - Hold option is missing when submitting expense to workspace offline as employee

[IuliiaHerets]

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: 9.0.64-4 Reproducible in staging?: Y Reproducible in production?: Y If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y Email or phone of affected tester (no customers): [email protected] Issue reported by: Applause Internal Team

Action Performed:

Precondition:

• User is invited to a workspace.

1. Go to staging.new.expensify.com
2. Go to workspace chat.
3. Go offline.
4. Submit a manual expense to the workspace chat.
5. Click on the expense preview.
6. Click on the report header.

Expected Result:

Hold option will be present when submitting expense to workspace chat offline as employee.

Actual Result:

Hold option is missing when submitting expense to workspace chat offline as employee.

This issue is not reproducible when admin submits expense to workspace chat offline.

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

https://github.com/user-attachments/assets/0b9d7a9a-729a-4250-b45d-d41ff926dcbb

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021863711101687032670
• Upwork Job ID: 1863711101687032670
• Last Price Increase: 2024-12-02
• Automatic offers:
• FitseTLT | Contributor | 105258352

Issue Owner

Current Issue Owner: @jjcoffee

[melvin-bot[bot]]

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

[nkdengineer]

Proposal

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

Hold option is missing when submitting expense to workspace chat offline as employee.

What is the root cause of that problem?

In case offline, we set reportActorAccountID as managerID in here

https://github.com/Expensify/App/blob/2d2555b173b47c0bce0b1c16b275f1f98639664c/src/libs/ReportUtils.ts#L5152

This leads to isActionOwner = false here

https://github.com/Expensify/App/blob/abba10a7277880c487ef67486239f4f9e3b0c2f7/src/libs/ReportUtils.ts#L3318-L3321

=> canModifyStatus = false => canHoldRequest = false

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

1. We should add new field as actorAccountID = currentUserAccountID to buildOptimisticExpenseReport function here

2. Update this condition to align with what the BE is returning.

    const reportActorAccountID = (isInvoiceReport(iouReport) ? iouReport?.ownerAccountID : iouReport?.actorAccountID) ?? -1;

What alternative solutions did you explore? (Optional)

NA

[FitseTLT]

Proposal

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

Held expense - Hold option is missing when submitting expense to workspace offline as employee

What is the root cause of that problem?

For non-invoice reports we set the report preview action's actorAccountID to the managerID https://github.com/Expensify/App/blob/2d2555b173b47c0bce0b1c16b275f1f98639664c/src/libs/ReportUtils.ts#L5152 So when the current user is not the managerID of the expense report (for instance, when the managerID is set to the approver) isActionOwner will be false here https://github.com/Expensify/App/blob/2d2555b173b47c0bce0b1c16b275f1f98639664c/src/libs/ReportUtils.ts#L3328 and canModifyStatus will be false for the non-admin user (that's why the current issue is visible only when creating expenses as non-admin employee) and canHoldRequest will be false until the correct actorAccountID is set by the BE

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

We should set the correct actorAccountID for the previewAction that aligns with the BE which is the currentUserAccountID or the expense report ownerAccountID(payeeAccountID). We can easily set it to the the expense report's ownerAccountID adding isExpenseReport condition here https://github.com/Expensify/App/blob/2d2555b173b47c0bce0b1c16b275f1f98639664c/src/libs/ReportUtils.ts#L5152

    const reportActorAccountID = (isInvoiceReport(iouReport) || isExpenseReport(iouReport) ? iouReport?.ownerAccountID : iouReport?.managerID) ?? -1;

Note: applying the solution in the above proposal will make the actorAccountID to be set to -1 optimistically as IOU report's actorAccountID do not exist neither in iou reports nor in expense reports.

What alternative solutions did you explore? (Optional)

or we can directly set the currenUserAccountID/payeeAccountID for expense reports case by passing it to buildOptimisticReportPreview as a param for expense reports only.

[melvin-bot[bot]]

@strepanier03 Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot[bot]]

@strepanier03 Still overdue 6 days?! Let's take care of this!

[melvin-bot[bot]]

@strepanier03 10 days overdue. I'm getting more depressed than Marvin.

[strepanier03]

I'm unable to reproduce this, the Hold button is visible on my Pixel 8 while Offline and submitting expenses as an employee.

[FitseTLT]

You need to submit the expense offline @strepanier03

[strepanier03]

I did that @FitseTLT, I've run through the steps a few times now.

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[strepanier03]

@jjcoffee - Can you please try to repro this and if you're able proceed as normal with reviewing proposals. Thanks!

[MelvinBot]

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

[FitseTLT]

I did that @FitseTLT, I've run through the steps a few times now.

@strepanier03 You have to create the expense offline from the employee side that's the trick, from the workspace owner side it works fine. Here is a demo.

https://github.com/user-attachments/assets/f050d919-b854-4b05-b337-31254b20d94b

[jjcoffee]

I am able to repro too - it needs to be on a fresh expense though, otherwise the reportActorAccountID will already be correct. It also happens online if the request is slow.

[jjcoffee]

@nkdengineer's proposal LGTM overall, but I'm a bit uncertain on a few points.

We should add new field as actorAccountID = currentUserAccountID to buildOptimisticExpenseReport function here

It's unclear how this relates to the RCA, can you clarify why this will solve the particular case?

Update this condition to align with what the BE is returning.

What is BE returning?

[jjcoffee]

@FitseTLT's proposal is also a bit confusing on details.

when the current user is not the managerID of the expense report (for instance, when the managerID is set to the approver) isActionOwner will be false

This sounds like there's a specific case where the issue happens, but I don't think that's the case? Won't isActionOwner always be false optimistically for non-invoice reports?

We should set the correct actorAccountID for the previewAction that aligns with the BE which is the currentUserAccountID or the expense report ownerAccountID (payeeAccountID).

What's the fallback (iouReport?.managerID) actually for then? i.e. what case does it handle?

Note: applying the solution in the above proposal will make the actorAccountID to be set to -1 optimistically as IOU report's actorAccountID do not exist neither in iou reports nor in expense reports.

The proposal is to add actorAccountID as a new param, so that's why it doesn't exist (yet) :wink:

[jjcoffee]

Looks like we tried to switch to ownerAccountID in https://github.com/Expensify/App/commit/b704277034c4e9dad12c0d2e09d3820458dc2823 but this was reverted, so we've gotta be careful here.

[FitseTLT]

What's the fallback (iouReport?.managerID) actually for then? i.e. what case does it handle?

@jjcoffee for IOU reports. Note that this function is used for iou reports as well as expense reports.

[FitseTLT]

The proposal is to add actorAccountID as a new param, so that's why it doesn't exist (yet) 😉

Yeah and actorAccountID also doesn't exist in expense reports data returned from BE.

when the current user is not the managerID of the expense report (for instance, when the managerID is set to the approver) isActionOwner will be false

This sounds like there's a specific case where the issue happens, but I don't think that's the case? Won't isActionOwner always be false optimistically for non-invoice reports?

I am only explaining how hold option was not available for current issue @jjcoffee and there is a specific case the issue happens because from admin side for instance the issue is not reproducible because the condition passes as the isAdmin it true even if the isActionOwner is false. https://github.com/Expensify/App/blob/2d2555b173b47c0bce0b1c16b275f1f98639664c/src/libs/ReportUtils.ts#L3328 That is what I was trying to explain.

For expense reports we should set the actorAccountID to iouReport?.ownerAccountID (iouReport in this case is the expense report param passed to the function BTW)

[jjcoffee]

@nkdengineer or @FitseTLT any comment on this?

I'm a bit unsure if we can always rely on ownerAccountID given it was reverted before. Maybe @grgia can remember from several months back? :sweat_smile:

[FitseTLT]

@nkdengineer or @FitseTLT any comment on this?

I'm a bit unsure if we can always rely on ownerAccountID given it was reverted before. Maybe @grgia can remember from several months back? 😅

@jjcoffee for iou reports it should be set to the managerID but for expense reports it should be set to ownerAccountID. What we are fixing here is the expense report case only and I have debugged and confirmed what the BE is setting for both cases, I am pretty sure 👍

[jjcoffee]

Alright, you've convinced me :wink: Let's go with @FitseTLT's proposal for now. We can deal with any issues on the PR.

:ribbon::eyes::ribbon: C+ reviewed

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[grgia]

All yours! @FitseTLT

[melvin-bot[bot]]

@strepanier03, @jjcoffee, @grgia, @FitseTLT Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[jjcoffee]

Not overdue - just waiting for @FitseTLT to raise a PR :pray:

[melvin-bot[bot]]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot[bot]]

The solution for this issue has been :rocket: deployed to production :rocket: in version 9.0.78-6 and is now subject to a 7-day regression period :calendar:. Here is the list of pull requests that resolve this issue:

• https://github.com/Expensify/App/pull/53944

If no regressions arise, payment will be issued on 2025-01-02. :confetti_ball:

For reference, here are some details about the assignees on this issue:

• @jjcoffee requires payment through NewDot Manual Requests
• @FitseTLT requires payment automatic offer (Contributor)