Merge pull request #99814 from Abdukhamid000/revert-98012-split-defer

Revert "perf: defer the split-save API.write behind the destination layout"

(cherry picked from commit b8aba2b)

(cherry-picked to staging by francoisl)

Author: francoisl

src/libs/API/__mocks__/writeWhenReady.ts

@@ -3,9 +3,8 @@ import type {SearchActionsContextValue, SearchStateContextValue} from '@componen
 
 import type {CurrencyListActionsContextType} from '@hooks/useCurrencyList';
 
-import {createTransitionBarrier, write as apiWrite, writeWhenReady} from '@libs/API';
+import {write as apiWrite} from '@libs/API';
 import type {RevertSplitTransactionParams, SplitTransactionParams, SplitTransactionSplitsParam} from '@libs/API/parameters';
-import type {ApiRequestCommandParameters, WriteCommand} from '@libs/API/types';
 import {WRITE_COMMANDS} from '@libs/API/types';
 import DateUtils from '@libs/DateUtils';
 import {getMicroSecondOnyxErrorWithTranslationKey} from '@libs/ErrorUtils';
@@ -1990,24 +1989,14 @@ function updateSplitTransactions({
         }
     }
 
-    // optimisticData is applied synchronously, re-rendering the destination screen mid-transition.
-    // Defer the write until the transition ends, unless there is none to wait on.
-    const writeSplit = <TCommand extends WriteCommand>(command: TCommand, parameters: ApiRequestCommandParameters[TCommand]) => {
-        if (!isFromSplitExpensesFlow) {
-            apiWrite(command, parameters, onyxData);
-            return;
-        }
-        writeWhenReady(command, parameters, onyxData, createTransitionBarrier('navigation'));
-    };
-
     if (isReverseSplitOperation) {
         const parameters = {
             ...splits.at(0),
             comment: splits.at(0)?.comment?.comment,
             waypoints: splits.at(0)?.waypoints ? JSON.stringify(splits.at(0)?.waypoints) : undefined,
             copiedComments: splits.at(0)?.copiedComments ? JSON.stringify(splits.at(0)?.copiedComments) : undefined,
         } as RevertSplitTransactionParams;
-        writeSplit(WRITE_COMMANDS.REVERT_SPLIT_TRANSACTION, parameters);
+        apiWrite(WRITE_COMMANDS.REVERT_SPLIT_TRANSACTION, parameters, onyxData);
     } else {
         // Prepare splitApiParams for the Transaction_Split API call which requires a specific format for the splits
         // The format is: splits[0][amount], splits[0][category], splits[0][tag] etc.
@@ -2023,7 +2012,11 @@ function updateSplitTransactions({
             transactionID: originalTransactionID,
         };
 
-        writeSplit(isCreationOfSplits ? WRITE_COMMANDS.SPLIT_TRANSACTION : WRITE_COMMANDS.UPDATE_SPLIT_TRANSACTION, splitParameters);
+        if (isCreationOfSplits) {
+            apiWrite(WRITE_COMMANDS.SPLIT_TRANSACTION, splitParameters, onyxData);
+        } else {
+            apiWrite(WRITE_COMMANDS.UPDATE_SPLIT_TRANSACTION, splitParameters, onyxData);
+        }
     }
     TransitionTracker.runAfterTransitions({callback: () => removeDraftSplitTransaction(originalTransactionID), waitForUpcomingTransition: true});
 }
