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

[lanitochka17]

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.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): [email protected]
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?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6622555_1727899943377.RPReplay_Final1727899518.mp4

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-11-05

Issue Owner

Current Issue Owner: @OfstadC

[melvin-bot]

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]

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

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

[Krishna2323]

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?

• readOnly prop is not properly set to handle the case where admin/owner cannot edit the reciept.
  

  App/src/components/ReportActionItem/MoneyRequestView.tsx

  Line 528 in 19a137d

  readonly={readonly}

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


• We should update it to readonly={readonly || !canEditReceipt}.
• We also need to remove the isAdmin || isManager || part from the condition below, if we want, we can keep isManager.
  

  App/src/libs/ReportUtils.ts

  Lines 3098 to 3106 in 19a137d

  	if (fieldToEdit === CONST.EDIT_REQUEST_FIELD.RECEIPT) {
  	const isRequestor = currentUserAccountID === reportAction?.actorAccountID;
  	return (
  	!isInvoiceReport(moneyRequestReport) &&
  	!TransactionUtils.isReceiptBeingScanned(transaction) &&
  	!TransactionUtils.isDistanceRequest(transaction) &&
  	(isAdmin || isManager || isRequestor)
  	);
  	}

What alternative solutions did you explore? (Optional)

Result

[daledah]

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:

App/src/libs/ReportUtils.ts

Lines 3098 to 3106 in 7992d21

	if (fieldToEdit === CONST.EDIT_REQUEST_FIELD.RECEIPT) {
	const isRequestor = currentUserAccountID === reportAction?.actorAccountID;
	return (
	!isInvoiceReport(moneyRequestReport) &&
	!TransactionUtils.isReceiptBeingScanned(transaction) &&
	!TransactionUtils.isDistanceRequest(transaction) &&
	(isAdmin || isManager || isRequestor)
	);
	}

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:

App/src/libs/ReportUtils.ts

Lines 3098 to 3106 in 7992d21

	if (fieldToEdit === CONST.EDIT_REQUEST_FIELD.RECEIPT) {
	const isRequestor = currentUserAccountID === reportAction?.actorAccountID;
	return (
	!isInvoiceReport(moneyRequestReport) &&
	!TransactionUtils.isReceiptBeingScanned(transaction) &&
	!TransactionUtils.isDistanceRequest(transaction) &&
	(isAdmin || isManager || isRequestor)
	);
	}

What alternative solutions did you explore? (Optional)

NA

[abzokhattab]

Dupe of #47242

[Nodebrute]

Moving my proposal from here #47242 (comment)

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

App/src/libs/ReportUtils.ts

Line 2934 in 7481b6e

(isAdmin || isManager || isRequestor)

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

We can remove isAdmin from here

App/src/libs/ReportUtils.ts

Line 2934 in 7481b6e

(isAdmin || isManager || isRequestor)

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]

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

[melvin-bot]

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

[Ollyws]

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

[melvin-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]

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

[melvin-bot]

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

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

[daledah]

@Ollyws I can no longer reproduce the bug.

Screen.Recording.2024-10-17.at.17.17.18.mov

[melvin-bot]

@Ollyws, @OfstadC Huh... This is 4 days overdue. Who can take care of this?

[OfstadC]

Friendly bump @Ollyws 😃

[melvin-bot]

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

[Ollyws]

Apologies for the delay, looking into this...

[Ollyws]

I think as @Nodebrute did post their proposal first in the dupe issue #47242 (comment) it's fair that we go with that.

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[tgolen]

I think those actions are there intentionally, and that the idea was always that an admin should be able to do those things. So, if there is an error happening, I think we should dig into those errors and fix them, rather than removing the functionality.

@JmillsExpensify does this ring a bell for you and am I remembering this correctly?

[tgolen]

@lanitochka17 I confirmed that the frontend functionality is correct. Can you please update this issue to only focus on fixing the errors that happen? If this needs to be handled internally, that's fine, but I'd like to get some more information for this such as the logs for the delete/replace requests that have the errors.

@OfstadC If you're able to reproduce this, could you please try to get the requestID for me following this SO?

[tgolen]

Bump @lanitochka17 @OfstadC

[OfstadC]

Sorry @tgolen ! Was OoO - looking now 😃

[OfstadC]

@tgolen I was able to reproduce today

jsonCode
:
405
message
:
"405 Cannot detach receipt from unowned transaction"
onyxData
:
[]
requestID
:
"8e109b54d92daccc-MSP"

[tgolen]

Awesome! Here are the logs for that request.

It looks like that error is a very old error (at least 7 years old). So, it looks like in order for us to allow that operation, we need to make an update to Auth.

Can you please go here and open up a new issue using the "Open an internal issue for a backend fix" template with all the relevant info? Then it will get assigned to an engineer to fix.

[OfstadC]

Done! Thanks @tgolen for your help!

[melvin-bot]

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

[OfstadC]

Discussion in linked GH

[OfstadC]

Jules is assigned to E/E issue 😃