[HOLD for payment 2024-08-13] [$250] mWeb - Workspace - In archived workspace, in iou details page sideways caret are shown for fields

[lanitochka17]

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.10
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to https://staging.new.expensify.com/home

2. Tap on a workspace chat

3. Submit an expense entering all fields

4. Navigate to LHN

5. Tap profile icon -- workspaces -- workspace

6. Tap on 3 dots icon next to workspace

7. Delete the workspace

8. Navigate to LHN

9. Open the archived workspace chat

10. Open the IOU

11. Open amount and description field using sideways caret

12. Note hhm not here page is shown

Expected Result:

Like in task details page, sideways caret must not be shown in iou details page in archived workspace

Actual Result:

In archived workspace, in iou details page sideways caret for amount & description field are shown directing to hhm not here page

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

Bug6547855_1721460264010.Screenrecorder-2024-07-20-12-37-14-543_compress_1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~019bd42da3fc0d7c59
• Upwork Job ID: 1815412203411994715
• Last Price Increase: 2024-07-22
• Automatic offers:
• dominictb | Contributor | 103308909

Issue Owner

Current Issue Owner: @joekaufmanexpensify

[melvin-bot]

Triggered auto assignment to @joekaufmanexpensify (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]

@joekaufmanexpensify 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 - Выпуск 1

[Krishna2323]

Proposal

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

mWeb - Workspace - In archived workspace, in iou details page sideways caret are shown for fields

What is the root cause of that problem?

