Skip to content

[CP Staging] Filter page improvements #65411

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mountiny merged 5 commits into from
Jul 6, 2025
Merged

[CP Staging] Filter page improvements #65411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0c72bca
Reset Filters in UI instead of actual filter on reset click
parasharrajat Jul 3, 2025
9fc44c2
Fix duplicate items after selection
parasharrajat Jul 3, 2025
fe04631
Fix Filters
parasharrajat Jul 3, 2025
d8e5c7b
Fix category and tag filter pages
parasharrajat Jul 4, 2025
3b1bf61
More fixes
parasharrajat Jul 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions src/components/Search/SearchMultipleSelectionPicker.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,19 @@ function SearchMultipleSelectionPicker({items, initiallySelectedItems, pickerTit
const {sections, noResultsFound} = useMemo(() => {
const selectedItemsSection = selectedItems
.filter((item) => item?.name.toLowerCase().includes(debouncedSearchTerm?.toLowerCase()))
.sort((a, b) => sortOptionsWithEmptyValue(a.value as string, b.value as string))
.sort((a, b) => sortOptionsWithEmptyValue(a.value.toString(), b.value.toString()))
.map((item) => ({
text: item.name,
keyForList: item.name,
isSelected: true,
value: item.value,
}));
const remainingItemsSection = items
.filter((item) => selectedItems.some((selectedItem) => selectedItem.value === item.value) === false && item?.name?.toLowerCase().includes(debouncedSearchTerm?.toLowerCase()))
.sort((a, b) => sortOptionsWithEmptyValue(a.value as string, b.value as string))
.filter(
(item) =>
!selectedItems.some((selectedItem) => selectedItem.value.toString() === item.value.toString()) && item?.name?.toLowerCase().includes(debouncedSearchTerm?.toLowerCase()),
)
.sort((a, b) => sortOptionsWithEmptyValue(a.value.toString(), b.value.toString()))
.map((item) => ({
text: item.name,
keyForList: item.name,
Expand Down
3 changes: 3 additions & 0 deletions src/libs/PolicyUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,18 +59,18 @@
let activePolicyId: OnyxEntry<string>;
let isLoadingReportData = true;

Onyx.connect({

Check warning on line 62 in src/libs/PolicyUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.POLICY,
waitForCollectionCallback: true,
callback: (value) => (allPolicies = value),
});

Onyx.connect({

Check warning on line 68 in src/libs/PolicyUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.NVP_ACTIVE_POLICY_ID,
callback: (value) => (activePolicyId = value),
});

Onyx.connect({

Check warning on line 73 in src/libs/PolicyUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.IS_LOADING_REPORT_DATA,
initWithStoredValues: false,
callback: (value) => (isLoadingReportData = value ?? false),
Expand Down Expand Up @@ -578,6 +578,9 @@
allTaxRates[taxRate.name] = [taxRateKey];
return;
}
if (allTaxRates[taxRate.name].includes(taxRateKey)) {
return;
}
allTaxRates[taxRate.name].push(taxRateKey);
});
});
Expand Down
9 changes: 2 additions & 7 deletions src/libs/SearchQueryUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ function buildSearchQueryString(queryJSON?: SearchQueryJSON) {
*/
function buildQueryStringFromFilterFormValues(filterValues: Partial<SearchAdvancedFiltersForm>) {
// We separate type and status filters from other filters to maintain hashes consistency for saved searches
const {type, status, policyID, groupBy, ...otherFilters} = filterValues;
const {type, status, groupBy, ...otherFilters} = filterValues;
const filtersString: string[] = [];

// When switching types/setting the type, ensure we aren't polluting our query with filters that are
Expand Down Expand Up @@ -436,11 +436,6 @@ function buildQueryStringFromFilterFormValues(filterValues: Partial<SearchAdvanc
filtersString.push(`${CONST.SEARCH.SYNTAX_ROOT_KEYS.STATUS}:${filterValueArray.map(sanitizeSearchValue).join(',')}`);
}

if (policyID) {
const sanitizedPolicyIDs = Array.isArray(policyID) ? policyID.map((id) => sanitizeSearchValue(id)).join(',') : sanitizeSearchValue(policyID);
filtersString.push(`${CONST.SEARCH.SYNTAX_FILTER_KEYS.POLICY_ID}:${sanitizedPolicyIDs}`);
}

const mappedFilters = Object.entries(otherFilters)
.map(([filterKey, filterValue]) => {
if (
Expand Down Expand Up @@ -776,7 +771,7 @@ function buildUserReadableQueryString(
title += ` group-by:${groupBy}`;
}

if (policyID) {
if (policyID && policyID.length > 0) {
title += ` workspace:${policyID.map((id) => sanitizeSearchValue(policies?.[`${ONYXKEYS.COLLECTION.POLICY}${id}`]?.name ?? id)).join(',')}`;
}

Expand Down
2 changes: 1 addition & 1 deletion src/pages/Search/SearchAdvancedFiltersPage/SearchFiltersCategoryPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ function SearchFiltersCategoryPage() {
const items = [{name: translate('search.noCategory'), value: CONST.SEARCH.CATEGORY_EMPTY_VALUE as string}];
const uniqueCategoryNames = new Set<string>();

if (!selectedPoliciesCategories) {
if (!selectedPoliciesCategories || selectedPoliciesCategories.length === 0) {
Object.values(allPolicyCategories ?? {}).map((policyCategories) => Object.values(policyCategories ?? {}).forEach((category) => uniqueCategoryNames.add(category.name)));
} else {
selectedPoliciesCategories.forEach((category) => uniqueCategoryNames.add(category.name));
Expand Down
2 changes: 1 addition & 1 deletion src/pages/Search/SearchAdvancedFiltersPage/SearchFiltersTagPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ function SearchFiltersTagPage() {
const items = [{name: translate('search.noTag'), value: CONST.SEARCH.TAG_EMPTY_VALUE as string}];
const uniqueTagNames = new Set<string>();

if (!selectedPoliciesTagLists) {
if (!selectedPoliciesTagLists || selectedPoliciesTagLists.length === 0) {
const tagListsUnpacked = Object.values(allPolicyTagLists ?? {}).filter((item) => !!item) as PolicyTagLists[];
tagListsUnpacked
.map(getTagNamesFromTagsLists)
Expand Down
2 changes: 1 addition & 1 deletion src/pages/Search/SearchAdvancedFiltersPage/SearchFiltersTaxRatePage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ function SearchFiltersTaxRatePage() {
const selectedPoliciesTaxRates = policies?.map((policy) => policy?.taxRates?.taxes).filter((taxRates) => !!taxRates);

const taxItems = useMemo(() => {
if (!selectedPoliciesTaxRates) {
if (!selectedPoliciesTaxRates || selectedPoliciesTaxRates?.length === 0) {
return Object.entries(allTaxRates).map(([taxRateName, taxRateKeys]) => ({name: taxRateName, value: taxRateKeys}));
}
const selectedPoliciesTaxRatesItems = selectedPoliciesTaxRates.reduce(
Expand Down
2 changes: 1 addition & 1 deletion src/pages/Search/SearchAdvancedFiltersPage/SearchFiltersWorkspacePage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ function SearchFiltersWorkspacePage() {
}, [selectedOptions]);

@shubham1206agra shubham1206agra Jul 5, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@parasharrajat Can you fix this function by using below?

const applyChanges = useCallback(() => {
        const policyIds = selectedOptions.map((option) => (option ? option.toString() : undefined)).filter(Boolean) as string[];
        updateWorkspaceFilter(policyIds.length > 0 ? policyIds : null);
    }, [selectedOptions]);

@shubham1206agra shubham1206agra Jul 5, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, fix the buildQueryStringFromFilterFormValues function.

@shubham1206agra shubham1206agra Jul 5, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the diff

diff --git a/src/libs/SearchQueryUtils.ts b/src/libs/SearchQueryUtils.ts
index 52fb60daeaa..18239a27095 100644
--- a/src/libs/SearchQueryUtils.ts
+++ b/src/libs/SearchQueryUtils.ts
@@ -395,7 +395,7 @@ function buildSearchQueryString(queryJSON?: SearchQueryJSON) {
  */
 function buildQueryStringFromFilterFormValues(filterValues: Partial<SearchAdvancedFiltersForm>) {
     // We separate type and status filters from other filters to maintain hashes consistency for saved searches
-    const {type, status, policyID, groupBy, ...otherFilters} = filterValues;
+    const {type, status, groupBy, ...otherFilters} = filterValues;
     const filtersString: string[] = [];
 
     // When switching types/setting the type, ensure we aren't polluting our query with filters that are
@@ -436,11 +436,6 @@ function buildQueryStringFromFilterFormValues(filterValues: Partial<SearchAdvanc
         filtersString.push(`${CONST.SEARCH.SYNTAX_ROOT_KEYS.STATUS}:${filterValueArray.map(sanitizeSearchValue).join(',')}`);
     }
 
-    if (policyID) {
-        const sanitizedPolicyIDs = Array.isArray(policyID) ? policyID.map((id) => sanitizeSearchValue(id)).join(',') : sanitizeSearchValue(policyID);
-        filtersString.push(`${CONST.SEARCH.SYNTAX_FILTER_KEYS.POLICY_ID}:${sanitizedPolicyIDs}`);
-    }
-
     const mappedFilters = Object.entries(otherFilters)
         .map(([filterKey, filterValue]) => {
             if (

@parasharrajat parasharrajat Jul 5, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which issue are we solving here?

@shubham1206agra shubham1206agra Jul 5, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The query issue.

@parasharrajat parasharrajat Jul 5, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I think we need this logic for sanitizing but, the fix here might be to check the length as the value is an array. Pushing that change now.

@shubham1206agra shubham1206agra Jul 5, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is similar logic below which handles sanitizing

@shubham1206agra shubham1206agra Jul 5, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@parasharrajat See

App/src/libs/SearchQueryUtils.ts

Line 490 in 99e31a5

filterKey === FILTER_KEYS.POLICY_ID ||

@parasharrajat parasharrajat Jul 5, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see.

@parasharrajat parasharrajat Jul 5, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.


const resetChanges = useCallback(() => {
updateWorkspaceFilter(null);
setSelectedOptions([]);
}, []);

return (
Expand Down
Loading