Сhat - Message deleted in offline are not striked out

[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.60
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
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. Go offline
3. Open a report
4. Send a message
5. Delete the message

Expected Result:

Message deleted in offline must be striked out

Actual Result:

Message deleted in offline are not striked out

Workaround:

Unknown

Platforms:

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

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6661815_1731357681420.Screenrecorder-2024-11-12-02-02-31-217.mp4

View all open jobs on GitHub

[melvin-bot]

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

[NJ-2020]

Edited by proposal-police: This proposal was edited at 2024-11-12 00:12:44 UTC.

Proposal

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

What is the root cause of that problem?

When we send message in offline mode then we delete the comment, it will invoke this function

App/src/libs/actions/Report.ts

Line 1431 in b2d6a0e

function deleteReportComment(reportID: string, reportAction: ReportAction) {

and apply the onyx data and set the pending action to DELETE

App/src/libs/actions/Report.ts

Lines 1450 to 1452 in b2d6a0e

	const optimisticReportActions: NullishDeep<ReportActions> = {
	[reportActionID]: {
	pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE,

App/src/libs/actions/Report.ts

Lines 1553 to 1559 in b2d6a0e

	API.write(
	WRITE_COMMANDS.DELETE_COMMENT,
	parameters,
	{optimisticData, successData, failureData},
	{
	checkAndFixConflictingRequest: (persistedRequests) => resolveCommentDeletionConflicts(persistedRequests, reportActionID, originalReportID),
	},

After this PR #51422 we will resolve if there's a conflicts when deleting the comment using resolveCommentDeletionConflicts function

App/src/libs/actions/Report.ts

Line 1558 in b2d6a0e

checkAndFixConflictingRequest: (persistedRequests) => resolveCommentDeletionConflicts(persistedRequests, reportActionID, originalReportID),

Inside resolveCommentDeletionConflicts function we check if there's AddComment request inside the persisted requests it will rollback the changes meaning it will delete the comment permanently

App/src/libs/actions/RequestConflictUtils.ts

Line 11 in b2d6a0e

const addNewMessage = new Set<string>([WRITE_COMMANDS.ADD_COMMENT, WRITE_COMMANDS.ADD_ATTACHMENT, WRITE_COMMANDS.ADD_TEXT_AND_ATTACHMENT]);

App/src/libs/actions/RequestConflictUtils.ts

Lines 71 to 74 in b2d6a0e

	// If we find a new message, we probably want to remove it and not perform any request given that the server
	// doesn't know about it yet.
	if (addNewMessage.has(request.command) && !request.isRollbacked) {
	addCommentFound = true;

App/src/libs/actions/RequestConflictUtils.ts

Lines 88 to 100 in b2d6a0e

	if (addCommentFound) {
	// The new message performs some changes in Onyx, so we need to rollback those changes.
	const rollbackData: OnyxUpdate[] = [
	{
	onyxMethod: Onyx.METHOD.MERGE,
	key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${originalReportID}`,
	value: {
	[reportActionID]: null,
	},
	},
	];
	Onyx.update(rollbackData);
	}

Since we send the message in offline mode so the AddComment request will go to PersistedRequests first then it will get proceed when it's online, so it will delete the comment permanently

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

Solution 1: We can remove this code, because we do not want the delete comment get deleted permanently in offline mode

App/src/libs/actions/RequestConflictUtils.ts

Lines 88 to 100 in b2d6a0e

	if (addCommentFound) {
	// The new message performs some changes in Onyx, so we need to rollback those changes.
	const rollbackData: OnyxUpdate[] = [
	{
	onyxMethod: Onyx.METHOD.MERGE,
	key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${originalReportID}`,
	value: {
	[reportActionID]: null,
	},
	},
	];
	Onyx.update(rollbackData);
	}

Solution 2: We can rollback the changes if it's not offline mode

I think we should not delete permanently in offline mode even there's a new comment, we should set the pending action to DELETE

What alternative solutions did you explore? (Optional)

We can rollback the changes if the index AddComment request is greater than the DeleteComment request meaning the user delete a comment then the user new comment we will rollback the changes

function resolveCommentDeletionConflicts(persistedRequests: OnyxRequest[], reportActionID: string, originalReportID: string, currentRequest: string): ConflictActionData {
    ...
    const sendNewMessage = [...addNewMessage];
    if (addNewMessage.has(request.command) && sendNewMessage.indexOf(request.command) > sendNewMessage.indexOf(currentRequest) && !request.isRollbacked) {
        addCommentFound = true;
        commentCouldBeThread[reportActionID] = commentIndicesToDelete.length;
    }

[FitseTLT]

This can be expected BTW. If the user created and deleted a message offline there is no need to show it or even send the create and delete API requests to the BE.

[kadiealexander]

This does seem like expected behaviour. Closing as not a bug, please reopen if you disagree.

[IuliiaHerets]

This issue is still valid and reproduces even if we create a message online and then delete it offline. Message not strike out

Desktop-deleted-msg-offline.mp4

[IuliiaHerets]

I created a new issue as it is a deploy blocker for build v9.0.64-0, issue happens only on MacOS #52753

[kadiealexander]

Android <> iOS bug swap with @jliexpensify

[jliexpensify]

@IuliiaHerets - can you confirm this is a Chrome/Safari issue only? Not an Android one?

[jliexpensify]

It also seems like we could close this in favour of #52753?

[IuliiaHerets]

@jliexpensify Yes, issue has happened only on MacOS (Safari, Chrome and Desktop).