[HOLD for payment 2024-10-10] [$250] Expenses - Deleted Expense that was edited appears when user goes offline

[IuliiaHerets]

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.36-1
Reproducible in staging?: Y
Reproducible in production?: Y
Email or phone of affected tester (no customers): applausetester+uo0914@applause.expensifail.com
Issue reported by: Applause Internal Team

Action Performed:

1. Submit any expense to Workspace or 1:1 chat
2. Tap and edit any details of the expense
3. Tap header of expense and delete expense
4. After returning to chat, go offline

Expected Result:

Deleted expense doesn't appear

Actual Result:

Deleted expense re-appears greyed out and says "user owes" after going offline

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Bug6606016_1726527694571.Screen_Recording_2024-09-16_at_5.54.04_PM.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021836153049228663209
• Upwork Job ID: 1836153049228663209
• Last Price Increase: 2024-09-24
• Automatic offers:
• huult | Contributor | 104163087

Issue Owner

Current Issue Owner: @aimane-chnaif

[melvin-bot]

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

[IuliiaHerets]

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

[melvin-bot]

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

[melvin-bot]

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

[mallenexpensify]

👀 plz @aimane-chnaif . Can you confirm that you're able to reproduce? It appears I was but I ran into a couple issues an my flow was funky.

2024-09-17_14-14-22.mp4

[aimane-chnaif]

reproduced on latest staging v9.0.37-3

bug.mp4

[huult]

Proposal

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

Deleted Expense that was edited appears when user goes offline

What is the root cause of that problem?

The reason for this issue is that reportAction.pendingAction is not updated when the DeleteMoneyRequest command is called. As a result, reportAction.pendingAction remains delete after the command is executed. This causes the condition ReportActionsUtils.shouldReportActionBeVisible(reportAction, reportAction.reportActionID) to return true, and since isOffline is also true, the issue occurs

App/src/pages/home/report/ReportActionsList.tsx

Lines 194 to 199 in 9160fa5

	(reportAction) =>
	(isOffline ||
	ReportActionsUtils.isDeletedParentAction(reportAction) ||
	reportAction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE ||
	reportAction.errors) &&
	ReportActionsUtils.shouldReportActionBeVisible(reportAction, reportAction.reportActionID),

The reason DeleteMoneyRequest did not update pendingAction is because we called the UpdateMoneyRequestDate command to update the date, and it returned a lastUpdateID greater than DeleteMoneyRequest.

App/src/libs/Middleware/SaveResponseInOnyx.ts

Line 33 in 539327d

if (requestsToIgnoreLastUpdateID.includes(request.command) || !OnyxUpdates.doesClientNeedToBeUpdated(Number(response?.previousUpdateID ?? 0))) {

The warning I mentioned for that.

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

We need the backend to update lastUpdateID for DeleteMoneyRequest because we send clientUpdateID to the API, but the response contains a different value, as shown in the image below:

send clientUpdateID: 1973749768

and reponse previousUpdateID: 1973749978 that should be previousUpdateID: 1973749769 and lastUpdateID: 1973749980

or
If we don't want to update the backend, then we should add the command DELETE_MONEY_REQUEST to the ignore list, something like this:

// ./src/libs/Middleware/SaveResponseInOnyx.ts#L8
const requestsToIgnoreLastUpdateID: string[] = [
    WRITE_COMMANDS.OPEN_APP,
+   WRITE_COMMANDS.DELETE_MONEY_REQUEST,
     ...
];

POC

Screen.Recording.2024-09-22.at.15.37.25.mp4

[mallenexpensify]

@aimane-chnaif 👀 on @huult 's proposal above plz

[aimane-chnaif]

@huult thanks for the proposal.

The reason for this issue is that reportAction.pendingAction is not updated when the DeleteMoneyRequest command is called.

This doesn't make sense to me. We're setting pendingAction to null upon DeleteMoneyRequest api success:

App/src/libs/actions/IOU.ts

Lines 5944 to 5953 in a99cf94

	{
	onyxMethod: Onyx.METHOD.MERGE,
	key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${chatReport?.reportID}`,
	value: {
	[reportPreviewAction?.reportActionID ?? '-1']: {
	pendingAction: null,
	errors: null,
	},
	},
	},

[aimane-chnaif]

I noticed that this bug only happens when edit date field. Not happening on other fields like description.
So I don't think the bug is related to lastUpdateID for DeleteMoneyRequest.

[huult]

The reason DeleteMoneyRequest did not update pendingAction is because we called the UpdateMoneyRequestDate command to update the date, and it returned a lastUpdateID greater than DeleteMoneyRequest.

Can you check this command? The API responds, but the data is not stored locally due to the reason I mentioned. You can add a log above this condition, and you will see.

[aimane-chnaif]

@huult please also check #49329 (comment).
Please explain why this happens only on date field.

[aimane-chnaif]

The root cause is quite similar to #44114 (comment)
Possibly fix in BE 🎀👀🎀

[melvin-bot]

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

[melvin-bot]

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

[mallenexpensify]

@jasperhuangg , can you confirm if you think this issue is BE?
You don't need to leave yourself assigned.

[huult]

@jasperhuangg , When does the expense delete successfully on the server? I waited for 1 minute and disconnected the network, but the issue still occurred.
If we know when the deletion is successful, I suggest keeping the data, including the amount, until it is deleted on the server and then deleting it.

[jasperhuangg]

@huult, I see; sorry for the misunderstanding. In that case, your alternative approach of adding the command to the ignore list should work! If the user intends to delete the money request, it doesn't seem like we need to wait for any requests to update fields in the money request.

[melvin-bot]

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

[melvin-bot]

@mallenexpensify, @jasperhuangg, @aimane-chnaif Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[huult]

The PR for this issue is already open

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

• fix deleted expense still visible offline #49878

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

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

• @huult requires payment automatic offer (Contributor)
• @aimane-chnaif requires payment through NewDot Manual Requests

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

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

[mallenexpensify]

Contributor: @huult paid $250 via Upwork
Contributor+: @aimane-chnaif paid $250 via Upwork

@aimane-chnaif can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~021836153049228663209

Please fill out the BZ checklist above too, thx

[aimane-chnaif]

• The PR that introduced the bug has been identified. Link to the PR: The root cause is in backend
• 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.

1. Submit an expense to a Workspace or 1:1 chat.
2. Tap and edit the date of the expense.
3. Tap the header of the expense and delete it.
4. Return to the chat and go offline.
5. Verify that the deleted expense does not reappear.

[mallenexpensify]

test case created

• https://github.com/Expensify/Expensify/issues/435989

@aimane-chnaif can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~021836153049228663209

[melvin-bot]

@huult, @mallenexpensify, @jasperhuangg, @aimane-chnaif Whoops! This issue is 2 days overdue. Let's get this updated quick!

[mallenexpensify]

@aimane-chnaif paid, payment comment above updated. Thx