• We aren't checking for archived room in canEditMoneyRequest util function.
  

  App/src/libs/ReportUtils.ts

  Line 2667 in f3a8f73

  function canEditMoneyRequest(reportAction: OnyxInputOrEntry<ReportAction<typeof CONST.REPORT.ACTIONS.TYPE.IOU>>): boolean {

• There is one more issue in isDeletedAction util function, it doesn't check if the action does has pending delete action.
  

  App/src/libs/ReportActionsUtils.ts

  Lines 113 to 124 in f3a8f73

  	function isDeletedAction(reportAction: OnyxInputOrEntry<ReportAction | OptimisticIOUReportAction>): boolean {
  	const message = reportAction?.message ?? [];
  	if (!Array.isArray(message)) {
  	return message?.html === '' ?? message?.deleted;
  	}
  	// A legacy deleted comment has either an empty array or an object with html field with empty string as value
  	const isLegacyDeletedComment = message.length === 0 || message[0]?.html === '';
  	return isLegacyDeletedComment || !!message[0]?.deleted;
  	}

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

• Add archived room check in canEditMoneyRequest and return false if repot is archived.

    if (isArchivedRoom(moneyRequestReport, getReportNameValuePairs(moneyRequestReport?.reportID))) {
        return false;
    }

• Update isDeletedAction to also check for pending action and return true if reportAction?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE is true.

    if (!reportAction || reportAction?.pendingAction === CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE) {
        return true;
    }

What alternative solutions did you explore? (Optional)

[dominictb]

Proposal

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

• In archived workspace, in iou details page sideways caret for amount & description field are shown directing to hhm not here page

What is the root cause of that problem?

• In each field in money request view, we allow user to click on it if user can edit that field and that field is not readonly. For example, the amount field:
  

  App/src/components/ReportActionItem/MoneyRequestView.tsx

  Line 492 in f3a8f73

  interactive={canEditAmount && !readonly}

• But, in IOURequestStepAmount, beside the above check, we use additional check in withWritableReportOrNotFound, which will display the not found page if:
  

  App/src/pages/iou/request/step/withWritableReportOrNotFound.tsx

  Line 62 in f3a8f73

  const canUserPerformWriteAction = ReportUtils.canUserPerformWriteAction(report);

  
  is false.

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

• For each field in money request view, we should add an additional check like:
  

  App/src/pages/iou/request/step/withWritableReportOrNotFound.tsx

  Line 62 in f3a8f73

  const canUserPerformWriteAction = ReportUtils.canUserPerformWriteAction(report);

• For example, with amount field:
  

  App/src/components/ReportActionItem/MoneyRequestView.tsx

  Line 506 in f3a8f73

  interactive={canEdit && !readonly}

  
  will become:

                        interactive={canUserPerformWriteAction && canEditAmount && !readonly}
                        shouldShowRightIcon={canUserPerformWriteAction && canEditAmount}

with:

    const canUserPerformWriteAction = ReportUtils.canUserPerformWriteAction(report);

[Krishna2323]

Proposal Updated

• Found one more issue in isDeletedAction util function and added RCA and solution for that.

[joekaufmanexpensify]

This doesn't seem to be happening only on Android. I reproduced it on Chrome web. I also don't think the task steps are necessary for reproducing. They help show that you can't edit fields on a task with an archived workspace, but I removed them from OP to make the bug easier to reproduce.

[joekaufmanexpensify]

Makes sense to me that we wouldn't allow you to edit any fields on an expense when the workspace it's reported on is archived.

[melvin-bot]

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

[melvin-bot]

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

[Tony-MK]

Proposal

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

mWeb - Workspace - In archived workspace, in iou details page sideways caret are shown for fields

What is the root cause of that problem?

The root cause of the problem is that canEditMoneyRequest function will return true in this case when the policy is not found and isManager is true to allow the user to edit an expense report even if it is a state higher than OPEN (0).

App/src/libs/ReportUtils.ts

Lines 2717 to 2719 in 38cddee

	// Admin & managers can always edit coding fields such as tag, category, billable, etc. As long as the report has a state higher than OPEN.
	if ((isAdmin || isManager) && !isOpenExpenseReport(moneyRequestReport)) {
	return true;

However, if the expense is not open, the canEditMoneyRequest function returns true to allow the user still to edit the description, category, or tag sections.

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

Let's check if the moneyRequestReport is not archived on the line.

App/src/libs/ReportUtils.ts

Line 2719 in 38cddee

return true;

return !isArchivedRoom(moneyRequestReport);

Just like how a user can not edit a task from an archived report.

Screen.Recording.2024-07-23.at.10.53.31.mov

[joekaufmanexpensify]

Pending review of proposals

[hungvu193]

I'll take a look tomorrow morning

[melvin-bot]

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

[dominictb]

@hungvu193 Any feedback about my solution? Using ReportUtils.canUserPerformWriteAction(report) will make the logic consistency and make sure we do not encounter similar issue in future. Thanks

[dominictb]

@hungvu193 FYI, your selected proposal does not fix the case that the expense report failed to create like below:

Screen.Recording.2024-07-24.at.11.25.09.mov

or maybe I do not understand what that proposal's author means. Please correct me if I was wrong.

[hungvu193]

Thanks for the information, IIRC we still allow user to edit information with error report.
Can you please confirm? @joekaufmanexpensify

[dominictb]

FYI, I tried to edit any field in expense that has an error, and here is what BE returns:

[hungvu193]

I see your point here, no need to discuss further, let wait for this confirmation. Thanks

[cristipaval]

@hungvu193 please tag me again after you clarify. Thanks!

[joekaufmanexpensify]

Thanks for the information, IIRC we still allow user to edit information with error report.
Can you please confirm?

Is the question whether it's expected that you can edit an expense when it has an error like that and the workspace is not archived? Or when the workspace is archived?

If the latter, then I would expect you could not edit the expense. If the workspace is not archived, I would think you could, but I am not 100% sure. Please confirm the steps to get an expense into that state and I will test on my end.

[melvin-bot]

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

[joekaufmanexpensify]

@dominictb is there an ETA for the PR here?

[dominictb]

Will be ready in a few hours.

[joekaufmanexpensify]

TY!

[dominictb]

Still working on it. The PR will be ready shortly.

[joekaufmanexpensify]

PR merged

[melvin-bot]

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

[melvin-bot]

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

• fix: disable editing money request of archived workspace #46560

If no regressions arise, payment will be issued on 2024-08-13. 🎊

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

• @hungvu193 requires payment through NewDot Manual Requests
• @dominictb requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@hungvu193] The PR that introduced the bug has been identified. Link to the PR: Feat/dupe detection confirmation #42571 is not really the offending PR, we can treat this issue as an improvement.
• [@hungvu193] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: N/A
• [@hungvu193] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
• [@hungvu193] Determine if we should create a regression test for this bug.
• [@hungvu193] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@joekaufmanexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: https://github.com/Expensify/Expensify/issues/420006

[joekaufmanexpensify]

@hungvu193 could you complete BZ so we can prep for payment?

[hungvu193]

• The PR that introduced the bug has been identified. Link to the PR: Feat/dupe detection confirmation #42571 is not really the offending PR, we can treat this issue as an improvement.
• The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: N/A
• A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
• Determine if we should create a regression test for this bug: Yes

Regression test:

1. Create an expense for any workspace.
2. Go to that workspace and delete the workspace.
3. Visit that expense again, click on that expense to open the detail page.
4. Verify that you can not edit the expense information and the sideways caret aren't shown.

Do we 👍 or 👎 ?

[joekaufmanexpensify]

Checklist is complete. TY!

[joekaufmanexpensify]

All set to issue payment. We need to pay:

• @dominictb - $250 for PR via upwork.
• @hungvu193 - $250 for C+ via NewDot.

[joekaufmanexpensify]

@dominictb $250 sent and contract ended!

[joekaufmanexpensify]

Upwork job closed.

[joekaufmanexpensify]

Closing for now, since all that's left is for @hungvu193 to request payment via NewDot. Thanks everyone!

[JmillsExpensify]

$250 approved for @hungvu193