Remove allReportActions in ClearReportActionErrors - batch 1

src/components/ReportActionItem/MoneyRequestReceiptView.tsx

@@ -158,6 +158,7 @@ function MoneyRequestReceiptView({
     const [parentReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(parentReportID)}`);
     const [chatReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getNonEmptyStringOnyxID(parentReport?.parentReportID)}`);
     const [parentReportActions] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${parentReportID}`);
+    const [allReportActions] = useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS);
     const [conciergeReportID] = useOnyx(ONYXKEYS.CONCIERGE_REPORT_ID);
     const [introSelected] = useOnyx(ONYXKEYS.NVP_INTRO_SELECTED);
     const [isSelfTourViewed] = useOnyx(ONYXKEYS.NVP_ONBOARDING, {selector: hasSeenTourSelector});
@@ -570,15 +571,29 @@ function MoneyRequestReceiptView({
                 return;
             }
             clearError(linkedTransactionID);
-            clearAllRelatedReportActionErrors({reportID: report.reportID, reportAction: parentReportAction, originalReportID, isOffline, reports: reportsParentHierarchy});
+            clearAllRelatedReportActionErrors({
+                reportID: report.reportID,
+                reportAction: parentReportAction,
+                originalReportID,
+                isOffline,
+                reports: reportsParentHierarchy,
+                reportActions: allReportActions,
+            });
             return;
         }
         if (!isEmptyObject(transactionAndReportActionErrors)) {
             revert(transaction, getLastModifiedExpense(report?.reportID));
         }
         if (!isEmptyObject(errorsWithoutReportCreation)) {
             clearError(transaction.transactionID);
-            clearAllRelatedReportActionErrors({reportID: report.reportID, reportAction: parentReportAction, originalReportID, isOffline, reports: reportsParentHierarchy});
+            clearAllRelatedReportActionErrors({
+                reportID: report.reportID,
+                reportAction: parentReportAction,
+                originalReportID,
+                isOffline,
+                reports: reportsParentHierarchy,
+                reportActions: allReportActions,
+            });
         }
         if (!isEmptyObject(reportCreationError)) {
             if (isInNarrowPaneModal) {

src/libs/actions/ClearReportActionErrors.ts

@@ -1,4 +1,4 @@
-import {getLinkedTransactionID, getReportAction, getReportActionMessage, isCreatedTaskReportAction, isRejectedAction} from '@libs/ReportActionsUtils';
+import {getLinkedTransactionID, getReportActionMessage, isCreatedTaskReportAction, isRejectedAction} from '@libs/ReportActionsUtils';
 import {getOriginalReportID} from '@libs/ReportUtils';
 import {buildOptimisticSnapshotData} from '@libs/SearchQueryUtils';
 
@@ -33,7 +33,7 @@ Onyx.connectWithoutView({
     },
 });
 
-function clearReportActionErrors(reportAction: ReportAction, originalReportID: string | undefined, keys?: string[]) {
+function clearReportActionErrors(reportAction: ReportAction, originalReportID: string | undefined, keys?: string[], reportActions?: OnyxCollection<OnyxTypes.ReportActions>) {
     if (!reportAction?.reportActionID) {
         return;
     }
@@ -52,7 +52,7 @@ function clearReportActionErrors(reportAction: ReportAction, originalReportID: s
         // Also clean up any orphaned optimistic reject actions (REJECTED/REJECTED_TO_SUBMITTER)
         // from the same report. When a rejection fails, the comment action gets the error but the
         // corresponding reject action remains with pendingAction ADD and no errors.
-        const siblingActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${originalReportID}`];
+        const siblingActions = (reportActions ?? allReportActions)?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${originalReportID}`];
         if (siblingActions) {
             for (const [actionID, siblingAction] of Object.entries(siblingActions)) {
                 if (
@@ -125,21 +125,22 @@ type ClearAllRelatedReportActionErrorsParams = {
     ignore?: IgnoreDirection;
     keys?: string[];
     reports?: OnyxCollection<ReportHierarchyInfo>;
+    reportActions?: OnyxCollection<OnyxTypes.ReportActions>;
 };
 
-function clearAllRelatedReportActionErrors({reportID, reportAction, originalReportID, isOffline, ignore, keys, reports}: ClearAllRelatedReportActionErrorsParams) {
+function clearAllRelatedReportActionErrors({reportID, reportAction, originalReportID, isOffline, ignore, keys, reports, reportActions}: ClearAllRelatedReportActionErrorsParams) {
     const errorKeys = keys ?? Object.keys(reportAction?.errors ?? {});
     if (!reportAction || errorKeys.length === 0 || !reportID) {
         return;
     }
 
-    clearReportActionErrors(reportAction, originalReportID, keys);
+    clearReportActionErrors(reportAction, originalReportID, keys, reportActions);
 
     const report = (reports ?? allReports)?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`];
     if (report?.parentReportID && report?.parentReportActionID && ignore !== 'parent') {
-        const parentReportAction = getReportAction(report.parentReportID, report.parentReportActionID);
+        const parentReportActions = (reportActions ?? allReportActions)?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.parentReportID}`] ?? {};
+        const parentReportAction = parentReportActions[report.parentReportActionID];
         const parentErrorKeys = Object.keys(parentReportAction?.errors ?? {}).filter((err) => errorKeys.includes(err));
-        const parentReportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.parentReportID}`] ?? {};
         const parentOriginalReportID = getOriginalReportID(report.parentReportID, parentReportAction, parentReportActions, isOffline);
 
         clearAllRelatedReportActionErrors({
@@ -150,11 +151,12 @@ function clearAllRelatedReportActionErrors({reportID, reportAction, originalRepo
             ignore: 'child',
             keys: parentErrorKeys,
             reports,
+            reportActions,
         });
     }
 
     if (reportAction.childReportID && ignore !== 'child') {
-        const childActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportAction.childReportID}`] ?? {};
+        const childActions = (reportActions ?? allReportActions)?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${reportAction.childReportID}`] ?? {};
         for (const action of Object.values(childActions)) {
             const childErrorKeys = Object.keys(action.errors ?? {}).filter((err) => errorKeys.includes(err));
             const childOriginalReportID = getOriginalReportID(reportAction.childReportID, action, childActions, isOffline);
@@ -166,6 +168,7 @@ function clearAllRelatedReportActionErrors({reportID, reportAction, originalRepo
                 ignore: 'parent',
                 keys: childErrorKeys,
                 reports,
+                reportActions,
             });
         }
     }

