Merge branch 'id-optimize-branches-overview' into 'master'

Improve UI and performance of branches overview page

See merge request gitlab-org/gitlab!50096

app/controllers/projects/branches_controller.rb

@@ -18,8 +18,8 @@ class Projects::BranchesController < Projects::ApplicationController
   def index
     respond_to do |format|
       format.html do
-        @sort = params[:sort].presence || sort_value_recently_updated
         @mode = params[:state].presence || 'overview'
+        @sort = sort_value_for_mode
         @overview_max_branches = 5
 
         # Fetch branches for the specified mode
@@ -125,6 +125,12 @@ class Projects::BranchesController < Projects::ApplicationController
 
   private
 
+  def sort_value_for_mode
+    return params[:sort] if params[:sort].present?
+
+    'stale' == @mode ? sort_value_oldest_updated : sort_value_recently_updated
+  end
+
   # It can be expensive to calculate the diverging counts for each
   # branch. Normally the frontend should be specifying a set of branch
   # names, but prior to
@@ -169,19 +175,32 @@ class Projects::BranchesController < Projects::ApplicationController
   end
 
   def fetch_branches_by_mode
-    if @mode == 'overview'
-      # overview mode
-      @active_branches, @stale_branches = BranchesFinder.new(@repository, sort: sort_value_recently_updated).execute.partition(&:active?)
-      # Here we get one more branch to indicate if there are more data we're not showing
-      @active_branches = @active_branches.first(@overview_max_branches + 1)
-      @stale_branches = @stale_branches.first(@overview_max_branches + 1)
-      @branches = @active_branches + @stale_branches
+    return fetch_branches_for_overview if @mode == 'overview'
+
+    # active/stale/all view mode
+    @branches = BranchesFinder.new(@repository, params.merge(sort: @sort)).execute
+    @branches = @branches.select { |b| b.state.to_s == @mode } if %w[active stale].include?(@mode)
+    @branches = Kaminari.paginate_array(@branches).page(params[:page])
+  end
+
+  def fetch_branches_for_overview
+    # Here we get one more branch to indicate if there are more data we're not showing
+    limit = @overview_max_branches + 1
+
+    if Feature.enabled?(:branch_list_keyset_pagination, project, default_enabled: true)
+      @active_branches =
+        BranchesFinder.new(@repository, { per_page: limit, sort: sort_value_recently_updated })
+          .execute(gitaly_pagination: true).select(&:active?)
+      @stale_branches =
+        BranchesFinder.new(@repository, { per_page: limit, sort: sort_value_oldest_updated })
+          .execute(gitaly_pagination: true).select(&:stale?)
     else
-      # active/stale/all view mode
-      @branches = BranchesFinder.new(@repository, params.merge(sort: @sort)).execute
-      @branches = @branches.select { |b| b.state.to_s == @mode } if %w[active stale].include?(@mode)
-      @branches = Kaminari.paginate_array(@branches).page(params[:page])
+      @active_branches, @stale_branches = BranchesFinder.new(@repository, sort: sort_value_recently_updated).execute.partition(&:active?)
+      @active_branches = @active_branches.first(limit)
+      @stale_branches = @stale_branches.first(limit)
     end
+
+    @branches = @active_branches + @stale_branches
   end
 
   def confidential_issue_project

changelogs/unreleased/id-optimize-branches-overview.yml

+---
+title: Improve UI and performance of branches overview page
+merge_request: 50096
+author:
+type: performance

spec/controllers/projects/branches_controller_spec.rb

@@ -512,7 +512,8 @@ RSpec.describe Projects::BranchesController do
               state: 'all'
             }
 
-        expect(controller.instance_variable_get(:@branch_pipeline_statuses)["master"].group).to eq("success")
+        expect(assigns[:branch_pipeline_statuses]["master"].group).to eq("success")
+        expect(assigns[:sort]).to eq('updated_desc')
       end
     end
 
@@ -543,8 +544,8 @@ RSpec.describe Projects::BranchesController do
               state: 'all'
             }
 
-        expect(controller.instance_variable_get(:@branch_pipeline_statuses)["master"].group).to eq("running")
-        expect(controller.instance_variable_get(:@branch_pipeline_statuses)["test"].group).to eq("success")
+        expect(assigns[:branch_pipeline_statuses]["master"].group).to eq("running")
+        expect(assigns[:branch_pipeline_statuses]["test"].group).to eq("success")
       end
     end
 
@@ -555,10 +556,11 @@ RSpec.describe Projects::BranchesController do
             params: {
               namespace_id: project.namespace,
               project_id: project,
-              state: 'all'
+              state: 'stale'
             }
 
-        expect(controller.instance_variable_get(:@branch_pipeline_statuses)).to be_blank
+        expect(assigns[:branch_pipeline_statuses]).to be_blank
+        expect(assigns[:sort]).to eq('updated_asc')
       end
     end
 
@@ -573,10 +575,12 @@ RSpec.describe Projects::BranchesController do
             params: {
               namespace_id: project.namespace,
               project_id: project,
+              sort: 'name_asc',
               state: 'all'
             }
 
         expect(response).to have_gitlab_http_status(:ok)
+        expect(assigns[:sort]).to eq('name_asc')
       end
     end
 
@@ -635,6 +639,34 @@ RSpec.describe Projects::BranchesController do
         expect(response).to redirect_to project_branches_filtered_path(project, state: 'all')
       end
     end
+
+    context 'fetching branches for overview' do
+      before do
+        get :index, format: :html, params: {
+          namespace_id: project.namespace, project_id: project, state: 'overview'
+        }
+      end
+
+      it 'sets active and stale branches' do
+        expect(assigns[:active_branches]).to eq([])
+        expect(assigns[:stale_branches].map(&:name)).to eq(
+          ["feature", "improve/awesome", "merge-test", "markdown", "feature_conflict", "'test'"]
+        )
+      end
+
+      context 'branch_list_keyset_pagination is disabled' do
+        before do
+          stub_feature_flags(branch_list_keyset_pagination: false)
+        end
+
+        it 'sets active and stale branches' do
+          expect(assigns[:active_branches]).to eq([])
+          expect(assigns[:stale_branches].map(&:name)).to eq(
+            ["feature", "improve/awesome", "merge-test", "markdown", "feature_conflict", "'test'"]
+          )
+        end
+      end
+    end
   end
 
   describe 'GET diverging_commit_counts' do