Merge branch '40187-project-branch-dashboard-with-active-stale-branches' into 'master'

Add overview of branches and filtered views for active/stale branches

Closes #40187 et #17293

See merge request gitlab-org/gitlab-ce!15402

app/controllers/projects/branches_controller.rb

@@ -7,13 +7,19 @@ class Projects::BranchesController < Projects::ApplicationController
   before_action :authorize_download_code!
   before_action :authorize_push_code!, only: [:new, :create, :destroy, :destroy_all_merged]
 
-  def index
-    @sort = params[:sort].presence || sort_value_recently_updated
-    @branches = BranchesFinder.new(@repository, params.merge(sort: @sort)).execute
-    @branches = Kaminari.paginate_array(@branches).page(params[:page])
+  # Support legacy URLs
+  before_action :redirect_for_legacy_index_sort_or_search, only: [:index]
 
+  def index
     respond_to do |format|
       format.html do
+        @sort = params[:sort].presence || sort_value_recently_updated
+        @mode = params[:state].presence || 'overview'
+        @overview_max_branches = 5
+
+        # Fetch branches for the specified mode
+        fetch_branches_by_mode
+
         @refs_pipelines = @project.pipelines.latest_successful_for_refs(@branches.map(&:name))
         @merged_branch_names =
           repository.merged_branch_names(@branches.map(&:name))
@@ -28,7 +34,9 @@ class Projects::BranchesController < Projects::ApplicationController
         end
       end
       format.json do
-        render json: @branches.map(&:name)
+        branches = BranchesFinder.new(@repository, params).execute
+        branches = Kaminari.paginate_array(branches).page(params[:page])
+        render json: branches.map(&:name)
       end
     end
   end
@@ -123,4 +131,27 @@ class Projects::BranchesController < Projects::ApplicationController
       context: 'autodeploy'
     )
   end
+
+  def redirect_for_legacy_index_sort_or_search
+    # Normalize a legacy URL with redirect
+    if request.format != :json && !params[:state].presence && [:sort, :search, :page].any? { |key| params[key].presence }
+      redirect_to project_branches_filtered_path(@project, state: 'all'), notice: 'Update your bookmarked URLs as filtered/sorted branches URL has been changed.'
+    end
+  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
+    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])
+    end
+  end
 end

app/finders/branches_finder.rb

 class BranchesFinder
-  def initialize(repository, params)
+  def initialize(repository, params = {})
     @repository = repository
     @params = params
   end

app/helpers/branches_helper.rb

 module BranchesHelper
-  def filter_branches_path(options = {})
-    exist_opts = {
-      search: params[:search],
-      sort: params[:sort]
-    }
-
-    options = exist_opts.merge(options)
-
-    project_branches_path(@project, @id, options)
-  end
-
   def project_branches
     options_for_select(@project.repository.branch_names, @project.default_branch)
   end

app/views/projects/branches/_panel.html.haml

+- branches = local_assigns.fetch(:branches)
+- state = local_assigns.fetch(:state)
+- panel_title = local_assigns.fetch(:panel_title)
+- show_more_text = local_assigns.fetch(:show_more_text)
+- project = local_assigns.fetch(:project)
+- overview_max_branches = local_assigns.fetch(:overview_max_branches)
+
+- return unless branches.any?
+
+.panel.panel-default.prepend-top-10
+  .panel-heading
+    %h4.panel-title
+      = panel_title
+  %ul.content-list.all-branches
+    - branches.first(overview_max_branches).each do |branch|
+      = render "projects/branches/branch", branch: branch, merged: project.repository.merged_to_root_ref?(branch)
+  - if branches.size > overview_max_branches
+    .panel-footer.text-center
+      = link_to show_more_text, project_branches_filtered_path(project, state: state), id: "state-#{state}", data: { state: state }

app/views/projects/branches/index.html.haml

@@ -3,26 +3,35 @@
 
 %div{ class: container_class }
   .top-area.adjust
