fix(dashboard): Limit perPage and page uri changes (#3845) #3873
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdd pagination constants and validators, validate and bound page/per-page values, persist items-per-page in user settings, and propagate validated per-page values through list pages and table components; UI per-page options and pagination UI sizing are updated to use the new constants. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant LP as ListPage
participant USP as UserSettingsProvider
participant DT as DataTable
participant PU as pagination.util
Note over LP,USP: Initial load
LP->>USP: request settings (itemsPerPage)
USP->>PU: validatePerPageValue(itemsPerPage)
PU-->>USP: validated perPage
LP->>DT: props { page, itemsPerPage, totalItems }
DT->>DT: compute totalPages, bound page/pageSize
DT-->>U: render table with pagination
Note over U,PU: User changes "Rows per page"
U->>DT: select perPage option
DT-->>LP: onPageChange(perPage, page)
LP->>PU: validatePerPageValue(perPage)
PU-->>LP: validated perPage
LP->>USP: setItemsPerPage(validated perPage)
USP-->>LP: persist + update context
LP->>DT: updated props (itemsPerPage)
DT->>DT: sync internal pagination via useEffect
DT-->>U: re-render with new page size
sequenceDiagram
autonumber
participant SRV as Server settings
participant USP as UserSettingsProvider
participant PU as pagination.util
Note over SRV,USP: Settings hydration/merge
SRV-->>USP: server-provided settings
USP->>PU: validatePerPageValue(server.itemsPerPage)
PU-->>USP: validated itemsPerPage
USP-->>USP: merge with defaults and local storage (validated)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/dashboard/src/lib/components/data-table/data-table-pagination.tsx (1)
31-31: Consider testing the max-width constraints.Changing from fixed widths (
w-[70px],w-[100px]) to max-widths (max-w-20,max-w-28) improves responsive behavior but could cause layout shifts if content exceeds the constraint. Verify that text truncation or wrapping works as expected for all supported locales and page sizes.Also applies to: 44-44
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/dashboard/src/lib/components/data-table/data-table-pagination.tsx(3 hunks)packages/dashboard/src/lib/components/data-table/data-table.tsx(3 hunks)packages/dashboard/src/lib/framework/page/list-page.tsx(3 hunks)packages/dashboard/src/lib/providers/user-settings.tsx(7 hunks)packages/dashboard/src/lib/utils/pagination.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
packages/dashboard/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
packages/dashboard/src/**/*.{ts,tsx}: Use TanStack Query (useQuery or useMutation) for data fetching; do not use Apollo Client.
When creating useQuery calls, follow the provided pattern: import { api } from '@/graphql/api.js'; import { graphql } from '@/graphql/graphql.js'; use useQuery with queryKey, staleTime, and queryFn using api.query.
When performing mutations, follow the provided useMutation pattern: use graphql() for the mutation document, api.mutate for mutationFn, and do not pass variables at declaration.
Files:
packages/dashboard/src/lib/utils/pagination.tspackages/dashboard/src/lib/components/data-table/data-table.tsxpackages/dashboard/src/lib/components/data-table/data-table-pagination.tsxpackages/dashboard/src/lib/providers/user-settings.tsxpackages/dashboard/src/lib/framework/page/list-page.tsx
packages/dashboard/src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
React component props objects should be typed as Readonly<...>.
Files:
packages/dashboard/src/lib/utils/pagination.tspackages/dashboard/src/lib/components/data-table/data-table.tsxpackages/dashboard/src/lib/components/data-table/data-table-pagination.tsxpackages/dashboard/src/lib/providers/user-settings.tsxpackages/dashboard/src/lib/framework/page/list-page.tsx
packages/dashboard/src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
packages/dashboard/src/**/*.{tsx,jsx}: Use React for all UI components in the dashboard package.
Prefer re-using components from /src/lib/components, especially Shadcn components from /src/lib/components/ui.
Prefer using the FormFieldWrapper component for forms over raw Shadcn components.
All labels or user-facing messages should use localization via the Trans component or useLingui().
Files:
packages/dashboard/src/lib/components/data-table/data-table.tsxpackages/dashboard/src/lib/components/data-table/data-table-pagination.tsxpackages/dashboard/src/lib/providers/user-settings.tsxpackages/dashboard/src/lib/framework/page/list-page.tsx
packages/dashboard/src/lib/components/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
Use Shadcn UI and Tailwind CSS for UI components.
Files:
packages/dashboard/src/lib/components/data-table/data-table.tsxpackages/dashboard/src/lib/components/data-table/data-table-pagination.tsx
🧬 Code graph analysis (3)
packages/dashboard/src/lib/components/data-table/data-table-pagination.tsx (1)
packages/dashboard/src/lib/components/ui/select.tsx (3)
SelectTrigger(181-181)SelectValue(182-182)SelectContent(174-174)
packages/dashboard/src/lib/providers/user-settings.tsx (1)
packages/dashboard/src/lib/utils/pagination.ts (1)
validatePerPageValue(7-12)
packages/dashboard/src/lib/framework/page/list-page.tsx (1)
packages/dashboard/src/lib/utils/pagination.ts (1)
validatePerPageValue(7-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (19)
- GitHub Check: unit tests (24.x)
- GitHub Check: unit tests (22.x)
- GitHub Check: build (24.x)
- GitHub Check: unit tests (20.x)
- GitHub Check: build (22.x)
- GitHub Check: build (20.x)
- GitHub Check: publish_install (macos-latest, 24.x)
- GitHub Check: publish_install (macos-latest, 22.x)
- GitHub Check: publish_install (windows-latest, 24.x)
- GitHub Check: publish_install (macos-latest, 20.x)
- GitHub Check: publish_install (ubuntu-latest, 22.x)
- GitHub Check: publish_install (ubuntu-latest, 24.x)
- GitHub Check: publish_install (ubuntu-latest, 20.x)
- GitHub Check: publish_install (windows-latest, 20.x)
- GitHub Check: publish_install (windows-latest, 22.x)
- GitHub Check: publish_install (ubuntu-latest, 22.x)
- GitHub Check: publish_install (ubuntu-latest, 24.x)
- GitHub Check: publish_install (windows-latest, 24.x)
- GitHub Check: publish_install (windows-latest, 22.x)
🔇 Additional comments (7)
packages/dashboard/src/lib/components/data-table/data-table-pagination.tsx (1)
7-7: LGTM! Centralized per-page values.The use of
ALLOWED_PER_PAGE_VALUESfrom constants ensures consistency across pagination components and aligns with the PR's goal of standardizing pagination behavior.Also applies to: 35-39
packages/dashboard/src/lib/framework/page/list-page.tsx (1)
490-490: LGTM! Global itemsPerPage now persisted.The change from per-table settings to a global
itemsPerPagepreference viasetItemsPerPageensures consistent pagination across all list pages, which aligns with the PR objectives.Also applies to: 568-568
packages/dashboard/src/lib/components/data-table/data-table.tsx (2)
125-134: LGTM! Safe pagination with bounds checking.The safe page calculation ensures the requested page stays within
[1, totalPages], preventing invalid pagination states. The initialization withsafePageand validatedpageSizeis correct.
184-195: LGTM! Pagination state synchronized with props.The
useEffectcorrectly syncs internal pagination state whensafePageorpageSizechanges, ensuring the table reflects validated prop-driven pagination values. The dependency array is correct to avoid infinite loops.packages/dashboard/src/lib/utils/pagination.ts (1)
1-12: LGTM! Centralized pagination validation.The
validatePerPageValueutility provides consistent validation across all pagination-related components. The type castas (typeof ALLOWED_PER_PAGE_VALUES)[number]is necessary for TypeScript's type narrowing and the fallback toDEFAULT_PER_PAGEensures robustness.packages/dashboard/src/lib/providers/user-settings.tsx (2)
30-30: LGTM! ItemsPerPage integrated into user settings.The addition of
itemsPerPageto theUserSettingsinterface, default settings, and context type is consistent with the existing settings structure and supports the PR's goal of global pagination preferences.Also applies to: 45-45, 66-66
90-95: LGTM! Consistent validation across all settings paths.
validatePerPageValueis correctly applied when loading from localStorage (lines 90-95), merging server settings (lines 157-164), and updating viasetItemsPerPage(line 228), ensuringitemsPerPagealways contains a valid value regardless of the source.Also applies to: 157-164, 228-228
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/dashboard/src/lib/framework/page/list-page.tsx (1)
28-28: Remove unused import.The
DEFAULT_PER_PAGEconstant is imported but not used anywhere in this file. After addressing the previous review comment that removed the redundant?? DEFAULT_PER_PAGEfallback, this import became unnecessary.Apply this diff:
-import { DEFAULT_PER_PAGE } from '@/vdb/constants.js';packages/dashboard/src/lib/providers/user-settings.tsx (1)
83-83: Wrap component props in Readonly.Per coding guidelines, React component props objects should be typed as
Readonly<...>.As per coding guidelines
Apply this diff:
-export const UserSettingsProvider: React.FC<UserSettingsProviderProps> = ({ queryClient, children }) => { +export const UserSettingsProvider: React.FC<Readonly<UserSettingsProviderProps>> = ({ queryClient, children }) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/dashboard/src/lib/constants.ts(1 hunks)packages/dashboard/src/lib/framework/page/list-page.tsx(3 hunks)packages/dashboard/src/lib/providers/user-settings.tsx(8 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
packages/dashboard/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
packages/dashboard/src/**/*.{ts,tsx}: Use TanStack Query (useQuery or useMutation) for data fetching; do not use Apollo Client.
When creating useQuery calls, follow the provided pattern: import { api } from '@/graphql/api.js'; import { graphql } from '@/graphql/graphql.js'; use useQuery with queryKey, staleTime, and queryFn using api.query.
When performing mutations, follow the provided useMutation pattern: use graphql() for the mutation document, api.mutate for mutationFn, and do not pass variables at declaration.
Files:
packages/dashboard/src/lib/constants.tspackages/dashboard/src/lib/providers/user-settings.tsxpackages/dashboard/src/lib/framework/page/list-page.tsx
packages/dashboard/src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
React component props objects should be typed as Readonly<...>.
Files:
packages/dashboard/src/lib/constants.tspackages/dashboard/src/lib/providers/user-settings.tsxpackages/dashboard/src/lib/framework/page/list-page.tsx
packages/dashboard/src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
packages/dashboard/src/**/*.{tsx,jsx}: Use React for all UI components in the dashboard package.
Prefer re-using components from /src/lib/components, especially Shadcn components from /src/lib/components/ui.
Prefer using the FormFieldWrapper component for forms over raw Shadcn components.
All labels or user-facing messages should use localization via the Trans component or useLingui().
Files:
packages/dashboard/src/lib/providers/user-settings.tsxpackages/dashboard/src/lib/framework/page/list-page.tsx
🧬 Code graph analysis (2)
packages/dashboard/src/lib/providers/user-settings.tsx (2)
packages/dashboard/src/lib/constants.ts (1)
DEFAULT_PER_PAGE(19-19)packages/dashboard/src/lib/utils/pagination.ts (1)
validatePerPageValue(7-12)
packages/dashboard/src/lib/framework/page/list-page.tsx (1)
packages/dashboard/src/lib/utils/pagination.ts (1)
validatePerPageValue(7-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: e2e tests (20.x, postgres)
- GitHub Check: codegen / codegen
- GitHub Check: unit tests (24.x)
- GitHub Check: unit tests (20.x)
- GitHub Check: build (24.x)
- GitHub Check: build (22.x)
- GitHub Check: build (20.x)
- GitHub Check: publish_install (ubuntu-latest, 24.x)
- GitHub Check: publish_install (windows-latest, 22.x)
- GitHub Check: publish_install (ubuntu-latest, 22.x)
- GitHub Check: publish_install (macos-latest, 22.x)
- GitHub Check: publish_install (windows-latest, 24.x)
- GitHub Check: publish_install (ubuntu-latest, 24.x)
- GitHub Check: publish_install (ubuntu-latest, 20.x)
- GitHub Check: publish_install (windows-latest, 24.x)
- GitHub Check: publish_install (windows-latest, 22.x)
- GitHub Check: publish_install (macos-latest, 24.x)
- GitHub Check: publish_install (windows-latest, 20.x)
- GitHub Check: publish_install (macos-latest, 20.x)
- GitHub Check: publish_install (ubuntu-latest, 22.x)
🔇 Additional comments (6)
packages/dashboard/src/lib/constants.ts (1)
16-21: LGTM! Well-defined pagination constants.The constants are appropriately scoped and the
as constassertion onALLOWED_PER_PAGE_VALUESensures type safety by creating a readonly tuple with literal types.packages/dashboard/src/lib/framework/page/list-page.tsx (2)
493-497: Pagination logic is well-validated and safe.The validation approach correctly handles edge cases:
Number.parseIntreturning NaN is safely handled byvalidatePerPageValue- Route parameters take precedence over user settings
- Fallback to user settings ensures a valid value is always used
568-568: Global itemsPerPage persistence aligns with PR objectives.The change from per-table settings to global
setItemsPerPageensures consistent pagination behavior across all list pages, as intended by the PR.packages/dashboard/src/lib/providers/user-settings.tsx (3)
89-94: Validation is correctly applied during localStorage initialization.The
validatePerPageValueensures that even if corrupted or outdated data exists in localStorage, only valid per-page values are loaded into the application state.
157-163: Server settings merge handles validation properly.The validation during server settings merge ensures consistency between client and server state, preventing invalid values from propagating through the system.
227-227: Setter applies validation at the right boundary.Validating in the setter function ensures that no invalid values can be persisted, regardless of the caller's input.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/dashboard/src/lib/framework/page/list-page.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
packages/dashboard/src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
packages/dashboard/src/**/*.{tsx,jsx}: Use React for all UI components in the dashboard package.
Prefer re-using components from /src/lib/components, especially Shadcn components from /src/lib/components/ui.
Prefer using the FormFieldWrapper component for forms over raw Shadcn components.
All labels or user-facing messages should use localization via the Trans component or useLingui().
Files:
packages/dashboard/src/lib/framework/page/list-page.tsx
packages/dashboard/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
packages/dashboard/src/**/*.{ts,tsx}: Use TanStack Query (useQuery or useMutation) for data fetching; do not use Apollo Client.
When creating useQuery calls, follow the provided pattern: import { api } from '@/graphql/api.js'; import { graphql } from '@/graphql/graphql.js'; use useQuery with queryKey, staleTime, and queryFn using api.query.
When performing mutations, follow the provided useMutation pattern: use graphql() for the mutation document, api.mutate for mutationFn, and do not pass variables at declaration.
Files:
packages/dashboard/src/lib/framework/page/list-page.tsx
packages/dashboard/src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
React component props objects should be typed as Readonly<...>.
Files:
packages/dashboard/src/lib/framework/page/list-page.tsx
🧬 Code graph analysis (1)
packages/dashboard/src/lib/framework/page/list-page.tsx (1)
packages/dashboard/src/lib/utils/pagination.ts (1)
validatePerPageValue(7-12)
🔇 Additional comments (2)
packages/dashboard/src/lib/framework/page/list-page.tsx (2)
492-494: LGTM! Past issue resolved.The redundant fallback has been removed, and both branches correctly use
validatePerPageValue. The validation function handles invalid input (including NaN) by returningDEFAULT_PER_PAGE, ensuringitemsPerPageis always a valid number.
567-567: LGTM! Centralized pagination preference.Calling
setItemsPerPage(perPage)for all page changes correctly centralizes the per-page preference at the global level, aligning with the PR's goal to ensure pagination is validated, consistent, and synchronized across navigation and reloads.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/dashboard/src/lib/utils/pagination.ts (1)
18-23: Consider removing redundant NaN check.The function correctly validates page numbers and handles edge cases. However, the
!Number.isNaN(value)check is redundant becauseNumber.isFinite(value)already returnsfalsefor NaN.Apply this diff to simplify:
export function validatePageValue(value: number): number { - if (Number.isFinite(value) && !Number.isNaN(value) && value >= 1 && Number.isInteger(value)) { + if (Number.isFinite(value) && value >= 1 && Number.isInteger(value)) { return value; } return 1; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/dashboard/src/lib/framework/page/list-page.tsx(3 hunks)packages/dashboard/src/lib/utils/pagination.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
packages/dashboard/src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
packages/dashboard/src/**/*.{tsx,jsx}: Use React for all UI components in the dashboard package.
Prefer re-using components from /src/lib/components, especially Shadcn components from /src/lib/components/ui.
Prefer using the FormFieldWrapper component for forms over raw Shadcn components.
All labels or user-facing messages should use localization via the Trans component or useLingui().
Files:
packages/dashboard/src/lib/framework/page/list-page.tsx
packages/dashboard/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
packages/dashboard/src/**/*.{ts,tsx}: Use TanStack Query (useQuery or useMutation) for data fetching; do not use Apollo Client.
When creating useQuery calls, follow the provided pattern: import { api } from '@/graphql/api.js'; import { graphql } from '@/graphql/graphql.js'; use useQuery with queryKey, staleTime, and queryFn using api.query.
When performing mutations, follow the provided useMutation pattern: use graphql() for the mutation document, api.mutate for mutationFn, and do not pass variables at declaration.
Files:
packages/dashboard/src/lib/framework/page/list-page.tsxpackages/dashboard/src/lib/utils/pagination.ts
packages/dashboard/src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
React component props objects should be typed as Readonly<...>.
Files:
packages/dashboard/src/lib/framework/page/list-page.tsxpackages/dashboard/src/lib/utils/pagination.ts
🧬 Code graph analysis (2)
packages/dashboard/src/lib/framework/page/list-page.tsx (1)
packages/dashboard/src/lib/utils/pagination.ts (2)
validatePerPageValue(7-12)validatePageValue(18-23)
packages/dashboard/src/lib/utils/pagination.ts (1)
packages/dashboard/src/lib/constants.ts (2)
ALLOWED_PER_PAGE_VALUES(20-20)DEFAULT_PER_PAGE(19-19)
🔇 Additional comments (7)
packages/dashboard/src/lib/framework/page/list-page.tsx (5)
20-20: LGTM!The import statement correctly brings in the validation utilities needed for pagination handling.
492-494: LGTM!The itemsPerPage calculation correctly validates both the URL parameter and the user settings fallback. The validation handles invalid input (including NaN from
Number.parseInt) by returningDEFAULT_PER_PAGE.
496-496: LGTM!The page calculation correctly validates the URL parameter using
validatePageValue, which handles invalid input (including NaN, negative values, and non-integers) by returning 1.
500-501: LGTM!The pagination object correctly uses the validated
pageanditemsPerPagevalues.
566-569: LGTM!The
onPageChangehandler correctly persists the pagination state to the URL and updates the globalitemsPerPagesetting. This change aligns with the PR objective to centralize pagination handling across the dashboard.packages/dashboard/src/lib/utils/pagination.ts (2)
1-1: LGTM!The import statement correctly brings in the pagination constants needed for validation.
7-12: LGTM!The function correctly validates that the per-page value is in the allowed list, defaulting to
DEFAULT_PER_PAGEfor invalid values (including NaN). The type assertion is necessary for theincludescheck to work with the const array type.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/dashboard/src/app/routes/_authenticated/_collections/components/collection-contents-preview-table.tsx(3 hunks)packages/dashboard/src/app/routes/_authenticated/_collections/components/collection-contents-table.tsx(3 hunks)packages/dashboard/src/app/routes/_authenticated/_customer-groups/components/customer-group-members-table.tsx(3 hunks)packages/dashboard/src/app/routes/_authenticated/_customers/components/customer-order-table.tsx(2 hunks)packages/dashboard/src/app/routes/_authenticated/_facets/components/facet-values-table.tsx(3 hunks)packages/dashboard/src/app/routes/_authenticated/_products/components/product-options-table.tsx(3 hunks)packages/dashboard/src/app/routes/_authenticated/_products/components/product-variants-table.tsx(3 hunks)packages/dashboard/src/app/routes/_authenticated/_zones/components/zone-countries-table.tsx(3 hunks)packages/dashboard/src/lib/components/shared/paginated-list-data-table.tsx(6 hunks)packages/dashboard/src/lib/utils/pagination.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
packages/dashboard/src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
packages/dashboard/src/**/*.{tsx,jsx}: Use React for all UI components in the dashboard package.
Prefer re-using components from /src/lib/components, especially Shadcn components from /src/lib/components/ui.
Prefer using the FormFieldWrapper component for forms over raw Shadcn components.
All labels or user-facing messages should use localization via the Trans component or useLingui().
Files:
packages/dashboard/src/app/routes/_authenticated/_collections/components/collection-contents-table.tsxpackages/dashboard/src/lib/components/shared/paginated-list-data-table.tsxpackages/dashboard/src/app/routes/_authenticated/_products/components/product-options-table.tsxpackages/dashboard/src/app/routes/_authenticated/_customer-groups/components/customer-group-members-table.tsxpackages/dashboard/src/app/routes/_authenticated/_customers/components/customer-order-table.tsxpackages/dashboard/src/app/routes/_authenticated/_products/components/product-variants-table.tsxpackages/dashboard/src/app/routes/_authenticated/_collections/components/collection-contents-preview-table.tsxpackages/dashboard/src/app/routes/_authenticated/_zones/components/zone-countries-table.tsxpackages/dashboard/src/app/routes/_authenticated/_facets/components/facet-values-table.tsx
packages/dashboard/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
packages/dashboard/src/**/*.{ts,tsx}: Use TanStack Query (useQuery or useMutation) for data fetching; do not use Apollo Client.
When creating useQuery calls, follow the provided pattern: import { api } from '@/graphql/api.js'; import { graphql } from '@/graphql/graphql.js'; use useQuery with queryKey, staleTime, and queryFn using api.query.
When performing mutations, follow the provided useMutation pattern: use graphql() for the mutation document, api.mutate for mutationFn, and do not pass variables at declaration.
Files:
packages/dashboard/src/app/routes/_authenticated/_collections/components/collection-contents-table.tsxpackages/dashboard/src/lib/components/shared/paginated-list-data-table.tsxpackages/dashboard/src/app/routes/_authenticated/_products/components/product-options-table.tsxpackages/dashboard/src/app/routes/_authenticated/_customer-groups/components/customer-group-members-table.tsxpackages/dashboard/src/app/routes/_authenticated/_customers/components/customer-order-table.tsxpackages/dashboard/src/app/routes/_authenticated/_products/components/product-variants-table.tsxpackages/dashboard/src/lib/utils/pagination.tspackages/dashboard/src/app/routes/_authenticated/_collections/components/collection-contents-preview-table.tsxpackages/dashboard/src/app/routes/_authenticated/_zones/components/zone-countries-table.tsxpackages/dashboard/src/app/routes/_authenticated/_facets/components/facet-values-table.tsx
packages/dashboard/src/app/routes/**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
Use TanStack Router for all routing and navigation in the dashboard app.
Files:
packages/dashboard/src/app/routes/_authenticated/_collections/components/collection-contents-table.tsxpackages/dashboard/src/app/routes/_authenticated/_products/components/product-options-table.tsxpackages/dashboard/src/app/routes/_authenticated/_customer-groups/components/customer-group-members-table.tsxpackages/dashboard/src/app/routes/_authenticated/_customers/components/customer-order-table.tsxpackages/dashboard/src/app/routes/_authenticated/_products/components/product-variants-table.tsxpackages/dashboard/src/app/routes/_authenticated/_collections/components/collection-contents-preview-table.tsxpackages/dashboard/src/app/routes/_authenticated/_zones/components/zone-countries-table.tsxpackages/dashboard/src/app/routes/_authenticated/_facets/components/facet-values-table.tsx
packages/dashboard/src/app/routes/**
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
Do not create new directories or files in /src/app/routes/ that do not fit the pattern: route files (except those in /components, /hooks, or ending with .graphql.ts).
Files:
packages/dashboard/src/app/routes/_authenticated/_collections/components/collection-contents-table.tsxpackages/dashboard/src/app/routes/_authenticated/_products/components/product-options-table.tsxpackages/dashboard/src/app/routes/_authenticated/_customer-groups/components/customer-group-members-table.tsxpackages/dashboard/src/app/routes/_authenticated/_customers/components/customer-order-table.tsxpackages/dashboard/src/app/routes/_authenticated/_products/components/product-variants-table.tsxpackages/dashboard/src/app/routes/_authenticated/_collections/components/collection-contents-preview-table.tsxpackages/dashboard/src/app/routes/_authenticated/_zones/components/zone-countries-table.tsxpackages/dashboard/src/app/routes/_authenticated/_facets/components/facet-values-table.tsx
packages/dashboard/src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
React component props objects should be typed as Readonly<...>.
Files:
packages/dashboard/src/app/routes/_authenticated/_collections/components/collection-contents-table.tsxpackages/dashboard/src/lib/components/shared/paginated-list-data-table.tsxpackages/dashboard/src/app/routes/_authenticated/_products/components/product-options-table.tsxpackages/dashboard/src/app/routes/_authenticated/_customer-groups/components/customer-group-members-table.tsxpackages/dashboard/src/app/routes/_authenticated/_customers/components/customer-order-table.tsxpackages/dashboard/src/app/routes/_authenticated/_products/components/product-variants-table.tsxpackages/dashboard/src/lib/utils/pagination.tspackages/dashboard/src/app/routes/_authenticated/_collections/components/collection-contents-preview-table.tsxpackages/dashboard/src/app/routes/_authenticated/_zones/components/zone-countries-table.tsxpackages/dashboard/src/app/routes/_authenticated/_facets/components/facet-values-table.tsx
packages/dashboard/src/lib/components/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
Use Shadcn UI and Tailwind CSS for UI components.
Files:
packages/dashboard/src/lib/components/shared/paginated-list-data-table.tsx
🧠 Learnings (1)
📚 Learning: 2025-07-23T07:05:04.344Z
Learnt from: CR
PR: vendure-ecommerce/vendure#0
File: .cursor/rules/dashboard.mdc:0-0
Timestamp: 2025-07-23T07:05:04.344Z
Learning: Applies to packages/dashboard/src/**/*.{ts,tsx} : When creating useQuery calls, follow the provided pattern: import { api } from '@/graphql/api.js'; import { graphql } from '@/graphql/graphql.js'; use useQuery with queryKey, staleTime, and queryFn using api.query.
Applied to files:
packages/dashboard/src/app/routes/_authenticated/_customer-groups/components/customer-group-members-table.tsx
🧬 Code graph analysis (10)
packages/dashboard/src/app/routes/_authenticated/_collections/components/collection-contents-table.tsx (1)
packages/dashboard/src/lib/utils/pagination.ts (1)
validatePerPageValue(7-12)
packages/dashboard/src/lib/components/shared/paginated-list-data-table.tsx (1)
packages/dashboard/src/lib/utils/pagination.ts (2)
validatePageValue(18-23)validatePerPageValue(7-12)
packages/dashboard/src/app/routes/_authenticated/_products/components/product-options-table.tsx (1)
packages/dashboard/src/lib/utils/pagination.ts (1)
validatePerPageValue(7-12)
packages/dashboard/src/app/routes/_authenticated/_customer-groups/components/customer-group-members-table.tsx (1)
packages/dashboard/src/lib/utils/pagination.ts (1)
validatePerPageValue(7-12)
packages/dashboard/src/app/routes/_authenticated/_customers/components/customer-order-table.tsx (1)
packages/dashboard/src/lib/utils/pagination.ts (1)
validatePerPageValue(7-12)
packages/dashboard/src/app/routes/_authenticated/_products/components/product-variants-table.tsx (2)
packages/dashboard/src/lib/hooks/use-local-format.ts (1)
useLocalFormat(27-175)packages/dashboard/src/lib/utils/pagination.ts (1)
validatePerPageValue(7-12)
packages/dashboard/src/lib/utils/pagination.ts (1)
packages/dashboard/src/lib/constants.ts (2)
ALLOWED_PER_PAGE_VALUES(20-20)DEFAULT_PER_PAGE(19-19)
packages/dashboard/src/app/routes/_authenticated/_collections/components/collection-contents-preview-table.tsx (1)
packages/dashboard/src/lib/utils/pagination.ts (1)
validatePerPageValue(7-12)
packages/dashboard/src/app/routes/_authenticated/_zones/components/zone-countries-table.tsx (1)
packages/dashboard/src/lib/utils/pagination.ts (1)
validatePerPageValue(7-12)
packages/dashboard/src/app/routes/_authenticated/_facets/components/facet-values-table.tsx (1)
packages/dashboard/src/lib/utils/pagination.ts (1)
validatePerPageValue(7-12)
🪛 Biome (2.1.2)
packages/dashboard/src/app/routes/_authenticated/_facets/components/facet-values-table.tsx
[error] 8-8: Shouldn't redeclare 'useUserSettings'. Consider to delete it or rename it.
'useUserSettings' is defined here:
(lint/suspicious/noRedeclare)
[error] 43-43: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: e2e tests (20.x, mysql)
- GitHub Check: e2e tests (20.x, mariadb)
- GitHub Check: e2e tests (20.x, postgres)
- GitHub Check: build (20.x)
- GitHub Check: unit tests (22.x)
- GitHub Check: unit tests (20.x)
- GitHub Check: build (22.x)
- GitHub Check: publish_install (ubuntu-latest, 24.x)
- GitHub Check: publish_install (ubuntu-latest, 22.x)
- GitHub Check: publish_install (windows-latest, 24.x)
- GitHub Check: publish_install (windows-latest, 22.x)
- GitHub Check: publish_install (macos-latest, 24.x)
- GitHub Check: publish_install (macos-latest, 20.x)
- GitHub Check: publish_install (windows-latest, 24.x)
- GitHub Check: publish_install (ubuntu-latest, 22.x)
- GitHub Check: publish_install (ubuntu-latest, 20.x)
- GitHub Check: publish_install (macos-latest, 22.x)
- GitHub Check: publish_install (ubuntu-latest, 24.x)
- GitHub Check: publish_install (windows-latest, 20.x)
- GitHub Check: publish_install (windows-latest, 22.x)
🔇 Additional comments (16)
packages/dashboard/src/app/routes/_authenticated/_collections/components/collection-contents-preview-table.tsx (1)
6-7: LGTM - Pagination persistence correctly implemented.The changes properly integrate validated per-page settings from the user context. The pattern of initializing with
validatePerPageValue(settings.itemsPerPage)and persisting viasetItemsPerPage(perPage)aligns with the PR's goal of centralizing pagination validation.Also applies to: 42-42, 45-45, 112-112
packages/dashboard/src/app/routes/_authenticated/_collections/components/collection-contents-table.tsx (1)
5-6: LGTM - Consistent pagination handling.The implementation correctly follows the established pattern for validated per-page persistence.
Also applies to: 34-34, 37-37, 70-70
packages/dashboard/src/app/routes/_authenticated/_products/components/product-options-table.tsx (1)
5-6: LGTM - Per-page persistence correctly integrated.The validated pagination initialization and persistence align with the PR's centralized approach.
Also applies to: 38-38, 41-41, 61-61
packages/dashboard/src/app/routes/_authenticated/_customers/components/customer-order-table.tsx (1)
9-10: LGTM - Pagination validation properly applied.The changes integrate validated user settings for per-page values as intended.
Also applies to: 17-17, 19-19, 84-84
packages/dashboard/src/app/routes/_authenticated/_zones/components/zone-countries-table.tsx (1)
12-13: LGTM - Validated pagination integrated.The implementation correctly follows the per-page persistence pattern.
Also applies to: 33-33, 35-35, 64-64
packages/dashboard/src/app/routes/_authenticated/_customer-groups/components/customer-group-members-table.tsx (1)
10-11: LGTM - Per-page handling properly implemented.The validated pagination initialization and persistence are correctly applied.
Also applies to: 47-47, 50-50, 83-83
packages/dashboard/src/app/routes/_authenticated/_products/components/product-variants-table.tsx (1)
18-19: LGTM - Pagination persistence correctly added.The changes properly integrate validated per-page settings and persistence.
Also applies to: 32-32, 35-35, 108-108
packages/dashboard/src/app/routes/_authenticated/_facets/components/facet-values-table.tsx (1)
43-43: LGTM after fixing the duplicate hook call.Once the duplicate
useUserSettings()call is resolved, the pagination validation and persistence logic will work correctly.Also applies to: 73-73
packages/dashboard/src/lib/utils/pagination.ts (2)
7-12: LGTM! Clear validation logic.The function correctly validates that the per-page value is in the allowed list and falls back to the default when invalid. The type assertion for TypeScript narrowing is appropriate.
18-23: LGTM! Robust page validation.The validation correctly checks for finite numbers, positive values, and integers. Returning 1 as the default is the right choice for invalid page numbers.
packages/dashboard/src/lib/components/shared/paginated-list-data-table.tsx (6)
19-19: LGTM! Correct import.The validation utilities are properly imported for use in pagination validation.
402-411: LGTM! Clean filter construction.The filter construction logic correctly handles both single values and arrays, mapping them to the appropriate filter format. The
_andwrapper ensures multiple filters are combined correctly.
450-451: LGTM! Consistent use of validated values in query key.Using validated pagination values in the query key ensures cache consistency and prevents issues with invalid values affecting query caching.
464-465: LGTM! Validated values used correctly in query variables.The validated pagination values are properly used for
take(with a sensible 100 cap) andskipcalculation. This ensures the backend receives valid pagination parameters.
490-491: LGTM! DataTable receives validated pagination state.Passing validated values to the DataTable ensures the UI displays the correct pagination state consistent with the query and prevents rendering issues from invalid values.
383-385: Verify parent pagination state synchronization
The table’svalidatePageValueandvalidatePerPageValueguard its display, but parent state may still hold invalid inputs. Confirm that:
ListPageand other parents sanitize incomingpage/itemsPerPageprops and call their state setters with the validated values- Their
onPageChangehandlers propagate the correctedpageanditemsPerPageback to parent state- URL query parameters are updated to match the sanitized pagination values
packages/dashboard/src/app/routes/_authenticated/_facets/components/facet-values-table.tsx
Show resolved
Hide resolved
packages/dashboard/src/app/routes/_authenticated/_facets/components/facet-values-table.tsx
Outdated
Show resolved
Hide resolved
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/dashboard/src/app/routes/_authenticated/_facets/components/facet-values-table.tsx (1)
67-73: Consider defensive validation ofperPagebefore persistence.While the
perPagevalue is likely already validated by the parent component and/or thesetItemsPerPagefunction, wrapping it withvalidatePerPageValuewould add an extra layer of robustness consistent with the PR's validation goals.onPageChange={(table, page, perPage) => { if (pageId) { setPageSize(perPage); setPage(page); - setItemsPerPage(perPage); + setItemsPerPage(validatePerPageValue(perPage)); } }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/dashboard/src/app/routes/_authenticated/_facets/components/facet-values-table.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
packages/dashboard/src/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
packages/dashboard/src/**/*.{tsx,jsx}: Use React for all UI components in the dashboard package.
Prefer re-using components from /src/lib/components, especially Shadcn components from /src/lib/components/ui.
Prefer using the FormFieldWrapper component for forms over raw Shadcn components.
All labels or user-facing messages should use localization via the Trans component or useLingui().
Files:
packages/dashboard/src/app/routes/_authenticated/_facets/components/facet-values-table.tsx
packages/dashboard/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
packages/dashboard/src/**/*.{ts,tsx}: Use TanStack Query (useQuery or useMutation) for data fetching; do not use Apollo Client.
When creating useQuery calls, follow the provided pattern: import { api } from '@/graphql/api.js'; import { graphql } from '@/graphql/graphql.js'; use useQuery with queryKey, staleTime, and queryFn using api.query.
When performing mutations, follow the provided useMutation pattern: use graphql() for the mutation document, api.mutate for mutationFn, and do not pass variables at declaration.
Files:
packages/dashboard/src/app/routes/_authenticated/_facets/components/facet-values-table.tsx
packages/dashboard/src/app/routes/**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
Use TanStack Router for all routing and navigation in the dashboard app.
Files:
packages/dashboard/src/app/routes/_authenticated/_facets/components/facet-values-table.tsx
packages/dashboard/src/app/routes/**
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
Do not create new directories or files in /src/app/routes/ that do not fit the pattern: route files (except those in /components, /hooks, or ending with .graphql.ts).
Files:
packages/dashboard/src/app/routes/_authenticated/_facets/components/facet-values-table.tsx
packages/dashboard/src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/dashboard.mdc)
React component props objects should be typed as Readonly<...>.
Files:
packages/dashboard/src/app/routes/_authenticated/_facets/components/facet-values-table.tsx
🧬 Code graph analysis (1)
packages/dashboard/src/app/routes/_authenticated/_facets/components/facet-values-table.tsx (1)
packages/dashboard/src/lib/utils/pagination.ts (1)
validatePerPageValue(7-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)
- GitHub Check: e2e tests (22.x, mariadb)
- GitHub Check: e2e tests (20.x, sqljs)
- GitHub Check: e2e tests (20.x, mariadb)
- GitHub Check: unit tests (24.x)
- GitHub Check: build (20.x)
- GitHub Check: unit tests (22.x)
- GitHub Check: build (22.x)
- GitHub Check: publish_install (macos-latest, 22.x)
- GitHub Check: publish_install (ubuntu-latest, 22.x)
- GitHub Check: publish_install (windows-latest, 24.x)
- GitHub Check: publish_install (macos-latest, 20.x)
- GitHub Check: publish_install (ubuntu-latest, 24.x)
- GitHub Check: publish_install (windows-latest, 20.x)
- GitHub Check: publish_install (macos-latest, 24.x)
- GitHub Check: publish_install (ubuntu-latest, 20.x)
- GitHub Check: publish_install (windows-latest, 22.x)
- GitHub Check: publish_install (ubuntu-latest, 22.x)
- GitHub Check: publish_install (ubuntu-latest, 24.x)
- GitHub Check: publish_install (windows-latest, 22.x)
- GitHub Check: publish_install (windows-latest, 24.x)
🔇 Additional comments (2)
packages/dashboard/src/app/routes/_authenticated/_facets/components/facet-values-table.tsx (2)
6-8: Past duplicate import issue has been resolved.The duplicate import of
useUserSettingsmentioned in previous reviews has been successfully fixed.
39-42: Past duplicateuseUserSettings()call issue has been resolved.The code now correctly uses a single
useUserSettings()call and properly initializespageSizewith the validated per-page value. No variable shadowing or redeclaration issues remain.
|
Hi, Do you want to bring this PR up to date with current master? One thing I notice is that it looks like you move the perPage from the table-specific setting into a global value. Is that right? What's the idea behind that? |
|
I’ll look into that. The perPage global setting was more my own expected behavior. When I change it on one place to 25 I expected it to be 25 everywhere. I don’t usually spend much time in dashboards like this, so if the per-table setup is the intended behavior, that makes sense. Let me know. |
2 participants
Description
The rows per page on the overview page for example Products you can change it with 10,20,30,40 and 50.
When you change that the URI is changed. #3845
This pull request improves the handling and validation of pagination settings across the dashboard, ensuring a consistent and user-friendly experience. The main focus is on centralizing pagination logic, validating user preferences, and syncing page size settings throughout the application.
Pagination logic and validation improvements:
validatePerPageValueinpagination.tsto ensure that only allowed per-page values are used, defaulting toDEFAULT_PER_PAGEif an invalid value is provided.DataTablePaginationandDataTablecomponents to useALLOWED_PER_PAGE_VALUESandDEFAULT_PER_PAGEfrom constants, and to validate and sync pagination state based on user settings and URL parameters.User settings enhancements:
UserSettingscontext and provider to include a globalitemsPerPagepreference, with validation and proper initialization from both local storage and server settings.Page and table integration:
ListPagecomponent to use the new validateditemsPerPagesetting, ensuring consistency between URL parameters, user preferences, and table state. The page change handler now updates the globalitemsPerPageinstead of per-table settings.These changes together ensure that pagination is robust, user preferences are respected and validated, and the UI remains consistent across navigation and reloads.
Breaking changes
Not that I know of
Summary by CodeRabbit
New Features
Bug Fixes
Style