Skip to content

Commit b38b386

Browse files
satender-kumar-collateSatender
andauthored
Fixes 26694 (#28330)
* added fix for 26694 * updated code as per Gitar comment * added E2E test cases --------- Co-authored-by: Satender <sommy@Satenders-MacBook-Pro.local>
1 parent c9a5bdb commit b38b386

5 files changed

Lines changed: 178 additions & 17 deletions

File tree

openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ExploreQuickFilters.spec.ts

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -329,6 +329,93 @@ test.describe('Tier filter - aggregation-based options', () => {
329329
});
330330
});
331331

332+
test.describe('Filter persistence after bug fixes', () => {
333+
test('explore tree sidebar selection is not cleared when a top dropdown filter is applied', async ({
334+
page,
335+
}) => {
336+
test.slow();
337+
338+
await test.step('Click on Databases in the explore tree to select it', async () => {
339+
const treeSearchRes = page.waitForResponse(
340+
(resp) =>
341+
resp.url().includes('/api/v1/search/query') &&
342+
resp.url().includes('index=dataAsset')
343+
);
344+
await page.getByTestId('explore-tree-title-Databases').click();
345+
await treeSearchRes;
346+
await waitForAllLoadersToDisappear(page);
347+
});
348+
349+
await test.step('Verify the Databases node is marked as selected', async () => {
350+
await expect(page.locator('.ant-tree-node-selected')).toBeVisible();
351+
});
352+
353+
await test.step('Apply Tag filter from top dropdown', async () => {
354+
await page.getByTestId('search-dropdown-Tag').click();
355+
await searchAndClickOnOption(
356+
page,
357+
{ key: 'tags.tagFQN', label: 'Tag', value: 'PersonalData.Personal' },
358+
true
359+
);
360+
const queryRes = page.waitForResponse(
361+
'/api/v1/search/query?*index=dataAsset*'
362+
);
363+
await page.getByTestId('update-btn').click();
364+
await queryRes;
365+
await waitForAllLoadersToDisappear(page);
366+
});
367+
368+
await test.step('Verify Databases node selection is still preserved after filter change', async () => {
369+
await expect(page.locator('.ant-tree-node-selected')).toBeVisible();
370+
});
371+
});
372+
373+
test('sort order is preserved in URL when explore tree node is clicked after applying a top dropdown filter', async ({
374+
page,
375+
}) => {
376+
test.slow();
377+
378+
await test.step('Toggle sort order to ascending', async () => {
379+
const sortRes = page.waitForResponse(
380+
'/api/v1/search/query?*sort_order=asc*'
381+
);
382+
await page.getByTestId('sort-order-button').click();
383+
await sortRes;
384+
await waitForAllLoadersToDisappear(page);
385+
});
386+
387+
await test.step('Apply Tag filter from top dropdown', async () => {
388+
await page.getByTestId('search-dropdown-Tag').click();
389+
await searchAndClickOnOption(
390+
page,
391+
{ key: 'tags.tagFQN', label: 'Tag', value: 'PersonalData.Personal' },
392+
true
393+
);
394+
const queryRes = page.waitForResponse(
395+
'/api/v1/search/query?*index=dataAsset*'
396+
);
397+
await page.getByTestId('update-btn').click();
398+
await queryRes;
399+
await waitForAllLoadersToDisappear(page);
400+
});
401+
402+
await test.step('Click on Databases in the explore tree', async () => {
403+
const treeSearchRes = page.waitForResponse(
404+
(resp) =>
405+
resp.url().includes('/api/v1/search/query') &&
406+
resp.url().includes('index=dataAsset')
407+
);
408+
await page.getByTestId('explore-tree-title-Databases').click();
409+
await treeSearchRes;
410+
await waitForAllLoadersToDisappear(page);
411+
});
412+
413+
await test.step('Verify sort order is preserved in the URL after tree node click', async () => {
414+
await expect(page).toHaveURL(/sortOrder=asc/);
415+
});
416+
});
417+
});
418+
332419
test.describe('Metric search result highlight', () => {
333420
const metric = new MetricClass();
334421

openmetadata-ui/src/main/resources/ui/src/components/ExploreV1/ExploreV1.component.tsx

Lines changed: 25 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -341,21 +341,24 @@ const ExploreV1: React.FC<ExploreProps> = ({
341341
onResetAllFilters();
342342
};
343343

344-
const handleQuickFiltersChange = (data: ExploreQuickFilterField[]) => {
345-
const must = getExploreQueryFilterMust(data);
346-
347-
onChangeAdvancedSearchQuickFilters(
348-
isEmpty(must)
349-
? undefined
350-
: {
351-
query: {
352-
bool: {
353-
must,
344+
const handleQuickFiltersChange = useCallback(
345+
(data: ExploreQuickFilterField[]) => {
346+
const must = getExploreQueryFilterMust(data);
347+
348+
onChangeAdvancedSearchQuickFilters(
349+
isEmpty(must)
350+
? undefined
351+
: {
352+
query: {
353+
bool: {
354+
must,
355+
},
354356
},
355-
},
356-
}
357-
);
358-
};
357+
}
358+
);
359+
},
360+
[onChangeAdvancedSearchQuickFilters]
361+
);
359362

360363
const handleQuickFiltersValueSelect = (field: ExploreQuickFilterField) => {
361364
setSelectedQuickFilters((pre) => {
@@ -405,7 +408,14 @@ const ExploreV1: React.FC<ExploreProps> = ({
405408
}
406409

407410
return <ExploreTree onFieldValueSelect={handleQuickFiltersChange} />;
408-
}, [searchQueryParam, tabItems]);
411+
}, [
412+
searchQueryParam,
413+
tabItems,
414+
handleQuickFiltersChange,
415+
activeTabKey,
416+
loading,
417+
onChangeSearchIndex,
418+
]);
409419

410420
useEffect(() => {
411421
const escapeKeyHandler = (e: KeyboardEvent) => {

openmetadata-ui/src/main/resources/ui/src/components/ExploreV1/ExploreV1.test.tsx

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import {
2828
MOCK_EXPLORE_TAB_ITEMS,
2929
} from '../Explore/Explore.mock';
3030
import { ExploreSearchIndex } from '../Explore/ExplorePage.interface';
31+
import ExploreTree from '../Explore/ExploreTree/ExploreTree';
3132
import ExploreV1 from './ExploreV1.component';
3233

3334
jest.mock('@openmetadata/ui-core-components', () => {
@@ -522,6 +523,28 @@ describe('ExploreV1', () => {
522523
await waitFor(() => expect(exportButton).toBeEnabled());
523524
});
524525

526+
it('passes updated onFieldValueSelect to ExploreTree when onChangeAdvancedSearchQuickFilters prop changes', () => {
527+
const callbackV1 = jest.fn();
528+
const callbackV2 = jest.fn();
529+
const ExploreTreeMock = ExploreTree as jest.Mock;
530+
531+
const { rerender } = render(
532+
<ExploreV1 {...props} onChangeAdvancedSearchQuickFilters={callbackV1} />,
533+
{ wrapper: Wrapper }
534+
);
535+
536+
rerender(
537+
<ExploreV1 {...props} onChangeAdvancedSearchQuickFilters={callbackV2} />
538+
);
539+
540+
const lastCallProps =
541+
ExploreTreeMock.mock.calls[ExploreTreeMock.mock.calls.length - 1][0];
542+
lastCallProps.onFieldValueSelect([]);
543+
544+
expect(callbackV2).toHaveBeenCalledTimes(1);
545+
expect(callbackV1).not.toHaveBeenCalled();
546+
});
547+
525548
it('disables export and shows alert when all matching assets exceed export limit', async () => {
526549
(searchQuery as jest.Mock).mockResolvedValueOnce({
527550
hits: { total: { value: 200001 }, hits: [] },

openmetadata-ui/src/main/resources/ui/src/pages/ExplorePage/ExplorePageV1.component.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,6 @@ const ExplorePageV1: FC<unknown> = () => {
373373

374374
const handleAdvanceSearchQuickFiltersChange = useCallback(
375375
(filter?: QueryFilterInterface) => {
376-
handlePageChange(1);
377376
setAdvancedSearchQuickFilters(filter);
378377
handleQuickFilterChange(filter);
379378
},

openmetadata-ui/src/main/resources/ui/src/pages/ExplorePage/ExplorePageV1.test.tsx

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,10 @@
1010
* See the License for the specific language governing permissions and
1111
* limitations under the License.
1212
*/
13-
import { render, screen } from '@testing-library/react';
13+
import { act, render, screen } from '@testing-library/react';
1414
import React from 'react';
15+
import { useNavigate } from 'react-router-dom';
16+
import ExploreV1 from '../../components/ExploreV1/ExploreV1.component';
1517
import ExplorePageV1 from './ExplorePageV1.component';
1618

1719
jest.mock(
@@ -67,4 +69,44 @@ describe('ExplorePageV1', () => {
6769

6870
expect(await screen.findByText('ExploreV1')).toBeInTheDocument();
6971
});
72+
73+
it('calls navigate exactly once with quickFilter when filter changes', async () => {
74+
const mockNavigate = jest.fn();
75+
(useNavigate as jest.Mock).mockReturnValue(mockNavigate);
76+
77+
let capturedCallback:
78+
| ((filter?: Record<string, unknown>) => void)
79+
| undefined;
80+
(ExploreV1 as jest.Mock).mockImplementationOnce(
81+
({
82+
onChangeAdvancedSearchQuickFilters,
83+
}: {
84+
onChangeAdvancedSearchQuickFilters?: (
85+
filter?: Record<string, unknown>
86+
) => void;
87+
}) => {
88+
capturedCallback = onChangeAdvancedSearchQuickFilters;
89+
90+
return <p>ExploreV1</p>;
91+
}
92+
);
93+
94+
render(<ExplorePageV1 {...mockProps} />);
95+
await screen.findByText('ExploreV1');
96+
97+
const testFilter = {
98+
query: {
99+
bool: {
100+
must: [{ bool: { should: [{ term: { entityType: 'table' } }] } }],
101+
},
102+
},
103+
};
104+
105+
act(() => {
106+
capturedCallback!(testFilter);
107+
});
108+
109+
expect(mockNavigate).toHaveBeenCalledTimes(1);
110+
expect(mockNavigate.mock.calls[0][0].search).toContain('quickFilter');
111+
});
70112
});

0 commit comments

Comments
 (0)