-    - if can?(current_user, :admin_project, @project)
-      .nav-text
-        - project_settings_link = link_to s_('Branches|project settings'), project_protected_branches_path(@project)
-        = s_('Branches|Protected branches can be managed in %{project_settings_link}').html_safe % { project_settings_link: project_settings_link }
+    %ul.nav-links.issues-state-filters
+      %li{ class: active_when(@mode == 'overview') }>
+        = link_to s_('Branches|Overview'), project_branches_path(@project), title: s_('Branches|Show overview of the branches')
+
+      %li{ class: active_when(@mode == 'active') }>
+        = link_to s_('Branches|Active'), project_branches_filtered_path(@project, state: 'active'), title: s_('Branches|Show active branches')
+
+      %li{ class: active_when(@mode == 'stale') }>
+        = link_to s_('Branches|Stale'), project_branches_filtered_path(@project, state: 'stale'), title: s_('Branches|Show stale branches')
+
+      %li{ class: active_when(!%w[overview active stale].include?(@mode)) }>
+        = link_to s_('Branches|All'), project_branches_filtered_path(@project, state: 'all'), title: s_('Branches|Show all branches')
 
     .nav-controls
-      = form_tag(filter_branches_path, method: :get) do
+      = form_tag(project_branches_filtered_path(@project, state: 'all'), method: :get) do
         = search_field_tag :search, params[:search], { placeholder: s_('Branches|Filter by branch name'), id: 'branch-search', class: 'form-control search-text-input input-short', spellcheck: false }
 
-      .dropdown.inline>
-        %button.dropdown-menu-toggle{ type: 'button', 'data-toggle' => 'dropdown' }
-          %span.light
-            = branches_sort_options_hash[@sort]
-          = icon('chevron-down')
-        %ul.dropdown-menu.dropdown-menu-align-right.dropdown-menu-selectable
-          %li.dropdown-header
-            = s_('Branches|Sort by')
-          - branches_sort_options_hash.each do |value, title|
-            %li
-              = link_to title, filter_branches_path(sort: value), class: ("is-active" if @sort == value)
+      - unless @mode == 'overview'
+        .dropdown.inline>
+          %button.dropdown-menu-toggle{ type: 'button', 'data-toggle' => 'dropdown' }
+            %span.light
+              = branches_sort_options_hash[@sort]
+            = icon('chevron-down')
+          %ul.dropdown-menu.dropdown-menu-align-right.dropdown-menu-selectable
+            %li.dropdown-header
+              = s_('Branches|Sort by')
+            - branches_sort_options_hash.each do |value, title|
+              %li
+                = link_to title, project_branches_filtered_path(@project, state: 'all', search: params[:search], sort: value), class: ("is-active" if @sort == value)
 
       - if can? current_user, :push_code, @project
         = link_to project_merged_branches_path(@project),
@@ -35,7 +44,17 @@
         = link_to new_project_branch_path(@project), class: 'btn btn-create' do
           = s_('Branches|New branch')
 
-  - if @branches.any?
+  - if can?(current_user, :admin_project, @project)
+    - project_settings_link = link_to s_('Branches|project settings'), project_protected_branches_path(@project)
+    .row-content-block
+      %h5
+        = s_('Branches|Protected branches can be managed in %{project_settings_link}.').html_safe % { project_settings_link: project_settings_link }
+
+  - if @mode == 'overview' && (@active_branches.any? || @stale_branches.any?)
+    = render "projects/branches/panel", branches: @active_branches, state: 'active', panel_title: s_('Branches|Active branches'), show_more_text: s_('Branches|Show more active branches'), project: @project, overview_max_branches: @overview_max_branches
+    = render "projects/branches/panel", branches: @stale_branches, state: 'stale', panel_title: s_('Branches|Stale branches'), show_more_text: s_('Branches|Show more stale branches'), project: @project, overview_max_branches: @overview_max_branches
+
+  - elsif @branches.any?
     %ul.content-list.all-branches
       - @branches.each do |branch|
         = render "projects/branches/branch", branch: branch, merged: @merged_branch_names.include?(branch.name)

changelogs/unreleased/40187-project-branch-dashboard-with-active-stale-branches.yml

+---
+title: Add overview of branches and a filter for active/stale branches
+merge_request: 15402
+author: Takuya Noguchi
+type: added

config/routes/repository.rb

@@ -49,6 +49,7 @@ scope format: false do
       end
     end
 
+    get '/branches/:state', to: 'branches#index', as: :branches_filtered, constraints: { state: /active|stale|all/ }
     resources :branches, only: [:index, :new, :create, :destroy]
     delete :merged_branches, controller: 'branches', action: :destroy_all_merged
     resources :tags, only: [:index, :show, :new, :create, :destroy] do

lib/gitlab/git/branch.rb

 module Gitlab
   module Git
     class Branch < Ref
+      STALE_BRANCH_THRESHOLD = 3.months
+
       def self.find(repo, branch_name)
         if branch_name.is_a?(Gitlab::Git::Branch)
           branch_name
@@ -12,6 +14,18 @@ module Gitlab
       def initialize(repository, name, target, target_commit)
         super(repository, name, target, target_commit)
       end
+
+      def active?
+        self.dereferenced_target.committed_date >= STALE_BRANCH_THRESHOLD.ago
+      end
+
+      def stale?
+        !active?
+      end
+
+      def state
+        active? ? :active : :stale
+      end
     end
   end
 end

spec/controllers/projects/branches_controller_spec.rb

@@ -407,10 +407,43 @@ describe Projects::BranchesController do
         get :index,
             namespace_id: project.namespace,
             project_id: project,
+            state: 'all',
             format: :html
 
         expect(response).to have_gitlab_http_status(200)
       end
     end
+
+    context 'when depreated sort/search/page parameters are specified' do
+      it 'returns with a status 301 when sort specified' do
+        get :index,
+            namespace_id: project.namespace,
+            project_id: project,
+            sort: 'updated_asc',
+            format: :html
+
+        expect(response).to redirect_to project_branches_filtered_path(project, state: 'all')
+      end
+
+      it 'returns with a status 301 when search specified' do
+        get :index,
+            namespace_id: project.namespace,
+            project_id: project,
+            search: 'feature',
+            format: :html
+
+        expect(response).to redirect_to project_branches_filtered_path(project, state: 'all')
+      end
+
+      it 'returns with a status 301 when page specified' do
+        get :index,
+            namespace_id: project.namespace,
+            project_id: project,
+            page: 2,
+            format: :html
+
+        expect(response).to redirect_to project_branches_filtered_path(project, state: 'all')
+      end
+    end
   end
 end

spec/features/projects/branches/download_buttons_spec.rb

@@ -29,7 +29,7 @@ feature 'Download buttons in branches page' do
   describe 'when checking branches' do
     context 'with artifacts' do
       before do
-        visit project_branches_path(project, search: 'binary-encoding')
+        visit project_branches_filtered_path(project, state: 'all', search: 'binary-encoding')
       end
 
       scenario 'shows download artifacts button' do

spec/features/projects/branches_spec.rb

@@ -11,15 +11,109 @@ describe 'Branches' do
       project.add_developer(user)
     end
 
-    describe 'Initial branches page' do
-      it 'shows all the branches sorted by last updated by default' do
+    context 'on the projects with 6 active branches and 4 stale branches' do
+      let(:project) { create(:project, :public, :empty_repo) }
+      let(:repository) { project.repository }
+      let(:threshold) { Gitlab::Git::Branch::STALE_BRANCH_THRESHOLD }
+
+      before do
+        # Add 4 stale branches
+        (1..4).reverse_each do |i|
+          Timecop.freeze((threshold + i).ago) { create_file(message: "a commit in stale-#{i}", branch_name: "stale-#{i}") }
+        end
+        # Add 6 active branches
+        (1..6).each do |i|
+          Timecop.freeze((threshold - i).ago) { create_file(message: "a commit in active-#{i}", branch_name: "active-#{i}") }
+        end
+      end
+
+      describe 'Overview page of the branches' do
+        it 'shows the first 5 active branches and the first 4 stale branches sorted by last updated' do
+          visit project_branches_path(project)
+
+          expect(page).to have_content(sorted_branches(repository, count: 5, sort_by: :updated_desc, state: 'active'))
+          expect(page).to have_content(sorted_branches(repository, count: 4, sort_by: :updated_desc, state: 'stale'))
+
+          expect(page).to have_link('Show more active branches', href: project_branches_filtered_path(project, state: 'active'))
+          expect(page).not_to have_content('Show more stale branches')
+        end
+      end
+
+      describe 'Active branches page' do
+        it 'shows 6 active branches sorted by last updated' do
+          visit project_branches_filtered_path(project, state: 'active')
+
+          expect(page).to have_content(sorted_branches(repository, count: 6, sort_by: :updated_desc, state: 'active'))
+        end
+      end
+
+      describe 'Stale branches page' do
+        it 'shows 4 active branches sorted by last updated' do
+          visit project_branches_filtered_path(project, state: 'stale')
+
+          expect(page).to have_content(sorted_branches(repository, count: 4, sort_by: :updated_desc, state: 'stale'))
+        end
+      end
+
+      describe 'All branches page' do
+        it 'shows 10 branches sorted by last updated' do
+          visit project_branches_filtered_path(project, state: 'all')
+
+          expect(page).to have_content(sorted_branches(repository, count: 10, sort_by: :updated_desc))
+        end
+      end
+
+      context 'with branches over more than one page' do
+        before do
+          allow(Kaminari.config).to receive(:default_per_page).and_return(5)
+        end
+
+        it 'shows only default_per_page active branches sorted by last updated' do
+          visit project_branches_filtered_path(project, state: 'active')
+
+          expect(page).to have_content(sorted_branches(repository, count: Kaminari.config.default_per_page, sort_by: :updated_desc, state: 'active'))
+        end
+
+        it 'shows only default_per_page branches sorted by last updated on All branches' do
+          visit project_branches_filtered_path(project, state: 'all')
+
+          expect(page).to have_content(sorted_branches(repository, count: Kaminari.config.default_per_page, sort_by: :updated_desc))
+        end
+      end
+    end
+
+    describe 'Find branches' do
+      it 'shows filtered branches', :js do
         visit project_branches_path(project)
 
