From e68f0a846db9dd7528a18669449e8e439b18d26d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Mon, 25 Nov 2024 20:56:00 +0100 Subject: [PATCH 01/25] wip, current problem: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current problem is with the max reports number. I currently only splice in the order function. but in the filter function we filter the personal details based on the recent reports. if the recent reports aren't limited in the filter function then then we will have fewer personal details (as more reports have been loaded). however i can't limit the recent reports in filter yet as they aren't ordered yet… we might need to split up getOptions for reports and pD actually --- .../Search/SearchFiltersChatsSelector.tsx | 2 +- .../SearchFiltersParticipantsSelector.tsx | 3 +- .../Search/SearchRouter/SearchRouter.tsx | 2 +- src/libs/OptionsListUtils.ts | 150 ++++++++++-------- src/pages/InviteReportParticipantsPage.tsx | 2 +- src/pages/NewChatPage.tsx | 2 +- src/pages/RoomInvitePage.tsx | 2 +- .../request/MoneyRequestAttendeeSelector.tsx | 4 +- .../MoneyRequestParticipantsSelector.tsx | 3 +- .../ShareLogList/BaseShareLogList.tsx | 2 +- .../Security/AddDelegate/AddDelegatePage.tsx | 2 +- src/pages/tasks/TaskAssigneeSelectorModal.tsx | 2 +- .../TaskShareDestinationSelectorModal.tsx | 2 +- src/pages/workspace/WorkspaceInvitePage.tsx | 2 +- tests/perf-test/OptionsListUtils.perf-test.ts | 2 +- tests/unit/OptionsListUtilsTest.ts | 136 +++++++--------- 16 files changed, 155 insertions(+), 163 deletions(-) diff --git a/src/components/Search/SearchFiltersChatsSelector.tsx b/src/components/Search/SearchFiltersChatsSelector.tsx index 1cdbbc341c29..c1297480b3c7 100644 --- a/src/components/Search/SearchFiltersChatsSelector.tsx +++ b/src/components/Search/SearchFiltersChatsSelector.tsx @@ -66,7 +66,7 @@ function SearchFiltersChatsSelector({initialReportIDs, onFiltersUpdate, isScreen }, [areOptionsInitialized, isScreenTransitionEnd, options]); const chatOptions = useMemo(() => { - return OptionsListUtils.filterOptions(defaultOptions, cleanSearchTerm, { + return OptionsListUtils.filterAndOrderOptions(defaultOptions, cleanSearchTerm, { selectedOptions, excludeLogins: CONST.EXPENSIFY_EMAILS, maxRecentReportsToShow: 0, diff --git a/src/components/Search/SearchFiltersParticipantsSelector.tsx b/src/components/Search/SearchFiltersParticipantsSelector.tsx index fc692c01f824..c84f71a34b6e 100644 --- a/src/components/Search/SearchFiltersParticipantsSelector.tsx +++ b/src/components/Search/SearchFiltersParticipantsSelector.tsx @@ -62,13 +62,12 @@ function SearchFiltersParticipantsSelector({initialAccountIDs, onFiltersUpdate}: { selectedOptions, excludeLogins: CONST.EXPENSIFY_EMAILS, - maxRecentReportsToShow: 0, }, ); }, [areOptionsInitialized, options.personalDetails, options.reports, selectedOptions]); const chatOptions = useMemo(() => { - return OptionsListUtils.filterOptions(defaultOptions, cleanSearchTerm, { + return OptionsListUtils.filterAndOrderOptions(defaultOptions, cleanSearchTerm, { selectedOptions, excludeLogins: CONST.EXPENSIFY_EMAILS, maxRecentReportsToShow: CONST.IOU.MAX_RECENT_REPORTS_TO_SHOW, diff --git a/src/components/Search/SearchRouter/SearchRouter.tsx b/src/components/Search/SearchRouter/SearchRouter.tsx index 3ee0874ff4fe..02b81717a28b 100644 --- a/src/components/Search/SearchRouter/SearchRouter.tsx +++ b/src/components/Search/SearchRouter/SearchRouter.tsx @@ -90,7 +90,7 @@ function SearchRouter({onRouterClose, shouldHideInputCaret}: SearchRouterProps) } Timing.start(CONST.TIMING.SEARCH_FILTER_OPTIONS); - const newOptions = OptionsListUtils.filterOptions(searchOptions, debouncedInputValue, {sortByReportTypeInSearch: true, preferChatroomsOverThreads: true}); + const newOptions = OptionsListUtils.filterAndOrderOptions(searchOptions, debouncedInputValue, {sortByReportTypeInSearch: true, preferChatroomsOverThreads: true}); Timing.end(CONST.TIMING.SEARCH_FILTER_OPTIONS); return { diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index 9e5ec3b18729..7db4a49e348f 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -1,7 +1,6 @@ /* eslint-disable no-continue */ import {Str} from 'expensify-common'; import lodashOrderBy from 'lodash/orderBy'; -import lodashSortBy from 'lodash/sortBy'; import Onyx from 'react-native-onyx'; import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; import type {SetNonNullable} from 'type-fest'; @@ -93,13 +92,11 @@ type GetOptionsConfig = { reportActions?: ReportActions; betas?: OnyxEntry; selectedOptions?: Option[]; - maxRecentReportsToShow?: number; excludeLogins?: string[]; includeMultipleParticipantReports?: boolean; includeRecentReports?: boolean; includeSelfDM?: boolean; showChatPreviewLine?: boolean; - sortPersonalDetailsByAlphaAsc?: boolean; forcePolicyNamePreview?: boolean; includeOwnedWorkspaceChats?: boolean; includeThreads?: boolean; @@ -153,12 +150,16 @@ type Options = { type PreviewConfig = {showChatPreviewLine?: boolean; forcePolicyNamePreview?: boolean; showPersonalDetails?: boolean}; -type FilterOptionsConfig = Pick & { +type FilterOptionsConfig = Pick & { + /* When sortByReportTypeInSearch flag is true, recentReports will include the personalDetails options as well. */ + sortByReportTypeInSearch?: boolean; + maxRecentReportsToShow?: number; +}; + +type OrderOptionsConfig = { preferChatroomsOverThreads?: boolean; preferPolicyExpenseChat?: boolean; preferRecentExpenseReports?: boolean; - /* When sortByReportTypeInSearch flag is true, recentReports will include the personalDetails options as well. */ - sortByReportTypeInSearch?: boolean; }; /** @@ -917,21 +918,32 @@ function createOptionFromReport(report: Report, personalDetails: OnyxEntry personalDetail.text?.toLowerCase()], 'asc'); +} + /** - * Options need to be sorted in the specific order + * Report Options need to be sorted in the specific order * @param options - list of options to be sorted * @param searchValue - search string * @returns a sorted list of options */ -function orderOptions( +function orderReportOptions( options: ReportUtils.OptionData[], searchValue: string | undefined, - {preferChatroomsOverThreads = false, preferPolicyExpenseChat = false, preferRecentExpenseReports = false} = {}, + {preferChatroomsOverThreads = false, preferPolicyExpenseChat = false, preferRecentExpenseReports = false}: OrderOptionsConfig = {}, ) { return lodashOrderBy( options, [ + // Sort descending by lastVisibleActionCreated + (option) => option?.lastVisibleActionCreated ?? 0, (option) => { + // TODO: what the fuck this is at the bottom already, is this even needed? i don't think so … + // if (option.private_isArchived) { + // return CONST.DATE.UNIX_EPOCH; // Keep archived at bottom + // } if (option.isPolicyExpenseChat && preferPolicyExpenseChat && option.policyID === activePolicyID) { return 0; } @@ -965,10 +977,20 @@ function orderOptions( preferRecentExpenseReports ? (option) => option?.lastIOUCreationDate ?? '' : '', preferRecentExpenseReports ? (option) => option?.isPolicyExpenseChat : 0, ], - ['asc', 'desc', 'desc'], + ['desc', 'asc', 'desc', 'desc'], ); } +function orderOptions(options: Pick, searchValue?: string, config?: OrderOptionsConfig) { + const orderedReportOptions = orderReportOptions(options.recentReports, searchValue, config); + const orderedPersonalDetailsOptions = orderPersonalDetailsOptions(options.personalDetails); + + return { + recentReports: orderedReportOptions, + personalDetails: orderedPersonalDetailsOptions, + }; +} + function canCreateOptimisticPersonalDetailOption({ recentReportOptions, personalDetailsOptions, @@ -1042,6 +1064,7 @@ function getUserToInviteOption({ return userToInvite; } +// TODO: rename /** * filter options based on specific conditions */ @@ -1051,12 +1074,10 @@ function getOptions( reportActions = {}, betas = [], selectedOptions = [], - maxRecentReportsToShow = 0, excludeLogins = [], includeMultipleParticipantReports = false, includeRecentReports = true, showChatPreviewLine = false, - sortPersonalDetailsByAlphaAsc = true, forcePolicyNamePreview = false, includeOwnedWorkspaceChats = false, includeThreads = false, @@ -1096,25 +1117,11 @@ function getOptions( }); }); - // Sorting the reports works like this: - // - Order everything by the last message timestamp (descending) - // - When searching, self DM is put at the top - // - All archived reports should remain at the bottom - const orderedReportOptions = lodashSortBy(filteredReportOptions, (option) => { - const report = option.item; - if (option.private_isArchived) { - return CONST.DATE.UNIX_EPOCH; - } - - return report?.lastVisibleActionCreated; - }); - orderedReportOptions.reverse(); - - const allReportOptions = orderedReportOptions.filter((option) => { + const allReportOptions = filteredReportOptions.filter((option) => { const report = option.item; if (!report) { - return; + return false; } const isThread = option.isThread; @@ -1126,51 +1133,46 @@ function getOptions( const accountIDs = ReportUtils.getParticipantsAccountIDsForDisplay(report); if (isPolicyExpenseChat && report.isOwnPolicyExpenseChat && !includeOwnedWorkspaceChats) { - return; + return false; } // When passing includeP2P false we are trying to hide features from users that are not ready for P2P and limited to workspace chats only. if (!includeP2P && !isPolicyExpenseChat) { - return; + return false; } if (isSelfDM && !includeSelfDM) { - return; + return false; } if (isThread && !includeThreads) { - return; + return false; } if (isTaskReport && !includeTasks) { - return; + return false; } if (isMoneyRequestReport && !includeMoneyRequests) { - return; + return false; } // In case user needs to add credit bank account, don't allow them to submit an expense from the workspace. if (includeOwnedWorkspaceChats && ReportUtils.hasIOUWaitingOnCurrentUserBankAccount(report)) { - return; + return false; } if ((!accountIDs || accountIDs.length === 0) && !isChatRoom) { - return; + return false; } - return option; + return true; }); const havingLoginPersonalDetails = includeP2P ? options.personalDetails.filter((detail) => !!detail?.login && !!detail.accountID && !detail?.isOptimisticPersonalDetail && (includeDomainEmail || !Str.isDomainEmail(detail.login))) : []; - let allPersonalDetailsOptions = havingLoginPersonalDetails; - - if (sortPersonalDetailsByAlphaAsc) { - // PersonalDetails should be ordered Alphabetically by default - https://github.com/Expensify/App/issues/8220#issuecomment-1104009435 - allPersonalDetailsOptions = lodashOrderBy(allPersonalDetailsOptions, [(personalDetail) => personalDetail.text?.toLowerCase()], 'asc'); - } + const allPersonalDetailsOptions = havingLoginPersonalDetails; const optionsToExclude: Option[] = [{login: CONST.EMAIL.NOTIFICATIONS}]; @@ -1198,11 +1200,6 @@ function getOptions( */ reportOption.alternateText = getAlternateText(reportOption, {showChatPreviewLine, forcePolicyNamePreview}); - // Stop adding options to the recentReports array when we reach the maxRecentReportsToShow value - if (recentReportOptions.length > 0 && recentReportOptions.length === maxRecentReportsToShow) { - break; - } - // Skip notifications@expensify.com if (reportOption.login === CONST.EMAIL.NOTIFICATIONS) { continue; @@ -1320,7 +1317,6 @@ function getSearchOptions(options: OptionList, betas: Beta[] = [], isUsedInChatF const optionList = getOptions(options, { betas, includeMultipleParticipantReports: true, - maxRecentReportsToShow: 0, // Unlimited showChatPreviewLine: isUsedInChatFinder, includeP2P: true, forcePolicyNamePreview: true, @@ -1395,7 +1391,6 @@ function getAttendeeOptions( includeP2P, canInviteUser, includeSelectedOptions: false, - maxRecentReportsToShow: 0, includeSelfDM: false, includeInvoiceRooms, action, @@ -1422,7 +1417,6 @@ function getShareDestinationOptions( { betas, selectedOptions, - maxRecentReportsToShow: 0, // Unlimited includeMultipleParticipantReports: true, showChatPreviewLine: true, forcePolicyNamePreview: true, @@ -1474,17 +1468,23 @@ function getMemberInviteOptions( reports: Array> = [], includeRecentReports = false, ): Options { - return getOptions( + const options = getOptions( {reports, personalDetails}, { betas, includeP2P: true, excludeLogins, - sortPersonalDetailsByAlphaAsc: true, includeSelectedOptions, includeRecentReports, }, ); + + const orderedOptions = orderOptions(options); + return { + ...options, + personalDetails: orderedOptions.personalDetails, + recentReports: orderedOptions.recentReports, + }; } /** @@ -1621,15 +1621,7 @@ function filteredPersonalDetailsOfRecentReports(recentReports: ReportUtils.Optio * Filters options based on the search input value */ function filterOptions(options: Options, searchInputValue: string, config?: FilterOptionsConfig): Options { - const { - sortByReportTypeInSearch = false, - canInviteUser = true, - maxRecentReportsToShow = 0, - excludeLogins = [], - preferChatroomsOverThreads = false, - preferPolicyExpenseChat = false, - preferRecentExpenseReports = false, - } = config ?? {}; + const {sortByReportTypeInSearch = false, canInviteUser = true, maxRecentReportsToShow = 0, excludeLogins = []} = config ?? {}; if (searchInputValue.trim() === '' && maxRecentReportsToShow > 0) { const recentReports = options.recentReports.slice(0, maxRecentReportsToShow); const personalDetails = filteredPersonalDetailsOfRecentReports(recentReports, options.personalDetails); @@ -1689,7 +1681,9 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt const {recentReports, personalDetails} = matchResults; + console.log('pre', {sortByReportTypeInSearch, pD: personalDetails.length, recentReports: recentReports.length}); const personalDetailsWithoutDMs = filteredPersonalDetailsOfRecentReports(recentReports, personalDetails); + console.log('post', personalDetailsWithoutDMs.length); let filteredPersonalDetails: ReportUtils.OptionData[] = personalDetailsWithoutDMs; let filteredRecentReports: ReportUtils.OptionData[] = recentReports; @@ -1709,19 +1703,36 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt } } - if (maxRecentReportsToShow > 0 && recentReports.length > maxRecentReportsToShow) { - recentReports.splice(maxRecentReportsToShow); - } - - const sortedRecentReports = orderOptions(filteredRecentReports, searchValue, {preferChatroomsOverThreads, preferPolicyExpenseChat, preferRecentExpenseReports}); + console.log('wat', filteredPersonalDetails.length); return { personalDetails: filteredPersonalDetails, - recentReports: sortedRecentReports, + recentReports: filteredRecentReports, userToInvite, currentUserOption: matchResults.currentUserOption, }; } +type FilterAndOrderConfig = FilterOptionsConfig & OrderOptionsConfig; +function filterAndOrderOptions(options: Options, searchInputValue: string, config?: FilterAndOrderConfig): Options { + const filteredOptions = filterOptions(options, searchInputValue, config); + console.log('after filterOptions', { + reports: filteredOptions.recentReports.length, + personalDetails: filteredOptions.personalDetails.length, + }); + const {personalDetails, recentReports} = orderOptions(filteredOptions, searchInputValue, config); + + let orderedReports = recentReports; + if (config?.maxRecentReportsToShow) { + orderedReports = orderedReports.slice(0, config.maxRecentReportsToShow); + } + + return { + ...filteredOptions, + personalDetails, + recentReports: orderedReports, + }; +} + function sortAlphabetically>, TKey extends keyof T>(items: T[], key: TKey): T[] { return items.sort((a, b) => (a[key] ?? '').toLowerCase().localeCompare((b[key] ?? '').toLowerCase())); } @@ -1769,7 +1780,10 @@ export { getShareLogOptions, filterOptions, filteredPersonalDetailsOfRecentReports, + orderReportOptions, + orderPersonalDetailsOptions, orderOptions, + filterAndOrderOptions, createOptionList, createOptionFromReport, getReportOption, diff --git a/src/pages/InviteReportParticipantsPage.tsx b/src/pages/InviteReportParticipantsPage.tsx index 2b2f7bbe25b8..d8c5f8c7585b 100644 --- a/src/pages/InviteReportParticipantsPage.tsx +++ b/src/pages/InviteReportParticipantsPage.tsx @@ -70,7 +70,7 @@ function InviteReportParticipantsPage({betas, report, didScreenTransitionEnd}: I }, [areOptionsInitialized, betas, excludedUsers, options.personalDetails, options.reports]); const inviteOptions = useMemo( - () => OptionsListUtils.filterOptions(defaultOptions, debouncedSearchTerm, {excludeLogins: excludedUsers}), + () => OptionsListUtils.filterAndOrderOptions(defaultOptions, debouncedSearchTerm, {excludeLogins: excludedUsers}), [debouncedSearchTerm, defaultOptions, excludedUsers], ); diff --git a/src/pages/NewChatPage.tsx b/src/pages/NewChatPage.tsx index 286dc75cb4f3..2a19cd031806 100755 --- a/src/pages/NewChatPage.tsx +++ b/src/pages/NewChatPage.tsx @@ -64,7 +64,7 @@ function useOptions() { }, [betas, listOptions.personalDetails, listOptions.reports, selectedOptions]); const options = useMemo(() => { - const filteredOptions = OptionsListUtils.filterOptions(defaultOptions, debouncedSearchTerm, { + const filteredOptions = OptionsListUtils.filterAndOrderOptions(defaultOptions, debouncedSearchTerm, { selectedOptions, maxRecentReportsToShow: CONST.IOU.MAX_RECENT_REPORTS_TO_SHOW, }); diff --git a/src/pages/RoomInvitePage.tsx b/src/pages/RoomInvitePage.tsx index 88e84809224a..feec7175cc8b 100644 --- a/src/pages/RoomInvitePage.tsx +++ b/src/pages/RoomInvitePage.tsx @@ -102,7 +102,7 @@ function RoomInvitePage({ if (debouncedSearchTerm.trim() === '') { return defaultOptions; } - const filteredOptions = OptionsListUtils.filterOptions(defaultOptions, debouncedSearchTerm, {excludeLogins: excludedUsers}); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(defaultOptions, debouncedSearchTerm, {excludeLogins: excludedUsers}); return filteredOptions; }, [debouncedSearchTerm, defaultOptions, excludedUsers]); diff --git a/src/pages/iou/request/MoneyRequestAttendeeSelector.tsx b/src/pages/iou/request/MoneyRequestAttendeeSelector.tsx index e27be90ce7fc..a94fd2e461fd 100644 --- a/src/pages/iou/request/MoneyRequestAttendeeSelector.tsx +++ b/src/pages/iou/request/MoneyRequestAttendeeSelector.tsx @@ -86,7 +86,7 @@ function MoneyRequestAttendeeSelector({attendees = [], onFinish, onAttendeesAdde action, ); if (isPaidGroupPolicy) { - optionList.recentReports = OptionsListUtils.orderOptions(optionList.recentReports, searchTerm, { + optionList.recentReports = OptionsListUtils.orderReportOptions(optionList.recentReports, searchTerm, { preferChatroomsOverThreads: true, preferPolicyExpenseChat: !!action, preferRecentExpenseReports: action === CONST.IOU.ACTION.CREATE, @@ -122,7 +122,7 @@ function MoneyRequestAttendeeSelector({attendees = [], onFinish, onAttendeesAdde headerMessage: '', }; } - const newOptions = OptionsListUtils.filterOptions(defaultOptions, debouncedSearchTerm, { + const newOptions = OptionsListUtils.filterAndOrderOptions(defaultOptions, debouncedSearchTerm, { excludeLogins: CONST.EXPENSIFY_EMAILS, maxRecentReportsToShow: CONST.IOU.MAX_RECENT_REPORTS_TO_SHOW, preferPolicyExpenseChat: isPaidGroupPolicy, diff --git a/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx b/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx index 26b8e3373767..70f1abdd3e44 100644 --- a/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx +++ b/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx @@ -124,7 +124,6 @@ function MoneyRequestParticipantsSelector({ canInviteUser: !isCategorizeOrShareAction, includeInvoiceRooms: iouType === CONST.IOU.TYPE.INVOICE, action, - maxRecentReportsToShow: 0, }, ); @@ -142,7 +141,7 @@ function MoneyRequestParticipantsSelector({ }; } - const newOptions = OptionsListUtils.filterOptions(defaultOptions, debouncedSearchTerm, { + const newOptions = OptionsListUtils.filterAndOrderOptions(defaultOptions, debouncedSearchTerm, { // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing canInviteUser: !isCategorizeOrShareAction, selectedOptions: participants as Participant[], diff --git a/src/pages/settings/AboutPage/ShareLogList/BaseShareLogList.tsx b/src/pages/settings/AboutPage/ShareLogList/BaseShareLogList.tsx index 2b79d441e686..47b418179049 100644 --- a/src/pages/settings/AboutPage/ShareLogList/BaseShareLogList.tsx +++ b/src/pages/settings/AboutPage/ShareLogList/BaseShareLogList.tsx @@ -56,7 +56,7 @@ function BaseShareLogList({onAttachLogToReport}: BaseShareLogListProps) { return defaultOptions; } - const filteredOptions = OptionsListUtils.filterOptions(defaultOptions, debouncedSearchValue, { + const filteredOptions = OptionsListUtils.filterAndOrderOptions(defaultOptions, debouncedSearchValue, { preferChatroomsOverThreads: true, sortByReportTypeInSearch: true, }); diff --git a/src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx b/src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx index efaf6d5e8480..4f37e5439951 100644 --- a/src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx +++ b/src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx @@ -56,7 +56,7 @@ function useOptions() { }, [optionsList.reports, optionsList.personalDetails, betas, existingDelegates, isLoading]); const options = useMemo(() => { - const filteredOptions = OptionsListUtils.filterOptions(defaultOptions, debouncedSearchValue.trim(), { + const filteredOptions = OptionsListUtils.filterAndOrderOptions(defaultOptions, debouncedSearchValue.trim(), { excludeLogins: [...CONST.EXPENSIFY_EMAILS, ...existingDelegates], maxRecentReportsToShow: CONST.IOU.MAX_RECENT_REPORTS_TO_SHOW, }); diff --git a/src/pages/tasks/TaskAssigneeSelectorModal.tsx b/src/pages/tasks/TaskAssigneeSelectorModal.tsx index a13475c3f15d..d51e62ae836a 100644 --- a/src/pages/tasks/TaskAssigneeSelectorModal.tsx +++ b/src/pages/tasks/TaskAssigneeSelectorModal.tsx @@ -69,7 +69,7 @@ function useOptions() { }, [optionsList.reports, optionsList.personalDetails, betas, isLoading]); const options = useMemo(() => { - const filteredOptions = OptionsListUtils.filterOptions(defaultOptions, debouncedSearchValue.trim(), { + const filteredOptions = OptionsListUtils.filterAndOrderOptionsrOptions(defaultOptions, debouncedSearchValue.trim(), { excludeLogins: CONST.EXPENSIFY_EMAILS, maxRecentReportsToShow: CONST.IOU.MAX_RECENT_REPORTS_TO_SHOW, }); diff --git a/src/pages/tasks/TaskShareDestinationSelectorModal.tsx b/src/pages/tasks/TaskShareDestinationSelectorModal.tsx index 9b85337aedcd..543afb4ba002 100644 --- a/src/pages/tasks/TaskShareDestinationSelectorModal.tsx +++ b/src/pages/tasks/TaskShareDestinationSelectorModal.tsx @@ -82,7 +82,7 @@ function TaskShareDestinationSelectorModal() { if (debouncedSearchValue.trim() === '') { return defaultOptions; } - const filteredReports = OptionsListUtils.filterOptions(defaultOptions, debouncedSearchValue.trim(), { + const filteredReports = OptionsListUtils.filterAndOrderOptions(defaultOptions, debouncedSearchValue.trim(), { excludeLogins: CONST.EXPENSIFY_EMAILS, canInviteUser: false, }); diff --git a/src/pages/workspace/WorkspaceInvitePage.tsx b/src/pages/workspace/WorkspaceInvitePage.tsx index d2ab6199cb90..eb0953a11ffb 100644 --- a/src/pages/workspace/WorkspaceInvitePage.tsx +++ b/src/pages/workspace/WorkspaceInvitePage.tsx @@ -86,7 +86,7 @@ function WorkspaceInvitePage({route, policy}: WorkspaceInvitePageProps) { }, [areOptionsInitialized, betas, excludedUsers, options.personalDetails]); const inviteOptions = useMemo( - () => OptionsListUtils.filterOptions(defaultOptions, debouncedSearchTerm, {excludeLogins: excludedUsers}), + () => OptionsListUtils.filterAndOrderOptions(defaultOptions, debouncedSearchTerm, {excludeLogins: excludedUsers}), [debouncedSearchTerm, defaultOptions, excludedUsers], ); diff --git a/tests/perf-test/OptionsListUtils.perf-test.ts b/tests/perf-test/OptionsListUtils.perf-test.ts index 19629c602477..ab3642b5f48f 100644 --- a/tests/perf-test/OptionsListUtils.perf-test.ts +++ b/tests/perf-test/OptionsListUtils.perf-test.ts @@ -109,7 +109,7 @@ describe('OptionsListUtils', () => { await waitForBatchedUpdates(); await measureFunction(() => { const formattedOptions = OptionsListUtils.getOptions({reports: options.reports, personalDetails: options.personalDetails}, {betas: mockedBetas}); - OptionsListUtils.filterOptions(formattedOptions, SEARCH_VALUE); + OptionsListUtils.filterAndOrderOptions(formattedOptions, SEARCH_VALUE); }); }); diff --git a/tests/unit/OptionsListUtilsTest.ts b/tests/unit/OptionsListUtilsTest.ts index cad6c010bb67..279baed208e5 100644 --- a/tests/unit/OptionsListUtilsTest.ts +++ b/tests/unit/OptionsListUtilsTest.ts @@ -412,14 +412,10 @@ describe('OptionsListUtils', () => { expect(results.recentReports.length).toBe(Object.values(OPTIONS.reports).length); }); - it('getOptions()', () => { - const MAX_RECENT_REPORTS = 5; - + it('orderOptions()', () => { // When we call getOptions() with no search value - let results = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}, {maxRecentReportsToShow: MAX_RECENT_REPORTS}); - - // We should expect maximum of 5 recent reports to be returned - expect(results.recentReports.length).toBe(MAX_RECENT_REPORTS); + let results: Pick = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); + results = OptionsListUtils.orderOptions(results); // We should expect all personalDetails except the currently logged in user to be returned // Filtering of personalDetails that have reports is done in filterOptions @@ -443,23 +439,21 @@ describe('OptionsListUtils', () => { // When we only pass personal details results = OptionsListUtils.getOptions({personalDetails: OPTIONS.personalDetails, reports: []}); + results = OptionsListUtils.orderOptions(results); // We should expect personal details sorted alphabetically expect(results.personalDetails.at(0)?.text).toBe('Black Panther'); expect(results.personalDetails.at(1)?.text).toBe('Black Widow'); expect(results.personalDetails.at(2)?.text).toBe('Captain America'); expect(results.personalDetails.at(3)?.text).toBe('Invisible Woman'); + }); + it('getOptions()', () => { // When we don't include personal detail to the result - results = OptionsListUtils.getOptions( - { - personalDetails: [], - reports: [], - }, - { - maxRecentReportsToShow: 0, - }, - ); + let results = OptionsListUtils.getOptions({ + personalDetails: [], + reports: [], + }); // Then no personal detail options will be returned expect(results.personalDetails.length).toBe(0); @@ -523,30 +517,18 @@ describe('OptionsListUtils', () => { // Filtering of personalDetails that have reports is done in filterOptions expect(results.personalDetails.length).toBe(Object.values(OPTIONS.personalDetails).length - 1); - // All personal details including those that have reports should be returned - // We should expect personal details sorted alphabetically - expect(results.personalDetails.at(0)?.text).toBe('Black Panther'); - expect(results.personalDetails.at(1)?.text).toBe('Black Widow'); - expect(results.personalDetails.at(2)?.text).toBe('Captain America'); - expect(results.personalDetails.at(3)?.text).toBe('Invisible Woman'); - expect(results.personalDetails.at(4)?.text).toBe('Mister Fantastic'); - expect(results.personalDetails.at(5)?.text).toBe('Mr Sinister'); - expect(results.personalDetails.at(6)?.text).toBe('Spider-Man'); - expect(results.personalDetails.at(7)?.text).toBe('The Incredible Hulk'); - expect(results.personalDetails.at(8)?.text).toBe('Thor'); - // And none of our personalDetails should include any of the users with recent reports const reportLogins = results.recentReports.map((reportOption) => reportOption.login); const personalDetailsOverlapWithReports = results.personalDetails.every((personalDetailOption) => reportLogins.includes(personalDetailOption.login)); expect(personalDetailsOverlapWithReports).toBe(false); - // When we provide no selected options to getOptions() - results = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}, {maxRecentReportsToShow: 5}); + // When we limit getOptions() + results = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); // Then one of our older report options (not in our five most recent) should appear in the personalDetails // but not in recentReports - expect(results.recentReports.every((option) => option.login !== 'peterparker@expensify.com')).toBe(true); - expect(results.personalDetails.every((option) => option.login !== 'peterparker@expensify.com')).toBe(false); + // expect(results.recentReports.every((option) => option.login !== 'peterparker@expensify.com')).toBe(true); + // expect(results.personalDetails.every((option) => option.login !== 'peterparker@expensify.com')).toBe(false); // When we provide a "selected" option to getOptions() results = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}, {excludeLogins: ['peterparker@expensify.com']}); @@ -676,7 +658,7 @@ describe('OptionsListUtils', () => { describe('filterOptions', () => { it('should return all options when search is empty', () => { const options = OptionsListUtils.getSearchOptions(OPTIONS, [CONST.BETAS.ALL]); - const filteredOptions = OptionsListUtils.filterOptions(options, ''); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, ''); expect(filteredOptions.recentReports.length + filteredOptions.personalDetails.length).toBe(12); }); @@ -685,7 +667,8 @@ describe('OptionsListUtils', () => { const searchText = 'man'; const options = OptionsListUtils.getSearchOptions(OPTIONS, [CONST.BETAS.ALL]); - const filteredOptions = OptionsListUtils.filterOptions(options, searchText, {sortByReportTypeInSearch: true}); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, searchText, {sortByReportTypeInSearch: true}); + expect(filteredOptions.recentReports.length).toBe(4); expect(filteredOptions.recentReports.at(0)?.text).toBe('Invisible Woman'); expect(filteredOptions.recentReports.at(1)?.text).toBe('Spider-Man'); @@ -697,7 +680,7 @@ describe('OptionsListUtils', () => { const searchText = 'mistersinister@marauders.com'; const options = OptionsListUtils.getSearchOptions(OPTIONS, [CONST.BETAS.ALL]); - const filteredOptions = OptionsListUtils.filterOptions(options, searchText); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, searchText); expect(filteredOptions.recentReports.length).toBe(1); expect(filteredOptions.recentReports.at(0)?.text).toBe('Mr Sinister'); @@ -706,7 +689,7 @@ describe('OptionsListUtils', () => { it('should find archived chats', () => { const searchText = 'Archived'; const options = OptionsListUtils.getSearchOptions(OPTIONS, [CONST.BETAS.ALL]); - const filteredOptions = OptionsListUtils.filterOptions(options, searchText); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, searchText); expect(filteredOptions.recentReports.length).toBe(1); expect(!!filteredOptions.recentReports.at(0)?.private_isArchived).toBe(true); @@ -717,7 +700,7 @@ describe('OptionsListUtils', () => { const OPTIONS_WITH_PERIODS = OptionsListUtils.createOptionList(PERSONAL_DETAILS_WITH_PERIODS, REPORTS); const options = OptionsListUtils.getSearchOptions(OPTIONS_WITH_PERIODS, [CONST.BETAS.ALL]); - const filteredOptions = OptionsListUtils.filterOptions(options, searchText, {sortByReportTypeInSearch: true}); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, searchText, {sortByReportTypeInSearch: true}); expect(filteredOptions.recentReports.length).toBe(1); expect(filteredOptions.recentReports.at(0)?.login).toBe('barry.allen@expensify.com'); @@ -727,7 +710,7 @@ describe('OptionsListUtils', () => { const searchText = 'avengers'; const options = OptionsListUtils.getSearchOptions(OPTIONS_WITH_WORKSPACE_ROOM, [CONST.BETAS.ALL]); - const filteredOptions = OptionsListUtils.filterOptions(options, searchText); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, searchText); expect(filteredOptions.recentReports.length).toBe(1); expect(filteredOptions.recentReports.at(0)?.subtitle).toBe('Avengers Room'); @@ -737,7 +720,7 @@ describe('OptionsListUtils', () => { const searchText = 'reedrichards@expensify.com'; const options = OptionsListUtils.getSearchOptions(OPTIONS, [CONST.BETAS.ALL]); - const filteredOptions = OptionsListUtils.filterOptions(options, searchText); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, searchText); expect(filteredOptions.recentReports.length).toBe(1); expect(filteredOptions.recentReports.at(0)?.login).toBe(searchText); @@ -748,7 +731,7 @@ describe('OptionsListUtils', () => { const OPTIONS_WITH_CHATROOMS = OptionsListUtils.createOptionList(PERSONAL_DETAILS, REPORTS_WITH_CHAT_ROOM); const options = OptionsListUtils.getSearchOptions(OPTIONS_WITH_CHATROOMS, [CONST.BETAS.ALL]); - const filterOptions = OptionsListUtils.filterOptions(options, searchText); + const filterOptions = OptionsListUtils.filterAndOrderOptions(options, searchText); expect(filterOptions.recentReports.length).toBe(2); expect(filterOptions.recentReports.at(1)?.isChatRoom).toBe(true); @@ -758,7 +741,7 @@ describe('OptionsListUtils', () => { const searchText = 'fantastic'; const options = OptionsListUtils.getSearchOptions(OPTIONS); - const filteredOptions = OptionsListUtils.filterOptions(options, searchText); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, searchText); expect(filteredOptions.recentReports.length).toBe(2); expect(filteredOptions.recentReports.at(0)?.text).toBe('Mister Fantastic'); @@ -769,7 +752,7 @@ describe('OptionsListUtils', () => { const searchText = 'test@email.com'; const options = OptionsListUtils.getSearchOptions(OPTIONS); - const filteredOptions = OptionsListUtils.filterOptions(options, searchText); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, searchText); expect(filteredOptions.userToInvite?.login).toBe(searchText); }); @@ -778,7 +761,7 @@ describe('OptionsListUtils', () => { const searchText = 'admin@expensify.com'; const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}, {excludeLogins: CONST.EXPENSIFY_EMAILS}); - const filterOptions = OptionsListUtils.filterOptions(options, searchText, {excludeLogins: CONST.EXPENSIFY_EMAILS}); + const filterOptions = OptionsListUtils.filterAndOrderOptions(options, searchText, {excludeLogins: CONST.EXPENSIFY_EMAILS}); expect(filterOptions.recentReports.length).toBe(0); }); @@ -786,7 +769,7 @@ describe('OptionsListUtils', () => { const searchText = 'test@email.com'; const options = OptionsListUtils.getSearchOptions(OPTIONS); - const filteredOptions = OptionsListUtils.filterOptions(options, searchText, {excludeLogins: CONST.EXPENSIFY_EMAILS}); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, searchText, {excludeLogins: CONST.EXPENSIFY_EMAILS}); expect(filteredOptions.userToInvite?.login).toBe(searchText); }); @@ -795,7 +778,7 @@ describe('OptionsListUtils', () => { const searchText = ''; const options = OptionsListUtils.getSearchOptions(OPTIONS); - const filteredOptions = OptionsListUtils.filterOptions(options, searchText, {maxRecentReportsToShow: 2}); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, searchText, {maxRecentReportsToShow: 2}); expect(filteredOptions.recentReports.length).toBe(2); }); @@ -804,20 +787,20 @@ describe('OptionsListUtils', () => { const searchText = 'natasharomanoff@expensify.com'; const options = OptionsListUtils.getSearchOptions(OPTIONS, [CONST.BETAS.ALL]); - const filteredOptions = OptionsListUtils.filterOptions(options, searchText); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, searchText); 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'); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(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'); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, 'peterparker@expensify.com'); expect(filteredOptions.personalDetails.length).toBe(1); expect(filteredOptions.personalDetails.at(0)?.text).toBe('Spider-Man'); @@ -833,7 +816,7 @@ describe('OptionsListUtils', () => { return filtered; }, []); const options = OptionsListUtils.getShareDestinationOptions(filteredReports, OPTIONS.personalDetails, []); - const filteredOptions = OptionsListUtils.filterOptions(options, 'mutants'); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, 'mutants'); expect(filteredOptions.recentReports.length).toBe(0); }); @@ -849,7 +832,7 @@ describe('OptionsListUtils', () => { }, []); const options = OptionsListUtils.getShareDestinationOptions(filteredReportsWithWorkspaceRooms, OPTIONS.personalDetails, []); - const filteredOptions = OptionsListUtils.filterOptions(options, 'Avengers Room'); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, 'Avengers Room'); expect(filteredOptions.recentReports.length).toBe(1); }); @@ -865,14 +848,14 @@ describe('OptionsListUtils', () => { }, []); const options = OptionsListUtils.getShareDestinationOptions(filteredReportsWithWorkspaceRooms, OPTIONS.personalDetails, []); - const filteredOptions = OptionsListUtils.filterOptions(options, 'Mutants Lair'); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(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 (getOptions)', () => { const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); - const filteredOptions = OptionsListUtils.filterOptions(options, 'hulk'); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, 'hulk'); expect(filteredOptions.recentReports.length).toBe(0); @@ -880,20 +863,9 @@ describe('OptionsListUtils', () => { expect(filteredOptions.personalDetails.at(0)?.login).toBe('brucebanner@expensify.com'); }); - it('should return all matching reports and personal details (getOptions)', () => { - const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}, {maxRecentReportsToShow: 5}); - const filteredOptions = OptionsListUtils.filterOptions(options, '.com'); - - expect(filteredOptions.recentReports.at(0)?.text).toBe('Captain America'); - - // We expect that only personal details that are not in the reports are included here - expect(filteredOptions.personalDetails.length).toBe(4); - expect(filteredOptions.personalDetails.at(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 (getOptions)', () => { const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); - const filteredOptions = OptionsListUtils.filterOptions(options, 'marc@expensify'); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, 'marc@expensify'); expect(filteredOptions.recentReports.length).toBe(0); expect(filteredOptions.personalDetails.length).toBe(0); @@ -902,7 +874,7 @@ describe('OptionsListUtils', () => { 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 (getOptions)', () => { const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); - const filteredOptions = OptionsListUtils.filterOptions(options, 'marc@expensify.com'); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, 'marc@expensify.com'); expect(filteredOptions.recentReports.length).toBe(0); expect(filteredOptions.personalDetails.length).toBe(0); @@ -911,7 +883,7 @@ describe('OptionsListUtils', () => { it('should return user to invite when search term has a period with options for it that do not contain the period (getOptions)', () => { const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); - const filteredOptions = OptionsListUtils.filterOptions(options, 'peter.parker@expensify.com'); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, 'peter.parker@expensify.com'); expect(filteredOptions.recentReports.length).toBe(0); expect(filteredOptions.userToInvite).not.toBe(null); @@ -919,7 +891,7 @@ describe('OptionsListUtils', () => { 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 (getOptions)', () => { const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); - const filteredOptions = OptionsListUtils.filterOptions(options, '5005550006'); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, '5005550006'); expect(filteredOptions.recentReports.length).toBe(0); expect(filteredOptions.personalDetails.length).toBe(0); @@ -929,7 +901,7 @@ describe('OptionsListUtils', () => { 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 (getOptions)', () => { const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); - const filteredOptions = OptionsListUtils.filterOptions(options, '+15005550006'); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, '+15005550006'); expect(filteredOptions.recentReports.length).toBe(0); expect(filteredOptions.personalDetails.length).toBe(0); @@ -939,7 +911,7 @@ describe('OptionsListUtils', () => { 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 (getOptions)', () => { const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); - const filteredOptions = OptionsListUtils.filterOptions(options, '+1 (800)324-3233'); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, '+1 (800)324-3233'); expect(filteredOptions.recentReports.length).toBe(0); expect(filteredOptions.personalDetails.length).toBe(0); @@ -949,7 +921,7 @@ describe('OptionsListUtils', () => { it('should not return any options or user to invite if contact number contains alphabet characters (getOptions)', () => { const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); - const filteredOptions = OptionsListUtils.filterOptions(options, '998243aaaa'); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, '998243aaaa'); expect(filteredOptions.recentReports.length).toBe(0); expect(filteredOptions.personalDetails.length).toBe(0); @@ -958,24 +930,32 @@ describe('OptionsListUtils', () => { it('should not return any options if search value does not match any personal details (getOptions)', () => { const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); - const filteredOptions = OptionsListUtils.filterOptions(options, 'magneto'); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(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 (getOptions)', () => { const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); - const filteredOptions = OptionsListUtils.filterOptions(options, 'peterparker@expensify.com', {sortByReportTypeInSearch: true}); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, 'peterparker@expensify.com', {sortByReportTypeInSearch: true}); expect(filteredOptions.recentReports.length).toBe(1); expect(filteredOptions.recentReports.at(0)?.text).toBe('Spider-Man'); expect(filteredOptions.personalDetails.length).toBe(0); }); it('should return all matching reports and personal details (getOptions)', () => { - const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}, {maxRecentReportsToShow: 5}); - const filteredOptions = OptionsListUtils.filterOptions(options, '.com'); + console.log({ + reports: OPTIONS.reports.length, + personalDetails: OPTIONS.personalDetails.length, + }); + const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); + console.log('after getOptions', { + reports: options.recentReports.length, + personalDetails: options.personalDetails.length, + }); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, '.com', {maxRecentReportsToShow: 5}); - expect(filteredOptions.personalDetails.length).toBe(4); + expect(filteredOptions.personalDetails.length).toBe(2); expect(filteredOptions.recentReports.length).toBe(5); expect(filteredOptions.personalDetails.at(0)?.login).toBe('natasharomanoff@expensify.com'); expect(filteredOptions.recentReports.at(0)?.text).toBe('Captain America'); @@ -985,7 +965,7 @@ describe('OptionsListUtils', () => { it('should return matching option when searching (getSearchOptions)', () => { const options = OptionsListUtils.getSearchOptions(OPTIONS); - const filteredOptions = OptionsListUtils.filterOptions(options, 'spider'); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, 'spider'); expect(filteredOptions.recentReports.length).toBe(1); expect(filteredOptions.recentReports.at(0)?.text).toBe('Spider-Man'); @@ -993,7 +973,7 @@ describe('OptionsListUtils', () => { 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'); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, 'fantastic'); expect(filteredOptions.recentReports.length).toBe(2); expect(filteredOptions.recentReports.at(0)?.text).toBe('Mister Fantastic'); @@ -1004,7 +984,7 @@ describe('OptionsListUtils', () => { .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}); + const filteredResults = OptionsListUtils.filterAndOrderOptions(results, 'barry.allen@expensify.com', {sortByReportTypeInSearch: true}); expect(filteredResults.recentReports.length).toBe(1); expect(filteredResults.recentReports.at(0)?.text).toBe('The Flash'); From 48d4fa17259201280e3ffa9c15685db657044731 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Tue, 26 Nov 2024 12:08:57 +0100 Subject: [PATCH 02/25] split out filtering and ordering very cleanly so we can apply splicing in the right moment --- src/libs/OptionsListUtils.ts | 194 ++++++++++++++++++++--------------- 1 file changed, 113 insertions(+), 81 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index 7db4a49e348f..13b9380b679e 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -1617,84 +1617,26 @@ function filteredPersonalDetailsOfRecentReports(recentReports: ReportUtils.Optio return personalDetails.filter((personalDetail) => !excludedLogins.has(personalDetail.login)); } -/** - * Filters options based on the search input value - */ +// TODO: try to see if all screens really need userToInvite & currentUserOption, or if this can be separate +// TODO: cleanup all config types function filterOptions(options: Options, searchInputValue: string, config?: FilterOptionsConfig): Options { - const {sortByReportTypeInSearch = false, canInviteUser = true, maxRecentReportsToShow = 0, excludeLogins = []} = config ?? {}; - if (searchInputValue.trim() === '' && maxRecentReportsToShow > 0) { - const recentReports = options.recentReports.slice(0, maxRecentReportsToShow); - const personalDetails = filteredPersonalDetailsOfRecentReports(recentReports, options.personalDetails); - return { - ...options, - recentReports, - personalDetails, - }; - } - const parsedPhoneNumber = PhoneNumber.parsePhoneNumber(LoginUtils.appendCountryCode(Str.removeSMSDomain(searchInputValue))); const searchValue = parsedPhoneNumber.possible && parsedPhoneNumber.number?.e164 ? parsedPhoneNumber.number.e164 : searchInputValue.toLowerCase(); const searchTerms = searchValue ? searchValue.split(' ') : []; - const optionsToExclude: Option[] = [{login: CONST.EMAIL.NOTIFICATIONS}]; - - excludeLogins.forEach((login) => { - optionsToExclude.push({login}); - }); - - const matchResults = searchTerms.reduceRight((items, term) => { - const recentReports = filterArrayByMatch(items.recentReports, term, (item) => { - const values: string[] = []; - if (item.text) { - values.push(item.text); - } - - if (item.login) { - values.push(item.login); - values.push(item.login.replace(CONST.EMAIL_SEARCH_REGEX, '')); - } - - if (item.isThread) { - if (item.alternateText) { - values.push(item.alternateText); - } - } else if (!!item.isChatRoom || !!item.isPolicyExpenseChat) { - if (item.subtitle) { - values.push(item.subtitle); - } - } - - return uniqFast(values); - }); - const personalDetails = filterArrayByMatch(items.personalDetails, term, (item) => uniqFast(getPersonalDetailSearchTerms(item))); - - const currentUserOptionSearchText = items.currentUserOption ? uniqFast(getCurrentUserSearchTerms(items.currentUserOption)).join(' ') : ''; - - const currentUserOption = isSearchStringMatch(term, currentUserOptionSearchText) ? items.currentUserOption : null; - return { - recentReports: recentReports ?? [], - personalDetails: personalDetails ?? [], - userToInvite: null, - currentUserOption, - }; - }, options); - - const {recentReports, personalDetails} = matchResults; - - console.log('pre', {sortByReportTypeInSearch, pD: personalDetails.length, recentReports: recentReports.length}); - const personalDetailsWithoutDMs = filteredPersonalDetailsOfRecentReports(recentReports, personalDetails); - console.log('post', personalDetailsWithoutDMs.length); - - let filteredPersonalDetails: ReportUtils.OptionData[] = personalDetailsWithoutDMs; - let filteredRecentReports: ReportUtils.OptionData[] = recentReports; - if (sortByReportTypeInSearch) { - filteredRecentReports = recentReports.concat(personalDetailsWithoutDMs); - filteredPersonalDetails = []; - } + const recentReports = filterReports(options.recentReports, searchTerms); + const personalDetails = filterPersonalDetails(options.personalDetails, searchTerms); + const {canInviteUser = true, excludeLogins = []} = config ?? {}; let userToInvite = null; if (canInviteUser) { if (recentReports.length === 0 && personalDetails.length === 0) { + const optionsToExclude: Option[] = [{login: CONST.EMAIL.NOTIFICATIONS}]; + + excludeLogins.forEach((login) => { + optionsToExclude.push({login}); + }); + userToInvite = getUserToInviteOption({ searchValue, selectedOptions: config?.selectedOptions, @@ -1703,33 +1645,123 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt } } - console.log('wat', filteredPersonalDetails.length); + const currentUserOption = filterCurrentUserOption(options.currentUserOption, searchTerms); + return { - personalDetails: filteredPersonalDetails, - recentReports: filteredRecentReports, + personalDetails, + recentReports, userToInvite, - currentUserOption: matchResults.currentUserOption, + currentUserOption, }; } +/** + * Filters options based on the search input value + */ +function filterReports(reports: ReportUtils.OptionData[], searchTerms: string[]): ReportUtils.OptionData[] { + // We search eventually for multiple whitespace separated search terms. + // We start with the search term at the end, and then narrow down those filtered search results with the next search term. + // We repeat (reduce) this until all search terms have been used: + const filteredReports = searchTerms.reduceRight( + (items, term) => + filterArrayByMatch(items, term, (item) => { + const values: string[] = []; + if (item.text) { + values.push(item.text); + } + + if (item.login) { + values.push(item.login); + values.push(item.login.replace(CONST.EMAIL_SEARCH_REGEX, '')); + } + + if (item.isThread) { + if (item.alternateText) { + values.push(item.alternateText); + } + } else if (!!item.isChatRoom || !!item.isPolicyExpenseChat) { + if (item.subtitle) { + values.push(item.subtitle); + } + } + + return uniqFast(values); + }), + // We start from all unfiltered reports: + reports, + ); + + return filteredReports; +} + +function filterPersonalDetails(personalDetails: ReportUtils.OptionData[], searchTerms: string[]): ReportUtils.OptionData[] { + return searchTerms.reduceRight( + (items, term) => + filterArrayByMatch(items, term, (item) => { + const values = getPersonalDetailSearchTerms(item); + return uniqFast(values); + }), + personalDetails, + ); +} + +function filterCurrentUserOption(currentUserOption: ReportUtils.OptionData | null | undefined, searchTerms: string[]): ReportUtils.OptionData | null | undefined { + return searchTerms.reduceRight((item, term) => { + if (!item) { + return null; + } + + const currentUserOptionSearchText = uniqFast(getCurrentUserSearchTerms(item)).join(' '); + return isSearchStringMatch(term, currentUserOptionSearchText) ? item : null; + }, currentUserOption); +} + type FilterAndOrderConfig = FilterOptionsConfig & OrderOptionsConfig; -function filterAndOrderOptions(options: Options, searchInputValue: string, config?: FilterAndOrderConfig): Options { +function filterAndOrderOptions(options: Options, searchInputValue: string, config: FilterAndOrderConfig = {}): Options { + const {maxRecentReportsToShow = 0, sortByReportTypeInSearch = false} = config; + + // Fast route: there's no search input value and we're limiting the number of recent reports to show + if (searchInputValue.trim().length === 0 && maxRecentReportsToShow > 0) { + const {personalDetails, recentReports} = orderOptions(options, searchInputValue, config); + const limitedRecentReports = recentReports.slice(0, maxRecentReportsToShow); + const filteredPersonalDetails = filteredPersonalDetailsOfRecentReports(limitedRecentReports, personalDetails); + + return { + ...options, + personalDetails: filteredPersonalDetails, + recentReports: limitedRecentReports, + }; + } + const filteredOptions = filterOptions(options, searchInputValue, config); - console.log('after filterOptions', { - reports: filteredOptions.recentReports.length, - personalDetails: filteredOptions.personalDetails.length, - }); - const {personalDetails, recentReports} = orderOptions(filteredOptions, searchInputValue, config); + const personalDetailsWithoutDMs = filteredPersonalDetailsOfRecentReports(filteredOptions.recentReports, filteredOptions.personalDetails); + + // sortByReportTypeInSearch option will show the personal details as part of the recent reports + let filteredPersonalDetails: ReportUtils.OptionData[] = personalDetailsWithoutDMs; + let filteredRecentReports: ReportUtils.OptionData[] = filteredOptions.recentReports; + if (sortByReportTypeInSearch) { + filteredRecentReports = filteredOptions.recentReports.concat(personalDetailsWithoutDMs); + filteredPersonalDetails = []; + } + + const orderedOptions = orderOptions( + { + recentReports: filteredRecentReports, + personalDetails: filteredPersonalDetails, + }, + searchInputValue, + config, + ); - let orderedReports = recentReports; + let orderedReports = orderedOptions.recentReports; if (config?.maxRecentReportsToShow) { orderedReports = orderedReports.slice(0, config.maxRecentReportsToShow); } return { ...filteredOptions, - personalDetails, recentReports: orderedReports, + personalDetails: orderedOptions.personalDetails, }; } From 65c770ea175a772af43b70788565135b6fd44be0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Tue, 26 Nov 2024 13:27:30 +0100 Subject: [PATCH 03/25] fix sorting not working --- src/libs/OptionsListUtils.ts | 7 ++----- tests/unit/OptionsListUtilsTest.ts | 14 ++++++-------- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index 13b9380b679e..2c1bed293b28 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -641,6 +641,7 @@ function createOption( policyID: undefined, isOptimisticPersonalDetail: false, lastMessageText: '', + lastVisibleActionCreated: undefined, }; const personalDetailMap = getPersonalDetailsForAccountIDs(accountIDs, personalDetails); @@ -677,6 +678,7 @@ function createOption( result.policyID = report.policyID; result.isSelfDM = ReportUtils.isSelfDM(report); result.notificationPreference = ReportUtils.getReportNotificationPreference(report); + result.lastVisibleActionCreated = report.lastVisibleActionCreated; const visibleParticipantAccountIDs = ReportUtils.getParticipantsAccountIDsForDisplay(report, true); @@ -937,13 +939,8 @@ function orderReportOptions( return lodashOrderBy( options, [ - // Sort descending by lastVisibleActionCreated (option) => option?.lastVisibleActionCreated ?? 0, (option) => { - // TODO: what the fuck this is at the bottom already, is this even needed? i don't think so … - // if (option.private_isArchived) { - // return CONST.DATE.UNIX_EPOCH; // Keep archived at bottom - // } if (option.isPolicyExpenseChat && preferPolicyExpenseChat && option.policyID === activePolicyID) { return 0; } diff --git a/tests/unit/OptionsListUtilsTest.ts b/tests/unit/OptionsListUtilsTest.ts index 279baed208e5..467fe6711def 100644 --- a/tests/unit/OptionsListUtilsTest.ts +++ b/tests/unit/OptionsListUtilsTest.ts @@ -669,7 +669,13 @@ describe('OptionsListUtils', () => { const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, searchText, {sortByReportTypeInSearch: true}); + // When sortByReportTypeInSearch is true, we expect all options to be part of the recentReports list: + expect(filteredOptions.personalDetails.length).toBe(0); + + // Expect to only find reports that matched our search text: expect(filteredOptions.recentReports.length).toBe(4); + + // This items should be ordered by most recent action (and other criteria such as whether they are archived): expect(filteredOptions.recentReports.at(0)?.text).toBe('Invisible Woman'); expect(filteredOptions.recentReports.at(1)?.text).toBe('Spider-Man'); expect(filteredOptions.recentReports.at(2)?.text).toBe('Black Widow'); @@ -944,15 +950,7 @@ describe('OptionsListUtils', () => { }); it('should return all matching reports and personal details (getOptions)', () => { - console.log({ - reports: OPTIONS.reports.length, - personalDetails: OPTIONS.personalDetails.length, - }); const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); - console.log('after getOptions', { - reports: options.recentReports.length, - personalDetails: options.personalDetails.length, - }); const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, '.com', {maxRecentReportsToShow: 5}); expect(filteredOptions.personalDetails.length).toBe(2); From 8bb573f1dcc19f4fa2ec53f2b9fcaa30e5b31aaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Tue, 26 Nov 2024 13:34:40 +0100 Subject: [PATCH 04/25] fix usage --- src/pages/NewChatPage.tsx | 1 - src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx | 1 - src/pages/tasks/TaskAssigneeSelectorModal.tsx | 3 +-- 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/pages/NewChatPage.tsx b/src/pages/NewChatPage.tsx index 2a19cd031806..cbc43254c412 100755 --- a/src/pages/NewChatPage.tsx +++ b/src/pages/NewChatPage.tsx @@ -56,7 +56,6 @@ function useOptions() { { betas: betas ?? [], selectedOptions, - maxRecentReportsToShow: 0, includeSelfDM: true, }, ); diff --git a/src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx b/src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx index 4f37e5439951..622e14ff9813 100644 --- a/src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx +++ b/src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx @@ -35,7 +35,6 @@ function useOptions() { { betas, excludeLogins: [...CONST.EXPENSIFY_EMAILS, ...existingDelegates], - maxRecentReportsToShow: 0, }, ); diff --git a/src/pages/tasks/TaskAssigneeSelectorModal.tsx b/src/pages/tasks/TaskAssigneeSelectorModal.tsx index d51e62ae836a..5461611c4260 100644 --- a/src/pages/tasks/TaskAssigneeSelectorModal.tsx +++ b/src/pages/tasks/TaskAssigneeSelectorModal.tsx @@ -48,7 +48,6 @@ function useOptions() { { betas, excludeLogins: CONST.EXPENSIFY_EMAILS, - maxRecentReportsToShow: 0, }, ); @@ -69,7 +68,7 @@ function useOptions() { }, [optionsList.reports, optionsList.personalDetails, betas, isLoading]); const options = useMemo(() => { - const filteredOptions = OptionsListUtils.filterAndOrderOptionsrOptions(defaultOptions, debouncedSearchValue.trim(), { + const filteredOptions = OptionsListUtils.filterAndOrderOptions(defaultOptions, debouncedSearchValue.trim(), { excludeLogins: CONST.EXPENSIFY_EMAILS, maxRecentReportsToShow: CONST.IOU.MAX_RECENT_REPORTS_TO_SHOW, }); From 717d750e534528f8c9737a19e9d76779f2c5e411 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Tue, 26 Nov 2024 14:21:22 +0100 Subject: [PATCH 05/25] rename getOptions -> getValidOptions --- .../SearchFiltersParticipantsSelector.tsx | 2 +- src/libs/OptionsListUtils.ts | 17 +++--- src/pages/NewChatPage.tsx | 2 +- .../MoneyRequestParticipantsSelector.tsx | 2 +- .../Security/AddDelegate/AddDelegatePage.tsx | 2 +- src/pages/tasks/TaskAssigneeSelectorModal.tsx | 2 +- tests/perf-test/OptionsListUtils.perf-test.ts | 2 +- tests/unit/OptionsListUtilsTest.ts | 55 ++++++++++--------- 8 files changed, 43 insertions(+), 41 deletions(-) diff --git a/src/components/Search/SearchFiltersParticipantsSelector.tsx b/src/components/Search/SearchFiltersParticipantsSelector.tsx index c84f71a34b6e..6fec134a608a 100644 --- a/src/components/Search/SearchFiltersParticipantsSelector.tsx +++ b/src/components/Search/SearchFiltersParticipantsSelector.tsx @@ -54,7 +54,7 @@ function SearchFiltersParticipantsSelector({initialAccountIDs, onFiltersUpdate}: return defaultListOptions; } - return OptionsListUtils.getOptions( + return OptionsListUtils.getValidOptions( { reports: options.reports, personalDetails: options.personalDetails, diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index 2c1bed293b28..246dbc7f7909 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -1061,11 +1061,10 @@ function getUserToInviteOption({ return userToInvite; } -// TODO: rename /** - * filter options based on specific conditions + * Options are reports and personal details. This function filters out the options that are not valid to be displayed. */ -function getOptions( +function getValidOptions( options: OptionList, { reportActions = {}, @@ -1311,7 +1310,7 @@ function getOptions( function getSearchOptions(options: OptionList, betas: Beta[] = [], isUsedInChatFinder = true): Options { Timing.start(CONST.TIMING.LOAD_SEARCH_OPTIONS); Performance.markStart(CONST.TIMING.LOAD_SEARCH_OPTIONS); - const optionList = getOptions(options, { + const optionList = getValidOptions(options, { betas, includeMultipleParticipantReports: true, showChatPreviewLine: isUsedInChatFinder, @@ -1331,7 +1330,7 @@ function getSearchOptions(options: OptionList, betas: Beta[] = [], isUsedInChatF } function getShareLogOptions(options: OptionList, betas: Beta[] = []): Options { - return getOptions(options, { + return getValidOptions(options, { betas, includeMultipleParticipantReports: true, includeP2P: true, @@ -1377,7 +1376,7 @@ function getAttendeeOptions( includeInvoiceRooms = false, action: IOUAction | undefined = undefined, ) { - return getOptions( + return getValidOptions( {reports, personalDetails}, { betas, @@ -1409,7 +1408,7 @@ function getShareDestinationOptions( includeOwnedWorkspaceChats = true, excludeUnknownUsers = true, ) { - return getOptions( + return getValidOptions( {reports, personalDetails}, { betas, @@ -1465,7 +1464,7 @@ function getMemberInviteOptions( reports: Array> = [], includeRecentReports = false, ): Options { - const options = getOptions( + const options = getValidOptions( {reports, personalDetails}, { betas, @@ -1784,7 +1783,7 @@ export { getAvatarsForAccountIDs, isCurrentUser, isPersonalDetailsReady, - getOptions, + getValidOptions, getSearchOptions, getShareDestinationOptions, getMemberInviteOptions, diff --git a/src/pages/NewChatPage.tsx b/src/pages/NewChatPage.tsx index cbc43254c412..6728a5181b63 100755 --- a/src/pages/NewChatPage.tsx +++ b/src/pages/NewChatPage.tsx @@ -48,7 +48,7 @@ function useOptions() { }); const defaultOptions = useMemo(() => { - const filteredOptions = OptionsListUtils.getOptions( + const filteredOptions = OptionsListUtils.getValidOptions( { reports: listOptions.reports ?? [], personalDetails: listOptions.personalDetails ?? [], diff --git a/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx b/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx index 70f1abdd3e44..3c9c165261ce 100644 --- a/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx +++ b/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx @@ -105,7 +105,7 @@ function MoneyRequestParticipantsSelector({ }; } - const optionList = OptionsListUtils.getOptions( + const optionList = OptionsListUtils.getValidOptions( { reports: options.reports, personalDetails: options.personalDetails, diff --git a/src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx b/src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx index 622e14ff9813..201d7c95df0f 100644 --- a/src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx +++ b/src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx @@ -27,7 +27,7 @@ function useOptions() { const existingDelegates = useMemo(() => account?.delegatedAccess?.delegates?.map((delegate) => delegate.email) ?? [], [account?.delegatedAccess?.delegates]); const defaultOptions = useMemo(() => { - const {recentReports, personalDetails, userToInvite, currentUserOption} = OptionsListUtils.getOptions( + const {recentReports, personalDetails, userToInvite, currentUserOption} = OptionsListUtils.getValidOptions( { reports: optionsList.reports, personalDetails: optionsList.personalDetails, diff --git a/src/pages/tasks/TaskAssigneeSelectorModal.tsx b/src/pages/tasks/TaskAssigneeSelectorModal.tsx index 5461611c4260..d0da5b673b76 100644 --- a/src/pages/tasks/TaskAssigneeSelectorModal.tsx +++ b/src/pages/tasks/TaskAssigneeSelectorModal.tsx @@ -40,7 +40,7 @@ function useOptions() { const {options: optionsList, areOptionsInitialized} = useOptionsList(); const defaultOptions = useMemo(() => { - const {recentReports, personalDetails, userToInvite, currentUserOption} = OptionsListUtils.getOptions( + const {recentReports, personalDetails, userToInvite, currentUserOption} = OptionsListUtils.getValidOptions( { reports: optionsList.reports, personalDetails: optionsList.personalDetails, diff --git a/tests/perf-test/OptionsListUtils.perf-test.ts b/tests/perf-test/OptionsListUtils.perf-test.ts index ab3642b5f48f..11a756041a3b 100644 --- a/tests/perf-test/OptionsListUtils.perf-test.ts +++ b/tests/perf-test/OptionsListUtils.perf-test.ts @@ -108,7 +108,7 @@ describe('OptionsListUtils', () => { test('[OptionsListUtils] getFilteredOptions with search value', async () => { await waitForBatchedUpdates(); await measureFunction(() => { - const formattedOptions = OptionsListUtils.getOptions({reports: options.reports, personalDetails: options.personalDetails}, {betas: mockedBetas}); + const formattedOptions = OptionsListUtils.getValidOptions({reports: options.reports, personalDetails: options.personalDetails}, {betas: mockedBetas}); OptionsListUtils.filterAndOrderOptions(formattedOptions, SEARCH_VALUE); }); }); diff --git a/tests/unit/OptionsListUtilsTest.ts b/tests/unit/OptionsListUtilsTest.ts index 467fe6711def..52cdfd1a3bdc 100644 --- a/tests/unit/OptionsListUtilsTest.ts +++ b/tests/unit/OptionsListUtilsTest.ts @@ -414,7 +414,10 @@ describe('OptionsListUtils', () => { it('orderOptions()', () => { // When we call getOptions() with no search value - let results: Pick = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); + let results: Pick = OptionsListUtils.getValidOptions({ + reports: OPTIONS.reports, + personalDetails: OPTIONS.personalDetails, + }); results = OptionsListUtils.orderOptions(results); // We should expect all personalDetails except the currently logged in user to be returned @@ -438,7 +441,7 @@ describe('OptionsListUtils', () => { expect(personalDetailWithExistingReport?.reportID).toBe('2'); // When we only pass personal details - results = OptionsListUtils.getOptions({personalDetails: OPTIONS.personalDetails, reports: []}); + results = OptionsListUtils.getValidOptions({personalDetails: OPTIONS.personalDetails, reports: []}); results = OptionsListUtils.orderOptions(results); // We should expect personal details sorted alphabetically @@ -450,7 +453,7 @@ describe('OptionsListUtils', () => { it('getOptions()', () => { // When we don't include personal detail to the result - let results = OptionsListUtils.getOptions({ + let results = OptionsListUtils.getValidOptions({ personalDetails: [], reports: [], }); @@ -460,7 +463,7 @@ describe('OptionsListUtils', () => { // Test for Concierge's existence in chat options - results = OptionsListUtils.getOptions({reports: OPTIONS_WITH_CONCIERGE.reports, personalDetails: OPTIONS_WITH_CONCIERGE.personalDetails}); + results = OptionsListUtils.getValidOptions({reports: OPTIONS_WITH_CONCIERGE.reports, personalDetails: OPTIONS_WITH_CONCIERGE.personalDetails}); // Concierge is included in the results by default. We should expect all the personalDetails to show // (minus the currently logged in user) @@ -469,7 +472,7 @@ describe('OptionsListUtils', () => { expect(results.recentReports).toEqual(expect.arrayContaining([expect.objectContaining({login: 'concierge@expensify.com'})])); // Test by excluding Concierge from the results - results = OptionsListUtils.getOptions( + results = OptionsListUtils.getValidOptions( { reports: OPTIONS_WITH_CONCIERGE.reports, personalDetails: OPTIONS_WITH_CONCIERGE.personalDetails, @@ -485,7 +488,7 @@ describe('OptionsListUtils', () => { expect(results.personalDetails).not.toEqual(expect.arrayContaining([expect.objectContaining({login: 'concierge@expensify.com'})])); // Test by excluding Chronos from the results - results = OptionsListUtils.getOptions({reports: OPTIONS_WITH_CHRONOS.reports, personalDetails: OPTIONS_WITH_CHRONOS.personalDetails}, {excludeLogins: [CONST.EMAIL.CHRONOS]}); + results = OptionsListUtils.getValidOptions({reports: OPTIONS_WITH_CHRONOS.reports, personalDetails: OPTIONS_WITH_CHRONOS.personalDetails}, {excludeLogins: [CONST.EMAIL.CHRONOS]}); // All the personalDetails should be returned minus the currently logged in user and Concierge // Filtering of personalDetails that have reports is done in filterOptions @@ -493,7 +496,7 @@ describe('OptionsListUtils', () => { expect(results.personalDetails).not.toEqual(expect.arrayContaining([expect.objectContaining({login: 'chronos@expensify.com'})])); // Test by excluding Receipts from the results - results = OptionsListUtils.getOptions( + results = OptionsListUtils.getValidOptions( { reports: OPTIONS_WITH_RECEIPTS.reports, personalDetails: OPTIONS_WITH_RECEIPTS.personalDetails, @@ -511,7 +514,7 @@ describe('OptionsListUtils', () => { it('getOptions() for group Chat', () => { // When we call getOptions() with no search value - let results = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); + let results = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); // We should expect all the personalDetails to show except the currently logged in user // Filtering of personalDetails that have reports is done in filterOptions @@ -523,7 +526,7 @@ describe('OptionsListUtils', () => { expect(personalDetailsOverlapWithReports).toBe(false); // When we limit getOptions() - results = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); + results = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); // Then one of our older report options (not in our five most recent) should appear in the personalDetails // but not in recentReports @@ -531,14 +534,14 @@ describe('OptionsListUtils', () => { // expect(results.personalDetails.every((option) => option.login !== 'peterparker@expensify.com')).toBe(false); // When we provide a "selected" option to getOptions() - results = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}, {excludeLogins: ['peterparker@expensify.com']}); + results = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}, {excludeLogins: ['peterparker@expensify.com']}); // Then the option should not appear anywhere in either list expect(results.recentReports.every((option) => option.login !== 'peterparker@expensify.com')).toBe(true); expect(results.personalDetails.every((option) => option.login !== 'peterparker@expensify.com')).toBe(true); // Test Concierge's existence in new group options - results = OptionsListUtils.getOptions({reports: OPTIONS_WITH_CONCIERGE.reports, personalDetails: OPTIONS_WITH_CONCIERGE.personalDetails}); + results = OptionsListUtils.getValidOptions({reports: OPTIONS_WITH_CONCIERGE.reports, personalDetails: OPTIONS_WITH_CONCIERGE.personalDetails}); // Concierge is included in the results by default. We should expect all the personalDetails to show // (minus the currently logged in user) @@ -547,7 +550,7 @@ describe('OptionsListUtils', () => { expect(results.recentReports).toEqual(expect.arrayContaining([expect.objectContaining({login: 'concierge@expensify.com'})])); // Test by excluding Concierge from the results - results = OptionsListUtils.getOptions( + results = OptionsListUtils.getValidOptions( { reports: OPTIONS_WITH_CONCIERGE.reports, personalDetails: OPTIONS_WITH_CONCIERGE.personalDetails, @@ -565,7 +568,7 @@ describe('OptionsListUtils', () => { expect(results.recentReports).not.toEqual(expect.arrayContaining([expect.objectContaining({login: 'concierge@expensify.com'})])); // Test by excluding Chronos from the results - results = OptionsListUtils.getOptions({reports: OPTIONS_WITH_CHRONOS.reports, personalDetails: OPTIONS_WITH_CHRONOS.personalDetails}, {excludeLogins: [CONST.EMAIL.CHRONOS]}); + results = OptionsListUtils.getValidOptions({reports: OPTIONS_WITH_CHRONOS.reports, personalDetails: OPTIONS_WITH_CHRONOS.personalDetails}, {excludeLogins: [CONST.EMAIL.CHRONOS]}); // We should expect all the personalDetails to show (minus // the currently logged in user and Concierge) @@ -575,7 +578,7 @@ describe('OptionsListUtils', () => { expect(results.recentReports).not.toEqual(expect.arrayContaining([expect.objectContaining({login: 'chronos@expensify.com'})])); // Test by excluding Receipts from the results - results = OptionsListUtils.getOptions( + results = OptionsListUtils.getValidOptions( { reports: OPTIONS_WITH_RECEIPTS.reports, personalDetails: OPTIONS_WITH_RECEIPTS.personalDetails, @@ -766,7 +769,7 @@ describe('OptionsListUtils', () => { it('should not return any results if the search value is on an exluded logins list', () => { const searchText = 'admin@expensify.com'; - const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}, {excludeLogins: CONST.EXPENSIFY_EMAILS}); + const options = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}, {excludeLogins: CONST.EXPENSIFY_EMAILS}); const filterOptions = OptionsListUtils.filterAndOrderOptions(options, searchText, {excludeLogins: CONST.EXPENSIFY_EMAILS}); expect(filterOptions.recentReports.length).toBe(0); }); @@ -860,7 +863,7 @@ describe('OptionsListUtils', () => { }); it('should show the option from personal details when searching for personal detail with no existing report (getOptions)', () => { - const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); + const options = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, 'hulk'); expect(filteredOptions.recentReports.length).toBe(0); @@ -870,7 +873,7 @@ describe('OptionsListUtils', () => { }); 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 (getOptions)', () => { - const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); + const options = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, 'marc@expensify'); expect(filteredOptions.recentReports.length).toBe(0); @@ -879,7 +882,7 @@ describe('OptionsListUtils', () => { }); 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 (getOptions)', () => { - const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); + const options = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, 'marc@expensify.com'); expect(filteredOptions.recentReports.length).toBe(0); @@ -888,7 +891,7 @@ describe('OptionsListUtils', () => { }); it('should return user to invite when search term has a period with options for it that do not contain the period (getOptions)', () => { - const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); + const options = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, 'peter.parker@expensify.com'); expect(filteredOptions.recentReports.length).toBe(0); @@ -896,7 +899,7 @@ describe('OptionsListUtils', () => { }); 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 (getOptions)', () => { - const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); + const options = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, '5005550006'); expect(filteredOptions.recentReports.length).toBe(0); @@ -906,7 +909,7 @@ describe('OptionsListUtils', () => { }); 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 (getOptions)', () => { - const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); + const options = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, '+15005550006'); expect(filteredOptions.recentReports.length).toBe(0); @@ -916,7 +919,7 @@ describe('OptionsListUtils', () => { }); 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 (getOptions)', () => { - const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); + const options = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, '+1 (800)324-3233'); expect(filteredOptions.recentReports.length).toBe(0); @@ -926,7 +929,7 @@ describe('OptionsListUtils', () => { }); it('should not return any options or user to invite if contact number contains alphabet characters (getOptions)', () => { - const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); + const options = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, '998243aaaa'); expect(filteredOptions.recentReports.length).toBe(0); @@ -935,14 +938,14 @@ describe('OptionsListUtils', () => { }); it('should not return any options if search value does not match any personal details (getOptions)', () => { - const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); + const options = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterAndOrderOptions(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 (getOptions)', () => { - const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); + const options = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, 'peterparker@expensify.com', {sortByReportTypeInSearch: true}); expect(filteredOptions.recentReports.length).toBe(1); expect(filteredOptions.recentReports.at(0)?.text).toBe('Spider-Man'); @@ -950,7 +953,7 @@ describe('OptionsListUtils', () => { }); it('should return all matching reports and personal details (getOptions)', () => { - const options = OptionsListUtils.getOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); + const options = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, '.com', {maxRecentReportsToShow: 5}); expect(filteredOptions.personalDetails.length).toBe(2); From 8ff82483cd94ee1e1f7dc7263cb4641a9191deb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Tue, 26 Nov 2024 14:52:50 +0100 Subject: [PATCH 06/25] remove any todos --- src/libs/OptionsListUtils.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index 246dbc7f7909..8fa8898bfec6 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -1613,8 +1613,6 @@ function filteredPersonalDetailsOfRecentReports(recentReports: ReportUtils.Optio return personalDetails.filter((personalDetail) => !excludedLogins.has(personalDetail.login)); } -// TODO: try to see if all screens really need userToInvite & currentUserOption, or if this can be separate -// TODO: cleanup all config types function filterOptions(options: Options, searchInputValue: string, config?: FilterOptionsConfig): Options { const parsedPhoneNumber = PhoneNumber.parsePhoneNumber(LoginUtils.appendCountryCode(Str.removeSMSDomain(searchInputValue))); const searchValue = parsedPhoneNumber.possible && parsedPhoneNumber.number?.e164 ? parsedPhoneNumber.number.e164 : searchInputValue.toLowerCase(); From 8bd35e350b9d95ce76090c81ca43e224a318936c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Mon, 2 Dec 2024 12:02:28 +0100 Subject: [PATCH 07/25] add missing `userToInvite` back to filterOptions --- src/libs/OptionsListUtils.ts | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index 9beba5e49595..2ebe1bc2bc35 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -114,7 +114,6 @@ type GetOptionsConfig = { type GetUserToInviteConfig = { searchValue: string; - excludeUnknownUsers?: boolean; optionsToExclude?: Array>; selectedOptions?: Array>; reportActions?: ReportActions; @@ -151,6 +150,7 @@ type FilterOptionsConfig = Pick 'login' in optionToExclude && optionToExclude.login === PhoneNumber.addSMSDomainIfPhoneNumber(searchValue).toLowerCase()) !== -1; - if (!searchValue || isCurrentUserLogin || isInSelectedOption || (!isValidEmail && !isValidPhoneNumber && !shouldAcceptName) || isInOptionToExclude || excludeUnknownUsers) { + if (!searchValue || isCurrentUserLogin || isInSelectedOption || (!isValidEmail && !isValidPhoneNumber && !shouldAcceptName) || isInOptionToExclude) { return null; } @@ -1596,10 +1594,33 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt const currentUserOption = filterCurrentUserOption(options.currentUserOption, searchTerms); + const {canInviteUser = true, excludeLogins = []} = config ?? {}; + let userToInvite = null; + if (canInviteUser) { + const canCreateOptimisticDetail = canCreateOptimisticPersonalDetailOption({ + recentReportOptions: recentReports, + personalDetailsOptions: personalDetails, + currentUserOption, + }); + if (canCreateOptimisticDetail) { + const optionsToExclude: Option[] = [{login: CONST.EMAIL.NOTIFICATIONS}]; + + excludeLogins.forEach((login) => { + optionsToExclude.push({login}); + }); + + userToInvite = getUserToInviteOption({ + searchValue, + selectedOptions: config?.selectedOptions, + optionsToExclude, + }); + } + } + return { personalDetails, recentReports, - userToInvite: null, + userToInvite, currentUserOption, }; } From 2e2448c0c4157c5811bf80ae8cc12b645151d9eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Mon, 2 Dec 2024 12:15:47 +0100 Subject: [PATCH 08/25] refactor: move userToInvite to its own function --- src/libs/OptionsListUtils.ts | 96 +++++++++++++++++++++--------------- 1 file changed, 55 insertions(+), 41 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index 2ebe1bc2bc35..abf9d9c7d380 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -1584,47 +1584,6 @@ function filteredPersonalDetailsOfRecentReports(recentReports: ReportUtils.Optio return personalDetails.filter((personalDetail) => !excludedLogins.has(personalDetail.login)); } -function filterOptions(options: Options, searchInputValue: string, config?: FilterOptionsConfig): Options { - const parsedPhoneNumber = PhoneNumber.parsePhoneNumber(LoginUtils.appendCountryCode(Str.removeSMSDomain(searchInputValue))); - const searchValue = parsedPhoneNumber.possible && parsedPhoneNumber.number?.e164 ? parsedPhoneNumber.number.e164 : searchInputValue.toLowerCase(); - const searchTerms = searchValue ? searchValue.split(' ') : []; - - const recentReports = filterReports(options.recentReports, searchTerms); - const personalDetails = filterPersonalDetails(options.personalDetails, searchTerms); - - const currentUserOption = filterCurrentUserOption(options.currentUserOption, searchTerms); - - const {canInviteUser = true, excludeLogins = []} = config ?? {}; - let userToInvite = null; - if (canInviteUser) { - const canCreateOptimisticDetail = canCreateOptimisticPersonalDetailOption({ - recentReportOptions: recentReports, - personalDetailsOptions: personalDetails, - currentUserOption, - }); - if (canCreateOptimisticDetail) { - const optionsToExclude: Option[] = [{login: CONST.EMAIL.NOTIFICATIONS}]; - - excludeLogins.forEach((login) => { - optionsToExclude.push({login}); - }); - - userToInvite = getUserToInviteOption({ - searchValue, - selectedOptions: config?.selectedOptions, - optionsToExclude, - }); - } - } - - return { - personalDetails, - recentReports, - userToInvite, - currentUserOption, - }; -} - /** * Filters options based on the search input value */ @@ -1686,6 +1645,61 @@ function filterCurrentUserOption(currentUserOption: ReportUtils.OptionData | nul }, currentUserOption); } +function filterOptions(options: Options, searchInputValue: string, config?: FilterOptionsConfig): Options { + const parsedPhoneNumber = PhoneNumber.parsePhoneNumber(LoginUtils.appendCountryCode(Str.removeSMSDomain(searchInputValue))); + const searchValue = parsedPhoneNumber.possible && parsedPhoneNumber.number?.e164 ? parsedPhoneNumber.number.e164 : searchInputValue.toLowerCase(); + const searchTerms = searchValue ? searchValue.split(' ') : []; + + const recentReports = filterReports(options.recentReports, searchTerms); + const personalDetails = filterPersonalDetails(options.personalDetails, searchTerms); + const currentUserOption = filterCurrentUserOption(options.currentUserOption, searchTerms); + const userToInvite = filterUserToInvite( + { + recentReports, + personalDetails, + currentUserOption, + userToInvite: null, + }, + searchValue, + config, + ); + + return { + personalDetails, + recentReports, + userToInvite, + currentUserOption, + }; +} + +function filterUserToInvite(options: Options, searchValue: string, config?: FilterOptionsConfig): ReportUtils.OptionData | null { + const {canInviteUser = true, excludeLogins = []} = config ?? {}; + if (!canInviteUser) { + return null; + } + + const canCreateOptimisticDetail = canCreateOptimisticPersonalDetailOption({ + recentReportOptions: options.recentReports, + personalDetailsOptions: options.personalDetails, + currentUserOption: options.currentUserOption, + }); + + if (!canCreateOptimisticDetail) { + return null; + } + + const optionsToExclude: Option[] = [{login: CONST.EMAIL.NOTIFICATIONS}]; + excludeLogins.forEach((login) => { + optionsToExclude.push({login}); + }); + + return getUserToInviteOption({ + searchValue, + selectedOptions: config?.selectedOptions, + optionsToExclude, + }); +} + type FilterAndOrderConfig = FilterOptionsConfig & OrderOptionsConfig; function filterAndOrderOptions(options: Options, searchInputValue: string, config: FilterAndOrderConfig = {}): Options { const {maxRecentReportsToShow = 0, sortByReportTypeInSearch = false} = config; From e46e6ecae08c2a3b550630cc80239033b2b31c6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Mon, 2 Dec 2024 12:18:00 +0100 Subject: [PATCH 09/25] fix getOptions -> getValidOptions --- src/components/Search/SearchRouter/SearchRouterList.tsx | 2 +- tests/unit/OptionsListUtilsTest.ts | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/components/Search/SearchRouter/SearchRouterList.tsx b/src/components/Search/SearchRouter/SearchRouterList.tsx index 63dd1e6af229..7386e86c1747 100644 --- a/src/components/Search/SearchRouter/SearchRouterList.tsx +++ b/src/components/Search/SearchRouter/SearchRouterList.tsx @@ -140,7 +140,7 @@ function SearchRouterList( return []; } - const filteredOptions = OptionsListUtils.getOptions( + const filteredOptions = OptionsListUtils.getValidOptions( { reports: options.reports, personalDetails: options.personalDetails, diff --git a/tests/unit/OptionsListUtilsTest.ts b/tests/unit/OptionsListUtilsTest.ts index 52cdfd1a3bdc..4c5c09431815 100644 --- a/tests/unit/OptionsListUtilsTest.ts +++ b/tests/unit/OptionsListUtilsTest.ts @@ -999,7 +999,6 @@ describe('OptionsListUtils', () => { recentReportOptions: OPTIONS.reports, personalDetailsOptions: OPTIONS.personalDetails, currentUserOption: null, - excludeUnknownUsers: false, }); expect(canCreate).toBe(false); From 77e222442a3d236b378907573cd5da9df7034a97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Mon, 2 Dec 2024 12:43:23 +0100 Subject: [PATCH 10/25] clean: order functions --- src/libs/OptionsListUtils.ts | 54 ++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index abf9d9c7d380..545469bcd503 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -1645,33 +1645,6 @@ function filterCurrentUserOption(currentUserOption: ReportUtils.OptionData | nul }, currentUserOption); } -function filterOptions(options: Options, searchInputValue: string, config?: FilterOptionsConfig): Options { - const parsedPhoneNumber = PhoneNumber.parsePhoneNumber(LoginUtils.appendCountryCode(Str.removeSMSDomain(searchInputValue))); - const searchValue = parsedPhoneNumber.possible && parsedPhoneNumber.number?.e164 ? parsedPhoneNumber.number.e164 : searchInputValue.toLowerCase(); - const searchTerms = searchValue ? searchValue.split(' ') : []; - - const recentReports = filterReports(options.recentReports, searchTerms); - const personalDetails = filterPersonalDetails(options.personalDetails, searchTerms); - const currentUserOption = filterCurrentUserOption(options.currentUserOption, searchTerms); - const userToInvite = filterUserToInvite( - { - recentReports, - personalDetails, - currentUserOption, - userToInvite: null, - }, - searchValue, - config, - ); - - return { - personalDetails, - recentReports, - userToInvite, - currentUserOption, - }; -} - function filterUserToInvite(options: Options, searchValue: string, config?: FilterOptionsConfig): ReportUtils.OptionData | null { const {canInviteUser = true, excludeLogins = []} = config ?? {}; if (!canInviteUser) { @@ -1700,6 +1673,33 @@ function filterUserToInvite(options: Options, searchValue: string, config?: Filt }); } +function filterOptions(options: Options, searchInputValue: string, config?: FilterOptionsConfig): Options { + const parsedPhoneNumber = PhoneNumber.parsePhoneNumber(LoginUtils.appendCountryCode(Str.removeSMSDomain(searchInputValue))); + const searchValue = parsedPhoneNumber.possible && parsedPhoneNumber.number?.e164 ? parsedPhoneNumber.number.e164 : searchInputValue.toLowerCase(); + const searchTerms = searchValue ? searchValue.split(' ') : []; + + const recentReports = filterReports(options.recentReports, searchTerms); + const personalDetails = filterPersonalDetails(options.personalDetails, searchTerms); + const currentUserOption = filterCurrentUserOption(options.currentUserOption, searchTerms); + const userToInvite = filterUserToInvite( + { + recentReports, + personalDetails, + currentUserOption, + userToInvite: null, + }, + searchValue, + config, + ); + + return { + personalDetails, + recentReports, + userToInvite, + currentUserOption, + }; +} + type FilterAndOrderConfig = FilterOptionsConfig & OrderOptionsConfig; function filterAndOrderOptions(options: Options, searchInputValue: string, config: FilterAndOrderConfig = {}): Options { const {maxRecentReportsToShow = 0, sortByReportTypeInSearch = false} = config; From 08d11873c70b6fa57ee99ec4b5bf9c133585f689 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Mon, 2 Dec 2024 13:40:54 +0100 Subject: [PATCH 11/25] cleanup configs --- src/libs/OptionsListUtils.ts | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index 545469bcd503..2c7ba30df33c 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -107,7 +107,6 @@ type GetOptionsConfig = { includeInvoiceRooms?: boolean; includeDomainEmail?: boolean; action?: IOUAction; - shouldAcceptName?: boolean; recentAttendees?: Attendee[]; shouldBoldTitleByDefault?: boolean; }; @@ -115,11 +114,9 @@ type GetOptionsConfig = { type GetUserToInviteConfig = { searchValue: string; optionsToExclude?: Array>; - selectedOptions?: Array>; reportActions?: ReportActions; - showChatPreviewLine?: boolean; shouldAcceptName?: boolean; -}; +} & Pick; type MemberForList = { text: string; @@ -146,14 +143,15 @@ type Options = { type PreviewConfig = {showChatPreviewLine?: boolean; forcePolicyNamePreview?: boolean; showPersonalDetails?: boolean}; -type FilterOptionsConfig = Pick & { - /* When sortByReportTypeInSearch flag is true, recentReports will include the personalDetails options as well. */ - sortByReportTypeInSearch?: boolean; - maxRecentReportsToShow?: number; +type FilterUserToInviteConfig = Pick & { canInviteUser?: boolean; + excludeLogins?: string[]; }; type OrderOptionsConfig = { + /* When sortByReportTypeInSearch flag is true, recentReports will include the personalDetails options as well. */ + sortByReportTypeInSearch?: boolean; + maxRecentReportsToShow?: number; preferChatroomsOverThreads?: boolean; preferPolicyExpenseChat?: boolean; preferRecentExpenseReports?: boolean; @@ -1645,7 +1643,7 @@ function filterCurrentUserOption(currentUserOption: ReportUtils.OptionData | nul }, currentUserOption); } -function filterUserToInvite(options: Options, searchValue: string, config?: FilterOptionsConfig): ReportUtils.OptionData | null { +function filterUserToInvite(options: Omit, searchValue: string, config?: FilterUserToInviteConfig): ReportUtils.OptionData | null { const {canInviteUser = true, excludeLogins = []} = config ?? {}; if (!canInviteUser) { return null; @@ -1668,12 +1666,12 @@ function filterUserToInvite(options: Options, searchValue: string, config?: Filt return getUserToInviteOption({ searchValue, - selectedOptions: config?.selectedOptions, optionsToExclude, + ...config, }); } -function filterOptions(options: Options, searchInputValue: string, config?: FilterOptionsConfig): Options { +function filterOptions(options: Options, searchInputValue: string, config?: FilterUserToInviteConfig): Options { const parsedPhoneNumber = PhoneNumber.parsePhoneNumber(LoginUtils.appendCountryCode(Str.removeSMSDomain(searchInputValue))); const searchValue = parsedPhoneNumber.possible && parsedPhoneNumber.number?.e164 ? parsedPhoneNumber.number.e164 : searchInputValue.toLowerCase(); const searchTerms = searchValue ? searchValue.split(' ') : []; @@ -1686,7 +1684,6 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt recentReports, personalDetails, currentUserOption, - userToInvite: null, }, searchValue, config, @@ -1700,7 +1697,7 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt }; } -type FilterAndOrderConfig = FilterOptionsConfig & OrderOptionsConfig; +type FilterAndOrderConfig = FilterUserToInviteConfig & OrderOptionsConfig; function filterAndOrderOptions(options: Options, searchInputValue: string, config: FilterAndOrderConfig = {}): Options { const {maxRecentReportsToShow = 0, sortByReportTypeInSearch = false} = config; From a75bf95c7d624734d8c9f5c2add029f33dbdcc39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Mon, 2 Dec 2024 14:48:02 +0100 Subject: [PATCH 12/25] fix ordering --- src/libs/OptionsListUtils.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index 2c7ba30df33c..d0fe15c79967 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -934,7 +934,6 @@ function orderReportOptions( return lodashOrderBy( options, [ - (option) => option?.lastVisibleActionCreated ?? 0, (option) => { if (option.isPolicyExpenseChat && preferPolicyExpenseChat && option.policyID === activePolicyID) { return 0; @@ -965,11 +964,17 @@ function orderReportOptions( // When option.login is an exact match with the search value, returning 0 puts it at the top of the option list return 0; }, + (option) => { + if (option.private_isArchived) { + return CONST.DATE.UNIX_EPOCH; + } + return option?.lastVisibleActionCreated ?? ''; + }, // For Submit Expense flow, prioritize the most recent expense reports and then policy expense chats (without expense requests) preferRecentExpenseReports ? (option) => option?.lastIOUCreationDate ?? '' : '', preferRecentExpenseReports ? (option) => option?.isPolicyExpenseChat : 0, ], - ['desc', 'asc', 'desc', 'desc'], + ['asc', 'desc', 'desc', 'desc'], ); } From 2eaeaf42ed7696ff3dc98b34567ee17be4cd2f5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Wed, 4 Dec 2024 10:57:29 +0100 Subject: [PATCH 13/25] fix ordering when `maxRecentReportsToShow` option is set (added test back) --- src/libs/OptionsListUtils.ts | 30 ++++++++++--------- tests/unit/OptionsListUtilsTest.ts | 46 ++++++++++++------------------ 2 files changed, 35 insertions(+), 41 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index d0fe15c79967..ce7d10581720 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -1719,35 +1719,37 @@ function filterAndOrderOptions(options: Options, searchInputValue: string, confi }; } - const filteredOptions = filterOptions(options, searchInputValue, config); - const personalDetailsWithoutDMs = filteredPersonalDetailsOfRecentReports(filteredOptions.recentReports, filteredOptions.personalDetails); + const filterResult = filterOptions(options, searchInputValue, config); + let {recentReports: filteredReports, personalDetails: filteredPersonalDetails} = filterResult; + + if (config?.maxRecentReportsToShow) { + filteredReports = orderReportOptions(filteredReports, searchInputValue, config); + filteredReports = filteredReports.slice(0, config.maxRecentReportsToShow); + } + + const personalDetailsWithoutDMs = filteredPersonalDetailsOfRecentReports(filteredReports, filteredPersonalDetails); // sortByReportTypeInSearch option will show the personal details as part of the recent reports - let filteredPersonalDetails: ReportUtils.OptionData[] = personalDetailsWithoutDMs; - let filteredRecentReports: ReportUtils.OptionData[] = filteredOptions.recentReports; if (sortByReportTypeInSearch) { - filteredRecentReports = filteredOptions.recentReports.concat(personalDetailsWithoutDMs); + filteredReports = filteredReports.concat(personalDetailsWithoutDMs); filteredPersonalDetails = []; + } else { + filteredPersonalDetails = personalDetailsWithoutDMs; } const orderedOptions = orderOptions( { - recentReports: filteredRecentReports, + recentReports: filteredReports, personalDetails: filteredPersonalDetails, }, searchInputValue, config, ); - let orderedReports = orderedOptions.recentReports; - if (config?.maxRecentReportsToShow) { - orderedReports = orderedReports.slice(0, config.maxRecentReportsToShow); - } - return { - ...filteredOptions, - recentReports: orderedReports, - personalDetails: orderedOptions.personalDetails, + ...orderedOptions, + userToInvite: filterResult.userToInvite, + currentUserOption: filterResult.currentUserOption, }; } diff --git a/tests/unit/OptionsListUtilsTest.ts b/tests/unit/OptionsListUtilsTest.ts index 4c5c09431815..c2cc698d60c9 100644 --- a/tests/unit/OptionsListUtilsTest.ts +++ b/tests/unit/OptionsListUtilsTest.ts @@ -413,7 +413,7 @@ describe('OptionsListUtils', () => { }); it('orderOptions()', () => { - // When we call getOptions() with no search value + // When we call getValidOptions() with no search value let results: Pick = OptionsListUtils.getValidOptions({ reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails, @@ -451,7 +451,7 @@ describe('OptionsListUtils', () => { expect(results.personalDetails.at(3)?.text).toBe('Invisible Woman'); }); - it('getOptions()', () => { + it('getValidOptions()', () => { // When we don't include personal detail to the result let results = OptionsListUtils.getValidOptions({ personalDetails: [], @@ -512,8 +512,8 @@ describe('OptionsListUtils', () => { expect(results.personalDetails).not.toEqual(expect.arrayContaining([expect.objectContaining({login: 'receipts@expensify.com'})])); }); - it('getOptions() for group Chat', () => { - // When we call getOptions() with no search value + it('getValidOptions() for group Chat', () => { + // When we call getValidOptions() with no search value let results = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); // We should expect all the personalDetails to show except the currently logged in user @@ -525,15 +525,7 @@ describe('OptionsListUtils', () => { const personalDetailsOverlapWithReports = results.personalDetails.every((personalDetailOption) => reportLogins.includes(personalDetailOption.login)); expect(personalDetailsOverlapWithReports).toBe(false); - // When we limit getOptions() - results = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); - - // Then one of our older report options (not in our five most recent) should appear in the personalDetails - // but not in recentReports - // expect(results.recentReports.every((option) => option.login !== 'peterparker@expensify.com')).toBe(true); - // expect(results.personalDetails.every((option) => option.login !== 'peterparker@expensify.com')).toBe(false); - - // When we provide a "selected" option to getOptions() + // When we provide a "selected" option to getValidOptions() results = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}, {excludeLogins: ['peterparker@expensify.com']}); // Then the option should not appear anywhere in either list @@ -658,7 +650,7 @@ describe('OptionsListUtils', () => { expect(formattedMembers.every((personalDetail) => !personalDetail.isDisabled)).toBe(true); }); - describe('filterOptions', () => { + describe('filterAndOrderOptions', () => { it('should return all options when search is empty', () => { const options = OptionsListUtils.getSearchOptions(OPTIONS, [CONST.BETAS.ALL]); const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, ''); @@ -862,7 +854,7 @@ describe('OptionsListUtils', () => { expect(filteredOptions.recentReports.length).toBe(0); }); - it('should show the option from personal details when searching for personal detail with no existing report (getOptions)', () => { + it('should show the option from personal details when searching for personal detail with no existing report', () => { const options = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, 'hulk'); @@ -872,7 +864,7 @@ describe('OptionsListUtils', () => { expect(filteredOptions.personalDetails.at(0)?.login).toBe('brucebanner@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 (getOptions)', () => { + 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', () => { const options = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, 'marc@expensify'); @@ -881,7 +873,7 @@ describe('OptionsListUtils', () => { 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 (getOptions)', () => { + 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', () => { const options = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, 'marc@expensify.com'); @@ -890,7 +882,7 @@ describe('OptionsListUtils', () => { 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 (getOptions)', () => { + it('should return user to invite when search term has a period with options for it that do not contain the period', () => { const options = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, 'peter.parker@expensify.com'); @@ -898,7 +890,7 @@ describe('OptionsListUtils', () => { 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 (getOptions)', () => { + 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', () => { const options = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, '5005550006'); @@ -908,7 +900,7 @@ describe('OptionsListUtils', () => { 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 (getOptions)', () => { + 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', () => { const options = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, '+15005550006'); @@ -918,7 +910,7 @@ describe('OptionsListUtils', () => { 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 (getOptions)', () => { + 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', () => { const options = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, '+1 (800)324-3233'); @@ -928,7 +920,7 @@ describe('OptionsListUtils', () => { expect(filteredOptions.userToInvite?.login).toBe('+18003243233'); }); - it('should not return any options or user to invite if contact number contains alphabet characters (getOptions)', () => { + it('should not return any options or user to invite if contact number contains alphabet characters', () => { const options = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, '998243aaaa'); @@ -937,14 +929,14 @@ describe('OptionsListUtils', () => { expect(filteredOptions.userToInvite).toBe(null); }); - it('should not return any options if search value does not match any personal details (getOptions)', () => { + it('should not return any options if search value does not match any personal details', () => { const options = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterAndOrderOptions(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 (getOptions)', () => { + it('should return one recent report and no personal details if a search value provides an email', () => { const options = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, 'peterparker@expensify.com', {sortByReportTypeInSearch: true}); expect(filteredOptions.recentReports.length).toBe(1); @@ -952,13 +944,13 @@ describe('OptionsListUtils', () => { expect(filteredOptions.personalDetails.length).toBe(0); }); - it('should return all matching reports and personal details (getOptions)', () => { + it('should return all matching reports and personal details', () => { const options = OptionsListUtils.getValidOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, '.com', {maxRecentReportsToShow: 5}); - expect(filteredOptions.personalDetails.length).toBe(2); - expect(filteredOptions.recentReports.length).toBe(5); + expect(filteredOptions.personalDetails.length).toBe(4); expect(filteredOptions.personalDetails.at(0)?.login).toBe('natasharomanoff@expensify.com'); + expect(filteredOptions.recentReports.length).toBe(5); expect(filteredOptions.recentReports.at(0)?.text).toBe('Captain America'); expect(filteredOptions.recentReports.at(1)?.text).toBe('Mr Sinister'); expect(filteredOptions.recentReports.at(2)?.text).toBe('Black Panther'); From 8c98f84c12cb8b4189f78778115388b5c0a7b10d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Wed, 4 Dec 2024 11:12:07 +0100 Subject: [PATCH 14/25] remove maxRecentOptionsToShow --- src/components/Search/SearchFiltersChatsSelector.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/src/components/Search/SearchFiltersChatsSelector.tsx b/src/components/Search/SearchFiltersChatsSelector.tsx index c1297480b3c7..30c202295c87 100644 --- a/src/components/Search/SearchFiltersChatsSelector.tsx +++ b/src/components/Search/SearchFiltersChatsSelector.tsx @@ -69,7 +69,6 @@ function SearchFiltersChatsSelector({initialReportIDs, onFiltersUpdate, isScreen return OptionsListUtils.filterAndOrderOptions(defaultOptions, cleanSearchTerm, { selectedOptions, excludeLogins: CONST.EXPENSIFY_EMAILS, - maxRecentReportsToShow: 0, }); }, [defaultOptions, cleanSearchTerm, selectedOptions]); From 9e37f36cb6a458d0f3bb84e1347836371e70e744 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Wed, 4 Dec 2024 11:18:39 +0100 Subject: [PATCH 15/25] add test to verify behaviour of maxRecentReportsToShow: 0 --- src/libs/OptionsListUtils.ts | 5 +++++ tests/unit/OptionsListUtilsTest.ts | 10 ++++++++++ 2 files changed, 15 insertions(+) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index ce7d10581720..93d197b1f9c6 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -1703,6 +1703,11 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt } type FilterAndOrderConfig = FilterUserToInviteConfig & OrderOptionsConfig; + +/** + * Filters and orders the options based on the search input value. + * Note that personal details that are part of the recent reports will always be shown as part of the recent reports (ie. DMs). + */ function filterAndOrderOptions(options: Options, searchInputValue: string, config: FilterAndOrderConfig = {}): Options { const {maxRecentReportsToShow = 0, sortByReportTypeInSearch = false} = config; diff --git a/tests/unit/OptionsListUtilsTest.ts b/tests/unit/OptionsListUtilsTest.ts index c2cc698d60c9..b11f6dfd244c 100644 --- a/tests/unit/OptionsListUtilsTest.ts +++ b/tests/unit/OptionsListUtilsTest.ts @@ -983,6 +983,16 @@ describe('OptionsListUtils', () => { expect(filteredResults.recentReports.at(0)?.text).toBe('The Flash'); }); }); + + it('should return all results when setting maxRecentReportsToShow to 0', () => { + const options = OptionsListUtils.getSearchOptions(OPTIONS); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, '', {maxRecentReportsToShow: 0}); + + expect(filteredOptions.recentReports.length).toBe(10); + // There are only two personal details that have no reports. Personal details will be + // excluded from the list if they have reports (DMs). + expect(filteredOptions.personalDetails.length).toBe(2); + }); }); describe('canCreateOptimisticPersonalDetailOption', () => { From 1e7a3b4f96b04c6d577246355a8c1b8b5c0db0e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Wed, 4 Dec 2024 11:20:29 +0100 Subject: [PATCH 16/25] simplify into one var --- src/libs/OptionsListUtils.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index 93d197b1f9c6..af2564604b69 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -1160,10 +1160,9 @@ function getValidOptions( return true; }); - const havingLoginPersonalDetails = includeP2P + const allPersonalDetailsOptions = includeP2P ? options.personalDetails.filter((detail) => !!detail?.login && !!detail.accountID && !detail?.isOptimisticPersonalDetail && (includeDomainEmail || !Str.isDomainEmail(detail.login))) : []; - const allPersonalDetailsOptions = havingLoginPersonalDetails; const optionsToExclude: Option[] = [{login: CONST.EMAIL.NOTIFICATIONS}]; From e26cccbf2393660dd00a215bf7f12dadef288529 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Wed, 4 Dec 2024 13:57:11 +0100 Subject: [PATCH 17/25] fix ordering --- src/libs/OptionsListUtils.ts | 9 +++++++-- tests/unit/OptionsListUtilsTest.ts | 10 +++++----- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index af2564604b69..c15db8b256cc 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -968,7 +968,12 @@ function orderReportOptions( if (option.private_isArchived) { return CONST.DATE.UNIX_EPOCH; } - return option?.lastVisibleActionCreated ?? ''; + + if (searchValue) { + return [option.isSelfDM ?? false, option?.lastVisibleActionCreated]; + } + + return option?.lastVisibleActionCreated; }, // For Submit Expense flow, prioritize the most recent expense reports and then policy expense chats (without expense requests) preferRecentExpenseReports ? (option) => option?.lastIOUCreationDate ?? '' : '', @@ -1741,7 +1746,7 @@ function filterAndOrderOptions(options: Options, searchInputValue: string, confi filteredPersonalDetails = personalDetailsWithoutDMs; } - const orderedOptions = orderOptions( + const orderedOptions = orderOptions( { recentReports: filteredReports, personalDetails: filteredPersonalDetails, diff --git a/tests/unit/OptionsListUtilsTest.ts b/tests/unit/OptionsListUtilsTest.ts index b11f6dfd244c..c7d276071a09 100644 --- a/tests/unit/OptionsListUtilsTest.ts +++ b/tests/unit/OptionsListUtilsTest.ts @@ -664,17 +664,17 @@ describe('OptionsListUtils', () => { const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, searchText, {sortByReportTypeInSearch: true}); - // When sortByReportTypeInSearch is true, we expect all options to be part of the recentReports list: + // When sortByReportTypeInSearch is true, we expect all options to be part of the recentReports list and reports should be first: expect(filteredOptions.personalDetails.length).toBe(0); // Expect to only find reports that matched our search text: expect(filteredOptions.recentReports.length).toBe(4); // This items should be ordered by most recent action (and other criteria such as whether they are archived): - expect(filteredOptions.recentReports.at(0)?.text).toBe('Invisible Woman'); - expect(filteredOptions.recentReports.at(1)?.text).toBe('Spider-Man'); - expect(filteredOptions.recentReports.at(2)?.text).toBe('Black Widow'); - expect(filteredOptions.recentReports.at(3)?.text).toBe('Mister Fantastic, Invisible Woman'); + expect(filteredOptions.recentReports.at(0)?.text).toBe('Invisible Woman'); // '2022-11-22 03:26:02.019' + expect(filteredOptions.recentReports.at(1)?.text).toBe('Spider-Man'); // '2022-11-22 03:26:02.016' + expect(filteredOptions.recentReports.at(2)?.text).toBe('Black Widow'); // This is a personal detail, which has no lastVisibleActionCreated, but matches the login + expect(filteredOptions.recentReports.at(3)?.text).toBe('Mister Fantastic, Invisible Woman'); // This again is a report with '2022-11-22 03:26:02.015' }); it('should filter users by email', () => { From 5373208ad442b154c6af8de060041a379e2458e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Wed, 4 Dec 2024 17:29:37 +0100 Subject: [PATCH 18/25] fix ordering logic --- src/libs/OptionsListUtils.ts | 64 +++++++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 20 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index c15db8b256cc..eb8cf991b8b6 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -921,19 +921,30 @@ function orderPersonalDetailsOptions(options: ReportUtils.OptionData[]) { } /** - * Report Options need to be sorted in the specific order + * Orders report options without grouping them by kind. + * Usually used when there is no search value + */ +function orderReportOptions(options: ReportUtils.OptionData[]) { + return lodashOrderBy(options, [sortComparatorReportOptionByArchivedStatus, sortComparatorReportOptionByDate], ['asc', 'desc']); +} + +/** + * Ordering for report options when you have a search value, will order them by kind additionally. * @param options - list of options to be sorted * @param searchValue - search string * @returns a sorted list of options */ -function orderReportOptions( +function orderReportOptionsWithSearch( options: ReportUtils.OptionData[], - searchValue: string | undefined, + searchValue: string, {preferChatroomsOverThreads = false, preferPolicyExpenseChat = false, preferRecentExpenseReports = false}: OrderOptionsConfig = {}, ) { return lodashOrderBy( options, [ + // Archived reports should always be at the bottom: + sortComparatorReportOptionByArchivedStatus, + // Sorting by kind: (option) => { if (option.isPolicyExpenseChat && preferPolicyExpenseChat && option.policyID === activePolicyID) { return 0; @@ -951,7 +962,7 @@ function orderReportOptions( if (preferChatroomsOverThreads && option.isThread) { return 4; } - if (!!option.isChatRoom || option.private_isArchived) { + if (option.isChatRoom) { return 3; } if (!option.login) { @@ -964,27 +975,33 @@ function orderReportOptions( // When option.login is an exact match with the search value, returning 0 puts it at the top of the option list return 0; }, - (option) => { - if (option.private_isArchived) { - return CONST.DATE.UNIX_EPOCH; - } - - if (searchValue) { - return [option.isSelfDM ?? false, option?.lastVisibleActionCreated]; - } - - return option?.lastVisibleActionCreated; - }, + // Within the same kind, sort by the last visible action created date + sortComparatorReportOptionByDate, // For Submit Expense flow, prioritize the most recent expense reports and then policy expense chats (without expense requests) preferRecentExpenseReports ? (option) => option?.lastIOUCreationDate ?? '' : '', preferRecentExpenseReports ? (option) => option?.isPolicyExpenseChat : 0, ], - ['asc', 'desc', 'desc', 'desc'], + ['asc', 'asc', 'desc', 'desc', 'desc'], ); } +function sortComparatorReportOptionByArchivedStatus(option: ReportUtils.OptionData) { + return option.private_isArchived ? 1 : 0; +} + +function sortComparatorReportOptionByDate(options: ReportUtils.OptionData) { + // If there is no date (ie. a personal detail option), the option will be sorted to the bottom + // (comparing a dateString > '' returns true, and we are sorting descending, so the dateString will come before '') + return options.lastVisibleActionCreated ?? ''; +} + function orderOptions(options: Pick, searchValue?: string, config?: OrderOptionsConfig) { - const orderedReportOptions = orderReportOptions(options.recentReports, searchValue, config); + let orderedReportOptions: ReportUtils.OptionData[]; + if (searchValue) { + orderedReportOptions = orderReportOptionsWithSearch(options.recentReports, searchValue, config); + } else { + orderedReportOptions = orderReportOptions(options.recentReports); + } const orderedPersonalDetailsOptions = orderPersonalDetailsOptions(options.personalDetails); return { @@ -1294,6 +1311,7 @@ function getSearchOptions(options: OptionList, betas: Beta[] = [], isUsedInChatF Performance.markStart(CONST.TIMING.LOAD_SEARCH_OPTIONS); const optionList = getValidOptions(options, { betas, + includeRecentReports: true, includeMultipleParticipantReports: true, showChatPreviewLine: isUsedInChatFinder, includeP2P: true, @@ -1305,10 +1323,14 @@ function getSearchOptions(options: OptionList, betas: Beta[] = [], isUsedInChatF includeSelfDM: true, shouldBoldTitleByDefault: !isUsedInChatFinder, }); + const orderedOptions = orderOptions(optionList); Timing.end(CONST.TIMING.LOAD_SEARCH_OPTIONS); Performance.markEnd(CONST.TIMING.LOAD_SEARCH_OPTIONS); - return optionList; + return { + ...optionList, + ...orderedOptions, + }; } function getShareLogOptions(options: OptionList, betas: Beta[] = []): Options { @@ -1715,6 +1737,7 @@ type FilterAndOrderConfig = FilterUserToInviteConfig & OrderOptionsConfig; function filterAndOrderOptions(options: Options, searchInputValue: string, config: FilterAndOrderConfig = {}): Options { const {maxRecentReportsToShow = 0, sortByReportTypeInSearch = false} = config; + // TODO: the fast route can maybe be removed, its quite identical with the rest of the code // Fast route: there's no search input value and we're limiting the number of recent reports to show if (searchInputValue.trim().length === 0 && maxRecentReportsToShow > 0) { const {personalDetails, recentReports} = orderOptions(options, searchInputValue, config); @@ -1732,7 +1755,7 @@ function filterAndOrderOptions(options: Options, searchInputValue: string, confi let {recentReports: filteredReports, personalDetails: filteredPersonalDetails} = filterResult; if (config?.maxRecentReportsToShow) { - filteredReports = orderReportOptions(filteredReports, searchInputValue, config); + filteredReports = orderReportOptionsWithSearch(filteredReports, searchInputValue, config); filteredReports = filteredReports.slice(0, config.maxRecentReportsToShow); } @@ -1746,7 +1769,7 @@ function filterAndOrderOptions(options: Options, searchInputValue: string, confi filteredPersonalDetails = personalDetailsWithoutDMs; } - const orderedOptions = orderOptions( + const orderedOptions = orderOptions( { recentReports: filteredReports, personalDetails: filteredPersonalDetails, @@ -1810,6 +1833,7 @@ export { filterOptions, filteredPersonalDetailsOfRecentReports, orderReportOptions, + orderReportOptionsWithSearch, orderPersonalDetailsOptions, orderOptions, filterAndOrderOptions, From c60f18dd4a9560e1c67923164b68073863ad14d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Wed, 4 Dec 2024 18:15:15 +0100 Subject: [PATCH 19/25] fix broken screens --- .../request/MoneyRequestAttendeeSelector.tsx | 4 +++- .../MoneyRequestParticipantsSelector.tsx | 17 ++++++++++------- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/pages/iou/request/MoneyRequestAttendeeSelector.tsx b/src/pages/iou/request/MoneyRequestAttendeeSelector.tsx index 7690e7bc0acf..db28fb46d165 100644 --- a/src/pages/iou/request/MoneyRequestAttendeeSelector.tsx +++ b/src/pages/iou/request/MoneyRequestAttendeeSelector.tsx @@ -85,11 +85,13 @@ function MoneyRequestAttendeeSelector({attendees = [], onFinish, onAttendeesAdde action, ); if (isPaidGroupPolicy) { - optionList.recentReports = OptionsListUtils.orderReportOptions(optionList.recentReports, searchTerm, { + const orderedOptions = OptionsListUtils.orderOptions(optionList, searchTerm, { preferChatroomsOverThreads: true, preferPolicyExpenseChat: !!action, preferRecentExpenseReports: action === CONST.IOU.ACTION.CREATE, }); + optionList.recentReports = orderedOptions.recentReports; + optionList.personalDetails = orderedOptions.personalDetails; } if (optionList.currentUserOption && !isCurrentUserAttendee) { optionList.recentReports = [optionList.currentUserOption, ...optionList.personalDetails]; diff --git a/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx b/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx index b491d4c0fb02..5031b0a04140 100644 --- a/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx +++ b/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx @@ -126,15 +126,18 @@ function MoneyRequestParticipantsSelector({ }, ); - if (isPaidGroupPolicy) { - optionList.recentReports = OptionsListUtils.orderReportOptions(optionList.recentReports, undefined, { - preferChatroomsOverThreads: true, - preferPolicyExpenseChat: !!action, - preferRecentExpenseReports: action === CONST.IOU.ACTION.CREATE, - }); + if (!isPaidGroupPolicy) { + return optionList; } - return optionList; + const orderedOptions = OptionsListUtils.orderOptions(optionList, undefined, { + sortByReportTypeInSearch: true, + }); + + return { + ...optionList, + ...orderedOptions, + }; }, [action, areOptionsInitialized, betas, didScreenTransitionEnd, iouType, isCategorizeOrShareAction, isPaidGroupPolicy, options.personalDetails, options.reports, participants]); const chatOptions = useMemo(() => { From ec8df5e6a641dd26c5832e0ec4c4aaa12e4831a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Wed, 4 Dec 2024 19:43:38 +0100 Subject: [PATCH 20/25] remove slightly redundant code --- src/libs/OptionsListUtils.ts | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index eb8cf991b8b6..5e31f20eee82 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -1735,23 +1735,13 @@ type FilterAndOrderConfig = FilterUserToInviteConfig & OrderOptionsConfig; * Note that personal details that are part of the recent reports will always be shown as part of the recent reports (ie. DMs). */ function filterAndOrderOptions(options: Options, searchInputValue: string, config: FilterAndOrderConfig = {}): Options { - const {maxRecentReportsToShow = 0, sortByReportTypeInSearch = false} = config; + const {sortByReportTypeInSearch = false} = config; - // TODO: the fast route can maybe be removed, its quite identical with the rest of the code - // Fast route: there's no search input value and we're limiting the number of recent reports to show - if (searchInputValue.trim().length === 0 && maxRecentReportsToShow > 0) { - const {personalDetails, recentReports} = orderOptions(options, searchInputValue, config); - const limitedRecentReports = recentReports.slice(0, maxRecentReportsToShow); - const filteredPersonalDetails = filteredPersonalDetailsOfRecentReports(limitedRecentReports, personalDetails); - - return { - ...options, - personalDetails: filteredPersonalDetails, - recentReports: limitedRecentReports, - }; + let filterResult = options; + if (searchInputValue.trim().length > 0) { + filterResult = filterOptions(options, searchInputValue, config); } - const filterResult = filterOptions(options, searchInputValue, config); let {recentReports: filteredReports, personalDetails: filteredPersonalDetails} = filterResult; if (config?.maxRecentReportsToShow) { From e369474b1692d1881fbc8be2c3b92ef930c50a72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Fri, 6 Dec 2024 11:03:11 +0100 Subject: [PATCH 21/25] use `filterAndOrderOptions` --- src/components/Search/SearchRouter/SearchRouterList.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/Search/SearchRouter/SearchRouterList.tsx b/src/components/Search/SearchRouter/SearchRouterList.tsx index 81d5aee84ff0..3fe7cc9e2de4 100644 --- a/src/components/Search/SearchRouter/SearchRouterList.tsx +++ b/src/components/Search/SearchRouter/SearchRouterList.tsx @@ -378,7 +378,7 @@ function SearchRouterList( } Timing.start(CONST.TIMING.SEARCH_FILTER_OPTIONS); - const filteredOptions = OptionsListUtils.filterOptions(searchOptions, autocompleteQueryValue, {sortByReportTypeInSearch: true, preferChatroomsOverThreads: true}); + const filteredOptions = OptionsListUtils.filterAndOrderOptions(searchOptions, autocompleteQueryValue, {sortByReportTypeInSearch: true, preferChatroomsOverThreads: true}); Timing.end(CONST.TIMING.SEARCH_FILTER_OPTIONS); const reportOptions: OptionData[] = [...filteredOptions.recentReports, ...filteredOptions.personalDetails]; From 3285c33d4d6fdcd42905d6ab7cb9c9de12a432e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Fri, 6 Dec 2024 12:39:22 +0100 Subject: [PATCH 22/25] potentially fix search --- src/pages/iou/request/MoneyRequestParticipantsSelector.tsx | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx b/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx index 5031b0a04140..a4dfe26b6a86 100644 --- a/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx +++ b/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx @@ -126,12 +126,8 @@ function MoneyRequestParticipantsSelector({ }, ); - if (!isPaidGroupPolicy) { - return optionList; - } - const orderedOptions = OptionsListUtils.orderOptions(optionList, undefined, { - sortByReportTypeInSearch: true, + sortByReportTypeInSearch: isPaidGroupPolicy, }); return { From dfa6fd6347d945c4a1190d3f9001901360617ab3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Fri, 6 Dec 2024 12:58:17 +0100 Subject: [PATCH 23/25] maxRecentReportsToShow: 0 should return zero reports --- src/libs/OptionsListUtils.ts | 2 +- tests/unit/OptionsListUtilsTest.ts | 14 ++++---------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index 5e31f20eee82..899e6452d131 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -1744,7 +1744,7 @@ function filterAndOrderOptions(options: Options, searchInputValue: string, confi let {recentReports: filteredReports, personalDetails: filteredPersonalDetails} = filterResult; - if (config?.maxRecentReportsToShow) { + if (typeof config?.maxRecentReportsToShow === 'number') { filteredReports = orderReportOptionsWithSearch(filteredReports, searchInputValue, config); filteredReports = filteredReports.slice(0, config.maxRecentReportsToShow); } diff --git a/tests/unit/OptionsListUtilsTest.ts b/tests/unit/OptionsListUtilsTest.ts index c7d276071a09..8916b7c3bac8 100644 --- a/tests/unit/OptionsListUtilsTest.ts +++ b/tests/unit/OptionsListUtilsTest.ts @@ -781,7 +781,11 @@ describe('OptionsListUtils', () => { const options = OptionsListUtils.getSearchOptions(OPTIONS); const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, searchText, {maxRecentReportsToShow: 2}); + // Note: in the past maxRecentReportsToShow: 0 would return all recent reports, this has changed, and is expected to return none now + const limitToZeroOptions = OptionsListUtils.filterAndOrderOptions(options, searchText, {maxRecentReportsToShow: 0}); + expect(filteredOptions.recentReports.length).toBe(2); + expect(limitToZeroOptions.recentReports.length).toBe(0); }); it('should not return any user to invite if email exists on the personal details list', () => { @@ -983,16 +987,6 @@ describe('OptionsListUtils', () => { expect(filteredResults.recentReports.at(0)?.text).toBe('The Flash'); }); }); - - it('should return all results when setting maxRecentReportsToShow to 0', () => { - const options = OptionsListUtils.getSearchOptions(OPTIONS); - const filteredOptions = OptionsListUtils.filterAndOrderOptions(options, '', {maxRecentReportsToShow: 0}); - - expect(filteredOptions.recentReports.length).toBe(10); - // There are only two personal details that have no reports. Personal details will be - // excluded from the list if they have reports (DMs). - expect(filteredOptions.personalDetails.length).toBe(2); - }); }); describe('canCreateOptimisticPersonalDetailOption', () => { From ee50f312740fa0a0942556987d17cab869fab967 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Mon, 9 Dec 2024 14:20:36 +0100 Subject: [PATCH 24/25] fix ordering by making it two separate order function calls as before --- src/libs/OptionsListUtils.ts | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index 899e6452d131..eb78cdaa4f3e 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -939,11 +939,11 @@ function orderReportOptionsWithSearch( searchValue: string, {preferChatroomsOverThreads = false, preferPolicyExpenseChat = false, preferRecentExpenseReports = false}: OrderOptionsConfig = {}, ) { + const orderedByDate = orderReportOptions(options); + return lodashOrderBy( - options, + orderedByDate, [ - // Archived reports should always be at the bottom: - sortComparatorReportOptionByArchivedStatus, // Sorting by kind: (option) => { if (option.isPolicyExpenseChat && preferPolicyExpenseChat && option.policyID === activePolicyID) { @@ -962,7 +962,7 @@ function orderReportOptionsWithSearch( if (preferChatroomsOverThreads && option.isThread) { return 4; } - if (option.isChatRoom) { + if (!!option.isChatRoom || option.private_isArchived) { return 3; } if (!option.login) { @@ -975,13 +975,11 @@ function orderReportOptionsWithSearch( // When option.login is an exact match with the search value, returning 0 puts it at the top of the option list return 0; }, - // Within the same kind, sort by the last visible action created date - sortComparatorReportOptionByDate, // For Submit Expense flow, prioritize the most recent expense reports and then policy expense chats (without expense requests) preferRecentExpenseReports ? (option) => option?.lastIOUCreationDate ?? '' : '', preferRecentExpenseReports ? (option) => option?.isPolicyExpenseChat : 0, ], - ['asc', 'asc', 'desc', 'desc', 'desc'], + ['asc', 'desc', 'desc'], ); } @@ -995,7 +993,11 @@ function sortComparatorReportOptionByDate(options: ReportUtils.OptionData) { return options.lastVisibleActionCreated ?? ''; } -function orderOptions(options: Pick, searchValue?: string, config?: OrderOptionsConfig) { +type ReportAndPersonalDetailOptions = Pick; + +function orderOptions(options: ReportAndPersonalDetailOptions): ReportAndPersonalDetailOptions; +function orderOptions(options: ReportAndPersonalDetailOptions, searchValue: string, config?: OrderOptionsConfig): ReportAndPersonalDetailOptions; +function orderOptions(options: ReportAndPersonalDetailOptions, searchValue?: string, config?: OrderOptionsConfig) { let orderedReportOptions: ReportUtils.OptionData[]; if (searchValue) { orderedReportOptions = orderReportOptionsWithSearch(options.recentReports, searchValue, config); @@ -1750,26 +1752,21 @@ function filterAndOrderOptions(options: Options, searchInputValue: string, confi } const personalDetailsWithoutDMs = filteredPersonalDetailsOfRecentReports(filteredReports, filteredPersonalDetails); + const orderedPersonalDetails = orderPersonalDetailsOptions(personalDetailsWithoutDMs); // sortByReportTypeInSearch option will show the personal details as part of the recent reports if (sortByReportTypeInSearch) { - filteredReports = filteredReports.concat(personalDetailsWithoutDMs); + filteredReports = filteredReports.concat(orderedPersonalDetails); filteredPersonalDetails = []; } else { - filteredPersonalDetails = personalDetailsWithoutDMs; + filteredPersonalDetails = orderedPersonalDetails; } - const orderedOptions = orderOptions( - { - recentReports: filteredReports, - personalDetails: filteredPersonalDetails, - }, - searchInputValue, - config, - ); + const orderedReports = orderReportOptionsWithSearch(filteredReports, searchInputValue, config); return { - ...orderedOptions, + recentReports: orderedReports, + personalDetails: filteredPersonalDetails, userToInvite: filterResult.userToInvite, currentUserOption: filterResult.currentUserOption, }; From 25a13520337bb2377661db19979053a0a69fffee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hanno=20J=2E=20G=C3=B6decke?= Date: Mon, 9 Dec 2024 14:25:51 +0100 Subject: [PATCH 25/25] call orderOptions --- src/pages/iou/request/MoneyRequestParticipantsSelector.tsx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx b/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx index a4dfe26b6a86..07e34d9692b1 100644 --- a/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx +++ b/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx @@ -126,15 +126,13 @@ function MoneyRequestParticipantsSelector({ }, ); - const orderedOptions = OptionsListUtils.orderOptions(optionList, undefined, { - sortByReportTypeInSearch: isPaidGroupPolicy, - }); + const orderedOptions = OptionsListUtils.orderOptions(optionList); return { ...optionList, ...orderedOptions, }; - }, [action, areOptionsInitialized, betas, didScreenTransitionEnd, iouType, isCategorizeOrShareAction, isPaidGroupPolicy, options.personalDetails, options.reports, participants]); + }, [action, areOptionsInitialized, betas, didScreenTransitionEnd, iouType, isCategorizeOrShareAction, options.personalDetails, options.reports, participants]); const chatOptions = useMemo(() => { if (!areOptionsInitialized) {