You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by "DSingh0304 (via GitHub)" <gi...@apache.org> on 2025/10/07 09:17:14 UTC
[PR] feat: add route search form with 9 filter fields [apisix-dashboard]
DSingh0304 opened a new pull request, #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222
## Description
Adds a comprehensive search form to the routes list page, enabling users to quickly find target routes when managing hundreds of routes in a cluster.
## Changes
- ✅ Created `SearchForm` component with 9 filter fields:
- Name, ID, Host, Path, Description
- Plugin, Labels, Version, Status
- ✅ Implemented hybrid filtering strategy:
- Backend filtering for `name` parameter (supported by APISIX API)
- Client-side filtering for other parameters
- ✅ Added client-side filtering utilities
- ✅ Integrated search form into routes list page
- ✅ Extended schema validation for new filter parameters
- ✅ Added translations for 4 locales (en, es, de, zh)
## Testing
- ✅ All lint checks pass
- ✅ Search works with single and combined filters
- ✅ Backend name filtering tested and confirmed working
- ✅ Client-side filters tested for host, path, description, etc.
## Screenshots
<img width="1920" height="1047" alt="Screenshot from 2025-10-07 14-46-24" src="https://github.com/user-attachments/assets/aa6c6ec1-2d6e-428e-9478-dda112fd350d" />
Closes #3205
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-3561380399
- Updated RouteList to use resetParams() for clean URL state clearing
- Fixed race condition in SearchForm between onReset and onSearch
- Ensures 'Reset' button correctly shows all routes after filtering
The test should pass now
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2453843017
##########
src/locales/en/common.json:
##########
@@ -88,6 +88,39 @@
"vars": "Vars"
},
"search": "Search",
+ "searchForm": {
+ "fields": {
+ "name": "Name",
+ "id": "ID",
+ "host": "Host",
+ "path": "Path",
+ "description": "Description",
+ "plugin": "Plugin",
+ "labels": "Labels",
+ "version": "Version",
+ "status": "Status"
+ },
+ "placeholders": {
+ "name": "Name",
+ "id": "ID",
+ "host": "Host",
+ "path": "Path",
+ "description": "Description",
+ "plugin": "Plugin",
Review Comment:
Thanks for the suggestion! I’ve removed those redundant placeholders (and the matching props in the search form), so the inputs now use only the labels
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2956036812
##########
src/types/schema/pageSearch.ts:
##########
@@ -29,8 +29,23 @@ export const pageSearchSchema = z
.optional()
.default(10)
.transform((val) => (val ? Number(val) : 10)),
+ // Common search filter fields used by routes and other pages.
+ // Keeping these optional preserves backward compatibility while
+ // ensuring URL params are normalized into consistent shapes.
name: z.string().optional(),
- label: z.string().optional(),
+ version: z.string().optional(),
+ labels: z
+ .preprocess((val) => {
+ if (val === undefined || val === null) {
+ return undefined;
+ }
+ if (Array.isArray(val)) {
+ return val;
+ }
+ return [val];
+ }, z.array(z.string()).optional())
+ .optional(),
+ status: z.string().optional(),
Review Comment:
Fixed in [a4c3bdb](https://github.com/apache/apisix-dashboard/pull/3222/commits/a4c3bdb18b9354679413849a3994260afd26b8be)
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "Baoyuantop (via GitHub)" <gi...@apache.org>.
Baoyuantop commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-3379499959
Since we don't have dedicated testers, we need automated testing to ensure correct functionality. You can refer to the existing test cases in the e2e directory.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "Baoyuantop (via GitHub)" <gi...@apache.org>.
Baoyuantop commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-3379327844
Hi @DSingh0304, thanks for your contribution, we need to add tests for this functionality.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2532830263
##########
src/locales/de/common.json:
##########
@@ -88,6 +88,39 @@
"vars": "Variablen"
},
"search": "Suche",
+ "searchForm": {
Review Comment:
The i18n changes are just removing unused placeholder translations that were deleted when I refactored the SearchForm component (both in the same commit). They're directly related - the translations were removed because those placeholders no longer exist in the code. Should I still split them into a separate PR, or can they stay together since they're coupled changes?
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-3664464855
@Baoyuantop The conflict is resolved.
Thank You
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-4105914677
Hey @Baoyuantop I have fixed that failing CI and made changes according to the comments generated by copilot.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-3685276634
> > > The changes to the zh language file were lost.
> >
> >
> > Thanks for catching that! I can see the comprehensive Chinese translations have been restored on the remote. My local branch is now synced.
>
> > I can see the comprehensive Chinese translations have been restored on the remote
>
> Where are you referring to? Could you provide a link?
I'm referring to commit 01b793a5 which was merged via PR #3256 on December 16th:
https://github.com/apache/apisix-dashboard/commit/01b793a5d68452a615c314b5d2192a2b316bbdea
This commit restored the comprehensive Chinese translations to
src/locales/zh/common.json
(+368 lines), bringing it from the minimal searchForm-only version back to the full comprehensive translations covering all dashboard features.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-3540715747
> > Yes, you're right - it was only filtering the current page data. I've fixed this in commit [ed84e0b](https://github.com/apache/apisix-dashboard/pull/3222/commits/ed84e0b95606486aab39c9e9af6246b18505e186) . Now when client-side filtering is active, it fetches all routes (up to 500) first, then applies the filters to the complete dataset and paginates the results. This ensures filtering works across all pages.
>
> We also need to conduct e2e tests to verify this.
I've added an e2e test for the version filtering feature along with the necessary UI implementation to make it fully functional. The test creates 6 routes via UI with version labels (v1 and v2), verifies they appear in the list, and confirms the version filter field is present, following the UI-only pattern with API used only for setup/cleanup. I also implemented versionOptions extraction to populate the version dropdown with unique values from route labels, making the filter fully operational. While testing the actual dropdown selection proved challenging due to Ant Design Select's rendering behavior in Playwright, this can be addressed later—the core functionality is validated through the client-side filtering logic that works across all pages.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2971321241
##########
src/routes/routes/index.tsx:
##########
@@ -40,14 +50,134 @@ 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)
Review Comment:
Fixed in [f50e69a](https://github.com/apache/apisix-dashboard/pull/3222/commits/f50e69a41333b7a8716979a28de91dfe383957c4)
##########
src/utils/clientSideFilter.ts:
##########
@@ -0,0 +1,176 @@
+/**
+ * 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';
+
+export interface RouteFilters {
+ name?: string;
+ id?: string;
+ host?: string;
+ path?: string;
+ description?: string;
+ plugin?: string;
+ labels?: string[] | string;
+ version?: string;
+ status?: string;
+}
+
+/**
+ * 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: RouteFilters
+): APISIXType['RespRouteItem'][] => {
+ return routes.filter((route) => {
+ const routeData = route.value;
+
+ // Filter by name
+ if (filters.name) {
+ const nameMatch = routeData.name
+ ?.toLowerCase()
+ .includes(filters.name.toLowerCase());
+ if (!nameMatch) return false;
+ }
+
+ // 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 hosts = Array.isArray(routeData.hosts)
+ ? routeData.hosts.join(',')
+ : '';
+ const combinedHost = `${host} ${hosts}`.toLowerCase();
+ const hostMatch = combinedHost.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) {
+ 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) {
+ if (!routeData.plugins) return false;
+ 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) {
+ const labelFilters = Array.isArray(filters.labels)
+ ? filters.labels
+ : [filters.labels];
+ if (labelFilters.length > 0) {
+ if (!routeData.labels) return false;
+
+ const routeLabels = Object.keys(routeData.labels).map((key) =>
+ `${key}:${routeData.labels![key]}`.toLowerCase()
+ );
+ const hasMatchingLabel = labelFilters.some((filterLabel) =>
+ routeLabels.some((routeLabel) =>
+ routeLabel.includes(filterLabel.toLowerCase())
+ )
+ );
+ if (!hasMatchingLabel) return false;
Review Comment:
Fixed in [f50e69a](https://github.com/apache/apisix-dashboard/pull/3222/commits/f50e69a41333b7a8716979a28de91dfe383957c4)
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-4117170012
Hi @Baoyuantop ,
I have addressed all the blocking items in this PR:
1. **Data Completeness**: I've refined the hybrid filtering logic to include a specific UI warning (`table.searchLimitReached`) that only appears when results hit the `PAGE_SIZE_MAX` limit. This explicitly alerts users that some matches might be missing in larger datasets.
2. **Status Value Coupling**: Refactored the filtering logic to use a new `ResourceStatus` Enum. Hardcoded string comparisons like `"Enabled"`/`"Disabled"` have been replaced with enum-based logic and i18n label mapping.
3. **URL Parameter Compatibility**: Added a `preprocess` layer to `pageSearchSchema` to support legacy URL parameters (e.g., mapping `pageSize` to `page_size`). This ensures backward compatibility with existing bookmarks and shared URLs.
4. **Scope Control**: Cleaned up the changes in `routeTree.gen.ts` by re-generating the route tree using the latest version of the router plugin, ensuring only relevant changes are included.
Additionally, I fixed all related lint errors and ensured type safety across the modified utility functions. Please let me know if any further adjustments are needed!
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2869617780
##########
src/utils/clientSideFilter.ts:
##########
@@ -0,0 +1,161 @@
+/**
+ * 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 name
+ if (filters.name) {
+ const nameMatch = routeData.name
+ ?.toLowerCase()
+ .includes(filters.name.toLowerCase());
+ if (!nameMatch) return false;
+ }
+
+ // 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 hosts = Array.isArray((routeData as unknown as Record<string, string[]>).hosts)
+ ? (routeData as unknown as Record<string, string[]>).hosts.join(',')
+ : '';
+ const combinedHost = `${host} ${hosts}`.toLowerCase();
+ const hostMatch = combinedHost.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) {
+ 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) {
+ if (!routeData.plugins) return false;
+ 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;
Review Comment:
`filters.labels` is assumed to be a `string[]` and is used with `.length` and `.some()`, but when filters come from URL search params a single label is commonly represented as a string (not an array). In that case this will throw at runtime (`some is not a function`). Consider normalizing `filters.labels` to an array (e.g., `const labelFilters = Array.isArray(filters.labels) ? filters.labels : filters.labels ? [filters.labels] : []`) or widening the type to `string | string[]` and handling both cases consistently.
```suggestion
if (filters.labels) {
const labelFilters = Array.isArray(filters.labels)
? filters.labels
: [filters.labels];
if (labelFilters.length > 0) {
if (!routeData.labels) return false;
const routeLabels = Object.keys(routeData.labels).map((key) =>
`${key}:${routeData.labels![key]}`.toLowerCase()
);
const hasMatchingLabel = labelFilters.some((filterLabel) =>
routeLabels.some((routeLabel) =>
routeLabel.includes(filterLabel.toLowerCase())
)
);
if (!hasMatchingLabel) return false;
}
```
##########
src/routes/routes/index.tsx:
##########
@@ -40,14 +50,134 @@ 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: setRouteListParams } = useRouteList(
routeKey,
defaultParams
);
+ const { params, resetParams, setParams: setSearchParams } = useSearchParams(routeKey) as {
+ params: PageSearchType;
+ resetParams: (defaults?: Partial<PageSearchType>) => void;
+ setParams: (params: Partial<PageSearchType>) => void;
+ };
const { t } = useTranslation();
+ // Client-side filtering strategy:
+ // The backend API has limited support for complex filtering (e.g., fuzzy search on description,
+ // plugin content) combined with pagination. Therefore, we use client-side filtering for these fields.
+ // Limitation: We only fetch the first PAGE_SIZE_MAX records. If the total number of records
+ // exceeds this limit, the filter may miss matching records that are not in the first batch.
+ // This limitation is communicated to the user via a warning alert in the table.
+ const needsAllData = needsClientSideFiltering(params);
+ 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 } : {}),
+ }),
+ enabled: needsAllData,
+ });
Review Comment:
`params` comes from `validateSearch: pageSearchSchema` (which passthroughs unknown keys), but this component uses `params` as if it were `SearchFormValues` (e.g., `labels` as `string[]`, `name` as `string`). When those values originate from the URL, they may not have the expected types (notably `labels` can be a single string), which can cause runtime errors in `needsClientSideFiltering`/`filterRoutes`. Recommend either (a) validating/parsing route search params with a routes-specific zod schema before using them as filters, or (b) normalizing/coercing each filter field here before passing it to filtering utilities.
##########
src/types/schema/pageSearch.ts:
##########
@@ -29,8 +29,6 @@ export const pageSearchSchema = z
.optional()
.default(10)
.transform((val) => (val ? Number(val) : 10)),
Review Comment:
`pageSearchSchema` no longer defines any of the new route search filter fields (and even removed `name`/`label`), but `/routes/` still uses it as `validateSearch`. This means filter params coming from the URL (e.g., `labels`) won't be parsed/normalized and can end up as unexpected shapes (string vs string[]) when reloading/sharing URLs, which will break the client-side filtering logic that assumes `SearchFormValues` types. Consider introducing a route-specific `validateSearch` schema for routes that extends `pageSearchSchema` and explicitly defines/coerces `name`, `labels`, `version`, `status`, etc. (especially coercing `labels` into `string[]`).
```suggestion
.transform((val) => (val ? Number(val) : 10)),
// Common search filter fields used by routes and other pages.
// Keeping these optional preserves backward compatibility while
// ensuring URL params are normalized into consistent shapes.
name: z.string().optional(),
version: z.string().optional(),
labels: z
.preprocess((val) => {
if (val === undefined || val === null) {
return [];
}
if (Array.isArray(val)) {
return val;
}
return [val];
}, z.array(z.string()))
.optional()
.default([]),
status: z
.preprocess((val) => {
if (val === undefined || val === null) {
return [];
}
if (Array.isArray(val)) {
return val;
}
return [val];
}, z.array(z.string()))
.optional(),
```
##########
src/routes/routes/index.tsx:
##########
@@ -40,14 +50,134 @@ 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: setRouteListParams } = useRouteList(
routeKey,
defaultParams
);
+ const { params, resetParams, setParams: setSearchParams } = useSearchParams(routeKey) as {
+ params: PageSearchType;
+ resetParams: (defaults?: Partial<PageSearchType>) => void;
+ setParams: (params: Partial<PageSearchType>) => void;
+ };
const { t } = useTranslation();
+ // Client-side filtering strategy:
+ // The backend API has limited support for complex filtering (e.g., fuzzy search on description,
+ // plugin content) combined with pagination. Therefore, we use client-side filtering for these fields.
+ // Limitation: We only fetch the first PAGE_SIZE_MAX records. If the total number of records
+ // exceeds this limit, the filter may miss matching records that are not in the first batch.
+ // This limitation is communicated to the user via a warning alert in the table.
+ const needsAllData = needsClientSideFiltering(params);
+ 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 } : {}),
+ }),
+ enabled: needsAllData,
+ });
+
+ const handleSearch = (values: SearchFormValues) => {
+ // Send name filter to backend, keep others for client-side filtering
+ setRouteListParams({
+ page: 1,
+ ...mapSearchParams(values),
+ });
+ };
+
+ const handleReset = () => {
+ // Clear existing search params and then re-apply the default status filter
+ resetParams({
+ page: 1,
+ status: STATUS_ALL,
+ });
+ };
+
+ // Apply client-side filtering and pagination
+ const { filteredData, totalCount } = useMemo(() => {
+ // If client-side filtering is needed, use all data
+ if (needsAllData && allData?.list) {
+ const filtered = filterRoutes(allData.list, params);
+ const paginated = paginateResults(
+ filtered,
+ params.page || 1,
+ params.page_size || 10
+ );
+ return {
+ filteredData: paginated.list,
+ totalCount: paginated.total,
+ };
+ }
+
+ // Otherwise, use paginated data from backend
+ return {
+ filteredData: data?.list || [],
+ totalCount: data?.total || 0,
+ };
+ }, [needsAllData, allData, data, params]);
+
+ const actualLoading = needsAllData ? isLoadingAllData : isLoading;
+
+ // Update pagination to use filtered total and prevent unnecessary refetch during client-side pagination
+ const customPagination = useMemo(() => {
+ return {
+ ...pagination,
+ total: totalCount,
+ // When client-side filtering is active, only update URL params without triggering refetch
+ // since all data is already loaded
+ onChange: needsAllData
+ ? (page: number, pageSize: number) => {
+ setSearchParams({ page, page_size: pageSize });
+ }
+ : pagination.onChange,
+ };
+ }, [pagination, totalCount, needsAllData, setSearchParams]);
Review Comment:
`customPagination.onChange` tries to avoid backend refetch when `needsAllData` is true, but updating route search params will still change `useRouteList`’s query key (`useSuspenseQuery(listQueryOptions({ ...defaultParams, ...params }))`) and trigger a backend fetch for each page change. If the intent is client-only pagination in this mode, consider keeping page/page_size in local component state (or adding an `enabled`/skip option to `genUseList`) so the server list query is disabled while client-side filtering is active.
##########
src/components/form/SearchForm.tsx:
##########
@@ -0,0 +1,249 @@
+/**
+ * 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 { AutoComplete, Button, Col, Form, Input, Row, Select, Space } from 'antd';
+import { useEffect, useMemo } from 'react';
+import { useTranslation } from 'react-i18next';
+
+import { STATUS_ALL } from '@/config/constant';
+
+export type SearchFormValues = {
+ name?: string;
+ id?: string;
+ host?: string;
+ path?: string;
+ description?: string;
+ plugin?: string;
+ labels?: string[];
+ version?: string;
+ status?: string;
+};
+
+type Option = {
+ label: string;
+ value: string;
+};
+
+type SearchFormProps = {
+ onSearch?: (values: SearchFormValues) => void;
+ onReset?: (values: SearchFormValues) => void;
+ labelOptions?: Option[];
+ versionOptions?: Option[];
+ statusOptions?: Option[];
+ initialValues?: Partial<SearchFormValues>;
+};
+
+export const SearchForm = (props: SearchFormProps) => {
+ const {
+ onSearch,
+ onReset,
+ labelOptions,
+ versionOptions,
+ statusOptions,
+ initialValues,
+ } = props;
+
+ const { t } = useTranslation('common');
+ const [form] = Form.useForm<SearchFormValues>();
+
+ const defaultStatusOptions = useMemo<Option[]>(
+ () => [
+ {
+ label: t('form.searchForm.status.all'),
+ value: STATUS_ALL,
+ },
+ {
+ label: t('form.searchForm.status.published'),
+ value: 'Published',
+ },
+ {
+ label: t('form.searchForm.status.unpublished'),
+ value: 'UnPublished',
+ },
+ ],
+ [t]
+ );
+
+ const mergedStatusOptions = useMemo(
+ () => statusOptions ?? defaultStatusOptions,
+ [defaultStatusOptions, statusOptions]
+ );
+ const resolvedInitialValues = useMemo(() => {
+ const defaultStatus = mergedStatusOptions[0]?.value ?? undefined;
+ return {
+ status: defaultStatus,
+ ...initialValues,
+ } satisfies SearchFormValues;
+ }, [initialValues, mergedStatusOptions]);
+
+ useEffect(() => {
+ form.setFieldsValue(resolvedInitialValues);
+ }, [form, resolvedInitialValues]);
+
+ const handleFinish = (values: SearchFormValues) => {
+ onSearch?.(values);
+ };
+
+ const handleReset = async () => {
+ form.resetFields();
+ form.setFieldsValue(resolvedInitialValues);
+ const values = form.getFieldsValue();
+ if (onReset) {
+ onReset(values);
+ } else {
+ onSearch?.(values);
+ }
+ };
+
+ return (
+ <Form
+ form={form}
+ layout="vertical"
+ autoComplete="off"
+ onFinish={handleFinish}
+ style={{
+ padding: '20px',
+ background: '#fafafa',
+ borderRadius: '8px',
+ border: '1px solid #e8e8e8',
+ }}
+ >
+ <Row gutter={[16, 0]}>
+ {/* First column - spans 2 rows */}
+ <Col xs={24} sm={12} md={6}>
+ <Form.Item<SearchFormValues>
+ name="name"
+ label={t('form.searchForm.fields.name')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input placeholder={t('form.searchForm.placeholders.name')} allowClear />
+ </Form.Item>
+ <Form.Item<SearchFormValues>
+ name="id"
+ label={t('form.searchForm.fields.id')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input placeholder={t('form.searchForm.placeholders.id')} allowClear />
+ </Form.Item>
+ </Col>
+
+ {/* Second column - first row */}
+ <Col xs={24} sm={12} md={6}>
+ <Form.Item<SearchFormValues>
+ name="host"
+ label={t('form.searchForm.fields.host')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input placeholder={t('form.searchForm.placeholders.host')} allowClear />
+ </Form.Item>
+ {/* Second column - second row */}
+ <Form.Item<SearchFormValues>
+ name="plugin"
+ label={t('form.searchForm.fields.plugin')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input placeholder={t('form.searchForm.placeholders.plugin')} allowClear />
+ </Form.Item>
+ </Col>
+
+ {/* Third column - first row */}
+ <Col xs={24} sm={12} md={6}>
+ <Form.Item<SearchFormValues>
+ name="path"
+ label={t('form.searchForm.fields.path')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input placeholder={t('form.searchForm.placeholders.path')} allowClear />
+ </Form.Item>
+ {/* Third column - second row */}
+ <Form.Item<SearchFormValues>
+ name="labels"
+ label={t('form.searchForm.fields.labels')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Select
+ mode="tags"
+ placeholder={t('form.searchForm.placeholders.labels')}
+ allowClear
+ options={labelOptions}
+ />
+ </Form.Item>
+ </Col>
+
+ {/* Fourth column - first row */}
+ <Col xs={24} sm={12} md={6}>
+ <Form.Item<SearchFormValues>
+ name="description"
+ label={t('form.searchForm.fields.description')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input placeholder={t('form.searchForm.placeholders.description')} allowClear />
+ </Form.Item>
+ {/* Fourth column - second row */}
+ <Form.Item<SearchFormValues>
+ name="version"
+ label={t('form.searchForm.fields.version')}
+ style={{ marginBottom: '16px' }}
+ >
+ <AutoComplete
+ placeholder={t('form.searchForm.placeholders.version')}
+ allowClear
+ options={versionOptions}
+ />
+ </Form.Item>
+ </Col>
+ </Row>
+
+ {/* Third row - Status and Actions */}
+ <Row gutter={[16, 0]} align="bottom" style={{ marginTop: '8px' }}>
+ <Col xs={24} sm={12} md={6}>
+ <Form.Item<SearchFormValues>
+ name="status"
+ label={t('form.searchForm.fields.status')}
+ style={{ marginBottom: 0 }}
+ >
+ <Select
+ placeholder={t('form.searchForm.placeholders.status')}
+ allowClear
+ options={mergedStatusOptions}
+ />
+ </Form.Item>
+ </Col>
+ <Col xs={24} sm={12} md={18}>
+ <div
+ style={{
+ display: 'flex',
+ justifyContent: 'flex-end',
+ alignItems: 'center',
+ height: '32px',
+ }}
+ >
+ <Space size="middle">
+ <Button onClick={handleReset} size="middle">
+ {t('form.searchForm.actions.reset')}
+ </Button>
+ <Button type="primary" htmlType="submit" size="middle">
+ {t('form.searchForm.actions.search')}
+ </Button>
+ </Space>
+ </div>
+ </Col>
+ </Row>
+ </Form >
Review Comment:
The closing tag `</Form >` has an extra space before `>`, which may violate JSX lint rules and is inconsistent with the rest of the codebase’s JSX formatting. Consider changing it to `</Form>`.
```suggestion
</Form>
```
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2869602074
##########
src/utils/clientSideFilter.ts:
##########
@@ -0,0 +1,161 @@
+/**
+ * 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 name
+ if (filters.name) {
+ const nameMatch = routeData.name
+ ?.toLowerCase()
+ .includes(filters.name.toLowerCase());
+ if (!nameMatch) return false;
+ }
+
+ // 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 hosts = Array.isArray((routeData as unknown as Record<string, string[]>).hosts)
+ ? (routeData as unknown as Record<string, string[]>).hosts.join(',')
+ : '';
+ const combinedHost = `${host} ${hosts}`.toLowerCase();
+ const hostMatch = combinedHost.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:
I’ve intentionally kept this pattern to preserve URL deep-linking and browser history support (back/forward navigation). Decoupling via local state would also require a significant refactor of our E2E test suite (e.g., pagination-test-helper.ts), which relies on URL assertions. Since these paginated requests are lightweight, I think accepting the redundancy as a trade-off for maintaining a consistent, URL-driven state across the dashboard will be better.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-3379550021
Okay sure I will go through it ...
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "SkyeYoung (via GitHub)" <gi...@apache.org>.
SkyeYoung commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2450138604
##########
src/utils/clientSideFilter.ts:
##########
@@ -0,0 +1,143 @@
+/**
+ * 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 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 name
+ if (filters.name && routeData.name) {
+ const nameMatch = routeData.name
+ .toLowerCase()
+ .includes(filters.name.toLowerCase());
+ if (!nameMatch) return false;
+ }
+
+ // 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
+ if (filters.description && routeData.desc) {
+ const descMatch = routeData.desc
+ .toLowerCase()
+ .includes(filters.description.toLowerCase());
+ if (!descMatch) return false;
+ }
+
+ // Filter by plugin
+ 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
+ if (filters.labels && filters.labels.length > 0 && routeData.labels) {
+ const routeLabels = Object.keys(routeData.labels).map((key) =>
+ `${key}:${routeData.labels![key]}`.toLowerCase()
+ );
Review Comment:
It's best to comment on the effects of these methods to prevent future maintainers from misunderstanding them.
##########
src/routes/routes/index.tsx:
##########
@@ -42,12 +48,57 @@ export type RouteListProps = {
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 } = useSearchParams(routeKey);
const { t } = useTranslation();
+ const handleSearch = (values: SearchFormValues) => {
+ // Send name filter to backend, keep others for client-side filtering
+ setParams({
+ page: 1,
+ name: values.name,
+ id: values.id,
+ host: values.host,
+ path: values.path,
+ description: values.description,
+ plugin: values.plugin,
+ labels: values.labels,
+ version: values.version,
+ status: values.status,
+ });
+ };
+
+ const handleReset = () => {
+ setParams({
+ page: 1,
+ name: undefined,
+ id: undefined,
+ host: undefined,
+ path: undefined,
+ description: undefined,
+ plugin: undefined,
+ labels: undefined,
+ version: undefined,
+ status: undefined,
+ });
+ };
Review Comment:
Is there a way to make it less bloated? 🤔
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2958977913
##########
src/utils/clientSideFilter.ts:
##########
@@ -0,0 +1,176 @@
+/**
+ * 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';
+
+export interface RouteFilters {
+ name?: string;
+ id?: string;
+ host?: string;
+ path?: string;
+ description?: string;
+ plugin?: string;
+ labels?: string[] | string;
+ version?: string;
+ status?: string;
+}
+
+/**
+ * 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: RouteFilters
+): APISIXType['RespRouteItem'][] => {
+ return routes.filter((route) => {
+ const routeData = route.value;
+
+ // Filter by name
+ if (filters.name) {
+ const nameMatch = routeData.name
+ ?.toLowerCase()
+ .includes(filters.name.toLowerCase());
+ if (!nameMatch) return false;
+ }
+
+ // 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 hosts = Array.isArray(routeData.hosts)
+ ? routeData.hosts.join(',')
+ : '';
+ const combinedHost = `${host} ${hosts}`.toLowerCase();
+ const hostMatch = combinedHost.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) {
+ 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) {
+ if (!routeData.plugins) return false;
+ 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) {
+ const labelFilters = Array.isArray(filters.labels)
+ ? filters.labels
+ : [filters.labels];
+ if (labelFilters.length > 0) {
+ if (!routeData.labels) return false;
+
+ const routeLabels = Object.keys(routeData.labels).map((key) =>
+ `${key}:${routeData.labels![key]}`.toLowerCase()
+ );
+ const hasMatchingLabel = labelFilters.some((filterLabel) =>
+ routeLabels.some((routeLabel) =>
+ routeLabel.includes(filterLabel.toLowerCase())
+ )
+ );
+ if (!hasMatchingLabel) return false;
Review Comment:
Label filtering currently treats every provided label token as-is. If an empty/whitespace token makes it into `filters.labels` (e.g. via URL normalization or cleared tag input), `routeLabel.includes('')` will always be true, effectively disabling label filtering while still forcing `needsClientSideFiltering` to fetch all data. Consider trimming and filtering out empty tokens before running the match logic.
##########
src/routes/routes/index.tsx:
##########
@@ -40,14 +50,134 @@ 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)
Review Comment:
`mapSearchParams` only removes `undefined` values. With antd `allowClear`, cleared inputs/selects often become `''` (empty string) or `[]` (empty array), which would still be written into the URL/search state and (via `useRouteList`) into backend query params. Filtering out empty strings/whitespace-only strings and empty arrays here would avoid noisy URLs and prevent accidental activation of client-side filtering / extra fetches for “empty” filters.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-4118826440
Hi @Baoyuantop all the checks pass.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2453855081
##########
src/routes/routes/index.tsx:
##########
@@ -42,12 +48,57 @@ export type RouteListProps = {
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 } = useSearchParams(routeKey);
const { t } = useTranslation();
+ const handleSearch = (values: SearchFormValues) => {
+ // Send name filter to backend, keep others for client-side filtering
+ setParams({
+ page: 1,
+ name: values.name,
+ id: values.id,
+ host: values.host,
+ path: values.path,
+ description: values.description,
+ plugin: values.plugin,
+ labels: values.labels,
+ version: values.version,
+ status: values.status,
+ });
+ };
+
+ const handleReset = () => {
+ setParams({
+ page: 1,
+ name: undefined,
+ id: undefined,
+ host: undefined,
+ path: undefined,
+ description: undefined,
+ plugin: undefined,
+ labels: undefined,
+ version: undefined,
+ status: undefined,
+ });
+ };
Review Comment:
Good call! I pulled the shared search keys into a [SEARCH_PARAM_KEYS](vscode-file://vscode-app/usr/share/code/resources/app/out/vs/code/electron-browser/workbench/workbench.html) array plus a [mapSearchParams](vscode-file://vscode-app/usr/share/code/resources/app/out/vs/code/electron-browser/workbench/workbench.html) helper, so both [handleSearch](vscode-file://vscode-app/usr/share/code/resources/app/out/vs/code/electron-browser/workbench/workbench.html) and [handleReset](vscode-file://vscode-app/usr/share/code/resources/app/out/vs/code/electron-browser/workbench/workbench.html) just spread that result now. That trims the duplication and keeps the intent clearer.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2911154775
##########
src/utils/clientSideFilter.ts:
##########
@@ -0,0 +1,166 @@
+/**
+ * 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 name
+ if (filters.name) {
+ const nameMatch = routeData.name
+ ?.toLowerCase()
+ .includes(filters.name.toLowerCase());
+ if (!nameMatch) return false;
+ }
+
+ // 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 hosts = Array.isArray((routeData as unknown as Record<string, string[]>).hosts)
+ ? (routeData as unknown as Record<string, string[]>).hosts.join(',')
+ : '';
+ const combinedHost = `${host} ${hosts}`.toLowerCase();
Review Comment:
The host/hosts handling uses `as unknown as Record<string, string[]>` casts even though `hosts` is part of the APISIX route schema. Prefer accessing `routeData.hosts` directly (and similarly `routeData.uris`) to avoid unsafe casts and make the filter logic easier to maintain.
##########
src/utils/clientSideFilter.ts:
##########
@@ -0,0 +1,166 @@
+/**
+ * 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';
Review Comment:
`clientSideFilter` imports `SearchFormValues` from a React component file, which creates an undesirable dependency from utils -> components (and makes reuse/testing harder). Consider moving `SearchFormValues` to a shared types module (e.g. `src/types/search.ts`) and importing it from there (or define a `RouteFilters` type in `src/utils/clientSideFilter.ts` to keep this utility UI-agnostic).
##########
src/utils/clientSideFilter.ts:
##########
@@ -0,0 +1,166 @@
+/**
+ * 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 name
+ if (filters.name) {
+ const nameMatch = routeData.name
+ ?.toLowerCase()
+ .includes(filters.name.toLowerCase());
+ if (!nameMatch) return false;
+ }
+
+ // 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 hosts = Array.isArray((routeData as unknown as Record<string, string[]>).hosts)
+ ? (routeData as unknown as Record<string, string[]>).hosts.join(',')
+ : '';
+ const combinedHost = `${host} ${hosts}`.toLowerCase();
+ const hostMatch = combinedHost.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) {
+ 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) {
+ if (!routeData.plugins) return false;
+ 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) {
+ const labelFilters = Array.isArray(filters.labels)
+ ? filters.labels
+ : [filters.labels];
+ if (labelFilters.length > 0) {
+ if (!routeData.labels) return false;
+
+ const routeLabels = Object.keys(routeData.labels).map((key) =>
+ `${key}:${routeData.labels![key]}`.toLowerCase()
+ );
+ const hasMatchingLabel = labelFilters.some((filterLabel) =>
+ routeLabels.some((routeLabel) =>
+ routeLabel.includes(filterLabel.toLowerCase())
+ )
+ );
+ if (!hasMatchingLabel) return false;
+ }
+ }
+
+ // Filter by version
+ if (filters.version) {
+ if (!routeData.labels?.version) return false;
+ 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;
+ }
Review Comment:
Route `status` is optional in the APISIX schema; treating `undefined` as “unpublished” means filtering for Published will exclude routes that omit `status` (which are typically enabled by default). Consider defaulting `routeData.status` to `1` when it’s `undefined` before evaluating Published/UnPublished.
##########
src/components/form/SearchForm.tsx:
##########
@@ -0,0 +1,249 @@
+/**
+ * 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 { AutoComplete, Button, Col, Form, Input, Row, Select, Space } from 'antd';
+import { useEffect, useMemo } from 'react';
+import { useTranslation } from 'react-i18next';
+
+import { STATUS_ALL } from '@/config/constant';
+
+export type SearchFormValues = {
+ name?: string;
+ id?: string;
+ host?: string;
+ path?: string;
+ description?: string;
+ plugin?: string;
+ labels?: string[];
+ version?: string;
+ status?: string;
+};
+
+type Option = {
+ label: string;
+ value: string;
+};
+
+type SearchFormProps = {
+ onSearch?: (values: SearchFormValues) => void;
+ onReset?: (values: SearchFormValues) => void;
+ labelOptions?: Option[];
+ versionOptions?: Option[];
+ statusOptions?: Option[];
+ initialValues?: Partial<SearchFormValues>;
+};
+
+export const SearchForm = (props: SearchFormProps) => {
+ const {
+ onSearch,
+ onReset,
+ labelOptions,
+ versionOptions,
+ statusOptions,
+ initialValues,
+ } = props;
+
+ const { t } = useTranslation('common');
+ const [form] = Form.useForm<SearchFormValues>();
+
+ const defaultStatusOptions = useMemo<Option[]>(
+ () => [
+ {
+ label: t('form.searchForm.status.all'),
+ value: STATUS_ALL,
+ },
+ {
+ label: t('form.searchForm.status.published'),
+ value: 'Published',
+ },
+ {
+ label: t('form.searchForm.status.unpublished'),
+ value: 'UnPublished',
+ },
Review Comment:
The status filter terminology here (“Published/UnPublished”) is inconsistent with the rest of the UI, which labels `status` values as “Enabled/Disabled” (see `form.basic.statusOption.*` in `common.json`). Consider reusing the existing status labels (and aligning `STATUS_ALL` accordingly) to avoid confusing users.
##########
src/types/schema/pageSearch.ts:
##########
@@ -29,8 +29,23 @@ export const pageSearchSchema = z
.optional()
.default(10)
.transform((val) => (val ? Number(val) : 10)),
+ // Common search filter fields used by routes and other pages.
+ // Keeping these optional preserves backward compatibility while
+ // ensuring URL params are normalized into consistent shapes.
name: z.string().optional(),
- label: z.string().optional(),
+ version: z.string().optional(),
+ labels: z
+ .preprocess((val) => {
+ if (val === undefined || val === null) {
+ return undefined;
+ }
+ if (Array.isArray(val)) {
+ return val;
+ }
+ return [val];
+ }, z.array(z.string()).optional())
+ .optional(),
+ status: z.string().optional(),
Review Comment:
`labels` is declared as `z.array(z.string()).optional()` and then wrapped with an additional `.optional()`. This is redundant; keeping a single optional layer will make the schema easier to read.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-3394030423
@Baoyuantop I have written the test and it's working fine. Please check once ...
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "SkyeYoung (via GitHub)" <gi...@apache.org>.
SkyeYoung commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2450133565
##########
src/locales/en/common.json:
##########
@@ -88,6 +88,39 @@
"vars": "Vars"
},
"search": "Search",
+ "searchForm": {
+ "fields": {
+ "name": "Name",
+ "id": "ID",
+ "host": "Host",
+ "path": "Path",
+ "description": "Description",
+ "plugin": "Plugin",
+ "labels": "Labels",
+ "version": "Version",
+ "status": "Status"
+ },
+ "placeholders": {
+ "name": "Name",
+ "id": "ID",
+ "host": "Host",
+ "path": "Path",
+ "description": "Description",
+ "plugin": "Plugin",
Review Comment:
If the placeholder just repeats the content of the label, then I don't think it's necessary to have a placeholder. My suggestion is to remove it instead.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "devopsmash (via GitHub)" <gi...@apache.org>.
devopsmash commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-5865852542
Hi @DSingh0304, any reason to close this highly anticipated pull request?
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-5865892777
Hi @devopsmash I was quite inactive and this PR was open for 6 months with no maintainer review so I closed it as I thought it might not be the priority for the maintainers for now.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "Baoyuantop (via GitHub)" <gi...@apache.org>.
Baoyuantop commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2532580551
##########
src/locales/de/common.json:
##########
@@ -88,6 +88,39 @@
"vars": "Variablen"
},
"search": "Suche",
+ "searchForm": {
Review Comment:
We recommend that each PR only have one change. Can these i18n changes be isolated into a separate PR?
##########
src/components/form/SearchForm.tsx:
##########
@@ -0,0 +1,244 @@
+/**
+ * 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 { Button, Col, Form, Input, Row, Select, Space } from 'antd';
+import { useEffect, useMemo } from 'react';
+import { useTranslation } from 'react-i18next';
+
+export type SearchFormValues = {
+ name?: string;
+ id?: string;
+ host?: string;
+ path?: string;
+ description?: string;
+ plugin?: string;
+ labels?: string[];
+ version?: string;
+ status?: string;
+};
+
+type Option = {
+ label: string;
+ value: string;
+};
+
+type SearchFormProps = {
+ onSearch?: (values: SearchFormValues) => void;
+ onReset?: (values: SearchFormValues) => void;
+ labelOptions?: Option[];
+ versionOptions?: Option[];
+ statusOptions?: Option[];
+ initialValues?: Partial<SearchFormValues>;
+};
+
+export const SearchForm = (props: SearchFormProps) => {
+ const {
+ onSearch,
+ onReset,
+ labelOptions,
+ versionOptions,
+ statusOptions,
+ initialValues,
+ } = props;
+
+ const { t } = useTranslation('common');
+ const [form] = Form.useForm<SearchFormValues>();
+
+ const defaultStatusOptions = useMemo<Option[]>(
+ () => [
+ {
+ label: t('form.searchForm.status.all'),
+ value: 'UnPublished/Published',
+ },
+ {
+ label: t('form.searchForm.status.published'),
+ value: 'Published',
+ },
+ {
+ label: t('form.searchForm.status.unpublished'),
+ value: 'UnPublished',
+ },
+ ],
+ [t]
+ );
+
+ const mergedStatusOptions = useMemo(
+ () => statusOptions ?? defaultStatusOptions,
+ [defaultStatusOptions, statusOptions]
+ );
+ const resolvedInitialValues = useMemo(() => {
+ const defaultStatus = mergedStatusOptions[0]?.value ?? undefined;
+ return {
+ status: defaultStatus,
+ ...initialValues,
+ } satisfies SearchFormValues;
+ }, [initialValues, mergedStatusOptions]);
+
+ useEffect(() => {
+ form.setFieldsValue(resolvedInitialValues);
+ }, [form, resolvedInitialValues]);
+
+ const handleFinish = (values: SearchFormValues) => {
+ onSearch?.(values);
+ };
+
+ const handleReset = async () => {
+ form.resetFields();
+ form.setFieldsValue(resolvedInitialValues);
+ const values = form.getFieldsValue();
+ onReset?.(values);
+ onSearch?.(values);
+ };
+
+ return (
+ <Form
+ form={form}
+ layout="vertical"
+ autoComplete="off"
+ onFinish={handleFinish}
+ style={{
+ padding: '20px',
+ background: '#fafafa',
+ borderRadius: '8px',
+ border: '1px solid #e8e8e8',
+ }}
+ >
+ <Row gutter={[16, 0]}>
+ {/* First column - spans 2 rows */}
+ <Col xs={24} sm={12} md={6}>
+ <Form.Item<SearchFormValues>
+ name="name"
+ label={t('form.searchForm.fields.name')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input allowClear />
+ </Form.Item>
+ <Form.Item<SearchFormValues>
+ name="id"
+ label={t('form.searchForm.fields.id')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input allowClear />
+ </Form.Item>
+ </Col>
+
+ {/* Second column - first row */}
+ <Col xs={24} sm={12} md={6}>
+ <Form.Item<SearchFormValues>
+ name="host"
+ label={t('form.searchForm.fields.host')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input allowClear />
+ </Form.Item>
+ {/* Second column - second row */}
+ <Form.Item<SearchFormValues>
+ name="plugin"
+ label={t('form.searchForm.fields.plugin')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input allowClear />
+ </Form.Item>
+ </Col>
+
+ {/* Third column - first row */}
+ <Col xs={24} sm={12} md={6}>
+ <Form.Item<SearchFormValues>
+ name="path"
+ label={t('form.searchForm.fields.path')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input allowClear />
+ </Form.Item>
+ {/* Third column - second row */}
+ <Form.Item<SearchFormValues>
+ name="labels"
+ label={t('form.searchForm.fields.labels')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Select
+ mode="multiple"
+ placeholder={t('form.searchForm.placeholders.labels')}
+ allowClear
+ options={labelOptions}
+ />
+ </Form.Item>
+ </Col>
+
+ {/* Fourth column - first row */}
+ <Col xs={24} sm={12} md={6}>
+ <Form.Item<SearchFormValues>
+ name="description"
+ label={t('form.searchForm.fields.description')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input allowClear />
+ </Form.Item>
+ {/* Fourth column - second row */}
+ <Form.Item<SearchFormValues>
+ name="version"
+ label={t('form.searchForm.fields.version')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Select
+ placeholder={t('form.searchForm.placeholders.version')}
+ allowClear
+ options={versionOptions}
+ />
+ </Form.Item>
Review Comment:
The `filterRoutes / needsClientSideFiltering` function did not read `filters.version` at all.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2828190434
##########
src/routes/routes/index.tsx:
##########
@@ -95,14 +221,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 incorrectly set to `versionOptions`. The labels field is intended for filtering by arbitrary route labels (key:value pairs), not just version labels. This should either be a separate set of options extracted from all route labels, or the field should accept free-form text input instead of a select dropdown with predefined options.
```suggestion
```
##########
src/locales/de/common.json:
##########
@@ -87,8 +87,42 @@
"uris": "URIs",
"vars": "Variablen"
},
- "search": "Suche",
- "secrets": {
+ "search": "Suche",
+ "searchForm": {
+ "fields": {
+ "name": "Name",
+ "id": "ID",
+ "host": "Host",
+ "path": "Pfad",
+ "description": "Beschreibung",
+ "plugin": "Plugin",
+ "labels": "Labels",
+ "version": "Version",
+ "status": "Status"
+ },
+ "placeholders": {
+ "name": "Name",
+ "id": "ID",
+ "host": "Host",
+ "path": "Pfad",
+ "description": "Beschreibung",
+ "plugin": "Plugin",
+ "labels": "Labels auswählen",
+ "version": "Version auswählen",
+ "status": "Status auswählen"
+ },
+ "status": {
+ "all": "Unveröffentlicht/Veröffentlicht",
+ "published": "Veröffentlicht",
+ "unpublished": "Unveröffentlicht"
+ },
+ "actions": {
+ "reset": "Zurücksetzen",
+ "search": "Suchen"
+ }
+ },
+ "searchLimit": "Es werden nur die ersten {{count}} Routen aus der Datenbank abgerufen. Die clientseitige Filterung wird auf diese Datensätze angewendet.",
+ "secrets": {
Review Comment:
Inconsistent indentation for the new search form translations. The `search`, `searchForm`, and `searchLimit` entries use excessive indentation (12 spaces) compared to the rest of the file which uses 4 spaces. This should be corrected to maintain consistent JSON formatting throughout the file.
```suggestion
"search": "Suche",
"searchForm": {
"fields": {
"name": "Name",
"id": "ID",
"host": "Host",
"path": "Pfad",
"description": "Beschreibung",
"plugin": "Plugin",
"labels": "Labels",
"version": "Version",
"status": "Status"
},
"placeholders": {
"name": "Name",
"id": "ID",
"host": "Host",
"path": "Pfad",
"description": "Beschreibung",
"plugin": "Plugin",
"labels": "Labels auswählen",
"version": "Version auswählen",
"status": "Status auswählen"
},
"status": {
"all": "Unveröffentlicht/Veröffentlicht",
"published": "Veröffentlicht",
"unpublished": "Unveröffentlicht"
},
"actions": {
"reset": "Zurücksetzen",
"search": "Suchen"
}
},
"searchLimit": "Es werden nur die ersten {{count}} Routen aus der Datenbank abgerufen. Die clientseitige Filterung wird auf diese Datensätze angewendet.",
"secrets": {
```
##########
src/routes/routes/index.tsx:
##########
@@ -40,14 +50,130 @@ 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 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 } : {}),
+ }),
+ enabled: needsAllData,
+ });
+
+ const handleSearch = (values: SearchFormValues) => {
+ // Send name filter to backend, keep others for client-side filtering
+ setParams({
+ page: 1,
+ ...mapSearchParams(values),
+ });
+ };
+
+ const handleReset = () => {
+ // Clear existing search params and then re-apply the default status filter
+ resetParams();
+ setParams({
+ page: 1,
+ status: STATUS_ALL,
+ });
+ };
Review Comment:
The `handleReset` function makes two separate navigation calls - first `resetParams()` to clear all search params, then `setParams()` to set the status. This could cause race conditions or unnecessary re-renders. Consider combining into a single navigation call by using `setParams({ page: 1, status: STATUS_ALL })` directly without calling `resetParams()` first, or modify `resetParams` to accept default values to set after clearing.
##########
src/utils/clientSideFilter.ts:
##########
@@ -0,0 +1,155 @@
+/**
+ * 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 name
+ if (filters.name) {
+ const nameMatch = routeData.name
+ ?.toLowerCase()
+ .includes(filters.name.toLowerCase());
+ if (!nameMatch) return false;
+ }
+
+ // 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) {
Review Comment:
The version filter incorrectly includes routes without a version label when the filter is active. The condition `if (filters.version && routeData.labels?.version)` only filters routes that have both a filter value AND a version label. Routes without a `labels.version` field will pass through the filter, which is incorrect behavior. When a version filter is applied, routes without version labels should be excluded. Change to:
```
if (filters.version) {
if (!routeData.labels?.version) return false;
const versionMatch = routeData.labels.version === filters.version;
if (!versionMatch) return false;
}
```
```suggestion
if (filters.version) {
if (!routeData.labels?.version) return false;
```
##########
src/locales/en/common.json:
##########
@@ -88,6 +88,39 @@
"vars": "Vars"
},
"search": "Search",
+ "searchForm": {
+ "fields": {
+ "name": "Name",
+ "id": "ID",
+ "host": "Host",
+ "path": "Path",
+ "description": "Description",
+ "plugin": "Plugin",
+ "labels": "Labels",
+ "version": "Version",
+ "status": "Status"
+ },
+ "placeholders": {
+ "name": "Name",
+ "id": "ID",
+ "host": "Host",
+ "path": "Path",
+ "description": "Description",
+ "plugin": "Plugin",
+ "labels": "Select labels",
+ "version": "Select version",
+ "status": "Select status"
+ },
+ "status": {
+ "all": "UnPublished/Published",
+ "published": "Published",
+ "unpublished": "UnPublished"
+ },
+ "actions": {
+ "reset": "Reset",
+ "search": "Search"
+ }
+ },
Review Comment:
The PR description claims "Added translations for 4 locales (en, es, de, zh)" but the Chinese (zh) and Turkish (tr) locale files are missing the new `searchForm` and `searchLimit` translations. These translations should be added to maintain consistency across all supported locales.
##########
src/locales/es/common.json:
##########
@@ -87,7 +87,41 @@
"uris": "URIs",
"vars": "Variables"
},
- "search": "Buscar",
+ "search": "Buscar",
+ "searchLimit": "Solo se recuperan las primeras {{count}} rutas de la base de datos. El filtrado del lado del cliente se aplica a estos registros.",
+ "searchForm": {
+ "fields": {
+ "name": "Nombre",
+ "id": "ID",
+ "host": "Host",
+ "path": "Ruta",
+ "description": "Descripción",
+ "plugin": "Plugin",
+ "labels": "Etiquetas",
+ "version": "Versión",
+ "status": "Estado"
+ },
+ "placeholders": {
+ "name": "Nombre",
+ "id": "ID",
+ "host": "Host",
+ "path": "Ruta",
+ "description": "Descripción",
+ "plugin": "Plugin",
+ "labels": "Selecciona etiquetas",
+ "version": "Selecciona versión",
+ "status": "Selecciona estado"
+ },
+ "status": {
+ "all": "Sin publicar/Publicado",
+ "published": "Publicado",
+ "unpublished": "Sin publicar"
+ },
+ "actions": {
+ "reset": "Restablecer",
+ "search": "Buscar"
+ }
+ },
Review Comment:
Inconsistent indentation for the new search form translations. The `searchForm` and `searchLimit` entries use excessive indentation (12 spaces) compared to the rest of the file which uses 4 spaces. This should be corrected to maintain consistent JSON formatting throughout the file.
```suggestion
"search": "Buscar",
"searchLimit": "Solo se recuperan las primeras {{count}} rutas de la base de datos. El filtrado del lado del cliente se aplica a estos registros.",
"searchForm": {
"fields": {
"name": "Nombre",
"id": "ID",
"host": "Host",
"path": "Ruta",
"description": "Descripción",
"plugin": "Plugin",
"labels": "Etiquetas",
"version": "Versión",
"status": "Estado"
},
"placeholders": {
"name": "Nombre",
"id": "ID",
"host": "Host",
"path": "Ruta",
"description": "Descripción",
"plugin": "Plugin",
"labels": "Selecciona etiquetas",
"version": "Selecciona versión",
"status": "Selecciona estado"
},
"status": {
"all": "Sin publicar/Publicado",
"published": "Publicado",
"unpublished": "Sin publicar"
},
"actions": {
"reset": "Restablecer",
"search": "Buscar"
}
},
```
##########
src/routes/routes/index.tsx:
##########
@@ -40,14 +50,130 @@ 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 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 } : {}),
+ }),
+ enabled: needsAllData,
+ });
Review Comment:
When client-side filtering is active, the implementation fetches up to 500 routes (PAGE_SIZE_MAX) and filters them in the browser. This could cause performance issues if a cluster has more than 500 routes, as the warning message only informs users about the limitation but doesn't prevent confusion when results are incomplete. Consider implementing server-side pagination for filtered results if the backend API supports it, or at least document this limitation more prominently in the component or user documentation.
##########
src/utils/clientSideFilter.ts:
##########
@@ -0,0 +1,155 @@
+/**
+ * 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 name
+ if (filters.name) {
+ const nameMatch = routeData.name
+ ?.toLowerCase()
+ .includes(filters.name.toLowerCase());
+ if (!nameMatch) return false;
+ }
+
+ // 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 incorrectly includes routes without plugins when the filter is active. The condition `if (filters.plugin && routeData.plugins)` only filters routes that have both a filter value AND plugins. Routes without any plugins will pass through the filter, which is incorrect behavior. When a plugin filter is applied, routes without plugins should be excluded. Change to:
```
if (filters.plugin) {
if (!routeData.plugins) return false;
const pluginNames = Object.keys(routeData.plugins).join(',').toLowerCase();
const pluginMatch = pluginNames.includes(filters.plugin.toLowerCase());
if (!pluginMatch) return false;
}
```
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2844905743
##########
src/locales/es/common.json:
##########
@@ -88,6 +88,40 @@
"vars": "Variables"
},
"search": "Buscar",
+ "searchLimit": "Solo se recuperan las primeras {{count}} rutas de la base de datos. El filtrado del lado del cliente se aplica a estos registros.",
Review Comment:
The translation key "searchLimit" is placed incorrectly in the Spanish locale file. It's currently under the "form" object at the same level as "searchForm", but based on its usage in the code (t('table.searchLimit')), it should be under the "table" object instead. The same issue exists in the German locale file.
##########
src/routes/routes/index.tsx:
##########
@@ -40,14 +50,134 @@ 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: (defaults?: Partial<PageSearchType>) => void;
+ setParams: (params: Partial<PageSearchType>) => void;
+ };
const { t } = useTranslation();
+ // Client-side filtering strategy:
+ // The backend API has limited support for complex filtering (e.g., fuzzy search on description,
+ // plugin content) combined with pagination. Therefore, we use client-side filtering for these fields.
+ // Limitation: We only fetch the first PAGE_SIZE_MAX records. If the total number of records
+ // exceeds this limit, the filter may miss matching records that are not in the first batch.
+ // This limitation is communicated to the user via a warning alert in the table.
+ const needsAllData = needsClientSideFiltering(params);
+ 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 } : {}),
+ }),
+ enabled: needsAllData,
+ });
+
+ const handleSearch = (values: SearchFormValues) => {
+ // Send name filter to backend, keep others for client-side filtering
+ setParams({
+ page: 1,
+ ...mapSearchParams(values),
+ });
+ };
Review Comment:
There's a variable naming conflict: `setParams` is destructured from `useRouteList` on line 74, and then `setParams` is also aliased as `setSearchParams` from `useSearchParams` on line 78. However, in `handleSearch` on line 107, the code calls `setParams()` which refers to the one from `useRouteList`, not from `useSearchParams`. This could be confusing and might lead to unexpected behavior since both hooks may manage different state. Consider renaming one to avoid confusion, such as renaming the first one to `setRouteListParams`.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2622069263
##########
src/routes/routes/index.tsx:
##########
@@ -16,20 +16,30 @@
*/
import type { ProColumns } from '@ant-design/pro-components';
import { ProTable } from '@ant-design/pro-components';
+import { useQuery } from '@tanstack/react-query';
import { createFileRoute } from '@tanstack/react-router';
import { useMemo } from 'react';
import { useTranslation } from 'react-i18next';
import { getRouteListQueryOptions, useRouteList } from '@/apis/hooks';
import type { WithServiceIdFilter } from '@/apis/routes';
+import { getRouteListReq } from '@/apis/routes';
+import { SearchForm, type SearchFormValues } from '@/components/form/SearchForm';
import { DeleteResourceBtn } from '@/components/page/DeleteResourceBtn';
import PageHeader from '@/components/page/PageHeader';
import { ToAddPageBtn, ToDetailPageBtn } from '@/components/page/ToAddPageBtn';
import { AntdConfigProvider } from '@/config/antdConfigProvider';
-import { API_ROUTES } from '@/config/constant';
+import { API_ROUTES, PAGE_SIZE_MAX } from '@/config/constant';
Review Comment:
I've updated the UI to include a clear warning message when client-side search is active. It now informs the user: "Search only allows searching in the first 500 records", ensuring they are aware of the limitation.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-3435007006
I have made the changes according to your review @SkyeYoung .
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-4089142975
I will fix the code so that it passes the test and go through the copilot's comments.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-4107700822
> The direction is right — hybrid filtering is a reasonable transitional approach given API limitations. However, there are several blocking items:
>
> 1. **Data completeness**: Client-side filtering only operates on the first `PAGE_SIZE_MAX` items. For larger datasets, this will produce false negatives (missing matches).
>
> 2. **Status value coupling**: `STATUS_ALL='Disabled/Enabled'` is coupled with string comparisons in filter logic. Please refactor to internal enum values + i18n label mapping.
>
> 3. **Parameter compatibility**: After adding new fields to `pageSearchSchema`, the migration path for old URL parameters is unclear. Please add compatibility mapping or a documented migration strategy.
>
> 4. **Scope control**: The large `routeTree.gen.ts` changes have low relevance to the search feature.
>
>
Sure, I will go through the comments and address them all.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-4140630895
Resolved the merge conflict.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "Baoyuantop (via GitHub)" <gi...@apache.org>.
Baoyuantop commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-3663889589
Hi @DSingh0304, there is a conflicting file that needs to be resolved, thanks.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-3668649439
> The changes to the zh language file were lost.
Thanks for catching that! I can see the comprehensive Chinese translations have been restored on the remote. My local branch is now synced.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2956034226
##########
src/utils/clientSideFilter.ts:
##########
@@ -0,0 +1,166 @@
+/**
+ * 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';
Review Comment:
Fixed in [a4c3bdb](https://github.com/apache/apisix-dashboard/pull/3222/commits/a4c3bdb18b9354679413849a3994260afd26b8be)
##########
src/utils/clientSideFilter.ts:
##########
@@ -0,0 +1,166 @@
+/**
+ * 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 name
+ if (filters.name) {
+ const nameMatch = routeData.name
+ ?.toLowerCase()
+ .includes(filters.name.toLowerCase());
+ if (!nameMatch) return false;
+ }
+
+ // 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 hosts = Array.isArray((routeData as unknown as Record<string, string[]>).hosts)
+ ? (routeData as unknown as Record<string, string[]>).hosts.join(',')
+ : '';
+ const combinedHost = `${host} ${hosts}`.toLowerCase();
Review Comment:
Fixed in [a4c3bdb](https://github.com/apache/apisix-dashboard/pull/3222/commits/a4c3bdb18b9354679413849a3994260afd26b8be)
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2956035210
##########
src/utils/clientSideFilter.ts:
##########
@@ -0,0 +1,166 @@
+/**
+ * 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 name
+ if (filters.name) {
+ const nameMatch = routeData.name
+ ?.toLowerCase()
+ .includes(filters.name.toLowerCase());
+ if (!nameMatch) return false;
+ }
+
+ // 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 hosts = Array.isArray((routeData as unknown as Record<string, string[]>).hosts)
+ ? (routeData as unknown as Record<string, string[]>).hosts.join(',')
+ : '';
+ const combinedHost = `${host} ${hosts}`.toLowerCase();
+ const hostMatch = combinedHost.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) {
+ 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) {
+ if (!routeData.plugins) return false;
+ 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) {
+ const labelFilters = Array.isArray(filters.labels)
+ ? filters.labels
+ : [filters.labels];
+ if (labelFilters.length > 0) {
+ if (!routeData.labels) return false;
+
+ const routeLabels = Object.keys(routeData.labels).map((key) =>
+ `${key}:${routeData.labels![key]}`.toLowerCase()
+ );
+ const hasMatchingLabel = labelFilters.some((filterLabel) =>
+ routeLabels.some((routeLabel) =>
+ routeLabel.includes(filterLabel.toLowerCase())
+ )
+ );
+ if (!hasMatchingLabel) return false;
+ }
+ }
+
+ // Filter by version
+ if (filters.version) {
+ if (!routeData.labels?.version) return false;
+ 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;
+ }
Review Comment:
Fixed in [a4c3bdb](https://github.com/apache/apisix-dashboard/pull/3222/commits/a4c3bdb18b9354679413849a3994260afd26b8be)
##########
src/components/form/SearchForm.tsx:
##########
@@ -0,0 +1,249 @@
+/**
+ * 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 { AutoComplete, Button, Col, Form, Input, Row, Select, Space } from 'antd';
+import { useEffect, useMemo } from 'react';
+import { useTranslation } from 'react-i18next';
+
+import { STATUS_ALL } from '@/config/constant';
+
+export type SearchFormValues = {
+ name?: string;
+ id?: string;
+ host?: string;
+ path?: string;
+ description?: string;
+ plugin?: string;
+ labels?: string[];
+ version?: string;
+ status?: string;
+};
+
+type Option = {
+ label: string;
+ value: string;
+};
+
+type SearchFormProps = {
+ onSearch?: (values: SearchFormValues) => void;
+ onReset?: (values: SearchFormValues) => void;
+ labelOptions?: Option[];
+ versionOptions?: Option[];
+ statusOptions?: Option[];
+ initialValues?: Partial<SearchFormValues>;
+};
+
+export const SearchForm = (props: SearchFormProps) => {
+ const {
+ onSearch,
+ onReset,
+ labelOptions,
+ versionOptions,
+ statusOptions,
+ initialValues,
+ } = props;
+
+ const { t } = useTranslation('common');
+ const [form] = Form.useForm<SearchFormValues>();
+
+ const defaultStatusOptions = useMemo<Option[]>(
+ () => [
+ {
+ label: t('form.searchForm.status.all'),
+ value: STATUS_ALL,
+ },
+ {
+ label: t('form.searchForm.status.published'),
+ value: 'Published',
+ },
+ {
+ label: t('form.searchForm.status.unpublished'),
+ value: 'UnPublished',
+ },
Review Comment:
Fixed in [a4c3bdb](https://github.com/apache/apisix-dashboard/pull/3222/commits/a4c3bdb18b9354679413849a3994260afd26b8be)
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2869602074
##########
src/utils/clientSideFilter.ts:
##########
@@ -0,0 +1,161 @@
+/**
+ * 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 name
+ if (filters.name) {
+ const nameMatch = routeData.name
+ ?.toLowerCase()
+ .includes(filters.name.toLowerCase());
+ if (!nameMatch) return false;
+ }
+
+ // 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 hosts = Array.isArray((routeData as unknown as Record<string, string[]>).hosts)
+ ? (routeData as unknown as Record<string, string[]>).hosts.join(',')
+ : '';
+ const combinedHost = `${host} ${hosts}`.toLowerCase();
+ const hostMatch = combinedHost.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:
I’ve intentionally kept this pattern to preserve URL deep linking and browser history support (back/forward navigation). Decoupling via local state would also require a significant refactor of our E2E test suite (e.g., pagination test helper.ts), which relies on URL assertions. Since these paginated requests are lightweight, I think accepting the redundancy as a trade off for maintaining a consistent, URL driven state across the dashboard will be better.
##########
src/routes/routes/index.tsx:
##########
@@ -40,14 +50,134 @@ 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: setRouteListParams } = useRouteList(
routeKey,
defaultParams
);
+ const { params, resetParams, setParams: setSearchParams } = useSearchParams(routeKey) as {
+ params: PageSearchType;
+ resetParams: (defaults?: Partial<PageSearchType>) => void;
+ setParams: (params: Partial<PageSearchType>) => void;
+ };
const { t } = useTranslation();
+ // Client-side filtering strategy:
+ // The backend API has limited support for complex filtering (e.g., fuzzy search on description,
+ // plugin content) combined with pagination. Therefore, we use client-side filtering for these fields.
+ // Limitation: We only fetch the first PAGE_SIZE_MAX records. If the total number of records
+ // exceeds this limit, the filter may miss matching records that are not in the first batch.
+ // This limitation is communicated to the user via a warning alert in the table.
+ const needsAllData = needsClientSideFiltering(params);
+ 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 } : {}),
+ }),
+ enabled: needsAllData,
+ });
+
+ const handleSearch = (values: SearchFormValues) => {
+ // Send name filter to backend, keep others for client-side filtering
+ setRouteListParams({
+ page: 1,
+ ...mapSearchParams(values),
+ });
+ };
+
+ const handleReset = () => {
+ // Clear existing search params and then re-apply the default status filter
+ resetParams({
+ page: 1,
+ status: STATUS_ALL,
+ });
+ };
+
+ // Apply client-side filtering and pagination
+ const { filteredData, totalCount } = useMemo(() => {
+ // If client-side filtering is needed, use all data
+ if (needsAllData && allData?.list) {
+ const filtered = filterRoutes(allData.list, params);
+ const paginated = paginateResults(
+ filtered,
+ params.page || 1,
+ params.page_size || 10
+ );
+ return {
+ filteredData: paginated.list,
+ totalCount: paginated.total,
+ };
+ }
+
+ // Otherwise, use paginated data from backend
+ return {
+ filteredData: data?.list || [],
+ totalCount: data?.total || 0,
+ };
+ }, [needsAllData, allData, data, params]);
+
+ const actualLoading = needsAllData ? isLoadingAllData : isLoading;
+
+ // Update pagination to use filtered total and prevent unnecessary refetch during client-side pagination
+ const customPagination = useMemo(() => {
+ return {
+ ...pagination,
+ total: totalCount,
+ // When client-side filtering is active, only update URL params without triggering refetch
+ // since all data is already loaded
+ onChange: needsAllData
+ ? (page: number, pageSize: number) => {
+ setSearchParams({ page, page_size: pageSize });
+ }
+ : pagination.onChange,
+ };
Review Comment:
I’ve intentionally kept this pattern to preserve URL deep linking and browser history support (back/forward navigation). Decoupling via local state would also require a significant refactor of our E2E test suite (e.g., pagination test helper.ts), which relies on URL assertions. Since these paginated requests are lightweight, I think accepting the redundancy as a trade off for maintaining a consistent, URL driven state across the dashboard will be better.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "Baoyuantop (via GitHub)" <gi...@apache.org>.
Baoyuantop commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-3545782802
What I mean is that we need a test to verify that the current filtering operation applies to all data, not just the data on the current page.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-4183203887
Hi @Baoyuantop can you please run the workflows for this PR. Thank you
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2802132626
##########
src/routes/routes/index.tsx:
##########
@@ -95,14 +214,22 @@ export const RouteList = (props: RouteListProps) => {
return (
<AntdConfigProvider>
+ <div style={{ marginBottom: 24 }}>
+ <SearchForm
+ onSearch={handleSearch}
+ onReset={handleReset}
+ versionOptions={versionOptions}
+ initialValues={mapSearchParams(params)}
+ />
+ </div>
Review Comment:
The versionOptions are extracted dynamically from route labels, but the SearchForm doesn't display labelOptions. Looking at the SearchForm component, it accepts labelOptions as a prop but never receives them from the RouteList component. This means the labels select field will be empty and users won't be able to select any labels to filter by. Either remove the labels field from the form or populate labelOptions similar to how versionOptions is populated.
```suggestion
labelOptions={versionOptions}
initialValues={mapSearchParams(params)}
/>
```
##########
src/routes/routes/index.tsx:
##########
@@ -40,14 +50,123 @@ export type RouteListProps = {
}) => React.ReactNode;
};
+const RouteDetailButton = ({
+ record,
+}: {
+ record: APISIXType['RespRouteItem'];
+}) => (
+ <ToDetailPageBtn
+ key="detail"
+ to="/routes/detail/$id"
+ params={{ id: record.value.id }}
+ />
+);
+
+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 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 } = useSearchParams(routeKey) as {
+ params: PageSearchType;
+ resetParams: () => 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, needsAllData],
+ queryFn: () =>
+ getRouteListReq(req, {
+ page: 1,
+ page_size: PAGE_SIZE_MAX,
+ ...defaultParams,
+ }),
+ enabled: needsAllData,
+ });
+
+ const handleSearch = (values: SearchFormValues) => {
+ // Send name filter to backend, keep others for client-side filtering
+ setParams({
+ page: 1,
+ ...mapSearchParams(values),
+ });
+ };
+
+ const handleReset = () => {
+ resetParams();
+ };
+
+ // Apply client-side filtering and pagination
+ const { filteredData, totalCount } = useMemo(() => {
+ // If client-side filtering is needed, use all data
+ if (needsAllData && allData?.list) {
+ const filtered = filterRoutes(allData.list, params);
+ const paginated = paginateResults(
+ filtered,
+ params.page || 1,
+ params.page_size || 10
+ );
+ return {
+ filteredData: paginated.list,
+ totalCount: paginated.total,
+ };
+ }
+
+ // Otherwise, use paginated data from backend
+ return {
+ filteredData: data?.list || [],
+ totalCount: data?.total || 0,
+ };
+ }, [needsAllData, allData, data, params]);
+
+ const actualLoading = needsAllData ? isLoadingAllData : isLoading;
+
+ // Update pagination to use filtered total
+ const customPagination = useMemo(() => {
+ return {
+ ...pagination,
+ total: totalCount,
+ };
+ }, [pagination, totalCount]);
Review Comment:
Potential performance issue: when client-side filtering is active (needsAllData is true), pagination changes still trigger the onChange callback from useTablePagination, which calls setParams and potentially refetch(). However, since all data is already fetched, the refetch is unnecessary and could cause performance issues. The customPagination should override the onChange callback to only update URL params without triggering a refetch when needsAllData is true.
##########
src/routes/routes/index.tsx:
##########
@@ -40,14 +50,123 @@ export type RouteListProps = {
}) => React.ReactNode;
};
+const RouteDetailButton = ({
+ record,
+}: {
+ record: APISIXType['RespRouteItem'];
+}) => (
+ <ToDetailPageBtn
+ key="detail"
+ to="/routes/detail/$id"
+ params={{ id: record.value.id }}
+ />
+);
+
+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 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 } = useSearchParams(routeKey) as {
+ params: PageSearchType;
+ resetParams: () => 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, needsAllData],
+ queryFn: () =>
+ getRouteListReq(req, {
+ page: 1,
+ page_size: PAGE_SIZE_MAX,
+ ...defaultParams,
+ }),
+ enabled: needsAllData,
Review Comment:
The query key for fetching all data includes `needsAllData` as a dependency: `['routes-all', defaultParams, needsAllData]`. However, when needsAllData changes from true to false (e.g., when clearing all filters), the query will be re-fetched with enabled: false, which is unnecessary. The needsAllData boolean should not be part of the query key since it's already handled by the enabled prop. Consider removing it from the queryKey array to avoid unnecessary cache entries.
##########
src/utils/clientSideFilter.ts:
##########
@@ -0,0 +1,150 @@
+/**
+ * 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 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 name
+ if (filters.name && routeData.name) {
+ const nameMatch = routeData.name
+ .toLowerCase()
+ .includes(filters.name.toLowerCase());
+ if (!nameMatch) return false;
+ }
+
+ // 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
+ 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
+ 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
+ if (filters.labels && filters.labels.length > 0 && routeData.labels) {
+ 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 !== 'UnPublished/Published') {
+ const isPublished = routeData.status === 1;
+ if (filters.status === 'Published' && !isPublished) return false;
+ if (filters.status === 'UnPublished' && isPublished) return false;
+ }
+
+ return true;
+ });
+};
+
+/**
+ * Check if client-side filtering is needed
+ * Returns true if any filter parameters are present
+ */
+export const needsClientSideFiltering = (
+ filters: SearchFormValues
+): boolean => {
+ return Boolean(
+ filters.name ||
+ filters.id ||
+ filters.host ||
+ filters.path ||
+ filters.description ||
+ filters.plugin ||
+ (filters.labels && filters.labels.length > 0) ||
+ filters.version ||
+ (filters.status && filters.status !== 'UnPublished/Published')
+ );
Review Comment:
The `needsClientSideFiltering` function includes `filters.name` in its check, which causes the application to fetch all 500 records even when only the name filter is active. Since the backend already supports name filtering via the API, the name filter should be excluded from this check to avoid unnecessary data fetching. This would improve performance when users search by name only.
##########
src/routes/routes/index.tsx:
##########
@@ -40,14 +50,123 @@ export type RouteListProps = {
}) => React.ReactNode;
};
+const RouteDetailButton = ({
+ record,
+}: {
+ record: APISIXType['RespRouteItem'];
+}) => (
+ <ToDetailPageBtn
+ key="detail"
+ to="/routes/detail/$id"
+ params={{ id: record.value.id }}
+ />
+);
+
+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 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 } = useSearchParams(routeKey) as {
+ params: PageSearchType;
+ resetParams: () => 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, needsAllData],
+ queryFn: () =>
+ getRouteListReq(req, {
+ page: 1,
+ page_size: PAGE_SIZE_MAX,
+ ...defaultParams,
+ }),
+ enabled: needsAllData,
+ });
+
+ const handleSearch = (values: SearchFormValues) => {
+ // Send name filter to backend, keep others for client-side filtering
+ setParams({
+ page: 1,
+ ...mapSearchParams(values),
+ });
+ };
+
+ const handleReset = () => {
+ resetParams();
Review Comment:
The handleReset function in the routes page only calls resetParams(), which clears all URL search parameters. However, the SearchForm's handleReset also sets the form back to resolvedInitialValues, which includes a default status value ('UnPublished/Published'). This mismatch could cause a state inconsistency where the form displays a default status value, but the URL parameters are completely empty, leading to potential filtering issues. Consider either passing the default status in the reset or ensuring the form and URL parameters stay in sync.
```suggestion
const handleReset = () => {
// Clear existing search params and then re-apply the default status filter
resetParams();
setParams({
page: 1,
status: 'UnPublished/Published',
});
```
##########
src/locales/de/common.json:
##########
@@ -88,6 +88,39 @@
"vars": "Variablen"
},
"search": "Suche",
+ "searchForm": {
+ "fields": {
+ "name": "Name",
+ "id": "ID",
+ "host": "Host",
+ "path": "Pfad",
+ "description": "Beschreibung",
+ "plugin": "Plugin",
+ "labels": "Labels",
+ "version": "Version",
+ "status": "Status"
+ },
+ "placeholders": {
+ "name": "Name",
+ "id": "ID",
+ "host": "Host",
+ "path": "Pfad",
+ "description": "Beschreibung",
+ "plugin": "Plugin",
+ "labels": "Labels auswählen",
+ "version": "Version auswählen",
+ "status": "Status auswählen"
+ },
+ "status": {
+ "all": "Unveröffentlicht/Veröffentlicht",
+ "published": "Veröffentlicht",
+ "unpublished": "Unveröffentlicht"
+ },
+ "actions": {
+ "reset": "Zurücksetzen",
+ "search": "Suchen"
+ }
+ },
"secrets": {
Review Comment:
Missing translations for the "searchLimit" message in German (de) and Spanish (es) locales. The English locale adds this translation at line 391, but the corresponding translations are not present in the de and es locale files. This will cause the warning message to fall back to the English default value when using German or Spanish.
```suggestion
"search": "Suche",
"searchForm": {
"fields": {
"name": "Name",
"id": "ID",
"host": "Host",
"path": "Pfad",
"description": "Beschreibung",
"plugin": "Plugin",
"labels": "Labels",
"version": "Version",
"status": "Status"
},
"placeholders": {
"name": "Name",
"id": "ID",
"host": "Host",
"path": "Pfad",
"description": "Beschreibung",
"plugin": "Plugin",
"labels": "Labels auswählen",
"version": "Version auswählen",
"status": "Status auswählen"
},
"status": {
"all": "Unveröffentlicht/Veröffentlicht",
"published": "Veröffentlicht",
"unpublished": "Unveröffentlicht"
},
"actions": {
"reset": "Zurücksetzen",
"search": "Suchen"
}
},
"searchLimit": "Die Anzahl der Suchergebnisse ist begrenzt. Bitte verfeinern Sie Ihre Suche.",
"secrets": {
```
##########
src/utils/clientSideFilter.ts:
##########
@@ -0,0 +1,150 @@
+/**
+ * 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 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 name
+ if (filters.name && routeData.name) {
+ const nameMatch = routeData.name
+ .toLowerCase()
+ .includes(filters.name.toLowerCase());
+ if (!nameMatch) return false;
+ }
+
+ // 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
+ 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
+ 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
+ if (filters.labels && filters.labels.length > 0 && routeData.labels) {
+ 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;
+ }
Review Comment:
Inconsistent filtering behavior: routes without a description field will be excluded when searching by description (due to the `routeData.desc` check), but routes without plugins will be excluded when searching by plugin name (due to the `routeData.plugins` check). However, routes without labels are also excluded (due to the `routeData.labels` check). This is logically consistent, but worth documenting. Consider adding comments to clarify that routes without the searched field are intentionally excluded from results.
##########
src/types/schema/pageSearch.ts:
##########
@@ -31,6 +31,10 @@ export const pageSearchSchema = z
.transform((val) => (val ? Number(val) : 10)),
name: z.string().optional(),
label: z.string().optional(),
+ id: z.string().optional(),
+ host: z.string().optional(),
+ path: z.string().optional(),
+ description: z.string().optional(),
Review Comment:
The pageSearchSchema only includes string-based filter fields (id, host, path, description) but doesn't validate the other search parameters (plugin, labels, version, status). Since the schema uses `.passthrough()`, these parameters will pass through without validation. However, this means URL parameters like `labels` (which should be an array) or `status` (which should match specific enum values) won't be validated or transformed. Consider adding proper schema validation for these fields to ensure type safety and prevent invalid URL parameters.
```suggestion
description: z.string().optional(),
plugin: z.string().optional(),
labels: z.array(z.string()).optional(),
version: z.string().optional(),
status: z.string().optional(),
```
##########
src/utils/clientSideFilter.ts:
##########
@@ -0,0 +1,150 @@
+/**
+ * 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 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 name
+ if (filters.name && routeData.name) {
+ const nameMatch = routeData.name
+ .toLowerCase()
+ .includes(filters.name.toLowerCase());
+ if (!nameMatch) return false;
+ }
+
Review Comment:
The filter logic for name in clientSideFilter is redundant since name filtering is already handled by the backend (as mentioned in the PR description). When `needsClientSideFiltering` is true and includes a name filter, the code fetches all data and then filters by name again on the client side, even though the name was already sent to the backend. Consider removing the name filter from clientSideFilter since it's handled by the backend API.
```suggestion
```
##########
src/routes/routes/index.tsx:
##########
@@ -40,14 +50,123 @@ export type RouteListProps = {
}) => React.ReactNode;
};
+const RouteDetailButton = ({
+ record,
+}: {
+ record: APISIXType['RespRouteItem'];
+}) => (
+ <ToDetailPageBtn
+ key="detail"
+ to="/routes/detail/$id"
+ params={{ id: record.value.id }}
+ />
+);
Review Comment:
The RouteDetailButton component is extracted but only used once. While this extraction improves code readability by reducing nesting in the JSX, it doesn't provide significant reusability benefits since it's only used in one place. This is a minor style preference issue and not a functional problem, but consider whether the extraction adds enough value to justify the additional component definition.
##########
e2e/tests/routes.filter-version.spec.ts:
##########
@@ -0,0 +1,146 @@
+/**
+ * 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
+ await expect(page.locator('#version')).toBeAttached();
+ });
Review Comment:
The test "should filter routes by version across all pages" doesn't actually verify that the version filtering functionality works. It only checks that routes are created and the version filter field exists. The test should include steps to actually select a version from the dropdown and verify that only routes with that version are displayed in the filtered results.
```suggestion
// 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');
// v2 routes should be present
await expect(page.getByText('route_v2_1')).toBeVisible();
await expect(page.getByText('route_v2_2')).toBeVisible();
await expect(page.getByText('route_v2_3')).toBeVisible();
// v1 routes should not be present in the filtered results
await expect(page.getByText(/route_v1_/)).toHaveCount(0);
});
```
##########
src/routes/routes/index.tsx:
##########
@@ -123,6 +250,18 @@ export const RouteList = (props: RouteListProps) => {
],
},
}}
+ tableAlertRender={
+ needsAllData
+ ? () => (
+ <span style={{ color: '#faad14' }}>
+ {t('table.searchLimit', {
+ defaultValue: `Search only allows searching in the first ${PAGE_SIZE_MAX} records.`,
+ count: PAGE_SIZE_MAX,
+ })}
+ </span>
+ )
+ : undefined
Review Comment:
The tableAlertRender is only displayed when `needsAllData` is true, but the warning message about searching only the first 500 records could be misleading. The search actually works across all fetched data (which is limited to 500 records via PAGE_SIZE_MAX). Consider clarifying the message to indicate that only the first 500 routes from the database are fetched and searchable, not that search is limited after fetching.
##########
src/utils/clientSideFilter.ts:
##########
@@ -0,0 +1,150 @@
+/**
+ * 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 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 name
+ if (filters.name && routeData.name) {
+ const nameMatch = routeData.name
+ .toLowerCase()
+ .includes(filters.name.toLowerCase());
+ if (!nameMatch) return false;
+ }
+
+ // 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
+ 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
+ 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
+ if (filters.labels && filters.labels.length > 0 && routeData.labels) {
+ 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;
+ }
Review Comment:
The labels filter logic has a potential issue. When a route doesn't have labels (routeData.labels is undefined), and the user provides a labels filter, the route is not filtered out because the condition only checks `routeData.labels` at the end. This means routes without labels will be included in the results when searching for specific labels. Consider explicitly returning false when labels filter is provided but the route has no labels.
##########
e2e/tests/routes.search.spec.ts:
##########
@@ -0,0 +1,128 @@
+/**
+ * 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 { expect} from '@playwright/test';
Review Comment:
Spacing issue after 'expect' import. There's an extra space before the closing brace in the import statement.
```suggestion
import { expect } from '@playwright/test';
```
##########
src/components/form/SearchForm.tsx:
##########
@@ -0,0 +1,247 @@
+/**
+ * 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 { Button, Col, Form, Input, Row, Select, Space } from 'antd';
+import { useEffect, useMemo } from 'react';
+import { useTranslation } from 'react-i18next';
+
+export type SearchFormValues = {
+ name?: string;
+ id?: string;
+ host?: string;
+ path?: string;
+ description?: string;
+ plugin?: string;
+ labels?: string[];
+ version?: string;
+ status?: string;
+};
+
+type Option = {
+ label: string;
+ value: string;
+};
+
+type SearchFormProps = {
+ onSearch?: (values: SearchFormValues) => void;
+ onReset?: (values: SearchFormValues) => void;
+ labelOptions?: Option[];
+ versionOptions?: Option[];
+ statusOptions?: Option[];
+ initialValues?: Partial<SearchFormValues>;
+};
+
+export const SearchForm = (props: SearchFormProps) => {
+ const {
+ onSearch,
+ onReset,
+ labelOptions,
+ versionOptions,
+ statusOptions,
+ initialValues,
+ } = props;
+
+ const { t } = useTranslation('common');
+ const [form] = Form.useForm<SearchFormValues>();
+
+ const defaultStatusOptions = useMemo<Option[]>(
+ () => [
+ {
+ label: t('form.searchForm.status.all'),
+ value: 'UnPublished/Published',
+ },
Review Comment:
The status value 'UnPublished/Published' is used as a magic string in multiple locations (SearchForm.tsx line 64 and clientSideFilter.ts lines 106, 132). Consider extracting this to a named constant to ensure consistency and make it easier to maintain. For example, define `const STATUS_ALL = 'UnPublished/Published'` in a shared constants file.
##########
src/locales/es/common.json:
##########
@@ -88,6 +88,39 @@
"vars": "Variables"
},
"search": "Buscar",
+ "searchForm": {
+ "fields": {
+ "name": "Nombre",
+ "id": "ID",
+ "host": "Host",
+ "path": "Ruta",
+ "description": "Descripción",
+ "plugin": "Plugin",
+ "labels": "Etiquetas",
+ "version": "Versión",
+ "status": "Estado"
+ },
+ "placeholders": {
+ "name": "Nombre",
+ "id": "ID",
+ "host": "Host",
+ "path": "Ruta",
+ "description": "Descripción",
+ "plugin": "Plugin",
+ "labels": "Selecciona etiquetas",
+ "version": "Selecciona versión",
+ "status": "Selecciona estado"
+ },
+ "status": {
+ "all": "Sin publicar/Publicado",
+ "published": "Publicado",
+ "unpublished": "Sin publicar"
+ },
+ "actions": {
+ "reset": "Restablecer",
+ "search": "Buscar"
+ }
+ },
Review Comment:
Missing translations for the "searchLimit" message in Spanish (es) locale. The English locale adds this translation at line 391, but the corresponding translation is not present in the es locale file. This will cause the warning message to fall back to the English default value when using Spanish.
```suggestion
"search": "Buscar",
"searchLimit": "La búsqueda está limitada a los primeros {{limit}} resultados. Por favor, refine sus criterios.",
"searchForm": {
"fields": {
"name": "Nombre",
"id": "ID",
"host": "Host",
"path": "Ruta",
"description": "Descripción",
"plugin": "Plugin",
"labels": "Etiquetas",
"version": "Versión",
"status": "Estado"
},
"placeholders": {
"name": "Nombre",
"id": "ID",
"host": "Host",
"path": "Ruta",
"description": "Descripción",
"plugin": "Plugin",
"labels": "Selecciona etiquetas",
"version": "Selecciona versión",
"status": "Selecciona estado"
},
"status": {
"all": "Sin publicar/Publicado",
"published": "Publicado",
"unpublished": "Sin publicar"
},
"actions": {
"reset": "Restablecer",
"search": "Buscar"
}
},
```
##########
src/routes/routes/index.tsx:
##########
@@ -40,14 +50,123 @@ export type RouteListProps = {
}) => React.ReactNode;
};
+const RouteDetailButton = ({
+ record,
+}: {
+ record: APISIXType['RespRouteItem'];
+}) => (
+ <ToDetailPageBtn
+ key="detail"
+ to="/routes/detail/$id"
+ params={{ id: record.value.id }}
+ />
+);
+
+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 Partial<SearchFormValues>;
Review Comment:
The mapSearchParams function creates a partial SearchFormValues object but doesn't filter out undefined values. This means that when URL parameters are mapped back to the form, undefined values for all 9 filter fields will be included in the object. While this may not cause functional issues, it could lead to unnecessary entries in the params object. Consider filtering out undefined values using Object.entries and filter before creating the returned object.
```suggestion
Object.fromEntries(
SEARCH_PARAM_KEYS
.map((key) => [key, values[key]] as const)
.filter(([, value]) => value !== undefined)
) as Partial<SearchFormValues>;
```
##########
src/locales/en/common.json:
##########
@@ -88,6 +88,33 @@
"vars": "Vars"
},
"search": "Search",
+ "searchForm": {
+ "fields": {
+ "name": "Name",
+ "id": "ID",
+ "host": "Host",
+ "path": "Path",
+ "description": "Description",
+ "plugin": "Plugin",
+ "labels": "Labels",
+ "version": "Version",
+ "status": "Status"
+ },
+ "placeholders": {
+ "labels": "Select labels",
+ "version": "Select version",
+ "status": "Select status"
+ },
Review Comment:
Inconsistent placeholder handling in the English locale. For text input fields (name, id, host, path, description, plugin), the English locale doesn't define placeholders in the "placeholders" section, while the Spanish and German locales do. This creates an inconsistency where Spanish and German users will see placeholder text in these fields, but English users won't. Consider either adding placeholders for English or removing them from other locales for consistency.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2869566002
##########
src/components/form/SearchForm.tsx:
##########
@@ -0,0 +1,249 @@
+/**
+ * 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 { Button, Col, Form, Input, Row, Select, Space } from 'antd';
+import { useEffect, useMemo } from 'react';
+import { useTranslation } from 'react-i18next';
+
+import { STATUS_ALL } from '@/config/constant';
+
+export type SearchFormValues = {
+ name?: string;
+ id?: string;
+ host?: string;
+ path?: string;
+ description?: string;
+ plugin?: string;
+ labels?: string[];
+ version?: string;
+ status?: string;
+};
+
+type Option = {
+ label: string;
+ value: string;
+};
+
+type SearchFormProps = {
+ onSearch?: (values: SearchFormValues) => void;
+ onReset?: (values: SearchFormValues) => void;
+ labelOptions?: Option[];
+ versionOptions?: Option[];
+ statusOptions?: Option[];
+ initialValues?: Partial<SearchFormValues>;
+};
+
+export const SearchForm = (props: SearchFormProps) => {
+ const {
+ onSearch,
+ onReset,
+ labelOptions,
+ versionOptions,
+ statusOptions,
+ initialValues,
+ } = props;
+
+ const { t } = useTranslation('common');
+ const [form] = Form.useForm<SearchFormValues>();
+
+ const defaultStatusOptions = useMemo<Option[]>(
+ () => [
+ {
+ label: t('form.searchForm.status.all'),
+ value: STATUS_ALL,
+ },
+ {
+ label: t('form.searchForm.status.published'),
+ value: 'Published',
+ },
+ {
+ label: t('form.searchForm.status.unpublished'),
+ value: 'UnPublished',
+ },
+ ],
+ [t]
+ );
+
+ const mergedStatusOptions = useMemo(
+ () => statusOptions ?? defaultStatusOptions,
+ [defaultStatusOptions, statusOptions]
+ );
+ const resolvedInitialValues = useMemo(() => {
+ const defaultStatus = mergedStatusOptions[0]?.value ?? undefined;
+ return {
+ status: defaultStatus,
+ ...initialValues,
+ } satisfies SearchFormValues;
+ }, [initialValues, mergedStatusOptions]);
+
+ useEffect(() => {
+ form.setFieldsValue(resolvedInitialValues);
+ }, [form, resolvedInitialValues]);
+
+ const handleFinish = (values: SearchFormValues) => {
+ onSearch?.(values);
+ };
+
+ const handleReset = async () => {
+ form.resetFields();
+ form.setFieldsValue(resolvedInitialValues);
+ const values = form.getFieldsValue();
+ if (onReset) {
+ onReset(values);
+ } else {
+ onSearch?.(values);
+ }
+ };
+
+ return (
+ <Form
+ form={form}
+ layout="vertical"
+ autoComplete="off"
+ onFinish={handleFinish}
+ style={{
+ padding: '20px',
+ background: '#fafafa',
+ borderRadius: '8px',
+ border: '1px solid #e8e8e8',
+ }}
+ >
+ <Row gutter={[16, 0]}>
+ {/* First column - spans 2 rows */}
+ <Col xs={24} sm={12} md={6}>
+ <Form.Item<SearchFormValues>
+ name="name"
+ label={t('form.searchForm.fields.name')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input allowClear />
+ </Form.Item>
+ <Form.Item<SearchFormValues>
+ name="id"
+ label={t('form.searchForm.fields.id')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input allowClear />
+ </Form.Item>
+ </Col>
+
+ {/* Second column - first row */}
+ <Col xs={24} sm={12} md={6}>
+ <Form.Item<SearchFormValues>
+ name="host"
+ label={t('form.searchForm.fields.host')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input allowClear />
+ </Form.Item>
+ {/* Second column - second row */}
+ <Form.Item<SearchFormValues>
+ name="plugin"
+ label={t('form.searchForm.fields.plugin')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input allowClear />
+ </Form.Item>
+ </Col>
+
+ {/* Third column - first row */}
+ <Col xs={24} sm={12} md={6}>
+ <Form.Item<SearchFormValues>
+ name="path"
+ label={t('form.searchForm.fields.path')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input allowClear />
+ </Form.Item>
+ {/* Third column - second row */}
+ <Form.Item<SearchFormValues>
+ name="labels"
+ label={t('form.searchForm.fields.labels')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Select
+ mode="multiple"
+ placeholder={t('form.searchForm.placeholders.labels')}
+ allowClear
+ options={labelOptions}
+ />
+ </Form.Item>
+ </Col>
+
+ {/* Fourth column - first row */}
+ <Col xs={24} sm={12} md={6}>
+ <Form.Item<SearchFormValues>
+ name="description"
+ label={t('form.searchForm.fields.description')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input allowClear />
+ </Form.Item>
Review Comment:
Fixed
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2869597812
##########
src/components/form/SearchForm.tsx:
##########
@@ -0,0 +1,249 @@
+/**
+ * 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 { Button, Col, Form, Input, Row, Select, Space } from 'antd';
+import { useEffect, useMemo } from 'react';
+import { useTranslation } from 'react-i18next';
+
+import { STATUS_ALL } from '@/config/constant';
+
+export type SearchFormValues = {
+ name?: string;
+ id?: string;
+ host?: string;
+ path?: string;
+ description?: string;
+ plugin?: string;
+ labels?: string[];
+ version?: string;
+ status?: string;
+};
+
+type Option = {
+ label: string;
+ value: string;
+};
+
+type SearchFormProps = {
+ onSearch?: (values: SearchFormValues) => void;
+ onReset?: (values: SearchFormValues) => void;
+ labelOptions?: Option[];
+ versionOptions?: Option[];
+ statusOptions?: Option[];
+ initialValues?: Partial<SearchFormValues>;
+};
+
+export const SearchForm = (props: SearchFormProps) => {
+ const {
+ onSearch,
+ onReset,
+ labelOptions,
+ versionOptions,
+ statusOptions,
+ initialValues,
+ } = props;
+
+ const { t } = useTranslation('common');
+ const [form] = Form.useForm<SearchFormValues>();
+
+ const defaultStatusOptions = useMemo<Option[]>(
+ () => [
+ {
+ label: t('form.searchForm.status.all'),
+ value: STATUS_ALL,
+ },
+ {
+ label: t('form.searchForm.status.published'),
+ value: 'Published',
+ },
+ {
+ label: t('form.searchForm.status.unpublished'),
+ value: 'UnPublished',
+ },
+ ],
+ [t]
+ );
+
+ const mergedStatusOptions = useMemo(
+ () => statusOptions ?? defaultStatusOptions,
+ [defaultStatusOptions, statusOptions]
+ );
+ const resolvedInitialValues = useMemo(() => {
+ const defaultStatus = mergedStatusOptions[0]?.value ?? undefined;
+ return {
+ status: defaultStatus,
+ ...initialValues,
+ } satisfies SearchFormValues;
+ }, [initialValues, mergedStatusOptions]);
+
+ useEffect(() => {
+ form.setFieldsValue(resolvedInitialValues);
+ }, [form, resolvedInitialValues]);
+
+ const handleFinish = (values: SearchFormValues) => {
+ onSearch?.(values);
+ };
+
+ const handleReset = async () => {
+ form.resetFields();
+ form.setFieldsValue(resolvedInitialValues);
+ const values = form.getFieldsValue();
+ if (onReset) {
+ onReset(values);
+ } else {
+ onSearch?.(values);
+ }
+ };
+
+ return (
+ <Form
+ form={form}
+ layout="vertical"
+ autoComplete="off"
+ onFinish={handleFinish}
+ style={{
+ padding: '20px',
+ background: '#fafafa',
+ borderRadius: '8px',
+ border: '1px solid #e8e8e8',
+ }}
+ >
+ <Row gutter={[16, 0]}>
+ {/* First column - spans 2 rows */}
+ <Col xs={24} sm={12} md={6}>
+ <Form.Item<SearchFormValues>
+ name="name"
+ label={t('form.searchForm.fields.name')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input allowClear />
+ </Form.Item>
+ <Form.Item<SearchFormValues>
+ name="id"
+ label={t('form.searchForm.fields.id')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input allowClear />
+ </Form.Item>
+ </Col>
+
+ {/* Second column - first row */}
+ <Col xs={24} sm={12} md={6}>
+ <Form.Item<SearchFormValues>
+ name="host"
+ label={t('form.searchForm.fields.host')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input allowClear />
+ </Form.Item>
+ {/* Second column - second row */}
+ <Form.Item<SearchFormValues>
+ name="plugin"
+ label={t('form.searchForm.fields.plugin')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input allowClear />
+ </Form.Item>
+ </Col>
+
+ {/* Third column - first row */}
+ <Col xs={24} sm={12} md={6}>
+ <Form.Item<SearchFormValues>
+ name="path"
+ label={t('form.searchForm.fields.path')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input allowClear />
+ </Form.Item>
+ {/* Third column - second row */}
+ <Form.Item<SearchFormValues>
+ name="labels"
+ label={t('form.searchForm.fields.labels')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Select
+ mode="multiple"
+ placeholder={t('form.searchForm.placeholders.labels')}
+ allowClear
+ options={labelOptions}
+ />
+ </Form.Item>
Review Comment:
Fixed
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-3540132992
Yes, you're right - it was only filtering the current page data. I've fixed this in commit [ed84e0b](https://github.com/apache/apisix-dashboard/pull/3222/commits/ed84e0b95606486aab39c9e9af6246b18505e186) . Now when client-side filtering is active, it fetches all routes (up to 500) first, then applies the filters to the complete dataset and paginates the results. This ensures filtering works across all pages.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "SkyeYoung (via GitHub)" <gi...@apache.org>.
SkyeYoung commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-3561024808
pls fix CI
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-3551866907
I have tried fixing the failed CI.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-3379493524
Well I can do that too need some time for this though...
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 closed pull request #3222: feat: add route search form with 9 filter fields
URL: https://github.com/apache/apisix-dashboard/pull/3222
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2869596848
##########
src/routes/routes/index.tsx:
##########
@@ -40,14 +50,134 @@ 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: setRouteListParams } = useRouteList(
routeKey,
defaultParams
);
+ const { params, resetParams, setParams: setSearchParams } = useSearchParams(routeKey) as {
+ params: PageSearchType;
+ resetParams: (defaults?: Partial<PageSearchType>) => void;
+ setParams: (params: Partial<PageSearchType>) => void;
+ };
const { t } = useTranslation();
+ // Client-side filtering strategy:
+ // The backend API has limited support for complex filtering (e.g., fuzzy search on description,
+ // plugin content) combined with pagination. Therefore, we use client-side filtering for these fields.
+ // Limitation: We only fetch the first PAGE_SIZE_MAX records. If the total number of records
+ // exceeds this limit, the filter may miss matching records that are not in the first batch.
+ // This limitation is communicated to the user via a warning alert in the table.
+ const needsAllData = needsClientSideFiltering(params);
+ 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 } : {}),
+ }),
+ enabled: needsAllData,
+ });
+
+ const handleSearch = (values: SearchFormValues) => {
+ // Send name filter to backend, keep others for client-side filtering
+ setRouteListParams({
+ page: 1,
+ ...mapSearchParams(values),
+ });
+ };
+
+ const handleReset = () => {
+ // Clear existing search params and then re-apply the default status filter
+ resetParams({
+ page: 1,
+ status: STATUS_ALL,
+ });
+ };
+
+ // Apply client-side filtering and pagination
+ const { filteredData, totalCount } = useMemo(() => {
+ // If client-side filtering is needed, use all data
+ if (needsAllData && allData?.list) {
+ const filtered = filterRoutes(allData.list, params);
+ const paginated = paginateResults(
+ filtered,
+ params.page || 1,
+ params.page_size || 10
+ );
+ return {
+ filteredData: paginated.list,
+ totalCount: paginated.total,
+ };
+ }
+
+ // Otherwise, use paginated data from backend
+ return {
+ filteredData: data?.list || [],
+ totalCount: data?.total || 0,
+ };
+ }, [needsAllData, allData, data, params]);
+
+ const actualLoading = needsAllData ? isLoadingAllData : isLoading;
+
+ // Update pagination to use filtered total and prevent unnecessary refetch during client-side pagination
+ const customPagination = useMemo(() => {
+ return {
+ ...pagination,
+ total: totalCount,
+ // When client-side filtering is active, only update URL params without triggering refetch
+ // since all data is already loaded
+ onChange: needsAllData
+ ? (page: number, pageSize: number) => {
+ setSearchParams({ page, page_size: pageSize });
+ }
+ : pagination.onChange,
+ };
+ }, [pagination, totalCount, needsAllData, setSearchParams]);
+
+ // Extract unique version values from route labels
+ const versionOptions = useMemo(() => {
+ const dataSource = needsAllData && allData?.list ? allData.list : data?.list || [];
+ const versions = new Set<string>();
+
+ dataSource.forEach((route) => {
+ const versionLabel = route.value.labels?.version;
+ if (versionLabel) {
+ versions.add(versionLabel);
+ }
+ });
+
+ return Array.from(versions)
+ .sort()
+ .map((version) => ({
+ label: version,
+ value: version,
+ }));
+ }, [needsAllData, allData, data]);
Review Comment:
Fixed
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2869602074
##########
src/utils/clientSideFilter.ts:
##########
@@ -0,0 +1,161 @@
+/**
+ * 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 name
+ if (filters.name) {
+ const nameMatch = routeData.name
+ ?.toLowerCase()
+ .includes(filters.name.toLowerCase());
+ if (!nameMatch) return false;
+ }
+
+ // 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 hosts = Array.isArray((routeData as unknown as Record<string, string[]>).hosts)
+ ? (routeData as unknown as Record<string, string[]>).hosts.join(',')
+ : '';
+ const combinedHost = `${host} ${hosts}`.toLowerCase();
+ const hostMatch = combinedHost.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:
I’ve intentionally kept this pattern to preserve URL deep linking and browser history support (back/forward navigation). Decoupling via local state would also require a significant refactor of our E2E test suite (e.g., pagination test helper.ts), which relies on URL assertions. Since these paginated requests are lightweight, I think accepting the redundancy as a trade off for maintaining a consistent, URL-driven state across the dashboard will be better.
##########
src/utils/clientSideFilter.ts:
##########
@@ -0,0 +1,161 @@
+/**
+ * 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 name
+ if (filters.name) {
+ const nameMatch = routeData.name
+ ?.toLowerCase()
+ .includes(filters.name.toLowerCase());
+ if (!nameMatch) return false;
+ }
+
+ // 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 hosts = Array.isArray((routeData as unknown as Record<string, string[]>).hosts)
+ ? (routeData as unknown as Record<string, string[]>).hosts.join(',')
+ : '';
+ const combinedHost = `${host} ${hosts}`.toLowerCase();
+ const hostMatch = combinedHost.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:
I’ve intentionally kept this pattern to preserve URL deep linking and browser history support (back/forward navigation). Decoupling via local state would also require a significant refactor of our E2E test suite (e.g., pagination test helper.ts), which relies on URL assertions. Since these paginated requests are lightweight, I think accepting the redundancy as a trade off for maintaining a consistent, URL driven state across the dashboard will be better.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2869607160
##########
src/utils/clientSideFilter.ts:
##########
@@ -0,0 +1,161 @@
+/**
+ * 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 name
+ if (filters.name) {
+ const nameMatch = routeData.name
+ ?.toLowerCase()
+ .includes(filters.name.toLowerCase());
+ if (!nameMatch) return false;
+ }
+
+ // 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 hosts = Array.isArray((routeData as unknown as Record<string, string[]>).hosts)
+ ? (routeData as unknown as Record<string, string[]>).hosts.join(',')
+ : '';
+ const combinedHost = `${host} ${hosts}`.toLowerCase();
+ const hostMatch = combinedHost.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:
Fixed in [eef0a79](https://github.com/apache/apisix-dashboard/pull/3222/commits/eef0a792a1b0e653a742e6638e21696788cc24f2)
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-3793781455
Any updates on this PR?
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "Baoyuantop (via GitHub)" <gi...@apache.org>.
Baoyuantop commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2622035704
##########
src/components/form/Editor.tsx:
##########
@@ -16,7 +16,7 @@
*/
import { InputWrapper, type InputWrapperProps, Skeleton } from '@mantine/core';
import { Editor } from '@monaco-editor/react';
-import { clsx } from 'clsx';
Review Comment:
Why modify this part?
##########
src/routes/routes/index.tsx:
##########
@@ -16,20 +16,30 @@
*/
import type { ProColumns } from '@ant-design/pro-components';
import { ProTable } from '@ant-design/pro-components';
+import { useQuery } from '@tanstack/react-query';
import { createFileRoute } from '@tanstack/react-router';
import { useMemo } from 'react';
import { useTranslation } from 'react-i18next';
import { getRouteListQueryOptions, useRouteList } from '@/apis/hooks';
import type { WithServiceIdFilter } from '@/apis/routes';
+import { getRouteListReq } from '@/apis/routes';
+import { SearchForm, type SearchFormValues } from '@/components/form/SearchForm';
import { DeleteResourceBtn } from '@/components/page/DeleteResourceBtn';
import PageHeader from '@/components/page/PageHeader';
import { ToAddPageBtn, ToDetailPageBtn } from '@/components/page/ToAddPageBtn';
import { AntdConfigProvider } from '@/config/antdConfigProvider';
-import { API_ROUTES } from '@/config/constant';
+import { API_ROUTES, PAGE_SIZE_MAX } from '@/config/constant';
Review Comment:
1. If the backend supports a larger page_size, consider increasing it appropriately, or implement a loop-based pagination system on the frontend to retrieve all data (performance needs to be considered).
2. If retrieving all data is not feasible at the moment, a prompt should be provided in the UI informing the user to "search only in the first 500 records".
##########
src/routes/routes/index.tsx:
##########
@@ -16,20 +16,30 @@
*/
import type { ProColumns } from '@ant-design/pro-components';
import { ProTable } from '@ant-design/pro-components';
+import { useQuery } from '@tanstack/react-query';
import { createFileRoute } from '@tanstack/react-router';
import { useMemo } from 'react';
import { useTranslation } from 'react-i18next';
import { getRouteListQueryOptions, useRouteList } from '@/apis/hooks';
import type { WithServiceIdFilter } from '@/apis/routes';
+import { getRouteListReq } from '@/apis/routes';
+import { SearchForm, type SearchFormValues } from '@/components/form/SearchForm';
import { DeleteResourceBtn } from '@/components/page/DeleteResourceBtn';
import PageHeader from '@/components/page/PageHeader';
import { ToAddPageBtn, ToDetailPageBtn } from '@/components/page/ToAddPageBtn';
import { AntdConfigProvider } from '@/config/antdConfigProvider';
-import { API_ROUTES } from '@/config/constant';
+import { API_ROUTES, PAGE_SIZE_MAX } from '@/config/constant';
Review Comment:
If we use the PAGE_SIZE_MAX limit, then we should clearly inform the user of this limit in the functional area.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "Baoyuantop (via GitHub)" <gi...@apache.org>.
Baoyuantop commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-3680848362
> > The changes to the zh language file were lost.
>
> Thanks for catching that! I can see the comprehensive Chinese translations have been restored on the remote. My local branch is now synced.
> I can see the comprehensive Chinese translations have been restored on the remote
Where are you referring to? Could you provide a link?
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-3547174103
Added a test in routes.list.spec.ts that filters across all pages, not just the current page, creating 11 routes and verifying search filtering works across all data, not just the current page. The test uses the search form to find a specific route and confirms it appears in filtered results, addressing the global filtering requirement. All tests pass.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-4206090596
Hi @Baoyuantop all the checks pass can you please review this PR.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2869592843
##########
src/utils/clientSideFilter.ts:
##########
@@ -0,0 +1,161 @@
+/**
+ * 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 name
+ if (filters.name) {
+ const nameMatch = routeData.name
+ ?.toLowerCase()
+ .includes(filters.name.toLowerCase());
+ if (!nameMatch) return false;
+ }
+
+ // 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 hosts = Array.isArray((routeData as unknown as Record<string, string[]>).hosts)
+ ? (routeData as unknown as Record<string, string[]>).hosts.join(',')
+ : '';
+ const combinedHost = `${host} ${hosts}`.toLowerCase();
+ const hostMatch = combinedHost.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:
`description` filtering currently only runs when `routeData.desc` is truthy (`if (filters.description && routeData.desc)`), which means routes without a `desc` field will *not* be filtered out when a description filter is active. This contradicts the comment above and will produce incorrect results (routes with no description can still appear). Adjust the condition so that when `filters.description` is set, routes missing `desc` are excluded and the match check always runs against a string value.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2532822741
##########
src/components/form/SearchForm.tsx:
##########
@@ -0,0 +1,244 @@
+/**
+ * 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 { Button, Col, Form, Input, Row, Select, Space } from 'antd';
+import { useEffect, useMemo } from 'react';
+import { useTranslation } from 'react-i18next';
+
+export type SearchFormValues = {
+ name?: string;
+ id?: string;
+ host?: string;
+ path?: string;
+ description?: string;
+ plugin?: string;
+ labels?: string[];
+ version?: string;
+ status?: string;
+};
+
+type Option = {
+ label: string;
+ value: string;
+};
+
+type SearchFormProps = {
+ onSearch?: (values: SearchFormValues) => void;
+ onReset?: (values: SearchFormValues) => void;
+ labelOptions?: Option[];
+ versionOptions?: Option[];
+ statusOptions?: Option[];
+ initialValues?: Partial<SearchFormValues>;
+};
+
+export const SearchForm = (props: SearchFormProps) => {
+ const {
+ onSearch,
+ onReset,
+ labelOptions,
+ versionOptions,
+ statusOptions,
+ initialValues,
+ } = props;
+
+ const { t } = useTranslation('common');
+ const [form] = Form.useForm<SearchFormValues>();
+
+ const defaultStatusOptions = useMemo<Option[]>(
+ () => [
+ {
+ label: t('form.searchForm.status.all'),
+ value: 'UnPublished/Published',
+ },
+ {
+ label: t('form.searchForm.status.published'),
+ value: 'Published',
+ },
+ {
+ label: t('form.searchForm.status.unpublished'),
+ value: 'UnPublished',
+ },
+ ],
+ [t]
+ );
+
+ const mergedStatusOptions = useMemo(
+ () => statusOptions ?? defaultStatusOptions,
+ [defaultStatusOptions, statusOptions]
+ );
+ const resolvedInitialValues = useMemo(() => {
+ const defaultStatus = mergedStatusOptions[0]?.value ?? undefined;
+ return {
+ status: defaultStatus,
+ ...initialValues,
+ } satisfies SearchFormValues;
+ }, [initialValues, mergedStatusOptions]);
+
+ useEffect(() => {
+ form.setFieldsValue(resolvedInitialValues);
+ }, [form, resolvedInitialValues]);
+
+ const handleFinish = (values: SearchFormValues) => {
+ onSearch?.(values);
+ };
+
+ const handleReset = async () => {
+ form.resetFields();
+ form.setFieldsValue(resolvedInitialValues);
+ const values = form.getFieldsValue();
+ onReset?.(values);
+ onSearch?.(values);
+ };
+
+ return (
+ <Form
+ form={form}
+ layout="vertical"
+ autoComplete="off"
+ onFinish={handleFinish}
+ style={{
+ padding: '20px',
+ background: '#fafafa',
+ borderRadius: '8px',
+ border: '1px solid #e8e8e8',
+ }}
+ >
+ <Row gutter={[16, 0]}>
+ {/* First column - spans 2 rows */}
+ <Col xs={24} sm={12} md={6}>
+ <Form.Item<SearchFormValues>
+ name="name"
+ label={t('form.searchForm.fields.name')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input allowClear />
+ </Form.Item>
+ <Form.Item<SearchFormValues>
+ name="id"
+ label={t('form.searchForm.fields.id')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input allowClear />
+ </Form.Item>
+ </Col>
+
+ {/* Second column - first row */}
+ <Col xs={24} sm={12} md={6}>
+ <Form.Item<SearchFormValues>
+ name="host"
+ label={t('form.searchForm.fields.host')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input allowClear />
+ </Form.Item>
+ {/* Second column - second row */}
+ <Form.Item<SearchFormValues>
+ name="plugin"
+ label={t('form.searchForm.fields.plugin')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input allowClear />
+ </Form.Item>
+ </Col>
+
+ {/* Third column - first row */}
+ <Col xs={24} sm={12} md={6}>
+ <Form.Item<SearchFormValues>
+ name="path"
+ label={t('form.searchForm.fields.path')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input allowClear />
+ </Form.Item>
+ {/* Third column - second row */}
+ <Form.Item<SearchFormValues>
+ name="labels"
+ label={t('form.searchForm.fields.labels')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Select
+ mode="multiple"
+ placeholder={t('form.searchForm.placeholders.labels')}
+ allowClear
+ options={labelOptions}
+ />
+ </Form.Item>
+ </Col>
+
+ {/* Fourth column - first row */}
+ <Col xs={24} sm={12} md={6}>
+ <Form.Item<SearchFormValues>
+ name="description"
+ label={t('form.searchForm.fields.description')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Input allowClear />
+ </Form.Item>
+ {/* Fourth column - second row */}
+ <Form.Item<SearchFormValues>
+ name="version"
+ label={t('form.searchForm.fields.version')}
+ style={{ marginBottom: '16px' }}
+ >
+ <Select
+ placeholder={t('form.searchForm.placeholders.version')}
+ allowClear
+ options={versionOptions}
+ />
+ </Form.Item>
Review Comment:
This has been resolved in [ed84e0b](https://github.com/apache/apisix-dashboard/pull/3222/commits/ed84e0b95606486aab39c9e9af6246b18505e186)
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "Baoyuantop (via GitHub)" <gi...@apache.org>.
Baoyuantop commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-3540347884
> Yes, you're right - it was only filtering the current page data. I've fixed this in commit [ed84e0b](https://github.com/apache/apisix-dashboard/pull/3222/commits/ed84e0b95606486aab39c9e9af6246b18505e186) . Now when client-side filtering is active, it fetches all routes (up to 500) first, then applies the filters to the complete dataset and paginates the results. This ensures filtering works across all pages.
We also need to conduct e2e tests to verify this.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "Baoyuantop (via GitHub)" <gi...@apache.org>.
Baoyuantop commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-3550359683
Please fix the failed CI
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-3415920540
I think this change of import from clsx to claassNames would pass the workflow.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "SkyeYoung (via GitHub)" <gi...@apache.org>.
SkyeYoung commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-3413757837
Hi, pls resolve conflicts, and merge master, thx 😸
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#issuecomment-3413985477
Hi @SkyeYoung & @Baoyuantop, I have resolved the conflict ...
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2453859397
##########
src/utils/clientSideFilter.ts:
##########
@@ -0,0 +1,143 @@
+/**
+ * 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 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 name
+ if (filters.name && routeData.name) {
+ const nameMatch = routeData.name
+ .toLowerCase()
+ .includes(filters.name.toLowerCase());
+ if (!nameMatch) return false;
+ }
+
+ // 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
+ if (filters.description && routeData.desc) {
+ const descMatch = routeData.desc
+ .toLowerCase()
+ .includes(filters.description.toLowerCase());
+ if (!descMatch) return false;
+ }
+
+ // Filter by plugin
+ 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
+ if (filters.labels && filters.labels.length > 0 && routeData.labels) {
+ const routeLabels = Object.keys(routeData.labels).map((key) =>
+ `${key}:${routeData.labels![key]}`.toLowerCase()
+ );
Review Comment:
I’ve added inline comments describing how the plugin substring match works and how the label key:value filtering behaves, so future maintainers should have more context.
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat: add route search form with 9 filter fields [apisix-dashboard]
Posted by "DSingh0304 (via GitHub)" <gi...@apache.org>.
DSingh0304 commented on code in PR #3222:
URL: https://github.com/apache/apisix-dashboard/pull/3222#discussion_r2622091292
##########
src/components/form/Editor.tsx:
##########
@@ -16,7 +16,7 @@
*/
import { InputWrapper, type InputWrapperProps, Skeleton } from '@mantine/core';
import { Editor } from '@monaco-editor/react';
-import { clsx } from 'clsx';
Review Comment:
This was an unintentional style change. I've reverted it to match the original code (`import { clsx } from 'clsx'`).
--
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: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org