From e7f354c39c04be1361ba0926d772c5d276f7f637 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Jasikowski?= Date: Wed, 8 Jul 2026 18:15:06 +0200 Subject: [PATCH 1/6] Open the agent DM via an optimistic reportID instead of guessing its identity Generate the owner<->agent DM's reportID client-side, the same way any other new chat's reportID is generated, and write the DM to Onyx optimistically alongside the agent itself. CreateAgent (see Expensify/Auth#22772 and Expensify/Web-Expensify#54367) creates the DM under that exact reportID, so the client can navigate to it immediately after submitting, online or offline, without waiting for the response or reconstructing the agent's identity from Onyx collection diffs. This replaces the previous approach of polling the agent-prompt collection to detect the newly created agent, which produced multiple race-condition bugs across review rounds. --- src/libs/API/parameters/CreateAgentParams.ts | 1 + src/libs/actions/Agent.ts | 61 +++++++++- src/libs/actions/Report/index.ts | 1 + src/pages/settings/Agents/AddAgentPage.tsx | 19 ++-- tests/unit/AgentActionTest.ts | 105 ++++++++++++++---- .../unit/pages/settings/AddAgentPageTest.tsx | 42 ++++--- 6 files changed, 181 insertions(+), 48 deletions(-) diff --git a/src/libs/API/parameters/CreateAgentParams.ts b/src/libs/API/parameters/CreateAgentParams.ts index e64555281f0a..740806948e14 100644 --- a/src/libs/API/parameters/CreateAgentParams.ts +++ b/src/libs/API/parameters/CreateAgentParams.ts @@ -8,6 +8,7 @@ type CreateAgentParams = { policyID?: string; optimisticAccountID: string; isPersonalAgent: boolean; + reportID: string; }; export default CreateAgentParams; diff --git a/src/libs/actions/Agent.ts b/src/libs/actions/Agent.ts index 8dd3a1fd0956..eceee4c749ff 100644 --- a/src/libs/actions/Agent.ts +++ b/src/libs/actions/Agent.ts @@ -4,7 +4,7 @@ import {AGENT_AVATARS} from '@libs/Avatars/AgentAvatarCatalog'; import type {CustomRNImageManipulatorResult} from '@libs/cropOrRotateImage/types'; import {getMicroSecondOnyxErrorWithTranslationKey} from '@libs/ErrorUtils'; import Navigation from '@libs/Navigation/Navigation'; -import {generateReportID} from '@libs/ReportUtils'; +import {buildOptimisticChatReport, buildOptimisticCreatedReportAction, generateReportID} from '@libs/ReportUtils'; import type {AvatarSource} from '@libs/UserAvatarUtils'; import CONST from '@src/CONST'; @@ -29,6 +29,8 @@ function openProfilePage() { function createAgent( firstName: string | undefined, prompt: string, + ownerAccountID: number, + ownerLogin: string | undefined, customExpensifyAvatarID?: string, file?: File | CustomRNImageManipulatorResult, optimisticAvatarURI?: string, @@ -51,6 +53,18 @@ function createAgent( ...(avatarURI ? {avatar: avatarURI, avatarThumbnail: avatarURI} : {}), }; + // The owner<->agent DM's reportID, generated the same way as any other new chat's reportID. CreateAgent + // creates the DM under this exact ID (see CreateAgent.cpp), so we can write the report to Onyx and + // navigate to it immediately, instead of waiting for the response and guessing the agent's identity. + const optimisticReportID = generateReportID(); + const optimisticChatReport = buildOptimisticChatReport({ + participantList: [ownerAccountID, optimisticAccountID], + ownerAccountID, + optimisticReportID, + currentUserAccountID: ownerAccountID, + }); + const optimisticCreatedAction = buildOptimisticCreatedReportAction({emailCreatingAction: ownerLogin ?? CONST.REPORT.OWNER_EMAIL_FAKE, currentUserAccountID: ownerAccountID}); + const optimisticData: AnyOnyxUpdate[] = [ { onyxMethod: Onyx.METHOD.MERGE, @@ -62,6 +76,21 @@ function createAgent( key: `${ONYXKEYS.COLLECTION.SHARED_NVP_AGENT_PROMPT}${optimisticAccountID}`, value: {prompt, pendingAction: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD}, }, + { + onyxMethod: Onyx.METHOD.SET, + key: `${ONYXKEYS.COLLECTION.REPORT}${optimisticReportID}`, + value: {...optimisticChatReport, pendingFields: {createChat: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD}}, + }, + { + onyxMethod: Onyx.METHOD.SET, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${optimisticReportID}`, + value: {[optimisticCreatedAction.reportActionID]: optimisticCreatedAction}, + }, + { + onyxMethod: Onyx.METHOD.SET, + key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${optimisticReportID}`, + value: {isOptimisticReport: true}, + }, ]; const successData: AnyOnyxUpdate[] = [ @@ -75,6 +104,16 @@ function createAgent( key: `${ONYXKEYS.COLLECTION.SHARED_NVP_AGENT_PROMPT}${optimisticAccountID}`, value: null, }, + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT}${optimisticReportID}`, + value: {pendingFields: {createChat: null}}, + }, + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${optimisticReportID}`, + value: {isOptimisticReport: false}, + }, ]; const failureData: AnyOnyxUpdate[] = [ @@ -92,16 +131,32 @@ function createAgent( errors: getMicroSecondOnyxErrorWithTranslationKey('agentsPage.error.genericAdd'), }, }, + { + onyxMethod: Onyx.METHOD.SET, + key: `${ONYXKEYS.COLLECTION.REPORT}${optimisticReportID}`, + value: null, + }, + { + onyxMethod: Onyx.METHOD.SET, + key: `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${optimisticReportID}`, + value: null, + }, + { + onyxMethod: Onyx.METHOD.SET, + key: `${ONYXKEYS.COLLECTION.REPORT_METADATA}${optimisticReportID}`, + value: null, + }, ]; write( // Flag this as the user's personal agent; the backend makes personal agents a full co-pilot of the creator. + // reportID is the owner<->agent DM's optimistic reportID; CreateAgent creates the DM under this exact ID. WRITE_COMMANDS.CREATE_AGENT, - {firstName, prompt, customExpensifyAvatarID, file, policyID, optimisticAccountID: String(optimisticAccountID), isPersonalAgent: true}, + {firstName, prompt, customExpensifyAvatarID, file, policyID, optimisticAccountID: String(optimisticAccountID), isPersonalAgent: true, reportID: optimisticReportID}, {optimisticData, successData, failureData}, ); - return {optimisticAccountID, avatarURI}; + return {optimisticAccountID, avatarURI, optimisticReportID}; } function clearAgentError(optimisticAccountID: number) { diff --git a/src/libs/actions/Report/index.ts b/src/libs/actions/Report/index.ts index 40cee2652e23..304edf11f0b5 100644 --- a/src/libs/actions/Report/index.ts +++ b/src/libs/actions/Report/index.ts @@ -8014,6 +8014,7 @@ export { navigateToAndOpenReport, navigateToAndOpenReportWithAccountIDs, navigateToAndCreateGroupChat, + navigateToReport, navigateToConciergeChat, navigateToConciergeChatAndDeleteReport, clearCreateChatError, diff --git a/src/pages/settings/Agents/AddAgentPage.tsx b/src/pages/settings/Agents/AddAgentPage.tsx index ff0eb09dd6c0..71cdca24811d 100644 --- a/src/pages/settings/Agents/AddAgentPage.tsx +++ b/src/pages/settings/Agents/AddAgentPage.tsx @@ -14,6 +14,7 @@ import useLocalize from '@hooks/useLocalize'; import useThemeStyles from '@hooks/useThemeStyles'; import useWindowDimensions from '@hooks/useWindowDimensions'; +import {navigateToReport} from '@libs/actions/Report'; import {AGENT_AVATARS} from '@libs/Avatars/AgentAvatarCatalog'; import type {AgentAvatarID} from '@libs/Avatars/AgentAvatarCatalog'; import {isMobile} from '@libs/Browser'; @@ -46,7 +47,7 @@ function AddAgentPage({route}: AddAgentPageProps) { const styles = useThemeStyles(); const {windowWidth, windowHeight} = useWindowDimensions(); const shouldUseScrollableLayout = useIsInLandscapeMode() || (isMobile() && windowWidth > windowHeight); - const {displayName} = useCurrentUserPersonalDetails(); + const {accountID: ownerAccountID, login: ownerLogin, displayName} = useCurrentUserPersonalDetails(); const defaultAgentName = displayName ? translate('addAgentPage.defaultAgentName', displayName) : undefined; const defaultPrompt = translate('addAgentPage.defaultPrompt'); const expensifyIcons = useMemoizedLazyExpensifyIcons(['Pencil']); @@ -97,16 +98,14 @@ function AddAgentPage({route}: AddAgentPageProps) { const prompt = values[INPUT_IDS.PROMPT].trim(); const pendingFile = pendingFileRef.current; - // Pure optimistic flow — no waiting on the server, online or offline. `createAgent` - // returns the optimistic accountID it wrote into Onyx so we can hand it to the next - // screen and let it render the agent with opacity until CREATE_AGENT resolves. - if (pendingFile) { - createAgent(firstName, prompt, undefined, pendingFile.file, pendingFile.uri, policyID); - } else { - createAgent(firstName, prompt, selectedPresetID ?? undefined, undefined, undefined, policyID); - } + // Pure optimistic flow: `createAgent` writes the agent and the owner<->agent DM to Onyx under a + // reportID it generates client-side, and CreateAgent creates the DM under that exact ID (see + // CreateAgent.cpp), so we can navigate to the DM immediately, online or offline, without waiting. + const {optimisticReportID} = pendingFile + ? createAgent(firstName, prompt, ownerAccountID, ownerLogin, undefined, pendingFile.file, pendingFile.uri, policyID) + : createAgent(firstName, prompt, ownerAccountID, ownerLogin, selectedPresetID ?? undefined, undefined, undefined, policyID); - Navigation.goBack(); + navigateToReport(optimisticReportID, {shouldDismissModal: true}); }; return ( diff --git a/tests/unit/AgentActionTest.ts b/tests/unit/AgentActionTest.ts index 18b90279d991..6f657b3d2d14 100644 --- a/tests/unit/AgentActionTest.ts +++ b/tests/unit/AgentActionTest.ts @@ -38,25 +38,28 @@ function getOptimisticAccountID(optimisticData: AnyOnyxUpdate[]): number { return Number(Object.keys(personalDetailUpdate.value as Record).at(0)); } +const OWNER_ACCOUNT_ID = 999; +const OWNER_LOGIN = 'owner@test.com'; + describe('createAgent', () => { beforeEach(() => { jest.clearAllMocks(); }); it('calls write with CREATE_AGENT command and provided params', () => { - createAgent('My Agent', 'Reject gambling expenses.'); + createAgent('My Agent', 'Reject gambling expenses.', OWNER_ACCOUNT_ID, OWNER_LOGIN); expect(mockWrite).toHaveBeenCalledWith(WRITE_COMMANDS.CREATE_AGENT, expect.objectContaining({firstName: 'My Agent', prompt: 'Reject gambling expenses.'}), expect.any(Object)); }); it('passes undefined firstName through unchanged', () => { - createAgent(undefined, 'Some prompt'); + createAgent(undefined, 'Some prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN); expect(mockWrite).toHaveBeenCalledWith(WRITE_COMMANDS.CREATE_AGENT, expect.objectContaining({firstName: undefined, prompt: 'Some prompt'}), expect.any(Object)); }); it('optimistic personal detail entry has a positive account ID from generateReportID', () => { - createAgent('Bot', 'My prompt'); + createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN); const {optimisticData} = getWriteOptions(); const accountID = getOptimisticAccountID(optimisticData); @@ -65,7 +68,7 @@ describe('createAgent', () => { }); it('optimistic personal detail entry stores displayName and marks entry as optimistic', () => { - createAgent('Bot', 'My prompt'); + createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN); const {optimisticData} = getWriteOptions(); const personalDetailUpdate = optimisticData.find((u) => u.key === ONYXKEYS.PERSONAL_DETAILS_LIST); @@ -78,7 +81,7 @@ describe('createAgent', () => { }); it('optimistic personal detail entry stores undefined displayName when firstName is undefined', () => { - createAgent(undefined, 'My prompt'); + createAgent(undefined, 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN); const {optimisticData} = getWriteOptions(); const personalDetailUpdate = optimisticData.find((u) => u.key === ONYXKEYS.PERSONAL_DETAILS_LIST); @@ -91,7 +94,7 @@ describe('createAgent', () => { }); it('optimistic prompt entry uses the same account ID as the personal detail entry', () => { - createAgent('Bot', 'My prompt'); + createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN); const {optimisticData} = getWriteOptions(); const accountID = getOptimisticAccountID(optimisticData); @@ -104,7 +107,7 @@ describe('createAgent', () => { }); it('passes customExpensifyAvatarID to write params when provided', () => { - createAgent('Bot', 'My prompt', 'bot-avatar--blue'); + createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN, 'bot-avatar--blue'); expect(mockWrite).toHaveBeenCalledWith( WRITE_COMMANDS.CREATE_AGENT, @@ -115,21 +118,21 @@ describe('createAgent', () => { it('passes file to write params when provided', () => { const mockFile = createMock({uri: 'file://photo.jpg', name: 'photo.jpg'}); - createAgent('Bot', 'My prompt', undefined, mockFile, 'file://photo.jpg'); + createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN, undefined, mockFile, 'file://photo.jpg'); expect(mockWrite).toHaveBeenCalledWith(WRITE_COMMANDS.CREATE_AGENT, expect.objectContaining({firstName: 'Bot', prompt: 'My prompt', file: mockFile}), expect.any(Object)); }); it('uploads file in the CREATE_AGENT call itself — no separate UPDATE_AGENT_AVATAR write', () => { const mockFile = createMock({uri: 'file://photo.jpg', name: 'photo.jpg'}); - createAgent('Bot', 'My prompt', undefined, mockFile, 'file://photo.jpg'); + createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN, undefined, mockFile, 'file://photo.jpg'); expect(mockWrite).toHaveBeenCalledTimes(1); expect(mockWrite).not.toHaveBeenCalledWith(WRITE_COMMANDS.UPDATE_AGENT_AVATAR, expect.anything(), expect.anything()); }); it('includes resolved avatar URI in optimistic and failure personal detail data for a preset ID', () => { - createAgent('Bot', 'My prompt', 'bot-avatar--blue'); + createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN, 'bot-avatar--blue'); const {optimisticData, failureData} = getWriteOptions(); const accountID = getOptimisticAccountID(optimisticData); @@ -145,7 +148,7 @@ describe('createAgent', () => { it('includes optimisticAvatarURI in optimistic and failure personal detail data for a custom file URI', () => { const fileURI = 'file://local-photo.jpg'; - createAgent('Bot', 'My prompt', undefined, undefined, fileURI); + createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN, undefined, undefined, fileURI); const {optimisticData, failureData} = getWriteOptions(); const accountID = getOptimisticAccountID(optimisticData); @@ -160,7 +163,7 @@ describe('createAgent', () => { }); it('does not include avatar fields in optimistic data when no avatar args are given', () => { - createAgent('Bot', 'My prompt'); + createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN); const {optimisticData} = getWriteOptions(); const accountID = getOptimisticAccountID(optimisticData); @@ -171,21 +174,83 @@ describe('createAgent', () => { }); it('forwards policyID in the write params when provided', () => { - createAgent('Bot', 'My prompt', undefined, undefined, undefined, 'POLICY_42'); + createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN, undefined, undefined, undefined, 'POLICY_42'); expect(mockWrite).toHaveBeenCalledWith(WRITE_COMMANDS.CREATE_AGENT, expect.objectContaining({firstName: 'Bot', prompt: 'My prompt', policyID: 'POLICY_42'}), expect.any(Object)); }); it('returns the optimistic accountID and avatarURI so callers can chain follow-up navigation', () => { - const result = createAgent('Bot', 'My prompt', 'bot-avatar--blue'); + const result = createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN, 'bot-avatar--blue'); expect(result.optimisticAccountID).toEqual(expect.any(Number)); expect(result.optimisticAccountID).toBeGreaterThan(0); expect(result.avatarURI).toBeTruthy(); }); + it('returns an optimisticReportID for the owner<->agent DM so the caller can navigate immediately', () => { + const result = createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN); + + expect(result.optimisticReportID).toEqual(expect.any(String)); + expect(result.optimisticReportID).toBeTruthy(); + }); + + it('passes the optimistic reportID through to CreateAgent so the DM is created under that exact ID', () => { + const result = createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN); + + expect(mockWrite).toHaveBeenCalledWith(WRITE_COMMANDS.CREATE_AGENT, expect.objectContaining({reportID: result.optimisticReportID}), expect.any(Object)); + }); + + it('optimistic data writes the owner<->agent DM report with both participants and a pending createChat field', () => { + const result = createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN); + + const {optimisticData} = getWriteOptions(); + const accountID = getOptimisticAccountID(optimisticData); + const reportUpdate = optimisticData.find((u) => u.key === `${ONYXKEYS.COLLECTION.REPORT}${result.optimisticReportID}`); + + expect(reportUpdate?.onyxMethod).toBe('set'); + expect(reportUpdate?.value).toHaveProperty(['participants', String(OWNER_ACCOUNT_ID)]); + expect(reportUpdate?.value).toHaveProperty(['participants', String(accountID)]); + expect(reportUpdate?.value).toMatchObject({pendingFields: {createChat: CONST.RED_BRICK_ROAD_PENDING_ACTION.ADD}}); + }); + + it('optimistic data marks the DM report as optimistic and adds a created action', () => { + const result = createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN); + + const {optimisticData} = getWriteOptions(); + const metadataUpdate = optimisticData.find((u) => u.key === `${ONYXKEYS.COLLECTION.REPORT_METADATA}${result.optimisticReportID}`); + const actionsUpdate = optimisticData.find((u) => u.key === `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${result.optimisticReportID}`); + + expect(metadataUpdate?.value).toMatchObject({isOptimisticReport: true}); + expect(actionsUpdate?.onyxMethod).toBe('set'); + expect(actionsUpdate?.value).toBeTruthy(); + }); + + it('success data clears the pending createChat field and optimistic report flag', () => { + const result = createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN); + + const {successData} = getWriteOptions(); + const reportUpdate = successData.find((u) => u.key === `${ONYXKEYS.COLLECTION.REPORT}${result.optimisticReportID}`); + const metadataUpdate = successData.find((u) => u.key === `${ONYXKEYS.COLLECTION.REPORT_METADATA}${result.optimisticReportID}`); + + expect(reportUpdate?.value).toMatchObject({pendingFields: {createChat: null}}); + expect(metadataUpdate?.value).toMatchObject({isOptimisticReport: false}); + }); + + it('failure data rolls back the optimistic DM report, its actions, and its metadata', () => { + const result = createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN); + + const {failureData} = getWriteOptions(); + const reportUpdate = failureData.find((u) => u.key === `${ONYXKEYS.COLLECTION.REPORT}${result.optimisticReportID}`); + const actionsUpdate = failureData.find((u) => u.key === `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${result.optimisticReportID}`); + const metadataUpdate = failureData.find((u) => u.key === `${ONYXKEYS.COLLECTION.REPORT_METADATA}${result.optimisticReportID}`); + + expect(reportUpdate?.value).toBeNull(); + expect(actionsUpdate?.value).toBeNull(); + expect(metadataUpdate?.value).toBeNull(); + }); + it('does not touch the policy when no policyID is provided', () => { - createAgent('Bot', 'My prompt'); + createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN); const {optimisticData, successData, failureData} = getWriteOptions(); const allKeys: string[] = [...optimisticData, ...successData, ...failureData].map((u) => String(u.key)); @@ -194,7 +259,7 @@ describe('createAgent', () => { }); it('omits login on the optimistic personal detail entry — the real email is server-assigned', () => { - createAgent('Bot', 'My prompt', undefined, undefined, undefined, 'POLICY_42'); + createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN, undefined, undefined, undefined, 'POLICY_42'); const {optimisticData} = getWriteOptions(); const personalDetailUpdate = optimisticData.find((u) => u.key === ONYXKEYS.PERSONAL_DETAILS_LIST); @@ -205,7 +270,7 @@ describe('createAgent', () => { }); it('does not merge ADD_AGENT_FORM (navigation handles UX after submit)', () => { - createAgent('Bot', 'My prompt'); + createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN); const {optimisticData, successData, failureData} = getWriteOptions(); @@ -215,7 +280,7 @@ describe('createAgent', () => { }); it('success data nulls out both optimistic entries', () => { - createAgent('Bot', 'My prompt'); + createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN); const {optimisticData, successData} = getWriteOptions(); const accountID = getOptimisticAccountID(optimisticData); @@ -228,13 +293,13 @@ describe('createAgent', () => { }); it('passes the optimistic accountID through to the server so it can echo a real-ID mapping', () => { - const result = createAgent('Bot', 'My prompt'); + const result = createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN); expect(mockWrite).toHaveBeenCalledWith(WRITE_COMMANDS.CREATE_AGENT, expect.objectContaining({optimisticAccountID: String(result.optimisticAccountID)}), expect.any(Object)); }); it('failure data preserves optimistic personal detail and merges errors onto the prompt entry', () => { - createAgent('Bot', 'My prompt'); + createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN); const {optimisticData, failureData} = getWriteOptions(); const accountID = getOptimisticAccountID(optimisticData); diff --git a/tests/unit/pages/settings/AddAgentPageTest.tsx b/tests/unit/pages/settings/AddAgentPageTest.tsx index a98ddb358ba5..f6a4150ebfc3 100644 --- a/tests/unit/pages/settings/AddAgentPageTest.tsx +++ b/tests/unit/pages/settings/AddAgentPageTest.tsx @@ -2,6 +2,7 @@ import {render} from '@testing-library/react-native'; import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; +import {navigateToReport} from '@libs/actions/Report'; import Navigation from '@libs/Navigation/Navigation'; import type {PlatformStackRouteProp} from '@libs/Navigation/PlatformStackNavigation/types'; import type {SettingsNavigatorParamList} from '@libs/Navigation/types'; @@ -9,15 +10,25 @@ import type {SettingsNavigatorParamList} from '@libs/Navigation/types'; import AddAgentPage from '@pages/settings/Agents/AddAgentPage'; import {setInitialPresetID, setNavigationToken} from '@pages/settings/Agents/pendingAgentAvatarStore'; +import {createAgent} from '@userActions/Agent'; + import ROUTES from '@src/ROUTES'; import type SCREENS from '@src/SCREENS'; import React from 'react'; jest.mock('@userActions/Agent', () => ({ - createAgent: jest.fn(() => ({optimisticAccountID: -123456, avatarURI: undefined})), + createAgent: jest.fn(), +})); + +const mockCreateAgent = jest.mocked(createAgent); + +jest.mock('@libs/actions/Report', () => ({ + navigateToReport: jest.fn(), })); +const mockNavigateToReport = jest.mocked(navigateToReport); + const mockTranslate = jest.fn().mockImplementation((key: string, param?: string) => (param !== undefined ? `${key}(${param})` : key)); jest.mock('@hooks/useLocalize', () => jest.fn(() => ({translate: mockTranslate}))); @@ -43,8 +54,6 @@ jest.mock('@hooks/useLazyAsset', () => ({ useMemoizedLazyExpensifyIcons: jest.fn(() => ({Pencil: 1})), })); -jest.mock('@hooks/useOnyx', () => jest.fn(() => [undefined, {status: 'loaded'}])); - jest.mock('@libs/Navigation/Navigation', () => ({ goBack: jest.fn(), navigate: jest.fn(), @@ -114,7 +123,6 @@ jest.mock('@pages/settings/Agents/pendingAgentAvatarStore', () => ({ const mockSetInitialPresetID = jest.mocked(setInitialPresetID); const mockSetNavigationToken = jest.mocked(setNavigationToken); const mockNavigate = jest.mocked(Navigation.navigate); -const mockGoBack = jest.mocked(Navigation.goBack); const mockUseCurrentUserPersonalDetails = jest.mocked(useCurrentUserPersonalDetails); type AddAgentRouteProp = PlatformStackRouteProp; @@ -123,10 +131,15 @@ function makeRoute(params: AddAgentRouteProp['params'] = {}): AddAgentRouteProp return {name: '', key: '', params} as unknown as AddAgentRouteProp; } +const OWNER_ACCOUNT_ID = 999; +const OWNER_LOGIN = 'owner@test.com'; +const OPTIMISTIC_REPORT_ID = '4567890123456789'; + describe('AddAgentPage', () => { beforeEach(() => { jest.clearAllMocks(); - mockUseCurrentUserPersonalDetails.mockReturnValue({accountID: 0}); + mockUseCurrentUserPersonalDetails.mockReturnValue({accountID: OWNER_ACCOUNT_ID, login: OWNER_LOGIN}); + mockCreateAgent.mockReturnValue({optimisticAccountID: -123456, avatarURI: undefined, optimisticReportID: OPTIMISTIC_REPORT_ID}); mockAvatarOnPress = undefined; }); @@ -142,7 +155,7 @@ describe('AddAgentPage', () => { }); it('translates default agent name using current user displayName', () => { - mockUseCurrentUserPersonalDetails.mockReturnValue({accountID: 0, displayName: 'Nicolas'}); + mockUseCurrentUserPersonalDetails.mockReturnValue({accountID: OWNER_ACCOUNT_ID, login: OWNER_LOGIN, displayName: 'Nicolas'}); render( { }); it('sets default agent name as InputWrapper defaultValue when displayName exists', () => { - mockUseCurrentUserPersonalDetails.mockReturnValue({accountID: 0, displayName: 'Nicolas'}); + mockUseCurrentUserPersonalDetails.mockReturnValue({accountID: OWNER_ACCOUNT_ID, login: OWNER_LOGIN, displayName: 'Nicolas'}); const {toJSON} = render( { }); it('sets no default agent name when displayName is absent', () => { - mockUseCurrentUserPersonalDetails.mockReturnValue({accountID: 0}); - const {toJSON} = render( { mockFormOnSubmit = undefined; }); - it('goes back when policyID is absent in route params', () => { + it('creates the agent with the owner accountID and login, then navigates to the optimistic DM report and dismisses the modal', () => { render( { mockFormOnSubmit?.({firstName: 'Bot', prompt: 'Reject gambling.'}); - expect(mockGoBack).toHaveBeenCalledTimes(1); - expect(mockNavigate).not.toHaveBeenCalled(); + // The 5th arg (selected preset avatar ID) is randomized on mount, so only assert the args that matter here. + expect(mockCreateAgent).toHaveBeenCalledWith('Bot', 'Reject gambling.', OWNER_ACCOUNT_ID, OWNER_LOGIN, expect.anything(), undefined, undefined, undefined); + expect(mockNavigateToReport).toHaveBeenCalledWith(OPTIMISTIC_REPORT_ID, {shouldDismissModal: true}); }); - it('goes back when policyID is present without navigating to a workflow editor', () => { + it('forwards policyID from route params to createAgent', () => { render( { mockFormOnSubmit?.({firstName: 'Bot', prompt: 'Reject gambling.'}); - expect(mockGoBack).toHaveBeenCalledTimes(1); - expect(mockNavigate).not.toHaveBeenCalled(); + expect(mockCreateAgent).toHaveBeenCalledWith('Bot', 'Reject gambling.', OWNER_ACCOUNT_ID, OWNER_LOGIN, expect.anything(), undefined, undefined, 'POL_42'); + expect(mockNavigateToReport).toHaveBeenCalledWith(OPTIMISTIC_REPORT_ID, {shouldDismissModal: true}); }); }); }); From ca7a0ad14615d3e683153888ce2a150ee2333d81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Jasikowski?= Date: Thu, 9 Jul 2026 14:27:20 +0200 Subject: [PATCH 2/6] Fix race in navigateToReport that could clobber the navigation dismissModal() and the delayed navigate() were scheduled independently via setTimeout + isNavigationReady(), which only checks the nav ref exists and doesn't wait for the dismiss transition to settle. Under load (e.g. creating several agents in quick succession) the dismissal's own delayed navigation could land after our navigate() and send the user back to the previous screen instead of the report they just opened. Sequence the navigate through dismissModal's afterTransition callback instead, matching the existing dismissModalWithReport pattern. --- src/libs/actions/Report/index.ts | 28 +++++++++------ tests/actions/ReportTest.ts | 4 ++- .../actions/Report/navigateToReportTest.ts | 35 +++++++++++++++++++ 3 files changed, 55 insertions(+), 12 deletions(-) create mode 100644 tests/unit/libs/actions/Report/navigateToReportTest.ts diff --git a/src/libs/actions/Report/index.ts b/src/libs/actions/Report/index.ts index 304edf11f0b5..0513b8d74767 100644 --- a/src/libs/actions/Report/index.ts +++ b/src/libs/actions/Report/index.ts @@ -2247,12 +2247,25 @@ function createTransactionThreadReport(params: CreateTransactionThreadReportPara function navigateToReport(reportID: string | undefined, options?: {shouldDismissModal?: boolean; afterTransition?: () => void}) { const shouldDismissModal = options?.shouldDismissModal ?? true; - if (shouldDismissModal) { + const navigateToRoute = () => { if (!reportID) { - Navigation.dismissModal({afterTransition: options?.afterTransition}); return; } - Navigation.dismissModal(); + const route = ROUTES.REPORT_WITH_ID.getRoute(reportID); + if (options?.afterTransition) { + Navigation.navigate(route, {afterTransition: options.afterTransition}); + } else { + Navigation.navigate(route); + } + }; + + if (shouldDismissModal) { + // Navigating from dismissModal's afterTransition (rather than a setTimeout scheduled independently of it) + // ensures the navigation runs only once the dismiss transition has actually settled. Firing it + // independently can let the dismissal's own state update land afterward and clobber this navigation, + // e.g. when modals are dismissed in quick succession. + Navigation.dismissModal({afterTransition: reportID ? navigateToRoute : options?.afterTransition}); + return; } if (!reportID) { @@ -2260,14 +2273,7 @@ function navigateToReport(reportID: string | undefined, options?: {shouldDismiss } // In some cases when RHP modal gets hidden and then we navigate to report Composer focus breaks, wrapping navigation in setTimeout fixes this setTimeout(() => { - Navigation.isNavigationReady().then(() => { - const route = ROUTES.REPORT_WITH_ID.getRoute(reportID); - if (options?.afterTransition) { - Navigation.navigate(route, {afterTransition: options.afterTransition}); - } else { - Navigation.navigate(route); - } - }); + Navigation.isNavigationReady().then(navigateToRoute); }, 0); } diff --git a/tests/actions/ReportTest.ts b/tests/actions/ReportTest.ts index 6fe21e0a4b31..a9508b17301b 100644 --- a/tests/actions/ReportTest.ts +++ b/tests/actions/ReportTest.ts @@ -134,7 +134,9 @@ jest.mock('@libs/Navigation/Navigation', () => ({ getActiveRoute: jest.fn().mockReturnValue(''), getActiveRouteWithoutParams: jest.fn(() => ''), dismissModalWithReport: jest.fn(), - dismissModal: jest.fn(), + dismissModal: jest.fn((options?: {afterTransition?: () => void}) => { + options?.afterTransition?.(); + }), dismissToSuperWideRHP: jest.fn(), isNavigationReady: jest.fn(() => Promise.resolve()), isActiveRoute: jest.fn(() => false), diff --git a/tests/unit/libs/actions/Report/navigateToReportTest.ts b/tests/unit/libs/actions/Report/navigateToReportTest.ts new file mode 100644 index 000000000000..d37c8f5203eb --- /dev/null +++ b/tests/unit/libs/actions/Report/navigateToReportTest.ts @@ -0,0 +1,35 @@ +import {navigateToReport} from '@libs/actions/Report'; +import Navigation from '@libs/Navigation/Navigation'; + +import ROUTES from '@src/ROUTES'; + +jest.mock('@libs/Navigation/Navigation', () => ({ + dismissModal: jest.fn(), + navigate: jest.fn(), + isNavigationReady: jest.fn(() => Promise.resolve()), +})); + +describe('navigateToReport', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('defers navigating to the report until dismissModal signals the transition has settled', () => { + navigateToReport('123'); + + // The navigation to the report must be sequenced through dismissModal's afterTransition callback + // instead of firing independently, otherwise a slower dismissal can finish afterward and clobber it. + expect(Navigation.dismissModal).toHaveBeenCalledTimes(1); + const dismissModalArgs = jest.mocked(Navigation.dismissModal).mock.calls.at(0)?.at(0) as {afterTransition?: () => void} | undefined; + const afterTransition = dismissModalArgs?.afterTransition; + expect(afterTransition).toBeInstanceOf(Function); + + // Must not navigate before the dismiss transition has actually settled. + expect(Navigation.navigate).not.toHaveBeenCalled(); + + afterTransition?.(); + + expect(Navigation.navigate).toHaveBeenCalledTimes(1); + expect(Navigation.navigate).toHaveBeenCalledWith(ROUTES.REPORT_WITH_ID.getRoute('123')); + }); +}); From 11e1ae88201cf791dae93c0e1c3d77579fece6a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Jasikowski?= Date: Thu, 9 Jul 2026 14:27:50 +0200 Subject: [PATCH 3/6] Show the known display name for an optimistic personal detail getDisplayNameForParticipant short-circuited to the formatted login for any optimistic personal detail, on the assumption that a login is the only thing known about it (true for invite-by-email flows). Agent creation is different: the display name the user chose is already known, but login is intentionally omitted (it's server-assigned), so the DM title resolved to an empty string. Since CREATE_AGENT's response usually lands within a second online, this was an imperceptible flicker there, but offline the write stays queued indefinitely and the blank title persists. Only take the login-based shortcut when no display name is already known, which existing optimistic-personal-detail callers already set to the same value as the login anyway, so their behavior is unchanged. --- src/libs/ReportUtils.ts | 6 ++++-- tests/unit/ReportUtilsTest.ts | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index bde50fe949a4..01069dd04129 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -3531,8 +3531,10 @@ function getDisplayNameForParticipant({ // This is to check if account is an invite/optimistically created one // and prevent from falling back to 'Hidden', so a correct value is shown - // when searching for a new user - if (personalDetails.isOptimisticPersonalDetail === true) { + // when searching for a new user. Optimistic personal details that already have a known displayName + // (e.g. an agent pending creation, whose name the user just chose) fall through to the normal name + // resolution below instead, so that name is shown rather than an empty/placeholder login. + if (personalDetails.isOptimisticPersonalDetail === true && !personalDetails.displayName) { return formattedLogin; } diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index 22b715f09ff8..0e6fcfe3700e 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -13941,6 +13941,21 @@ describe('ReportUtils', () => { const displayName = getDisplayNameForParticipant({formatPhoneNumber, accountID: iouReport.ownerAccountID}); expect(displayName).toBe(fakePersonalDetails?.[1]?.displayName); }); + it('should return the known displayName for an optimistic personal detail that already has one (e.g. an agent pending creation)', async () => { + const optimisticAccountID = 8399123; + await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, { + [optimisticAccountID]: { + accountID: optimisticAccountID, + displayName: "Michal's Agent", + isOptimisticPersonalDetail: true, + }, + }); + + waitForBatchedUpdates(); + + const displayName = getDisplayNameForParticipant({formatPhoneNumber, accountID: optimisticAccountID}); + expect(displayName).toBe("Michal's Agent"); + }); it('should surface a GBR when copiloted into an approver account with a report with outstanding child request', async () => { await Onyx.clear(); From 2b2f0376db8191d839cba2862ba0f2bc241c26c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Jasikowski?= Date: Thu, 9 Jul 2026 14:46:38 +0200 Subject: [PATCH 4/6] Rename CreateAgent's reportID param to optimisticReportID Matches the Auth/Web-Expensify rename: CreateAgent's primary subject is the agent account, not a report, so the owner<->agent DM's client-supplied ID follows the optimisticReportID naming SharePolicy and MoveIOUReportToExistingPolicy already use for the same kind of auxiliary, side-effect report ID. --- src/libs/API/parameters/CreateAgentParams.ts | 2 +- src/libs/actions/Agent.ts | 4 ++-- tests/unit/AgentActionTest.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libs/API/parameters/CreateAgentParams.ts b/src/libs/API/parameters/CreateAgentParams.ts index 740806948e14..ca4e9d825b76 100644 --- a/src/libs/API/parameters/CreateAgentParams.ts +++ b/src/libs/API/parameters/CreateAgentParams.ts @@ -8,7 +8,7 @@ type CreateAgentParams = { policyID?: string; optimisticAccountID: string; isPersonalAgent: boolean; - reportID: string; + optimisticReportID: string; }; export default CreateAgentParams; diff --git a/src/libs/actions/Agent.ts b/src/libs/actions/Agent.ts index eceee4c749ff..9e0474f23fde 100644 --- a/src/libs/actions/Agent.ts +++ b/src/libs/actions/Agent.ts @@ -150,9 +150,9 @@ function createAgent( write( // Flag this as the user's personal agent; the backend makes personal agents a full co-pilot of the creator. - // reportID is the owner<->agent DM's optimistic reportID; CreateAgent creates the DM under this exact ID. + // optimisticReportID is the owner<->agent DM's optimistic reportID; CreateAgent creates the DM under this exact ID. WRITE_COMMANDS.CREATE_AGENT, - {firstName, prompt, customExpensifyAvatarID, file, policyID, optimisticAccountID: String(optimisticAccountID), isPersonalAgent: true, reportID: optimisticReportID}, + {firstName, prompt, customExpensifyAvatarID, file, policyID, optimisticAccountID: String(optimisticAccountID), isPersonalAgent: true, optimisticReportID}, {optimisticData, successData, failureData}, ); diff --git a/tests/unit/AgentActionTest.ts b/tests/unit/AgentActionTest.ts index 6f657b3d2d14..f6ada3ec6fd6 100644 --- a/tests/unit/AgentActionTest.ts +++ b/tests/unit/AgentActionTest.ts @@ -197,7 +197,7 @@ describe('createAgent', () => { it('passes the optimistic reportID through to CreateAgent so the DM is created under that exact ID', () => { const result = createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN); - expect(mockWrite).toHaveBeenCalledWith(WRITE_COMMANDS.CREATE_AGENT, expect.objectContaining({reportID: result.optimisticReportID}), expect.any(Object)); + expect(mockWrite).toHaveBeenCalledWith(WRITE_COMMANDS.CREATE_AGENT, expect.objectContaining({optimisticReportID: result.optimisticReportID}), expect.any(Object)); }); it('optimistic data writes the owner<->agent DM report with both participants and a pending createChat field', () => { From 127a5a73a5e087b975cf2f31b38b24933b0b2828 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Jasikowski?= Date: Thu, 9 Jul 2026 15:59:21 +0200 Subject: [PATCH 5/6] Use Navigation.dismissModalWithReport instead of navigateToReport dismissModalWithReport is the established pattern for "dismiss the modal and open a report" (used by WorkspaceNewRoomPage, NewChatPage, Split, TeachersUnite, etc.) and already sequences the navigate through dismissModal's afterTransition correctly, so it isn't affected by the race that motivated exporting and patching navigateToReport. That patch is no longer needed, so revert navigateToReport back to its prior implementation and drop its export. --- src/libs/actions/Report/index.ts | 29 ++++++--------- src/pages/settings/Agents/AddAgentPage.tsx | 3 +- tests/actions/ReportTest.ts | 4 +-- .../actions/Report/navigateToReportTest.ts | 35 ------------------- .../unit/pages/settings/AddAgentPageTest.tsx | 13 +++---- 5 files changed, 17 insertions(+), 67 deletions(-) delete mode 100644 tests/unit/libs/actions/Report/navigateToReportTest.ts diff --git a/src/libs/actions/Report/index.ts b/src/libs/actions/Report/index.ts index 0513b8d74767..40cee2652e23 100644 --- a/src/libs/actions/Report/index.ts +++ b/src/libs/actions/Report/index.ts @@ -2247,25 +2247,12 @@ function createTransactionThreadReport(params: CreateTransactionThreadReportPara function navigateToReport(reportID: string | undefined, options?: {shouldDismissModal?: boolean; afterTransition?: () => void}) { const shouldDismissModal = options?.shouldDismissModal ?? true; - const navigateToRoute = () => { + if (shouldDismissModal) { if (!reportID) { + Navigation.dismissModal({afterTransition: options?.afterTransition}); return; } - const route = ROUTES.REPORT_WITH_ID.getRoute(reportID); - if (options?.afterTransition) { - Navigation.navigate(route, {afterTransition: options.afterTransition}); - } else { - Navigation.navigate(route); - } - }; - - if (shouldDismissModal) { - // Navigating from dismissModal's afterTransition (rather than a setTimeout scheduled independently of it) - // ensures the navigation runs only once the dismiss transition has actually settled. Firing it - // independently can let the dismissal's own state update land afterward and clobber this navigation, - // e.g. when modals are dismissed in quick succession. - Navigation.dismissModal({afterTransition: reportID ? navigateToRoute : options?.afterTransition}); - return; + Navigation.dismissModal(); } if (!reportID) { @@ -2273,7 +2260,14 @@ function navigateToReport(reportID: string | undefined, options?: {shouldDismiss } // In some cases when RHP modal gets hidden and then we navigate to report Composer focus breaks, wrapping navigation in setTimeout fixes this setTimeout(() => { - Navigation.isNavigationReady().then(navigateToRoute); + Navigation.isNavigationReady().then(() => { + const route = ROUTES.REPORT_WITH_ID.getRoute(reportID); + if (options?.afterTransition) { + Navigation.navigate(route, {afterTransition: options.afterTransition}); + } else { + Navigation.navigate(route); + } + }); }, 0); } @@ -8020,7 +8014,6 @@ export { navigateToAndOpenReport, navigateToAndOpenReportWithAccountIDs, navigateToAndCreateGroupChat, - navigateToReport, navigateToConciergeChat, navigateToConciergeChatAndDeleteReport, clearCreateChatError, diff --git a/src/pages/settings/Agents/AddAgentPage.tsx b/src/pages/settings/Agents/AddAgentPage.tsx index 71cdca24811d..940e6ffdf70f 100644 --- a/src/pages/settings/Agents/AddAgentPage.tsx +++ b/src/pages/settings/Agents/AddAgentPage.tsx @@ -14,7 +14,6 @@ import useLocalize from '@hooks/useLocalize'; import useThemeStyles from '@hooks/useThemeStyles'; import useWindowDimensions from '@hooks/useWindowDimensions'; -import {navigateToReport} from '@libs/actions/Report'; import {AGENT_AVATARS} from '@libs/Avatars/AgentAvatarCatalog'; import type {AgentAvatarID} from '@libs/Avatars/AgentAvatarCatalog'; import {isMobile} from '@libs/Browser'; @@ -105,7 +104,7 @@ function AddAgentPage({route}: AddAgentPageProps) { ? createAgent(firstName, prompt, ownerAccountID, ownerLogin, undefined, pendingFile.file, pendingFile.uri, policyID) : createAgent(firstName, prompt, ownerAccountID, ownerLogin, selectedPresetID ?? undefined, undefined, undefined, policyID); - navigateToReport(optimisticReportID, {shouldDismissModal: true}); + Navigation.dismissModalWithReport({reportID: optimisticReportID}); }; return ( diff --git a/tests/actions/ReportTest.ts b/tests/actions/ReportTest.ts index a9508b17301b..6fe21e0a4b31 100644 --- a/tests/actions/ReportTest.ts +++ b/tests/actions/ReportTest.ts @@ -134,9 +134,7 @@ jest.mock('@libs/Navigation/Navigation', () => ({ getActiveRoute: jest.fn().mockReturnValue(''), getActiveRouteWithoutParams: jest.fn(() => ''), dismissModalWithReport: jest.fn(), - dismissModal: jest.fn((options?: {afterTransition?: () => void}) => { - options?.afterTransition?.(); - }), + dismissModal: jest.fn(), dismissToSuperWideRHP: jest.fn(), isNavigationReady: jest.fn(() => Promise.resolve()), isActiveRoute: jest.fn(() => false), diff --git a/tests/unit/libs/actions/Report/navigateToReportTest.ts b/tests/unit/libs/actions/Report/navigateToReportTest.ts deleted file mode 100644 index d37c8f5203eb..000000000000 --- a/tests/unit/libs/actions/Report/navigateToReportTest.ts +++ /dev/null @@ -1,35 +0,0 @@ -import {navigateToReport} from '@libs/actions/Report'; -import Navigation from '@libs/Navigation/Navigation'; - -import ROUTES from '@src/ROUTES'; - -jest.mock('@libs/Navigation/Navigation', () => ({ - dismissModal: jest.fn(), - navigate: jest.fn(), - isNavigationReady: jest.fn(() => Promise.resolve()), -})); - -describe('navigateToReport', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - - it('defers navigating to the report until dismissModal signals the transition has settled', () => { - navigateToReport('123'); - - // The navigation to the report must be sequenced through dismissModal's afterTransition callback - // instead of firing independently, otherwise a slower dismissal can finish afterward and clobber it. - expect(Navigation.dismissModal).toHaveBeenCalledTimes(1); - const dismissModalArgs = jest.mocked(Navigation.dismissModal).mock.calls.at(0)?.at(0) as {afterTransition?: () => void} | undefined; - const afterTransition = dismissModalArgs?.afterTransition; - expect(afterTransition).toBeInstanceOf(Function); - - // Must not navigate before the dismiss transition has actually settled. - expect(Navigation.navigate).not.toHaveBeenCalled(); - - afterTransition?.(); - - expect(Navigation.navigate).toHaveBeenCalledTimes(1); - expect(Navigation.navigate).toHaveBeenCalledWith(ROUTES.REPORT_WITH_ID.getRoute('123')); - }); -}); diff --git a/tests/unit/pages/settings/AddAgentPageTest.tsx b/tests/unit/pages/settings/AddAgentPageTest.tsx index f6a4150ebfc3..963ffaa35a63 100644 --- a/tests/unit/pages/settings/AddAgentPageTest.tsx +++ b/tests/unit/pages/settings/AddAgentPageTest.tsx @@ -2,7 +2,6 @@ import {render} from '@testing-library/react-native'; import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails'; -import {navigateToReport} from '@libs/actions/Report'; import Navigation from '@libs/Navigation/Navigation'; import type {PlatformStackRouteProp} from '@libs/Navigation/PlatformStackNavigation/types'; import type {SettingsNavigatorParamList} from '@libs/Navigation/types'; @@ -23,12 +22,6 @@ jest.mock('@userActions/Agent', () => ({ const mockCreateAgent = jest.mocked(createAgent); -jest.mock('@libs/actions/Report', () => ({ - navigateToReport: jest.fn(), -})); - -const mockNavigateToReport = jest.mocked(navigateToReport); - const mockTranslate = jest.fn().mockImplementation((key: string, param?: string) => (param !== undefined ? `${key}(${param})` : key)); jest.mock('@hooks/useLocalize', () => jest.fn(() => ({translate: mockTranslate}))); @@ -57,6 +50,7 @@ jest.mock('@hooks/useLazyAsset', () => ({ jest.mock('@libs/Navigation/Navigation', () => ({ goBack: jest.fn(), navigate: jest.fn(), + dismissModalWithReport: jest.fn(), })); jest.mock('@react-navigation/native', () => { @@ -123,6 +117,7 @@ jest.mock('@pages/settings/Agents/pendingAgentAvatarStore', () => ({ const mockSetInitialPresetID = jest.mocked(setInitialPresetID); const mockSetNavigationToken = jest.mocked(setNavigationToken); const mockNavigate = jest.mocked(Navigation.navigate); +const mockDismissModalWithReport = jest.mocked(Navigation.dismissModalWithReport); const mockUseCurrentUserPersonalDetails = jest.mocked(useCurrentUserPersonalDetails); type AddAgentRouteProp = PlatformStackRouteProp; @@ -258,7 +253,7 @@ describe('AddAgentPage', () => { // The 5th arg (selected preset avatar ID) is randomized on mount, so only assert the args that matter here. expect(mockCreateAgent).toHaveBeenCalledWith('Bot', 'Reject gambling.', OWNER_ACCOUNT_ID, OWNER_LOGIN, expect.anything(), undefined, undefined, undefined); - expect(mockNavigateToReport).toHaveBeenCalledWith(OPTIMISTIC_REPORT_ID, {shouldDismissModal: true}); + expect(mockDismissModalWithReport).toHaveBeenCalledWith({reportID: OPTIMISTIC_REPORT_ID}); }); it('forwards policyID from route params to createAgent', () => { @@ -272,7 +267,7 @@ describe('AddAgentPage', () => { mockFormOnSubmit?.({firstName: 'Bot', prompt: 'Reject gambling.'}); expect(mockCreateAgent).toHaveBeenCalledWith('Bot', 'Reject gambling.', OWNER_ACCOUNT_ID, OWNER_LOGIN, expect.anything(), undefined, undefined, 'POL_42'); - expect(mockNavigateToReport).toHaveBeenCalledWith(OPTIMISTIC_REPORT_ID, {shouldDismissModal: true}); + expect(mockDismissModalWithReport).toHaveBeenCalledWith({reportID: OPTIMISTIC_REPORT_ID}); }); }); }); From f60e75288d5eff696027af97d1c730ddfd7fec3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Jasikowski?= Date: Thu, 9 Jul 2026 21:07:20 +0200 Subject: [PATCH 6/6] Pass createdReportActionID to CreateAgent, like OpenReport's callers do createAgent() already builds an optimistic CREATED action for the DM report (reportActions_), but left its ID random and never forwarded it, so the server generated a different, duplicate CREATED action instead of reconciling onto the client's optimistic one. Generate the ID up front and pass it as createdReportActionID, the same way openReport() forwards its optimistic CREATED action's ID. --- src/libs/API/parameters/CreateAgentParams.ts | 1 + src/libs/actions/Agent.ts | 13 +++++++++++-- tests/unit/AgentActionTest.ts | 14 ++++++++++++++ 3 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/libs/API/parameters/CreateAgentParams.ts b/src/libs/API/parameters/CreateAgentParams.ts index ca4e9d825b76..a1ec63bfcff9 100644 --- a/src/libs/API/parameters/CreateAgentParams.ts +++ b/src/libs/API/parameters/CreateAgentParams.ts @@ -9,6 +9,7 @@ type CreateAgentParams = { optimisticAccountID: string; isPersonalAgent: boolean; optimisticReportID: string; + createdReportActionID: string; }; export default CreateAgentParams; diff --git a/src/libs/actions/Agent.ts b/src/libs/actions/Agent.ts index 9e0474f23fde..69a0427bd436 100644 --- a/src/libs/actions/Agent.ts +++ b/src/libs/actions/Agent.ts @@ -4,6 +4,7 @@ import {AGENT_AVATARS} from '@libs/Avatars/AgentAvatarCatalog'; import type {CustomRNImageManipulatorResult} from '@libs/cropOrRotateImage/types'; import {getMicroSecondOnyxErrorWithTranslationKey} from '@libs/ErrorUtils'; import Navigation from '@libs/Navigation/Navigation'; +import {rand64} from '@libs/NumberUtils'; import {buildOptimisticChatReport, buildOptimisticCreatedReportAction, generateReportID} from '@libs/ReportUtils'; import type {AvatarSource} from '@libs/UserAvatarUtils'; @@ -63,7 +64,15 @@ function createAgent( optimisticReportID, currentUserAccountID: ownerAccountID, }); - const optimisticCreatedAction = buildOptimisticCreatedReportAction({emailCreatingAction: ownerLogin ?? CONST.REPORT.OWNER_EMAIL_FAKE, currentUserAccountID: ownerAccountID}); + // Generated up front (rather than left to buildOptimisticCreatedReportAction's default) so it can also + // be sent to CreateAgent, the same way OpenReport's callers forward createdReportActionID: the DM's + // CREATED action then reconciles onto this exact ID instead of the server generating a duplicate. + const createdReportActionID = rand64(); + const optimisticCreatedAction = buildOptimisticCreatedReportAction({ + emailCreatingAction: ownerLogin ?? CONST.REPORT.OWNER_EMAIL_FAKE, + currentUserAccountID: ownerAccountID, + optimisticReportActionID: createdReportActionID, + }); const optimisticData: AnyOnyxUpdate[] = [ { @@ -152,7 +161,7 @@ function createAgent( // Flag this as the user's personal agent; the backend makes personal agents a full co-pilot of the creator. // optimisticReportID is the owner<->agent DM's optimistic reportID; CreateAgent creates the DM under this exact ID. WRITE_COMMANDS.CREATE_AGENT, - {firstName, prompt, customExpensifyAvatarID, file, policyID, optimisticAccountID: String(optimisticAccountID), isPersonalAgent: true, optimisticReportID}, + {firstName, prompt, customExpensifyAvatarID, file, policyID, optimisticAccountID: String(optimisticAccountID), isPersonalAgent: true, optimisticReportID, createdReportActionID}, {optimisticData, successData, failureData}, ); diff --git a/tests/unit/AgentActionTest.ts b/tests/unit/AgentActionTest.ts index f6ada3ec6fd6..5c7faf78d4cb 100644 --- a/tests/unit/AgentActionTest.ts +++ b/tests/unit/AgentActionTest.ts @@ -200,6 +200,20 @@ describe('createAgent', () => { expect(mockWrite).toHaveBeenCalledWith(WRITE_COMMANDS.CREATE_AGENT, expect.objectContaining({optimisticReportID: result.optimisticReportID}), expect.any(Object)); }); + it('passes createdReportActionID through to CreateAgent matching the key of the optimistic CREATED action, so the DM created action reconciles onto it instead of a duplicate', () => { + const result = createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN); + + const {optimisticData} = getWriteOptions(); + const actionsValue: unknown = optimisticData.find((u) => u.key === `${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${result.optimisticReportID}`)?.value; + if (!actionsValue || typeof actionsValue !== 'object') { + throw new Error('No reportActions update in optimisticData'); + } + const createdActionID = Object.keys(actionsValue).at(0); + + expect(createdActionID).toBeTruthy(); + expect(mockWrite).toHaveBeenCalledWith(WRITE_COMMANDS.CREATE_AGENT, expect.objectContaining({createdReportActionID: createdActionID}), expect.any(Object)); + }); + it('optimistic data writes the owner<->agent DM report with both participants and a pending createChat field', () => { const result = createAgent('Bot', 'My prompt', OWNER_ACCOUNT_ID, OWNER_LOGIN);