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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
