From 0937deaee4ba30ad94d7f52473b35f46e86a78eb Mon Sep 17 00:00:00 2001 From: Pranita Date: Thu, 18 Sep 2025 16:16:30 +0530 Subject: [PATCH] address PR comments --- .../Auth/AuthProviders/AuthProvider.tsx | 76 ++---------- .../AppPipelineModel/AddPipeLineModal.tsx | 15 +-- .../DomainsWidget/DomainsWidget.test.tsx | 113 ++++++++---------- .../Widgets/DomainsWidget/DomainsWidget.tsx | 17 ++- .../AddQueryPage/AddQueryPage.component.tsx | 17 ++- .../pages/AddQueryPage/AddQueryPage.test.tsx | 4 +- .../main/resources/ui/src/rest/searchAPI.ts | 57 ++------- 7 files changed, 95 insertions(+), 204 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/AuthProvider.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/AuthProvider.tsx index 24499b05115..a83929ddd2e 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/AuthProvider.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/AuthProvider.tsx @@ -26,7 +26,7 @@ import { InternalAxiosRequestConfig, } from 'axios'; import { CookieStorage } from 'cookie-storage'; -import { isNil, isNumber } from 'lodash'; +import { isEmpty, isNil, isNumber } from 'lodash'; import { WebStorageStateStore } from 'oidc-client'; import Qs from 'qs'; import { @@ -61,7 +61,6 @@ import { AuthProvider as AuthProviderEnum } from '../../../generated/settings/se import { useApplicationStore } from '../../../hooks/useApplicationStore'; import useCustomLocation from '../../../hooks/useCustomLocation/useCustomLocation'; import { useDomainStore } from '../../../hooks/useDomainStore'; -import { QueryFilterInterface } from '../../../pages/ExplorePage/ExplorePage.interface'; import axiosClient from '../../../rest'; import { getDomainList } from '../../../rest/domainAPI'; import { @@ -480,69 +479,18 @@ export const AuthProvider = ({ } // Parse and update the query parameter - const urlParams = config.url.includes('?') - ? Qs.parse(config.url.split('?')[1]) - : {}; - const queryParams = { ...urlParams, ...config.params }; + const queryParams = Qs.parse(config.url.split('?')[1]); + // adding quotes for exact matching + const domainStatement = `(domains.fullyQualifiedName:"${escapeESReservedCharacters( + activeDomain + )}")`; + queryParams.q = queryParams.q ?? ''; + queryParams.q += isEmpty(queryParams.q) + ? domainStatement + : ` AND ${domainStatement}`; - if (config.url.includes('?')) { - config.url = config.url.split('?')[0]; - } - - const domainStatement = escapeESReservedCharacters(activeDomain); - - // Create domain filter using term query for exact matching - const domainFilter = { - term: { - 'domains.fullyQualifiedName': domainStatement, - }, - }; - - const createDomainFilter = () => ({ - query: domainFilter, - }); - - const mergeWithDomainFilter = ( - existingMust: QueryFilterInterface[] - ) => ({ - query: { - bool: { - must: [...existingMust, domainFilter], - }, - }, - }); - - if (queryParams.query_filter) { - try { - const existingFilter = JSON.parse( - queryParams.query_filter as string - ); - const existingQuery = existingFilter.query || existingFilter; - - if ( - existingQuery?.bool?.must && - Array.isArray(existingQuery.bool.must) - ) { - queryParams.query_filter = JSON.stringify( - mergeWithDomainFilter(existingQuery.bool.must) - ); - } else if (existingQuery && typeof existingQuery === 'object') { - queryParams.query_filter = JSON.stringify( - mergeWithDomainFilter([existingQuery]) - ); - } else { - queryParams.query_filter = JSON.stringify(createDomainFilter()); - } - } catch (error) { - queryParams.query_filter = JSON.stringify(createDomainFilter()); - } - } else { - queryParams.query_filter = JSON.stringify(createDomainFilter()); - } - - config.params = { - ...queryParams, - }; + // Update the URL with the modified query parameter + config.url = `${config.url.split('?')[0]}?${Qs.stringify(queryParams)}`; } else { config.params = { ...config.params, diff --git a/openmetadata-ui/src/main/resources/ui/src/components/Entity/EntityLineage/AppPipelineModel/AddPipeLineModal.tsx b/openmetadata-ui/src/main/resources/ui/src/components/Entity/EntityLineage/AppPipelineModel/AddPipeLineModal.tsx index 6ec1b2f637f..e86eb076ad3 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/Entity/EntityLineage/AppPipelineModel/AddPipeLineModal.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/Entity/EntityLineage/AppPipelineModel/AddPipeLineModal.tsx @@ -24,7 +24,7 @@ import { ERROR_PLACEHOLDER_TYPE, SIZE } from '../../../../enums/common.enum'; import { EntityType } from '../../../../enums/entity.enum'; import { SearchIndex } from '../../../../enums/search.enum'; import { EntityReference } from '../../../../generated/entity/type'; -import { searchData } from '../../../../rest/miscAPI'; +import { searchQuery } from '../../../../rest/searchAPI'; import { getEntityName, getEntityReferenceFromEntity, @@ -69,12 +69,13 @@ const AddPipeLineModal = ({ const getSearchResults = async (value = '*') => { try { - const data = await searchData(value, 1, PAGE_SIZE, '', '', '', [ - SearchIndex.PIPELINE, - SearchIndex.STORED_PROCEDURE, - ]); - - const edgeOptions = data.data.hits.hits.map((hit) => + const data = await searchQuery({ + query: value, + pageNumber: 1, + pageSize: PAGE_SIZE, + searchIndex: [SearchIndex.PIPELINE, SearchIndex.STORED_PROCEDURE], + }); + const edgeOptions = data.hits.hits.map((hit) => getEntityReferenceFromEntity( hit._source, hit._source.entityType as EntityType diff --git a/openmetadata-ui/src/main/resources/ui/src/components/MyData/Widgets/DomainsWidget/DomainsWidget.test.tsx b/openmetadata-ui/src/main/resources/ui/src/components/MyData/Widgets/DomainsWidget/DomainsWidget.test.tsx index 93c9682b70e..64147340270 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/MyData/Widgets/DomainsWidget/DomainsWidget.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/MyData/Widgets/DomainsWidget/DomainsWidget.test.tsx @@ -22,7 +22,7 @@ import { Domain, DomainType, } from '../../../../generated/entity/domains/domain'; -import { searchData } from '../../../../rest/miscAPI'; +import { searchQuery } from '../../../../rest/searchAPI'; import DomainsWidget from './DomainsWidget'; const mockProps = { @@ -67,26 +67,20 @@ const mockDomains: Domain[] = [ ]; const mockSearchResponse = { - data: { - hits: { - hits: mockDomains.map((domain) => ({ - _source: domain, - _index: 'domain_search_index', - _id: domain.id, - })), - total: { value: mockDomains.length }, - }, - aggregations: {}, + hits: { + hits: mockDomains.map((domain) => ({ + _source: domain, + _index: 'domain_search_index', + _id: domain.id, + })), + total: { value: mockDomains.length }, }, - status: 200, - statusText: 'OK', - headers: {}, - config: {}, + aggregations: {}, } as any; // Mock API functions -jest.mock('../../../../rest/miscAPI', () => ({ - searchData: jest.fn(), +jest.mock('../../../../rest/searchAPI', () => ({ + searchQuery: jest.fn(), })); jest.mock('../../../../constants/Widgets.constant', () => ({ @@ -99,7 +93,7 @@ jest.mock('../../../../utils/DomainUtils', () => ({ getDomainIcon: jest.fn().mockReturnValue(
), })); -const mockSearchData = searchData as jest.MockedFunction; +const mockSearchQuery = searchQuery as jest.MockedFunction; const mockGetSortField = getSortField as jest.MockedFunction< typeof getSortField @@ -121,7 +115,7 @@ describe('DomainsWidget', () => { mockGetSortField.mockReturnValue('updatedAt'); mockGetSortOrder.mockReturnValue('desc'); mockApplySortToData.mockImplementation((data) => data); - mockSearchData.mockResolvedValue(mockSearchResponse); + mockSearchQuery.mockResolvedValue(mockSearchResponse); }); const renderDomainsWidget = (props = {}) => { @@ -161,15 +155,13 @@ describe('DomainsWidget', () => { }); it('renders empty state when no domains', async () => { - mockSearchData.mockResolvedValue({ + mockSearchQuery.mockResolvedValue({ ...mockSearchResponse, - data: { - hits: { - hits: [], - total: { value: 0 }, - }, - aggregations: {}, + hits: { + hits: [], + total: { value: 0 }, }, + aggregations: {}, }); renderDomainsWidget(); @@ -184,7 +176,7 @@ describe('DomainsWidget', () => { }); it('renders error state when API fails', async () => { - mockSearchData.mockRejectedValue(new Error('API Error')); + mockSearchQuery.mockRejectedValue(new Error('API Error')); renderDomainsWidget(); @@ -196,19 +188,18 @@ describe('DomainsWidget', () => { }); }); - it('calls searchData with correct parameters on mount', async () => { + it('calls searchQuery with correct parameters on mount', async () => { renderDomainsWidget(); await waitFor(() => { - expect(mockSearchData).toHaveBeenCalledWith( - '', - 1, - PAGE_SIZE_MEDIUM, - '', - 'updatedAt', - 'desc', - 'domain_search_index' - ); + expect(mockSearchQuery).toHaveBeenCalledWith({ + query: '', + pageNumber: 1, + pageSize: PAGE_SIZE_MEDIUM, + sortField: 'updatedAt', + sortOrder: 'desc', + searchIndex: 'domain_search_index', + }); }); }); @@ -231,7 +222,7 @@ describe('DomainsWidget', () => { // Simulate sort option selection - this would trigger the callback // Since the dropdown behavior is complex, we'll test the effect // by verifying the API is called again with new sort parameters - expect(mockSearchData).toHaveBeenCalled(); + expect(mockSearchQuery).toHaveBeenCalled(); }); it('renders domains in full size layout', async () => { @@ -321,19 +312,17 @@ describe('DomainsWidget', () => { }) ); - mockSearchData.mockResolvedValue({ + mockSearchQuery.mockResolvedValue({ ...mockSearchResponse, - data: { - hits: { - hits: manyDomains.map((domain) => ({ - _source: domain, - _index: 'domain_search_index', - _id: domain.id, - })), - total: { value: manyDomains.length }, - }, - aggregations: {}, + hits: { + hits: manyDomains.map((domain) => ({ + _source: domain, + _index: 'domain_search_index', + _id: domain.id, + })), + total: { value: manyDomains.length }, }, + aggregations: {}, }); renderDomainsWidget(); @@ -358,7 +347,7 @@ describe('DomainsWidget', () => { }); it('handles loading state correctly', () => { - mockSearchData.mockImplementation( + mockSearchQuery.mockImplementation( () => new Promise(() => { // Never resolves to simulate loading state @@ -377,21 +366,19 @@ describe('DomainsWidget', () => { assets: undefined, }; - mockSearchData.mockResolvedValue({ + mockSearchQuery.mockResolvedValue({ ...mockSearchResponse, - data: { - hits: { - hits: [ - { - _source: domainWithNoAssets, - _index: 'domain_search_index', - _id: domainWithNoAssets.id, - }, - ], - total: { value: 1 }, - }, - aggregations: {}, + hits: { + hits: [ + { + _source: domainWithNoAssets, + _index: 'domain_search_index', + _id: domainWithNoAssets.id, + }, + ], + total: { value: 1 }, }, + aggregations: {}, }); renderDomainsWidget(); diff --git a/openmetadata-ui/src/main/resources/ui/src/components/MyData/Widgets/DomainsWidget/DomainsWidget.tsx b/openmetadata-ui/src/main/resources/ui/src/components/MyData/Widgets/DomainsWidget/DomainsWidget.tsx index 7234ba71e16..ae356500db9 100644 --- a/openmetadata-ui/src/main/resources/ui/src/components/MyData/Widgets/DomainsWidget/DomainsWidget.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/components/MyData/Widgets/DomainsWidget/DomainsWidget.tsx @@ -36,7 +36,7 @@ import { WidgetCommonProps, WidgetConfig, } from '../../../../pages/CustomizablePage/CustomizablePage.interface'; -import { searchData } from '../../../../rest/miscAPI'; +import { searchQuery } from '../../../../rest/searchAPI'; import { getDomainIcon } from '../../../../utils/DomainUtils'; import { getDomainDetailsPath } from '../../../../utils/RouterUtils'; import ErrorPlaceHolder from '../../../common/ErrorWithPlaceholder/ErrorPlaceHolder'; @@ -73,17 +73,16 @@ const DomainsWidget = ({ const sortField = getSortField(selectedSortBy); const sortOrder = getSortOrder(selectedSortBy); - const res = await searchData( - '', - INITIAL_PAGING_VALUE, - PAGE_SIZE_MEDIUM, - '', + const res = await searchQuery({ + query: '', + pageNumber: INITIAL_PAGING_VALUE, + pageSize: PAGE_SIZE_MEDIUM, sortField, sortOrder, - SearchIndex.DOMAIN - ); + searchIndex: SearchIndex.DOMAIN, + }); - const domains = res?.data?.hits?.hits.map((hit) => hit._source); + const domains = res?.hits?.hits.map((hit) => hit._source); const sortedDomains = applySortToData(domains, selectedSortBy); setDomains(sortedDomains as Domain[]); } catch { diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/AddQueryPage/AddQueryPage.component.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/AddQueryPage/AddQueryPage.component.tsx index 97c549fdc8c..a7d9dcbb02d 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/AddQueryPage/AddQueryPage.component.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/AddQueryPage/AddQueryPage.component.tsx @@ -39,8 +39,8 @@ import { withPageLayout } from '../../hoc/withPageLayout'; import { useApplicationStore } from '../../hooks/useApplicationStore'; import { useFqn } from '../../hooks/useFqn'; import { FieldProp, FieldTypes } from '../../interface/FormUtils.interface'; -import { searchData } from '../../rest/miscAPI'; import { postQuery } from '../../rest/queryAPI'; +import { searchQuery } from '../../rest/searchAPI'; import { getTableDetailsByFQN } from '../../rest/tableAPI'; import { getPartialNameFromFQN } from '../../utils/CommonUtils'; import { getCurrentMillis } from '../../utils/date-time/DateTimeUtils'; @@ -99,15 +99,12 @@ const AddQueryPage = () => { searchValue = '' ): Promise => { try { - const { data } = await searchData( - searchValue, - INITIAL_PAGING_VALUE, - PAGE_SIZE_MEDIUM, - '', - '', - '', - SearchIndex.TABLE - ); + const data = await searchQuery({ + query: searchValue, + pageNumber: INITIAL_PAGING_VALUE, + pageSize: PAGE_SIZE_MEDIUM, + searchIndex: SearchIndex.TABLE, + }); const options = data.hits.hits.map((value) => ({ label: getEntityLabel(value._source), value: value._source.id, diff --git a/openmetadata-ui/src/main/resources/ui/src/pages/AddQueryPage/AddQueryPage.test.tsx b/openmetadata-ui/src/main/resources/ui/src/pages/AddQueryPage/AddQueryPage.test.tsx index b1bb130ae1a..0f602d35bf7 100644 --- a/openmetadata-ui/src/main/resources/ui/src/pages/AddQueryPage/AddQueryPage.test.tsx +++ b/openmetadata-ui/src/main/resources/ui/src/pages/AddQueryPage/AddQueryPage.test.tsx @@ -22,8 +22,8 @@ jest.mock('../../rest/tableAPI', () => ({ jest.mock('../../rest/queryAPI', () => ({ postQuery: jest.fn().mockImplementation(() => Promise.resolve()), })); -jest.mock('../../rest/miscAPI', () => ({ - searchData: jest.fn().mockImplementation(() => Promise.resolve()), +jest.mock('../../rest/searchAPI', () => ({ + searchQuery: jest.fn().mockImplementation(() => Promise.resolve()), })); jest.mock('react-router-dom', () => ({ useParams: jest.fn().mockReturnValue({ fqn: MOCK_TABLE.fullyQualifiedName }), diff --git a/openmetadata-ui/src/main/resources/ui/src/rest/searchAPI.ts b/openmetadata-ui/src/main/resources/ui/src/rest/searchAPI.ts index eed71f36d16..6949cb659f2 100644 --- a/openmetadata-ui/src/main/resources/ui/src/rest/searchAPI.ts +++ b/openmetadata-ui/src/main/resources/ui/src/rest/searchAPI.ts @@ -176,55 +176,14 @@ export const rawSearchQuery = < const queryWithSlash = getQueryWithSlash(query || ''); - // Build structured queryFilter to handle filters properly - let structuredQueryFilter = queryFilter; + const apiQuery = + query && query !== '**' + ? filters + ? `${queryWithSlash} AND ` + : queryWithSlash + : ''; - // If filters is provided (for backward compatibility with simple field:value filters) - // Combine it with the existing queryFilter - if (filters) { - // Check if filters contains AND or OR operators (which shouldn't happen in new code) - if (filters.includes(' AND ') || filters.includes(' OR ')) { - // For backward compatibility, handle complex filters with AND/OR using query_string - const filterQuery = { - query_string: { - query: filters, - default_field: '*', - }, - }; - - structuredQueryFilter = queryFilter - ? { - query: { - bool: { - must: [queryFilter.query || queryFilter, filterQuery], - }, - }, - } - : { query: filterQuery }; - } else if (filters.includes(':')) { - // Simple field:value filters (backward compatibility) - const filterQuery = { - query_string: { - query: filters, - default_field: '*', - }, - }; - - structuredQueryFilter = queryFilter - ? { - query: { - bool: { - must: [queryFilter.query || queryFilter, filterQuery], - }, - }, - } - : { query: filterQuery }; - } - } - - // The q parameter should only contain the search text, no filters - const apiQuery = query && query !== '**' ? queryWithSlash : ''; - const apiUrl = `/search/query?q=${apiQuery}`; + const apiUrl = `/search/query?q=${apiQuery}${filters ?? ''}`; return APIClient.get< SearchResponse< @@ -237,7 +196,7 @@ export const rawSearchQuery = < from: (pageNumber - 1) * pageSize, size: pageSize, deleted: includeDeleted, - query_filter: JSON.stringify(structuredQueryFilter), + query_filter: JSON.stringify(queryFilter), post_filter: JSON.stringify(postFilter), sort_field: sortField, sort_order: sortOrder,