Merge pull request #51422 from gedu/gedu/add_comment_text_update_by_e…

…dit_comment

Add comment text update by edit comment

src/libs/Network/SequentialQueue.ts

@@ -8,6 +8,7 @@ import * as QueuedOnyxUpdates from '@userActions/QueuedOnyxUpdates';
 import CONST from '@src/CONST';
 import ONYXKEYS from '@src/ONYXKEYS';
 import type OnyxRequest from '@src/types/onyx/Request';
+import type {ConflictData} from '@src/types/onyx/Request';
 import * as NetworkStore from './NetworkStore';
 
 type RequestError = Error & {
@@ -96,15 +97,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 +114,7 @@ function process(): Promise<void> {
                 .then(process)
                 .catch(() => {
                     Onyx.update(requestToProcess.failureData ?? []);
-                    PersistedRequests.remove(requestToProcess);
+                    PersistedRequests.endRequestAndRemoveFromQueue(requestToProcess);
                     RequestThrottle.clear();
                     return process();
                 });
@@ -204,6 +205,24 @@ function isPaused(): boolean {
 // Flush the queue when the connection resumes
 NetworkStore.onReconnection(flush);
 
+function handleConflictActions(conflictAction: ConflictData, newRequest: OnyxRequest) {
+    if (conflictAction.type === 'push') {
+        PersistedRequests.save(newRequest);
+    } else if (conflictAction.type === 'replace') {
+        PersistedRequests.update(conflictAction.index, conflictAction.request ?? newRequest);
+    } else if (conflictAction.type === 'delete') {
+        PersistedRequests.deleteRequestsByIndices(conflictAction.indices);
+        if (conflictAction.pushNewRequest) {
+            PersistedRequests.save(newRequest);
+        }
+        if (conflictAction.nextAction) {
+            handleConflictActions(conflictAction.nextAction, newRequest);
+        }
+    } else {
+        Log.info(`[SequentialQueue] No action performed to command ${newRequest.command} and it will be ignored.`);
+    }
+}
+
 function push(newRequest: OnyxRequest) {
     const {checkAndFixConflictingRequest} = newRequest;
 
@@ -215,14 +234,7 @@ function push(newRequest: OnyxRequest) {
         // don't try to serialize a function.
         // eslint-disable-next-line no-param-reassign
         delete newRequest.checkAndFixConflictingRequest;
-
-        if (conflictAction.type === 'push') {
-            PersistedRequests.save(newRequest);
-        } else if (conflictAction.type === 'replace') {
-            PersistedRequests.update(conflictAction.index, newRequest);
-        } else {
-            Log.info(`[SequentialQueue] No action performed to command ${newRequest.command} and it will be ignored.`);
-        }
+        handleConflictActions(conflictAction, newRequest);
     } else {
         // Add request to Persisted Requests so that it can be retried if it fails
         PersistedRequests.save(newRequest);

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, resolveEditCommentWithNewAddCommentRequest} from './RequestConflictUtils';
 import * as Session from './Session';
 import * as Welcome from './Welcome';
 import * as OnboardingFlow from './Welcome/OnboardingFlow';
@@ -162,7 +162,7 @@ type GuidedSetupData = Array<
 type ReportError = {
     type?: string;
 };
-
+const addNewMessageWithText = new Set<string>([WRITE_COMMANDS.ADD_COMMENT, WRITE_COMMANDS.ADD_TEXT_AND_ATTACHMENT]);
 let conciergeChatReportID: string | undefined;
 let currentUserAccountID = -1;
 let currentUserEmail: string | undefined;
@@ -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
@@ -1700,7 +1707,13 @@ function editReportComment(reportID: string, originalReportAction: OnyxEntry<Rep
         parameters,
         {optimisticData, successData, failureData},
         {
-            checkAndFixConflictingRequest: (persistedRequests) => resolveDuplicationConflictAction(persistedRequests, createUpdateCommentMatcher(reportActionID)),
+            checkAndFixConflictingRequest: (persistedRequests) => {
+                const addCommentIndex = persistedRequests.findIndex((request) => addNewMessageWithText.has(request.command) && request.data?.reportActionID === reportActionID);
+                if (addCommentIndex > -1) {
+                    return resolveEditCommentWithNewAddCommentRequest(persistedRequests, parameters, reportActionID, addCommentIndex);
+                }
+                return resolveDuplicationConflictAction(persistedRequests, createUpdateCommentMatcher(reportActionID));
+            },
         },
     );
 }

src/libs/actions/RequestConflictUtils.ts

@@ -1,15 +1,30 @@
+import type {OnyxUpdate} from 'react-native-onyx';
+import Onyx from 'react-native-onyx';
+import type {UpdateCommentParams} from '@libs/API/parameters';
 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 +50,98 @@ 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,
+        },
+    };
+}
+
+function resolveEditCommentWithNewAddCommentRequest(persistedRequests: OnyxRequest[], parameters: UpdateCommentParams, reportActionID: string, addCommentIndex: number): ConflictActionData {
+    const indicesToDelete: number[] = [];
+    persistedRequests.forEach((request, index) => {
+        if (request.command !== WRITE_COMMANDS.UPDATE_COMMENT || request.data?.reportActionID !== reportActionID) {
+            return;
+        }
+        indicesToDelete.push(index);
+    });
+
+    const currentAddComment = persistedRequests.at(addCommentIndex);
+    let nextAction = null;
+    if (currentAddComment) {
+        currentAddComment.data = {...currentAddComment.data, ...parameters};
+        nextAction = {
+            type: 'replace',
+            index: addCommentIndex,
+            request: currentAddComment,
+        };
+
+        if (indicesToDelete.length === 0) {
+            return {
+                conflictAction: nextAction,
+            } as ConflictActionData;
+        }
+    }
+
+    return {
+        conflictAction: {
+            type: 'delete',
+            indices: indicesToDelete,
+            pushNewRequest: false,
+            nextAction,
+        },
+    } as ConflictActionData;
+}
+
+export {resolveDuplicationConflictAction, resolveCommentDeletionConflicts, resolveEditCommentWithNewAddCommentRequest, createUpdateCommentMatcher};

