refactor: restore traditional pagination

[labmecanicatec]

• Re-enable pageable->GetPageSize() call
• Replace null parameters with actual page number and size
• Use page->GetPageNumber() and page->GetPageSize() for pagination
• Improves performance by limiting results per request
• Restore CreatePagination and CreatePageLink methods
• Enable page navigation with prev/next buttons
• Display results range and total count
• Add data attributes for pagination handling
• Replace view all behavior with paginated browsing

Close: #1612
Assisted-by: Claude:claude-haiku-4.5

[Copilot]

Pull request overview

This PR restores traditional, server-driven pagination across multiple admin and credits log screens by re-enabling page size usage from IPageable/PageInfo and adding a new Smarty {pagination} helper to render page navigation links.

Changes:

• Re-enable and consistently pass GetPageNumber() / GetPageSize() into list-loading services/repositories to avoid accidental “pageSize=1” limiting when null is coerced.
• Add a Smarty {pagination pageInfo=$PageInfo} function and wire it into several templates to display page navigation and result counts.
• Adjust frontend pagination click handling to target the new pagination link markup.

Reviewed changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
Web/scripts/ajax-helpers.js	Updates AJAX pagination click selector for the new pagination link markup.
tpl/Credits/transaction_log.tpl	Adds {pagination} to the user transaction log partial and normalizes {datatable} call syntax.
tpl/Credits/credit_log.tpl	Adds {pagination} to the user credit log partial and normalizes {datatable} call syntax.
tpl/Admin/Users/manage_users.tpl	Adds {pagination} below the users table.
tpl/Admin/Users/credit_log.tpl	Adds {pagination} and minor include formatting cleanup.
tpl/Admin/Schedules/view_schedules.tpl	Adds {pagination} below the schedules table.
tpl/Admin/Schedules/manage_schedules.tpl	Adds {pagination} and swaps datatablefilter to datatable for the schedules table initialization.
tpl/Admin/Resources/manage_resources.tpl	Adds {pagination} below the resources table.
tpl/Admin/Reservations/manage_reservations.tpl	Adds {pagination} below the reservations table.
tpl/Admin/Payments/transaction_log.tpl	Adds {pagination} to the admin payments transaction log partial and normalizes {datatable} call syntax.
tpl/Admin/Groups/manage_groups.tpl	Adds {pagination} below the groups table.
tpl/Admin/Blackouts/manage_blackouts.tpl	Adds {pagination} below the blackouts table.
Presenters/ViewSchedulesPresenter.php	Passes page number + page size to schedules list retrieval.
Presenters/Admin/ManageUsersPresenter.php	Passes page size to user repository list retrieval; minor formatting cleanups.
Presenters/Admin/ManageSchedulesPresenter.php	Passes page size to schedules list retrieval.
Presenters/Admin/ManageResourcesPresenter.php	Passes page size to resources repository list retrieval.
Presenters/Admin/ManageReservationsPresenter.php	Passes page size to reservations filtered load; minor formatting cleanups.
Presenters/Admin/ManageGroupsPresenter.php	Passes page size to groups repository list retrieval.
Presenters/Admin/ManageBlackoutsPresenter.php	Passes page size to blackouts filtered load.
Pages/Admin/ManageUsersPage.php	Restores GetPageSize() by delegating to the wrapped pageable implementation.
Pages/Admin/ManageSchedulesPage.php	Restores GetPageSize() (with existing cap logic) to support server pagination again.
Pages/Admin/ManageResourcesPage.php	Restores GetPageSize() (with existing cap logic) to support server pagination again.
Pages/Admin/ManageReservationsPage.php	Restores GetPageSize() by delegating to the wrapped pageable implementation.
Pages/Admin/ManageGroupsPage.php	Restores GetPageSize() by delegating to the wrapped pageable implementation.
Pages/Admin/ManageBlackoutsPage.php	Restores GetPageSize() by delegating to the wrapped pageable implementation.
lib/Common/SmartyPage.php	Registers {pagination} and implements CreatePagination/CreatePageLink; adjusts DataTables initialization.

Suppressed comments (2)

Web/scripts/ajax-helpers.js:277

