From a50a4df07eb05a3b8586e833a83f089a8955782f Mon Sep 17 00:00:00 2001 From: c3024 Date: Tue, 8 Oct 2024 19:46:33 +0530 Subject: [PATCH 01/13] move report deduplicating logic to filterOptions --- src/libs/OptionsListUtils.ts | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index 11d8385d538b..f01f75cda1d8 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -2073,11 +2073,6 @@ function getOptions( } } } - - // Add this login to the exclude list so it won't appear when we process the personal details - if (reportOption.login) { - optionsToExclude.push({login: reportOption.login}); - } } } @@ -2493,7 +2488,14 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt preferRecentExpenseReports = false, } = config ?? {}; if (searchInputValue.trim() === '' && maxRecentReportsToShow > 0) { - return {...options, recentReports: options.recentReports.slice(0, maxRecentReportsToShow)}; + const recentReports = options.recentReports.slice(0, maxRecentReportsToShow); + const excludedLogins = new Set(recentReports.map(report => report.login)); + const filteredPersonalDetails = options.personalDetails.filter(personalDetail => !excludedLogins.has(personalDetail.login)); + return { + ...options, + recentReports, + personalDetails: filteredPersonalDetails, + }; } const parsedPhoneNumber = PhoneNumber.parsePhoneNumber(LoginUtils.appendCountryCode(Str.removeSMSDomain(searchInputValue))); @@ -2570,6 +2572,11 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt recentReports.splice(maxRecentReportsToShow); } + const excludedLogins = new Set(recentReports.map(report => report.login)); + const filteredPersonalDetails = personalDetails.filter(personalDetail => !excludedLogins.has(personalDetail.login)); + personalDetails = filteredPersonalDetails; + + return { personalDetails, recentReports: orderOptions(recentReports, searchValue, {preferChatroomsOverThreads, preferPolicyExpenseChat, preferRecentExpenseReports}), From deb1ba14bf778c339f288779c2ff12111036c6c8 Mon Sep 17 00:00:00 2001 From: c3024 Date: Tue, 8 Oct 2024 21:02:04 +0530 Subject: [PATCH 02/13] prettier and lint --- src/libs/OptionsListUtils.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index f01f75cda1d8..a0adc60dd053 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -2489,8 +2489,8 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt } = config ?? {}; if (searchInputValue.trim() === '' && maxRecentReportsToShow > 0) { const recentReports = options.recentReports.slice(0, maxRecentReportsToShow); - const excludedLogins = new Set(recentReports.map(report => report.login)); - const filteredPersonalDetails = options.personalDetails.filter(personalDetail => !excludedLogins.has(personalDetail.login)); + const excludedLogins = new Set(recentReports.map((report) => report.login)); + const filteredPersonalDetails = options.personalDetails.filter((personalDetail) => !excludedLogins.has(personalDetail.login)); return { ...options, recentReports, @@ -2572,11 +2572,10 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt recentReports.splice(maxRecentReportsToShow); } - const excludedLogins = new Set(recentReports.map(report => report.login)); - const filteredPersonalDetails = personalDetails.filter(personalDetail => !excludedLogins.has(personalDetail.login)); + const excludedLogins = new Set(recentReports.map((report) => report.login)); + const filteredPersonalDetails = personalDetails.filter((personalDetail) => !excludedLogins.has(personalDetail.login)); personalDetails = filteredPersonalDetails; - return { personalDetails, recentReports: orderOptions(recentReports, searchValue, {preferChatroomsOverThreads, preferPolicyExpenseChat, preferRecentExpenseReports}), From ffd08a406f7bd9f3538cadf8f561d83ad816d38a Mon Sep 17 00:00:00 2001 From: c3024 Date: Tue, 8 Oct 2024 23:21:45 +0530 Subject: [PATCH 03/13] add missed filter case and fix tests --- src/libs/OptionsListUtils.ts | 5 +- tests/unit/OptionsListUtilsTest.ts | 88 +++++++++++++++++++----------- 2 files changed, 61 insertions(+), 32 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index a0adc60dd053..657ceafcec9d 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -2538,9 +2538,12 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt const currentUserOption = isSearchStringMatch(term, currentUserOptionSearchText) ? items.currentUserOption : null; + const excludedLogins = new Set(recentReports.map((report) => report.login)); + const filteredPersonalDetails = personalDetails.filter((personalDetail) => !excludedLogins.has(personalDetail.login)); + return { recentReports: recentReports ?? [], - personalDetails: personalDetails ?? [], + personalDetails: filteredPersonalDetails ?? [], userToInvite: null, currentUserOption, categoryOptions: [], diff --git a/tests/unit/OptionsListUtilsTest.ts b/tests/unit/OptionsListUtilsTest.ts index d94303b98124..5e26932b6f9b 100644 --- a/tests/unit/OptionsListUtilsTest.ts +++ b/tests/unit/OptionsListUtilsTest.ts @@ -404,9 +404,11 @@ describe('OptionsListUtils', () => { it('getSearchOptions()', () => { // When we filter in the Search view without providing a searchValue - const results = OptionsListUtils.getSearchOptions(OPTIONS, '', [CONST.BETAS.ALL]); - // Then the 2 personalDetails that don't have reports should be returned - expect(results.personalDetails.length).toBe(2); + let results = OptionsListUtils.getSearchOptions(OPTIONS, '', [CONST.BETAS.ALL]); + + // All 2 personalDetails (including those that have reports) should be returned + // Filtering of personalDetails that have reports is done in filterOptions + expect(results.personalDetails.length).toBe(9); // Then all of the reports should be shown including the archived rooms. expect(results.recentReports.length).toBe(Object.values(OPTIONS.reports).length); @@ -422,15 +424,25 @@ describe('OptionsListUtils', () => { // We should expect maximimum of 5 recent reports to be returned expect(results.recentReports.length).toBe(MAX_RECENT_REPORTS); - // We should expect all personalDetails to be returned, - // minus the currently logged in user and recent reports count - expect(results.personalDetails.length).toBe(Object.values(OPTIONS.personalDetails).length - 1 - MAX_RECENT_REPORTS); + // We should expect all personalDetails except the currently logged in user to be returned + // 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 Widow'); - expect(results.personalDetails.at(1)?.text).toBe('Invisible Woman'); - expect(results.personalDetails.at(2)?.text).toBe('Spider-Man'); - expect(results.personalDetails.at(3)?.text).toBe('The Incredible Hulk'); + 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'); + // expect(results.personalDetails.at(0)?.text).toBe('Black Widow'); + // expect(results.personalDetails.at(1)?.text).toBe('Invisible Woman'); + // expect(results.personalDetails.at(2)?.text).toBe('Spider-Man'); + // expect(results.personalDetails.at(3)?.text).toBe('The Incredible Hulk'); // Then the result which has an existing report should also have the reportID attached const personalDetailWithExistingReport = results.personalDetails.find((personalDetail) => personalDetail.login === 'peterparker@expensify.com'); @@ -481,29 +493,33 @@ describe('OptionsListUtils', () => { results = OptionsListUtils.getFilteredOptions(OPTIONS_WITH_CONCIERGE.reports, OPTIONS_WITH_CONCIERGE.personalDetails); // Concierge is included in the results by default. We should expect all the personalDetails to show - // (minus the 5 that are already showing and the currently logged in user) - expect(results.personalDetails.length).toBe(Object.values(OPTIONS_WITH_CONCIERGE.personalDetails).length - 1 - MAX_RECENT_REPORTS); + // (minus the currently logged in user) + // Filtering of personalDetails that have reports is done in filterOptions + expect(results.personalDetails.length).toBe(Object.values(OPTIONS_WITH_CONCIERGE.personalDetails).length - 1); expect(results.recentReports).toEqual(expect.arrayContaining([expect.objectContaining({login: 'concierge@expensify.com'})])); // Test by excluding Concierge from the results results = OptionsListUtils.getFilteredOptions(OPTIONS_WITH_CONCIERGE.reports, OPTIONS_WITH_CONCIERGE.personalDetails, [], '', [], [CONST.EMAIL.CONCIERGE]); // All the personalDetails should be returned minus the currently logged in user and Concierge - expect(results.personalDetails.length).toBe(Object.values(OPTIONS_WITH_CONCIERGE.personalDetails).length - 2 - MAX_RECENT_REPORTS); + // Filtering of personalDetails that have reports is done in filterOptions + expect(results.personalDetails.length).toBe(Object.values(OPTIONS_WITH_CONCIERGE.personalDetails).length - 2); expect(results.personalDetails).not.toEqual(expect.arrayContaining([expect.objectContaining({login: 'concierge@expensify.com'})])); // Test by excluding Chronos from the results results = OptionsListUtils.getFilteredOptions(OPTIONS_WITH_CHRONOS.reports, OPTIONS_WITH_CHRONOS.personalDetails, [], '', [], [CONST.EMAIL.CHRONOS]); // All the personalDetails should be returned minus the currently logged in user and Concierge - expect(results.personalDetails.length).toBe(Object.values(OPTIONS_WITH_CHRONOS.personalDetails).length - 2 - MAX_RECENT_REPORTS); + // Filtering of personalDetails that have reports is done in filterOptions + expect(results.personalDetails.length).toBe(Object.values(OPTIONS_WITH_CHRONOS.personalDetails).length - 2); expect(results.personalDetails).not.toEqual(expect.arrayContaining([expect.objectContaining({login: 'chronos@expensify.com'})])); // Test by excluding Receipts from the results results = OptionsListUtils.getFilteredOptions(OPTIONS_WITH_RECEIPTS.reports, OPTIONS_WITH_RECEIPTS.personalDetails, [], '', [], [CONST.EMAIL.RECEIPTS]); // All the personalDetails should be returned minus the currently logged in user and Concierge - expect(results.personalDetails.length).toBe(Object.values(OPTIONS_WITH_RECEIPTS.personalDetails).length - 2 - MAX_RECENT_REPORTS); + // Filtering of personalDetails that have reports is done in filterOptions + expect(results.personalDetails.length).toBe(Object.values(OPTIONS_WITH_RECEIPTS.personalDetails).length - 2); expect(results.personalDetails).not.toEqual(expect.arrayContaining([expect.objectContaining({login: 'receipts@expensify.com'})])); }); @@ -514,15 +530,21 @@ describe('OptionsListUtils', () => { // Then we should expect only a maxmimum of 5 recent reports to be returned expect(results.recentReports.length).toBe(5); - // And we should expect all the personalDetails to show (minus the 5 that are already - // showing and the currently logged in user) - expect(results.personalDetails.length).toBe(Object.values(OPTIONS.personalDetails).length - 6); + // And we should expect all the personalDetails to show except the currently logged in user + // 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 Widow'); - expect(results.personalDetails.at(1)?.text).toBe('Invisible Woman'); - expect(results.personalDetails.at(2)?.text).toBe('Spider-Man'); - expect(results.personalDetails.at(3)?.text).toBe('The Incredible Hulk'); + 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); @@ -548,34 +570,38 @@ describe('OptionsListUtils', () => { results = OptionsListUtils.getFilteredOptions(OPTIONS_WITH_CONCIERGE.reports, OPTIONS_WITH_CONCIERGE.personalDetails); // Concierge is included in the results by default. We should expect all the personalDetails to show - // (minus the 5 that are already showing and the currently logged in user) - expect(results.personalDetails.length).toBe(Object.values(OPTIONS_WITH_CONCIERGE.personalDetails).length - 6); + // (minus the currently logged in user) + // Filtering of personalDetails that have reports is done in filterOptions + expect(results.personalDetails.length).toBe(Object.values(OPTIONS_WITH_CONCIERGE.personalDetails).length - 1); expect(results.recentReports).toEqual(expect.arrayContaining([expect.objectContaining({login: 'concierge@expensify.com'})])); // Test by excluding Concierge from the results results = OptionsListUtils.getFilteredOptions(OPTIONS_WITH_CONCIERGE.reports, OPTIONS_WITH_CONCIERGE.personalDetails, [], '', [], [CONST.EMAIL.CONCIERGE]); - // We should expect all the personalDetails to show (minus the 5 that are already showing, + // We should expect all the personalDetails to show (minus // the currently logged in user and Concierge) - expect(results.personalDetails.length).toBe(Object.values(OPTIONS_WITH_CONCIERGE.personalDetails).length - 7); + // Filtering of personalDetails that have reports is done in filterOptions + expect(results.personalDetails.length).toBe(Object.values(OPTIONS_WITH_CONCIERGE.personalDetails).length - 2); expect(results.personalDetails).not.toEqual(expect.arrayContaining([expect.objectContaining({login: 'concierge@expensify.com'})])); expect(results.recentReports).not.toEqual(expect.arrayContaining([expect.objectContaining({login: 'concierge@expensify.com'})])); // Test by excluding Chronos from the results results = OptionsListUtils.getFilteredOptions(OPTIONS_WITH_CHRONOS.reports, OPTIONS_WITH_CHRONOS.personalDetails, [], '', [], [CONST.EMAIL.CHRONOS]); - // We should expect all the personalDetails to show (minus the 5 that are already showing, + // We should expect all the personalDetails to show (minus // the currently logged in user and Concierge) - expect(results.personalDetails.length).toBe(Object.values(OPTIONS_WITH_CHRONOS.personalDetails).length - 7); + // Filtering of personalDetails that have reports is done in filterOptions + expect(results.personalDetails.length).toBe(Object.values(OPTIONS_WITH_CHRONOS.personalDetails).length - 2); expect(results.personalDetails).not.toEqual(expect.arrayContaining([expect.objectContaining({login: 'chronos@expensify.com'})])); expect(results.recentReports).not.toEqual(expect.arrayContaining([expect.objectContaining({login: 'chronos@expensify.com'})])); // Test by excluding Receipts from the results results = OptionsListUtils.getFilteredOptions(OPTIONS_WITH_RECEIPTS.reports, OPTIONS_WITH_RECEIPTS.personalDetails, [], '', [], [CONST.EMAIL.RECEIPTS]); - // We should expect all the personalDetails to show (minus the 5 that are already showing, + // We should expect all the personalDetails to show (minus // the currently logged in user and Concierge) - expect(results.personalDetails.length).toBe(Object.values(OPTIONS_WITH_RECEIPTS.personalDetails).length - 7); + // Filtering of personalDetails that have reports is done in filterOptions + expect(results.personalDetails.length).toBe(Object.values(OPTIONS_WITH_RECEIPTS.personalDetails).length - 2); expect(results.personalDetails).not.toEqual(expect.arrayContaining([expect.objectContaining({login: 'receipts@expensify.com'})])); expect(results.recentReports).not.toEqual(expect.arrayContaining([expect.objectContaining({login: 'receipts@expensify.com'})])); }); @@ -2610,7 +2636,7 @@ describe('OptionsListUtils', () => { const options = OptionsListUtils.getSearchOptions(OPTIONS, '', [CONST.BETAS.ALL]); const filteredOptions = OptionsListUtils.filterOptions(options, ''); - expect(options.recentReports.length + options.personalDetails.length).toBe(filteredOptions.recentReports.length + filteredOptions.personalDetails.length); + expect(filteredOptions.recentReports.length + filteredOptions.personalDetails.length).toBe(12); }); it('should return filtered options in correct order', () => { From 68709d9c50f0fd91d25f1840b37f42b327eaf736 Mon Sep 17 00:00:00 2001 From: c3024 Date: Wed, 9 Oct 2024 16:44:45 +0530 Subject: [PATCH 04/13] extract filtering to function, fix lint in tests --- src/libs/OptionsListUtils.ts | 19 +++++++++---------- tests/unit/OptionsListUtilsTest.ts | 6 +++--- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index 657ceafcec9d..6363d446e6e4 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -2487,14 +2487,17 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt preferPolicyExpenseChat = false, preferRecentExpenseReports = false, } = config ?? {}; + function filteredPersonalDetailsOfRecentReports(recentReports: ReportUtils.OptionData[], personalDetails: ReportUtils.OptionData[]) { + const excludedLogins = new Set(recentReports.map((report) => report.login)); + return personalDetails.filter((personalDetail) => !excludedLogins.has(personalDetail.login)); + } if (searchInputValue.trim() === '' && maxRecentReportsToShow > 0) { const recentReports = options.recentReports.slice(0, maxRecentReportsToShow); - const excludedLogins = new Set(recentReports.map((report) => report.login)); - const filteredPersonalDetails = options.personalDetails.filter((personalDetail) => !excludedLogins.has(personalDetail.login)); + const personalDetails = filteredPersonalDetailsOfRecentReports(recentReports, options.personalDetails); return { ...options, recentReports, - personalDetails: filteredPersonalDetails, + personalDetails, }; } @@ -2538,8 +2541,7 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt const currentUserOption = isSearchStringMatch(term, currentUserOptionSearchText) ? items.currentUserOption : null; - const excludedLogins = new Set(recentReports.map((report) => report.login)); - const filteredPersonalDetails = personalDetails.filter((personalDetail) => !excludedLogins.has(personalDetail.login)); + const filteredPersonalDetails = filteredPersonalDetailsOfRecentReports(recentReports, personalDetails); return { recentReports: recentReports ?? [], @@ -2574,13 +2576,10 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt if (maxRecentReportsToShow > 0 && recentReports.length > maxRecentReportsToShow) { recentReports.splice(maxRecentReportsToShow); } - - const excludedLogins = new Set(recentReports.map((report) => report.login)); - const filteredPersonalDetails = personalDetails.filter((personalDetail) => !excludedLogins.has(personalDetail.login)); - personalDetails = filteredPersonalDetails; + const filteredPersonalDetails = filteredPersonalDetailsOfRecentReports(recentReports, personalDetails); return { - personalDetails, + personalDetails: filteredPersonalDetails, recentReports: orderOptions(recentReports, searchValue, {preferChatroomsOverThreads, preferPolicyExpenseChat, preferRecentExpenseReports}), userToInvite, currentUserOption: matchResults.currentUserOption, diff --git a/tests/unit/OptionsListUtilsTest.ts b/tests/unit/OptionsListUtilsTest.ts index 5e26932b6f9b..aafc8baf088a 100644 --- a/tests/unit/OptionsListUtilsTest.ts +++ b/tests/unit/OptionsListUtilsTest.ts @@ -525,7 +525,7 @@ describe('OptionsListUtils', () => { it('getFilteredOptions() for group Chat', () => { // When we call getFilteredOptions() with no search value - let results = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + const results = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); // Then we should expect only a maxmimum of 5 recent reports to be returned expect(results.recentReports.length).toBe(5); @@ -578,7 +578,7 @@ describe('OptionsListUtils', () => { // Test by excluding Concierge from the results results = OptionsListUtils.getFilteredOptions(OPTIONS_WITH_CONCIERGE.reports, OPTIONS_WITH_CONCIERGE.personalDetails, [], '', [], [CONST.EMAIL.CONCIERGE]); - // We should expect all the personalDetails to show (minus + // We should expect all the personalDetails to show (minus // the currently logged in user and Concierge) // Filtering of personalDetails that have reports is done in filterOptions expect(results.personalDetails.length).toBe(Object.values(OPTIONS_WITH_CONCIERGE.personalDetails).length - 2); @@ -598,7 +598,7 @@ describe('OptionsListUtils', () => { // Test by excluding Receipts from the results results = OptionsListUtils.getFilteredOptions(OPTIONS_WITH_RECEIPTS.reports, OPTIONS_WITH_RECEIPTS.personalDetails, [], '', [], [CONST.EMAIL.RECEIPTS]); - // We should expect all the personalDetails to show (minus + // We should expect all the personalDetails to show (minus // the currently logged in user and Concierge) // Filtering of personalDetails that have reports is done in filterOptions expect(results.personalDetails.length).toBe(Object.values(OPTIONS_WITH_RECEIPTS.personalDetails).length - 2); From b317a18a77efd3122ce3ce1b7dbf6e8edff7aa16 Mon Sep 17 00:00:00 2001 From: c3024 Date: Wed, 9 Oct 2024 17:39:14 +0530 Subject: [PATCH 05/13] fix lint --- tests/unit/OptionsListUtilsTest.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/unit/OptionsListUtilsTest.ts b/tests/unit/OptionsListUtilsTest.ts index aafc8baf088a..c2599cc8ca5d 100644 --- a/tests/unit/OptionsListUtilsTest.ts +++ b/tests/unit/OptionsListUtilsTest.ts @@ -404,7 +404,7 @@ describe('OptionsListUtils', () => { it('getSearchOptions()', () => { // When we filter in the Search view without providing a searchValue - let results = OptionsListUtils.getSearchOptions(OPTIONS, '', [CONST.BETAS.ALL]); + const results = OptionsListUtils.getSearchOptions(OPTIONS, '', [CONST.BETAS.ALL]); // All 2 personalDetails (including those that have reports) should be returned // Filtering of personalDetails that have reports is done in filterOptions @@ -439,10 +439,6 @@ describe('OptionsListUtils', () => { 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'); - // expect(results.personalDetails.at(0)?.text).toBe('Black Widow'); - // expect(results.personalDetails.at(1)?.text).toBe('Invisible Woman'); - // expect(results.personalDetails.at(2)?.text).toBe('Spider-Man'); - // expect(results.personalDetails.at(3)?.text).toBe('The Incredible Hulk'); // Then the result which has an existing report should also have the reportID attached const personalDetailWithExistingReport = results.personalDetails.find((personalDetail) => personalDetail.login === 'peterparker@expensify.com'); @@ -525,7 +521,7 @@ describe('OptionsListUtils', () => { it('getFilteredOptions() for group Chat', () => { // When we call getFilteredOptions() with no search value - const results = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + let results = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); // Then we should expect only a maxmimum of 5 recent reports to be returned expect(results.recentReports.length).toBe(5); From 28b226d92595edbcdfc808fa0e5f392dc3302839 Mon Sep 17 00:00:00 2001 From: c3024 Date: Wed, 9 Oct 2024 18:19:22 +0530 Subject: [PATCH 06/13] correct filtering logic for personal details --- src/libs/OptionsListUtils.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index 6363d446e6e4..6d579c0064e6 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -2540,12 +2540,9 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt const currentUserOptionSearchText = items.currentUserOption ? uniqFast(getCurrentUserSearchTerms(items.currentUserOption)).join(' ') : ''; const currentUserOption = isSearchStringMatch(term, currentUserOptionSearchText) ? items.currentUserOption : null; - - const filteredPersonalDetails = filteredPersonalDetailsOfRecentReports(recentReports, personalDetails); - return { recentReports: recentReports ?? [], - personalDetails: filteredPersonalDetails ?? [], + personalDetails: personalDetails ?? [], userToInvite: null, currentUserOption, categoryOptions: [], @@ -2557,6 +2554,7 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt let {recentReports, personalDetails} = matchResults; if (sortByReportTypeInSearch) { + personalDetails = filteredPersonalDetailsOfRecentReports(recentReports, personalDetails); recentReports = recentReports.concat(personalDetails); personalDetails = []; recentReports = orderOptions(recentReports, searchValue); From b69ea35f12e6f020dd7416a7008847f76c702442 Mon Sep 17 00:00:00 2001 From: c3024 Date: Mon, 14 Oct 2024 20:43:00 +0530 Subject: [PATCH 07/13] enforce empty search term with non-empty options --- src/libs/OptionsListUtils.ts | 89 +++++++++++++------ src/pages/EditReportFieldDropdown.tsx | 35 +++----- .../MoneyRequestParticipantsSelector.tsx | 39 +++----- 3 files changed, 86 insertions(+), 77 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index 6d579c0064e6..f72422af0390 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -2205,34 +2205,71 @@ function getIOUConfirmationOptionsFromPayeePersonalDetail(personalDetail: OnyxEn /** * Build the options for the New Group view */ +type FilteredOptionsParams = { + reports?: Array>; + personalDetails?: Array>; + betas?: OnyxEntry; + searchValue?: string; + selectedOptions?: Array>; + excludeLogins?: string[]; + includeOwnedWorkspaceChats?: boolean; + includeP2P?: boolean; + includeCategories?: boolean; + categories?: PolicyCategories; + recentlyUsedCategories?: string[]; + includeTags?: boolean; + tags?: PolicyTags | Array; + recentlyUsedTags?: string[]; + canInviteUser?: boolean; + includeSelectedOptions?: boolean; + includeTaxRates?: boolean; + taxRates?: TaxRatesWithDefault; + maxRecentReportsToShow?: number; + includeSelfDM?: boolean; + includePolicyReportFieldOptions?: boolean; + policyReportFieldOptions?: string[]; + recentlyUsedPolicyReportFieldOptions?: string[]; + includeInvoiceRooms?: boolean; + action?: IOUAction; + sortByReportTypeInSearch?: boolean; +}; + +type FilteredOptionsParamsWithDefaultSearchValue = Omit & {searchValue?: ''}; + +type FilteredOptionsParamsWithoutOptions = Omit & { reports?: []; personalDetails?: [] }; + function getFilteredOptions( - reports: Array> = [], - personalDetails: Array> = [], - betas: OnyxEntry = [], - searchValue = '', - selectedOptions: Array> = [], - excludeLogins: string[] = [], - includeOwnedWorkspaceChats = false, - includeP2P = true, - includeCategories = false, - categories: PolicyCategories = {}, - recentlyUsedCategories: string[] = [], - includeTags = false, - tags: PolicyTags | Array = {}, - recentlyUsedTags: string[] = [], - canInviteUser = true, - includeSelectedOptions = false, - includeTaxRates = false, - maxRecentReportsToShow: number = CONST.IOU.MAX_RECENT_REPORTS_TO_SHOW, - taxRates: TaxRatesWithDefault = {} as TaxRatesWithDefault, - includeSelfDM = false, - includePolicyReportFieldOptions = false, - policyReportFieldOptions: string[] = [], - recentlyUsedPolicyReportFieldOptions: string[] = [], - includeInvoiceRooms = false, - action: IOUAction | undefined = undefined, - sortByReportTypeInSearch = false, + params: FilteredOptionsParamsWithDefaultSearchValue | FilteredOptionsParamsWithoutOptions, ) { + + const { + reports = [], + personalDetails = [], + betas = [], + searchValue = '', + selectedOptions = [], + excludeLogins = [], + includeOwnedWorkspaceChats = false, + includeP2P = true, + includeCategories = false, + categories = {}, + recentlyUsedCategories = [], + includeTags = false, + tags = {}, + recentlyUsedTags = [], + canInviteUser = true, + includeSelectedOptions = false, + includeTaxRates = false, + maxRecentReportsToShow = CONST.IOU.MAX_RECENT_REPORTS_TO_SHOW, + taxRates = {} as TaxRatesWithDefault, + includeSelfDM = false, + includePolicyReportFieldOptions = false, + policyReportFieldOptions = [], + recentlyUsedPolicyReportFieldOptions = [], + includeInvoiceRooms = false, + action, + sortByReportTypeInSearch, + } = params; return getOptions( {reports, personalDetails}, { diff --git a/src/pages/EditReportFieldDropdown.tsx b/src/pages/EditReportFieldDropdown.tsx index a46a6db6f8a8..0c2b896a35cd 100644 --- a/src/pages/EditReportFieldDropdown.tsx +++ b/src/pages/EditReportFieldDropdown.tsx @@ -64,37 +64,22 @@ function EditReportFieldDropdownPage({onSubmit, fieldKey, fieldValue, fieldOptio const [sections, headerMessage] = useMemo(() => { const validFieldOptions = fieldOptions?.filter((option) => !!option)?.sort(localeCompare); - const {policyReportFieldOptions} = OptionsListUtils.getFilteredOptions( - [], - [], - [], - debouncedSearchValue, - [ + const {policyReportFieldOptions} = OptionsListUtils.getFilteredOptions({ + searchValue: debouncedSearchValue, + selectedOptions: [ { keyForList: fieldValue, searchText: fieldValue, text: fieldValue, }, ], - [], - false, - false, - false, - {}, - [], - false, - {}, - [], - false, - false, - undefined, - CONST.IOU.MAX_RECENT_REPORTS_TO_SHOW, - undefined, - undefined, - true, - validFieldOptions, - recentlyUsedOptions, - ); + + includeP2P: false, + canInviteUser: false, + includePolicyReportFieldOptions: true, + policyReportFieldOptions: validFieldOptions, + recentlyUsedPolicyReportFieldOptions: recentlyUsedOptions, + }); const policyReportFieldData = policyReportFieldOptions?.[0]?.data ?? []; const header = OptionsListUtils.getHeaderMessageForNonUserList(policyReportFieldData.length > 0, debouncedSearchValue); diff --git a/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx b/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx index a2652b8693ee..27d1045bf331 100644 --- a/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx +++ b/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx @@ -114,40 +114,26 @@ function MoneyRequestParticipantsSelector({ }; } - const optionList = OptionsListUtils.getFilteredOptions( - options.reports, - options.personalDetails, + const optionList = OptionsListUtils.getFilteredOptions({ + reports: options.reports, + personalDetails: options.personalDetails, betas, - '', - participants as Participant[], - CONST.EXPENSIFY_EMAILS, + selectedOptions: participants as Participant[], + excludeLogins: CONST.EXPENSIFY_EMAILS, // If we are using this component in the "Submit expense" or the combined submit/track flow then we pass the includeOwnedWorkspaceChats argument so that the current user // sees the option to submit an expense from their admin on their own Workspace Chat. - (iouType === CONST.IOU.TYPE.SUBMIT || iouType === CONST.IOU.TYPE.CREATE || iouType === CONST.IOU.TYPE.SPLIT) && action !== CONST.IOU.ACTION.SUBMIT, + includeOwnedWorkspaceChats: (iouType === CONST.IOU.TYPE.SUBMIT || iouType === CONST.IOU.TYPE.CREATE || iouType === CONST.IOU.TYPE.SPLIT) && action !== CONST.IOU.ACTION.SUBMIT, // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing - (canUseP2PDistanceRequests || iouRequestType !== CONST.IOU.REQUEST_TYPE.DISTANCE) && !isCategorizeOrShareAction, - false, - {}, - [], - false, - {}, - [], + includeP2P: (canUseP2PDistanceRequests || iouRequestType !== CONST.IOU.REQUEST_TYPE.DISTANCE) && !isCategorizeOrShareAction, // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing - (canUseP2PDistanceRequests || iouRequestType !== CONST.IOU.REQUEST_TYPE.DISTANCE) && !isCategorizeOrShareAction, - false, - false, - 0, - undefined, - undefined, - undefined, - undefined, - undefined, - iouType === CONST.IOU.TYPE.INVOICE, + canInviteUser: (canUseP2PDistanceRequests || iouRequestType !== CONST.IOU.REQUEST_TYPE.DISTANCE) && !isCategorizeOrShareAction, + includeInvoiceRooms: iouType === CONST.IOU.TYPE.INVOICE, action, - isPaidGroupPolicy, - ); + sortByReportTypeInSearch: isPaidGroupPolicy, + searchValue: '', + }); return optionList; }, [ @@ -163,6 +149,7 @@ function MoneyRequestParticipantsSelector({ options.reports, participants, isPaidGroupPolicy, + debouncedSearchTerm ]); const chatOptions = useMemo(() => { From dfab13e745f460c33b2068befb3419d415a484f4 Mon Sep 17 00:00:00 2001 From: c3024 Date: Wed, 16 Oct 2024 20:27:58 +0530 Subject: [PATCH 08/13] fix params for all getFilteredOptions calls --- src/components/CategoryPicker.tsx | 18 +- .../SearchFiltersParticipantsSelector.tsx | 27 +- src/components/TagPicker/index.tsx | 11 +- src/libs/OptionsListUtils.ts | 7 +- src/pages/EditReportFieldDropdown.tsx | 4 +- src/pages/NewChatPage.tsx | 29 +-- .../MoneyRequestParticipantsSelector.tsx | 4 +- .../Security/AddDelegate/AddDelegatePage.tsx | 29 +-- src/pages/tasks/TaskAssigneeSelectorModal.tsx | 26 +- tests/perf-test/OptionsListUtils.perf-test.ts | 7 +- tests/unit/OptionsListUtilsTest.ts | 240 +++++++----------- 11 files changed, 145 insertions(+), 257 deletions(-) diff --git a/src/components/CategoryPicker.tsx b/src/components/CategoryPicker.tsx index a8117fc1f4a0..33d97c6909f5 100644 --- a/src/components/CategoryPicker.tsx +++ b/src/components/CategoryPicker.tsx @@ -44,20 +44,14 @@ function CategoryPicker({selectedCategory, policyID, onSubmit}: CategoryPickerPr const [sections, headerMessage, shouldShowTextInput] = useMemo(() => { const categories = policyCategories ?? policyCategoriesDraft ?? {}; const validPolicyRecentlyUsedCategories = policyRecentlyUsedCategories?.filter?.((p) => !isEmptyObject(p)); - const {categoryOptions} = OptionsListUtils.getFilteredOptions( - [], - [], - [], - debouncedSearchValue, + const {categoryOptions} = OptionsListUtils.getFilteredOptions({ + searchValue: debouncedSearchValue, selectedOptions, - [], - false, - false, - true, + includeP2P: false, + includeCategories: true, categories, - validPolicyRecentlyUsedCategories, - false, - ); + recentlyUsedCategories: validPolicyRecentlyUsedCategories, + }); const categoryData = categoryOptions?.at(0)?.data ?? []; const header = OptionsListUtils.getHeaderMessageForNonUserList(categoryData.length > 0, debouncedSearchValue); diff --git a/src/components/Search/SearchFiltersParticipantsSelector.tsx b/src/components/Search/SearchFiltersParticipantsSelector.tsx index a76ca767ae2a..e7a60a5dc212 100644 --- a/src/components/Search/SearchFiltersParticipantsSelector.tsx +++ b/src/components/Search/SearchFiltersParticipantsSelector.tsx @@ -57,28 +57,13 @@ function SearchFiltersParticipantsSelector({initialAccountIDs, onFiltersUpdate}: return defaultListOptions; } - return OptionsListUtils.getFilteredOptions( - options.reports, - options.personalDetails, - undefined, - '', + return OptionsListUtils.getFilteredOptions({ + reports: options.reports, + personalDetails: options.personalDetails, selectedOptions, - CONST.EXPENSIFY_EMAILS, - false, - true, - false, - {}, - [], - false, - {}, - [], - true, - false, - false, - 0, - undefined, - false, - ); + excludeLogins: CONST.EXPENSIFY_EMAILS, + maxRecentReportsToShow: 0, + }); }, [areOptionsInitialized, options.personalDetails, options.reports, selectedOptions]); const chatOptions = useMemo(() => { diff --git a/src/components/TagPicker/index.tsx b/src/components/TagPicker/index.tsx index 1d72285be9a0..6046a5bf2bbd 100644 --- a/src/components/TagPicker/index.tsx +++ b/src/components/TagPicker/index.tsx @@ -87,7 +87,16 @@ function TagPicker({selectedTag, tagListName, policyTags, tagListIndex, policyRe }, [selectedOptions, policyTagList, shouldShowDisabledAndSelectedOption]); const sections = useMemo( - () => OptionsListUtils.getFilteredOptions([], [], [], searchValue, selectedOptions, [], false, false, false, {}, [], true, enabledTags, policyRecentlyUsedTagsList, false).tagOptions, + () => + OptionsListUtils.getFilteredOptions({ + searchValue, + selectedOptions, + includeP2P: false, + includeTags: true, + tags: enabledTags, + recentlyUsedTags: policyRecentlyUsedTagsList, + canInviteUser: false, + }).tagOptions, [searchValue, enabledTags, selectedOptions, policyRecentlyUsedTagsList], ); diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index f72422af0390..fd59f52b4da9 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -2236,12 +2236,9 @@ type FilteredOptionsParams = { type FilteredOptionsParamsWithDefaultSearchValue = Omit & {searchValue?: ''}; -type FilteredOptionsParamsWithoutOptions = Omit & { reports?: []; personalDetails?: [] }; - -function getFilteredOptions( - params: FilteredOptionsParamsWithDefaultSearchValue | FilteredOptionsParamsWithoutOptions, -) { +type FilteredOptionsParamsWithoutOptions = Omit & {reports?: []; personalDetails?: []}; +function getFilteredOptions(params: FilteredOptionsParamsWithDefaultSearchValue | FilteredOptionsParamsWithoutOptions) { const { reports = [], personalDetails = [], diff --git a/src/pages/EditReportFieldDropdown.tsx b/src/pages/EditReportFieldDropdown.tsx index 0c2b896a35cd..0b51dc9a1615 100644 --- a/src/pages/EditReportFieldDropdown.tsx +++ b/src/pages/EditReportFieldDropdown.tsx @@ -73,13 +73,13 @@ function EditReportFieldDropdownPage({onSubmit, fieldKey, fieldValue, fieldOptio text: fieldValue, }, ], - + includeP2P: false, canInviteUser: false, includePolicyReportFieldOptions: true, policyReportFieldOptions: validFieldOptions, recentlyUsedPolicyReportFieldOptions: recentlyUsedOptions, - }); + }); const policyReportFieldData = policyReportFieldOptions?.[0]?.data ?? []; const header = OptionsListUtils.getHeaderMessageForNonUserList(policyReportFieldData.length > 0, debouncedSearchValue); diff --git a/src/pages/NewChatPage.tsx b/src/pages/NewChatPage.tsx index 527c33bd08e4..43c6df5b9703 100755 --- a/src/pages/NewChatPage.tsx +++ b/src/pages/NewChatPage.tsx @@ -52,28 +52,15 @@ function useOptions({isGroupChat}: NewChatPageProps) { }); const defaultOptions = useMemo(() => { - const filteredOptions = OptionsListUtils.getFilteredOptions( - listOptions.reports ?? [], - listOptions.personalDetails ?? [], - betas ?? [], - '', + const filteredOptions = OptionsListUtils.getFilteredOptions({ + reports: listOptions.reports ?? [], + personalDetails: listOptions.personalDetails ?? [], + betas: betas ?? [], selectedOptions, - isGroupChat ? excludedGroupEmails : [], - false, - true, - false, - {}, - [], - false, - {}, - [], - true, - undefined, - undefined, - 0, - undefined, - true, - ); + excludeLogins: isGroupChat ? excludedGroupEmails : [], + maxRecentReportsToShow: 0, + includeSelfDM: true, + }); return filteredOptions; }, [betas, isGroupChat, listOptions.personalDetails, listOptions.reports, selectedOptions]); diff --git a/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx b/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx index 27d1045bf331..db03173e5df1 100644 --- a/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx +++ b/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx @@ -133,7 +133,7 @@ function MoneyRequestParticipantsSelector({ action, sortByReportTypeInSearch: isPaidGroupPolicy, searchValue: '', - }); + }); return optionList; }, [ @@ -149,7 +149,7 @@ function MoneyRequestParticipantsSelector({ options.reports, participants, isPaidGroupPolicy, - debouncedSearchTerm + debouncedSearchTerm, ]); const chatOptions = useMemo(() => { diff --git a/src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx b/src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx index ade900f365a7..2cd569ca421b 100644 --- a/src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx +++ b/src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx @@ -27,26 +27,15 @@ function useOptions() { const existingDelegates = useMemo(() => account?.delegatedAccess?.delegates?.map((delegate) => delegate.email) ?? [], [account?.delegatedAccess?.delegates]); const defaultOptions = useMemo(() => { - const {recentReports, personalDetails, userToInvite, currentUserOption} = OptionsListUtils.getFilteredOptions( - optionsList.reports, - optionsList.personalDetails, - betas, - '', - [], - [...CONST.EXPENSIFY_EMAILS, ...existingDelegates], - false, - true, - false, - {}, - [], - false, - {}, - [], - true, - false, - false, - 0, - ); + const {recentReports, personalDetails, userToInvite, currentUserOption} = OptionsListUtils.getFilteredOptions({ + reports: optionsList.reports, + personalDetails: optionsList.personalDetails, + betas: betas, + + excludeLogins: [...CONST.EXPENSIFY_EMAILS, ...existingDelegates], + + maxRecentReportsToShow: 0, + }); const headerMessage = OptionsListUtils.getHeaderMessage((recentReports?.length || 0) + (personalDetails?.length || 0) !== 0, !!userToInvite, ''); diff --git a/src/pages/tasks/TaskAssigneeSelectorModal.tsx b/src/pages/tasks/TaskAssigneeSelectorModal.tsx index 3f44882e2fd0..5a0912de45a5 100644 --- a/src/pages/tasks/TaskAssigneeSelectorModal.tsx +++ b/src/pages/tasks/TaskAssigneeSelectorModal.tsx @@ -40,26 +40,14 @@ function useOptions() { const {options: optionsList, areOptionsInitialized} = useOptionsList(); const defaultOptions = useMemo(() => { - const {recentReports, personalDetails, userToInvite, currentUserOption} = OptionsListUtils.getFilteredOptions( - optionsList.reports, - optionsList.personalDetails, + const {recentReports, personalDetails, userToInvite, currentUserOption} = OptionsListUtils.getFilteredOptions({ + reports: optionsList.reports, + personalDetails: optionsList.personalDetails, betas, - '', - [], - CONST.EXPENSIFY_EMAILS, - false, - true, - false, - {}, - [], - false, - {}, - [], - true, - false, - false, - 0, - ); + excludeLogins: CONST.EXPENSIFY_EMAILS, + + maxRecentReportsToShow: 0, + }); const headerMessage = OptionsListUtils.getHeaderMessage((recentReports?.length || 0) + (personalDetails?.length || 0) !== 0 || !!currentUserOption, !!userToInvite, ''); diff --git a/tests/perf-test/OptionsListUtils.perf-test.ts b/tests/perf-test/OptionsListUtils.perf-test.ts index 16522297a416..aec79df10ef9 100644 --- a/tests/perf-test/OptionsListUtils.perf-test.ts +++ b/tests/perf-test/OptionsListUtils.perf-test.ts @@ -105,9 +105,12 @@ describe('OptionsListUtils', () => { }); /* Testing getFilteredOptions */ - test('[OptionsListUtils] getFilteredOptions with search value', async () => { + test('[OptionsListUtils] getFilteredOptions', async () => { await waitForBatchedUpdates(); - await measureFunction(() => OptionsListUtils.getFilteredOptions(options.reports, options.personalDetails, mockedBetas, SEARCH_VALUE)); + // Ideally getFilteredOptions should not be used with both options and search value + // The more performant filterOptions should be used instead to pass search value with options containing reports and personal details + // @ts-expect-error + await measureFunction(() => OptionsListUtils.getFilteredOptions({reports: options.reports, personalDetails: options.personalDetails, betas: mockedBetas, searchValue: SEARCH_VALUE})); }); /* Testing getShareDestinationOptions */ diff --git a/tests/unit/OptionsListUtilsTest.ts b/tests/unit/OptionsListUtilsTest.ts index c2599cc8ca5d..f5d157498e32 100644 --- a/tests/unit/OptionsListUtilsTest.ts +++ b/tests/unit/OptionsListUtilsTest.ts @@ -419,7 +419,7 @@ describe('OptionsListUtils', () => { const MAX_RECENT_REPORTS = 5; // When we call getFilteredOptions() with no search value - let results = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + let results = OptionsListUtils.getFilteredOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); // We should expect maximimum of 5 recent reports to be returned expect(results.recentReports.length).toBe(MAX_RECENT_REPORTS); @@ -445,7 +445,7 @@ describe('OptionsListUtils', () => { expect(personalDetailWithExistingReport?.reportID).toBe('2'); // When we only pass personal details - results = OptionsListUtils.getFilteredOptions([], OPTIONS.personalDetails, [], ''); + results = OptionsListUtils.getFilteredOptions({personalDetails: OPTIONS.personalDetails}); // We should expect personal details sorted alphabetically expect(results.personalDetails.at(0)?.text).toBe('Black Panther'); @@ -454,39 +454,16 @@ describe('OptionsListUtils', () => { expect(results.personalDetails.at(3)?.text).toBe('Invisible Woman'); // When we don't include personal detail to the result - results = OptionsListUtils.getFilteredOptions( - [], - [], - [], - '', - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - undefined, - 0, - undefined, - undefined, - undefined, - undefined, - undefined, - false, - ); + results = OptionsListUtils.getFilteredOptions({ + maxRecentReportsToShow: 0, + }); // Then no personal detail options will be returned expect(results.personalDetails.length).toBe(0); // Test for Concierge's existence in chat options - results = OptionsListUtils.getFilteredOptions(OPTIONS_WITH_CONCIERGE.reports, OPTIONS_WITH_CONCIERGE.personalDetails); + results = OptionsListUtils.getFilteredOptions({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) @@ -495,7 +472,11 @@ describe('OptionsListUtils', () => { expect(results.recentReports).toEqual(expect.arrayContaining([expect.objectContaining({login: 'concierge@expensify.com'})])); // Test by excluding Concierge from the results - results = OptionsListUtils.getFilteredOptions(OPTIONS_WITH_CONCIERGE.reports, OPTIONS_WITH_CONCIERGE.personalDetails, [], '', [], [CONST.EMAIL.CONCIERGE]); + results = OptionsListUtils.getFilteredOptions({ + reports: OPTIONS_WITH_CONCIERGE.reports, + personalDetails: OPTIONS_WITH_CONCIERGE.personalDetails, + excludeLogins: [CONST.EMAIL.CONCIERGE], + }); // All the personalDetails should be returned minus the currently logged in user and Concierge // Filtering of personalDetails that have reports is done in filterOptions @@ -503,7 +484,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.getFilteredOptions(OPTIONS_WITH_CHRONOS.reports, OPTIONS_WITH_CHRONOS.personalDetails, [], '', [], [CONST.EMAIL.CHRONOS]); + results = OptionsListUtils.getFilteredOptions({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 @@ -511,7 +492,11 @@ describe('OptionsListUtils', () => { expect(results.personalDetails).not.toEqual(expect.arrayContaining([expect.objectContaining({login: 'chronos@expensify.com'})])); // Test by excluding Receipts from the results - results = OptionsListUtils.getFilteredOptions(OPTIONS_WITH_RECEIPTS.reports, OPTIONS_WITH_RECEIPTS.personalDetails, [], '', [], [CONST.EMAIL.RECEIPTS]); + results = OptionsListUtils.getFilteredOptions({ + reports: OPTIONS_WITH_RECEIPTS.reports, + personalDetails: OPTIONS_WITH_RECEIPTS.personalDetails, + excludeLogins: [CONST.EMAIL.RECEIPTS], + }); // All the personalDetails should be returned minus the currently logged in user and Concierge // Filtering of personalDetails that have reports is done in filterOptions @@ -521,7 +506,7 @@ describe('OptionsListUtils', () => { it('getFilteredOptions() for group Chat', () => { // When we call getFilteredOptions() with no search value - let results = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + let results = OptionsListUtils.getFilteredOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); // Then we should expect only a maxmimum of 5 recent reports to be returned expect(results.recentReports.length).toBe(5); @@ -548,7 +533,7 @@ describe('OptionsListUtils', () => { expect(personalDetailsOverlapWithReports).toBe(false); // When we provide no selected options to getFilteredOptions() - results = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], '', []); + results = OptionsListUtils.getFilteredOptions({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 @@ -556,14 +541,14 @@ describe('OptionsListUtils', () => { expect(results.personalDetails.every((option) => option.login !== 'peterparker@expensify.com')).toBe(false); // When we provide a "selected" option to getFilteredOptions() - results = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], '', [{login: 'peterparker@expensify.com'}]); + results = OptionsListUtils.getFilteredOptions({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.getFilteredOptions(OPTIONS_WITH_CONCIERGE.reports, OPTIONS_WITH_CONCIERGE.personalDetails); + results = OptionsListUtils.getFilteredOptions({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) @@ -572,7 +557,11 @@ describe('OptionsListUtils', () => { expect(results.recentReports).toEqual(expect.arrayContaining([expect.objectContaining({login: 'concierge@expensify.com'})])); // Test by excluding Concierge from the results - results = OptionsListUtils.getFilteredOptions(OPTIONS_WITH_CONCIERGE.reports, OPTIONS_WITH_CONCIERGE.personalDetails, [], '', [], [CONST.EMAIL.CONCIERGE]); + results = OptionsListUtils.getFilteredOptions({ + reports: OPTIONS_WITH_CONCIERGE.reports, + personalDetails: OPTIONS_WITH_CONCIERGE.personalDetails, + excludeLogins: [CONST.EMAIL.CONCIERGE], + }); // We should expect all the personalDetails to show (minus // the currently logged in user and Concierge) @@ -582,7 +571,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.getFilteredOptions(OPTIONS_WITH_CHRONOS.reports, OPTIONS_WITH_CHRONOS.personalDetails, [], '', [], [CONST.EMAIL.CHRONOS]); + results = OptionsListUtils.getFilteredOptions({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) @@ -592,7 +581,11 @@ describe('OptionsListUtils', () => { expect(results.recentReports).not.toEqual(expect.arrayContaining([expect.objectContaining({login: 'chronos@expensify.com'})])); // Test by excluding Receipts from the results - results = OptionsListUtils.getFilteredOptions(OPTIONS_WITH_RECEIPTS.reports, OPTIONS_WITH_RECEIPTS.personalDetails, [], '', [], [CONST.EMAIL.RECEIPTS]); + results = OptionsListUtils.getFilteredOptions({ + reports: OPTIONS_WITH_RECEIPTS.reports, + personalDetails: OPTIONS_WITH_RECEIPTS.personalDetails, + excludeLogins: [CONST.EMAIL.RECEIPTS], + }); // We should expect all the personalDetails to show (minus // the currently logged in user and Concierge) @@ -1102,61 +1095,54 @@ describe('OptionsListUtils', () => { }, ]; - const smallResult = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], emptySearch, [], [], false, false, true, smallCategoriesList); + const smallResult = OptionsListUtils.getFilteredOptions({ + reports: OPTIONS.reports, + personalDetails: OPTIONS.personalDetails, + searchValue: emptySearch, + includeP2P: false, + includeCategories: true, + categories: smallCategoriesList, + }); expect(smallResult.categoryOptions).toStrictEqual(smallResultList); - const smallSearchResult = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], search, [], [], false, false, true, smallCategoriesList); + const smallSearchResult = OptionsListUtils.getFilteredOptions({searchValue: search, includeP2P: false, includeCategories: true, categories: smallCategoriesList}); expect(smallSearchResult.categoryOptions).toStrictEqual(smallSearchResultList); - const smallWrongSearchResult = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], wrongSearch, [], [], false, false, true, smallCategoriesList); + const smallWrongSearchResult = OptionsListUtils.getFilteredOptions({searchValue: wrongSearch, includeP2P: false, includeCategories: true, categories: smallCategoriesList}); expect(smallWrongSearchResult.categoryOptions).toStrictEqual(smallWrongSearchResultList); - const largeResult = OptionsListUtils.getFilteredOptions( - OPTIONS.reports, - OPTIONS.personalDetails, - [], - emptySearch, + const largeResult = OptionsListUtils.getFilteredOptions({ + searchValue: emptySearch, selectedOptions, - [], - false, - false, - true, - largeCategoriesList, + includeP2P: false, + includeCategories: true, + categories: largeCategoriesList, recentlyUsedCategories, - ); + }); expect(largeResult.categoryOptions).toStrictEqual(largeResultList); - const largeSearchResult = OptionsListUtils.getFilteredOptions( - OPTIONS.reports, - OPTIONS.personalDetails, - [], - search, + const largeSearchResult = OptionsListUtils.getFilteredOptions({ + searchValue: search, selectedOptions, - [], - false, - false, - true, - largeCategoriesList, + + includeP2P: false, + includeCategories: true, + categories: largeCategoriesList, recentlyUsedCategories, - ); + }); expect(largeSearchResult.categoryOptions).toStrictEqual(largeSearchResultList); - const largeWrongSearchResult = OptionsListUtils.getFilteredOptions( - OPTIONS.reports, - OPTIONS.personalDetails, - [], - wrongSearch, + const largeWrongSearchResult = OptionsListUtils.getFilteredOptions({ + searchValue: wrongSearch, selectedOptions, - [], - false, - false, - true, - largeCategoriesList, + includeP2P: false, + includeCategories: true, + categories: largeCategoriesList, recentlyUsedCategories, - ); + }); expect(largeWrongSearchResult.categoryOptions).toStrictEqual(largeWrongSearchResultList); - const emptyResult = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], search, selectedOptions, [], false, false, true, emptyCategoriesList); + const emptyResult = OptionsListUtils.getFilteredOptions({searchValue: search, selectedOptions, includeP2P: false, includeCategories: true, categories: emptyCategoriesList}); expect(emptyResult.categoryOptions).toStrictEqual(emptySelectedResultList); }); @@ -1448,81 +1434,29 @@ describe('OptionsListUtils', () => { }, ]; - const smallResult = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], emptySearch, [], [], false, false, false, {}, [], true, smallTagsList); + const smallResult = OptionsListUtils.getFilteredOptions({searchValue: emptySearch, includeP2P: false, includeTags: true, tags: smallTagsList}); expect(smallResult.tagOptions).toStrictEqual(smallResultList); - const smallSearchResult = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], search, [], [], false, false, false, {}, [], true, smallTagsList); + const smallSearchResult = OptionsListUtils.getFilteredOptions({searchValue: search, includeP2P: false, includeTags: true, tags: smallTagsList}); expect(smallSearchResult.tagOptions).toStrictEqual(smallSearchResultList); - const smallWrongSearchResult = OptionsListUtils.getFilteredOptions( - OPTIONS.reports, - OPTIONS.personalDetails, - [], - wrongSearch, - [], - [], - false, - false, - false, - {}, - [], - true, - smallTagsList, - ); + const smallWrongSearchResult = OptionsListUtils.getFilteredOptions({searchValue: wrongSearch, includeP2P: false, includeTags: true, tags: smallTagsList}); expect(smallWrongSearchResult.tagOptions).toStrictEqual(smallWrongSearchResultList); - const largeResult = OptionsListUtils.getFilteredOptions( - OPTIONS.reports, - OPTIONS.personalDetails, - [], - emptySearch, - selectedOptions, - [], - false, - false, - false, - {}, - [], - true, - largeTagsList, - recentlyUsedTags, - ); + const largeResult = OptionsListUtils.getFilteredOptions({searchValue: emptySearch, selectedOptions, includeP2P: false, includeTags: true, tags: largeTagsList, recentlyUsedTags}); expect(largeResult.tagOptions).toStrictEqual(largeResultList); - const largeSearchResult = OptionsListUtils.getFilteredOptions( - OPTIONS.reports, - OPTIONS.personalDetails, - [], - search, - selectedOptions, - [], - false, - false, - false, - {}, - [], - true, - largeTagsList, - recentlyUsedTags, - ); + const largeSearchResult = OptionsListUtils.getFilteredOptions({searchValue: search, selectedOptions, includeP2P: false, includeTags: true, tags: largeTagsList, recentlyUsedTags}); expect(largeSearchResult.tagOptions).toStrictEqual(largeSearchResultList); - const largeWrongSearchResult = OptionsListUtils.getFilteredOptions( - OPTIONS.reports, - OPTIONS.personalDetails, - [], - wrongSearch, + const largeWrongSearchResult = OptionsListUtils.getFilteredOptions({ + searchValue: wrongSearch, selectedOptions, - [], - false, - false, - false, - {}, - [], - true, - largeTagsList, + includeP2P: false, + includeTags: true, + tags: largeTagsList, recentlyUsedTags, - ); + }); expect(largeWrongSearchResult.tagOptions).toStrictEqual(largeWrongSearchResultList); }); @@ -2731,7 +2665,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.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], searchText, [], CONST.EXPENSIFY_EMAILS); + const options = OptionsListUtils.getFilteredOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails, excludeLogins: CONST.EXPENSIFY_EMAILS}); const filterOptions = OptionsListUtils.filterOptions(options, searchText, {excludeLogins: CONST.EXPENSIFY_EMAILS}); expect(filterOptions.recentReports.length).toBe(0); }); @@ -2825,7 +2759,7 @@ describe('OptionsListUtils', () => { }); it('should show the option from personal details when searching for personal detail with no existing report (getFilteredOptions)', () => { - const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + const options = OptionsListUtils.getFilteredOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterOptions(options, 'hulk'); expect(filteredOptions.recentReports.length).toBe(0); @@ -2835,7 +2769,7 @@ describe('OptionsListUtils', () => { }); it('should return all matching reports and personal details (getFilteredOptions)', () => { - const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + const options = OptionsListUtils.getFilteredOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterOptions(options, '.com'); expect(filteredOptions.recentReports.length).toBe(5); @@ -2846,7 +2780,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 (getFilteredOptions)', () => { - const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + const options = OptionsListUtils.getFilteredOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterOptions(options, 'marc@expensify'); expect(filteredOptions.recentReports.length).toBe(0); @@ -2855,7 +2789,8 @@ 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 (getFilteredOptions)', () => { - const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + // const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + const options = OptionsListUtils.getFilteredOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterOptions(options, 'marc@expensify.com'); expect(filteredOptions.recentReports.length).toBe(0); @@ -2864,7 +2799,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 (getFilteredOptions)', () => { - const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + const options = OptionsListUtils.getFilteredOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterOptions(options, 'peter.parker@expensify.com'); expect(filteredOptions.recentReports.length).toBe(0); @@ -2872,7 +2807,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 (getFilteredOptions)', () => { - const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + const options = OptionsListUtils.getFilteredOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterOptions(options, '5005550006'); expect(filteredOptions.recentReports.length).toBe(0); @@ -2882,7 +2817,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 (getFilteredOptions)', () => { - const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + const options = OptionsListUtils.getFilteredOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterOptions(options, '+15005550006'); expect(filteredOptions.recentReports.length).toBe(0); @@ -2892,7 +2827,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 (getFilteredOptions)', () => { - const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + const options = OptionsListUtils.getFilteredOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterOptions(options, '+1 (800)324-3233'); expect(filteredOptions.recentReports.length).toBe(0); @@ -2902,7 +2837,8 @@ describe('OptionsListUtils', () => { }); it('should not return any options or user to invite if contact number contains alphabet characters (getFilteredOptions)', () => { - const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + // const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + const options = OptionsListUtils.getFilteredOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterOptions(options, '998243aaaa'); expect(filteredOptions.recentReports.length).toBe(0); @@ -2911,14 +2847,14 @@ describe('OptionsListUtils', () => { }); it('should not return any options if search value does not match any personal details (getFilteredOptions)', () => { - const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + const options = OptionsListUtils.getFilteredOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterOptions(options, 'magneto'); expect(filteredOptions.personalDetails.length).toBe(0); }); it('should return one recent report and no personal details if a search value provides an email (getFilteredOptions)', () => { - const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + const options = OptionsListUtils.getFilteredOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterOptions(options, 'peterparker@expensify.com', {sortByReportTypeInSearch: true}); expect(filteredOptions.recentReports.length).toBe(1); expect(filteredOptions.recentReports.at(0)?.text).toBe('Spider-Man'); @@ -2926,7 +2862,7 @@ describe('OptionsListUtils', () => { }); it('should return all matching reports and personal details (getFilteredOptions)', () => { - const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); + const options = OptionsListUtils.getFilteredOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterOptions(options, '.com'); expect(filteredOptions.personalDetails.length).toBe(4); From dd001f4d74adb20feb5f172fee943e6d6fc82708 Mon Sep 17 00:00:00 2001 From: c3024 Date: Wed, 16 Oct 2024 20:44:58 +0530 Subject: [PATCH 09/13] fix lint --- src/pages/EditReportFieldDropdown.tsx | 1 - src/pages/iou/request/MoneyRequestParticipantsSelector.tsx | 1 - src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx | 2 +- tests/perf-test/OptionsListUtils.perf-test.ts | 2 +- 4 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/pages/EditReportFieldDropdown.tsx b/src/pages/EditReportFieldDropdown.tsx index 0b51dc9a1615..dad93dce7964 100644 --- a/src/pages/EditReportFieldDropdown.tsx +++ b/src/pages/EditReportFieldDropdown.tsx @@ -11,7 +11,6 @@ import useLocalize from '@hooks/useLocalize'; import useTheme from '@hooks/useTheme'; import localeCompare from '@libs/LocaleCompare'; import * as OptionsListUtils from '@libs/OptionsListUtils'; -import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type {RecentlyUsedReportFields} from '@src/types/onyx'; diff --git a/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx b/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx index db03173e5df1..b590581c15cb 100644 --- a/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx +++ b/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx @@ -149,7 +149,6 @@ function MoneyRequestParticipantsSelector({ options.reports, participants, isPaidGroupPolicy, - debouncedSearchTerm, ]); const chatOptions = useMemo(() => { diff --git a/src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx b/src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx index 2cd569ca421b..6f433957015f 100644 --- a/src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx +++ b/src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx @@ -30,7 +30,7 @@ function useOptions() { const {recentReports, personalDetails, userToInvite, currentUserOption} = OptionsListUtils.getFilteredOptions({ reports: optionsList.reports, personalDetails: optionsList.personalDetails, - betas: betas, + betas, excludeLogins: [...CONST.EXPENSIFY_EMAILS, ...existingDelegates], diff --git a/tests/perf-test/OptionsListUtils.perf-test.ts b/tests/perf-test/OptionsListUtils.perf-test.ts index aec79df10ef9..0bfd84336760 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(); // Ideally getFilteredOptions should not be used with both options and search value // The more performant filterOptions should be used instead to pass search value with options containing reports and personal details - // @ts-expect-error + // @ts-expect-error pass both options and search value together await measureFunction(() => OptionsListUtils.getFilteredOptions({reports: options.reports, personalDetails: options.personalDetails, betas: mockedBetas, searchValue: SEARCH_VALUE})); }); From f9a76cf7bdc528749310c72ab3d6256b76add74c Mon Sep 17 00:00:00 2001 From: c3024 Date: Wed, 16 Oct 2024 21:02:38 +0530 Subject: [PATCH 10/13] migrate withOnyx to useOnyx --- src/components/TagPicker/index.tsx | 28 +++++++-------------------- src/pages/EditReportFieldDropdown.tsx | 19 +++++------------- 2 files changed, 12 insertions(+), 35 deletions(-) diff --git a/src/components/TagPicker/index.tsx b/src/components/TagPicker/index.tsx index 6046a5bf2bbd..9d3a70d4d50c 100644 --- a/src/components/TagPicker/index.tsx +++ b/src/components/TagPicker/index.tsx @@ -1,6 +1,5 @@ import React, {useMemo, useState} from 'react'; -import type {OnyxEntry} from 'react-native-onyx'; -import {withOnyx} from 'react-native-onyx'; +import {useOnyx} from 'react-native-onyx'; import SelectionList from '@components/SelectionList'; import RadioListItem from '@components/SelectionList/RadioListItem'; import useLocalize from '@hooks/useLocalize'; @@ -10,7 +9,7 @@ import * as PolicyUtils from '@libs/PolicyUtils'; import type * as ReportUtils from '@libs/ReportUtils'; import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {PolicyTag, PolicyTagLists, PolicyTags, RecentlyUsedTags} from '@src/types/onyx'; +import type {PolicyTag, PolicyTags} from '@src/types/onyx'; import type {PendingAction} from '@src/types/onyx/OnyxCommon'; type SelectedTagOption = { @@ -21,15 +20,7 @@ type SelectedTagOption = { pendingAction?: PendingAction; }; -type TagPickerOnyxProps = { - /** Collection of tag list on a policy */ - policyTags: OnyxEntry; - - /** List of recently used tags */ - policyRecentlyUsedTags: OnyxEntry; -}; - -type TagPickerProps = TagPickerOnyxProps & { +type TagPickerProps = { /** The policyID we are getting tags for */ // It's used in withOnyx HOC. // eslint-disable-next-line react/no-unused-prop-types @@ -51,7 +42,9 @@ type TagPickerProps = TagPickerOnyxProps & { tagListIndex: number; }; -function TagPicker({selectedTag, tagListName, policyTags, tagListIndex, policyRecentlyUsedTags, shouldShowDisabledAndSelectedOption = false, onSubmit}: TagPickerProps) { +function TagPicker({selectedTag, tagListName, policyID, tagListIndex, shouldShowDisabledAndSelectedOption = false, onSubmit}: TagPickerProps) { + const [policyTags] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`); + const [policyRecentlyUsedTags] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY_RECENTLY_USED_TAGS}${policyID}`); const styles = useThemeStyles(); const {translate} = useLocalize(); const [searchValue, setSearchValue] = useState(''); @@ -122,13 +115,6 @@ function TagPicker({selectedTag, tagListName, policyTags, tagListIndex, policyRe TagPicker.displayName = 'TagPicker'; -export default withOnyx({ - policyTags: { - key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY_TAGS}${policyID}`, - }, - policyRecentlyUsedTags: { - key: ({policyID}) => `${ONYXKEYS.COLLECTION.POLICY_RECENTLY_USED_TAGS}${policyID}`, - }, -})(TagPicker); +export default TagPicker; export type {SelectedTagOption}; diff --git a/src/pages/EditReportFieldDropdown.tsx b/src/pages/EditReportFieldDropdown.tsx index dad93dce7964..a6bccdf3fa12 100644 --- a/src/pages/EditReportFieldDropdown.tsx +++ b/src/pages/EditReportFieldDropdown.tsx @@ -1,6 +1,5 @@ import React, {useCallback, useMemo} from 'react'; -import {withOnyx} from 'react-native-onyx'; -import type {OnyxEntry} from 'react-native-onyx'; +import {useOnyx} from 'react-native-onyx'; import Icon from '@components/Icon'; import * as Expensicons from '@components/Icon/Expensicons'; import SelectionList from '@components/SelectionList'; @@ -12,7 +11,6 @@ import useTheme from '@hooks/useTheme'; import localeCompare from '@libs/LocaleCompare'; import * as OptionsListUtils from '@libs/OptionsListUtils'; import ONYXKEYS from '@src/ONYXKEYS'; -import type {RecentlyUsedReportFields} from '@src/types/onyx'; type EditReportFieldDropdownPageComponentProps = { /** Value of the policy report field */ @@ -32,13 +30,10 @@ type EditReportFieldDropdownPageComponentProps = { onSubmit: (form: Record) => void; }; -type EditReportFieldDropdownPageOnyxProps = { - recentlyUsedReportFields: OnyxEntry; -}; - -type EditReportFieldDropdownPageProps = EditReportFieldDropdownPageComponentProps & EditReportFieldDropdownPageOnyxProps; +type EditReportFieldDropdownPageProps = EditReportFieldDropdownPageComponentProps; -function EditReportFieldDropdownPage({onSubmit, fieldKey, fieldValue, fieldOptions, recentlyUsedReportFields}: EditReportFieldDropdownPageProps) { +function EditReportFieldDropdownPage({onSubmit, fieldKey, fieldValue, fieldOptions}: EditReportFieldDropdownPageProps) { + const [recentlyUsedReportFields] = useOnyx(ONYXKEYS.RECENTLY_USED_REPORT_FIELDS); const [searchValue, debouncedSearchValue, setSearchValue] = useDebouncedState(''); const theme = useTheme(); const {translate} = useLocalize(); @@ -105,8 +100,4 @@ function EditReportFieldDropdownPage({onSubmit, fieldKey, fieldValue, fieldOptio EditReportFieldDropdownPage.displayName = 'EditReportFieldDropdownPage'; -export default withOnyx({ - recentlyUsedReportFields: { - key: () => ONYXKEYS.RECENTLY_USED_REPORT_FIELDS, - }, -})(EditReportFieldDropdownPage); +export default EditReportFieldDropdownPage; From 63d5b19cb4136108062d6f765d1e1b3eb0d10865 Mon Sep 17 00:00:00 2001 From: c3024 Date: Thu, 17 Oct 2024 20:58:55 +0530 Subject: [PATCH 11/13] pass correct maxRecentReports to getFilteredOptions --- src/libs/OptionsListUtils.ts | 2 +- src/pages/iou/request/MoneyRequestParticipantsSelector.tsx | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index fd59f52b4da9..63fc86b7401b 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -2265,7 +2265,7 @@ function getFilteredOptions(params: FilteredOptionsParamsWithDefaultSearchValue recentlyUsedPolicyReportFieldOptions = [], includeInvoiceRooms = false, action, - sortByReportTypeInSearch, + sortByReportTypeInSearch = false, } = params; return getOptions( {reports, personalDetails}, diff --git a/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx b/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx index b590581c15cb..5dcf4dbd2ea6 100644 --- a/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx +++ b/src/pages/iou/request/MoneyRequestParticipantsSelector.tsx @@ -133,6 +133,7 @@ function MoneyRequestParticipantsSelector({ action, sortByReportTypeInSearch: isPaidGroupPolicy, searchValue: '', + maxRecentReportsToShow: 0, }); return optionList; From 3e72ecc4a5e675afe8b89dfe0cf51785dfa0e8d6 Mon Sep 17 00:00:00 2001 From: c3024 Date: Fri, 18 Oct 2024 17:11:24 +0530 Subject: [PATCH 12/13] review comments --- tests/perf-test/OptionsListUtils.perf-test.ts | 6 +++--- tests/unit/OptionsListUtilsTest.ts | 2 -- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/perf-test/OptionsListUtils.perf-test.ts b/tests/perf-test/OptionsListUtils.perf-test.ts index 0bfd84336760..04a953844d97 100644 --- a/tests/perf-test/OptionsListUtils.perf-test.ts +++ b/tests/perf-test/OptionsListUtils.perf-test.ts @@ -105,10 +105,10 @@ describe('OptionsListUtils', () => { }); /* Testing getFilteredOptions */ - test('[OptionsListUtils] getFilteredOptions', async () => { + test('[OptionsListUtils] getFilteredOptions with search value', async () => { await waitForBatchedUpdates(); - // Ideally getFilteredOptions should not be used with both options and search value - // The more performant filterOptions should be used instead to pass search value with options containing reports and personal details + // It's recommended not to use getFilteredOptions with both options and a search value + // For better performance, use filterOptions instead, especially when passing a search value with options that include reports and personal details. // @ts-expect-error pass both options and search value together await measureFunction(() => OptionsListUtils.getFilteredOptions({reports: options.reports, personalDetails: options.personalDetails, betas: mockedBetas, searchValue: SEARCH_VALUE})); }); diff --git a/tests/unit/OptionsListUtilsTest.ts b/tests/unit/OptionsListUtilsTest.ts index f5d157498e32..5a0cd6638a07 100644 --- a/tests/unit/OptionsListUtilsTest.ts +++ b/tests/unit/OptionsListUtilsTest.ts @@ -2789,7 +2789,6 @@ 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 (getFilteredOptions)', () => { - // const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); const options = OptionsListUtils.getFilteredOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterOptions(options, 'marc@expensify.com'); @@ -2837,7 +2836,6 @@ describe('OptionsListUtils', () => { }); it('should not return any options or user to invite if contact number contains alphabet characters (getFilteredOptions)', () => { - // const options = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], ''); const options = OptionsListUtils.getFilteredOptions({reports: OPTIONS.reports, personalDetails: OPTIONS.personalDetails}); const filteredOptions = OptionsListUtils.filterOptions(options, '998243aaaa'); From 0519359291d9e0c05380381463e35d8e310d0619 Mon Sep 17 00:00:00 2001 From: c3024 Date: Fri, 18 Oct 2024 17:44:57 +0530 Subject: [PATCH 13/13] comments explaining need for types and function --- src/libs/OptionsListUtils.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index 63fc86b7401b..be75787e5f43 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -2234,6 +2234,13 @@ type FilteredOptionsParams = { sortByReportTypeInSearch?: boolean; }; +// It is not recommended to pass a search value to getFilteredOptions when passing reports and personalDetails. +// If a search value is passed, the search value should be passed to filterOptions. +// When it is necessary to pass a search value when passing reports and personalDetails, follow these steps: +// 1. Use getFilteredOptions with reports and personalDetails only, without the search value. +// 2. Pass the returned options from getFilteredOptions to filterOptions along with the search value. +// The above constraints are enforced with TypeScript. + type FilteredOptionsParamsWithDefaultSearchValue = Omit & {searchValue?: ''}; type FilteredOptionsParamsWithoutOptions = Omit & {reports?: []; personalDetails?: []}; @@ -2521,6 +2528,7 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt preferPolicyExpenseChat = false, preferRecentExpenseReports = false, } = config ?? {}; + // Remove the personal details for the DMs that are already in the recent reports so that we don't show duplicates function filteredPersonalDetailsOfRecentReports(recentReports: ReportUtils.OptionData[], personalDetails: ReportUtils.OptionData[]) { const excludedLogins = new Set(recentReports.map((report) => report.login)); return personalDetails.filter((personalDetail) => !excludedLogins.has(personalDetail.login));