From 34399478d3323a131e308b35859a4c592f10f6b6 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Thu, 17 Sep 2026 17:09:09 +0800 Subject: [PATCH 1/2] resolve the account id in getQueryWithSubstitutions --- .../Search/SearchRouter/SearchRouter.tsx | 5 +- .../SearchRouter/getQueryWithSubstitutions.ts | 12 ++- src/hooks/usePersonalDetailByLogin.ts | 13 +++- src/libs/SearchQueryUtils.ts | 17 +--- tests/unit/Search/SearchQueryUtilsTest.ts | 36 +++------ .../Search/getQueryWithSubstitutionsTest.ts | 77 +++++++++++++++++++ 6 files changed, 114 insertions(+), 46 deletions(-) diff --git a/src/components/Search/SearchRouter/SearchRouter.tsx b/src/components/Search/SearchRouter/SearchRouter.tsx index 3e0f6f831c11..921e68419bcf 100644 --- a/src/components/Search/SearchRouter/SearchRouter.tsx +++ b/src/components/Search/SearchRouter/SearchRouter.tsx @@ -19,6 +19,7 @@ import useKeyboardShortcut from '@hooks/useKeyboardShortcut'; import {useMemoizedLazyExpensifyIcons} from '@hooks/useLazyAsset'; import useLocalize from '@hooks/useLocalize'; import useOnyx from '@hooks/useOnyx'; +import {useGetPersonalDetailsByLogin} from '@hooks/usePersonalDetailByLogin'; import useReportAttributes from '@hooks/useReportAttributes'; import useReportOrReportDraft from '@hooks/useReportOrReportDraft'; import useResponsiveLayout from '@hooks/useResponsiveLayout'; @@ -122,6 +123,7 @@ function SearchRouter({onRouterClose, shouldHideInputCaret, isSearchRouterDispla const [personalAndWorkspaceCards] = useOnyx(ONYXKEYS.DERIVED.PERSONAL_AND_WORKSPACE_CARD_LIST); const [allFeeds] = useOnyx(ONYXKEYS.COLLECTION.SHARED_NVP_PRIVATE_DOMAIN_MEMBER); const [bankAccountList] = useOnyx(ONYXKEYS.BANK_ACCOUNT_LIST); + const getPersonalDetailsByLogin = useGetPersonalDetailsByLogin(); const feedKeysWithCards = useFeedKeysWithAssignedCards(); const reportAttributes = useReportAttributes(); @@ -379,7 +381,7 @@ function SearchRouter({onRouterClose, shouldHideInputCaret, isSearchRouterDispla const submitSearch = useCallback( (queryString: SearchQueryString, shouldSkipAmountConversion = false) => { - const queryWithSubstitutions = getQueryWithSubstitutions(queryString, autocompleteSubstitutions, currentUserAccountID); + const queryWithSubstitutions = getQueryWithSubstitutions(queryString, autocompleteSubstitutions, currentUserAccountID, getPersonalDetailsByLogin()); const updatedQuery = getQueryWithUpdatedValues(queryWithSubstitutions, shouldSkipAmountConversion, policies); if (!updatedQuery) { return; @@ -406,6 +408,7 @@ function SearchRouter({onRouterClose, shouldHideInputCaret, isSearchRouterDispla [ autocompleteSubstitutions, currentUserAccountID, + getPersonalDetailsByLogin, onRouterClose, setAutocompleteQueryValue, setTextInputValue, diff --git a/src/components/Search/SearchRouter/getQueryWithSubstitutions.ts b/src/components/Search/SearchRouter/getQueryWithSubstitutions.ts index 99a4eb58df14..c8e2d7971c7c 100644 --- a/src/components/Search/SearchRouter/getQueryWithSubstitutions.ts +++ b/src/components/Search/SearchRouter/getQueryWithSubstitutions.ts @@ -1,3 +1,4 @@ +import type {PersonalDetailsByLogin} from '@components/PersonalDetailsByLoginProvider'; import type {SearchAutocompleteQueryRange, SearchFilterKey} from '@components/Search/types'; import {parse} from '@libs/SearchParser/autocompleteParser'; @@ -40,7 +41,7 @@ const getSubstitutionMapKeyWithIndex = (filterKey: SearchFilterKey, value: strin * } * return: `A from:9876 A` */ -function getQueryWithSubstitutions(changedQuery: string, substitutions: SubstitutionMap, currentUserAccountID?: number) { +function getQueryWithSubstitutions(changedQuery: string, substitutions: SubstitutionMap, currentUserAccountID?: number, personalDetailsByLogin?: PersonalDetailsByLogin) { const parsed = parse(changedQuery) as {ranges: SearchAutocompleteQueryRange[]}; const searchAutocompleteQueryRanges = parsed.ranges; @@ -75,6 +76,15 @@ function getQueryWithSubstitutions(changedQuery: string, substitutions: Substitu substitutionEntry = currentUserAccountID.toString(); } + // Resolve a login that was typed out by hand rather than picked from the autocomplete, so that the query carries + // account IDs whichever way the user filled it in. + if (!substitutionEntry && USER_FILTER_KEYS.has(range.key)) { + const accountID = personalDetailsByLogin?.[range.value]?.accountID; + if (accountID) { + substitutionEntry = accountID.toString(); + } + } + if (substitutionEntry) { const substitutionStart = range.start + lengthDiff; const substitutionEnd = range.start + range.length; diff --git a/src/hooks/usePersonalDetailByLogin.ts b/src/hooks/usePersonalDetailByLogin.ts index a32c28393378..66548aecdb9f 100644 --- a/src/hooks/usePersonalDetailByLogin.ts +++ b/src/hooks/usePersonalDetailByLogin.ts @@ -22,6 +22,17 @@ function haveSameDetails(first: PersonalDetailsByLogin, second: PersonalDetailsB return firstLogins.every((login) => first[login] === second[login]); } +/** + * Returns a getter for the whole login -> personal details map. + * + * Unlike the hooks below it does not subscribe to anything, so its consumers never re-render when personal + * details change. Because of that the getter may only be called from an event handler or another imperative + * context, never during render, where reading the store has to go through `useSyncExternalStore` to stay in sync. + */ +function useGetPersonalDetailsByLogin(): () => PersonalDetailsByLogin { + return useContext(PersonalDetailsByLoginContext).getSnapshot; +} + /** * Returns the personal details of a single login, or `undefined` when there are none for it. * @@ -82,4 +93,4 @@ function usePersonalDetailsByLogins(logins: Array, } export default usePersonalDetailByLogin; -export {usePersonalDetailsByLogins}; +export {useGetPersonalDetailsByLogin, usePersonalDetailsByLogins}; diff --git a/src/libs/SearchQueryUtils.ts b/src/libs/SearchQueryUtils.ts index 4beebe7150de..e6f7347d9185 100644 --- a/src/libs/SearchQueryUtils.ts +++ b/src/libs/SearchQueryUtils.ts @@ -68,7 +68,7 @@ import {validateAmount} from './MoneyRequestUtils'; import {getPreservedNavigatorState} from './Navigation/AppNavigator/createSplitNavigator/usePreserveNavigatorState'; import navigationRef from './Navigation/navigationRef'; import {isRecord} from './ObjectUtils'; -import {getPersonalDetailByEmail, temporaryGetDisplayNameOrDefault} from './PersonalDetailsUtils'; +import {temporaryGetDisplayNameOrDefault} from './PersonalDetailsUtils'; import {getCleanedTagName, getValidConnectedIntegration} from './PolicyUtils'; import {deprecatedGetReportName} from './ReportNameUtils'; import {parse as parseSearchQuery} from './SearchParser/searchParser'; @@ -523,21 +523,6 @@ function getUpdatedFilterValue(filterName: SyntaxFilterKey, filterValue: string }); } - if ( - filterName === CONST.SEARCH.SYNTAX_FILTER_KEYS.FROM || - filterName === CONST.SEARCH.SYNTAX_FILTER_KEYS.TO || - filterName === CONST.SEARCH.SYNTAX_FILTER_KEYS.PAYER || - filterName === CONST.SEARCH.SYNTAX_FILTER_KEYS.PAID_BY || - filterName === CONST.SEARCH.SYNTAX_FILTER_KEYS.EXPORTER || - filterName === CONST.SEARCH.SYNTAX_FILTER_KEYS.ATTENDEE - ) { - if (typeof filterValue === 'string') { - return getPersonalDetailByEmail(filterValue)?.accountID.toString() ?? filterValue; - } - - return filterValue.map((email) => getPersonalDetailByEmail(email)?.accountID.toString() ?? email); - } - if (filterName === CONST.SEARCH.SYNTAX_FILTER_KEYS.POLICY_ID) { if (typeof filterValue === 'string') { return resolvePolicyIDFromName(filterValue, policies); diff --git a/tests/unit/Search/SearchQueryUtilsTest.ts b/tests/unit/Search/SearchQueryUtilsTest.ts index d2dd473da64f..7ee4060294ec 100644 --- a/tests/unit/Search/SearchQueryUtilsTest.ts +++ b/tests/unit/Search/SearchQueryUtilsTest.ts @@ -1,7 +1,6 @@ import type {ASTNode, QueryFilter, SearchFilterKey, SearchQueryJSON} from '@components/Search/types'; import {generatePolicyID} from '@libs/actions/Policy/Policy'; -import type * as PersonalDetailsUtils from '@libs/PersonalDetailsUtils'; import CONST from '@src/CONST'; import DateUtils from '@src/libs/DateUtils'; @@ -76,15 +75,6 @@ jest.mock('@libs/Navigation/navigationRef', () => ({ }, })); -const personalDetailsFakeData = { - 'johndoe@example.com': { - accountID: 12345, - }, - 'janedoe@example.com': { - accountID: 78901, - }, -} as Record; - jest.mock('@libs/SearchParser/searchParser', () => { const actual = jest.requireActual<{parse: (...args: unknown[]) => unknown}>('@libs/SearchParser/searchParser'); return { @@ -93,16 +83,6 @@ jest.mock('@libs/SearchParser/searchParser', () => { }; }); -jest.mock('@libs/PersonalDetailsUtils', () => { - const actual = jest.requireActual('@libs/PersonalDetailsUtils'); - return { - ...actual, - getPersonalDetailByEmail(email: string) { - return personalDetailsFakeData[email]; - }, - }; -}); - // The default query is generated by default values from parser, which are defined in grammar. // We don't want to test or mock the grammar and the parser, so we're simply defining this string directly here. // Note: view:table is not included because it's only added when explicitly set by the user. @@ -332,16 +312,18 @@ describe('SearchQueryUtils', () => { expect(getQueryWithUpdatedValues('category:Travel,Meals')).toEqual(`${defaultQuery} category:Travel,Meals`); }); - test('returns query with user emails substituted', () => { + // Logins are resolved to account IDs upstream, in getQueryWithSubstitutions, so that the raw query carries + // account IDs too. This function only has to leave whatever it is handed alone. + test('leaves user logins untouched', () => { const userQuery = 'from:johndoe@example.com hello'; const result = getQueryWithUpdatedValues(userQuery); - expect(result).toEqual(`${defaultQuery} from:12345 hello`); + expect(result).toEqual(`${defaultQuery} from:johndoe@example.com hello`); }); - test('returns query with user emails substituted and preserves user ids', () => { - const userQuery = 'from:johndoe@example.com to:112233'; + test('preserves user ids', () => { + const userQuery = 'from:12345 to:112233'; const result = getQueryWithUpdatedValues(userQuery); @@ -349,7 +331,7 @@ describe('SearchQueryUtils', () => { }); test('returns query with all of the fields correctly substituted', () => { - const userQuery = 'from:9876,87654 to:janedoe@example.com hello amount:150 test'; + const userQuery = 'from:9876,87654 to:78901 hello amount:150 test'; const result = getQueryWithUpdatedValues(userQuery); @@ -357,7 +339,7 @@ describe('SearchQueryUtils', () => { }); test('returns query with updated groupBy', () => { - const userQuery = 'from:johndoe@example.com groupBy:reports'; + const userQuery = 'from:12345 groupBy:reports'; const result = getQueryWithUpdatedValues(userQuery); @@ -365,7 +347,7 @@ describe('SearchQueryUtils', () => { }); test('returns query with updated view', () => { - const userQuery = 'from:johndoe@example.com view:bar'; + const userQuery = 'from:12345 view:bar'; const result = getQueryWithUpdatedValues(userQuery); diff --git a/tests/unit/Search/getQueryWithSubstitutionsTest.ts b/tests/unit/Search/getQueryWithSubstitutionsTest.ts index b7635cf86546..6a73deca26ae 100644 --- a/tests/unit/Search/getQueryWithSubstitutionsTest.ts +++ b/tests/unit/Search/getQueryWithSubstitutionsTest.ts @@ -1,7 +1,18 @@ /* eslint-disable @typescript-eslint/naming-convention */ // we need "dirty" object key names in these tests +import type {PersonalDetailsByLogin} from '@components/PersonalDetailsByLoginProvider'; + import {getQueryWithSubstitutions} from '@src/components/Search/SearchRouter/getQueryWithSubstitutions'; +const personalDetailsByLogin = { + 'johndoe@example.com': { + accountID: 12345, + }, + 'janedoe@example.com': { + accountID: 78901, + }, +} as PersonalDetailsByLogin; + describe('getQueryWithSubstitutions should compute and return correct new query', () => { test('when both queries contain no substitutions', () => { // given this previous query: "foo" @@ -163,4 +174,70 @@ describe('getQueryWithSubstitutions should compute and return correct new query' expect(result).toBe('from:me'); }); + + test('when a login is typed out by hand rather than picked from the autocomplete, it resolves to an account ID', () => { + const userTypedQuery = 'from:johndoe@example.com hello'; + const substitutionsMock = {}; + + const result = getQueryWithSubstitutions(userTypedQuery, substitutionsMock, undefined, personalDetailsByLogin); + + expect(result).toBe('from:12345 hello'); + }); + + test('when a hand-typed login is used on every user-based filter key, each occurrence resolves', () => { + const userTypedQuery = + 'from:johndoe@example.com to:janedoe@example.com assignee:johndoe@example.com payer:janedoe@example.com exporter:johndoe@example.com attendee:janedoe@example.com'; + const substitutionsMock = {}; + + const result = getQueryWithSubstitutions(userTypedQuery, substitutionsMock, undefined, personalDetailsByLogin); + + expect(result).toBe('from:12345 to:78901 assignee:12345 payer:78901 exporter:12345 attendee:78901'); + }); + + test('when a comma separated list mixes hand-typed logins and account IDs, only the logins are resolved', () => { + const userTypedQuery = 'from:johndoe@example.com,55555,janedoe@example.com'; + const substitutionsMock = {}; + + const result = getQueryWithSubstitutions(userTypedQuery, substitutionsMock, undefined, personalDetailsByLogin); + + expect(result).toBe('from:12345,55555,78901'); + }); + + test('when a hand-typed login has no personal details, it is left unresolved', () => { + const userTypedQuery = 'from:nobody@example.com'; + const substitutionsMock = {}; + + const result = getQueryWithSubstitutions(userTypedQuery, substitutionsMock, undefined, personalDetailsByLogin); + + expect(result).toBe('from:nobody@example.com'); + }); + + test('when an existing substitution exists for a login, it takes precedence over the personal details', () => { + const userTypedQuery = 'from:johndoe@example.com'; + const substitutionsMock = { + 'from:johndoe@example.com': '5555', + }; + + const result = getQueryWithSubstitutions(userTypedQuery, substitutionsMock, undefined, personalDetailsByLogin); + + expect(result).toBe('from:5555'); + }); + + test('when a login is used on a non-user-based filter, it is not resolved to an account ID', () => { + const userTypedQuery = 'merchant:johndoe@example.com'; + const substitutionsMock = {}; + + const result = getQueryWithSubstitutions(userTypedQuery, substitutionsMock, undefined, personalDetailsByLogin); + + expect(result).toBe('merchant:johndoe@example.com'); + }); + + test('when personalDetailsByLogin is not passed, a hand-typed login is left unresolved', () => { + const userTypedQuery = 'from:johndoe@example.com'; + const substitutionsMock = {}; + + const result = getQueryWithSubstitutions(userTypedQuery, substitutionsMock); + + expect(result).toBe('from:johndoe@example.com'); + }); }); From 6cda50ec0336c5aad73c479ae47e1efd8f1af45c Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Thu, 17 Sep 2026 19:52:07 +0800 Subject: [PATCH 2/2] update comment --- src/hooks/usePersonalDetailByLogin.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hooks/usePersonalDetailByLogin.ts b/src/hooks/usePersonalDetailByLogin.ts index 66548aecdb9f..8d07da1eb28f 100644 --- a/src/hooks/usePersonalDetailByLogin.ts +++ b/src/hooks/usePersonalDetailByLogin.ts @@ -23,7 +23,7 @@ function haveSameDetails(first: PersonalDetailsByLogin, second: PersonalDetailsB } /** - * Returns a getter for the whole login -> personal details map. + * Returns a getter for the whole map of personal details keyed by login. * * Unlike the hooks below it does not subscribe to anything, so its consumers never re-render when personal * details change. Because of that the getter may only be called from an event handler or another imperative