From 4981a0454f8e6fb1b006d11d5b20611123f9c706 Mon Sep 17 00:00:00 2001 From: Tomasz Misiukiewicz Date: Mon, 29 Jul 2024 11:11:27 +0200 Subject: [PATCH 1/7] remove usage of searchText for personal details --- src/CONST.ts | 4 ++++ src/libs/OptionsListUtils.ts | 21 +++++++++---------- src/pages/NewChatPage.tsx | 18 +++++++--------- .../MoneyRequestParticipantsSelector.tsx | 2 +- 4 files changed, 23 insertions(+), 22 deletions(-) diff --git a/src/CONST.ts b/src/CONST.ts index d929a01e030a6..ad61f839d0cb9 100755 --- a/src/CONST.ts +++ b/src/CONST.ts @@ -1118,6 +1118,10 @@ const CONST = { // It's copied here so that the same regex pattern can be used in form validations to be consistent with the server. VALIDATE_FOR_HTML_TAG_REGEX: /<([^>\s]+)(?:[^>]*?)>/g, + // The regex below is used to remove dots only from the local part of the user email (local-part@domain) + // so when we are using search, we can match emails that have dots without explicitly writing the dots (e.g: fistlast@domain will match first.last@domain) + EMAIL_SEARCH_REGEX: /\.(?=[^\s@]*@)/g, + VALIDATE_FOR_LEADINGSPACES_HTML_TAG_REGEX: /<([\s]+.+[\s]*)>/g, WHITELISTED_TAGS: [/<>/, /< >/, /<->/, /<-->/, /
/, //], diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index 033f92b914f1f..f5da139138fa9 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -2451,7 +2451,7 @@ function formatSectionsFromSearchTerm( // This will add them to the list of options, deduping them if they already exist in the other lists const selectedParticipantsWithoutDetails = selectedOptions.filter((participant) => { const accountID = participant.accountID ?? null; - const isPartOfSearchTerm = participant.searchText?.toLowerCase().includes(searchTerm.trim().toLowerCase()); + const isPartOfSearchTerm = getPersonalDetailSearchTerms(participant).join(' ').toLowerCase().includes(searchTerm.trim().toLowerCase()); const isReportInRecentReports = filteredRecentReports.some((report) => report.accountID === accountID); const isReportInPersonalDetails = filteredPersonalDetails.some((personalDetail) => personalDetail.accountID === accountID); return isPartOfSearchTerm && !isReportInRecentReports && !isReportInPersonalDetails; @@ -2483,6 +2483,10 @@ function getFirstKeyForList(data?: Option[] | null) { return firstNonEmptyDataObj.keyForList ? firstNonEmptyDataObj.keyForList : ''; } + +function getPersonalDetailSearchTerms(item: Partial) { + return [item.participantsList?.[0]?.displayName ?? '', item.login ?? '', item.login?.replace(CONST.EMAIL_SEARCH_REGEX, '') ?? '']; +} /** * Filters options based on the search input value */ @@ -2504,10 +2508,6 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt const searchValue = parsedPhoneNumber.possible && parsedPhoneNumber.number?.e164 ? parsedPhoneNumber.number.e164 : searchInputValue.toLowerCase(); const searchTerms = searchValue ? searchValue.split(' ') : []; - // The regex below is used to remove dots only from the local part of the user email (local-part@domain) - // so that we can match emails that have dots without explicitly writing the dots (e.g: fistlast@domain will match first.last@domain) - const emailRegex = /\.(?=[^\s@]*@)/g; - const optionsToExclude: Option[] = [{login: CONST.EMAIL.NOTIFICATIONS}]; excludeLogins.forEach((login) => { @@ -2527,7 +2527,7 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt if (login) { keys.push(login); - keys.push(login.replace(emailRegex, '')); + keys.push(login.replace(CONST.EMAIL_SEARCH_REGEX, '')); } }); } @@ -2543,7 +2543,7 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt if (item.login) { values.push(item.login); - values.push(item.login.replace(emailRegex, '')); + values.push(item.login.replace(CONST.EMAIL_SEARCH_REGEX, '')); } if (item.isThread) { @@ -2569,14 +2569,12 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt return uniqFast(values); }); - const personalDetails = filterArrayByMatch(items.personalDetails, term, (item) => - uniqFast([item.participantsList?.[0]?.displayName ?? '', item.login ?? '', item.login?.replace(emailRegex, '') ?? '']), - ); + const personalDetails = filterArrayByMatch(items.personalDetails, term, (item) => uniqFast(getPersonalDetailSearchTerms(item))); const currentUserOptionSearchText = uniqFast([ items.currentUserOption?.text ?? '', items.currentUserOption?.login ?? '', - items.currentUserOption?.login?.replace(emailRegex, '') ?? '', + items.currentUserOption?.login?.replace(CONST.EMAIL_SEARCH_REGEX, '') ?? '', ]).join(' '); const currentUserOption = isSearchStringMatch(term, currentUserOptionSearchText) ? items.currentUserOption : null; @@ -2673,6 +2671,7 @@ export { canCreateOptimisticPersonalDetailOption, getUserToInviteOption, shouldShowViolations, + getPersonalDetailSearchTerms, }; export type {MemberForList, CategorySection, CategoryTreeSection, Options, OptionList, SearchOption, PayeePersonalDetails, Category, Tax, TaxRatesOption, Option, OptionTree}; diff --git a/src/pages/NewChatPage.tsx b/src/pages/NewChatPage.tsx index f4f525f1b60c4..90cfbf50588c1 100755 --- a/src/pages/NewChatPage.tsx +++ b/src/pages/NewChatPage.tsx @@ -86,16 +86,14 @@ function useOptions({isGroupChat}: NewChatPageProps) { return filteredOptions; }, [debouncedSearchTerm, defaultOptions, isGroupChat, selectedOptions]); - const headerMessage = useMemo( - () => - OptionsListUtils.getHeaderMessage( - options.personalDetails.length + options.recentReports.length !== 0, - !!options.userToInvite, - debouncedSearchTerm.trim(), - selectedOptions.some((participant) => participant?.searchText?.toLowerCase?.().includes(debouncedSearchTerm.trim().toLowerCase())), - ), - [debouncedSearchTerm, options.personalDetails.length, options.recentReports.length, options.userToInvite, selectedOptions], - ); + const headerMessage = useMemo(() => { + return OptionsListUtils.getHeaderMessage( + options.personalDetails.length + options.recentReports.length !== 0, + !!options.userToInvite, + debouncedSearchTerm.trim(), + selectedOptions.some((participant) => OptionsListUtils.getPersonalDetailSearchTerms(participant).join(' ').toLowerCase?.().includes(debouncedSearchTerm.trim().toLowerCase())), + ); + }, [debouncedSearchTerm, options.personalDetails.length, options.recentReports.length, options.userToInvite, selectedOptions]); useEffect(() => { if (!debouncedSearchTerm.length) { diff --git a/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx b/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx index 5f10b403f5e97..00ed1dd4ad331 100644 --- a/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx +++ b/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx @@ -216,7 +216,7 @@ function MoneyRequestParticipantsSelector({participants = CONST.EMPTY_ARRAY, onF (chatOptions.personalDetails ?? []).length + (chatOptions.recentReports ?? []).length !== 0, !!chatOptions?.userToInvite, debouncedSearchTerm.trim(), - participants.some((participant) => participant?.searchText?.toLowerCase().includes(debouncedSearchTerm.trim().toLowerCase())), + participants.some((participant) => OptionsListUtils.getPersonalDetailSearchTerms(participant).join(' ').toLowerCase().includes(debouncedSearchTerm.trim().toLowerCase())), ); return [newSections, headerMessage]; From dad7d5642d8482cfb652208d5a4d1edee1a9fad3 Mon Sep 17 00:00:00 2001 From: Tomasz Misiukiewicz Date: Mon, 29 Jul 2024 12:59:28 +0200 Subject: [PATCH 2/7] remove usage of searchtext in current user option --- src/libs/OptionsListUtils.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index f5da139138fa9..c237e85e04474 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -2112,7 +2112,7 @@ function getOptions( }); let currentUserOption = allPersonalDetailsOptions.find((personalDetailsOption) => personalDetailsOption.login === currentUserLogin); - if (searchValue && currentUserOption && !isSearchStringMatch(searchValue, currentUserOption.searchText)) { + if (searchValue && currentUserOption && !isSearchStringMatch(searchValue, getCurrentUserSearchTerms(currentUserOption).join(' '))) { currentUserOption = undefined; } @@ -2487,6 +2487,10 @@ function getFirstKeyForList(data?: Option[] | null) { function getPersonalDetailSearchTerms(item: Partial) { return [item.participantsList?.[0]?.displayName ?? '', item.login ?? '', item.login?.replace(CONST.EMAIL_SEARCH_REGEX, '') ?? '']; } + +function getCurrentUserSearchTerms(item: ReportUtils.OptionData) { + return [item.text ?? '', item.login ?? '', item.login?.replace(CONST.EMAIL_SEARCH_REGEX, '') ?? '']; +} /** * Filters options based on the search input value */ @@ -2571,11 +2575,7 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt }); const personalDetails = filterArrayByMatch(items.personalDetails, term, (item) => uniqFast(getPersonalDetailSearchTerms(item))); - const currentUserOptionSearchText = uniqFast([ - items.currentUserOption?.text ?? '', - items.currentUserOption?.login ?? '', - items.currentUserOption?.login?.replace(CONST.EMAIL_SEARCH_REGEX, '') ?? '', - ]).join(' '); + const currentUserOptionSearchText = items.currentUserOption ? uniqFast(getCurrentUserSearchTerms(items.currentUserOption)).join(' ') : ''; const currentUserOption = isSearchStringMatch(term, currentUserOptionSearchText) ? items.currentUserOption : null; @@ -2672,6 +2672,7 @@ export { getUserToInviteOption, shouldShowViolations, getPersonalDetailSearchTerms, + getCurrentUserSearchTerms, }; export type {MemberForList, CategorySection, CategoryTreeSection, Options, OptionList, SearchOption, PayeePersonalDetails, Category, Tax, TaxRatesOption, Option, OptionTree}; From a5e171e51d54d66d1c3b2bab0d9c96bffc4688da Mon Sep 17 00:00:00 2001 From: Tomasz Misiukiewicz Date: Mon, 29 Jul 2024 14:18:05 +0200 Subject: [PATCH 3/7] remove getSearchText function --- src/libs/OptionsListUtils.ts | 48 ------------------------------------ src/libs/SidebarUtils.ts | 1 - 2 files changed, 49 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index c237e85e04474..9b5f1bb8f59a6 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -512,50 +512,6 @@ function uniqFast(items: string[]): string[] { return result; } -/** - * Returns a string with all relevant search terms. - * - * This method must be incredibly performant. It was found to be a big performance bottleneck - * when dealing with accounts that have thousands of reports. For loops are more efficient than _.each - * Array.prototype.push.apply is faster than using the spread operator. - */ -function getSearchText( - report: OnyxInputOrEntry, - reportName: string, - personalDetailList: Array>, - isChatRoomOrPolicyExpenseChat: boolean, - isThread: boolean, -): string { - const searchTerms: string[] = []; - - for (const personalDetail of personalDetailList) { - if (personalDetail.login) { - // The regex below is used to remove dots only from the local part of the user email (local-part@domain) - // so that we can match emails that have dots without explicitly writing the dots (e.g: fistlast@domain will match first.last@domain) - // More info https://github.com/Expensify/App/issues/8007 - searchTerms.push(PersonalDetailsUtils.getDisplayNameOrDefault(personalDetail, '', false), personalDetail.login, personalDetail.login.replace(/\.(?=[^\s@]*@)/g, '')); - } - } - - if (report) { - Array.prototype.push.apply(searchTerms, reportName.split(/[,\s]/)); - - if (isThread) { - const title = ReportUtils.getReportName(report); - const chatRoomSubtitle = ReportUtils.getChatRoomSubtitle(report); - - Array.prototype.push.apply(searchTerms, title.split(/[,\s]/)); - Array.prototype.push.apply(searchTerms, chatRoomSubtitle?.split(/[,\s]/) ?? ['']); - } else if (isChatRoomOrPolicyExpenseChat) { - const chatRoomSubtitle = ReportUtils.getChatRoomSubtitle(report); - - Array.prototype.push.apply(searchTerms, chatRoomSubtitle?.split(/[,\s]/) ?? ['']); - } - } - - return uniqFast(searchTerms).join(' '); -} - /** * Get an object of error messages keyed by microtime by combining all error objects related to the report. */ @@ -885,9 +841,6 @@ function createOption( } result.text = reportName; - // Disabling this line for safeness as nullish coalescing works only if the value is undefined or null - // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing - result.searchText = getSearchText(report, reportName, personalDetailList, !!result.isChatRoom || !!result.isPolicyExpenseChat, !!result.isThread); result.icons = ReportUtils.getIcons(report, personalDetails, personalDetail?.avatar, personalDetail?.login, personalDetail?.accountID, null); result.subtitle = subtitle; @@ -2642,7 +2595,6 @@ export { getSearchValueForPhoneOrEmail, getPersonalDetailsForAccountIDs, getIOUConfirmationOptionsFromPayeePersonalDetail, - getSearchText, isSearchStringMatchUserDetails, getAllReportErrors, getPolicyExpenseReportOption, diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index 9dc0d2abcbda9..ea71dc58ad999 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -469,7 +469,6 @@ function getOptionData({ result.participantsList = participantPersonalDetailList; result.icons = ReportUtils.getIcons(report, personalDetails, personalDetail?.avatar, personalDetail?.login, personalDetail?.accountID, policy); - result.searchText = OptionsListUtils.getSearchText(report, reportName, participantPersonalDetailList, result.isChatRoom || result.isPolicyExpenseChat, result.isThread); result.displayNamesWithTooltips = displayNamesWithTooltips; if (status) { From 28aa5a400423e6d2a17aac7dea2320905514f217 Mon Sep 17 00:00:00 2001 From: Tomasz Misiukiewicz Date: Mon, 29 Jul 2024 14:50:44 +0200 Subject: [PATCH 4/7] remove unused search code --- src/libs/OptionsListUtils.ts | 27 +-------------------------- 1 file changed, 1 insertion(+), 26 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index 9b5f1bb8f59a6..2a444546979ae 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -744,7 +744,6 @@ function createOption( phoneNumber: undefined, hasDraftComment: false, keyForList: undefined, - searchText: undefined, isDefaultRoom: false, isPinned: false, isWaitingOnBankAccount: false, @@ -2015,22 +2014,6 @@ function getOptions( continue; } - // Finally check to see if this option is a match for the provided search string if we have one - const {searchText, participantsList, isChatRoom} = reportOption; - const participantNames = getParticipantNames(participantsList); - - if (searchValue) { - // Determine if the search is happening within a chat room and starts with the report ID - const isReportIdSearch = isChatRoom && Str.startsWith(reportOption.reportID ?? '-1', searchValue); - - // Check if the search string matches the search text or participant names considering the type of the room - const isSearchMatch = isSearchStringMatch(searchValue, searchText, participantNames, isChatRoom); - - if (!isReportIdSearch && !isSearchMatch) { - continue; - } - } - reportOption.isSelected = isReportSelected(reportOption, selectedOptions); if (action === CONST.IOU.ACTION.CATEGORIZE) { @@ -2055,19 +2038,11 @@ function getOptions( if (personalDetailsOptionsToExclude.some((optionToExclude) => optionToExclude.login === personalDetailOption.login)) { return; } - const {searchText, participantsList, isChatRoom} = personalDetailOption; - const participantNames = getParticipantNames(participantsList); - if (searchValue && !isSearchStringMatch(searchValue, searchText, participantNames, isChatRoom)) { - return; - } personalDetailsOptions.push(personalDetailOption); }); - let currentUserOption = allPersonalDetailsOptions.find((personalDetailsOption) => personalDetailsOption.login === currentUserLogin); - if (searchValue && currentUserOption && !isSearchStringMatch(searchValue, getCurrentUserSearchTerms(currentUserOption).join(' '))) { - currentUserOption = undefined; - } + const currentUserOption = allPersonalDetailsOptions.find((personalDetailsOption) => personalDetailsOption.login === currentUserLogin); let userToInvite: ReportUtils.OptionData | null = null; if ( From 5c333f3261424341ce52edfbce8273d75ebecc05 Mon Sep 17 00:00:00 2001 From: Tomasz Misiukiewicz Date: Tue, 30 Jul 2024 10:10:32 +0200 Subject: [PATCH 5/7] migrate OptionListUtils search tests to filterOptions --- tests/unit/OptionsListUtilsTest.ts | 384 +++++++++++++++-------------- 1 file changed, 204 insertions(+), 180 deletions(-) diff --git a/tests/unit/OptionsListUtilsTest.ts b/tests/unit/OptionsListUtilsTest.ts index 79f19649f3373..34a0a9af73836 100644 --- a/tests/unit/OptionsListUtilsTest.ts +++ b/tests/unit/OptionsListUtilsTest.ts @@ -424,38 +424,12 @@ describe('OptionsListUtils', () => { it('getSearchOptions()', () => { // When we filter in the Search view without providing a searchValue - let results = OptionsListUtils.getSearchOptions(OPTIONS, '', [CONST.BETAS.ALL]); + const results = OptionsListUtils.getSearchOptions(OPTIONS, '', [CONST.BETAS.ALL]); // Then the 2 personalDetails that don't have reports should be returned expect(results.personalDetails.length).toBe(2); // Then all of the reports should be shown including the archived rooms. expect(results.recentReports.length).toBe(Object.values(OPTIONS.reports).length); - - // When we filter again but provide a searchValue - results = OptionsListUtils.getSearchOptions(OPTIONS, 'spider'); - - // Then only one option should be returned and it's the one matching the search value - expect(results.recentReports.length).toBe(1); - expect(results.recentReports[0].login).toBe('peterparker@expensify.com'); - - // When we filter again but provide a searchValue that should match multiple times - results = OptionsListUtils.getSearchOptions(OPTIONS, 'fantastic'); - - // Value with latest lastVisibleActionCreated should be at the top. - expect(results.recentReports.length).toBe(2); - expect(results.recentReports[0].text).toBe('Mister Fantastic'); - expect(results.recentReports[1].text).toBe('Mister Fantastic, Invisible Woman'); - - return waitForBatchedUpdates() - .then(() => Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, PERSONAL_DETAILS_WITH_PERIODS)) - .then(() => { - const OPTIONS_WITH_PERIODS = OptionsListUtils.createOptionList(PERSONAL_DETAILS_WITH_PERIODS, REPORTS); - // When we filter again but provide a searchValue that should match with periods - results = OptionsListUtils.getSearchOptions(OPTIONS_WITH_PERIODS, 'barry.allen@expensify.com'); - // Then we expect to have the personal detail with period filtered - expect(results.recentReports.length).toBe(1); - expect(results.recentReports[0].text).toBe('The Flash'); - }); }); it('getFilteredOptions()', () => { @@ -522,34 +496,6 @@ describe('OptionsListUtils', () => { // Then no personal detail options will be returned expect(results.personalDetails.length).toBe(0); - // When we provide a search value that does not match any personal details - results = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], 'magneto'); - - // Then no options will be returned - expect(results.personalDetails.length).toBe(0); - - // When we provide a search value that matches an email - results = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], 'peterparker@expensify.com'); - - // Then one recentReports will be returned and it will be the correct option - // personalDetails should be empty array - expect(results.recentReports.length).toBe(1); - expect(results.recentReports[0].text).toBe('Spider-Man'); - expect(results.personalDetails.length).toBe(0); - - // When we provide a search value that matches a partial display name or email - results = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], '.com'); - - // Then several options will be returned and they will be each have the search string in their email or name - // even though the currently logged in user matches they should not show. - // Should be ordered by lastVisibleActionCreated values. - expect(results.personalDetails.length).toBe(4); - expect(results.recentReports.length).toBe(5); - expect(results.personalDetails[0].login).toBe('natasharomanoff@expensify.com'); - expect(results.recentReports[0].text).toBe('Captain America'); - expect(results.recentReports[1].text).toBe('Mr Sinister'); - expect(results.recentReports[2].text).toBe('Black Panther'); - // Test for Concierge's existence in chat options results = OptionsListUtils.getFilteredOptions(OPTIONS_WITH_CONCIERGE.reports, OPTIONS_WITH_CONCIERGE.personalDetails); @@ -603,27 +549,6 @@ describe('OptionsListUtils', () => { const personalDetailsOverlapWithReports = results.personalDetails.every((personalDetailOption) => reportLogins.includes(personalDetailOption.login)); expect(personalDetailsOverlapWithReports).toBe(false); - // When we search for an option that is only in a personalDetail with no existing report - results = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], 'hulk'); - - // Then reports should return no results - expect(results.recentReports.length).toBe(0); - - // And personalDetails should show just one option and it will be the one we expect - expect(results.personalDetails.length).toBe(1); - expect(results.personalDetails[0].login).toBe('brucebanner@expensify.com'); - - // When we search for an option that matches things in both personalDetails and reports - results = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], '.com'); - - // Then all single participant reports that match will show up in the recentReports array, Recently used contact should be at the top - expect(results.recentReports.length).toBe(5); - expect(results.recentReports[0].text).toBe('Captain America'); - - // And logins with no single participant reports will show up in personalDetails - expect(results.personalDetails.length).toBe(4); - expect(results.personalDetails[0].login).toBe('natasharomanoff@expensify.com'); - // When we provide no selected options to getFilteredOptions() results = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], '', []); @@ -639,72 +564,6 @@ describe('OptionsListUtils', () => { expect(results.recentReports.every((option) => option.login !== 'peterparker@expensify.com')).toBe(true); expect(results.personalDetails.every((option) => option.login !== 'peterparker@expensify.com')).toBe(true); - // When we add a search term for which no options exist and the searchValue itself - // is not a potential email or phone - results = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], 'marc@expensify'); - - // Then we should have no options or personal details at all and also that there is no userToInvite - expect(results.recentReports.length).toBe(0); - expect(results.personalDetails.length).toBe(0); - expect(results.userToInvite).toBe(null); - - // When we add a search term for which no options exist and the searchValue itself - // is a potential email - results = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], 'marc@expensify.com'); - - // Then we should have no options or personal details at all but there should be a userToInvite - expect(results.recentReports.length).toBe(0); - expect(results.personalDetails.length).toBe(0); - expect(results.userToInvite).not.toBe(null); - - // When we add a search term with a period, with options for it that don't contain the period - results = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], 'peter.parker@expensify.com'); - - // Then we should have no options at all but there should be a userToInvite - expect(results.recentReports.length).toBe(0); - expect(results.userToInvite).not.toBe(null); - - // When we add a search term for which no options exist and the searchValue itself - // is a potential phone number without country code added - results = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], '5005550006'); - - // Then we should have no options or personal details at all but there should be a userToInvite and the login - // should have the country code included - expect(results.recentReports.length).toBe(0); - expect(results.personalDetails.length).toBe(0); - expect(results.userToInvite).not.toBe(null); - expect(results.userToInvite?.login).toBe('+15005550006'); - - // When we add a search term for which no options exist and the searchValue itself - // is a potential phone number with country code added - results = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], '+15005550006'); - - // Then we should have no options or personal details at all but there should be a userToInvite and the login - // should have the country code included - expect(results.recentReports.length).toBe(0); - expect(results.personalDetails.length).toBe(0); - expect(results.userToInvite).not.toBe(null); - expect(results.userToInvite?.login).toBe('+15005550006'); - - // When we add a search term for which no options exist and the searchValue itself - // is a potential phone number with special characters added - results = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], '+1 (800)324-3233'); - - // Then we should have no options or personal details at all but there should be a userToInvite and the login - // should have the country code included - expect(results.recentReports.length).toBe(0); - expect(results.personalDetails.length).toBe(0); - expect(results.userToInvite).not.toBe(null); - expect(results.userToInvite?.login).toBe('+18003243233'); - - // When we use a search term for contact number that contains alphabet characters - results = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], '998243aaaa'); - - // Then we shouldn't have any results or user to invite - expect(results.recentReports.length).toBe(0); - expect(results.personalDetails.length).toBe(0); - expect(results.userToInvite).toBe(null); - // Test Concierge's existence in new group options results = OptionsListUtils.getFilteredOptions(OPTIONS_WITH_CONCIERGE.reports, OPTIONS_WITH_CONCIERGE.personalDetails); @@ -757,18 +616,6 @@ describe('OptionsListUtils', () => { // Then we should expect all the recent reports to show but exclude the archived rooms expect(results.recentReports.length).toBe(Object.values(OPTIONS.reports).length - 1); - // When we pass a search value that doesn't match the group chat name - results = OptionsListUtils.getShareDestinationOptions(filteredReports, OPTIONS.personalDetails, [], 'mutants'); - - // Then we should expect no recent reports to show - expect(results.recentReports.length).toBe(0); - - // When we pass a search value that matches the group chat name - results = OptionsListUtils.getShareDestinationOptions(filteredReports, OPTIONS.personalDetails, [], 'Iron Man, Fantastic'); - - // Then we should expect the group chat to show along with the contacts matching the search - expect(results.recentReports.length).toBe(1); - // Filter current REPORTS_WITH_WORKSPACE_ROOMS as we do in the component, before getting share destination options const filteredReportsWithWorkspaceRooms = Object.values(OPTIONS_WITH_WORKSPACE_ROOM.reports).reduce((filtered, option) => { const report = option.item; @@ -784,42 +631,17 @@ describe('OptionsListUtils', () => { // Then we should expect the DMS, the group chats and the workspace room to show // We should expect all the recent reports to show, excluding the archived rooms expect(results.recentReports.length).toBe(Object.values(OPTIONS_WITH_WORKSPACE_ROOM.reports).length - 1); - - // When we search for a workspace room - results = OptionsListUtils.getShareDestinationOptions(filteredReportsWithWorkspaceRooms, OPTIONS.personalDetails, [], 'Avengers Room'); - - // Then we should expect only the workspace room to show - expect(results.recentReports.length).toBe(1); - - // When we search for a workspace room that doesn't exist - results = OptionsListUtils.getShareDestinationOptions(filteredReportsWithWorkspaceRooms, OPTIONS.personalDetails, [], 'Mutants Lair'); - - // Then we should expect no results to show - expect(results.recentReports.length).toBe(0); }); it('getMemberInviteOptions()', () => { // When we only pass personal details - let results = OptionsListUtils.getMemberInviteOptions(OPTIONS.personalDetails, [], ''); + const results = OptionsListUtils.getMemberInviteOptions(OPTIONS.personalDetails, [], ''); // We should expect personal details to be sorted alphabetically expect(results.personalDetails[0].text).toBe('Black Panther'); expect(results.personalDetails[1].text).toBe('Black Widow'); expect(results.personalDetails[2].text).toBe('Captain America'); expect(results.personalDetails[3].text).toBe('Invisible Woman'); - - // When we provide a search value that does not match any personal details - results = OptionsListUtils.getMemberInviteOptions(OPTIONS.personalDetails, [], 'magneto'); - - // Then no options will be returned - expect(results.personalDetails.length).toBe(0); - - // When we provide a search value that matches an email - results = OptionsListUtils.getMemberInviteOptions(OPTIONS.personalDetails, [], 'peterparker@expensify.com'); - - // Then one personal should be in personalDetails list - expect(results.personalDetails.length).toBe(1); - expect(results.personalDetails[0].text).toBe('Spider-Man'); }); it('getFilteredOptions() for categories', () => { @@ -2950,6 +2772,208 @@ describe('OptionsListUtils', () => { expect(filteredOptions.personalDetails.length).toBe(1); expect(filteredOptions.userToInvite).toBe(null); }); + + it('should not return any options if search value does not match any personal details (getMemberInviteOptions)', () => { + const options = OptionsListUtils.getMemberInviteOptions(OPTIONS.personalDetails, [], ''); + const filteredOptions = OptionsListUtils.filterOptions(options, 'magneto'); + expect(filteredOptions.personalDetails.length).toBe(0); + }); + + it('should return one personal detail if search value matches an email (getMemberInviteOptions)', () => { + const options = OptionsListUtils.getMemberInviteOptions(OPTIONS.personalDetails, [], ''); + const filteredOptions = OptionsListUtils.filterOptions(options, 'peterparker@expensify.com'); + + expect(filteredOptions.personalDetails.length).toBe(1); + expect(filteredOptions.personalDetails[0].text).toBe('Spider-Man'); + }); + + it('should not show any recent reports if a search value does not match the group chat name (getShareDestinationsOptions)', () => { + // Filter current REPORTS as we do in the component, before getting share destination options + const filteredReports = Object.values(OPTIONS.reports).reduce((filtered, option) => { + const report = option.item; + if (ReportUtils.canUserPerformWriteAction(report) && ReportUtils.canCreateTaskInReport(report) && !ReportUtils.isCanceledTaskReport(report)) { + filtered.push(option); + } + return filtered; + }, []); + const options = OptionsListUtils.getShareDestinationOptions(filteredReports, OPTIONS.personalDetails, [], ''); + const filteredOptions = OptionsListUtils.filterOptions(options, 'mutants'); + + expect(filteredOptions.recentReports.length).toBe(0); + }); + + it('should return a workspace room when we search for a workspace room(getShareDestinationsOptions)', () => { + const filteredReportsWithWorkspaceRooms = Object.values(OPTIONS_WITH_WORKSPACE_ROOM.reports).reduce((filtered, option) => { + const report = option.item; + // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing + if (ReportUtils.canUserPerformWriteAction(report) || ReportUtils.isExpensifyOnlyParticipantInReport(report)) { + filtered.push(option); + } + return filtered; + }, []); + + const options = OptionsListUtils.getShareDestinationOptions(filteredReportsWithWorkspaceRooms, OPTIONS.personalDetails, [], ''); + const filteredOptions = OptionsListUtils.filterOptions(options, 'Avengers Room'); + + expect(filteredOptions.recentReports.length).toBe(1); + }); + + it('should not show any results if searching for a non-existing workspace room(getShareDestinationOptions)', () => { + const filteredReportsWithWorkspaceRooms = Object.values(OPTIONS_WITH_WORKSPACE_ROOM.reports).reduce((filtered, option) => { + const report = option.item; + // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing + if (ReportUtils.canUserPerformWriteAction(report) || ReportUtils.isExpensifyOnlyParticipantInReport(report)) { + filtered.push(option); + } + return filtered; + }, []); + + const options = OptionsListUtils.getShareDestinationOptions(filteredReportsWithWorkspaceRooms, OPTIONS.personalDetails, [], ''); + const filteredOptions = OptionsListUtils.filterOptions(options, 'Mutants Lair'); + + expect(filteredOptions.recentReports.length).toBe(0); + }); + + it('should show the option from personal details when searching for personal detail with no existing report (getFilteredOptions)', () => { + const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + const filteredOptions = OptionsListUtils.filterOptions(options, 'hulk'); + + expect(filteredOptions.recentReports.length).toBe(0); + + expect(filteredOptions.personalDetails.length).toBe(1); + expect(filteredOptions.personalDetails[0].login).toBe('brucebanner@expensify.com'); + }); + + it('should return all matching reports and personal details (getFilteredOptions)', () => { + const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + const filteredOptions = OptionsListUtils.filterOptions(options, '.com'); + + expect(filteredOptions.recentReports.length).toBe(5); + expect(filteredOptions.recentReports[0].text).toBe('Captain America'); + + expect(filteredOptions.personalDetails.length).toBe(4); + expect(filteredOptions.personalDetails[0].login).toBe('natasharomanoff@expensify.com'); + }); + + it('should not return any options or user to invite if there are no search results and the string does not match a potential email or phone (getFilteredOptions)', () => { + const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + const filteredOptions = OptionsListUtils.filterOptions(options, 'marc@expensify'); + + expect(filteredOptions.recentReports.length).toBe(0); + expect(filteredOptions.personalDetails.length).toBe(0); + expect(filteredOptions.userToInvite).toBe(null); + }); + + it('should not return any options but should return an user to invite if no matching options exist and the search value is a potential email (getFilteredOptions)', () => { + const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + const filteredOptions = OptionsListUtils.filterOptions(options, 'marc@expensify.com'); + + expect(filteredOptions.recentReports.length).toBe(0); + expect(filteredOptions.personalDetails.length).toBe(0); + expect(filteredOptions.userToInvite).not.toBe(null); + }); + + it('should return user to invite when search term has a period with options for it that do not contain the period (getFilteredOptions)', () => { + const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + const filteredOptions = OptionsListUtils.filterOptions(options, 'peter.parker@expensify.com'); + + expect(filteredOptions.recentReports.length).toBe(0); + expect(filteredOptions.userToInvite).not.toBe(null); + }); + + it('should not return options but should return an user to invite if no matching options exist and the search value is a potential phone number (getFilteredOptions)', () => { + const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + const filteredOptions = OptionsListUtils.filterOptions(options, '5005550006'); + + expect(filteredOptions.recentReports.length).toBe(0); + expect(filteredOptions.personalDetails.length).toBe(0); + expect(filteredOptions.userToInvite).not.toBe(null); + expect(filteredOptions.userToInvite?.login).toBe('+15005550006'); + }); + + it('should not return options but should return an user to invite if no matching options exist and the search value is a potential phone number with country code added (getFilteredOptions)', () => { + const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + const filteredOptions = OptionsListUtils.filterOptions(options, '+15005550006'); + + expect(filteredOptions.recentReports.length).toBe(0); + expect(filteredOptions.personalDetails.length).toBe(0); + expect(filteredOptions.userToInvite).not.toBe(null); + expect(filteredOptions.userToInvite?.login).toBe('+15005550006'); + }); + + it('should not return options but should return an user to invite if no matching options exist and the search value is a potential phone number with special characters added (getFilteredOptions)', () => { + const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + const filteredOptions = OptionsListUtils.filterOptions(options, '+1 (800)324-3233'); + + expect(filteredOptions.recentReports.length).toBe(0); + expect(filteredOptions.personalDetails.length).toBe(0); + expect(filteredOptions.userToInvite).not.toBe(null); + expect(filteredOptions.userToInvite?.login).toBe('+18003243233'); + }); + + it('should not return any options or user to invite if contact number contains alphabet characters (getFilteredOptions)', () => { + const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + const filteredOptions = OptionsListUtils.filterOptions(options, '998243aaaa'); + + expect(filteredOptions.recentReports.length).toBe(0); + expect(filteredOptions.personalDetails.length).toBe(0); + expect(filteredOptions.userToInvite).toBe(null); + }); + + it('should not return any options if search value does not match any personal details (getFilteredOptions)', () => { + const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + const filteredOptions = OptionsListUtils.filterOptions(options, 'magneto'); + + expect(filteredOptions.personalDetails.length).toBe(0); + }); + + it('should return one recent report and no personal details if a search value provides an email (getFilteredOptions)', () => { + const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + const filteredOptions = OptionsListUtils.filterOptions(options, 'peterparker@expensify.com', {sortByReportTypeInSearch: true}); + expect(filteredOptions.recentReports.length).toBe(1); + expect(filteredOptions.recentReports[0].text).toBe('Spider-Man'); + expect(filteredOptions.personalDetails.length).toBe(0); + }); + + it('should return all matching reports and personal details (getFilteredOptions)', () => { + const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + const filteredOptions = OptionsListUtils.filterOptions(options, '.com'); + + expect(filteredOptions.personalDetails.length).toBe(4); + expect(filteredOptions.recentReports.length).toBe(5); + expect(filteredOptions.personalDetails[0].login).toBe('natasharomanoff@expensify.com'); + expect(filteredOptions.recentReports[0].text).toBe('Captain America'); + expect(filteredOptions.recentReports[1].text).toBe('Mr Sinister'); + expect(filteredOptions.recentReports[2].text).toBe('Black Panther'); + }); + + it('should return matching option when searching (getSearchOptions)', () => { + const options = OptionsListUtils.getSearchOptions(OPTIONS, ''); + const filteredOptions = OptionsListUtils.filterOptions(options, 'spider'); + + expect(filteredOptions.recentReports.length).toBe(1); + expect(filteredOptions.recentReports[0].text).toBe('Spider-Man'); + }); + + it('should return latest lastVisibleActionCreated item on top when search value matches multiple items (getSearchOptions)', () => { + const options = OptionsListUtils.getSearchOptions(OPTIONS, ''); + const filteredOptions = OptionsListUtils.filterOptions(options, 'fantastic'); + + expect(filteredOptions.recentReports.length).toBe(2); + expect(filteredOptions.recentReports[0].text).toBe('Mister Fantastic'); + expect(filteredOptions.recentReports[1].text).toBe('Mister Fantastic, Invisible Woman'); + + return waitForBatchedUpdates() + .then(() => Onyx.set(ONYXKEYS.PERSONAL_DETAILS_LIST, PERSONAL_DETAILS_WITH_PERIODS)) + .then(() => { + const OPTIONS_WITH_PERIODS = OptionsListUtils.createOptionList(PERSONAL_DETAILS_WITH_PERIODS, REPORTS); + const results = OptionsListUtils.getSearchOptions(OPTIONS_WITH_PERIODS, ''); + const filteredResults = OptionsListUtils.filterOptions(results, 'barry.allen@expensify.com', {sortByReportTypeInSearch: true}); + + expect(filteredResults.recentReports.length).toBe(1); + expect(filteredResults.recentReports[0].text).toBe('The Flash'); + }); + }); }); describe('canCreateOptimisticPersonalDetailOption', () => { From b4997e2f07312194bb1e698392d30d35e1204bd2 Mon Sep 17 00:00:00 2001 From: Tomasz Misiukiewicz Date: Wed, 31 Jul 2024 15:47:10 +0200 Subject: [PATCH 6/7] link issue for email regex --- src/CONST.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/CONST.ts b/src/CONST.ts index 1199b652cf90b..5e72770392940 100755 --- a/src/CONST.ts +++ b/src/CONST.ts @@ -1122,6 +1122,7 @@ const CONST = { // The regex below is used to remove dots only from the local part of the user email (local-part@domain) // so when we are using search, we can match emails that have dots without explicitly writing the dots (e.g: fistlast@domain will match first.last@domain) + // More info https://github.com/Expensify/App/issues/8007 EMAIL_SEARCH_REGEX: /\.(?=[^\s@]*@)/g, VALIDATE_FOR_LEADINGSPACES_HTML_TAG_REGEX: /<([\s]+.+[\s]*)>/g, From 23fb1e97762f9765e046e284f143dd20fee5ce41 Mon Sep 17 00:00:00 2001 From: Tomasz Misiukiewicz Date: Mon, 5 Aug 2024 17:25:25 +0200 Subject: [PATCH 7/7] move normalizing search term out of loops --- src/libs/OptionsListUtils.ts | 3 ++- src/pages/NewChatPage.tsx | 6 +++--- src/pages/iou/request/MoneyRequestParticipantsSelector.tsx | 4 +++- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index 0373faf3d0d68..0ddcd5fe0be70 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -2366,11 +2366,12 @@ function formatSectionsFromSearchTerm( }; } + const cleanSearchTerm = searchTerm.trim().toLowerCase(); // If you select a new user you don't have a contact for, they won't get returned as part of a recent report or personal details // This will add them to the list of options, deduping them if they already exist in the other lists const selectedParticipantsWithoutDetails = selectedOptions.filter((participant) => { const accountID = participant.accountID ?? null; - const isPartOfSearchTerm = getPersonalDetailSearchTerms(participant).join(' ').toLowerCase().includes(searchTerm.trim().toLowerCase()); + const isPartOfSearchTerm = getPersonalDetailSearchTerms(participant).join(' ').toLowerCase().includes(cleanSearchTerm); const isReportInRecentReports = filteredRecentReports.some((report) => report.accountID === accountID); const isReportInPersonalDetails = filteredPersonalDetails.some((personalDetail) => personalDetail.accountID === accountID); return isPartOfSearchTerm && !isReportInRecentReports && !isReportInPersonalDetails; diff --git a/src/pages/NewChatPage.tsx b/src/pages/NewChatPage.tsx index 90cfbf50588c1..62bc6bf1c6c2d 100755 --- a/src/pages/NewChatPage.tsx +++ b/src/pages/NewChatPage.tsx @@ -85,15 +85,15 @@ function useOptions({isGroupChat}: NewChatPageProps) { return filteredOptions; }, [debouncedSearchTerm, defaultOptions, isGroupChat, selectedOptions]); - + const cleanSearchTerm = useMemo(() => debouncedSearchTerm.trim().toLowerCase(), [debouncedSearchTerm]); const headerMessage = useMemo(() => { return OptionsListUtils.getHeaderMessage( options.personalDetails.length + options.recentReports.length !== 0, !!options.userToInvite, debouncedSearchTerm.trim(), - selectedOptions.some((participant) => OptionsListUtils.getPersonalDetailSearchTerms(participant).join(' ').toLowerCase?.().includes(debouncedSearchTerm.trim().toLowerCase())), + selectedOptions.some((participant) => OptionsListUtils.getPersonalDetailSearchTerms(participant).join(' ').toLowerCase?.().includes(cleanSearchTerm)), ); - }, [debouncedSearchTerm, options.personalDetails.length, options.recentReports.length, options.userToInvite, selectedOptions]); + }, [cleanSearchTerm, debouncedSearchTerm, options.personalDetails.length, options.recentReports.length, options.userToInvite, selectedOptions]); useEffect(() => { if (!debouncedSearchTerm.length) { diff --git a/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx b/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx index 00ed1dd4ad331..96af09f5ce03e 100644 --- a/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx +++ b/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx @@ -68,6 +68,7 @@ function MoneyRequestParticipantsSelector({participants = CONST.EMPTY_ARRAY, onF shouldInitialize: didScreenTransitionEnd, }); + const cleanSearchTerm = useMemo(() => debouncedSearchTerm.trim().toLowerCase(), [debouncedSearchTerm]); const offlineMessage: string = isOffline ? `${translate('common.youAppearToBeOffline')} ${translate('search.resultsAreLimited')}` : ''; const isIOUSplit = iouType === CONST.IOU.TYPE.SPLIT; @@ -216,7 +217,7 @@ function MoneyRequestParticipantsSelector({participants = CONST.EMPTY_ARRAY, onF (chatOptions.personalDetails ?? []).length + (chatOptions.recentReports ?? []).length !== 0, !!chatOptions?.userToInvite, debouncedSearchTerm.trim(), - participants.some((participant) => OptionsListUtils.getPersonalDetailSearchTerms(participant).join(' ').toLowerCase().includes(debouncedSearchTerm.trim().toLowerCase())), + participants.some((participant) => OptionsListUtils.getPersonalDetailSearchTerms(participant).join(' ').toLowerCase().includes(cleanSearchTerm)), ); return [newSections, headerMessage]; @@ -230,6 +231,7 @@ function MoneyRequestParticipantsSelector({participants = CONST.EMPTY_ARRAY, onF chatOptions.userToInvite, personalDetails, translate, + cleanSearchTerm, ]); /**