[HOLD for payment 2023-12-28] [$500] IOU - Edit request option is present for paid message and error shows up when editing paid IOU

[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: 1.3.91-6
Reproducible in staging?: Yes
Reproducible in production?: Yes
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:

Issue found when executing PR #29934

Action Performed:

1. Go to staging.new.expensify.com
2. Request money from a workspace chat
3. Click on the expense preview to open expense report
4. Click Pay elsewhere
5. Hover over the system message for paid request
6. Click Edit request
7. Return to IOU report
8. Hover over IOU preview and click Edit request
9. Edit any field

Expected Result:

• "Edit request" option should not be available for the system message for paid request
• When clicking on "Edit request" on IOU preview, user should be able to edit the IOU details

Actual Result:

"Edit request" option is available for the system message for paid request. Clicking on it does not allow user to edit the request

• When clicking on "Edit request" on IOU preview and user edits IOU details, error shows up

Workaround:

Unknown

Platforms:

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

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

Screenshots/Videos

Bug6251899_1698331109368.Recording__296.1.mp4

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01a4085c4ba07d24d8
• Upwork Job ID: 1727671655146373120
• Last Price Increase: 2023-11-23

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @garrettmknight (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

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

[hungvu193]

Proposal

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

IOU - Edit request option is present for paid message and error shows up when editing paid IOU

What is the root cause of that problem?

We're showing edit option for even the system message:

App/src/pages/home/report/ContextMenu/ContextMenuActions.js

Lines 283 to 284 in a67253b

	shouldShow: (type, reportAction, isArchivedRoom, betas, menuTarget, isChronosReport) =>
	type === CONTEXT_MENU_TYPES.REPORT_ACTION && ReportUtils.canEditReportAction(reportAction) && !isArchivedRoom && !isChronosReport,

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

I think we shouldn't show the edit option for system message like that, so we need to update the condition here to:

const isSystemMessage = reportAction.actionName === CONST.REPORT.ACTIONS.TYPE.IOU && reportAction.originalMessage.type === CONST.IOU.REPORT_ACTION_TYPE.PAY;

return type === CONTEXT_MENU_TYPES.REPORT_ACTION && ReportUtils.canEditReportAction(reportAction) && !isArchivedRoom && !isChronosReport && !isSystemMessage;

or we just need to bring this logic into canEditReportAction for cleaner logic.

What alternative solutions did you explore? (Optional)

N/A

[dukenv0307]

Proposal

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

Edit request option is present for paid message and error shows up when editing paid IOU

What is the root cause of that problem?

The edit option is shown based on canEditReportAction

App/src/pages/home/report/ContextMenu/ContextMenuActions.js

Line 284 in 8c76d2a

type === CONTEXT_MENU_TYPES.REPORT_ACTION && ReportUtils.canEditReportAction(reportAction) && !isArchivedRoom && !isChronosReport,

Let's see the condition in canEditReportAction

App/src/libs/ReportUtils.js

Lines 1620 to 1627 in 8c76d2a

	return (
	reportAction.actorAccountID === currentUserAccountID &&
	isCommentOrIOU &&
	canEditMoneyRequest(reportAction) && // Returns true for non-IOU actions
	!isReportMessageAttachment(lodashGet(reportAction, ['message', 0], {})) &&
	!ReportActionsUtils.isDeletedAction(reportAction) &&
	!ReportActionsUtils.isCreatedTaskReportAction(reportAction) &&
	reportAction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE

We have the check canEditMoneyRequest that will return true if the moneyRequestReport is expense report and the user is the admin. So the paid message has edit message option

App/src/libs/ReportUtils.js

Lines 1572 to 1574 in 8c76d2a

	if (isAdmin) {
	return true;
	}

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

I think we shouldn't show the edit option for iou action. We show the edit option for MoneyRequestAction and when we click on it we will navigate to child report. It is the same behavior when we click on reply in thread so I think we only need to show reply in thread option for this case.

We should remove the check canEditMoneyRequest in canEditReportAction function and update isCommentOrIOU to isComment

const isComment = reportAction.actionName === CONST.REPORT.ACTIONS.TYPE.ADDCOMMENT
return (
    reportAction.actorAccountID === currentUserAccountID &&
    isComment &&
    !isReportMessageAttachment(lodashGet(reportAction, ['message', 0], {})) &&
    !ReportActionsUtils.isDeletedAction(reportAction) &&
    !ReportActionsUtils.isCreatedTaskReportAction(reportAction) &&
    reportAction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE
);

App/src/libs/ReportUtils.js

Lines 1620 to 1627 in 8c76d2a

	return (
	reportAction.actorAccountID === currentUserAccountID &&
	isCommentOrIOU &&
	canEditMoneyRequest(reportAction) && // Returns true for non-IOU actions
	!isReportMessageAttachment(lodashGet(reportAction, ['message', 0], {})) &&
	!ReportActionsUtils.isDeletedAction(reportAction) &&
	!ReportActionsUtils.isCreatedTaskReportAction(reportAction) &&
	reportAction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE

And remove the logic for the money request action case here

App/src/pages/home/report/ContextMenu/ContextMenuActions.js

Line 286 in 8c76d2a

if (ReportActionsUtils.isMoneyRequestAction(reportAction)) {

What alternative solutions did you explore? (Optional)

We can update canEditMoneyRequest to return false if the originalMessage type of the iou action is pay.

[Krishna2323]

Proposal

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

Edit request option is present for paid message and error shows up when editing paid IOU

What is the root cause of that problem?

We return true from canEditMoneyRequest function when user is admin.

App/src/libs/ReportUtils.js

Lines 1567 to 1574 in 8c76d2a

	const moneyRequestReport = getReport(moneyRequestReportID);
	const isReportSettled = isSettled(moneyRequestReport.reportID);
	const isAdmin = isExpenseReport(moneyRequestReport) && lodashGet(getPolicy(moneyRequestReport.policyID), 'role', '') === CONST.POLICY.ROLE.ADMIN;
	const isRequestor = currentUserAccountID === reportAction.actorAccountID;
	if (isAdmin) {
	return true;
	}

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

I don't think we should return true in any case if payments is settled, so we should only return true when user is admin and payment is not settled.

Option 1

    if (isAdmin && !isReportSettled) {
        return true;
    }

Option 2
Or we can remove isAdmin check and modify the last return statement like this:

    return !isReportSettled && (isRequestor || isAdmin);

Option 3
We can also check if the request action type is IOU and is settled using isSettled function like we do in canEditMoneyRequest function.

[tienifr]

Proposal

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

"Edit request" option is available for the system message for paid request. Clicking on it does not allow user to edit the request

When clicking on "Edit request" on IOU preview and user edits IOU details, error shows up

This one is currently not reproducible.

What is the root cause of that problem?

In here, we're not validating if the IOU report action is a 'CREATE' IOU report action.

IOU report action has many sub-types, create, pay, split, decline, ... Only the create type should be allowed to edit since it's the one that creates the money request. Other ones like pay, decline, ... are follow-up actions on that IOU and we should not allow editing the money request from these types.

Without that additional check, in this issue, since the pay request is also an IOU, it still shows the Edit request.

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

We need to update this with an additional check after this one: If the sub-type of the report action is not create, then early return.

if (reportAction.originalMessage.type !== CONST.IOU.REPORT_ACTION_TYPE.CREATE) {
    return false;
}

This will make sure everywhere we use canEditMoneyRequest, like in the ContextMenu case here, it will always return false for non-create report actions. And this will work for all sub-types of IOU, not just pay in this case.

What alternative solutions did you explore? (Optional)

NA

[mananjadhav]

Will test this and review this by tomorrow.

[garrettmknight]

@mananjadhav did you get a chance to test/review?

[melvin-bot]

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

[garrettmknight]

@mananjadhav can you give this one a look when you get a chance?

[mananjadhav]

I was stuck with low internet. I'll finish this by tomorrow.

[melvin-bot]

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

[garrettmknight]

@mananjadhav can you prioritize this one this week?

[mananjadhav]

@garrettmknight I was out sick, I'll have this prioritized today/tomorrow.

[melvin-bot]

@garrettmknight @mananjadhav 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!

[tienifr]

@puneetlath Could we continue?

[melvin-bot]

@puneetlath, @garrettmknight, @mananjadhav Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

@puneetlath, @garrettmknight, @mananjadhav Eep! 4 days overdue now. Issues have feelings too...

[puneetlath]

Let's do it. Sorry for the delay!

[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 1.4.14-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Fix/30448 Edit request option is present for paid message and error shows up when editing paid IOU #32631

If no regressions arise, payment will be issued on 2023-12-28. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

• External issue reporter
• Contributor that fixed the issue
• Contributor+ that helped on the issue and/or PR

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

• @mananjadhav requires payment through NewDot Manual Requests
• @tienifr requires payment (Needs manual offer from BZ)

[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:

• [@mananjadhav] The PR that introduced the bug has been identified. Link to the PR:
• [@mananjadhav] 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:
• [@mananjadhav] 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:
• [@mananjadhav] Determine if we should create a regression test for this bug.
• [@mananjadhav] 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.
• [@garrettmknight] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[garrettmknight]

Offer sent to @tienifr https://www.upwork.com/nx/wm/offer/100202927

[mananjadhav]

I couldn't pinpoint to an exact PR for this one. Because we have canEditMoneyRequest since long. I also don't think we need a regression test for this one. @puneetlath @garrettmknight wdyt?

@garrettmknight Can you please post a payout summary for me to raise a request?

[garrettmknight]

Summary of payments:

• Contributor: @tienifr $500 paid via Upwork
• Reviewer: @mananjadhav $500

[garrettmknight]

Dropping to weekly for payment

[JmillsExpensify]

$500 payment approved for @mananjadhav based on summary above.