Skip to content

Commit

Permalink
Adding conflict resolver for delete comment
Browse files Browse the repository at this point in the history
  • Loading branch information
gedu committed Oct 15, 2024
1 parent 8f5ba1e commit c503b78
Show file tree
Hide file tree
Showing 6 changed files with 577 additions and 9 deletions.
11 changes: 8 additions & 3 deletions src/libs/Network/SequentialQueue.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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();
});
Expand Down Expand Up @@ -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.`);
}
Expand Down
17 changes: 15 additions & 2 deletions src/libs/actions/PersistedRequests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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);
Expand Down Expand Up @@ -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};
61 changes: 60 additions & 1 deletion src/libs/actions/Report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1414,6 +1414,16 @@ function handleReportChanged(report: OnyxEntry<Report>) {
}
}
}
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,
]);

/** Deletes a comment from the report, basically sets it as empty string */
function deleteReportComment(reportID: string, reportAction: ReportAction) {
Expand Down Expand Up @@ -1538,7 +1548,56 @@ 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) => {
const indices: number[] = [];
let addCommentFound = false;

persistedRequests.forEach((request, index) => {
if (!commentsToBeDeleted.has(request.command) || request.data?.reportActionID !== reportActionID) {
return;
}
if (addNewMessage.has(request.command)) {
addCommentFound = true;
}
indices.push(index);
});

if (indices.length === 0) {
return {
conflictAction: {
type: 'push',
},
};
}

if (addCommentFound) {
const rollbackData: OnyxUpdate[] = [
{
onyxMethod: Onyx.METHOD.MERGE,
key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${originalReportID}`,
value: {
[reportActionID]: null,
},
},
];
Onyx.update(rollbackData);
}

return {
conflictAction: {
type: 'delete',
indices,
pushNewRequest: !addCommentFound,
},
};
},
},
);

// 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
Expand Down
22 changes: 21 additions & 1 deletion src/types/onyx/Request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -97,7 +117,7 @@ type ConflictActionData = {
/**
* The action to take in case of a conflict.
*/
conflictAction: ConflictRequestReplace | ConflictRequestPush | ConflictRequestNoAction;
conflictAction: ConflictRequestReplace | ConflictRequestDelete | ConflictRequestPush | ConflictRequestNoAction;
};

/**
Expand Down
Loading

0 comments on commit c503b78

Please sign in to comment.