tests/unit/ClearReportActionErrorsTest.ts

@@ -15,6 +15,7 @@ const PARENT_REPORT_ID = '2';
 const CHILD_REPORT_ID = '3';
 const GRANDPARENT_REPORT_ID = '4';
 const REPORT_ACTION_ID = '100';
+const SIBLING_REPORT_ACTION_ID = '101';
 const PARENT_REPORT_ACTION_ID = '200';
 const CHILD_REPORT_ACTION_ID = '300';
 const GRANDPARENT_REPORT_ACTION_ID = '400';
@@ -560,5 +561,198 @@ describe('ClearReportActionErrors', () => {
             const parentReportActions = await getReportActionsFromOnyx(PARENT_REPORT_ID);
             expect(parentReportActions?.[PARENT_REPORT_ACTION_ID]?.errors).toEqual({sharedError: 'Parent error message'});
         });
+
+        it('should clean up orphaned reject actions based on the reportActions parameter', async () => {
+            // Given an optimistic action in Onyx next to a sibling that Onyx knows as a plain submitted action,
+            // while the reportActions parameter presents that same sibling as an orphaned optimistic reject
+            // action (proving the sibling cleanup reads the caller-supplied data, not a hidden cache)
+            const reportAction = getFakeReportAction(Number(REPORT_ACTION_ID), {
+                errors: {error1: 'Error message'},
+                isOptimisticAction: true,
+            });
+            const seededSiblingAction = getFakeReportAction(Number(SIBLING_REPORT_ACTION_ID), {
+                pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
+                errors: undefined,
+            });
+            const rejectSiblingAction = getFakeReportAction(Number(SIBLING_REPORT_ACTION_ID), {
+                actionName: CONST.REPORT.ACTIONS.TYPE.REJECTED,
+                pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
+                errors: undefined,
+            });
+
+            await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, {
+                [REPORT_ACTION_ID]: reportAction,
+                [SIBLING_REPORT_ACTION_ID]: seededSiblingAction,
+            });
+            await waitForBatchedUpdates();
+
+            // When clearAllRelatedReportActionErrors is called with the sibling presented as a reject action in the parameter
+            clearAllRelatedReportActionErrors({
+                reportID: REPORT_ID,
+                reportAction,
+                originalReportID: REPORT_ID,
+                isOffline: false,
+                reportActions: {
+                    [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`]: {
+                        [REPORT_ACTION_ID]: reportAction,
+                        [SIBLING_REPORT_ACTION_ID]: rejectSiblingAction,
+                    },
+                },
+            });
+            await waitForBatchedUpdates();
+
+            // Then the sibling should be deleted because the parameter identified it as an orphaned reject action
+            const reportActions = await getReportActionsFromOnyx(REPORT_ID);
+            expect(reportActions?.[SIBLING_REPORT_ACTION_ID]).toBeUndefined();
+        });
+
+        it('should thread the reportActions parameter into the parent walk recursion', async () => {
+            // Given a parent action that Onyx knows as a plain action next to a non-reject sibling, while the
+            // reportActions parameter presents the parent action as optimistic and its sibling as an orphaned
+            // reject action — the sibling can only be deleted if the recursive parent call forwards the parameter
+            const reportAction = getFakeReportAction(Number(REPORT_ACTION_ID), {
+                errors: {sharedError: 'Error message'},
+            });
+            const seededParentReportAction = getFakeReportAction(Number(PARENT_REPORT_ACTION_ID), {
+                errors: {sharedError: 'Parent error message'},
+            });
+            const optimisticParentReportAction = getFakeReportAction(Number(PARENT_REPORT_ACTION_ID), {
+                errors: {sharedError: 'Parent error message'},
+                isOptimisticAction: true,
+            });
+            const seededSiblingAction = getFakeReportAction(Number(SIBLING_REPORT_ACTION_ID), {
+                pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
+                errors: undefined,
+            });
+            const rejectSiblingAction = getFakeReportAction(Number(SIBLING_REPORT_ACTION_ID), {
+                actionName: CONST.REPORT.ACTIONS.TYPE.REJECTED,
+                pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
+                errors: undefined,
+            });
+
+            await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, {
+                [REPORT_ACTION_ID]: reportAction,
+            });
+            await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${PARENT_REPORT_ID}`, {
+                [PARENT_REPORT_ACTION_ID]: seededParentReportAction,
+                [SIBLING_REPORT_ACTION_ID]: seededSiblingAction,
+            });
+            await waitForBatchedUpdates();
+
+            // When clearAllRelatedReportActionErrors is called with the full hierarchy and actions supplied via parameters
+            clearAllRelatedReportActionErrors({
+                reportID: REPORT_ID,
+                reportAction,
+                originalReportID: REPORT_ID,
+                isOffline: false,
+                reports: {
+                    [`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`]: {parentReportID: PARENT_REPORT_ID, parentReportActionID: PARENT_REPORT_ACTION_ID},
+                },
+                reportActions: {
+                    [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`]: {
+                        [REPORT_ACTION_ID]: reportAction,
+                    },
+                    [`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${PARENT_REPORT_ID}`]: {
+                        [PARENT_REPORT_ACTION_ID]: optimisticParentReportAction,
+                        [SIBLING_REPORT_ACTION_ID]: rejectSiblingAction,
+                    },
+                },
+            });
+            await waitForBatchedUpdates();
+
+            // Then the parent frame should have deleted the reject sibling, proving it read the forwarded parameter
+            const parentReportActions = await getReportActionsFromOnyx(PARENT_REPORT_ID);
+            expect(parentReportActions?.[SIBLING_REPORT_ACTION_ID]).toBeUndefined();
+        });
+
+        it('should not clean up reject actions when the supplied reportActions collection is missing the data', async () => {
+            // Given an optimistic action and an orphaned optimistic reject sibling both seeded in Onyx, but a
+            // caller-supplied reportActions collection that is empty (the function must trust the caller's data
+            // and not fall back to the module-level cache per key)
+            const reportAction = getFakeReportAction(Number(REPORT_ACTION_ID), {
+                errors: {error1: 'Error message'},
+                isOptimisticAction: true,
+            });
+            const rejectSiblingAction = getFakeReportAction(Number(SIBLING_REPORT_ACTION_ID), {
+                actionName: CONST.REPORT.ACTIONS.TYPE.REJECTED,
+                pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD,
+                errors: undefined,
+            });
+
+            await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, {
+                [REPORT_ACTION_ID]: reportAction,
+                [SIBLING_REPORT_ACTION_ID]: rejectSiblingAction,
+            });
+            await waitForBatchedUpdates();
+
+            // When clearAllRelatedReportActionErrors is called with an empty reportActions collection
+            clearAllRelatedReportActionErrors({reportID: REPORT_ID, reportAction, originalReportID: REPORT_ID, isOffline: false, reportActions: {}});
+            await waitForBatchedUpdates();
+
+            // Then the reject sibling should remain because the supplied collection has no data for the report
+            const reportActions = await getReportActionsFromOnyx(REPORT_ID);
+            expect(reportActions?.[SIBLING_REPORT_ACTION_ID]).toBeDefined();
+        });
+
+        it('should not clear child errors when the supplied reportActions collection is missing the data', async () => {
+            // Given a parent action with matching child errors seeded in Onyx, but a caller-supplied
+            // reportActions collection that is empty, so the child walk has nothing to iterate
+            const reportAction = getFakeReportAction(Number(REPORT_ACTION_ID), {
+                childReportID: CHILD_REPORT_ID,
+                errors: {sharedError: 'Error message'},
+            });
+            const childReportAction = getFakeReportAction(Number(CHILD_REPORT_ACTION_ID), {
+                errors: {sharedError: 'Child error message'},
+            });
+
+            await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, createMockReport());
+            await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, {
+                [REPORT_ACTION_ID]: reportAction,
+            });
+            await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${CHILD_REPORT_ID}`, {
+                [CHILD_REPORT_ACTION_ID]: childReportAction,
+            });
+            await waitForBatchedUpdates();
+
+            // When clearAllRelatedReportActionErrors is called with an empty reportActions collection
+            clearAllRelatedReportActionErrors({reportID: REPORT_ID, reportAction, originalReportID: REPORT_ID, isOffline: false, reportActions: {}});
+            await waitForBatchedUpdates();
+
+            // Then the child action's error should remain because the supplied collection has no child actions
+            const childReportActions = await getReportActionsFromOnyx(CHILD_REPORT_ID);
+            expect(childReportActions?.[CHILD_REPORT_ACTION_ID]?.errors).toEqual({sharedError: 'Child error message'});
+        });
+
+        it('should not clear parent errors when the supplied reportActions collection is missing the data', async () => {
+            // Given a report whose parent action with matching errors exists in Onyx, but a caller-supplied
+            // reportActions collection that is empty, so the parent action cannot be resolved from the input
+            const report = createMockReport({
+                parentReportID: PARENT_REPORT_ID,
+                parentReportActionID: PARENT_REPORT_ACTION_ID,
+            });
+            const reportAction = getFakeReportAction(Number(REPORT_ACTION_ID), {
+                errors: {sharedError: 'Error message'},
+            });
+            const parentReportAction = getFakeReportAction(Number(PARENT_REPORT_ACTION_ID), {
+                errors: {sharedError: 'Parent error message'},
+            });
+
+            await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, report);
+            await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, {
+                [REPORT_ACTION_ID]: reportAction,
+            });
+            await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${PARENT_REPORT_ID}`, {
+                [PARENT_REPORT_ACTION_ID]: parentReportAction,
+            });
+            await waitForBatchedUpdates();
+
+            // When clearAllRelatedReportActionErrors is called with an empty reportActions collection
+            clearAllRelatedReportActionErrors({reportID: REPORT_ID, reportAction, originalReportID: REPORT_ID, isOffline: false, reportActions: {}});
+            await waitForBatchedUpdates();
+
+            // Then the parent action's error should remain because the parent action was not in the supplied collection
+            const parentReportActions = await getReportActionsFromOnyx(PARENT_REPORT_ID);
+            expect(parentReportActions?.[PARENT_REPORT_ACTION_ID]?.errors).toEqual({sharedError: 'Parent error message'});
+        });
     });
 });