From 2e21c24fed25b43d109d4ae7e269459b8a85cc35 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Tue, 23 Apr 2024 11:30:19 +0800 Subject: [PATCH 1/2] don't include personal detail when sharing or categorizing tracked expense --- src/libs/OptionsListUtils.ts | 3 +- ...yForRefactorRequestParticipantsSelector.js | 33 ++-- tests/unit/OptionsListUtilsTest.ts | 156 ++++++++++++++++++ 3 files changed, 175 insertions(+), 17 deletions(-) diff --git a/src/libs/OptionsListUtils.ts b/src/libs/OptionsListUtils.ts index 2aad4179c337..79e767b48624 100644 --- a/src/libs/OptionsListUtils.ts +++ b/src/libs/OptionsListUtils.ts @@ -2008,6 +2008,7 @@ function getFilteredOptions( includePolicyReportFieldOptions = false, policyReportFieldOptions: string[] = [], recentlyUsedPolicyReportFieldOptions: string[] = [], + includePersonalDetails = true, ) { return getOptions( {reports, personalDetails}, @@ -2016,7 +2017,7 @@ function getFilteredOptions( searchInputValue: searchValue.trim(), selectedOptions, includeRecentReports: true, - includePersonalDetails: true, + includePersonalDetails, maxRecentReportsToShow: 5, excludeLogins, includeOwnedWorkspaceChats, diff --git a/src/pages/iou/request/MoneyTemporaryForRefactorRequestParticipantsSelector.js b/src/pages/iou/request/MoneyTemporaryForRefactorRequestParticipantsSelector.js index 53f547790768..762198090467 100644 --- a/src/pages/iou/request/MoneyTemporaryForRefactorRequestParticipantsSelector.js +++ b/src/pages/iou/request/MoneyTemporaryForRefactorRequestParticipantsSelector.js @@ -90,6 +90,7 @@ function MoneyTemporaryForRefactorRequestParticipantsSelector({participants, onF const {isSmallScreenWidth} = useWindowDimensions(); const isIOUSplit = iouType === CONST.IOU.TYPE.SPLIT; + const isCategorizeOrShareAction = [CONST.IOU.ACTION.CATEGORIZE, CONST.IOU.ACTION.SHARE].includes(action); useEffect(() => { Report.searchInServer(debouncedSearchTerm.trim()); @@ -117,15 +118,22 @@ function MoneyTemporaryForRefactorRequestParticipantsSelector({participants, onF // sees the option to submit an expense from their admin on their own Workspace Chat. (iouType === CONST.IOU.TYPE.SUBMIT || iouType === CONST.IOU.TYPE.SPLIT) && action !== CONST.IOU.ACTION.SUBMIT, - (canUseP2PDistanceRequests || iouRequestType !== CONST.IOU.REQUEST_TYPE.DISTANCE) && ![CONST.IOU.ACTION.CATEGORIZE, CONST.IOU.ACTION.SHARE].includes(action), + (canUseP2PDistanceRequests || iouRequestType !== CONST.IOU.REQUEST_TYPE.DISTANCE) && !isCategorizeOrShareAction, false, {}, [], false, {}, [], - (canUseP2PDistanceRequests || iouRequestType !== CONST.IOU.REQUEST_TYPE.DISTANCE) && ![CONST.IOU.ACTION.CATEGORIZE, CONST.IOU.ACTION.SHARE].includes(action), + (canUseP2PDistanceRequests || iouRequestType !== CONST.IOU.REQUEST_TYPE.DISTANCE) && !isCategorizeOrShareAction, false, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + !isCategorizeOrShareAction, ); const formatResults = OptionsListUtils.formatSectionsFromSearchTerm( @@ -150,13 +158,11 @@ function MoneyTemporaryForRefactorRequestParticipantsSelector({participants, onF shouldShow: chatOptions.recentReports.length > 0, }); - if (![CONST.IOU.ACTION.CATEGORIZE, CONST.IOU.ACTION.SHARE].includes(action)) { - newSections.push({ - title: translate('common.contacts'), - data: chatOptions.personalDetails, - shouldShow: chatOptions.personalDetails.length > 0, - }); - } + newSections.push({ + title: translate('common.contacts'), + data: chatOptions.personalDetails, + shouldShow: chatOptions.personalDetails.length > 0, + }); if (chatOptions.userToInvite && !OptionsListUtils.isCurrentUser(chatOptions.userToInvite)) { newSections.push({ @@ -185,6 +191,7 @@ function MoneyTemporaryForRefactorRequestParticipantsSelector({participants, onF personalDetails, translate, didScreenTransitionEnd, + isCategorizeOrShareAction, ]); /** @@ -349,13 +356,7 @@ function MoneyTemporaryForRefactorRequestParticipantsSelector({participants, onF ); const isAllSectionsEmpty = lodashEvery(sections, (section) => section.data.length === 0); - if ( - [CONST.IOU.ACTION.CATEGORIZE, CONST.IOU.ACTION.SHARE].includes(action) && - isAllSectionsEmpty && - didScreenTransitionEnd && - debouncedSearchTerm.trim() === '' && - areOptionsInitialized - ) { + if (isCategorizeOrShareAction && isAllSectionsEmpty && didScreenTransitionEnd && debouncedSearchTerm.trim() === '' && areOptionsInitialized) { return renderEmptyWorkspaceView(); } diff --git a/tests/unit/OptionsListUtilsTest.ts b/tests/unit/OptionsListUtilsTest.ts index 75ed7fa9d5b1..aecbba4755ef 100644 --- a/tests/unit/OptionsListUtilsTest.ts +++ b/tests/unit/OptionsListUtilsTest.ts @@ -433,6 +433,36 @@ describe('OptionsListUtils', () => { expect(results.personalDetails[2].text).toBe('Captain America'); expect(results.personalDetails[3].text).toBe('Invisible Woman'); + // When we don't include personal detail to the result + results = OptionsListUtils.getFilteredOptions( + [], + OPTIONS.personalDetails, + [], + '', + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + undefined, + false, + ); + + // Then no personal detail options will be returned + expect(results.personalDetails.length).toBe(0); + // When we provide a search value that does not match any personal details results = OptionsListUtils.getFilteredOptions(OPTIONS.reports, OPTIONS.personalDetails, [], 'magneto'); @@ -2587,6 +2617,132 @@ describe('OptionsListUtils', () => { expect(wrongSearchResult.taxRatesOptions).toStrictEqual(wrongSearchResultList); }); + it('getFilteredOptions() for taxRate', () => { + const search = 'rate'; + const emptySearch = ''; + const wrongSearch = 'bla bla'; + + const taxRatesWithDefault: TaxRatesWithDefault = { + name: 'Tax', + defaultExternalID: 'CODE1', + defaultValue: '0%', + foreignTaxDefault: 'CODE1', + taxes: { + CODE2: { + name: 'Tax rate 2', + value: '3%', + code: 'CODE2', + modifiedName: 'Tax rate 2 (3%)', + }, + CODE3: { + name: 'Tax option 3', + value: '5%', + code: 'CODE3', + modifiedName: 'Tax option 3 (5%)', + }, + CODE1: { + name: 'Tax exempt 1', + value: '0%', + code: 'CODE1', + modifiedName: 'Tax exempt 1 (0%) • Default', + }, + }, + }; + + const resultList: OptionsListUtils.CategorySection[] = [ + { + title: '', + shouldShow: false, + // data sorted alphabetically by name + data: [ + { + // Adds 'Default' title to default tax. + // Adds value to tax name for more description. + text: 'Tax exempt 1 (0%) • Default', + keyForList: 'Tax exempt 1 (0%) • Default', + searchText: 'Tax exempt 1 (0%) • Default', + tooltipText: 'Tax exempt 1 (0%) • Default', + isDisabled: undefined, + // creates a data option. + data: { + name: 'Tax exempt 1', + code: 'CODE1', + modifiedName: 'Tax exempt 1 (0%) • Default', + value: '0%', + }, + }, + { + text: 'Tax option 3 (5%)', + keyForList: 'Tax option 3 (5%)', + searchText: 'Tax option 3 (5%)', + tooltipText: 'Tax option 3 (5%)', + isDisabled: undefined, + data: { + name: 'Tax option 3', + code: 'CODE3', + modifiedName: 'Tax option 3 (5%)', + value: '5%', + }, + }, + { + text: 'Tax rate 2 (3%)', + keyForList: 'Tax rate 2 (3%)', + searchText: 'Tax rate 2 (3%)', + tooltipText: 'Tax rate 2 (3%)', + isDisabled: undefined, + data: { + name: 'Tax rate 2', + code: 'CODE2', + modifiedName: 'Tax rate 2 (3%)', + value: '3%', + }, + }, + ], + }, + ]; + + const searchResultList: OptionsListUtils.CategorySection[] = [ + { + title: '', + shouldShow: true, + // data sorted alphabetically by name + data: [ + { + text: 'Tax rate 2 (3%)', + keyForList: 'Tax rate 2 (3%)', + searchText: 'Tax rate 2 (3%)', + tooltipText: 'Tax rate 2 (3%)', + isDisabled: undefined, + data: { + name: 'Tax rate 2', + code: 'CODE2', + modifiedName: 'Tax rate 2 (3%)', + value: '3%', + }, + }, + ], + }, + ]; + + const wrongSearchResultList: OptionsListUtils.CategorySection[] = [ + { + title: '', + shouldShow: true, + data: [], + }, + ]; + + const result = OptionsListUtils.getFilteredOptions([], [], [], emptySearch, [], [], false, false, false, {}, [], false, {}, [], false, false, true, taxRatesWithDefault); + + expect(result.taxRatesOptions).toStrictEqual(resultList); + + const searchResult = OptionsListUtils.getFilteredOptions([], [], [], search, [], [], false, false, false, {}, [], false, {}, [], false, false, true, taxRatesWithDefault); + expect(searchResult.taxRatesOptions).toStrictEqual(searchResultList); + + const wrongSearchResult = OptionsListUtils.getFilteredOptions([], [], [], wrongSearch, [], [], false, false, false, {}, [], false, {}, [], false, false, true, taxRatesWithDefault); + expect(wrongSearchResult.taxRatesOptions).toStrictEqual(wrongSearchResultList); + }); + it('formatMemberForList()', () => { const formattedMembers = Object.values(PERSONAL_DETAILS).map((personalDetail) => OptionsListUtils.formatMemberForList(personalDetail)); From 6462567784092bbbdd08c73fcc1aec454f7c550f Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Tue, 23 Apr 2024 11:36:03 +0800 Subject: [PATCH 2/2] remove unintended code --- tests/unit/OptionsListUtilsTest.ts | 126 ----------------------------- 1 file changed, 126 deletions(-) diff --git a/tests/unit/OptionsListUtilsTest.ts b/tests/unit/OptionsListUtilsTest.ts index aecbba4755ef..7a7930cb5871 100644 --- a/tests/unit/OptionsListUtilsTest.ts +++ b/tests/unit/OptionsListUtilsTest.ts @@ -2617,132 +2617,6 @@ describe('OptionsListUtils', () => { expect(wrongSearchResult.taxRatesOptions).toStrictEqual(wrongSearchResultList); }); - it('getFilteredOptions() for taxRate', () => { - const search = 'rate'; - const emptySearch = ''; - const wrongSearch = 'bla bla'; - - const taxRatesWithDefault: TaxRatesWithDefault = { - name: 'Tax', - defaultExternalID: 'CODE1', - defaultValue: '0%', - foreignTaxDefault: 'CODE1', - taxes: { - CODE2: { - name: 'Tax rate 2', - value: '3%', - code: 'CODE2', - modifiedName: 'Tax rate 2 (3%)', - }, - CODE3: { - name: 'Tax option 3', - value: '5%', - code: 'CODE3', - modifiedName: 'Tax option 3 (5%)', - }, - CODE1: { - name: 'Tax exempt 1', - value: '0%', - code: 'CODE1', - modifiedName: 'Tax exempt 1 (0%) • Default', - }, - }, - }; - - const resultList: OptionsListUtils.CategorySection[] = [ - { - title: '', - shouldShow: false, - // data sorted alphabetically by name - data: [ - { - // Adds 'Default' title to default tax. - // Adds value to tax name for more description. - text: 'Tax exempt 1 (0%) • Default', - keyForList: 'Tax exempt 1 (0%) • Default', - searchText: 'Tax exempt 1 (0%) • Default', - tooltipText: 'Tax exempt 1 (0%) • Default', - isDisabled: undefined, - // creates a data option. - data: { - name: 'Tax exempt 1', - code: 'CODE1', - modifiedName: 'Tax exempt 1 (0%) • Default', - value: '0%', - }, - }, - { - text: 'Tax option 3 (5%)', - keyForList: 'Tax option 3 (5%)', - searchText: 'Tax option 3 (5%)', - tooltipText: 'Tax option 3 (5%)', - isDisabled: undefined, - data: { - name: 'Tax option 3', - code: 'CODE3', - modifiedName: 'Tax option 3 (5%)', - value: '5%', - }, - }, - { - text: 'Tax rate 2 (3%)', - keyForList: 'Tax rate 2 (3%)', - searchText: 'Tax rate 2 (3%)', - tooltipText: 'Tax rate 2 (3%)', - isDisabled: undefined, - data: { - name: 'Tax rate 2', - code: 'CODE2', - modifiedName: 'Tax rate 2 (3%)', - value: '3%', - }, - }, - ], - }, - ]; - - const searchResultList: OptionsListUtils.CategorySection[] = [ - { - title: '', - shouldShow: true, - // data sorted alphabetically by name - data: [ - { - text: 'Tax rate 2 (3%)', - keyForList: 'Tax rate 2 (3%)', - searchText: 'Tax rate 2 (3%)', - tooltipText: 'Tax rate 2 (3%)', - isDisabled: undefined, - data: { - name: 'Tax rate 2', - code: 'CODE2', - modifiedName: 'Tax rate 2 (3%)', - value: '3%', - }, - }, - ], - }, - ]; - - const wrongSearchResultList: OptionsListUtils.CategorySection[] = [ - { - title: '', - shouldShow: true, - data: [], - }, - ]; - - const result = OptionsListUtils.getFilteredOptions([], [], [], emptySearch, [], [], false, false, false, {}, [], false, {}, [], false, false, true, taxRatesWithDefault); - - expect(result.taxRatesOptions).toStrictEqual(resultList); - - const searchResult = OptionsListUtils.getFilteredOptions([], [], [], search, [], [], false, false, false, {}, [], false, {}, [], false, false, true, taxRatesWithDefault); - expect(searchResult.taxRatesOptions).toStrictEqual(searchResultList); - - const wrongSearchResult = OptionsListUtils.getFilteredOptions([], [], [], wrongSearch, [], [], false, false, false, {}, [], false, {}, [], false, false, true, taxRatesWithDefault); - expect(wrongSearchResult.taxRatesOptions).toStrictEqual(wrongSearchResultList); - }); - it('formatMemberForList()', () => { const formattedMembers = Object.values(PERSONAL_DETAILS).map((personalDetail) => OptionsListUtils.formatMemberForList(personalDetail));