From 2cd561513615fa49ae960e2ea786dbd60f49e90f Mon Sep 17 00:00:00 2001 From: Aniket Katkar Date: Thu, 8 Dec 2022 19:11:49 +0530 Subject: [PATCH] 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 --- .../resources/ui/src/enums/Explore.enum.ts | 17 +++++ .../pages/explore/ExplorePage.component.tsx | 24 ++++-- .../pages/explore/ExplorePage.interface.ts | 23 ++++++ .../ExplorePage/ExplorePageUtils.test.ts | 67 ++++++++++++++++ .../src/utils/ExplorePage/ExplorePageUtils.ts | 65 ++++++++++++++++ .../mocks/ExplorePageUtils.mock.ts | 76 +++++++++++++++++++ 6 files changed, 266 insertions(+), 6 deletions(-) create mode 100644 openmetadata-ui/src/main/resources/ui/src/enums/Explore.enum.ts create mode 100644 openmetadata-ui/src/main/resources/ui/src/pages/explore/ExplorePage.interface.ts create mode 100644 openmetadata-ui/src/main/resources/ui/src/utils/ExplorePage/ExplorePageUtils.test.ts create mode 100644 openmetadata-ui/src/main/resources/ui/src/utils/ExplorePage/ExplorePageUtils.ts create mode 100644 openmetadata-ui/src/main/resources/ui/src/utils/ExplorePage/mocks/ExplorePageUtils.mock.ts diff --git a/openmetadata-ui/src/main/resources/ui/src/enums/Explore.enum.ts b/openmetadata-ui/src/main/resources/ui/src/enums/Explore.enum.ts new file mode 100644 index 00000000000..82e307fee84 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/src/enums/Explore.enum.ts @@ -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', +} diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/explore/ExplorePage.component.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/explore/ExplorePage.component.tsx index fed7ef6cdd4..21f89b18c67 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/explore/ExplorePage.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/explore/ExplorePage.component.tsx @@ -36,11 +36,13 @@ import { INITIAL_SORT_ORDER, tabsInfo, } from '../../constants/explore.constants'; +import { getCombinedQueryFilterObject } from '../../utils/ExplorePage/ExplorePageUtils'; import { filterObjectToElasticsearchQuery, isFilterObject, } from '../../utils/FilterUtils'; import { showErrorToast } from '../../utils/ToastUtils'; +import { QueryFilterInterface } from './ExplorePage.interface'; const ExplorePage: FunctionComponent = () => { const location = useLocation(); @@ -80,7 +82,7 @@ const ExplorePage: FunctionComponent = () => { [location.search] ); - const elasticsearchPostFilterQuery = useMemo( + const elasticsearchQueryFilter = useMemo( () => filterObjectToElasticsearchQuery(postFilter), [postFilter] ); @@ -185,14 +187,25 @@ const ExplorePage: FunctionComponent = () => { return showDeletedParam === 'true'; }, [parsedSearch.showDeleted]); + const combinedQueryFilter = useMemo( + () => + // Both query filter objects have type as Record + // 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(() => { setIsLoading(true); Promise.all([ searchQuery({ query: searchQueryParam, searchIndex, - queryFilter: advancesSearchQueryFilter, - postFilter: elasticsearchPostFilterQuery, + queryFilter: combinedQueryFilter, sortField: sortValue, sortOrder, pageNumber: page, @@ -213,8 +226,7 @@ const ExplorePage: FunctionComponent = () => { query: searchQueryParam, pageNumber: 0, pageSize: 0, - queryFilter: advancesSearchQueryFilter, - postFilter: elasticsearchPostFilterQuery, + queryFilter: combinedQueryFilter, searchIndex: index, includeDeleted: showDeleted, trackTotalHits: true, @@ -248,7 +260,7 @@ const ExplorePage: FunctionComponent = () => { sortOrder, showDeleted, advancesSearchQueryFilter, - elasticsearchPostFilterQuery, + elasticsearchQueryFilter, page, ]); diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/explore/ExplorePage.interface.ts b/openmetadata-ui/src/main/resources/ui/src/pages/explore/ExplorePage.interface.ts new file mode 100644 index 00000000000..6745df44775 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/src/pages/explore/ExplorePage.interface.ts @@ -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; +} diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/ExplorePage/ExplorePageUtils.test.ts b/openmetadata-ui/src/main/resources/ui/src/utils/ExplorePage/ExplorePageUtils.test.ts new file mode 100644 index 00000000000..8bf504180a2 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/src/utils/ExplorePage/ExplorePageUtils.test.ts @@ -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 + // 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([]); + }); +}); diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/ExplorePage/ExplorePageUtils.ts b/openmetadata-ui/src/main/resources/ui/src/utils/ExplorePage/ExplorePageUtils.ts new file mode 100644 index 00000000000..ebdaf057d5f --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/src/utils/ExplorePage/ExplorePageUtils.ts @@ -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 }), + }, + }, + }; +}; diff --git a/openmetadata-ui/src/main/resources/ui/src/utils/ExplorePage/mocks/ExplorePageUtils.mock.ts b/openmetadata-ui/src/main/resources/ui/src/utils/ExplorePage/mocks/ExplorePageUtils.mock.ts new file mode 100644 index 00000000000..f414f8521a0 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/src/utils/ExplorePage/mocks/ExplorePageUtils.mock.ts @@ -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' } }] } }, +];