[HOLD E/E #359760] [$500] No "Hold Request" and "Add a receipt" option on workspace expense report as Admin

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Issue found when executing #35744 Version Number: 1.4.39-0 Reproducible in staging?: y Reproducible in production?: n If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: Applause internal team Slack conversation:

Action Performed:

1. Create a new expense request on a collect policy
2. Submit the expense
3. Open the expense report
4. Right-click/long-press the expense preview
5. (if employee) Verify there is no Delete Request (if admin) Verify there is a Delete Request option
6. Press the expense preview to open the transaction thread
7. Press the three-dots button
8. (if employee) Verify there is no Delete Request and Add receipt (if admin) Verify there is a Delete Request and Add receipt option

Expected Result:

There should be Hold Request and Add a receipt option on Admins side

Actual Result:

No "Hold Request" and "Add a receipt" option on workspace expense report as Admin

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

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/Expensify/App/assets/43996225/c5d1909f-974f-4739-a770-5959a320a8d7

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01c35580d297f84569
• Upwork Job ID: 1755778907181215744
• Last Price Increase: 2024-03-08

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

Triggered auto assignment to @MitchExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 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.

[melvin-bot[bot]]

Auto-assign attempt failed, all eligible assignees are OOO.

[bernhardoj]

As the test step coming from my PR, I would like to clarify one thing. The delete request and add receipt option will be shown if the user is an admin AND the action/request owner, so this is not a bug.

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

[hoangzinh]

The delete request and add receipt option will be shown if the user is an admin AND the action/request owner, so this is not a bug.

@bernhardoj could you share where did we discuss about this behavior? Because I think the Admin should have permission to delete request option, shouldn't they?

[bernhardoj]

The action owner condition has existed for quite some time https://github.com/Expensify/App/blob/9463e22ba5c0897d519c0ccc85c12ae591742541/src/libs/ReportUtils.ts#L1239

and I added the admin condition based on this comment

[hoangzinh]

Thank @bernhardoj. @MitchExpensify what do you think about our new expected behavior?

[marcaaron]

Create a new expense request on a collect policy

Gonna mark this as a non-blocker as people on Collect policies are not actually using these flows yet

[melvin-bot[bot]]

@hoangzinh, @MitchExpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[MitchExpensify]

Where is the option to "Delete Request" in your video @kavimuru ? Are both screens from the admins perspective?

[MitchExpensify]

This seems pretty clearly wave 6, right @greg-schroeder?

[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? 💸

[MitchExpensify]

Friendly bump @greg-schroeder

[greg-schroeder]

Looking

[greg-schroeder]

I think Wave 8 is correct, as the behavior that doesn't match expected is tied to the admin's experience ... if the submitter experience is working as expected, it wouldn't fit in Wave 6's current scope

[melvin-bot[bot]]

@hoangzinh, @MitchExpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot[bot]]

@hoangzinh @MitchExpensify this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[hoangzinh]

@greg-schroeder @MitchExpensify Sorry but what is the correct expected behavior for this issue? Thanks

[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? 💸

[brandonhenry]

Proposal

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

The issue is that administrators do not have the options to "Delete Request" and "Add a receipt" on workspace expense reports. This functionality is expected to be available for administrators to manage expenses efficiently.

What is the root cause of that problem?

The root cause is a missing check to determine if the user is an admin before displaying the "Delete Request" and "Add a receipt" options in the ContextMenuActions. Specifically, the logic to filter context menu actions does not currently consider the user's role when determining which actions to show.

https://github.com/Expensify/App/blob/5eed6322bd2e559c684481e8bc0122b0825779e7/src/pages/home/report/ContextMenu/BaseReportActionContextMenu.tsx#L128

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

To solve this problem, we should update the Delete Request and Add a Receipt ContextMenuActions shouldShow logic to include a check for whether the user is an admin when deciding to show the "Delete Request" and "Add a receipt" options.

https://github.com/Expensify/App/blob/5eed6322bd2e559c684481e8bc0122b0825779e7/src/pages/home/report/ContextMenu/ContextMenuActions.tsx#L412

I am seeing a repeating need to know if the current user is an admin for a specific report, we should add that util.

This involves creating a new function isReportAdmin in ReportUtils that checks if the user is an admin of the policy on a given report ID

1. Add a new utility function in ReportUtils to determine if a user can delete a report action:

// In ReportUtils.js
function isReportAdmin(reportID: string): boolean {
    const report = getReport(reportID); 
    const policy = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`] ?? null;
    return policy?.role === CONST.POLICY.ROLE.ADMIN && !isEmptyObject(report) && !isDM(report);
}

2. Use this utility function in ContextMenuActions to conditionally show "Delete Request" and "Add a receipt" options based on if you are an admin or not

// Modify the shouldShow function for the relevant actions in ContextMenuActions.js
{
    // For "Delete Request" action
    isAnonymousAction: false,
    textTranslateKey: 'reportActionContextMenu.deleteAction',
    icon: Expensicons.Trashcan,
    shouldShow: (type, reportAction, isArchivedRoom, betas, menuTarget, isChronosReport, reportID) =>
        // Until deleting parent threads is supported in FE, we will prevent the user from deleting a thread parent
        type === CONST.CONTEXT_MENU_TYPES.REPORT_ACTION &&
        ReportUtils.isReportAdmin(reportID) &&
        !isArchivedRoom &&
        !isChronosReport &&
        !ReportActionsUtils.isMessageDeleted(reportAction),
}

This approach ensures that only admins can see and use the "Delete Request" and "Add a receipt" options, aligning with the expected results of the original post

[Victor-Nyagudi]

I'm confused on the expected result here.

• Why would the admin have an "Add receipt" option for a money request they didn't make/submit? Shouldn't the person who submitted the request be the only one to add a receipt because they probably are the only one who has it?

• Giving the admin the ability to delete money requests they didn't make sounds a bit suspicious. That means they could go about deleting any money request in the WS.

As the test step coming from my PR, I would like to clarify one thing. The delete request and add receipt option will be shown if the user is an admin AND the action/request owner, so this is not a bug.

This sounds about right.

[MitchExpensify]

I'm actually not 100% on the expected behavior for admins viewing an expense - @greg-schroeder do you know if admins should in fact be able to Add a receipt or Delete?

[melvin-bot[bot]]

@hoangzinh @MitchExpensify this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[melvin-bot[bot]]

@hoangzinh, @MitchExpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[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? 💸

[MitchExpensify]

@greg-schroeder do you know if admins should in fact be able to Add a receipt or Delete?

Friendly bump @greg-schroeder

[MitchExpensify]

do you know if admins should in fact be able to Add a receipt or Delete?

@trjExpensify are you able to confirm this perhaps?

[hoangzinh]

@MitchExpensify we can put this issue hold on for https://github.com/Expensify/App/pull/36388. As @youssef-lr mentioned here https://github.com/Expensify/App/pull/36388#discussion_r1506888099 => new behavior should be: we only display delete request if current user is action/request owner

[trjExpensify]

@trjExpensify are you able to confirm this perhaps?

To answer your question, no they shouldn't be able to delete someone else's request, only put it on hold.

[youssef-lr]

@trjExpensify we don't allow modifying receipts for admins as well, so I'm thinking we can just close this issue?

[trjExpensify]

Hm, you sure? I thought an admin/approver on a workspace can attach a receipt for a submitter today on OldDot?

[youssef-lr]

Ah okay. I though since they can't edit a receipt they can't attach one either. What's the use case for this though? I think I agree with @Victor-Nyagudi here

[JmillsExpensify]

Why would the admin have an "Add receipt" option for a money request they didn't make/submit? Shouldn't the person who submitted the request be the only one to add a receipt because they probably are the only one who has it?

This is a pretty common use case, especially for employees who are provided a corporate card and the admin has access to the service/receipts. In fact, in some companies employees are instructed to add no receipts, the accounting team handles it for them. I think we need to support this case in order to continue supporting our customers.

[youssef-lr]

Interesting, thanks for clarifying @JmillsExpensify! So with that in mind, should we also allow admins to replace receipts? Because currently we don't.

[youssef-lr]

Also, since this falls under editing expenses in general, I think we can have this on hold on my PR, because chances are I'll have to update the changes from this issue this later on

[JmillsExpensify]

Interesting, thanks for clarifying @JmillsExpensify! So with that in mind, should we also allow admins to replace receipts? Because currently we don't.

We don't allow this, on the logic that admins should be able to add a receipt is none exists. If one does, then they shouldn't be able to "delete" one receipt in order to add a new one. So that's why we allow adding a receipt but not replacing.

[hoangzinh]

Hi team, just wanna sum up if I understand correctly

• "Delete Request" option => employee who is action/request owner
• "Add a receipt" option => employee who is action/request owner or admins/approvers on that workspace

[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? 💸

[melvin-bot[bot]]

@hoangzinh @MitchExpensify this issue is now 4 weeks old and preventing us from maintaining WAQ, can you:

• Decide whether any proposals currently meet our guidelines and can be approved as-is today
• If no proposals meet that standard, please take this issue internal and treat it as one of your highest priorities
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[melvin-bot[bot]]

Current assignee @hoangzinh is eligible for the Internal assigner, not assigning anyone new.

[youssef-lr]

Assigning myself to work on this internally as part of a PR that touches this flow.

[youssef-lr]

Not overdue, still in the works.

[melvin-bot[bot]]

@hoangzinh, @youssef-lr, @MitchExpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[youssef-lr]

Still held.

[melvin-bot[bot]]

@hoangzinh, @youssef-lr, @MitchExpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[youssef-lr]

Same. We're getting close to getting backend PRs merged. Once done the App PR should be merged and fix this issue.

[MitchExpensify]

Almost ready to un-hold @youssef-lr ?