From b1b43ae50d74554f832856cd873063307e1dc304 Mon Sep 17 00:00:00 2001 From: Mukher Date: Wed, 8 Jul 2026 18:35:03 +0500 Subject: [PATCH 1/7] restore invited login when OpenReport settles the participant without one --- .../Middleware/HandleUnusedOptimisticID.ts | 54 +++++++++++++++++-- 1 file changed, 51 insertions(+), 3 deletions(-) diff --git a/src/libs/Middleware/HandleUnusedOptimisticID.ts b/src/libs/Middleware/HandleUnusedOptimisticID.ts index c6b4ac7d7b6f..70cb508246fb 100644 --- a/src/libs/Middleware/HandleUnusedOptimisticID.ts +++ b/src/libs/Middleware/HandleUnusedOptimisticID.ts @@ -17,7 +17,41 @@ import clone from 'lodash/clone'; import Onyx from 'react-native-onyx'; // Local cache of reportID to optimistic Onyx data -const reportOptimisticData = new Map; redundantParticipants: Record} | undefined>(); +const reportOptimisticData = new Map; redundantParticipants: Record; invitedEmails: string[]} | undefined>(); + +// Pairs the login invited via emailList with the participant the server settled without one. Restores only the unambiguous 1:1 case. +function getRestoredPersonalDetails(responseOnyxData: AnyOnyxUpdate[], reportID: string, redundantParticipants: Record, invitedEmails: string[]): PersonalDetailsList { + const invitedEmail = invitedEmails.length === 1 ? invitedEmails.at(0) : undefined; + if (!invitedEmail) { + return {}; + } + + const settledParticipantIDs = new Set(); + const loginlessDetailIDs = new Set(); + for (const update of responseOnyxData) { + if (update.key === `${ONYXKEYS.COLLECTION.REPORT}${reportID}`) { + const participants = (update.value as Report | undefined)?.participants ?? {}; + for (const accountID of Object.keys(participants)) { + settledParticipantIDs.add(accountID); + } + } + if (update.key === ONYXKEYS.PERSONAL_DETAILS_LIST) { + for (const [accountID, detail] of Object.entries((update.value as PersonalDetailsList | undefined) ?? {})) { + if (!detail || detail.login) { + continue; + } + loginlessDetailIDs.add(accountID); + } + } + } + + const candidates = [...settledParticipantIDs].filter((accountID) => loginlessDetailIDs.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. @@ -37,7 +71,11 @@ 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)); + const cleanupData = prepareOnyxDataForCleanUpOptimisticParticipants(currentRequestReportID); + const invitedEmails = String(request.data?.emailList ?? '') + .split(',') + .filter(Boolean); + reportOptimisticData.set(currentRequestReportID, cleanupData ? {...cleanupData, invitedEmails} : undefined); } const responseOnyxData = response?.onyxData ?? []; @@ -82,7 +120,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, invitedEmails} = reportOptimisticData.get(currentRequestReportID) ?? {}; reportOptimisticData.delete(currentRequestReportID); if (!isEmptyObject(settledPersonalDetails) && !isEmptyObject(redundantParticipants)) { (response.onyxData as AnyOnyxUpdate[]).push( @@ -100,6 +138,16 @@ 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 ?? {}, invitedEmails ?? []); + if (!isEmptyObject(restoredPersonalDetails)) { + (response.onyxData as AnyOnyxUpdate[]).push({ + onyxMethod: Onyx.METHOD.MERGE, + key: ONYXKEYS.PERSONAL_DETAILS_LIST, + value: restoredPersonalDetails, + }); + } } return response; }); From 4f380cbe99e103bdd12f43df043b9a0473435f2b Mon Sep 17 00:00:00 2001 From: Mukher Date: Thu, 9 Jul 2026 07:53:25 +0500 Subject: [PATCH 2/7] fixed spell fail --- src/libs/Middleware/HandleUnusedOptimisticID.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libs/Middleware/HandleUnusedOptimisticID.ts b/src/libs/Middleware/HandleUnusedOptimisticID.ts index 70cb508246fb..e502b7de7071 100644 --- a/src/libs/Middleware/HandleUnusedOptimisticID.ts +++ b/src/libs/Middleware/HandleUnusedOptimisticID.ts @@ -27,7 +27,7 @@ function getRestoredPersonalDetails(responseOnyxData: AnyOnyxUpdate[], reportID: } const settledParticipantIDs = new Set(); - const loginlessDetailIDs = new Set(); + const missingLoginDetailIDs = new Set(); for (const update of responseOnyxData) { if (update.key === `${ONYXKEYS.COLLECTION.REPORT}${reportID}`) { const participants = (update.value as Report | undefined)?.participants ?? {}; @@ -40,12 +40,12 @@ function getRestoredPersonalDetails(responseOnyxData: AnyOnyxUpdate[], reportID: if (!detail || detail.login) { continue; } - loginlessDetailIDs.add(accountID); + missingLoginDetailIDs.add(accountID); } } } - const candidates = [...settledParticipantIDs].filter((accountID) => loginlessDetailIDs.has(accountID) && !(accountID in redundantParticipants)); + const candidates = [...settledParticipantIDs].filter((accountID) => missingLoginDetailIDs.has(accountID) && !(accountID in redundantParticipants)); const settledAccountID = candidates.length === 1 ? Number(candidates.at(0)) : undefined; if (!settledAccountID) { return {}; From b0e954677c6bb87b3f6872032cefaed0833d54bd Mon Sep 17 00:00:00 2001 From: Mukher Date: Thu, 9 Jul 2026 07:55:13 +0500 Subject: [PATCH 3/7] added missing Openreport tests --- tests/unit/MiddlewareTest.ts | 145 +++++++++++++++++++++++++++++++++++ 1 file changed, 145 insertions(+) diff --git a/tests/unit/MiddlewareTest.ts b/tests/unit/MiddlewareTest.ts index 7ff1a062217d..4c2f94c9d955 100644 --- a/tests/unit/MiddlewareTest.ts +++ b/tests/unit/MiddlewareTest.ts @@ -492,5 +492,150 @@ 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, + }); + + (global.fetch as jest.Mock).mockImplementationOnce(async () => ({ + ok: true, + + json: async () => ({ + 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, + }); + + (global.fetch as jest.Mock).mockImplementationOnce(async () => ({ + ok: true, + + json: async () => ({ + 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); + }); }); }); From bf2b0d840dbab0954f68f4b0008df2908ab43b90 Mon Sep 17 00:00:00 2001 From: Mukher Date: Thu, 9 Jul 2026 15:05:07 +0500 Subject: [PATCH 4/7] fix eslint errors: no-base-to-string on emailList and unsafe Mock assertions Narrow request.data.emailList with a typeof check instead of a bare template-string coercion, and replace (global.fetch as jest.Mock) casts with jest.spyOn(HttpUtils, 'xhr') so the new middleware tests type-check without unsafe assertions. Also restructure the participants for-in loop back into a single guard if to satisfy guard-for-in. --- .../Middleware/HandleUnusedOptimisticID.ts | 62 ++++++-- src/libs/actions/Report/index.ts | 14 +- tests/unit/MiddlewareTest.ts | 143 ++++++++++++------ 3 files changed, 152 insertions(+), 67 deletions(-) diff --git a/src/libs/Middleware/HandleUnusedOptimisticID.ts b/src/libs/Middleware/HandleUnusedOptimisticID.ts index e502b7de7071..03183b7c3d98 100644 --- a/src/libs/Middleware/HandleUnusedOptimisticID.ts +++ b/src/libs/Middleware/HandleUnusedOptimisticID.ts @@ -1,6 +1,7 @@ import {prepareOnyxDataForCleanUpOptimisticParticipants} from '@libs/actions/Report'; import {WRITE_COMMANDS} from '@libs/API/types'; import deepReplaceKeysAndValues from '@libs/deepReplaceKeysAndValues'; +import {getLoginByAccountID} from '@libs/PersonalDetailsUtils'; import type {Middleware} from '@libs/Request'; import * as PersistedRequests from '@userActions/PersistedRequests'; @@ -17,10 +18,23 @@ import clone from 'lodash/clone'; import Onyx from 'react-native-onyx'; // Local cache of reportID to optimistic Onyx data -const reportOptimisticData = new Map; redundantParticipants: Record; invitedEmails: string[]} | undefined>(); +const reportOptimisticData = new Map< + string, + {settledPersonalDetails: OnyxEntry; redundantParticipants: Record; missingLoginParticipants: number[]; invitedEmails: string[]} | undefined +>(); -// Pairs the login invited via emailList with the participant the server settled without one. Restores only the unambiguous 1:1 case. -function getRestoredPersonalDetails(responseOnyxData: AnyOnyxUpdate[], reportID: string, redundantParticipants: Record, invitedEmails: string[]): PersonalDetailsList { +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 {}; @@ -29,15 +43,14 @@ function getRestoredPersonalDetails(responseOnyxData: AnyOnyxUpdate[], reportID: const settledParticipantIDs = new Set(); const missingLoginDetailIDs = new Set(); for (const update of responseOnyxData) { - if (update.key === `${ONYXKEYS.COLLECTION.REPORT}${reportID}`) { - const participants = (update.value as Report | undefined)?.participants ?? {}; - for (const accountID of Object.keys(participants)) { + 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) { - for (const [accountID, detail] of Object.entries((update.value as PersonalDetailsList | undefined) ?? {})) { - if (!detail || detail.login) { + if (update.key === ONYXKEYS.PERSONAL_DETAILS_LIST && isRecord(update.value)) { + for (const [accountID, detail] of Object.entries(update.value)) { + if (!isRecord(detail) || detail.login) { continue; } missingLoginDetailIDs.add(accountID); @@ -45,6 +58,10 @@ function getRestoredPersonalDetails(responseOnyxData: AnyOnyxUpdate[], reportID: } } + 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) { @@ -72,9 +89,8 @@ const handleUnusedOptimisticID: Middleware = (requestResponse, request, isFromSe // 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 const cleanupData = prepareOnyxDataForCleanUpOptimisticParticipants(currentRequestReportID); - const invitedEmails = String(request.data?.emailList ?? '') - .split(',') - .filter(Boolean); + const emailList = request.data?.emailList; + const invitedEmails = typeof emailList === 'string' ? emailList.split(',').filter(Boolean) : []; reportOptimisticData.set(currentRequestReportID, cleanupData ? {...cleanupData, invitedEmails} : undefined); } @@ -120,7 +136,7 @@ const handleUnusedOptimisticID: Middleware = (requestResponse, request, isFromSe } if (!!currentRequestReportID && request?.command === WRITE_COMMANDS.OPEN_REPORT && !!response?.onyxData && reportOptimisticData.has(currentRequestReportID)) { - const {settledPersonalDetails, redundantParticipants, invitedEmails} = reportOptimisticData.get(currentRequestReportID) ?? {}; + const {settledPersonalDetails, redundantParticipants, missingLoginParticipants, invitedEmails} = reportOptimisticData.get(currentRequestReportID) ?? {}; reportOptimisticData.delete(currentRequestReportID); if (!isEmptyObject(settledPersonalDetails) && !isEmptyObject(redundantParticipants)) { (response.onyxData as AnyOnyxUpdate[]).push( @@ -140,7 +156,13 @@ 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 ?? {}, invitedEmails ?? []); + const restoredPersonalDetails = getRestoredPersonalDetails( + response.onyxData as AnyOnyxUpdate[], + currentRequestReportID, + redundantParticipants ?? {}, + missingLoginParticipants ?? [], + invitedEmails ?? [], + ); if (!isEmptyObject(restoredPersonalDetails)) { (response.onyxData as AnyOnyxUpdate[]).push({ onyxMethod: Onyx.METHOD.MERGE, @@ -149,6 +171,18 @@ const handleUnusedOptimisticID: Middleware = (requestResponse, request, isFromSe }); } } + // Drop explicitly empty logins some commands echo (e.g. OpenPublicProfilePage) so they can't blank out a login the client already knows + for (const update of response?.onyxData ?? []) { + if (update.key !== ONYXKEYS.PERSONAL_DETAILS_LIST || !isRecord(update.value)) { + continue; + } + for (const [accountID, detail] of Object.entries(update.value)) { + if (!isRecord(detail) || detail.login !== '' || !getLoginByAccountID(Number(accountID))) { + continue; + } + delete detail.login; + } + } return response; }); diff --git a/src/libs/actions/Report/index.ts b/src/libs/actions/Report/index.ts index 40cee2652e23..870748b5b723 100644 --- a/src/libs/actions/Report/index.ts +++ b/src/libs/actions/Report/index.ts @@ -2090,21 +2090,23 @@ function createGroupChat( function prepareOnyxDataForCleanUpOptimisticParticipants( reportID: string, -): {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) { - if (!allPersonalDetails?.[accountID]?.isOptimisticPersonalDetail) { - continue; + if (allPersonalDetails?.[accountID]?.isOptimisticPersonalDetail) { + settledPersonalDetails[accountID] = null; + redundantParticipants[accountID] = null; + } else if (allPersonalDetails?.[accountID] && !allPersonalDetails[accountID].login) { + missingLoginParticipants.push(Number(accountID)); } - settledPersonalDetails[accountID] = null; - redundantParticipants[accountID] = null; } - return {settledPersonalDetails, redundantParticipants}; + return {settledPersonalDetails, redundantParticipants, missingLoginParticipants}; } /** diff --git a/tests/unit/MiddlewareTest.ts b/tests/unit/MiddlewareTest.ts index 4c2f94c9d955..bf77a2c9e2b2 100644 --- a/tests/unit/MiddlewareTest.ts +++ b/tests/unit/MiddlewareTest.ts @@ -521,32 +521,28 @@ describe('Middleware', () => { requestIndex: 13, }); - (global.fetch as jest.Mock).mockImplementationOnce(async () => ({ - ok: true, - - json: async () => ({ - jsonCode: 200, - onyxData: [ - { - onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT}${optimisticReportID}`, - value: { - reportID: optimisticReportID, - participants: {[settledAccountID]: {notificationPreference: 'always'}}, - }, + 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, - }, + }, + { + onyxMethod: Onyx.METHOD.MERGE, + key: ONYXKEYS.PERSONAL_DETAILS_LIST, + value: { + [settledAccountID]: { + accountID: settledAccountID, }, }, - ], - }), - })); + }, + ], + }); SequentialQueue.unpause(); await SequentialQueue.waitForIdle(); @@ -593,33 +589,29 @@ describe('Middleware', () => { requestIndex: 14, }); - (global.fetch as jest.Mock).mockImplementationOnce(async () => ({ - ok: true, - - json: async () => ({ - jsonCode: 200, - onyxData: [ - { - onyxMethod: Onyx.METHOD.MERGE, - key: `${ONYXKEYS.COLLECTION.REPORT}${optimisticReportID}`, - value: { - reportID: optimisticReportID, - participants: {[settledAccountID]: {notificationPreference: 'always'}}, - }, + 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, - }, + }, + { + onyxMethod: Onyx.METHOD.MERGE, + key: ONYXKEYS.PERSONAL_DETAILS_LIST, + value: { + [settledAccountID]: { + accountID: settledAccountID, + login: serverLogin, }, }, - ], - }), - })); + }, + ], + }); SequentialQueue.unpause(); await SequentialQueue.waitForIdle(); @@ -637,5 +629,62 @@ describe('Middleware', () => { expect(personalDetails?.[optimisticAccountID]).toBeUndefined(); expect(personalDetails?.[settledAccountID]?.login).toBe(serverLogin); }); + + test('OpenReport restores the invited login when the participant is already known without one from a server search', 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: 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); + }); }); }); From 93a3d4f5c715c4513745e4b2a89df5efe41ef925 Mon Sep 17 00:00:00 2001 From: Mukher Date: Sun, 12 Jul 2026 13:53:25 +0500 Subject: [PATCH 5/7] scope the empty-login sanitizer to OpenPublicProfilePage The sanitizer previously ran for the response of any API command that included a PERSONAL_DETAILS_LIST merge with an empty login, not just the OpenReport/profile flow it exists for. Narrow it to the one command it's meant to guard against, and add tests covering both that it still fixes OpenPublicProfilePage and that it now leaves other commands alone. --- .../Middleware/HandleUnusedOptimisticID.ts | 21 ++-- tests/unit/MiddlewareTest.ts | 102 ++++++++++++++++++ 2 files changed, 114 insertions(+), 9 deletions(-) diff --git a/src/libs/Middleware/HandleUnusedOptimisticID.ts b/src/libs/Middleware/HandleUnusedOptimisticID.ts index 03183b7c3d98..95da021ff3c8 100644 --- a/src/libs/Middleware/HandleUnusedOptimisticID.ts +++ b/src/libs/Middleware/HandleUnusedOptimisticID.ts @@ -1,5 +1,5 @@ import {prepareOnyxDataForCleanUpOptimisticParticipants} from '@libs/actions/Report'; -import {WRITE_COMMANDS} from '@libs/API/types'; +import {READ_COMMANDS, WRITE_COMMANDS} from '@libs/API/types'; import deepReplaceKeysAndValues from '@libs/deepReplaceKeysAndValues'; import {getLoginByAccountID} from '@libs/PersonalDetailsUtils'; import type {Middleware} from '@libs/Request'; @@ -171,16 +171,19 @@ const handleUnusedOptimisticID: Middleware = (requestResponse, request, isFromSe }); } } - // Drop explicitly empty logins some commands echo (e.g. OpenPublicProfilePage) so they can't blank out a login the client already knows - for (const update of response?.onyxData ?? []) { - if (update.key !== ONYXKEYS.PERSONAL_DETAILS_LIST || !isRecord(update.value)) { - continue; - } - for (const [accountID, detail] of Object.entries(update.value)) { - if (!isRecord(detail) || detail.login !== '' || !getLoginByAccountID(Number(accountID))) { + // OpenPublicProfilePage can echo back an explicitly empty login for an account the client + // already knows a login for; drop that so it can't blank out the value we already have. + if (request?.command === READ_COMMANDS.OPEN_PUBLIC_PROFILE_PAGE) { + for (const update of response?.onyxData ?? []) { + if (update.key !== ONYXKEYS.PERSONAL_DETAILS_LIST || !isRecord(update.value)) { continue; } - delete detail.login; + for (const [accountID, detail] of Object.entries(update.value)) { + if (!isRecord(detail) || detail.login !== '' || !getLoginByAccountID(Number(accountID))) { + continue; + } + delete detail.login; + } } } return response; diff --git a/tests/unit/MiddlewareTest.ts b/tests/unit/MiddlewareTest.ts index bf77a2c9e2b2..b8678951a358 100644 --- a/tests/unit/MiddlewareTest.ts +++ b/tests/unit/MiddlewareTest.ts @@ -686,5 +686,107 @@ describe('Middleware', () => { }); expect(personalDetails?.[knownAccountID]?.login).toBe(invitedEmail); }); + + test('OpenPublicProfilePage does not blank out a login the client already knows', async () => { + const knownAccountID = 333; + const knownLogin = 'known@example.com'; + await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, { + [knownAccountID]: { + accountID: knownAccountID, + login: knownLogin, + }, + }); + + Request.addMiddleware(handleUnusedOptimisticID); + Request.addMiddleware(SaveResponseInOnyx); + + SequentialQueue.push({ + command: 'OpenPublicProfilePage', + data: {authToken: 'testToken', accountID: knownAccountID}, + requestIndex: 16, + }); + + jest.spyOn(HttpUtils, 'xhr').mockResolvedValueOnce({ + jsonCode: 200, + onyxData: [ + { + onyxMethod: Onyx.METHOD.MERGE, + key: ONYXKEYS.PERSONAL_DETAILS_LIST, + value: { + [knownAccountID]: { + accountID: knownAccountID, + 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?.[knownAccountID]?.login).toBe(knownLogin); + }); + + test('A command other than OpenPublicProfilePage is not touched by the empty-login sanitizer', async () => { + const knownAccountID = 333; + const knownLogin = 'known@example.com'; + await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, { + [knownAccountID]: { + accountID: knownAccountID, + login: knownLogin, + }, + }); + + Request.addMiddleware(handleUnusedOptimisticID); + Request.addMiddleware(SaveResponseInOnyx); + + SequentialQueue.push({ + command: 'SomeOtherCommand', + data: {authToken: 'testToken'}, + requestIndex: 17, + }); + + jest.spyOn(HttpUtils, 'xhr').mockResolvedValueOnce({ + jsonCode: 200, + onyxData: [ + { + onyxMethod: Onyx.METHOD.MERGE, + key: ONYXKEYS.PERSONAL_DETAILS_LIST, + value: { + [knownAccountID]: { + accountID: knownAccountID, + 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?.[knownAccountID]?.login).toBe(''); + }); }); }); From e725a8826a926e0f1b9ea63739ab0948ddae448c Mon Sep 17 00:00:00 2001 From: Mukher Date: Thu, 16 Jul 2026 05:49:12 +0500 Subject: [PATCH 6/7] respect empty login --- .../Middleware/HandleUnusedOptimisticID.ts | 6 +- src/libs/actions/Report/index.ts | 1 + tests/unit/MiddlewareTest.ts | 69 +++++++++++++++++++ 3 files changed, 73 insertions(+), 3 deletions(-) diff --git a/src/libs/Middleware/HandleUnusedOptimisticID.ts b/src/libs/Middleware/HandleUnusedOptimisticID.ts index 38399c934067..ce727950cfd1 100644 --- a/src/libs/Middleware/HandleUnusedOptimisticID.ts +++ b/src/libs/Middleware/HandleUnusedOptimisticID.ts @@ -70,7 +70,8 @@ function getRestoredPersonalDetails( } if (update.key === ONYXKEYS.PERSONAL_DETAILS_LIST && isRecord(update.value)) { for (const [accountID, detail] of Object.entries(update.value)) { - if (!isRecord(detail) || detail.login) { + // 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); @@ -191,8 +192,7 @@ const handleUnusedOptimisticID: Middleware = (requestResponse, request, isFromSe }); } } - // OpenPublicProfilePage can echo back an explicitly empty login for an account the client - // already knows a login for; drop that so it can't blank out the value we already have. + // OpenPublicProfilePage never populates login, so a login: '' from it is serializer noise — don't let it blank out a known login if (request?.command === READ_COMMANDS.OPEN_PUBLIC_PROFILE_PAGE) { for (const update of response?.onyxData ?? []) { if (update.key !== ONYXKEYS.PERSONAL_DETAILS_LIST || !isRecord(update.value)) { diff --git a/src/libs/actions/Report/index.ts b/src/libs/actions/Report/index.ts index 2324b04371e2..b8700c3627ec 100644 --- a/src/libs/actions/Report/index.ts +++ b/src/libs/actions/Report/index.ts @@ -2145,6 +2145,7 @@ function prepareOnyxDataForCleanUpOptimisticParticipants( settledPersonalDetails[accountID] = null; redundantParticipants[accountID] = null; } else if (personalDetails?.[accountID] && !personalDetails[accountID].login) { + // '' counts as missing: SearchForReports seeds login: '' for unvalidated accounts, and in Onyx the two are indistinguishable missingLoginParticipants.push(Number(accountID)); } } diff --git a/tests/unit/MiddlewareTest.ts b/tests/unit/MiddlewareTest.ts index b8678951a358..74bd83ff2fe9 100644 --- a/tests/unit/MiddlewareTest.ts +++ b/tests/unit/MiddlewareTest.ts @@ -630,6 +630,75 @@ describe('Middleware', () => { 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 one from a server search', async () => { const optimisticReportID = '1234'; const knownAccountID = 333; From 89eaf61490e47c939a41338a9d398e9ce9283df1 Mon Sep 17 00:00:00 2001 From: Mukher Date: Thu, 16 Jul 2026 16:20:31 +0500 Subject: [PATCH 7/7] restore only when the login key is missing --- .../Middleware/HandleUnusedOptimisticID.ts | 17 +--- src/libs/actions/Report/index.ts | 3 +- tests/unit/MiddlewareTest.ts | 85 +++++-------------- 3 files changed, 21 insertions(+), 84 deletions(-) diff --git a/src/libs/Middleware/HandleUnusedOptimisticID.ts b/src/libs/Middleware/HandleUnusedOptimisticID.ts index ce727950cfd1..7a33eaf4238d 100644 --- a/src/libs/Middleware/HandleUnusedOptimisticID.ts +++ b/src/libs/Middleware/HandleUnusedOptimisticID.ts @@ -1,7 +1,6 @@ import {prepareOnyxDataForCleanUpOptimisticParticipants} from '@libs/actions/Report'; -import {READ_COMMANDS, WRITE_COMMANDS} from '@libs/API/types'; +import {WRITE_COMMANDS} from '@libs/API/types'; import deepReplaceKeysAndValues from '@libs/deepReplaceKeysAndValues'; -import {getLoginByAccountID} from '@libs/PersonalDetailsUtils'; import type {Middleware} from '@libs/Request'; import * as PersistedRequests from '@userActions/PersistedRequests'; @@ -192,20 +191,6 @@ const handleUnusedOptimisticID: Middleware = (requestResponse, request, isFromSe }); } } - // OpenPublicProfilePage never populates login, so a login: '' from it is serializer noise — don't let it blank out a known login - if (request?.command === READ_COMMANDS.OPEN_PUBLIC_PROFILE_PAGE) { - for (const update of response?.onyxData ?? []) { - if (update.key !== ONYXKEYS.PERSONAL_DETAILS_LIST || !isRecord(update.value)) { - continue; - } - for (const [accountID, detail] of Object.entries(update.value)) { - if (!isRecord(detail) || detail.login !== '' || !getLoginByAccountID(Number(accountID))) { - continue; - } - delete detail.login; - } - } - } return response; }); diff --git a/src/libs/actions/Report/index.ts b/src/libs/actions/Report/index.ts index b8700c3627ec..8be9d7dbe418 100644 --- a/src/libs/actions/Report/index.ts +++ b/src/libs/actions/Report/index.ts @@ -2144,8 +2144,7 @@ function prepareOnyxDataForCleanUpOptimisticParticipants( if (personalDetails?.[accountID]?.isOptimisticPersonalDetail) { settledPersonalDetails[accountID] = null; redundantParticipants[accountID] = null; - } else if (personalDetails?.[accountID] && !personalDetails[accountID].login) { - // '' counts as missing: SearchForReports seeds login: '' for unvalidated accounts, and in Onyx the two are indistinguishable + } else if (personalDetails?.[accountID] && !Object.hasOwn(personalDetails[accountID], 'login')) { missingLoginParticipants.push(Number(accountID)); } } diff --git a/tests/unit/MiddlewareTest.ts b/tests/unit/MiddlewareTest.ts index 74bd83ff2fe9..a2ea9276f88e 100644 --- a/tests/unit/MiddlewareTest.ts +++ b/tests/unit/MiddlewareTest.ts @@ -699,7 +699,7 @@ describe('Middleware', () => { expect(personalDetails?.[settledAccountID]?.login).toBe(''); }); - test('OpenReport restores the invited login when the participant is already known without one from a server search', async () => { + 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'; @@ -711,8 +711,6 @@ describe('Middleware', () => { [ONYXKEYS.PERSONAL_DETAILS_LIST]: { [knownAccountID]: { accountID: knownAccountID, - login: '', - displayName: '', }, }, }); @@ -756,64 +754,21 @@ describe('Middleware', () => { expect(personalDetails?.[knownAccountID]?.login).toBe(invitedEmail); }); - test('OpenPublicProfilePage does not blank out a login the client already knows', async () => { + 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 knownLogin = 'known@example.com'; - await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, { - [knownAccountID]: { - accountID: knownAccountID, - login: knownLogin, + const invitedEmail = 'invited@example.com'; + await Onyx.multiSet({ + [`${ONYXKEYS.COLLECTION.REPORT}${optimisticReportID}` as const]: { + reportID: optimisticReportID, + participants: {[knownAccountID]: {notificationPreference: 'always'}}, }, - }); - - Request.addMiddleware(handleUnusedOptimisticID); - Request.addMiddleware(SaveResponseInOnyx); - - SequentialQueue.push({ - command: 'OpenPublicProfilePage', - data: {authToken: 'testToken', accountID: knownAccountID}, - requestIndex: 16, - }); - - jest.spyOn(HttpUtils, 'xhr').mockResolvedValueOnce({ - jsonCode: 200, - onyxData: [ - { - onyxMethod: Onyx.METHOD.MERGE, - key: ONYXKEYS.PERSONAL_DETAILS_LIST, - value: { - [knownAccountID]: { - accountID: knownAccountID, - 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); + [ONYXKEYS.PERSONAL_DETAILS_LIST]: { + [knownAccountID]: { + accountID: knownAccountID, + login: '', + displayName: '', }, - }); - }); - expect(personalDetails?.[knownAccountID]?.login).toBe(knownLogin); - }); - - test('A command other than OpenPublicProfilePage is not touched by the empty-login sanitizer', async () => { - const knownAccountID = 333; - const knownLogin = 'known@example.com'; - await Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, { - [knownAccountID]: { - accountID: knownAccountID, - login: knownLogin, }, }); @@ -821,9 +776,9 @@ describe('Middleware', () => { Request.addMiddleware(SaveResponseInOnyx); SequentialQueue.push({ - command: 'SomeOtherCommand', - data: {authToken: 'testToken'}, - requestIndex: 17, + command: 'OpenReport', + data: {authToken: 'testToken', reportID: optimisticReportID, createdReportActionID: '5678', emailList: invitedEmail}, + requestIndex: 16, }); jest.spyOn(HttpUtils, 'xhr').mockResolvedValueOnce({ @@ -831,12 +786,10 @@ describe('Middleware', () => { onyxData: [ { onyxMethod: Onyx.METHOD.MERGE, - key: ONYXKEYS.PERSONAL_DETAILS_LIST, + key: `${ONYXKEYS.COLLECTION.REPORT}${optimisticReportID}`, value: { - [knownAccountID]: { - accountID: knownAccountID, - login: '', - }, + reportID: optimisticReportID, + participants: {[knownAccountID]: {notificationPreference: 'always'}}, }, }, ],