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.5k stars 2.85k forks source link

[$250] Scan - "Delete" and "Replace" options are displayed for an admin which causes an error #50113

Open lanitochka17 opened 3 weeks ago

lanitochka17 commented 3 weeks 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.43-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/5035807 Email or phone of affected tester (no customers): sustinov@applausemail.com Issue reported by: Applause - Internal Team

Action Performed:

Prerequisite Create a workspace and invite an employee to the workspace Login as an owner and employee

  1. As the employee navigate to the workspace chat and click on the + icon > Submit expense > Scan
  2. Upload a receipt and finish the flow
  3. After the expense is created navigate to the tab where you are logged in as the admin/owner
  4. Click on the receipt thumbnail
  5. Here notice that the "Delete" and "Replace" options are displayed
  6. Click on "Replace" > Choose another receipt or take a photo of it > Upload
  7. Click on "Delete" > Confirm > Notice that an error is thrown

Expected Result:

The workspace admin cannot edit the receipt file so there should not be "Delete" and "Replace" options

Actual Result:

The ‘Delete’ and ‘Replace’ options are displayed, but when ‘Delete’ is executed, an error occurs and when ‘Replace’ is executed, the receipt is replaced

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/user-attachments/assets/164a7d6d-600e-43b3-929c-a6811ec8c88a

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021843656533685450338
  • Upwork Job ID: 1843656533685450338
  • Last Price Increase: 2024-10-22
Issue OwnerCurrent Issue Owner: @OfstadC
melvin-bot[bot] commented 3 weeks ago

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

lanitochka17 commented 3 weeks ago

@OfstadC 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

lanitochka17 commented 3 weeks ago

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

Krishna2323 commented 3 weeks ago

Edited by proposal-police: This proposal was edited at 2024-10-02 20:36:01 UTC.

Proposal


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

Scan - "Delete" and "Replace" options are displayed for an admin which causes an error

What is the root cause of that problem?

What alternative solutions did you explore? (Optional)

Result

daledah commented 3 weeks ago

Proposal

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

The ‘Delete’ and ‘Replace’ options are displayed, but when ‘Delete’ is executed, an error occurs and when ‘Replace’ is executed, the receipt is replaced

What is the root cause of that problem?

In ReportUtils.canEditFieldOfMoneyRequest:

https://github.com/Expensify/App/blob/7992d213cae59eff0b6706b8871586e8d586df0a/src/libs/ReportUtils.ts#L3098-L3106

If user is admin or manager, this function returns true, so admins can edit receipt.

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

Remove isAdmin and isManager from this condition:

https://github.com/Expensify/App/blob/7992d213cae59eff0b6706b8871586e8d586df0a/src/libs/ReportUtils.ts#L3098-L3106

What alternative solutions did you explore? (Optional)

NA

abzokhattab commented 3 weeks ago

Dupe of https://github.com/Expensify/App/issues/47242

Nodebrute commented 3 weeks ago

Moving my proposal from here https://github.com/Expensify/App/issues/47242#issuecomment-2284327596

Proposal

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

"Delete" and "Replace" options are displayed for an admin which causes an error

What is the root cause of that problem?

We are allowing Admin to edit receipt here https://github.com/Expensify/App/blob/7481b6ec6c5d0238d3c4aa9cf6e65325bec5741d/src/libs/ReportUtils.ts#L2934

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

We can remove isAdmin from here https://github.com/Expensify/App/blob/7481b6ec6c5d0238d3c4aa9cf6e65325bec5741d/src/libs/ReportUtils.ts#L2934

What alternative solutions did you explore? (Optional)

In case we only want to allow requestor to delete and replace the reciept than we can remove both options isAdmin and isManager

melvin-bot[bot] commented 2 weeks ago

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

melvin-bot[bot] commented 2 weeks ago

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

Ollyws commented 2 weeks ago

Is this still reproducible for everyone else? I'm only getting the download option as admin/owner:

Screenshot 2024-10-10 at 10 06 01

melvin-bot[bot] commented 1 week ago

📣 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] commented 1 week ago

@Ollyws, @OfstadC Eep! 4 days overdue now. Issues have feelings too...

melvin-bot[bot] commented 1 week ago

@Ollyws @OfstadC 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!

Ollyws commented 1 week ago

@Nodebrute @abzokhattab @daledah are you still able to reproduce?

daledah commented 1 week ago

@Ollyws I can no longer reproduce the bug.

https://github.com/user-attachments/assets/95809a7c-f68c-42f4-a570-9afc386e340c

MelvinBot commented 1 week ago

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

melvin-bot[bot] commented 4 days ago

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

OfstadC commented 4 days ago

I've asked for retesting in #qa

melvin-bot[bot] commented 4 days ago

@Ollyws, @OfstadC Eep! 4 days overdue now. Issues have feelings too...

OfstadC commented 3 days ago

Waiting on QA to retest