Code Maintenance/68063: Move work package activity tab from in-memory to database-level pagination

app/services/work_packages/activities_tab/paginator.rb

@@ -28,17 +28,26 @@
 # See COPYRIGHT and LICENSE files for more details.
 #++
 
-# Paginates work package activities (journals and changesets) with support for filtering and anchor navigation.
+# Paginates work package activities (journals and changesets) with support for
+# filtering and anchor navigation.
 #
 # Filter modes:
 # - :all - Shows all activities (default)
 # - :only_comments - Shows only journals with notes
 # - :only_changes - Shows only journals with detected changes using SQL heuristics
 #
-# Anchor format (filter is reset to :all when using anchors):
+# Anchor format:
 # - "comment-{journal_id}" - Navigate to specific journal by ID
 # - "activity-{sequence_version}" - Navigate to journal by sequence version
 #
+# Anchored navigation bypasses the active filter so a deep link to a record
+# that wouldn't match the filter still resolves.
+#
+# Internally, the activities feed is materialised as a single UNION ALL
+# relation of journals and changesets (see {ActivitiesQuery}) and paginated
+# by pagy at the database level. Only the page slice is hydrated and wrapped
+# for eager-loading, keeping per-request cost independent of total history size.
+#
 # @param work_package [WorkPackage] The work package to paginate activities for
 # @param params [Hash] Pagination and filtering parameters
 #
@@ -52,8 +61,6 @@ 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
@@ -67,124 +74,139 @@ def initialize(work_package, params = {})
   end
 
   def call
-    anchor_type, target_record_id = extract_target_record_id
+    anchor_type, target_record_id = parse_anchor
 
-    pagy, records =
+    pagy_obj, page_relation =
       if anchor_type && target_record_id
-        @filter = :all # Ignore filter when jumping to specific journal
-        pagy_array_for_target_journal(anchor_type, target_record_id)
+        pagy_at_anchor(anchor_type, target_record_id)
       else
-        pagy(:offset, capped_journals, **pagy_options)
+        pagy(:offset, activities_scope, **pagy_options)
       end
 
-    # For UI display: if user wants "oldest first" UI, reverse the array
-    records = records.reverse if journal_sorting.asc?
+    activities = load_activities(page_relation)
+    activities = activities.reverse if journal_sorting.asc?
 
-    [pagy, records]
+    [pagy_obj, activities]
   end
 
   private
 
+  # Activities relation (UNION of journals and changesets). Anchored
+  # navigation passes `filter: :all` so a deep link to a record that
+  # wouldn't match the active filter still resolves.
+  def activities_scope(filter: self.filter)
+    ActivitiesQuery.new(work_package, filter:).call
+  end
+
+  def visible_journals
+    work_package.journals.internal_visible
+  end
+
+  def limit
+    params[:limit] || Pagy::DEFAULT[:limit]
+  end
+
   def pagy_options
     {
       page: params[:page] || 1,
-      limit: params[:limit] || Pagy::DEFAULT[:limit],
+      limit:,
       request: { params: }
     }.compact
   end
 
-  def extract_target_record_id
+  def parse_anchor
     anchor = params[:anchor] # e.g., "comment-78758" (without #)
-    return nil unless anchor
+    return unless anchor
 
     match = anchor.match(/^(comment|activity)-(\d+)$/)
-    match && match.length == 3 ? [match[1].inquiry, match[2].to_i] : []
-  end
-
-  def pagy_array_for_target_journal(anchor_type, target_record_id)
-    journals = base_journals
-
-    target_index = journals.find_index do |record|
-      if anchor_type.comment?
-        record.id == target_record_id
-      elsif anchor_type.activity?
-        record.sequence_version == target_record_id
-      else
-        false
-      end
-    end
+    return unless match
 
-    if target_index
-      limit = pagy_options[:limit]
-      target_page = (target_index / limit) + 1
-      pagy(:offset, journals, **pagy_options, page: target_page)
-    else
-      # Journal might be filtered out or deleted - fallback to page 1
-      pagy(:offset, journals, **pagy_options, page: 1)
-    end
+    [match[1].inquiry, match[2].to_i]
   end
 
