Skip to content

Commit ed44b50

Browse files
stefanonardoclaude
stefanonardo
and
claude
committed
OCPBUGS-81519: Fix Search page state mutation and unnecessary component remounts
Fix Set reference copy bug in resource selection handlers, replace filter-embedded component keys with stable keys to prevent WebSocket watch teardown, and add memo/debounce to ResourceList to avoid expensive re-renders during typing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 46a9c16 commit ed44b50

3 files changed

Lines changed: 347 additions & 37 deletions

File tree

‎frontend/packages/console-app/src/components/data-view/__tests__/useConsoleDataViewFilters.spec.tsx‎

Lines changed: 258 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,258 @@
1+
import type { FC, ReactNode } from 'react';
2+
import { render, renderHook, act } from '@testing-library/react';
3+
import { MemoryRouter, useNavigate } from 'react-router';
4+
import type { K8sResourceCommon } from '@console/dynamic-plugin-sdk/src/extensions/console-types';
5+
import type { ResourceFilters } from '../types';
6+
import { useConsoleDataViewFilters } from '../useConsoleDataViewFilters';
7+
8+
jest.mock('@console/internal/components/factory/table-filters', () => ({
9+
exactMatch: (filter: string, value: string) => !filter || value?.includes(filter),
10+
fuzzyCaseInsensitive: (filter: string, value: string) =>
11+
!filter || value?.toLowerCase().includes(filter.toLowerCase()),
12+
}));
13+
14+
jest.mock('@console/shared/src/utils/label-filter', () => ({
15+
mapLabelsToStrings: (labels: Record<string, string> = {}) =>
16+
Object.entries(labels).map(([k, v]) => `${k}=${v}`),
17+
}));
18+
19+
jest.mock('@console/app/src/components/user-preferences/search/useExactSearch', () => ({
20+
useExactSearch: jest.fn(() => [false, true]),
21+
}));
22+
23+
const { useExactSearch } = jest.requireMock(
24+
'@console/app/src/components/user-preferences/search/useExactSearch',
25+
) as { useExactSearch: jest.Mock };
26+
27+
const mockData: K8sResourceCommon[] = [
28+
{ metadata: { name: 'api-server', labels: { app: 'api' } }, kind: 'Pod', apiVersion: 'v1' },
29+
{
30+
metadata: { name: 'web-frontend', labels: { app: 'web', tier: 'frontend' } },
31+
kind: 'Pod',
32+
apiVersion: 'v1',
33+
},
34+
{
35+
metadata: { name: 'api-gateway', labels: { app: 'api', tier: 'gateway' } },
36+
kind: 'Pod',
37+
apiVersion: 'v1',
38+
},
39+
];
40+
41+
const initialFilters: ResourceFilters = { name: '', label: '' };
42+
43+
const createWrapper = (initialEntries: string[] = ['/']): FC<{ children: ReactNode }> => {
44+
const Wrapper: FC<{ children: ReactNode }> = ({ children }) => (
45+
<MemoryRouter initialEntries={initialEntries}>{children}</MemoryRouter>
46+
);
47+
Wrapper.displayName = 'MemoryRouterWrapper';
48+
return Wrapper;
49+
};
50+
51+
describe('useConsoleDataViewFilters', () => {
52+
let consoleErrorSpy: jest.SpyInstance;
53+
54+
beforeEach(() => {
55+
useExactSearch.mockReturnValue([false, true]);
56+
// Suppress React warning about render-phase updates from PF's useDataViewFilters
57+
consoleErrorSpy = jest.spyOn(console, 'error').mockImplementation((msg: string) => {
58+
if (typeof msg === 'string' && msg.includes('Cannot update a component')) {
59+
// noop
60+
}
61+
});
62+
});
63+
64+
afterEach(() => {
65+
consoleErrorSpy.mockRestore();
66+
jest.restoreAllMocks();
67+
});
68+
69+
it('should return all data when no filters are set', () => {
70+
const { result } = renderHook(
71+
() => useConsoleDataViewFilters({ data: mockData, initialFilters }),
72+
{ wrapper: createWrapper() },
73+
);
74+
75+
expect(result.current.filters).toEqual({ name: '', label: '' });
76+
expect(result.current.filteredData).toHaveLength(3);
77+
});
78+
79+
it('should initialize filters from URL search params on mount', () => {
80+
const { result } = renderHook(
81+
() => useConsoleDataViewFilters({ data: mockData, initialFilters }),
82+
{ wrapper: createWrapper(['/?name=api']) },
83+
);
84+
85+
expect(result.current.filters.name).toBe('api');
86+
expect(result.current.filteredData).toHaveLength(2);
87+
expect(result.current.filteredData.map((d) => d.metadata.name)).toEqual([
88+
'api-server',
89+
'api-gateway',
90+
]);
91+
});
92+
93+
it('should filter by name using fuzzy matching by default', () => {
94+
const { result } = renderHook(
95+
() => useConsoleDataViewFilters({ data: mockData, initialFilters }),
96+
{ wrapper: createWrapper(['/?name=front']) },
97+
);
98+
99+
expect(result.current.filteredData).toHaveLength(1);
100+
expect(result.current.filteredData[0].metadata.name).toBe('web-frontend');
101+
});
102+
103+
it('should filter by name using exact matching when exact search is enabled', () => {
104+
useExactSearch.mockReturnValue([true, true]);
105+
106+
const { result } = renderHook(
107+
() => useConsoleDataViewFilters({ data: mockData, initialFilters }),
108+
{ wrapper: createWrapper(['/?name=api']) },
109+
);
110+
111+
expect(result.current.filteredData).toHaveLength(2);
112+
expect(result.current.filteredData.map((d) => d.metadata.name)).toEqual([
113+
'api-server',
114+
'api-gateway',
115+
]);
116+
});
117+
118+
it('should filter by label', () => {
119+
const { result } = renderHook(
120+
() => useConsoleDataViewFilters({ data: mockData, initialFilters }),
121+
{ wrapper: createWrapper(['/?label=app%3Dapi']) },
122+
);
123+
124+
expect(result.current.filteredData).toHaveLength(2);
125+
expect(result.current.filteredData.map((d) => d.metadata.name)).toEqual([
126+
'api-server',
127+
'api-gateway',
128+
]);
129+
});
130+
131+
it('should filter by both name and label simultaneously', () => {
132+
const { result } = renderHook(
133+
() => useConsoleDataViewFilters({ data: mockData, initialFilters }),
134+
{ wrapper: createWrapper(['/?name=gateway&label=app%3Dapi']) },
135+
);
136+
137+
expect(result.current.filteredData).toHaveLength(1);
138+
expect(result.current.filteredData[0].metadata.name).toBe('api-gateway');
139+
});
140+
141+
it('should update filters and filteredData via onSetFilters', () => {
142+
const { result } = renderHook(
143+
() => useConsoleDataViewFilters({ data: mockData, initialFilters }),
144+
{ wrapper: createWrapper() },
145+
);
146+
147+
expect(result.current.filteredData).toHaveLength(3);
148+
149+
act(() => {
150+
result.current.onSetFilters({ name: 'web' } as ResourceFilters);
151+
});
152+
153+
expect(result.current.filters.name).toBe('web');
154+
expect(result.current.filteredData).toHaveLength(1);
155+
expect(result.current.filteredData[0].metadata.name).toBe('web-frontend');
156+
});
157+
158+
it('should clear all filters via clearAllFilters', () => {
159+
const { result } = renderHook(
160+
() => useConsoleDataViewFilters({ data: mockData, initialFilters }),
161+
{ wrapper: createWrapper(['/?name=api&label=app%3Dapi']) },
162+
);
163+
164+
expect(result.current.filteredData).toHaveLength(2);
165+
166+
act(() => {
167+
result.current.clearAllFilters();
168+
});
169+
170+
expect(result.current.filters.name).toBe('');
171+
expect(result.current.filters.label).toBe('');
172+
expect(result.current.filteredData).toHaveLength(3);
173+
});
174+
175+
it('should sync filters when URL changes externally after mount', () => {
176+
let hookResult: ReturnType<typeof useConsoleDataViewFilters>;
177+
let navigate: ReturnType<typeof useNavigate>;
178+
179+
const TestComponent = () => {
180+
hookResult = useConsoleDataViewFilters({ data: mockData, initialFilters });
181+
navigate = useNavigate();
182+
return null;
183+
};
184+
185+
render(
186+
<MemoryRouter initialEntries={['/']}>
187+
<TestComponent />
188+
</MemoryRouter>,
189+
);
190+
191+
expect(hookResult.filteredData).toHaveLength(3);
192+
193+
act(() => {
194+
navigate('/?name=api');
195+
});
196+
197+
expect(hookResult.filters.name).toBe('api');
198+
expect(hookResult.filteredData).toHaveLength(2);
199+
expect(hookResult.filteredData.map((d) => d.metadata.name)).toEqual([
200+
'api-server',
201+
'api-gateway',
202+
]);
203+
});
204+
205+
it('should support custom getObjectMetadata', () => {
206+
type CustomResource = { id: string; displayName: string };
207+
const customData: CustomResource[] = [
208+
{ id: '1', displayName: 'Alpha' },
209+
{ id: '2', displayName: 'Beta' },
210+
];
211+
const getObjectMetadata = (obj: CustomResource) => ({
212+
name: obj.displayName,
213+
labels: undefined,
214+
});
215+
216+
const { result } = renderHook(
217+
() =>
218+
useConsoleDataViewFilters({
219+
data: customData,
220+
initialFilters,
221+
getObjectMetadata,
222+
}),
223+
{ wrapper: createWrapper(['/?name=alpha']) },
224+
);
225+
226+
expect(result.current.filteredData).toHaveLength(1);
227+
expect((result.current.filteredData[0] as CustomResource).displayName).toBe('Alpha');
228+
});
229+
230+
it('should support matchesAdditionalFilters', () => {
231+
const matchesAdditionalFilters = (_obj: K8sResourceCommon, filters: ResourceFilters) =>
232+
!filters.name || _obj.metadata.name.startsWith('api');
233+
234+
const { result } = renderHook(
235+
() =>
236+
useConsoleDataViewFilters({
237+
data: mockData,
238+
initialFilters,
239+
matchesAdditionalFilters,
240+
}),
241+
{ wrapper: createWrapper(['/?name=a']) },
242+
);
243+
244+
expect(result.current.filteredData).toHaveLength(2);
245+
expect(result.current.filteredData.map((d) => d.metadata.name)).toEqual([
246+
'api-server',
247+
'api-gateway',
248+
]);
249+
});
250+
251+
it('should handle empty data array', () => {
252+
const { result } = renderHook(() => useConsoleDataViewFilters({ data: [], initialFilters }), {
253+
wrapper: createWrapper(['/?name=api']),
254+
});
255+
256+
expect(result.current.filteredData).toHaveLength(0);
257+
});
258+
});

