From 1ba2dd2775cf2ee73aa419d690307fc9e95517d1 Mon Sep 17 00:00:00 2001 From: Alexander Brandon Coles Date: Mon, 25 May 2026 22:49:56 +0200 Subject: [PATCH] [#75314] Replace deprecated Pagy max_pages option Pagy 43.4.3 deprecated `:max_pages` and recommends capping records before pagination instead. Caps the combined array in `base_journals`. https://community.openproject.org/wp/75314 --- .../work_packages/activities_tab/paginator.rb | 10 ++++- .../activities_tab/paginator_spec.rb | 41 +++++++++++++++++++ 2 files changed, 49 insertions(+), 2 deletions(-) diff --git a/app/services/work_packages/activities_tab/paginator.rb b/app/services/work_packages/activities_tab/paginator.rb index 052a2256a08..7efccf5cb4c 100644 --- a/app/services/work_packages/activities_tab/paginator.rb +++ b/app/services/work_packages/activities_tab/paginator.rb @@ -52,6 +52,8 @@ class WorkPackages::ActivitiesTab::Paginator include Pagy::Method include WorkPackages::ActivitiesTab::JournalSortingInquirable + MAX_PAGES = 100 + def self.paginate(work_package, params = {}) new(work_package, params).call end @@ -72,7 +74,7 @@ class WorkPackages::ActivitiesTab::Paginator @filter = :all # Ignore filter when jumping to specific journal pagy_array_for_target_journal(anchor_type, target_record_id) else - pagy(:offset, base_journals, **pagy_options) + pagy(:offset, capped_journals, **pagy_options) end # For UI display: if user wants "oldest first" UI, reverse the array @@ -87,7 +89,6 @@ class WorkPackages::ActivitiesTab::Paginator { page: params[:page] || 1, limit: params[:limit] || Pagy::DEFAULT[:limit], - max_pages: 100, request: { params: } }.compact end @@ -127,6 +128,11 @@ class WorkPackages::ActivitiesTab::Paginator combine_and_sort_records(fetch_journals, fetch_revisions) end + def capped_journals + max_records = (params[:limit] || Pagy::DEFAULT[:limit]) * MAX_PAGES + base_journals.first(max_records) + end + def fetch_journals API::V3::Activities::ActivityEagerLoadingWrapper.wrap(fetch_ar_journals) end diff --git a/spec/services/work_packages/activities_tab/paginator_spec.rb b/spec/services/work_packages/activities_tab/paginator_spec.rb index 02de1178c8f..e6323340eb6 100644 --- a/spec/services/work_packages/activities_tab/paginator_spec.rb +++ b/spec/services/work_packages/activities_tab/paginator_spec.rb @@ -206,6 +206,47 @@ RSpec.describe WorkPackages::ActivitiesTab::Paginator, with_settings: { journal_ end end + context "when record count exceeds MAX_PAGES cap" do + let(:test_limit) { 2 } + + before do + stub_const("#{described_class}::MAX_PAGES", 3) + 8.times do |i| + create(:work_package_journal, user:, notes: "Comment #{i + 1}", journable: work_package, version: i + 2) + end + params[:limit] = test_limit + end + + it "caps total records to limit * MAX_PAGES" do + pagy, _records = paginator.call + + expect(pagy.count).to eq(test_limit * 3) + expect(pagy.pages).to eq(3) + end + + it "keeps the newest records when truncating" do + _pagy, records = paginator.call + + notes = records.map(&:notes) + expect(notes).to include("Comment 8") + expect(notes).not_to include("Comment 1") + end + + it "still resolves an anchor to a journal beyond the cap" do + oldest_journal = work_package.journals.order(:version).first + params[:anchor] = "comment-#{oldest_journal.id}" + pagy, records = paginator.call + + expect(records.map(&:id)).to include(oldest_journal.id) + + aggregate_failures "breaks out of capped limit" do + expect(pagy.count).to eq(work_package.journals.count) + expect(pagy.pages).to eq(5) + expect(pagy.page).to eq(5), "expected oldest journal on last page (page 5), got page #{pagy.page}" + end + end + end + context "with internal comments filtering" do let!(:internal_journal) do create(:work_package_journal,