Merge pull request #23495 from opf/fix/improve_speed_of_backlog_filtering

Fix/improve speed of backlog filtering

Author: ulferts

modules/backlogs/app/components/backlogs/backlog_component.html.erb

@@ -75,11 +75,18 @@ See COPYRIGHT and LICENSE files for more details.
     end
   %>
 
-  <%= render Backlogs::BucketComponent.with_collection(buckets, project:) %>
-
+  <% buckets.each do |bucket| %>
+    <%=
+      render Backlogs::BucketComponent.new(
+        project:,
+        backlog_bucket: bucket,
+        work_packages: work_packages_for(bucket)
+      )
+    %>
+  <% end %>
   <%=
     render Backlogs::InboxComponent.new(
-      work_packages: inbox_work_packages,
+      work_packages: work_packages_for_inbox,
       project: project
     )
   %>

modules/backlogs/app/components/backlogs/backlog_component.rb

@@ -34,15 +34,15 @@ class BacklogComponent < ApplicationComponent
     include OpTurbo::Streamable
     include CommonHelper
 
-    attr_reader :inbox_work_packages, :buckets, :project, :current_user
+    attr_reader :work_packages_by_backlog_id, :buckets, :project, :current_user
 
-    def initialize(inbox_work_packages:,
-                   buckets:,
+    def initialize(buckets:,
+                   work_packages_by_backlog_id:,
                    project:,
                    current_user: User.current)
       super()
 
-      @inbox_work_packages = inbox_work_packages
+      @work_packages_by_backlog_id = work_packages_by_backlog_id
       @buckets = buckets
       @project = project
       @current_user = current_user
@@ -55,7 +55,15 @@ def wrapper_uniq_by
     private
 
     def total
-      @total ||= inbox_work_packages.count + (buckets&.sum { it.work_packages.size } || 0)
+      @total ||= work_packages_by_backlog_id.values.sum(&:count)
+    end
+
+    def work_packages_for_inbox
+      work_packages_by_backlog_id[nil] || []
+    end
+
+    def work_packages_for(bucket)
+      work_packages_by_backlog_id[bucket.id] || []
     end
   end
 end

modules/backlogs/app/components/backlogs/bucket_component.rb

@@ -34,17 +34,16 @@ class BucketComponent < ApplicationComponent
     include OpTurbo::Streamable
     include CommonHelper
 
-    with_collection_parameter :backlog_bucket
-
     attr_reader :backlog_bucket, :work_packages, :project, :current_user
 
-    def initialize(backlog_bucket:, project:, current_user: User.current)
+    def initialize(backlog_bucket:, project:, work_packages: nil, current_user: User.current)
       super()
 
       @backlog_bucket = backlog_bucket
       @project = project
       @current_user = current_user
-      @work_packages = backlog_bucket.displayed_work_packages
+      @work_packages = work_packages || backlog_bucket.displayed_work_packages
+                                                      .includes(:status, :type, :assigned_to, :priority, :parent)
     end
 
     def wrapper_uniq_by

modules/backlogs/app/components/backlogs/inbox_component.rb

@@ -69,11 +69,7 @@ def show_more_label
     end
 
     def last_omitted_id
-      if work_packages.respond_to?(:reverse_order)
-        work_packages.reverse_order.offset(tail_size).limit(1).pick(:id)
-      else
-        work_packages[-(tail_size + 1)]&.id
-      end
+      work_packages[-(tail_size + 1)]&.id
     end
 
     private

modules/backlogs/app/components/backlogs/sprint_component.rb

@@ -46,8 +46,8 @@ def initialize(sprint:, project:, work_packages: nil, current_user: User.current
       @project = project
       @current_user = current_user
       @active_sprint_ids = active_sprint_ids
-      @work_packages = work_packages || sprint.work_packages_for(project).includes(:status, :type, :assigned_to, :priority,
-                                                                                   :parent)
+      @work_packages = work_packages || sprint.work_packages_for(project)
+                                              .includes(:status, :type, :assigned_to, :priority, :parent)
     end
 
     def wrapper_uniq_by

modules/backlogs/app/components/backlogs/sprints_component.html.erb

@@ -63,7 +63,7 @@ See COPYRIGHT and LICENSE files for more details.
     <%= render Backlogs::SprintComponent.new(
           sprint:,
           project:,
-          work_packages: work_packages_by_sprint_id[sprint.id],
+          work_packages: work_packages_by_sprint_id[sprint.id] || [],
           active_sprint_ids: active_sprint_ids
         ) %>
   <% end %>

modules/backlogs/app/controllers/backlogs/backlog_controller.rb

@@ -31,6 +31,7 @@
 module Backlogs
   class BacklogController < BaseController
     include ::WorkPackages::WithSplitView
+    include Backlogs::Concerns::ContainerLoading
 
     current_menu_item %i[show details] do
       :backlog
@@ -39,7 +40,7 @@ class BacklogController < BaseController
     def show
       case turbo_frame_request_id
       when "backlogs_container"
-        load_backlogs
+        load_container_data
 
         render partial: "backlogs/backlog/backlog_list", layout: false
       else
@@ -51,7 +52,7 @@ def details
       if turbo_frame_request?
         render "work_packages/split_view", layout: false
       else
-        load_backlogs
+        load_container_data
 
         render "backlogs/backlog/show"
       end
@@ -62,22 +63,5 @@ def details
     def split_view_base_route
       project_backlogs_backlog_path(@project, request.query_parameters)
     end
-
-    def load_backlogs
-      @backlog_buckets = BacklogBucket.for_project(@project)
-
-      @sprints = Sprint.for_project(@project)
-                       .not_completed
-                       .order_by_date
-                       .includes(:project, :task_boards)
-
-      @work_packages_by_sprint_id = WorkPackage
-                                      .where(sprint: @sprints, project: @project)
-                                      .includes(:type, :status, :assigned_to, :priority, :parent)
-                                      .order_by_position
-                                      .group_by(&:sprint_id)
-      @active_sprint_ids = @sprints.select(&:active?).map(&:id)
-      @inbox_work_packages = WorkPackage.backlogs_inbox_for(project: @project)
-    end
   end
 end

modules/backlogs/app/controllers/backlogs/concerns/container_loading.rb

@@ -0,0 +1,67 @@
+# 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.
+# ++
+
+module Backlogs::Concerns
+  module ContainerLoading
+    extend ActiveSupport::Concern
+
+    def load_container_data
+      load_sprint_data
+      load_backlog_data
+    end
+
+    def load_sprint_data
+      @sprints = Sprint.for_project(@project)
+                       .not_completed
+                       .order_by_date
+                       .includes(:project, :task_boards)
+      @active_sprint_ids = @sprints.select(&:active?).map(&:id)
+
+      @work_packages_by_sprint_id = WorkPackage
+                                      .where(sprint: @sprints, project: @project)
+                                      .includes(:type, :status, :assigned_to, :priority, :parent)
+                                      .order_by_position
+                                      .group_by(&:sprint_id)
+    end
+
+    def load_backlog_data
+      @backlog_buckets = BacklogBucket.for_project(@project)
+
+      # Includes the work packages of both the buckets and the inbox.
+      # This has the drawback of loading more work packages than are displayed in the inbox as pagination
+      # will only show the top 50 and lowest 10 work packages.
+      # But doing only a single query (+ includes) to the database has its benefits, and currently this seems quicker.
+      @work_packages_by_backlog_id = WorkPackage
+                                       .in_backlog_for(project: @project)
+                                       .includes(:type, :status, :assigned_to, :priority, :parent)
+                                       .group_by(&:backlog_bucket_id)
+    end
+  end
+end

modules/backlogs/app/controllers/backlogs/work_packages_controller.rb

@@ -31,6 +31,7 @@
 module Backlogs
   class WorkPackagesController < BaseController
     include OpTurbo::ComponentStream
+    include Backlogs::Concerns::ContainerLoading
 
     before_action :load_work_package
 
@@ -112,10 +113,11 @@ def sprint_component(sprint:)
     end
 
     def backlog_component
-      inbox_work_packages = WorkPackage.backlogs_inbox_for(project: @project)
-      buckets = BacklogBucket.for_project(@project)
+      load_backlog_data
 
-      Backlogs::BacklogComponent.new(inbox_work_packages:, buckets:, project: @project)
+      Backlogs::BacklogComponent.new(buckets: @backlog_buckets,
+                                     work_packages_by_backlog_id: @work_packages_by_backlog_id,
+                                     project: @project)
     end
 
     def load_work_package
@@ -133,7 +135,7 @@ def displayed_work_packages
       elsif @work_package.backlog_bucket_id?
         @work_packages.merge(@work_package.backlog_bucket.displayed_work_packages)
       else
-        @work_packages.merge(WorkPackage.backlogs_inbox_for(project: @project))
+        @work_packages.merge(WorkPackage.in_inbox_for(project: @project))
       end
     end
 

modules/backlogs/app/models/backlog_bucket.rb

@@ -52,6 +52,6 @@ class BacklogBucket < ApplicationRecord
   validates :name, :project, presence: true
 
   def self.for_project(project)
-    where(project:).order_alphabetically.includes(displayed_work_packages: %i[assigned_to priority parent])
+    where(project:).order_alphabetically
   end
 end