-  def base_journals
-    combine_and_sort_records(fetch_journals, fetch_revisions)
+  # An unresolvable anchor (deleted, never existed, not visible to the user)
+  # opens the tab at the newest page, the same as opening it without an
+  # anchor. Any `params[:page]` sent alongside is intentionally ignored: the
+  # anchor was the explicit navigation intent.
+  def pagy_at_anchor(anchor_type, target_record_id)
+    scope = activities_scope(filter: :all)
+    page = page_for_anchor(scope, anchor_type, target_record_id) || 1
+    pagy(:offset, scope, **pagy_options, page:)
   end
 
-  def capped_journals
-    max_records = (params[:limit] || Pagy::DEFAULT[:limit]) * MAX_PAGES
-    base_journals.first(max_records)
+  # Resolves an anchor to its target page by counting records ahead of it.
+  # Returns nil when the anchor is unresolvable so the caller falls back.
+  def page_for_anchor(scope, anchor_type, target_record_id)
+    activity_at, anchor_id = locate_anchor(anchor_type, target_record_id)
+    return nil unless activity_at && anchor_id
+
+    rows_ahead = scope
+      .where("(activities.activity_at, activities.id) > (?, ?)", activity_at, anchor_id)
+      .count(:all)
+
+    (rows_ahead / limit) + 1
   end
 
-  def fetch_journals
-    API::V3::Activities::ActivityEagerLoadingWrapper.wrap(fetch_ar_journals)
+  # Anchors must observe the same visibility rules as the activities feed.
+  # Otherwise the count-ahead would route an unviewable journal to a page
+  # number and leak the existence of internal journals through the URL.
+  def locate_anchor(anchor_type, target_record_id)
+    if anchor_type.comment?
+      visible_journals.where(id: target_record_id).pick(:created_at, :id)
+    elsif anchor_type.activity?
+      locate_anchor_by_sequence_version(target_record_id)
+    end
   end
 
-  def fetch_ar_journals
-    journals = work_package
-      .journals
-      .internal_visible
-      .includes(
-        :user,
-        :customizable_journals,
-        :attachable_journals,
-        :storable_journals,
-        :notifications
-      )
-      .reorder(version: :desc) # Always fetch newest first for pagination
+  def locate_anchor_by_sequence_version(sequence_version)
+    visible_journals
       .with_sequence_version
-
-    case filter
-    when :only_comments then apply_comments_only_filter(journals)
-    when :only_changes then apply_changes_only_filter(journals)
-    else
-      journals
-    end
+      .where(ranked: { sequence_version: sequence_version })
+      .pick(:created_at, :id)
   end
 
-  def fetch_revisions
-    return Changeset.none if filter == :only_comments
+  def load_activities(page_relation)
+    activity_refs = page_relation.pluck(Arel.sql("activities.kind"), Arel.sql("activities.id"))
+    activities_by_kind = load_page_activities_by_kind(activity_refs)
 
-    work_package.changesets.includes(:user, :repository)
+    ordered_activities = activity_refs.filter_map { |kind, id| activities_by_kind[kind][id] }
+    eager_load_journals(ordered_activities)
   end
 
-  def combine_and_sort_records(journals, revisions)
-    (journals + revisions).sort_by do |record|
-      timestamp = record_timestamp(record)
-      [-timestamp, -record.id] # Always sort DESC (newest first)
-    end
+  def load_page_activities_by_kind(activity_refs)
+    ids_by_kind = activity_refs.group_by(&:first).transform_values { it.map(&:last) }
+    {
+      ActivitiesQuery::KIND_JOURNAL => load_page_journals(ids_by_kind[ActivitiesQuery::KIND_JOURNAL] || []),
+      ActivitiesQuery::KIND_CHANGESET => load_page_changesets(ids_by_kind[ActivitiesQuery::KIND_CHANGESET] || [])
+    }
   end
 