• ajaxPagination reads data attributes from $(e.target). If the click originates from a nested element inside the (e.g., an icon/span), data-page/data-page-size will be missing and pagination will break. Use $(this) and/or select anchors by data attributes so all pagination links are handled consistently.

  element.find('a.page-link').on('click', function (e) {
    e.preventDefault();
    var a = $(e.target);
    callback(a.data('page'), a.data('page-size'));
  });

lib/Common/SmartyPage.php:749

• CreateDataTableFilter's language configuration maps the Info strings incorrectly: info is currently set to the Filter text, and infoEmpty is set to the Info text. This produces incorrect UI copy in the table footer.

           const tableFilter =  new DataTable("#' . $tableId . '", {
                "dom": \'<"d-flex justify-content-between my-1"fl><t>t<"d-flex justify-content-center"i><"d-flex justify-content-center"p><"clear">\',
                "pageLength": ' . $defaultPageSize . ',
                "lengthMenu": ' . $lengthMenu . ',
                language: {

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 8 comments.

Suppressed comments (1)

lib/Common/SmartyPage.php:791

• Use the existing PageInfo::All constant instead of the magic -1; this keeps the link coupled to the paging contract if its sentinel changes.

                $sb->Append($this->CreatePageLink(['class' => 'link-primary', 'page' => 1, 'size' => -1, 'text' => $viewAllText], $smarty));

[Copilot]

Copilot review overview

🟡 Changes recommended

CSV exports are truncated, and pagination correctness and accessibility issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 3

Open (3)

• CSV export is truncated to the current page · New
• Resource CSV export is limited to the current page · New
• User CSV export is limited to the current page · New

Resolved since last review (8)

• This script is injected repeatedly for AJAX credit/transaction-log fragments, and the user credits…
• CreateDataTable is shared by many templates, but this now disables client-side paging…
• Replacing datatablefilter with datatable removes this page’s only schedule filter because…
• For the newly server-paginated tables, DataTables still defaults to client-side ordering. Clicking…
• The regression fix is not protected by a test that verifies the repository receives the page size.…
• This selector excludes the generated “View All” anchor because CreatePagination() gives it only…
• The restored page-size cap uses an unexplained 10 and duplicates the same rule in…
• The restored page-size cap uses an unexplained 10 and duplicates the same rule in…

[JohnVillalovos]

🤖 This review was written by Claude Code, Anthropic's AI coding assistant. It covers the two commits in this PR (refactor: implement pagination and refactor: add pagination UI components), reviewed on a local checkout.

Server-side pagination is a worthwhile goal for large installs, and the new a[data-page][data-page-size] selector in ajaxPagination fixes AJAX paging on the credit and transaction logs (the old a.page selector never matched anything). Tests, php-cs-fixer and PHPStan all pass locally on these changes.

The review did find several regressions that should be fixed before merging.

Blocking

1. Every {datatable} table loses pagination, not only the server-paged ones. CreateDataTable now sets "paging": false for all ~21 templates that use it. About 10 of them have no server paging to replace it (announcements, accessories, quotas, attributes, payments, reservation colors, resource types, saved reports, view_resources.tpl, search results), so they now show every row with no controls. One option: give {datatable} a parameter such as serverPaged=true, and turn DataTables paging off only for those tables.

2. view_schedules.tpl is paged twice. It now has server pagination (10 per page) but still uses {datatablefilter}, which has its own DataTables paging and filter box. The filter only searches the rows on screen.

3. manage_schedules.tpl loses its filter box. Switching {datatablefilter} to {datatable} removes the search box, because {datatable} sets "searching": false.

4. Bulk update and bulk delete on Manage Resources only see the current page. resource.js builds the bulk list from the JS resources array, which the template fills from $Resources. That is now only the current page, so bulk actions silently skip everything else.

5. Export and sort only see the current page. On users, reservations, groups, resources and blackouts, the Copy/Excel/PDF/Print buttons now export one page instead of the full filtered list. Header sorting is still on in DataTables, so clicking a column sorts only the visible rows, while the server-side sort (GetSortField) has no UI. Either turn off client-side ordering or wire the headers to server-side sorting.

Other issues

• ReplaceQueryString drops query parameters. This bug was already in the code, but every pagination link now depends on this function. The replacement regex /(\?|&)(page=.*)/ is greedy, so it replaces page= and everything after it. For example, ?page=2&pageSize=50&sort=name becomes ?page=3, which drops sort and any filter parameters that come after page. Also, the "is the key present?" check uses str_contains($url, $key), so a URL that contains pageSize= but no page= passes the check, the regex then finds nothing to replace, and page is never added. Parsing with parse_url() + parse_str(), updating the array and rebuilding with http_build_query() would fix both problems. It would also make the separators consistent: the append branch currently uses &, while GetUrl() produces a raw &.
• "View All" undercuts the performance goal. It links with pageSize=-1, which the default.page.size config description says "is not supported for performance reasons".
• pageSize from the URL has no limits. Very large values (e.g. pageSize=999999) load everything, and negative values other than -1 go straight into LimitQuery. It should be clamped.
• Every page gets its own link. With thousands of rows this makes hundreds of links. A window of pages around the current one would scale better.
• Accessibility went backwards. The original <nav aria-label="…"> wrapper was dropped. The active item should also get aria-current="page", and « and » should be disabled on the first and last page.
• href is written without escaping. CreatePageLink puts the URL into the attribute as-is. It's mostly safe today because http_build_query encodes the query, but htmlspecialchars() would be the safer choice.
• Unrelated changes are mixed in. These include $(…).DataTable() → new DataTable(), the initComplete simplification, the table → tableFilter rename, the array-bracket reformatting in ManageReservationsPresenter and ManageUsersPresenter, and some template cleanup. They look harmless, but they belong in a separate commit or should at least be mentioned in the message.
• No tests. Nothing covers CreatePagination, ReplaceQueryString, or the presenters now passing the page size through.

Commit messages

• Type. Both commits change behavior, so they should be feat (or fix, with an issue reference) rather than refactor. A scope such as feat(admin): … would help.
• Typo. There's a double space after refactor: in the second subject.
• Rationale. 4ff40461d removed this pagination in favor of DataTables-only client-side paging. The message should explain why this PR goes back, and link an issue if there is one.
• Assisted-by format. The trailer should follow the repo's convention: Assisted-by: Claude:claude-haiku-4-5.
• Missing details. The message doesn't mention the manage_schedules change, the DataTables API change, or that ViewSchedulesPresenter goes from showing everything to showing pages.

[JohnVillalovos]

This review was generated by Codex.

Recommendation: request changes. The switch to server pagination leaves several existing table features operating on only the loaded page.

In 6a94f631:

• Column sorting sorts only the current page, rather than the full matching dataset.
• Excel/PDF exports, copy, and print include only the loaded page.
• Schedule search searches only the fetched subset, so schedules on other pages cannot be found.
• Taken independently, this commit limits results without providing navigation to subsequent server pages.

In 7fdb3017:

• The new pagination links expose a pre-existing defect in ReplaceQueryString(): ?pageSize=20 prevents adding a page parameter, and replacing an existing page parameter drops subsequent filters.
• Disabling pagination globally in CreateDataTable() removes pagination from unrelated tables without replacement server controls, including announcements, accessories, quotas, and saved reports.

Before merging, preserve query parameters when generating links, connect sorting/search/export behavior to the full matching dataset, and disable client pagination only for tables explicitly converted to server pagination.

The existing unit suite completes without failures, but neither commit adds regression tests covering these interactions. Browser and database integration behavior was not exercised during this review.

[labmecanicatec]

Copilot review overview

🟡 Changes recommended

CSV exports are truncated, and pagination correctness and accessibility issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced Findings: 3

Open (3)

• CSV export is truncated to the current page · New
• Resource CSV export is limited to the current page · New
• User CSV export is limited to the current page · New

Resolved since last review (8)

The truncation of the users, resources, and reservations tables when exporting to CSV can be considered a "bug" present in version 2.8—one that was "resolved" by removing traditional pagination. The expectation was that the user would apply the necessary filters, click "Show all," and then export to CSV.