Skip to content

Conversation

@martinjagodic
Copy link
Member

Add opt-in pagination to collections. Works with filtering and sorting, turns off when grouping. Basic tests added.

@github-actions github-actions bot added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Nov 5, 2025
@martinjagodic martinjagodic requested a review from Copilot November 5, 2025 11:32
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request implements pagination functionality for the Decap CMS, allowing collections to display entries in pages rather than loading all entries at once. The implementation supports both client-side and server-side pagination.

Key Changes:

  • Added pagination configuration options at both global and collection levels (enabled flag and per_page setting)
  • Implemented pagination state management in Redux with page size, current page, and total count tracking
  • Modified backend implementations to support optional pagination parameters
  • Created pagination UI component with navigation controls
  • Added client-side pagination support for filtered/sorted entries

Reviewed Changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
packages/decap-server/src/middlewares/types.ts Added optional pagination options to EntriesByFolderParams
packages/decap-server/src/middlewares/localGit/index.ts Implemented server-side pagination with cursor metadata
packages/decap-server/src/middlewares/localFs/index.ts Implemented server-side pagination with cursor metadata
packages/decap-server/src/middlewares/joi/index.ts Added Joi validation schema for pagination options
packages/decap-cms-locales/src/en/index.js Added English translations for pagination UI
packages/decap-cms-lib-util/src/implementation.ts Updated Implementation interface to include pagination options
packages/decap-cms-core/src/types/redux.ts Added pagination-related types and state definitions
packages/decap-cms-core/src/reducers/entries.ts Implemented pagination state management and selectors
packages/decap-cms-core/src/reducers/tests/entries.spec.js Added comprehensive pagination test cases
packages/decap-cms-core/src/lib/pagination.ts Created utility functions for pagination configuration
packages/decap-cms-core/src/lib/immutableHelpers.ts Added helpers for working with Immutable.js objects
packages/decap-cms-core/src/lib/entryHelpers.ts Added helpers for filtering, sorting, and grouping entries
packages/decap-cms-core/src/components/Collection/Entries/Pagination.js Created pagination UI component
packages/decap-cms-core/src/components/Collection/Entries/* Integrated pagination into entry listing components
packages/decap-cms-core/src/backend.ts Added pagination support to backend entry loading
packages/decap-cms-core/src/actions/entries.ts Implemented pagination actions and updated filter/sort logic
packages/decap-cms-backend-test/src/implementation.ts Updated test backend to support pagination
packages/decap-cms-backend-proxy/src/implementation.ts Updated proxy backend to pass pagination options
packages/decap-cms-backend-github/src/implementation.tsx Implemented pagination for GitHub backend
packages/decap-cms-backend-git-gateway/src/implementation.ts Updated git-gateway to pass pagination options
dev-test/config.yml Added example pagination configuration

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +270 to +273
map.setIn(
['pagination', collection],
existingPagination.merge({ currentPage, totalCount, pageSize }),
);
Copy link

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The map.setIn operation on line 270 does not actually mutate the map. It returns a new map that is not assigned back. Within a withMutations block, you must reassign the result or use a single chained operation. This will cause the pagination state to not be updated properly. Change to: const updated = existingPagination.merge({ currentPage, totalCount, pageSize }); map.setIn(['pagination', collection], updated);

Suggested change
map.setIn(
['pagination', collection],
existingPagination.merge({ currentPage, totalCount, pageSize }),
);
const updated = existingPagination.merge({ currentPage, totalCount, pageSize });
map.setIn(['pagination', collection], updated);

Copilot uses AI. Check for mistakes.
index: number,
depth: number,
pageSize = DEFAULT_PAGE_SIZE,
) {
Copy link

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Division by zero is possible if pageSize is 0. Although the default is 10, the function accepts a pageSize parameter that could be 0 or negative. Add validation: if (pageSize <= 0) throw new Error('pageSize must be positive'); at the start of the getCursor function.

Suggested change
) {
) {
if (pageSize <= 0) {
throw new Error('pageSize must be positive');
}

Copilot uses AI. Check for mistakes.

// If pagination is enabled, return only the requested page
// Otherwise, return all entries (for backward compatibility)
const ret = usePagination ? take(entries.slice((page - 1) * pageSize), pageSize) : entries;
Copy link

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic is incorrect. entries.slice((page - 1) * pageSize) already slices to the start of the desired page. Calling take(..., pageSize) on the result will work, but it's redundant. The correct implementation should be: entries.slice((page - 1) * pageSize, page * pageSize) to get exactly one page of results.

Suggested change
const ret = usePagination ? take(entries.slice((page - 1) * pageSize), pageSize) : entries;
const ret = usePagination ? entries.slice((page - 1) * pageSize, page * pageSize) : entries;

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +7
/**
* Type guard to check if an object is an Immutable.js Map
*/
export function isImmutableMap(obj: unknown): boolean {
return (
typeof obj === 'object' && obj !== null && typeof (obj as { get?: unknown }).get === 'function'
);
Copy link

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is too weak and will incorrectly identify any object with a get method as an Immutable Map (e.g., a plain JavaScript Map or a custom object). Use a more specific check like: return obj !== null && typeof obj === 'object' && 'toJS' in obj && typeof (obj as any).toJS === 'function' or check for Immutable's type using Map.isMap(obj) from the 'immutable' package.

Suggested change
/**
* Type guard to check if an object is an Immutable.js Map
*/
export function isImmutableMap(obj: unknown): boolean {
return (
typeof obj === 'object' && obj !== null && typeof (obj as { get?: unknown }).get === 'function'
);
import { Map } from 'immutable';
/**
* Type guard to check if an object is an Immutable.js Map
*/
export function isImmutableMap(obj: unknown): boolean {
return Map.isMap(obj);

Copilot uses AI. Check for mistakes.
Comment on lines +45 to +46
const startEntry = (currentPage - 1) * pageSize + 1;
const endEntry = Math.min(currentPage * pageSize, totalCount);
Copy link

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If totalCount is 0, startEntry will be 1, which is incorrect. The displayed text will say 'Showing 1-0 of 0'. Add a check: const startEntry = totalCount === 0 ? 0 : (currentPage - 1) * pageSize + 1;

Suggested change
const startEntry = (currentPage - 1) * pageSize + 1;
const endEntry = Math.min(currentPage * pageSize, totalCount);
const startEntry = totalCount === 0 ? 0 : (currentPage - 1) * pageSize + 1;
const endEntry = totalCount === 0 ? 0 : Math.min(currentPage * pageSize, totalCount);

Copilot uses AI. Check for mistakes.
return (obj as { get: (k: string) => T }).get(key);
}

if (typeof obj === 'object' && obj !== null) {
Copy link

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable 'obj' is of type date, object or regular expression, but it is compared to an expression of type null.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: feature code contributing to the implementation of a feature and/or user facing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants