search

Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.57k stars 2.92k forks source link

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

Open IuliiaHerets opened 5 days ago

IuliiaHerets commented 5 days ago

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.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): applausetester+21425443235@applause.expensifail.com Issue reported by: Applause Internal Team

Action Performed:

Precondition:

  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:

Screenshots/Videos

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

View all open jobs on GitHub

melvin-bot[bot] commented 5 days ago

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 commented 5 days ago

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 commented 5 days ago

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.