@@ -2062,7 +2055,11 @@ function updateSplitTransactionsFromSplitExpensesFlow(params: UpdateSplitTransac
     const reverseSplitKeepsOriginalInExpenseReport = isReverseSplitOperation && splitExpenses.at(0)?.reportID === expenseReportID;
     const willExpenseReportBecomeEmpty =
         !!expenseReportID && areAllExpenseReportTransactionsSplitChildren && !anyRemainingSplitStaysInExpenseReport && !reverseSplitKeepsOriginalInExpenseReport;
-    const isLastTransactionInReport = willExpenseReportBecomeEmpty || (isReverseSplitOperation && !reverseSplitKeepsOriginalInExpenseReport && expenseReportTransactions.length === 1);
+    const isLastTransactionInReport =
+        willExpenseReportBecomeEmpty ||
+        (isReverseSplitOperation &&
+            !reverseSplitKeepsOriginalInExpenseReport &&
+            Object.values(params.allTransactionsList ?? {}).filter((itemTransaction) => itemTransaction?.reportID === expenseReportID).length === 1);
     const fallbackReportID = params.expenseReport?.chatReportID ?? params.expenseReport?.parentReportID;
 
     if (isLastTransactionInReport && fallbackReportID) {
@@ -2192,4 +2189,3 @@ function updateSplitTransactionsFromSplitExpensesFlow(params: UpdateSplitTransac
 }
 
 export {updateSplitTransactions, updateSplitTransactionsFromSplitExpensesFlow};
-export type {UpdateSplitTransactionsParams};

src/libs/actions/IOU/SplitTransactionUpdate.ts

@@ -69,7 +69,6 @@ jest.mock('@libs/actions/IOU/PendingNewTransactions', () => ({
     deletePendingNewTransactionIDs: jest.fn(),
     isOneToTwoTransactionTransition: jest.fn(() => false),
 }));
-jest.mock('@libs/API/writeWhenReady');
 // In production, requestMoney defers its API.write() call until the target screen's
 // content lays out (or a safety timeout fires). In tests there is no target component
 // to flush the deferred write, so we bypass the deferral by executing the callback immediately.

tests/actions/IOU/SplitReportTotalsTest.ts

@@ -56,7 +56,6 @@ jest.mock('@src/libs/actions/Report', () => {
 
 jest.mock('@libs/Navigation/helpers/isSearchTopmostFullScreenRoute', () => jest.fn());
 
-jest.mock('@libs/API/writeWhenReady');
 jest.mock('@src/libs/SearchQueryUtils', () => {
     // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
     const actual = jest.requireActual('@src/libs/SearchQueryUtils');

tests/actions/IOUTest/SplitSelfDMTest.ts

@@ -10,11 +10,8 @@ import {createWorkspace, generatePolicyID, setWorkspaceApprovalMode} from '@libs
 import {addComment, notifyNewAction} from '@libs/actions/Report';
 import initSplitExpense from '@libs/actions/SplitExpenses';
 import type * as API from '@libs/API';
-import type {WriteCommand} from '@libs/API/types';
 import {WRITE_COMMANDS} from '@libs/API/types';
-import {createTransitionBarrier, writeWhenReady} from '@libs/API/writeWhenReady';
 import {getCurrencyDecimals, getCurrencySymbol} from '@libs/CurrencyUtils';
-import isSearchTopmostFullScreenRoute from '@libs/Navigation/helpers/isSearchTopmostFullScreenRoute';
 import {rand64} from '@libs/NumberUtils';
 import {getIOUActionForReportID, getIOUActionForTransactionID, getOriginalMessage, isActionOfType, isAddCommentAction, isDeletedAction, isMoneyRequestAction} from '@libs/ReportActionsUtils';
 import {buildOptimisticIOUReportAction, getAncestors, getReportOrDraftReport} from '@libs/ReportUtils';
@@ -38,7 +35,6 @@ import {
     updateSplitExpenseAmountField,
     updateSplitExpenseField,
 } from '@userActions/IOU/SplitExpenseItems';
-import type {UpdateSplitTransactionsParams} from '@userActions/IOU/SplitTransactionUpdate';
 import {updateSplitTransactions, updateSplitTransactionsFromSplitExpensesFlow} from '@userActions/IOU/SplitTransactionUpdate';
 
 import CONST from '@src/CONST';
@@ -125,7 +121,6 @@ jest.mock('@src/libs/actions/Report', () => {
 
 jest.mock('@libs/Navigation/helpers/isSearchTopmostFullScreenRoute', () => jest.fn());
 jest.mock('@libs/Navigation/helpers/isReportTopmostSplitNavigator', () => jest.fn());
-jest.mock('@libs/API/writeWhenReady');
 jest.mock('@libs/deferredLayoutWrite', () => ({
     registerDeferredWrite: (_key: string, callback: () => void) => callback(),
     flushDeferredWrite: jest.fn(),
@@ -10305,221 +10300,6 @@ describe('startSplitBill delegateAccountID forwarding', () => {
     });
 });
 
-/**
- * Fixture for the save tests below: an expense report with a single transaction, plus a draft that splits that
- * transaction into two.
- *
- * `withExistingSplitChildren` also seeds those two split transactions in Onyx. That is what makes a one-item
- * `splitExpenses` mean "collapse an existing split back into one expense" (a reverse split) rather than "split
- * this expense into one".
- */
-const buildSplitFlowParams = async ({withExistingSplitChildren = false, asSelfDMThread = false} = {}) => {
-    const expenseReport: Report = {
-        ...createRandomReport(9001, undefined),
-        type: CONST.REPORT.TYPE.EXPENSE,
-    };
-    const transaction: Transaction = {
-        amount: 100,
-        currency: 'USD',
-        transactionID: '9001',
-        reportID: expenseReport.reportID,
-        created: DateUtils.getDBTime(),
-        merchant: 'test',
-    };
-    // `asSelfDMThread` makes the thread a selfDM child of the expense report, which is the only shape that
-    // reaches the navigate-first branch in `updateSplitTransactionsFromSplitExpensesFlow`.
-    const transactionThread: Report = {
-        ...createRandomReport(9002, undefined),
-        type: CONST.REPORT.TYPE.CHAT,
-        ...(asSelfDMThread ? {chatType: CONST.REPORT.CHAT_TYPE.SELF_DM, parentReportID: expenseReport.reportID} : {}),
-    };
-    const iouAction: ReportAction = {
-        ...buildOptimisticIOUReportAction({
-            type: CONST.IOU.REPORT_ACTION_TYPE.CREATE,
-            amount: transaction.amount,
-            currency: transaction.currency,
-            comment: '',
-            participants: [],
-            transactionID: transaction.transactionID,
-            iouReportID: expenseReport.reportID,
-            getCurrencyDecimals: getCurrencyDecimalsLocal,
-        }),
-        childReportID: transactionThread.reportID,
-    };
-
-    await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${expenseReport.reportID}`, expenseReport);
-    await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${transactionThread.reportID}`, transactionThread);
-    await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${expenseReport.reportID}`, {[iouAction.reportActionID]: iouAction});
-    await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${transaction.transactionID}`, transaction);
-    if (withExistingSplitChildren) {
-        for (const childTransactionID of ['9003', '9004']) {
-            await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}${childTransactionID}`, {
-                ...transaction,
-                transactionID: childTransactionID,
-                amount: 50,
-                comment: {originalTransactionID: transaction.transactionID, source: CONST.IOU.TYPE.SPLIT},
-            });
-        }
-    }
-    await waitForBatchedUpdates();
-
-    let allTransactions: OnyxCollection<Transaction>;
-    let allReports: OnyxCollection<Report>;
-    await getOnyxData({
-        key: ONYXKEYS.COLLECTION.TRANSACTION,
-        callback: (value) => {
-            allTransactions = value;
-        },
-    });
-    await getOnyxData({
-        key: ONYXKEYS.COLLECTION.REPORT,
-        callback: (value) => {
-            allReports = value;
-        },
-    });
-
-    const reports = getTransactionAndExpenseReports(asSelfDMThread ? transactionThread.reportID : expenseReport.reportID);
-    const params: UpdateSplitTransactionsParams = {
-        allTransactionsList: allTransactions,
-        allReportsList: allReports,
-        allReportActionsList: undefined,
-        allReportNameValuePairsList: undefined,
-        transactionData: {
-            reportID: expenseReport.reportID,
-            originalTransactionID: transaction.transactionID,
-            splitExpenses: [
-                {transactionID: '9003', amount: 50, description: '', category: '', tags: [''], created: transaction.created, reportID: expenseReport.reportID},
-                {transactionID: '9004', amount: 50, description: '', category: '', tags: [''], created: transaction.created, reportID: expenseReport.reportID},
-            ],
-            splitExpensesTotal: 100,
-        },
-        searchContext: {currentSearchHash: -2},
-        policyCategories: undefined,
-        policy: undefined,
-        policyRecentlyUsedCategories: [],
-        iouReport: expenseReport,
-        firstIOU: iouAction,
-        isASAPSubmitBetaEnabled: false,
-        currentUserPersonalDetails,
-        transactionViolations: {},
-        policyRecentlyUsedCurrencies: [],
-        quickAction: undefined,
-        betas: [CONST.BETAS.ALL],
-        allPolicyTags: undefined,
-        personalDetails: {[RORY_ACCOUNT_ID]: {accountID: RORY_ACCOUNT_ID, login: RORY_EMAIL}},
-        transactionReport: reports.transactionReport,
-        expenseReport: reports.expenseReport,
-        isOffline: false,
-        delegateAccountID: undefined,
-        isTrackIntentUser: false,
-        getCurrencyDecimals: getCurrencyDecimalsLocal,
-        formatPhoneNumber,
-        getCurrencySymbol: getCurrencySymbolLocal,
-    };
-
-    return {expenseReport, iouAction, params};
-};
-
-/**
- * `@libs/API` re-declares `writeWhenReady` rather than re-exporting it, and its wrapper always forwards the
- * optional 5th argument. So the arity the mock records is the wrapper's, not the call site's, and asserting
- * with `toHaveBeenCalledWith` breaks whenever that wrapper's signature grows. Assert the arguments this flow
- * actually chooses - the command and the barrier - and ignore the rest.
- */
-function expectDeferredWriteFor(command: WriteCommand) {
-    const deferredCall = jest.mocked(writeWhenReady).mock.calls.find(([calledCommand]) => calledCommand === command);
-    expect(deferredCall).toBeDefined();
-    expect(deferredCall?.at(3)).toEqual(expect.any(Function));
-}
-
-describe('split save deferred write', () => {
-    beforeEach(() => {
-        jest.mocked(isSearchTopmostFullScreenRoute).mockReturnValue(false);
-    });
-
-    it('defers the write behind a navigation barrier when saving from the split-expenses flow', async () => {
-        // Given a split saved through the split-expenses flow
-        const {params} = await buildSplitFlowParams();
-
-        // When the split is saved
-        updateSplitTransactionsFromSplitExpensesFlow(params);
-        await waitForBatchedUpdates();
-
-        // Then the write goes through writeWhenReady gated on a screen transition specifically, so its
-        // optimistic data does not land while the press is still trying to paint, and a stray modal or
-        // keyboard blip cannot release it early
-        expect(createTransitionBarrier).toHaveBeenCalledWith('navigation');
-        expectDeferredWriteFor(WRITE_COMMANDS.SPLIT_TRANSACTION);
-    });
-
-    it('navigates back to the selfDM before deferring the write when Search is not topmost', async () => {
-        const Navigation = jest.requireMock('@src/libs/Navigation/Navigation');
-
-        // Given a split whose transaction thread is a selfDM, the one shape that reaches the navigate-first branch
-        const {params} = await buildSplitFlowParams({asSelfDMThread: true});
-
-        // When it is saved while Search is not the topmost full screen route
-        updateSplitTransactionsFromSplitExpensesFlow(params);
-        await waitForBatchedUpdates();
-
-        // Then the flow navigates back to the selfDM first, so the transaction thread is off screen before the
-        // data changes under it. The write is still deferred, it just happens after that navigation.
-        expect(Navigation.dismissModal).toHaveBeenCalled();
-        expectDeferredWriteFor(WRITE_COMMANDS.SPLIT_TRANSACTION);
-    });
-
-    it('defers the write for the same selfDM split when the Search page is topmost', async () => {
-        const Navigation = jest.requireMock('@src/libs/Navigation/Navigation');
-
-        // Given the identical selfDM fixture, changed only by Search being the topmost full screen route
-        jest.mocked(isSearchTopmostFullScreenRoute).mockReturnValue(true);
-        const {params} = await buildSplitFlowParams({asSelfDMThread: true});
-
-        // When the split is saved
-        updateSplitTransactionsFromSplitExpensesFlow(params);
-        await waitForBatchedUpdates();
-
-        // Then the navigate-first branch is skipped and the write goes through the barrier instead
-        expect(Navigation.dismissModal).not.toHaveBeenCalled();
-        expect(createTransitionBarrier).toHaveBeenCalledWith('navigation');
-        expectDeferredWriteFor(WRITE_COMMANDS.SPLIT_TRANSACTION);
-    });
-
-    it('defers the reverse-split write behind the same barrier', async () => {
-        // Given an existing two-way split being collapsed back to a single expense
-        const {params} = await buildSplitFlowParams({withExistingSplitChildren: true});
-        const remainingSplit = params.transactionData.splitExpenses.at(0);
-
-        // When the reverse split is saved from the split-expenses flow
-        updateSplitTransactions({
-            ...params,
-            transactionData: {...params.transactionData, splitExpenses: remainingSplit ? [remainingSplit] : []},
-            isFromSplitExpensesFlow: true,
-        });
-        await waitForBatchedUpdates();
-
-        // Then REVERT_SPLIT_TRANSACTION goes through the same fork as the creation path
-        expect(createTransitionBarrier).toHaveBeenCalledWith('navigation');
-        expectDeferredWriteFor(WRITE_COMMANDS.REVERT_SPLIT_TRANSACTION);
-    });
-
-    it('writes immediately when the caller is not the split-expenses flow', async () => {
-        // Given a direct updateSplitTransactions call, as useDeleteTransactions makes
-        const {params} = await buildSplitFlowParams();
-        const writeSpy = jest.spyOn(APIlib, 'write');
-
-        // When it runs outside the split-expenses flow
-        updateSplitTransactions({...params, isFromSplitExpensesFlow: false});
-        await waitForBatchedUpdates();
-
-        // Then the write still happens, it is just not deferred behind a barrier
-        expect(writeSpy).toHaveBeenCalledWith(WRITE_COMMANDS.SPLIT_TRANSACTION, expect.anything(), expect.anything());
-        expect(writeWhenReady).not.toHaveBeenCalled();
-        expect(createTransitionBarrier).not.toHaveBeenCalled();
-        writeSpy.mockRestore();
-    });
-});
-
 describe('resolveOptimisticSplitChatReportID', () => {
     it('returns the existing chat and no optimistic ID when the participants already share one', async () => {
         const reportID = 'existing-split-chat';

tests/actions/IOUTest/SplitTest.ts

@@ -101,7 +101,6 @@ jest.mock('@src/libs/actions/Report', () => {
 });
 jest.mock('@libs/Navigation/helpers/isSearchTopmostFullScreenRoute', () => jest.fn());
 jest.mock('@libs/Navigation/helpers/isReportTopmostSplitNavigator', () => jest.fn());
-jest.mock('@libs/API/writeWhenReady');
 // In production, requestMoney defers its API.write() call until the target screen's
 // content lays out (or a safety timeout fires). In tests there is no target component
 // to flush the deferred write, so we bypass the deferral by executing the callback immediately.