src/types/onyx/Request.ts

@@ -55,6 +55,11 @@ type RequestData = {
     shouldSkipWebProxy?: boolean;
 };
 
+/**
+ * Represents the possible actions to take in case of a conflict in the request queue.
+ */
+type ConflictData = ConflictRequestReplace | ConflictRequestDelete | ConflictRequestPush | ConflictRequestNoAction;
+
 /**
  * Model of a conflict request that has to be replaced in the request queue.
  */
@@ -68,6 +73,36 @@ type ConflictRequestReplace = {
      * The index of the request in the queue to update.
      */
     index: number;
+
+    /**
+     * The new request to replace the existing request in the queue.
+     */
+    request?: Request;
+};
+
+/**
+ * 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;
+
+    /**
+     * The next action to execute after the current conflict is resolved.
+     */
+    nextAction?: ConflictData;
 };
 
 /**
@@ -97,7 +132,7 @@ type ConflictActionData = {
     /**
      * The action to take in case of a conflict.
      */
-    conflictAction: ConflictRequestReplace | ConflictRequestPush | ConflictRequestNoAction;
+    conflictAction: ConflictData;
 };
 
 /**
@@ -115,6 +150,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 */
@@ -147,4 +187,4 @@ type PaginatedRequest = Request &
     };
 
 export default Request;
-export type {OnyxData, RequestType, PaginationConfig, PaginatedRequest, RequestConflictResolver, ConflictActionData};
+export type {OnyxData, RequestType, PaginationConfig, PaginatedRequest, RequestConflictResolver, ConflictActionData, ConflictData};