Skip to content

Commit

Permalink
fix params for all getFilteredOptions calls
Browse files Browse the repository at this point in the history
  • Loading branch information
c3024 committed Oct 16, 2024
1 parent b69ea35 commit dfab13e
Show file tree
Hide file tree
Showing 11 changed files with 145 additions and 257 deletions.
18 changes: 6 additions & 12 deletions src/components/CategoryPicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
27 changes: 6 additions & 21 deletions src/components/Search/SearchFiltersParticipantsSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down
11 changes: 10 additions & 1 deletion src/components/TagPicker/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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],
);

Expand Down
7 changes: 2 additions & 5 deletions src/libs/OptionsListUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2236,12 +2236,9 @@ type FilteredOptionsParams = {

type FilteredOptionsParamsWithDefaultSearchValue = Omit<FilteredOptionsParams, 'searchValue'> & {searchValue?: ''};

type FilteredOptionsParamsWithoutOptions = Omit<FilteredOptionsParams, 'reports' | 'personalDetails'> & { reports?: []; personalDetails?: [] };

function getFilteredOptions(
params: FilteredOptionsParamsWithDefaultSearchValue | FilteredOptionsParamsWithoutOptions,
) {
type FilteredOptionsParamsWithoutOptions = Omit<FilteredOptionsParams, 'reports' | 'personalDetails'> & {reports?: []; personalDetails?: []};

function getFilteredOptions(params: FilteredOptionsParamsWithDefaultSearchValue | FilteredOptionsParamsWithoutOptions) {
const {
reports = [],
personalDetails = [],
Expand Down
4 changes: 2 additions & 2 deletions src/pages/EditReportFieldDropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
29 changes: 8 additions & 21 deletions src/pages/NewChatPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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]);

Expand Down
4 changes: 2 additions & 2 deletions src/pages/iou/request/MoneyRequestParticipantsSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ function MoneyRequestParticipantsSelector({
action,
sortByReportTypeInSearch: isPaidGroupPolicy,
searchValue: '',
});
});

return optionList;
}, [

Check warning on line 139 in src/pages/iou/request/MoneyRequestParticipantsSelector.tsx

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

React Hook useMemo has an unnecessary dependency: 'debouncedSearchTerm'. Either exclude it or remove the dependency array

Check warning on line 139 in src/pages/iou/request/MoneyRequestParticipantsSelector.tsx

View workflow job for this annotation

GitHub Actions / ESLint check

React Hook useMemo has an unnecessary dependency: 'debouncedSearchTerm'. Either exclude it or remove the dependency array
Expand All @@ -149,7 +149,7 @@ function MoneyRequestParticipantsSelector({
options.reports,
participants,
isPaidGroupPolicy,
debouncedSearchTerm
debouncedSearchTerm,
]);

const chatOptions = useMemo(() => {
Expand Down
29 changes: 9 additions & 20 deletions src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,

Check failure on line 33 in src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Expected property shorthand

Check failure on line 33 in src/pages/settings/Security/AddDelegate/AddDelegatePage.tsx

View workflow job for this annotation

GitHub Actions / ESLint check

Expected property shorthand

excludeLogins: [...CONST.EXPENSIFY_EMAILS, ...existingDelegates],

maxRecentReportsToShow: 0,
});

const headerMessage = OptionsListUtils.getHeaderMessage((recentReports?.length || 0) + (personalDetails?.length || 0) !== 0, !!userToInvite, '');

Expand Down
26 changes: 7 additions & 19 deletions src/pages/tasks/TaskAssigneeSelectorModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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, '');

Expand Down
7 changes: 5 additions & 2 deletions tests/perf-test/OptionsListUtils.perf-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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

Check failure on line 112 in tests/perf-test/OptionsListUtils.perf-test.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Include a description after the "@ts-expect-error" directive to explain why the @ts-expect-error is necessary. The description must be 3 characters or longer

Check failure on line 112 in tests/perf-test/OptionsListUtils.perf-test.ts

View workflow job for this annotation

GitHub Actions / ESLint check

Include a description after the "@ts-expect-error" directive to explain why the @ts-expect-error is necessary. The description must be 3 characters or longer
await measureFunction(() => OptionsListUtils.getFilteredOptions({reports: options.reports, personalDetails: options.personalDetails, betas: mockedBetas, searchValue: SEARCH_VALUE}));
});

/* Testing getShareDestinationOptions */
Expand Down
Loading

0 comments on commit dfab13e

Please sign in to comment.