‎frontend/packages/console-app/src/components/data-view/useConsoleDataViewFilters.ts‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useMemo } from 'react';
1+
import { useEffect, useMemo, useRef } from 'react';
22
import { useDataViewFilters } from '@patternfly/react-data-view';
33
import { useSearchParams } from 'react-router';
44
import { useExactSearch } from '@console/app/src/components/user-preferences/search/useExactSearch';
@@ -38,6 +38,39 @@ export const useConsoleDataViewFilters = <
3838
setSearchParams,
3939
});
4040

41+
// Sync URL search params → internal filter state.
42+
// useDataViewFilters only reads searchParams on mount (empty deps useEffect).
43+
// This effect ensures filters stay in sync when the URL changes externally
44+
// (e.g., the Search page updating query params without remounting).
45+
const filtersRef = useRef(filters);
46+
filtersRef.current = filters;
47+
useEffect(() => {
48+
const updates: Partial<TFilters> = {};
49+
let hasChanges = false;
50+
for (const key of Object.keys(filtersRef.current)) {
51+
const currentValue = filtersRef.current[key];
52+
if (Array.isArray(currentValue)) {
53+
const urlValues = searchParams.getAll(key);
54+
if (
55+
urlValues.length !== currentValue.length ||
56+
urlValues.some((v, i) => v !== currentValue[i])
57+
) {
58+
updates[key] = urlValues;
59+
hasChanges = true;
60+
}
61+
} else {
62+
const urlValue = searchParams.get(key) ?? '';
63+
if (urlValue !== currentValue) {
64+
updates[key] = urlValue;
65+
hasChanges = true;
66+
}
67+
}
68+
}
69+
if (hasChanges) {
70+
onSetFilters(updates as TFilters);
71+
}
72+
}, [searchParams, onSetFilters]);
73+
4174
const filteredData = useMemo(
4275
() =>
4376
data?.filter((resource) => {

0 commit comments

Comments
 (0)