-  def record_timestamp(record)
-    if record.is_a?(API::V3::Activities::ActivityEagerLoadingWrapper)
-      record.created_at&.to_i
-    elsif record.is_a?(Changeset)
-      record.committed_on.to_i
-    end
+  def load_page_journals(ids)
+    return {} if ids.empty?
+
+    Journal
+      .where(id: ids)
+      .with_sequence_version
+      .includes(:user, :customizable_journals, :attachable_journals, :storable_journals, :notifications)
+      .index_by(&:id)
   end
 
-  def apply_comments_only_filter(scope)
-    scope.where.not(notes: [nil, ""])
+  def load_page_changesets(ids)
+    return {} if ids.empty?
+
+    Changeset
+      .where(id: ids)
+      .includes(:user, :repository)
+      .index_by(&:id)
   end
 
-  def apply_changes_only_filter(scope)
-    JournalChangesFilter.apply(scope)
+  # Substitutes journals with their eager-loading wrappers so the wrapper's
+  # batch queries (journable, predecessor, data, notifications) run against
+  # the page slice only. Order from the input is preserved.
+  def eager_load_journals(activities)
+    journals = activities.grep(Journal)
+    wrapped_by_id = API::V3::Activities::ActivityEagerLoadingWrapper.wrap(journals).index_by(&:id)
+
+    activities.map { it.is_a?(Journal) ? wrapped_by_id[it.id] : it }
   end
 end

app/services/work_packages/activities_tab/paginator/activities_query.rb

@@ -0,0 +1,95 @@
+# frozen_string_literal: true
+
+# -- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+# ++
+
+# Builds the UNION ALL of journals and changesets for a work package's activity
+# feed, normalised to `(id, kind, activity_at)` rows ordered newest-first.
+#
+# Hydration is intentionally left out: the paginator slices the page first, then
+# loads Journal/Changeset records and applies the eager-loading wrapper. That
+# keeps the wrapper running against the page slice instead of the full history.
+#
+# The carrier class is `Journal` for AR composition only — rows in this relation
+# are never materialised as Journal records.
+class WorkPackages::ActivitiesTab::Paginator::ActivitiesQuery
+  KIND_JOURNAL = Journal.name
+  KIND_CHANGESET = Changeset.name
+
+  def initialize(work_package, filter:)
+    @work_package = work_package
+    @filter = filter
+  end
+
+  def call
+    Journal
+      .from(Arel.sql("(#{union_sql}) AS activities"))
+      .select(Arel.sql("activities.id, activities.kind, activities.activity_at"))
+      .order(Arel.sql("activities.activity_at DESC, activities.id DESC"))
+  end
+
+  private
+
+  attr_reader :work_package, :filter
+
+  def union_sql
+    parts = [journals_leg_sql]
+    parts << changesets_leg_sql unless filter == :only_comments
+    parts.join(" UNION ALL ")
+  end
+
+  def journals_leg_sql
+    apply_filter(work_package.journals.internal_visible)
+      .unscope(:order)
+      .select("journals.id, #{quote(KIND_JOURNAL)} AS kind, journals.created_at AS activity_at")
+      .to_sql
+  end
+
+  def apply_filter(scope)
+    case filter
+    when :only_comments
+      scope.where.not(notes: [nil, ""])
+    when :only_changes
+      WorkPackages::ActivitiesTab::Paginator::JournalChangesFilter.apply(scope)
+    else
+      scope
+    end
+  end
+
+  def changesets_leg_sql
+    work_package
+      .changesets
+      .unscope(:order)
+      .select("changesets.id, #{quote(KIND_CHANGESET)} AS kind, changesets.committed_on AS activity_at")
+      .to_sql
+  end
+
+  def quote(value)
+    ActiveRecord::Base.connection.quote(value)
+  end
+end

db/migrate/20260528112247_add_index_journals_on_journable_and_created_at.rb

@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+class AddIndexJournalsOnJournableAndCreatedAt < ActiveRecord::Migration[8.1]
+  disable_ddl_transaction!
+
+  def change
+    add_index :journals,
+              %i[journable_type journable_id created_at id],
+              order: { created_at: :desc, id: :desc },
+              name: "index_journals_on_journable_and_created_at",
+              algorithm: :concurrently,
+              if_not_exists: true
+  end
+end