Fix(UI): Regression in search filters (#9214)

* Added functionality to combine the query filters for Elasticsearch and Advanced Search

* Added unit tests for newly added util functions for explore page

* Added license for enum file

* Code optimization and added comments in the code
This commit is contained in:
Aniket Katkar 2022-12-08 19:11:49 +05:30 committed by GitHub
parent 6166d193a3
commit 2cd5615136
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 266 additions and 6 deletions

View File

@ -0,0 +1,17 @@
/*
* Copyright 2022 Collate
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
export enum QueryFilterFieldsEnum {
MUST = 'must',
SHOULD = 'should',
}

View File

@ -36,11 +36,13 @@ import {
INITIAL_SORT_ORDER, INITIAL_SORT_ORDER,
tabsInfo, tabsInfo,
} from '../../constants/explore.constants'; } from '../../constants/explore.constants';
import { getCombinedQueryFilterObject } from '../../utils/ExplorePage/ExplorePageUtils';
import { import {
filterObjectToElasticsearchQuery, filterObjectToElasticsearchQuery,
isFilterObject, isFilterObject,
} from '../../utils/FilterUtils'; } from '../../utils/FilterUtils';
import { showErrorToast } from '../../utils/ToastUtils'; import { showErrorToast } from '../../utils/ToastUtils';
import { QueryFilterInterface } from './ExplorePage.interface';
const ExplorePage: FunctionComponent = () => { const ExplorePage: FunctionComponent = () => {
const location = useLocation(); const location = useLocation();
@ -80,7 +82,7 @@ const ExplorePage: FunctionComponent = () => {
[location.search] [location.search]
); );
const elasticsearchPostFilterQuery = useMemo( const elasticsearchQueryFilter = useMemo(
() => filterObjectToElasticsearchQuery(postFilter), () => filterObjectToElasticsearchQuery(postFilter),
[postFilter] [postFilter]
); );
@ -185,14 +187,25 @@ const ExplorePage: FunctionComponent = () => {
return showDeletedParam === 'true'; return showDeletedParam === 'true';
}, [parsedSearch.showDeleted]); }, [parsedSearch.showDeleted]);
const combinedQueryFilter = useMemo(
() =>
// Both query filter objects have type as Record<string, unknown>
// Here unknown will not allow us to directly access the properties
// That is why I first did typecast it into QueryFilterInterface type to access the properties.
getCombinedQueryFilterObject(
elasticsearchQueryFilter as unknown as QueryFilterInterface,
advancesSearchQueryFilter as unknown as QueryFilterInterface
),
[elasticsearchQueryFilter, advancesSearchQueryFilter]
);
useDeepCompareEffect(() => { useDeepCompareEffect(() => {
setIsLoading(true); setIsLoading(true);
Promise.all([ Promise.all([
searchQuery({ searchQuery({
query: searchQueryParam, query: searchQueryParam,
searchIndex, searchIndex,
queryFilter: advancesSearchQueryFilter, queryFilter: combinedQueryFilter,
postFilter: elasticsearchPostFilterQuery,
sortField: sortValue, sortField: sortValue,
sortOrder, sortOrder,
pageNumber: page, pageNumber: page,
@ -213,8 +226,7 @@ const ExplorePage: FunctionComponent = () => {
query: searchQueryParam, query: searchQueryParam,
pageNumber: 0, pageNumber: 0,
pageSize: 0, pageSize: 0,
queryFilter: advancesSearchQueryFilter, queryFilter: combinedQueryFilter,
postFilter: elasticsearchPostFilterQuery,
searchIndex: index, searchIndex: index,
includeDeleted: showDeleted, includeDeleted: showDeleted,
trackTotalHits: true, trackTotalHits: true,
@ -248,7 +260,7 @@ const ExplorePage: FunctionComponent = () => {
sortOrder, sortOrder,
showDeleted, showDeleted,
advancesSearchQueryFilter, advancesSearchQueryFilter,
elasticsearchPostFilterQuery, elasticsearchQueryFilter,
page, page,
]); ]);

View File

@ -0,0 +1,23 @@
/*
* Copyright 2022 Collate
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
export interface QueryFieldInterface {
bool: {
must?: QueryFilterInterface[];
should?: QueryFilterInterface[];
};
}
export interface QueryFilterInterface {
query: QueryFieldInterface;
}

View File

@ -0,0 +1,67 @@
/*
* Copyright 2022 Collate
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { QueryFilterFieldsEnum } from '../../enums/Explore.enum';
import { QueryFilterInterface } from '../../pages/explore/ExplorePage.interface';
import {
getCombinedFields,
getCombinedQueryFilterObject,
getQueryFiltersArray,
} from './ExplorePageUtils';
import {
mockAdvancedSearchQueryFilters,
mockCombinedMustFieldArray,
mockCombinedQueryFilterValue,
mockESQueryFilters,
mockQueryFilterArray,
} from './mocks/ExplorePageUtils.mock';
describe('ExplorePageUtils test', () => {
it('Function getCombinedQueryFilterObject should return proper combined filters for two different query filter objects', () => {
// Both query filter objects have type as Record<string, unknown>
// Here unknown will not allow us to directly access the properties
// That is why I first did typecast it into QueryFilterInterface type to access the properties.
const combinedQueryFilterObject = getCombinedQueryFilterObject(
mockESQueryFilters as unknown as QueryFilterInterface,
mockAdvancedSearchQueryFilters as unknown as QueryFilterInterface
);
expect(combinedQueryFilterObject).toEqual(mockCombinedQueryFilterValue);
});
it('Function getCombinedFields should return the value in the correct field given in the input', () => {
const combinedMustFieldArray = getCombinedFields(
QueryFilterFieldsEnum.MUST,
mockESQueryFilters as unknown as QueryFilterInterface,
mockAdvancedSearchQueryFilters as unknown as QueryFilterInterface
);
expect(combinedMustFieldArray).toEqual(mockCombinedMustFieldArray);
});
it('Function getQueryFiltersArray should return the array for non empty input array', () => {
const queryFilterArray = getQueryFiltersArray(
mockESQueryFilters.query.bool.must as unknown as QueryFilterInterface[]
);
expect(queryFilterArray).toEqual(mockQueryFilterArray);
});
it('Function getQueryFiltersArray should return an empty array for undefined or empty array input', () => {
const queryFilterArrayUndefined = getQueryFiltersArray(undefined);
const queryFilterArrayEmpty = getQueryFiltersArray([]);
expect(queryFilterArrayUndefined).toEqual([]);
expect(queryFilterArrayEmpty).toEqual([]);
});
});

View File

@ -0,0 +1,65 @@
/*
* Copyright 2022 Collate
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { isEmpty, isUndefined } from 'lodash';
import { QueryFilterFieldsEnum } from '../../enums/Explore.enum';
import { QueryFilterInterface } from '../../pages/explore/ExplorePage.interface';
export const getQueryFiltersArray = (
queryFilters: QueryFilterInterface[] | undefined
) => (isUndefined(queryFilters) ? [] : queryFilters);
export const getCombinedFields = (
field: QueryFilterFieldsEnum,
elasticsearchQueryFilter?: QueryFilterInterface,
advancesSearchQueryFilter?: QueryFilterInterface
): QueryFilterInterface[] => {
switch (field) {
case QueryFilterFieldsEnum.SHOULD: {
return [
...getQueryFiltersArray(elasticsearchQueryFilter?.query?.bool?.should),
...getQueryFiltersArray(advancesSearchQueryFilter?.query?.bool?.should),
];
}
case QueryFilterFieldsEnum.MUST: {
return [
...getQueryFiltersArray(elasticsearchQueryFilter?.query?.bool?.must),
...getQueryFiltersArray(advancesSearchQueryFilter?.query?.bool?.must),
];
}
}
};
export const getCombinedQueryFilterObject = (
elasticsearchQueryFilter?: QueryFilterInterface,
advancesSearchQueryFilter?: QueryFilterInterface
) => {
const mustField = getCombinedFields(
QueryFilterFieldsEnum.MUST,
elasticsearchQueryFilter,
advancesSearchQueryFilter
);
const shouldField = getCombinedFields(
QueryFilterFieldsEnum.SHOULD,
elasticsearchQueryFilter,
advancesSearchQueryFilter
);
return {
query: {
bool: {
...(isEmpty(mustField) ? {} : { must: mustField }),
...(isEmpty(shouldField) ? {} : { should: shouldField }),
},
},
};
};

View File

@ -0,0 +1,76 @@
/*
* Copyright 2022 Collate
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
export const mockAdvancedSearchQueryFilters = {
query: {
bool: {
must: [
{ bool: { must: [{ term: { 'owner.type': 'team' } }] } },
{
bool: {
must: [
{ term: { 'service.name': 'sample_data' } },
{ term: { 'databaseSchema.name': 'shopify' } },
],
},
},
],
},
},
};
export const mockESQueryFilters = {
query: {
bool: {
must: [
{ bool: { should: [{ term: { 'tags.tagFQN': 'PII.Sensitive' } }] } },
],
},
},
};
export const mockCombinedQueryFilterValue = {
query: {
bool: {
must: [
{ bool: { should: [{ term: { 'tags.tagFQN': 'PII.Sensitive' } }] } },
{ bool: { must: [{ term: { 'owner.type': 'team' } }] } },
{
bool: {
must: [
{ term: { 'service.name': 'sample_data' } },
{ term: { 'databaseSchema.name': 'shopify' } },
],
},
},
],
},
},
};
export const mockCombinedMustFieldArray = [
{ bool: { should: [{ term: { 'tags.tagFQN': 'PII.Sensitive' } }] } },
{ bool: { must: [{ term: { 'owner.type': 'team' } }] } },
{
bool: {
must: [
{ term: { 'service.name': 'sample_data' } },
{ term: { 'databaseSchema.name': 'shopify' } },
],
},
},
];
export const mockQueryFilterArray = [
{ bool: { should: [{ term: { 'tags.tagFQN': 'PII.Sensitive' } }] } },
];