Remove getPersonalDetailByEmail from getUpdatedFilterValue

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,

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;

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 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
+ * 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<TReturn>(logins: Array<string | undefined>,
 }
 
 export default usePersonalDetailByLogin;
-export {usePersonalDetailsByLogins};
+export {useGetPersonalDetailsByLogin, usePersonalDetailsByLogins};

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);

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<string, {accountID: number}>;
-
 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<typeof PersonalDetailsUtils>('@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,40 +312,42 @@ 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);
 
             expect(result).toEqual(`${defaultQuery} from:12345 to:112233`);
         });
 
         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);
 
             expect(result).toEqual(`${defaultQuery} from:9876,87654 to:78901 amount:15000 hello test`);
         });
 
         test('returns query with updated groupBy', () => {
-            const userQuery = 'from:johndoe@example.com groupBy:reports';
+            const userQuery = 'from:12345 groupBy:reports';
 
             const result = getQueryWithUpdatedValues(userQuery);
 
             expect(result).toEqual(`${defaultQuery} groupBy:reports from:12345`);
         });
 
         test('returns query with updated view', () => {
-            const userQuery = 'from:johndoe@example.com view:bar';
+            const userQuery = 'from:12345 view:bar';
 
             const result = getQueryWithUpdatedValues(userQuery);
 

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');
+    });
 });