+        fill_in 'branch-search', with: 'fix'
+        find('#branch-search').native.send_keys(:enter)
+
+        expect(page).to have_content('fix')
+        expect(find('.all-branches')).to have_selector('li', count: 1)
+      end
+    end
+
+    describe 'Delete unprotected branch on Overview' do
+      it 'removes branch after confirmation', :js do
+        visit project_branches_filtered_path(project, state: 'all')
+
+        expect(all('.all-branches').last).to have_selector('li', count: 20)
+        accept_confirm { find('.js-branch-add-pdf-text-binary .btn-remove').click }
+
+        expect(all('.all-branches').last).to have_selector('li', count: 19)
+      end
+    end
+
+    describe 'All branches page' do
+      it 'shows all the branches sorted by last updated by default' do
+        visit project_branches_filtered_path(project, state: 'all')
+
         expect(page).to have_content(sorted_branches(repository, count: 20, sort_by: :updated_desc))
       end
 
       it 'sorts the branches by name' do
-        visit project_branches_path(project)
+        visit project_branches_filtered_path(project, state: 'all')
 
         click_button "Last updated" # Open sorting dropdown
         click_link "Name"
@@ -28,7 +122,7 @@ describe 'Branches' do
       end
 
       it 'sorts the branches by oldest updated' do
-        visit project_branches_path(project)
+        visit project_branches_filtered_path(project, state: 'all')
 
         click_button "Last updated" # Open sorting dropdown
         click_link "Oldest updated"
@@ -41,13 +135,13 @@ describe 'Branches' do
 
         %w(one two three four five).each { |ref| repository.add_branch(user, ref, 'master') }
 
-        expect { visit project_branches_path(project) }.not_to exceed_query_limit(control_count)
+        expect { visit project_branches_filtered_path(project, state: 'all') }.not_to exceed_query_limit(control_count)
       end
     end
 
-    describe 'Find branches' do
+    describe 'Find branches on All branches' do
       it 'shows filtered branches', :js do
-        visit project_branches_path(project)
+        visit project_branches_filtered_path(project, state: 'all')
 
         fill_in 'branch-search', with: 'fix'
         find('#branch-search').native.send_keys(:enter)
@@ -57,9 +151,9 @@ describe 'Branches' do
       end
     end
 
-    describe 'Delete unprotected branch' do
+    describe 'Delete unprotected branch on All branches' do
       it 'removes branch after confirmation', :js do
-        visit project_branches_path(project)
+        visit project_branches_filtered_path(project, state: 'all')
 
         fill_in 'branch-search', with: 'fix'
 
@@ -73,6 +167,19 @@ describe 'Branches' do
         expect(find('.all-branches')).to have_selector('li', count: 0)
       end
     end
+
+    context 'on project with 0 branch' do
+      let(:project) { create(:project, :public, :empty_repo) }
+      let(:repository) { project.repository }
+
+      describe '0 branches on Overview' do
+        it 'shows warning' do
+          visit project_branches_path(project)
+
+          expect(page).not_to have_selector('.all-branches')
+        end
+      end
+    end
   end
 
   context 'logged in as master' do
@@ -83,7 +190,7 @@ describe 'Branches' do
 
     describe 'Initial branches page' do
       it 'shows description for admin' do
-        visit project_branches_path(project)
+        visit project_branches_filtered_path(project, state: 'all')
 
         expect(page).to have_content("Protected branches can be managed in project settings")
       end
@@ -102,12 +209,18 @@ describe 'Branches' do
     end
   end
 
