diff --git a/src/hooks/useBulkDuplicateReportAction.ts b/src/hooks/useBulkDuplicateReportAction.ts index f3c31d00ea90..109999968ea0 100644 --- a/src/hooks/useBulkDuplicateReportAction.ts +++ b/src/hooks/useBulkDuplicateReportAction.ts @@ -2,6 +2,7 @@ import {useSearchSelectionActions} from '@components/Search/SearchContext'; import type {SelectedReports} from '@components/Search/types'; import {bulkDuplicateReports} from '@libs/actions/IOU/Duplicate'; +import Log from '@libs/Log'; import {getPolicyExpenseChat} from '@libs/ReportUtils'; import CONST from '@src/CONST'; @@ -80,6 +81,8 @@ function useBulkDuplicateReportAction({selectedReports, allReports, searchData}: formatPhoneNumber, getCurrencyDecimals, conciergeChat, + }).catch((error: unknown) => { + Log.warn('[useBulkDuplicateReportAction] Failed to duplicate the selected reports', {error}); }); clearSelectedTransactions(undefined, true); diff --git a/src/libs/actions/IOU/Duplicate.ts b/src/libs/actions/IOU/Duplicate.ts index 16b8390baa7d..eb886b0f9977 100644 --- a/src/libs/actions/IOU/Duplicate.ts +++ b/src/libs/actions/IOU/Duplicate.ts @@ -63,7 +63,7 @@ import type {PerDiemExpenseInformation} from './PerDiem'; import type {CreateDistanceRequestInformation} from './Split'; import type {CreateTrackExpenseParams} from './TrackExpense'; -import {getAllReports, getAllTransactions} from '.'; +import {getAllReports, getAllTransactions, getCurrentUserAccountIDFromSession} from '.'; import {getCleanUpTransactionThreadReportOnyxData} from './DeleteMoneyRequest'; import {getMoneyRequestParticipantsFromReport} from './MoneyRequest'; import {submitPerDiemExpense} from './PerDiem'; @@ -1352,7 +1352,7 @@ type BulkDuplicateReportsParams = { conciergeChat: OnyxEntry; }; -function bulkDuplicateReports({ +async function bulkDuplicateReports({ dateFnsLocale, selectedReports: selectedReportsParam, allReports, @@ -1404,6 +1404,9 @@ function bulkDuplicateReports({ transactionsByReportID.set(transaction.reportID, list); } + let hasDuplicatedReport = false; + const accountIDAtStart = getCurrentUserAccountIDFromSession(); + for (const selectedReport of selectedReportsParam) { const reportID = selectedReport.reportID; if (!reportID) { @@ -1420,6 +1423,21 @@ function bulkDuplicateReports({ continue; } + if (hasDuplicatedReport) { + // Temporary until the backend exposes a single command that duplicates a whole selection. Until then, + // let the previous report's optimistic writes apply before blocking the thread again. + // eslint-disable-next-line no-await-in-loop + await new Promise((resolve) => { + setTimeout(resolve, 0); + }); + + // Signing out clears Onyx, so the reports and policies captured above belong to an account that is no + // longer active. Duplicating the rest of them would write the previous account's data under the new one. + if (getCurrentUserAccountIDFromSession() !== accountIDAtStart) { + return; + } + } + const reportPolicy = allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${report.policyID}`]; const isSourcePolicyValid = !!reportPolicy && isPolicyAccessible(reportPolicy, currentUserLogin); const chatReportID = report.chatReportID ?? report.parentReportID; @@ -1464,6 +1482,8 @@ function bulkDuplicateReports({ participantsPolicyTags, conciergeChat, }); + + hasDuplicatedReport = true; } playSound(SOUNDS.DONE); diff --git a/src/libs/actions/IOU/index.ts b/src/libs/actions/IOU/index.ts index 7a72470aa2ed..52eaecfaad3f 100644 --- a/src/libs/actions/IOU/index.ts +++ b/src/libs/actions/IOU/index.ts @@ -169,6 +169,10 @@ function getCurrentUserPersonalDetails(): OnyxEntry { return deprecatedCurrentUserPersonalDetails; } +function getCurrentUserAccountIDFromSession(): number { + return deprecatedUserAccountID; +} + function getRecentAttendees(): OnyxEntry { return recentAttendees; } @@ -191,6 +195,7 @@ export { getAllReportNameValuePairs, getAllTransactionDrafts, getCurrentUserPersonalDetails, + getCurrentUserAccountIDFromSession, getRecentAttendees, getAllSnapshots, getSearchQueryByHash, diff --git a/tests/actions/IOUTest/DuplicateTest.ts b/tests/actions/IOUTest/DuplicateTest.ts index ebe2d1225a45..e3fbfad7ec28 100644 --- a/tests/actions/IOUTest/DuplicateTest.ts +++ b/tests/actions/IOUTest/DuplicateTest.ts @@ -3623,13 +3623,74 @@ describe('actions/Duplicate', () => { [`${ONYXKEYS.COLLECTION.REPORT}${ACTIVE_PEC_REPORT_ID}`]: activePolicyExpenseChat, }; - bulkDuplicateReports(getDefaultBulkParams(['rpt1', 'rpt2'], {allReports})); + await bulkDuplicateReports(getDefaultBulkParams(['rpt1', 'rpt2'], {allReports})); await waitForBatchedUpdates(); expect(countWriteCommandCalls(WRITE_COMMANDS.CREATE_APP_REPORT)).toBe(2); expect(countWriteCommandCalls(WRITE_COMMANDS.REQUEST_MONEY)).toBe(2); }); + it('should not duplicate every selected report in a single synchronous pass', async () => { + const reportIDs = ['rpt1', 'rpt2', 'rpt3']; + const allReports: Record = { + [`${ONYXKEYS.COLLECTION.REPORT}${ACTIVE_PEC_REPORT_ID}`]: activePolicyExpenseChat, + }; + + for (const reportID of reportIDs) { + allReports[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`] = { + reportID, + policyID: SOURCE_POLICY_ID, + ownerAccountID: RORY_ACCOUNT_ID, + type: CONST.REPORT.TYPE.EXPENSE, + reportName: `Report ${reportID}`, + chatReportID: ACTIVE_PEC_REPORT_ID, + }; + await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}tx-${reportID}`, createCashTransaction(`1${reportID.slice(-1)}`, reportID)); + } + + const duplicating = bulkDuplicateReports(getDefaultBulkParams(reportIDs, {allReports})); + + expect(countWriteCommandCalls(WRITE_COMMANDS.CREATE_APP_REPORT)).toBe(1); + expect(countWriteCommandCalls(WRITE_COMMANDS.REQUEST_MONEY)).toBe(1); + + await duplicating; + await waitForBatchedUpdates(); + + expect(countWriteCommandCalls(WRITE_COMMANDS.CREATE_APP_REPORT)).toBe(3); + expect(countWriteCommandCalls(WRITE_COMMANDS.REQUEST_MONEY)).toBe(3); + }); + + it('should stop duplicating the remaining reports when the account changes mid-flight', async () => { + await Onyx.merge(ONYXKEYS.SESSION, {accountID: RORY_ACCOUNT_ID}); + await waitForBatchedUpdates(); + + const reportIDs = ['rpt1', 'rpt2', 'rpt3']; + const allReports: Record = { + [`${ONYXKEYS.COLLECTION.REPORT}${ACTIVE_PEC_REPORT_ID}`]: activePolicyExpenseChat, + }; + for (const reportID of reportIDs) { + allReports[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`] = { + reportID, + policyID: SOURCE_POLICY_ID, + ownerAccountID: RORY_ACCOUNT_ID, + type: CONST.REPORT.TYPE.EXPENSE, + reportName: `Report ${reportID}`, + chatReportID: ACTIVE_PEC_REPORT_ID, + }; + await Onyx.merge(`${ONYXKEYS.COLLECTION.TRANSACTION}tx-${reportID}`, createCashTransaction(`1${reportID.slice(-1)}`, reportID)); + } + + const duplicating = bulkDuplicateReports(getDefaultBulkParams(reportIDs, {allReports})); + expect(countWriteCommandCalls(WRITE_COMMANDS.CREATE_APP_REPORT)).toBe(1); + + await Onyx.merge(ONYXKEYS.SESSION, {accountID: RORY_ACCOUNT_ID + 1}); + await waitForBatchedUpdates(); + await duplicating; + + expect(countWriteCommandCalls(WRITE_COMMANDS.CREATE_APP_REPORT)).toBe(1); + expect(countWriteCommandCalls(WRITE_COMMANDS.REQUEST_MONEY)).toBe(1); + }); + it('should use source policy when accessible, and fall back to default policy when not', async () => { const chatForSource: Report = { reportID: 'chatSource', @@ -3671,7 +3732,7 @@ describe('actions/Duplicate', () => { [`${ONYXKEYS.COLLECTION.REPORT}${ACTIVE_PEC_REPORT_ID}`]: activePolicyExpenseChat, }; - bulkDuplicateReports( + await bulkDuplicateReports( getDefaultBulkParams(['rpt1', 'rpt2'], { allReports, allPolicyCategories: { @@ -3715,7 +3776,7 @@ describe('actions/Duplicate', () => { [`${ONYXKEYS.COLLECTION.REPORT}${ACTIVE_PEC_REPORT_ID}`]: activePolicyExpenseChat, }; - bulkDuplicateReports(getDefaultBulkParams(['rpt1'], {allReports})); + await bulkDuplicateReports(getDefaultBulkParams(['rpt1'], {allReports})); await waitForBatchedUpdates(); expect(countWriteCommandCalls(WRITE_COMMANDS.CREATE_APP_REPORT)).toBe(1); @@ -3740,7 +3801,7 @@ describe('actions/Duplicate', () => { [`${ONYXKEYS.COLLECTION.REPORT}${ACTIVE_PEC_REPORT_ID}`]: activePolicyExpenseChat, }; - bulkDuplicateReports(getDefaultBulkParams(['rpt1'], {allReports})); + await bulkDuplicateReports(getDefaultBulkParams(['rpt1'], {allReports})); await waitForBatchedUpdates(); expect(countWriteCommandCalls(WRITE_COMMANDS.CREATE_APP_REPORT)).toBe(1); @@ -3776,7 +3837,7 @@ describe('actions/Duplicate', () => { [`${ONYXKEYS.COLLECTION.REPORT}${ACTIVE_PEC_REPORT_ID}`]: activePolicyExpenseChat, }; - bulkDuplicateReports(getDefaultBulkParams(['rpt1'], {allReports})); + await bulkDuplicateReports(getDefaultBulkParams(['rpt1'], {allReports})); await waitForBatchedUpdates(); expect(countWriteCommandCalls(WRITE_COMMANDS.CREATE_APP_REPORT)).toBe(1); @@ -3804,7 +3865,7 @@ describe('actions/Duplicate', () => { [`${ONYXKEYS.COLLECTION.REPORT}${ACTIVE_PEC_REPORT_ID}`]: activePolicyExpenseChat, }; - bulkDuplicateReports(getDefaultBulkParams(['rpt1', 'nonexistent1', 'nonexistent2'], {allReports})); + await bulkDuplicateReports(getDefaultBulkParams(['rpt1', 'nonexistent1', 'nonexistent2'], {allReports})); await waitForBatchedUpdates(); expect(countWriteCommandCalls(WRITE_COMMANDS.CREATE_APP_REPORT)).toBe(1); @@ -3826,7 +3887,7 @@ describe('actions/Duplicate', () => { [`${ONYXKEYS.COLLECTION.REPORT}${ACTIVE_PEC_REPORT_ID}`]: activePolicyExpenseChat, }; - bulkDuplicateReports(getDefaultBulkParams(['rpt1'], {allReports})); + await bulkDuplicateReports(getDefaultBulkParams(['rpt1'], {allReports})); await waitForBatchedUpdates(); expect(countWriteCommandCalls(WRITE_COMMANDS.CREATE_APP_REPORT)).toBe(1); @@ -3864,7 +3925,7 @@ describe('actions/Duplicate', () => { [`${ONYXKEYS.COLLECTION.REPORT}${ACTIVE_PEC_REPORT_ID}`]: activePolicyExpenseChat, }; - bulkDuplicateReports(getDefaultBulkParams(['rpt1', 'rpt2'], {allReports})); + await bulkDuplicateReports(getDefaultBulkParams(['rpt1', 'rpt2'], {allReports})); await waitForBatchedUpdates(); expect(countWriteCommandCalls(WRITE_COMMANDS.CREATE_APP_REPORT)).toBe(2); @@ -3918,7 +3979,7 @@ describe('actions/Duplicate', () => { [`${ONYXKEYS.COLLECTION.REPORT}${ACTIVE_PEC_REPORT_ID}`]: activePolicyExpenseChat, }; - bulkDuplicateReports( + await bulkDuplicateReports( getDefaultBulkParams(['rpt1', 'rpt2'], { allReports, allPolicyTags: { @@ -3958,7 +4019,7 @@ describe('actions/Duplicate', () => { [`${ONYXKEYS.COLLECTION.REPORT}${ACTIVE_PEC_REPORT_ID}`]: activePolicyExpenseChat, }; - bulkDuplicateReports(getDefaultBulkParams(['rpt1'], {allReports})); + await bulkDuplicateReports(getDefaultBulkParams(['rpt1'], {allReports})); await waitForBatchedUpdates(); expect(countWriteCommandCalls(WRITE_COMMANDS.CREATE_APP_REPORT)).toBe(1); diff --git a/tests/unit/hooks/useBulkDuplicateReportActionTest.ts b/tests/unit/hooks/useBulkDuplicateReportActionTest.ts index 993aec57dfc3..2b36e413be93 100644 --- a/tests/unit/hooks/useBulkDuplicateReportActionTest.ts +++ b/tests/unit/hooks/useBulkDuplicateReportActionTest.ts @@ -16,7 +16,7 @@ import Onyx from 'react-native-onyx'; import createMock from '../../utils/createMock'; jest.mock('@libs/actions/IOU/Duplicate', () => ({ - bulkDuplicateReports: jest.fn(), + bulkDuplicateReports: jest.fn(() => Promise.resolve()), })); jest.mock('@hooks/useLocalize', () => ({ diff --git a/tests/unit/hooks/useSearchBulkActionsDuplicateTest.ts b/tests/unit/hooks/useSearchBulkActionsDuplicateTest.ts index 4d8d121d6384..91dceb5526ae 100644 --- a/tests/unit/hooks/useSearchBulkActionsDuplicateTest.ts +++ b/tests/unit/hooks/useSearchBulkActionsDuplicateTest.ts @@ -24,7 +24,7 @@ import createMock from '../../utils/createMock'; jest.mock('@libs/actions/IOU/Duplicate', () => ({ bulkDuplicateExpenses: jest.fn(), - bulkDuplicateReports: jest.fn(), + bulkDuplicateReports: jest.fn(() => Promise.resolve()), })); jest.mock('@libs/actions/Search', () => ({