Skip to content

Commit

Permalink
don't include personal detail when sharing or categorizing tracked ex…
Browse files Browse the repository at this point in the history 
…pense
  • Loading branch information
@bernhardoj
bernhardoj committed Apr 23, 2024
1 parent 64695c8 commit 2e21c24
Show file tree
Hide file tree
Showing 3 changed files with 175 additions and 17 deletions.
3 changes: 2 additions & 1 deletion src/libs/OptionsListUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2008,6 +2008,7 @@ function getFilteredOptions(
includePolicyReportFieldOptions = false,
policyReportFieldOptions: string[] = [],
recentlyUsedPolicyReportFieldOptions: string[] = [],
includePersonalDetails = true,
) {
return getOptions(
{reports, personalDetails},
Expand All @@ -2016,7 +2017,7 @@ function getFilteredOptions(
searchInputValue: searchValue.trim(),
selectedOptions,
includeRecentReports: true,
includePersonalDetails: true,
includePersonalDetails,
maxRecentReportsToShow: 5,
excludeLogins,
includeOwnedWorkspaceChats,
Expand Down
33 changes: 17 additions & 16 deletions src/pages/iou/request/MoneyTemporaryForRefactorRequestParticipantsSelector.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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,

This comment has been minimized.

Sign in to view

Copy link
@getusha

getusha Jul 4, 2024

Contributor

This may have caused #40790
We should have considered the permission canUseP2PDistanceRequests in this argument, just like we did with includeP2P above. We ended up modifying the function to use includeP2P and removed includePersonalDetails, which ultimately solved the issue.
);

const formatResults = OptionsListUtils.formatSectionsFromSearchTerm(
Expand All @@ -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({
Expand Down Expand Up @@ -185,6 +191,7 @@ function MoneyTemporaryForRefactorRequestParticipantsSelector({participants, onF
personalDetails,
translate,
didScreenTransitionEnd,
isCategorizeOrShareAction,
]);

/**
Expand Down Expand Up @@ -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();
}

Expand Down
156 changes: 156 additions & 0 deletions tests/unit/OptionsListUtilsTest.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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));

Expand Down

0 comments on commit 2e21c24

Please sign in to comment.