-  def sorted_branches(repository, count:, sort_by:)
+  def sorted_branches(repository, count:, sort_by:, state: nil)
+    branches = repository.branches_sorted_by(sort_by)
+    branches = branches.select { |b| state == 'active' ? b.active? : b.stale? } if state
     sorted_branches =
-      repository.branches_sorted_by(sort_by).first(count).map do |branch|
+      branches.first(count).map do |branch|
         Regexp.escape(branch.name)
       end
 
     Regexp.new(sorted_branches.join('.*'))
   end
+
+  def create_file(message: 'message', branch_name:)
+    repository.create_file(user, generate(:branch), 'content', message: message, branch_name: branch_name)
+  end
 end

spec/features/projects/environments/environment_spec.rb

@@ -234,7 +234,7 @@ feature 'Environment' do
     end
 
     scenario 'user deletes the branch with running environment' do
-      visit project_branches_path(project, search: 'feature')
+      visit project_branches_filtered_path(project, state: 'all', search: 'feature')
 
       remove_branch_with_hooks(project, user, 'feature') do
         page.within('.js-branch-feature') { find('a.btn-remove').click }

spec/features/projects/merge_request_button_spec.rb

@@ -81,8 +81,8 @@ feature 'Merge Request button' do
   context 'on branches page' do
     it_behaves_like 'Merge request button only shown when allowed' do
       let(:label) { 'Merge request' }
-      let(:url) { project_branches_path(project, search: 'feature') }
-      let(:fork_url) { project_branches_path(forked_project, search: 'feature') }
+      let(:url) { project_branches_filtered_path(project, state: 'all', search: 'feature') }
+      let(:fork_url) { project_branches_filtered_path(forked_project, state: 'all', search: 'feature') }
     end
   end
 

spec/lib/gitlab/git/branch_spec.rb

@@ -59,5 +59,69 @@ describe Gitlab::Git::Branch, seed_helper: true do
     it { expect(branch.dereferenced_target.sha).to eq(SeedRepo::LastCommit::ID) }
   end
 
+  context 'with active, stale and future branches' do
+    let(:repository) do
+      Gitlab::Git::Repository.new('default', TEST_MUTABLE_REPO_PATH, '')
+    end
+
+    let(:user) { create(:user) }
+    let(:committer) do
+      Gitlab::Git.committer_hash(email: user.email, name: user.name)
+    end
+    let(:params) do
+      parents = [repository.rugged.head.target]
+      tree = parents.first.tree
+
+      {
+        message: 'commit message',
+        author: committer,
+        committer: committer,
+        tree: tree,
+        parents: parents
+      }
+    end
+    let(:stale_sha) { Timecop.freeze(Gitlab::Git::Branch::STALE_BRANCH_THRESHOLD.ago - 5.days) { create_commit } }
+    let(:active_sha) { Timecop.freeze(Gitlab::Git::Branch::STALE_BRANCH_THRESHOLD.ago + 5.days) { create_commit } }
+    let(:future_sha) { Timecop.freeze(100.days.since) { create_commit } }
+
+    before do
+      repository.create_branch('stale-1', stale_sha)
+      repository.create_branch('active-1', active_sha)
+      repository.create_branch('future-1', future_sha)
+    end
+
+    after do
+      ensure_seeds
+    end
+
+    describe 'examine if the branch is active or stale' do
+      let(:stale_branch) { repository.find_branch('stale-1') }
+      let(:active_branch) { repository.find_branch('active-1') }
+      let(:future_branch) { repository.find_branch('future-1') }
+
+      describe '#active?' do
+        it { expect(stale_branch.active?).to be_falsey }
+        it { expect(active_branch.active?).to be_truthy }
+        it { expect(future_branch.active?).to be_truthy }
+      end
+
+      describe '#stale?' do
+        it { expect(stale_branch.stale?).to be_truthy }
+        it { expect(active_branch.stale?).to be_falsey }
+        it { expect(future_branch.stale?).to be_falsey }
+      end
+
+      describe '#state' do
+        it { expect(stale_branch.state).to eq(:stale) }
+        it { expect(active_branch.state).to eq(:active) }
+        it { expect(future_branch.state).to eq(:active) }
+      end
+    end
+  end
+
   it { expect(repository.branches.size).to eq(SeedRepo::Repo::BRANCHES.size) }
+
+  def create_commit
+    repository.create_commit(params.merge(committer: committer.merge(time: Time.now)))
+  end
 end