Copilot commented on code in PR #3222: URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2809778526
########## src/utils/clientSideFilter.ts: ########## @@ -0,0 +1,147 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 { STATUS_ALL } from '@/config/constant'; +import type { APISIXType } from '@/types/schema/apisix'; + +import type { SearchFormValues } from '../components/form/SearchForm'; + +/** + * Client-side filtering utility for routes + * Used as a fallback when backend doesn't support certain filter parameters + */ + +export const filterRoutes = ( + routes: APISIXType['RespRouteItem'][], + filters: SearchFormValues +): APISIXType['RespRouteItem'][] => { + return routes.filter((route) => { + const routeData = route.value; + + // Filter by ID + if (filters.id) { + const idMatch = String(routeData.id) + .toLowerCase() + .includes(filters.id.toLowerCase()); + if (!idMatch) return false; + } + + // Filter by host + if (filters.host) { + const host = Array.isArray(routeData.host) + ? routeData.host.join(',') + : routeData.host || ''; + const hostMatch = host.toLowerCase().includes(filters.host.toLowerCase()); + if (!hostMatch) return false; + } + + // Filter by path/URI + if (filters.path) { + const uri = Array.isArray(routeData.uri) + ? routeData.uri.join(',') + : routeData.uri || ''; + const uris = Array.isArray(routeData.uris) + ? routeData.uris.join(',') + : ''; + const combinedPath = `${uri} ${uris}`.toLowerCase(); + const pathMatch = combinedPath.includes(filters.path.toLowerCase()); + if (!pathMatch) return false; + } + + // Filter by description + // Note: Routes without a description field are excluded when description filter is active + if (filters.description && routeData.desc) { + const descMatch = routeData.desc + .toLowerCase() + .includes(filters.description.toLowerCase()); + if (!descMatch) return false; + } + + // Filter by plugin: treat the filter text as a substring across all plugin names + // Note: Routes without any plugins are excluded when plugin filter is active + if (filters.plugin && routeData.plugins) { + const pluginNames = Object.keys(routeData.plugins).join(',').toLowerCase(); + const pluginMatch = pluginNames.includes(filters.plugin.toLowerCase()); + if (!pluginMatch) return false; + } Review Comment: The plugin filter logic will exclude routes without plugins when a plugin filter is provided. The current condition 'if (filters.plugin && routeData.plugins)' means that when filters.plugin is set but routeData.plugins is undefined, the filter check is skipped and the route passes through. This should explicitly check: 'if (filters.plugin) { if (!routeData.plugins) return false; ... }' to ensure routes without plugins are filtered out when a plugin search is active. ########## e2e/tests/routes.filter-version.spec.ts: ########## @@ -0,0 +1,186 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 { routesPom } from '@e2e/pom/routes'; +import { e2eReq } from '@e2e/utils/req'; +import { test } from '@e2e/utils/test'; +import { uiHasToastMsg } from '@e2e/utils/ui'; +import { uiFillUpstreamRequiredFields } from '@e2e/utils/ui/upstreams'; +import { expect } from '@playwright/test'; + +import { deleteAllRoutes } from '@/apis/routes'; +import type { APISIXType } from '@/types/schema/apisix'; + +/** + * Test for version filtering across multiple pages. + * This verifies that client-side filtering works across all routes, + * not just the current page. + */ +test.describe('Routes version filter', () => { + test.describe.configure({ mode: 'serial' }); + + const nodes: APISIXType['UpstreamNode'][] = [ + { host: 'test.com', port: 80, weight: 100 }, + { host: 'test2.com', port: 80, weight: 100 }, + ]; + + test.beforeAll(async () => { + // Clean up any existing routes + await deleteAllRoutes(e2eReq); + }); + + test.afterAll(async () => { + // Clean up test routes + await deleteAllRoutes(e2eReq); + }); + + test('should filter routes by version across all pages', async ({ page }) => { + test.slow(); // This test creates multiple routes via UI + + await test.step('create routes with different versions via UI', async () => { + await routesPom.toIndex(page); + await routesPom.isIndexPage(page); + + // Create 3 routes with version v1 + for (let i = 1; i <= 3; i++) { + await routesPom.getAddRouteBtn(page).click(); + await routesPom.isAddPage(page); + + await page.getByLabel('Name', { exact: true }).first().fill(`route_v1_${i}`); + await page.getByLabel('URI', { exact: true }).fill(`/v1/test${i}`); + + // Select HTTP method + await page.getByRole('textbox', { name: 'HTTP Methods' }).click(); + await page.getByRole('option', { name: 'GET' }).click(); + + // Add label for version (in the route section, not upstream) + const routeLabelsField = page.getByRole('textbox', { name: 'Labels' }).first(); + await routeLabelsField.click(); + await routeLabelsField.fill('version:v1'); + await routeLabelsField.press('Enter'); + + // Add upstream + const upstreamSection = page.getByRole('group', { + name: 'Upstream', + exact: true, + }); + await uiFillUpstreamRequiredFields(upstreamSection, { + nodes, + name: `upstream_v1_${i}`, + desc: 'test', + }); + + await routesPom.getAddBtn(page).click(); + await uiHasToastMsg(page, { + hasText: 'Add Route Successfully', + }); + + // Go back to list + await routesPom.getRouteNavBtn(page).click(); + await routesPom.isIndexPage(page); + } + + // Create 3 routes with version v2 + for (let i = 1; i <= 3; i++) { + await routesPom.getAddRouteBtn(page).click(); + await routesPom.isAddPage(page); + + await page.getByLabel('Name', { exact: true }).first().fill(`route_v2_${i}`); + await page.getByLabel('URI', { exact: true }).fill(`/v2/test${i}`); + + // Select HTTP method + await page.getByRole('textbox', { name: 'HTTP Methods' }).click(); + await page.getByRole('option', { name: 'GET' }).click(); + + // Add label for version (in the route section, not upstream) + const routeLabelsField = page.getByRole('textbox', { name: 'Labels' }).first(); + await routeLabelsField.click(); + await routeLabelsField.fill('version:v2'); + await routeLabelsField.press('Enter'); + + // Add upstream + const upstreamSection = page.getByRole('group', { + name: 'Upstream', + exact: true, + }); + await uiFillUpstreamRequiredFields(upstreamSection, { + nodes, + name: `upstream_v2_${i}`, + desc: 'test', + }); + + await routesPom.getAddBtn(page).click(); + await uiHasToastMsg(page, { + hasText: 'Add Route Successfully', + }); + + // Go back to list + await routesPom.getRouteNavBtn(page).click(); + await routesPom.isIndexPage(page); + } + }); + + await test.step('verify all routes are created with labels', async () => { + // Verify routes are visible in list + await expect(page.getByText('route_v1_1')).toBeVisible(); + await expect(page.getByText('route_v2_1')).toBeVisible(); + + // Verify version filter field exists + const versionFilter = page.locator('#version'); + await expect(versionFilter).toBeAttached(); + }); + + await test.step('filter by version v1 and verify only v1 routes are shown', async () => { + const versionFilter = page.locator('#version'); + + // Apply version:v1 filter + await versionFilter.click(); + await versionFilter.fill('version:v1'); + await versionFilter.press('Enter'); Review Comment: The E2E test incorrectly fills the version filter. It fills 'version:v1' and 'version:v2' into the version Select field, but the version filter expects just the version value (e.g., 'v1' or 'v2'), not the label key:value format. The version options are extracted from route.value.labels?.version and should be selected by their value alone. The test should use '.click()' followed by selecting the option by role, rather than '.fill()'. ########## e2e/tests/routes.filter-version.spec.ts: ########## @@ -0,0 +1,186 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 { routesPom } from '@e2e/pom/routes'; +import { e2eReq } from '@e2e/utils/req'; +import { test } from '@e2e/utils/test'; +import { uiHasToastMsg } from '@e2e/utils/ui'; +import { uiFillUpstreamRequiredFields } from '@e2e/utils/ui/upstreams'; +import { expect } from '@playwright/test'; + +import { deleteAllRoutes } from '@/apis/routes'; +import type { APISIXType } from '@/types/schema/apisix'; + +/** + * Test for version filtering across multiple pages. + * This verifies that client-side filtering works across all routes, + * not just the current page. + */ +test.describe('Routes version filter', () => { + test.describe.configure({ mode: 'serial' }); + + const nodes: APISIXType['UpstreamNode'][] = [ + { host: 'test.com', port: 80, weight: 100 }, + { host: 'test2.com', port: 80, weight: 100 }, + ]; + + test.beforeAll(async () => { + // Clean up any existing routes + await deleteAllRoutes(e2eReq); + }); + + test.afterAll(async () => { + // Clean up test routes + await deleteAllRoutes(e2eReq); + }); + + test('should filter routes by version across all pages', async ({ page }) => { + test.slow(); // This test creates multiple routes via UI + + await test.step('create routes with different versions via UI', async () => { + await routesPom.toIndex(page); + await routesPom.isIndexPage(page); + + // Create 3 routes with version v1 + for (let i = 1; i <= 3; i++) { + await routesPom.getAddRouteBtn(page).click(); + await routesPom.isAddPage(page); + + await page.getByLabel('Name', { exact: true }).first().fill(`route_v1_${i}`); + await page.getByLabel('URI', { exact: true }).fill(`/v1/test${i}`); + + // Select HTTP method + await page.getByRole('textbox', { name: 'HTTP Methods' }).click(); + await page.getByRole('option', { name: 'GET' }).click(); + + // Add label for version (in the route section, not upstream) + const routeLabelsField = page.getByRole('textbox', { name: 'Labels' }).first(); + await routeLabelsField.click(); + await routeLabelsField.fill('version:v1'); + await routeLabelsField.press('Enter'); + + // Add upstream + const upstreamSection = page.getByRole('group', { + name: 'Upstream', + exact: true, + }); + await uiFillUpstreamRequiredFields(upstreamSection, { + nodes, + name: `upstream_v1_${i}`, + desc: 'test', + }); + + await routesPom.getAddBtn(page).click(); + await uiHasToastMsg(page, { + hasText: 'Add Route Successfully', + }); + + // Go back to list + await routesPom.getRouteNavBtn(page).click(); + await routesPom.isIndexPage(page); + } + + // Create 3 routes with version v2 + for (let i = 1; i <= 3; i++) { + await routesPom.getAddRouteBtn(page).click(); + await routesPom.isAddPage(page); + + await page.getByLabel('Name', { exact: true }).first().fill(`route_v2_${i}`); + await page.getByLabel('URI', { exact: true }).fill(`/v2/test${i}`); + + // Select HTTP method + await page.getByRole('textbox', { name: 'HTTP Methods' }).click(); + await page.getByRole('option', { name: 'GET' }).click(); + + // Add label for version (in the route section, not upstream) + const routeLabelsField = page.getByRole('textbox', { name: 'Labels' }).first(); + await routeLabelsField.click(); + await routeLabelsField.fill('version:v2'); + await routeLabelsField.press('Enter'); + + // Add upstream + const upstreamSection = page.getByRole('group', { + name: 'Upstream', + exact: true, + }); + await uiFillUpstreamRequiredFields(upstreamSection, { + nodes, + name: `upstream_v2_${i}`, + desc: 'test', + }); + + await routesPom.getAddBtn(page).click(); + await uiHasToastMsg(page, { + hasText: 'Add Route Successfully', + }); + + // Go back to list + await routesPom.getRouteNavBtn(page).click(); + await routesPom.isIndexPage(page); + } + }); + + await test.step('verify all routes are created with labels', async () => { + // Verify routes are visible in list + await expect(page.getByText('route_v1_1')).toBeVisible(); + await expect(page.getByText('route_v2_1')).toBeVisible(); + + // Verify version filter field exists + const versionFilter = page.locator('#version'); + await expect(versionFilter).toBeAttached(); + }); + + await test.step('filter by version v1 and verify only v1 routes are shown', async () => { + const versionFilter = page.locator('#version'); + + // Apply version:v1 filter + await versionFilter.click(); + await versionFilter.fill('version:v1'); + await versionFilter.press('Enter'); + + // v1 routes should be present + await expect(page.getByText('route_v1_1')).toBeVisible(); + await expect(page.getByText('route_v1_2')).toBeVisible(); + await expect(page.getByText('route_v1_3')).toBeVisible(); + + // v2 routes should not be present in the filtered results + await expect(page.getByText(/route_v2_/)).toHaveCount(0); + }); + + await test.step('filter by version v2 and verify only v2 routes are shown', async () => { + // Clear previous filter by reloading the page to reset state + await page.reload(); + await routesPom.isIndexPage(page); + + const versionFilter = page.locator('#version'); + await expect(versionFilter).toBeAttached(); + + // Apply version:v2 filter + await versionFilter.click(); + await versionFilter.fill('version:v2'); + await versionFilter.press('Enter'); Review Comment: The E2E test incorrectly fills the version filter. It fills 'version:v2' into the version Select field, but the version filter expects just the version value (e.g., 'v2'), not the label key:value format. The version options are extracted from route.value.labels?.version and should be selected by their value alone. The test should select the option by role (e.g., page.getByRole('option', { name: 'v2' }).click()), rather than using '.fill()'. ########## src/utils/clientSideFilter.ts: ########## @@ -0,0 +1,147 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 { STATUS_ALL } from '@/config/constant'; +import type { APISIXType } from '@/types/schema/apisix'; + +import type { SearchFormValues } from '../components/form/SearchForm'; + +/** + * Client-side filtering utility for routes + * Used as a fallback when backend doesn't support certain filter parameters + */ + +export const filterRoutes = ( + routes: APISIXType['RespRouteItem'][], + filters: SearchFormValues +): APISIXType['RespRouteItem'][] => { + return routes.filter((route) => { + const routeData = route.value; + + // Filter by ID + if (filters.id) { + const idMatch = String(routeData.id) + .toLowerCase() + .includes(filters.id.toLowerCase()); + if (!idMatch) return false; + } + + // Filter by host + if (filters.host) { + const host = Array.isArray(routeData.host) + ? routeData.host.join(',') + : routeData.host || ''; + const hostMatch = host.toLowerCase().includes(filters.host.toLowerCase()); + if (!hostMatch) return false; + } + + // Filter by path/URI + if (filters.path) { + const uri = Array.isArray(routeData.uri) + ? routeData.uri.join(',') + : routeData.uri || ''; + const uris = Array.isArray(routeData.uris) + ? routeData.uris.join(',') + : ''; + const combinedPath = `${uri} ${uris}`.toLowerCase(); + const pathMatch = combinedPath.includes(filters.path.toLowerCase()); + if (!pathMatch) return false; + } + + // Filter by description + // Note: Routes without a description field are excluded when description filter is active + if (filters.description && routeData.desc) { + const descMatch = routeData.desc + .toLowerCase() + .includes(filters.description.toLowerCase()); + if (!descMatch) return false; + } Review Comment: The description filter logic will exclude routes without a description field when a description filter is provided. The current condition 'if (filters.description && routeData.desc)' means that when filters.description is set but routeData.desc is undefined, the filter check is skipped and the route passes through. This should explicitly check: 'if (filters.description) { if (!routeData.desc) return false; ... }' to ensure routes without descriptions are filtered out when a description search is active. ########## src/utils/clientSideFilter.ts: ########## @@ -0,0 +1,147 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You 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 { STATUS_ALL } from '@/config/constant'; +import type { APISIXType } from '@/types/schema/apisix'; + +import type { SearchFormValues } from '../components/form/SearchForm'; + +/** + * Client-side filtering utility for routes + * Used as a fallback when backend doesn't support certain filter parameters + */ + +export const filterRoutes = ( + routes: APISIXType['RespRouteItem'][], + filters: SearchFormValues +): APISIXType['RespRouteItem'][] => { + return routes.filter((route) => { + const routeData = route.value; + + // Filter by ID + if (filters.id) { + const idMatch = String(routeData.id) + .toLowerCase() + .includes(filters.id.toLowerCase()); + if (!idMatch) return false; + } + + // Filter by host + if (filters.host) { + const host = Array.isArray(routeData.host) + ? routeData.host.join(',') + : routeData.host || ''; + const hostMatch = host.toLowerCase().includes(filters.host.toLowerCase()); + if (!hostMatch) return false; + } + + // Filter by path/URI + if (filters.path) { + const uri = Array.isArray(routeData.uri) + ? routeData.uri.join(',') + : routeData.uri || ''; + const uris = Array.isArray(routeData.uris) + ? routeData.uris.join(',') + : ''; + const combinedPath = `${uri} ${uris}`.toLowerCase(); + const pathMatch = combinedPath.includes(filters.path.toLowerCase()); + if (!pathMatch) return false; + } + + // Filter by description + // Note: Routes without a description field are excluded when description filter is active + if (filters.description && routeData.desc) { + const descMatch = routeData.desc + .toLowerCase() + .includes(filters.description.toLowerCase()); + if (!descMatch) return false; + } + + // Filter by plugin: treat the filter text as a substring across all plugin names + // Note: Routes without any plugins are excluded when plugin filter is active + if (filters.plugin && routeData.plugins) { + const pluginNames = Object.keys(routeData.plugins).join(',').toLowerCase(); + const pluginMatch = pluginNames.includes(filters.plugin.toLowerCase()); + if (!pluginMatch) return false; + } + + // Filter by labels: match provided label key:value tokens against route label pairs + // Note: Routes without any labels are excluded when labels filter is active + if (filters.labels && filters.labels.length > 0) { + if (!routeData.labels) return false; + + const routeLabels = Object.keys(routeData.labels).map((key) => + `${key}:${routeData.labels![key]}`.toLowerCase() + ); + const hasMatchingLabel = filters.labels.some((filterLabel) => + routeLabels.some((routeLabel) => + routeLabel.includes(filterLabel.toLowerCase()) + ) + ); + if (!hasMatchingLabel) return false; + } + + // Filter by version + if (filters.version && routeData.labels?.version) { + const versionMatch = routeData.labels.version === filters.version; + if (!versionMatch) return false; + } + + // Filter by status + if (filters.status && filters.status !== STATUS_ALL) { + const isPublished = routeData.status === 1; + if (filters.status === 'Published' && !isPublished) return false; + if (filters.status === 'UnPublished' && isPublished) return false; + } + + return true; + }); +}; Review Comment: The filterRoutes function doesn't filter by 'name', but when client-side filtering is active (needsAllData is true), params may include a 'name' filter that won't be applied. This means routes will not be filtered by name on the client side even though the user provided a name filter. Either add name filtering to filterRoutes, or ensure the allData query includes the name parameter in its request. ########## src/routes/routes/index.tsx: ########## @@ -123,6 +256,18 @@ export const RouteList = (props: RouteListProps) => { ], }, }} + tableAlertRender={ + needsAllData + ? () => ( + <span style={{ color: '#faad14' }}> + {t('table.searchLimit', { + defaultValue: `Only the first ${PAGE_SIZE_MAX} routes are fetched from the database. Client-side filtering is applied to these records.`, + count: PAGE_SIZE_MAX, + })} + </span> + ) Review Comment: Inconsistent indentation in the tableAlertRender JSX. The opening parenthesis after the arrow function is not properly aligned, and the closing parenthesis is also misaligned. The JSX should be properly indented or the arrow function formatting should be adjusted for consistency. ```suggestion <span style={{ color: '#faad14' }}> {t('table.searchLimit', { defaultValue: `Only the first ${PAGE_SIZE_MAX} routes are fetched from the database. Client-side filtering is applied to these records.`, count: PAGE_SIZE_MAX, })} </span> ) ``` ########## src/routes/routes/index.tsx: ########## @@ -40,14 +50,128 @@ export type RouteListProps = { }) => React.ReactNode; }; +const SEARCH_PARAM_KEYS: (keyof SearchFormValues)[] = [ + 'name', + 'id', + 'host', + 'path', + 'description', + 'plugin', + 'labels', + 'version', + 'status', +]; + +const mapSearchParams = (values: Partial<SearchFormValues>) => + Object.fromEntries( + SEARCH_PARAM_KEYS + .map((key) => [key, values[key]] as const) + .filter(([, value]) => value !== undefined) + ) as Partial<SearchFormValues>; + export const RouteList = (props: RouteListProps) => { const { routeKey, ToDetailBtn, defaultParams } = props; - const { data, isLoading, refetch, pagination } = useRouteList( + const { data, isLoading, refetch, pagination, setParams } = useRouteList( routeKey, defaultParams ); + const { params, resetParams, setParams: setSearchParams } = useSearchParams(routeKey) as { + params: PageSearchType; + resetParams: () => void; + setParams: (params: Partial<PageSearchType>) => void; + }; Review Comment: Duplicate useSearchParams usage. The useRouteList hook already internally calls useSearchParams and returns setParams (line 74). Creating a second useSearchParams instance (line 78) for the same routeKey creates two separate instances managing the same URL parameters, which can lead to inconsistent state. Use the setParams returned from useRouteList, or if you need resetParams, extract it from a single useSearchParams call and don't use useRouteList's setParams. ########## src/routes/routes/index.tsx: ########## @@ -95,14 +219,23 @@ export const RouteList = (props: RouteListProps) => { return ( <AntdConfigProvider> + <div style={{ marginBottom: 24 }}> + <SearchForm + onSearch={handleSearch} + onReset={handleReset} + versionOptions={versionOptions} + labelOptions={versionOptions} Review Comment: The labelOptions prop is being passed the same value as versionOptions. This appears incorrect - labelOptions should likely contain all unique label key:value pairs from routes, not just version labels. The labels field in SearchForm uses mode="multiple" and expects label options in key:value format, while version is a single select expecting just version values. ########## src/routes/routes/index.tsx: ########## @@ -40,14 +50,128 @@ export type RouteListProps = { }) => React.ReactNode; }; +const SEARCH_PARAM_KEYS: (keyof SearchFormValues)[] = [ + 'name', + 'id', + 'host', + 'path', + 'description', + 'plugin', + 'labels', + 'version', + 'status', +]; + +const mapSearchParams = (values: Partial<SearchFormValues>) => + Object.fromEntries( + SEARCH_PARAM_KEYS + .map((key) => [key, values[key]] as const) + .filter(([, value]) => value !== undefined) + ) as Partial<SearchFormValues>; + export const RouteList = (props: RouteListProps) => { const { routeKey, ToDetailBtn, defaultParams } = props; - const { data, isLoading, refetch, pagination } = useRouteList( + const { data, isLoading, refetch, pagination, setParams } = useRouteList( routeKey, defaultParams ); + const { params, resetParams, setParams: setSearchParams } = useSearchParams(routeKey) as { + params: PageSearchType; + resetParams: () => void; + setParams: (params: Partial<PageSearchType>) => void; + }; const { t } = useTranslation(); + // Fetch all data when client-side filtering is active + const needsAllData = needsClientSideFiltering(params); + const { data: allData, isLoading: isLoadingAllData } = useQuery({ + queryKey: ['routes-all', defaultParams], + queryFn: () => + getRouteListReq(req, { + page: 1, + page_size: PAGE_SIZE_MAX, + ...defaultParams, Review Comment: The allData query doesn't include search filters in its query key, which means when 'name' is provided (backend-supported filter), the client-side filtering will still use the cached unfiltered data. The queryKey should include all relevant filter params to ensure proper cache invalidation: queryKey: ['routes-all', defaultParams, params.name], or the backend name filtering should be applied to the allData request as well. ```suggestion const nameFilter = params.name; const { data: allData, isLoading: isLoadingAllData } = useQuery({ queryKey: ['routes-all', defaultParams, nameFilter], queryFn: () => getRouteListReq(req, { page: 1, page_size: PAGE_SIZE_MAX, ...defaultParams, ...(nameFilter ? { name: nameFilter } : {}), ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
