DeleteComment conflict resolver

src/libs/Network/SequentialQueue.ts

@@ -96,15 +96,15 @@ function process(): Promise<void> {
                 pause();
             }
 
-            PersistedRequests.remove(requestToProcess);
+            PersistedRequests.endRequestAndRemoveFromQueue(requestToProcess);
             RequestThrottle.clear();
             return process();
         })
         .catch((error: RequestError) => {
             // On sign out we cancel any in flight requests from the user. Since that user is no longer signed in their requests should not be retried.
             // Duplicate records don't need to be retried as they just mean the record already exists on the server
             if (error.name === CONST.ERROR.REQUEST_CANCELLED || error.message === CONST.ERROR.DUPLICATE_RECORD) {
-                PersistedRequests.remove(requestToProcess);
+                PersistedRequests.endRequestAndRemoveFromQueue(requestToProcess);
                 RequestThrottle.clear();
                 return process();
             }
@@ -113,7 +113,7 @@ function process(): Promise<void> {
                 .then(process)
                 .catch(() => {
                     Onyx.update(requestToProcess.failureData ?? []);
-                    PersistedRequests.remove(requestToProcess);
+                    PersistedRequests.endRequestAndRemoveFromQueue(requestToProcess);
                     RequestThrottle.clear();
                     return process();
                 });
@@ -220,6 +220,11 @@ function push(newRequest: OnyxRequest) {
             PersistedRequests.save(newRequest);
         } else if (conflictAction.type === 'replace') {
             PersistedRequests.update(conflictAction.index, newRequest);
+        } else if (conflictAction.type === 'delete') {
+            PersistedRequests.deleteRequestsByIndices(conflictAction.indices);
+            if (conflictAction.pushNewRequest) {
+                PersistedRequests.save(newRequest);
+            }
         } else {
             Log.info(`[SequentialQueue] No action performed to command ${newRequest.command} and it will be ignored.`);
         }

src/libs/actions/PersistedRequests.ts

@@ -53,7 +53,7 @@ function save(requestToPersist: Request) {
     });
 }
 
-function remove(requestToRemove: Request) {
+function endRequestAndRemoveFromQueue(requestToRemove: Request) {
     ongoingRequest = null;
     /**
      * We only remove the first matching request because the order of requests matters.
@@ -76,6 +76,19 @@ function remove(requestToRemove: Request) {
     });
 }
 
+function deleteRequestsByIndices(indices: number[]) {
+    // Create a Set from the indices array for efficient lookup
+    const indicesSet = new Set(indices);
+
+    // Create a new array excluding elements at the specified indices
+    persistedRequests = persistedRequests.filter((_, index) => !indicesSet.has(index));
+
+    // Update the persisted requests in storage or state as necessary
+    Onyx.set(ONYXKEYS.PERSISTED_REQUESTS, persistedRequests).then(() => {
+        Log.info(`Multiple (${indices.length}) requests removed from the queue. Queue length is ${persistedRequests.length}`);
+    });
+}
+
 function update(oldRequestIndex: number, newRequest: Request) {
     const requests = [...persistedRequests];
     requests.splice(oldRequestIndex, 1, newRequest);
@@ -117,7 +130,7 @@ function rollbackOngoingRequest() {
     }
 
     // Prepend ongoingRequest to persistedRequests
-    persistedRequests.unshift(ongoingRequest);
+    persistedRequests.unshift({...ongoingRequest, isRollbacked: true});
 
     // Clear the ongoingRequest
     ongoingRequest = null;
@@ -131,4 +144,4 @@ function getOngoingRequest(): Request | null {
     return ongoingRequest;
 }
 
-export {clear, save, getAll, remove, update, getLength, getOngoingRequest, processNextRequest, updateOngoingRequest, rollbackOngoingRequest};
+export {clear, save, getAll, endRequestAndRemoveFromQueue, update, getLength, getOngoingRequest, processNextRequest, updateOngoingRequest, rollbackOngoingRequest, deleteRequestsByIndices};

src/libs/actions/Report.ts

@@ -111,7 +111,7 @@ import {isEmptyObject} from '@src/types/utils/EmptyObject';
 import * as CachedPDFPaths from './CachedPDFPaths';
 import * as Modal from './Modal';
 import navigateFromNotification from './navigateFromNotification';
-import {createUpdateCommentMatcher, resolveDuplicationConflictAction} from './RequestConflictUtils';
+import {createUpdateCommentMatcher, resolveCommentDeletionConflicts, resolveDuplicationConflictAction} from './RequestConflictUtils';
 import * as Session from './Session';
 import * as Welcome from './Welcome';
 import * as OnboardingFlow from './Welcome/OnboardingFlow';
@@ -1535,7 +1535,14 @@ function deleteReportComment(reportID: string, reportAction: ReportAction) {
 
     CachedPDFPaths.clearByKey(reportActionID);
 
-    API.write(WRITE_COMMANDS.DELETE_COMMENT, parameters, {optimisticData, successData, failureData});
+    API.write(
+        WRITE_COMMANDS.DELETE_COMMENT,
+        parameters,
+        {optimisticData, successData, failureData},
+        {
+            checkAndFixConflictingRequest: (persistedRequests) => resolveCommentDeletionConflicts(persistedRequests, reportActionID, originalReportID),
+        },
+    );
 
     // if we are linking to the report action, and we are deleting it, and it's not a deleted parent action,
     // we should navigate to its report in order to not show not found page

src/libs/actions/RequestConflictUtils.ts

@@ -1,15 +1,29 @@
+import type {OnyxUpdate} from 'react-native-onyx';
+import Onyx from 'react-native-onyx';
 import {WRITE_COMMANDS} from '@libs/API/types';
+import ONYXKEYS from '@src/ONYXKEYS';
 import type OnyxRequest from '@src/types/onyx/Request';
 import type {ConflictActionData} from '@src/types/onyx/Request';
 
+type RequestMatcher = (request: OnyxRequest) => boolean;
+
+const addNewMessage = new Set<string>([WRITE_COMMANDS.ADD_COMMENT, WRITE_COMMANDS.ADD_ATTACHMENT, WRITE_COMMANDS.ADD_TEXT_AND_ATTACHMENT]);
+
+const commentsToBeDeleted = new Set<string>([
+    WRITE_COMMANDS.ADD_COMMENT,
+    WRITE_COMMANDS.ADD_ATTACHMENT,
+    WRITE_COMMANDS.ADD_TEXT_AND_ATTACHMENT,
+    WRITE_COMMANDS.UPDATE_COMMENT,
+    WRITE_COMMANDS.ADD_EMOJI_REACTION,
+    WRITE_COMMANDS.REMOVE_EMOJI_REACTION,
+]);
+
 function createUpdateCommentMatcher(reportActionID: string) {
     return function (request: OnyxRequest) {
         return request.command === WRITE_COMMANDS.UPDATE_COMMENT && request.data?.reportActionID === reportActionID;
     };
 }
 
-type RequestMatcher = (request: OnyxRequest) => boolean;
-
 /**
  * Determines the appropriate action for handling duplication conflicts in persisted requests.
  *
@@ -35,4 +49,62 @@ function resolveDuplicationConflictAction(persistedRequests: OnyxRequest[], requ
     };
 }
 
-export {resolveDuplicationConflictAction, createUpdateCommentMatcher};
+function resolveCommentDeletionConflicts(persistedRequests: OnyxRequest[], reportActionID: string, originalReportID: string): ConflictActionData {
+    const commentIndicesToDelete: number[] = [];
+    const commentCouldBeThread: Record<string, number> = {};
+    let addCommentFound = false;
+    persistedRequests.forEach((request, index) => {
+        // If the request will open a Thread, we should not delete the comment and we should send all the requests
+        if (request.command === WRITE_COMMANDS.OPEN_REPORT && request.data?.parentReportActionID === reportActionID && reportActionID in commentCouldBeThread) {
+            const indexToRemove = commentCouldBeThread[reportActionID];
+            commentIndicesToDelete.splice(indexToRemove, 1);
+            // The new message performs some changes in Onyx, we want to keep those changes.
+            addCommentFound = false;
+            return;
+        }
+
+        if (!commentsToBeDeleted.has(request.command) || request.data?.reportActionID !== reportActionID) {
+            return;
+        }
+
+        // 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;
+            commentCouldBeThread[reportActionID] = commentIndicesToDelete.length;
+        }
+        commentIndicesToDelete.push(index);
+    });
+
+    if (commentIndicesToDelete.length === 0) {
+        return {
+            conflictAction: {
+                type: 'push',
+            },
+        };
+    }
+
+    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);
+    }
+
+    return {
+        conflictAction: {
+            type: 'delete',
+            indices: commentIndicesToDelete,
+            pushNewRequest: !addCommentFound,
+        },
+    };
+}
+
+export {resolveDuplicationConflictAction, resolveCommentDeletionConflicts, createUpdateCommentMatcher};

src/types/onyx/Request.ts

@@ -70,6 +70,26 @@ type ConflictRequestReplace = {
     index: number;
 };
 
+/**
+ * Model of a conflict request that needs to be deleted from the request queue.
+ */
+type ConflictRequestDelete = {
+    /**
+     * The action to take in case of a conflict.
+     */
+    type: 'delete';
+
+    /**
+     * The indices of the requests in the queue that are to be deleted.
+     */
+    indices: number[];
+
+    /**
+     * A flag to mark if the new request should be pushed into the queue after deleting the conflicting requests.
+     */
+    pushNewRequest: boolean;
+};
+
 /**
  * Model of a conflict request that has to be enqueued at the end of request queue.
  */
@@ -97,7 +117,7 @@ type ConflictActionData = {
     /**
      * The action to take in case of a conflict.
      */
-    conflictAction: ConflictRequestReplace | ConflictRequestPush | ConflictRequestNoAction;
+    conflictAction: ConflictRequestReplace | ConflictRequestDelete | ConflictRequestPush | ConflictRequestNoAction;
 };
 
 /**
@@ -115,6 +135,11 @@ type RequestConflictResolver = {
      * the ongoing request, it will be removed from the persisted request queue.
      */
     persistWhenOngoing?: boolean;
+
+    /**
+     * A boolean flag to mark a request as rollbacked, if set to true it means the request failed and was added back into the queue.
+     */
+    isRollbacked?: boolean;
 };
 
 /** Model of requests sent to the API */