root
/
gitlab-ce
mirror of https://gitlab.com/free-releases/gitlab-ce.git
1
0
Fork 0
Code Issues Actions Packages Projects Releases Wiki Activity

Add overview of branches and a filter for active/stale branches

This commit is contained in:
Takuya Noguchi 2017-11-15 23:56:36 +09:00
parent 65348cf07b
commit 580d895363
14 changed files with 350 additions and 62 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
app/controllers/projects/branches_controller.rb
View File

@ -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

2
app/finders/branches_finder.rb
View File

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

11
app/helpers/branches_helper.rb
View File

@ -1,15 +1,4 @@
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

19
app/views/projects/branches/_panel.html.haml Normal file
View File

@ -0,0 +1,19 @@
- 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 }

53
app/views/projects/branches/index.html.haml
View File

@ -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)

5
changelogs/unreleased/40187-project-branch-dashboard-with-active-stale-branches.yml Normal file
View File

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

1
config/routes/repository.rb
View File

@ -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

14
lib/gitlab/git/branch.rb
View File

@ -1,6 +1,8 @@
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

33
spec/controllers/projects/branches_controller_spec.rb
View File

@ -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

2
spec/features/projects/branches/download_buttons_spec.rb
View File

@ -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

161
spec/features/projects/branches_spec.rb
View File

@ -11,37 +11,74 @@ 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
visit project_branches_path(project)
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 }
expect(page).to have_content(sorted_branches(repository, count: 20, sort_by: :updated_desc))
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
it 'sorts the branches by name' do
visit project_branches_path(project)
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)
click_button "Last updated" # Open sorting dropdown
click_link "Name"
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_content(sorted_branches(repository, count: 20, sort_by: :name))
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
it 'sorts the branches by oldest updated' do
visit project_branches_path(project)
describe 'Active branches page' do
it 'shows 6 active branches sorted by last updated' do
visit project_branches_filtered_path(project, state: 'active')
click_button "Last updated" # Open sorting dropdown
click_link "Oldest updated"
expect(page).to have_content(sorted_branches(repository, count: 20, sort_by: :updated_asc))
expect(page).to have_content(sorted_branches(repository, count: 6, sort_by: :updated_desc, state: 'active'))
end
end
it 'avoids a N+1 query in branches index' do
control_count = ActiveRecord::QueryRecorder.new { visit project_branches_path(project) }.count
describe 'Stale branches page' do
it 'shows 4 active branches sorted by last updated' do
visit project_branches_filtered_path(project, state: 'stale')
%w(one two three four five).each { |ref| repository.add_branch(user, ref, 'master') }
expect(page).to have_content(sorted_branches(repository, count: 4, sort_by: :updated_desc, state: 'stale'))
end
end
expect { visit project_branches_path(project) }.not_to exceed_query_limit(control_count)
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
@ -57,9 +94,66 @@ describe 'Branches' do
end
end
describe 'Delete unprotected branch' do
describe 'Delete unprotected branch on Overview' do
it 'removes branch after confirmation', :js do
visit project_branches_path(project)
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_filtered_path(project, state: 'all')
click_button "Last updated" # Open sorting dropdown
click_link "Name"
expect(page).to have_content(sorted_branches(repository, count: 20, sort_by: :name))
end
it 'sorts the branches by oldest updated' do
visit project_branches_filtered_path(project, state: 'all')
click_button "Last updated" # Open sorting dropdown
click_link "Oldest updated"
expect(page).to have_content(sorted_branches(repository, count: 20, sort_by: :updated_asc))
end
it 'avoids a N+1 query in branches index' do
control_count = ActiveRecord::QueryRecorder.new { visit project_branches_path(project) }.count
%w(one two three four five).each { |ref| repository.add_branch(user, ref, 'master') }
expect { visit project_branches_filtered_path(project, state: 'all') }.not_to exceed_query_limit(control_count)
end
end
describe 'Find branches on All branches' do
it 'shows filtered branches', :js do
visit project_branches_filtered_path(project, state: 'all')
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 All branches' do
it 'removes branch after confirmation', :js do
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

2
spec/features/projects/environments/environment_spec.rb
View File

@ -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 }

4
spec/features/projects/merge_request_button_spec.rb
View File

@ -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

64
spec/lib/gitlab/git/branch_spec.rb
View File

@ -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