diff --git a/src/libs/Middleware/HandleUnusedOptimisticID.ts b/src/libs/Middleware/HandleUnusedOptimisticID.ts index be449e76e54e..7a33eaf4238d 100644 --- a/src/libs/Middleware/HandleUnusedOptimisticID.ts +++ b/src/libs/Middleware/HandleUnusedOptimisticID.ts @@ -37,7 +37,58 @@ Onyx.connectWithoutView({ }); // Local cache of reportID to optimistic Onyx data -const reportOptimisticData = new Map; redundantParticipants: Record} | undefined>(); +const reportOptimisticData = new Map< + string, + {settledPersonalDetails: OnyxEntry; redundantParticipants: Record; missingLoginParticipants: number[]; invitedEmails: string[]} | undefined +>(); + +function isRecord(value: unknown): value is Record { + return !!value && typeof value === 'object'; +} + +// Pairs the login invited via emailList with the one participant that has no login. Restores only the unambiguous 1:1 case. +function getRestoredPersonalDetails( + responseOnyxData: AnyOnyxUpdate[], + reportID: string, + redundantParticipants: Record, + missingLoginParticipants: number[], + invitedEmails: string[], +): PersonalDetailsList { + const invitedEmail = invitedEmails.length === 1 ? invitedEmails.at(0) : undefined; + if (!invitedEmail) { + return {}; + } + + const settledParticipantIDs = new Set(); + const missingLoginDetailIDs = new Set(); + for (const update of responseOnyxData) { + if (update.key === `${ONYXKEYS.COLLECTION.REPORT}${reportID}` && isRecord(update.value) && isRecord(update.value.participants)) { + for (const accountID of Object.keys(update.value.participants)) { + settledParticipantIDs.add(accountID); + } + } + if (update.key === ONYXKEYS.PERSONAL_DETAILS_LIST && isRecord(update.value)) { + for (const [accountID, detail] of Object.entries(update.value)) { + // An explicit login in the response, even '', is respected; only an omitted key marks a candidate + if (!isRecord(detail) || 'login' in detail) { + continue; + } + missingLoginDetailIDs.add(accountID); + } + } + } + + for (const accountID of missingLoginParticipants) { + settledParticipantIDs.add(String(accountID)); + missingLoginDetailIDs.add(String(accountID)); + } + const candidates = [...settledParticipantIDs].filter((accountID) => missingLoginDetailIDs.has(accountID) && !(accountID in redundantParticipants)); + const settledAccountID = candidates.length === 1 ? Number(candidates.at(0)) : undefined; + if (!settledAccountID) { + return {}; + } + return {[settledAccountID]: {accountID: settledAccountID, login: invitedEmail}}; +} /** * This middleware checks for the presence of a field called preexistingReportID in the response. @@ -57,7 +108,10 @@ const handleUnusedOptimisticID: Middleware = (requestResponse, request, isFromSe // We're opening a new report, which can be a new or preexisting report // For new report, clean up optimistic data after this request returned successfully // For report redirect a preexisting report, clean up optimistic data after the request of preexisting report returned successfully - reportOptimisticData.set(currentRequestReportID, prepareOnyxDataForCleanUpOptimisticParticipants(currentRequestReportID, allPersonalDetails, currentUserAccountID)); + const cleanupData = prepareOnyxDataForCleanUpOptimisticParticipants(currentRequestReportID, allPersonalDetails, currentUserAccountID); + const emailList = request.data?.emailList; + const invitedEmails = typeof emailList === 'string' ? emailList.split(',').filter(Boolean) : []; + reportOptimisticData.set(currentRequestReportID, cleanupData ? {...cleanupData, invitedEmails} : undefined); } const responseOnyxData = response?.onyxData ?? []; @@ -102,7 +156,7 @@ const handleUnusedOptimisticID: Middleware = (requestResponse, request, isFromSe } if (!!currentRequestReportID && request?.command === WRITE_COMMANDS.OPEN_REPORT && !!response?.onyxData && reportOptimisticData.has(currentRequestReportID)) { - const {settledPersonalDetails, redundantParticipants} = reportOptimisticData.get(currentRequestReportID) ?? {}; + const {settledPersonalDetails, redundantParticipants, missingLoginParticipants, invitedEmails} = reportOptimisticData.get(currentRequestReportID) ?? {}; reportOptimisticData.delete(currentRequestReportID); if (!isEmptyObject(settledPersonalDetails) && !isEmptyObject(redundantParticipants)) { (response.onyxData as AnyOnyxUpdate[]).push( @@ -120,6 +174,22 @@ const handleUnusedOptimisticID: Middleware = (requestResponse, request, isFromSe }, ); } + + // The cleanup above deletes the only record carrying the typed login, so restore it onto the settled participant + const restoredPersonalDetails = getRestoredPersonalDetails( + response.onyxData as AnyOnyxUpdate[], + currentRequestReportID, + redundantParticipants ?? {}, + missingLoginParticipants ?? [], + invitedEmails ?? [], + ); + if (!isEmptyObject(restoredPersonalDetails)) { + (response.onyxData as AnyOnyxUpdate[]).push({ + onyxMethod: Onyx.METHOD.MERGE, + key: ONYXKEYS.PERSONAL_DETAILS_LIST, + value: restoredPersonalDetails, + }); + } } return response; }); diff --git a/src/libs/actions/Report/index.ts b/src/libs/actions/Report/index.ts index 4d41032a1c90..8be9d7dbe418 100644 --- a/src/libs/actions/Report/index.ts +++ b/src/libs/actions/Report/index.ts @@ -2126,13 +2126,14 @@ function prepareOnyxDataForCleanUpOptimisticParticipants( reportID: string, personalDetails: OnyxEntry, currentUserAccountID: number | undefined, -): {settledPersonalDetails: OnyxEntry; redundantParticipants: Record} | undefined { +): {settledPersonalDetails: OnyxEntry; redundantParticipants: Record; missingLoginParticipants: number[]} | undefined { const existingReport = allReports?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]; if (!existingReport?.participants) { return undefined; } const settledPersonalDetails: OnyxEntry = {}; const redundantParticipants: Record = {}; + const missingLoginParticipants: number[] = []; for (const accountID in existingReport.participants) { // Never clean up the current user's own personal details. Removing them here (even momentarily) drops the // current user's avatar down to the fallback avatar until the real details settle, which is what caused the @@ -2140,13 +2141,14 @@ function prepareOnyxDataForCleanUpOptimisticParticipants( if (Number(accountID) === currentUserAccountID) { continue; } - if (!personalDetails?.[accountID]?.isOptimisticPersonalDetail) { - continue; + if (personalDetails?.[accountID]?.isOptimisticPersonalDetail) { + settledPersonalDetails[accountID] = null; + redundantParticipants[accountID] = null; + } else if (personalDetails?.[accountID] && !Object.hasOwn(personalDetails[accountID], 'login')) { + missingLoginParticipants.push(Number(accountID)); } - settledPersonalDetails[accountID] = null; - redundantParticipants[accountID] = null; } - return {settledPersonalDetails, redundantParticipants}; + return {settledPersonalDetails, redundantParticipants, missingLoginParticipants}; } /** diff --git a/tests/actions/ReportTest.ts b/tests/actions/ReportTest.ts index b8bdf6016a06..4ec04db21422 100644 --- a/tests/actions/ReportTest.ts +++ b/tests/actions/ReportTest.ts @@ -9137,10 +9137,11 @@ describe('actions/Report', () => { CURRENT_USER_ACCOUNT_ID, ); - // Then only the optimistic participant is marked for clean up + // Then only the optimistic participant is marked for clean up, and the settled login-less participant is reported expect(result).toEqual({ settledPersonalDetails: {[OPTIMISTIC_ACCOUNT_ID]: null}, redundantParticipants: {[OPTIMISTIC_ACCOUNT_ID]: null}, + missingLoginParticipants: [SETTLED_ACCOUNT_ID], }); }); @@ -9155,7 +9156,7 @@ describe('actions/Report', () => { const result = Report.prepareOnyxDataForCleanUpOptimisticParticipants(REPORT_ID, {[SETTLED_ACCOUNT_ID]: {accountID: SETTLED_ACCOUNT_ID}}, CURRENT_USER_ACCOUNT_ID); - expect(result).toEqual({settledPersonalDetails: {}, redundantParticipants: {}}); + expect(result).toEqual({settledPersonalDetails: {}, redundantParticipants: {}, missingLoginParticipants: [SETTLED_ACCOUNT_ID]}); }); it('returns undefined when the report has no participants', async () => { @@ -9208,6 +9209,7 @@ describe('actions/Report', () => { expect(result).toEqual({ settledPersonalDetails: {[OPTIMISTIC_ACCOUNT_ID]: null}, redundantParticipants: {[OPTIMISTIC_ACCOUNT_ID]: null}, + missingLoginParticipants: [], }); }); }); diff --git a/tests/unit/MiddlewareTest.ts b/tests/unit/MiddlewareTest.ts index 7ff1a062217d..a2ea9276f88e 100644 --- a/tests/unit/MiddlewareTest.ts +++ b/tests/unit/MiddlewareTest.ts @@ -492,5 +492,323 @@ describe('Middleware', () => { expect(personalDetails?.[optimisticAccountID]).toBeUndefined(); expect(personalDetails?.[preexistingAccountID]).not.toBeUndefined(); }); + + test('OpenReport restores the invited login when the settled participant arrives without one', async () => { + const optimisticReportID = '1234'; + const optimisticAccountID = 999; + const settledAccountID = 333; + const invitedEmail = 'invited@example.com'; + await Onyx.multiSet({ + [`${ONYXKEYS.COLLECTION.REPORT}${optimisticReportID}` as const]: { + reportID: optimisticReportID, + participants: {[optimisticAccountID]: {notificationPreference: 'always'}}, + }, + [ONYXKEYS.PERSONAL_DETAILS_LIST]: { + [optimisticAccountID]: { + accountID: optimisticAccountID, + login: invitedEmail, + isOptimisticPersonalDetail: true, + }, + }, + }); + + Request.addMiddleware(handleUnusedOptimisticID); + Request.addMiddleware(SaveResponseInOnyx); + + SequentialQueue.push({ + command: 'OpenReport', + data: {authToken: 'testToken', reportID: optimisticReportID, createdReportActionID: '5678', emailList: invitedEmail}, + requestIndex: 13, + }); + + jest.spyOn(HttpUtils, 'xhr').mockResolvedValueOnce({ + jsonCode: 200, + onyxData: [ + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT}${optimisticReportID}`, + value: { + reportID: optimisticReportID, + participants: {[settledAccountID]: {notificationPreference: 'always'}}, + }, + }, + { + onyxMethod: Onyx.METHOD.MERGE, + key: ONYXKEYS.PERSONAL_DETAILS_LIST, + value: { + [settledAccountID]: { + accountID: settledAccountID, + }, + }, + }, + ], + }); + + SequentialQueue.unpause(); + await SequentialQueue.waitForIdle(); + await waitForBatchedUpdates(); + + const personalDetails = await new Promise>((resolve) => { + const connection = Onyx.connect({ + key: ONYXKEYS.PERSONAL_DETAILS_LIST, + callback: (data) => { + Onyx.disconnect(connection); + resolve(data); + }, + }); + }); + expect(personalDetails?.[optimisticAccountID]).toBeUndefined(); + expect(personalDetails?.[settledAccountID]?.login).toBe(invitedEmail); + }); + + test('OpenReport does not restore the invited login when the settled participant already has one', async () => { + const optimisticReportID = '1234'; + const optimisticAccountID = 999; + const settledAccountID = 333; + const serverLogin = 'server@example.com'; + await Onyx.multiSet({ + [`${ONYXKEYS.COLLECTION.REPORT}${optimisticReportID}` as const]: { + reportID: optimisticReportID, + participants: {[optimisticAccountID]: {notificationPreference: 'always'}}, + }, + [ONYXKEYS.PERSONAL_DETAILS_LIST]: { + [optimisticAccountID]: { + accountID: optimisticAccountID, + login: 'invited@example.com', + isOptimisticPersonalDetail: true, + }, + }, + }); + + Request.addMiddleware(handleUnusedOptimisticID); + Request.addMiddleware(SaveResponseInOnyx); + + SequentialQueue.push({ + command: 'OpenReport', + data: {authToken: 'testToken', reportID: optimisticReportID, createdReportActionID: '5678', emailList: 'invited@example.com'}, + requestIndex: 14, + }); + + jest.spyOn(HttpUtils, 'xhr').mockResolvedValueOnce({ + jsonCode: 200, + onyxData: [ + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT}${optimisticReportID}`, + value: { + reportID: optimisticReportID, + participants: {[settledAccountID]: {notificationPreference: 'always'}}, + }, + }, + { + onyxMethod: Onyx.METHOD.MERGE, + key: ONYXKEYS.PERSONAL_DETAILS_LIST, + value: { + [settledAccountID]: { + accountID: settledAccountID, + login: serverLogin, + }, + }, + }, + ], + }); + + SequentialQueue.unpause(); + await SequentialQueue.waitForIdle(); + await waitForBatchedUpdates(); + + const personalDetails = await new Promise>((resolve) => { + const connection = Onyx.connect({ + key: ONYXKEYS.PERSONAL_DETAILS_LIST, + callback: (data) => { + Onyx.disconnect(connection); + resolve(data); + }, + }); + }); + expect(personalDetails?.[optimisticAccountID]).toBeUndefined(); + expect(personalDetails?.[settledAccountID]?.login).toBe(serverLogin); + }); + + test('OpenReport does not restore the invited login when the response sets an explicitly empty one', async () => { + const optimisticReportID = '1234'; + const optimisticAccountID = 999; + const settledAccountID = 333; + const invitedEmail = 'invited@example.com'; + await Onyx.multiSet({ + [`${ONYXKEYS.COLLECTION.REPORT}${optimisticReportID}` as const]: { + reportID: optimisticReportID, + participants: {[optimisticAccountID]: {notificationPreference: 'always'}}, + }, + [ONYXKEYS.PERSONAL_DETAILS_LIST]: { + [optimisticAccountID]: { + accountID: optimisticAccountID, + login: invitedEmail, + isOptimisticPersonalDetail: true, + }, + }, + }); + + Request.addMiddleware(handleUnusedOptimisticID); + Request.addMiddleware(SaveResponseInOnyx); + + SequentialQueue.push({ + command: 'OpenReport', + data: {authToken: 'testToken', reportID: optimisticReportID, createdReportActionID: '5678', emailList: invitedEmail}, + requestIndex: 18, + }); + + jest.spyOn(HttpUtils, 'xhr').mockResolvedValueOnce({ + jsonCode: 200, + onyxData: [ + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT}${optimisticReportID}`, + value: { + reportID: optimisticReportID, + participants: {[settledAccountID]: {notificationPreference: 'always'}}, + }, + }, + { + onyxMethod: Onyx.METHOD.MERGE, + key: ONYXKEYS.PERSONAL_DETAILS_LIST, + value: { + [settledAccountID]: { + accountID: settledAccountID, + login: '', + }, + }, + }, + ], + }); + + SequentialQueue.unpause(); + await SequentialQueue.waitForIdle(); + await waitForBatchedUpdates(); + + const personalDetails = await new Promise>((resolve) => { + const connection = Onyx.connect({ + key: ONYXKEYS.PERSONAL_DETAILS_LIST, + callback: (data) => { + Onyx.disconnect(connection); + resolve(data); + }, + }); + }); + expect(personalDetails?.[optimisticAccountID]).toBeUndefined(); + expect(personalDetails?.[settledAccountID]?.login).toBe(''); + }); + + test('OpenReport restores the invited login when the participant is already known without a login key', async () => { + const optimisticReportID = '1234'; + const knownAccountID = 333; + const invitedEmail = 'invited@example.com'; + await Onyx.multiSet({ + [`${ONYXKEYS.COLLECTION.REPORT}${optimisticReportID}` as const]: { + reportID: optimisticReportID, + participants: {[knownAccountID]: {notificationPreference: 'always'}}, + }, + [ONYXKEYS.PERSONAL_DETAILS_LIST]: { + [knownAccountID]: { + accountID: knownAccountID, + }, + }, + }); + + Request.addMiddleware(handleUnusedOptimisticID); + Request.addMiddleware(SaveResponseInOnyx); + + SequentialQueue.push({ + command: 'OpenReport', + data: {authToken: 'testToken', reportID: optimisticReportID, createdReportActionID: '5678', emailList: invitedEmail}, + requestIndex: 15, + }); + + jest.spyOn(HttpUtils, 'xhr').mockResolvedValueOnce({ + jsonCode: 200, + onyxData: [ + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT}${optimisticReportID}`, + value: { + reportID: optimisticReportID, + participants: {[knownAccountID]: {notificationPreference: 'always'}}, + }, + }, + ], + }); + + SequentialQueue.unpause(); + await SequentialQueue.waitForIdle(); + await waitForBatchedUpdates(); + + const personalDetails = await new Promise>((resolve) => { + const connection = Onyx.connect({ + key: ONYXKEYS.PERSONAL_DETAILS_LIST, + callback: (data) => { + Onyx.disconnect(connection); + resolve(data); + }, + }); + }); + expect(personalDetails?.[knownAccountID]?.login).toBe(invitedEmail); + }); + + test('OpenReport does not restore the invited login when the participant is already known with an explicitly empty one', async () => { + const optimisticReportID = '1234'; + const knownAccountID = 333; + const invitedEmail = 'invited@example.com'; + await Onyx.multiSet({ + [`${ONYXKEYS.COLLECTION.REPORT}${optimisticReportID}` as const]: { + reportID: optimisticReportID, + participants: {[knownAccountID]: {notificationPreference: 'always'}}, + }, + [ONYXKEYS.PERSONAL_DETAILS_LIST]: { + [knownAccountID]: { + accountID: knownAccountID, + login: '', + displayName: '', + }, + }, + }); + + Request.addMiddleware(handleUnusedOptimisticID); + Request.addMiddleware(SaveResponseInOnyx); + + SequentialQueue.push({ + command: 'OpenReport', + data: {authToken: 'testToken', reportID: optimisticReportID, createdReportActionID: '5678', emailList: invitedEmail}, + requestIndex: 16, + }); + + jest.spyOn(HttpUtils, 'xhr').mockResolvedValueOnce({ + jsonCode: 200, + onyxData: [ + { + onyxMethod: Onyx.METHOD.MERGE, + key: `${ONYXKEYS.COLLECTION.REPORT}${optimisticReportID}`, + value: { + reportID: optimisticReportID, + participants: {[knownAccountID]: {notificationPreference: 'always'}}, + }, + }, + ], + }); + + SequentialQueue.unpause(); + await SequentialQueue.waitForIdle(); + await waitForBatchedUpdates(); + + const personalDetails = await new Promise>((resolve) => { + const connection = Onyx.connect({ + key: ONYXKEYS.PERSONAL_DETAILS_LIST, + callback: (data) => { + Onyx.disconnect(connection); + resolve(data); + }, + }); + }); + expect(personalDetails?.[knownAccountID]?.login).toBe(''); + }); }); });