Merge branch '26890-fix-default-branches-sorting' into 'master'

Fix the default branches sorting to actually be 'Last updated'

Closes #26890

See merge request gitlab-org/gitlab-ce!14295
This commit is contained in:
Robert Speicher 2017-10-04 15:21:19 +00:00
commit 8ace5fc188
5 changed files with 161 additions and 166 deletions

View File

@ -9,7 +9,7 @@ class Projects::BranchesController < Projects::ApplicationController
def index
@sort = params[:sort].presence || sort_value_recently_updated
@branches = BranchesFinder.new(@repository, params).execute
@branches = BranchesFinder.new(@repository, params.merge(sort: @sort)).execute
@branches = Kaminari.paginate_array(@branches).page(params[:page])
respond_to do |format|

View File

@ -0,0 +1,5 @@
---
title: Fix the default branches sorting to actually be 'Last updated'
merge_request: 14295
author:
type: fixed

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)
visit project_branches_path(project, search: 'binary-encoding')
end
scenario 'shows download artifacts button' do

View File

@ -5,12 +5,6 @@ describe 'Branches' do
let(:project) { create(:project, :public, :repository) }
let(:repository) { project.repository }
def set_protected_branch_name(branch_name)
find(".js-protected-branch-select").click
find(".dropdown-input-field").set(branch_name)
click_on("Create wildcard #{branch_name}")
end
context 'logged in as developer' do
before do
sign_in(user)
@ -18,12 +12,10 @@ describe 'Branches' do
end
describe 'Initial branches page' do
it 'shows all the branches' do
it 'shows all the branches sorted by last updated by default' do
visit project_branches_path(project)
repository.branches_sorted_by(:name).first(20).each do |branch|
expect(page).to have_content("#{branch.name}")
end
expect(page).to have_content(sorted_branches(repository, count: 20, sort_by: :updated_desc))
end
it 'sorts the branches by name' do
@ -32,22 +24,7 @@ describe 'Branches' do
click_button "Last updated" # Open sorting dropdown
click_link "Name"
sorted = repository.branches_sorted_by(:name).first(20).map do |branch|
Regexp.escape(branch.name)
end
expect(page).to have_content(/#{sorted.join(".*")}/)
end
it 'sorts the branches by last updated' do
visit project_branches_path(project)
click_button "Last updated" # Open sorting dropdown
click_link "Last updated"
sorted = repository.branches_sorted_by(:updated_desc).first(20).map do |branch|
Regexp.escape(branch.name)
end
expect(page).to have_content(/#{sorted.join(".*")}/)
expect(page).to have_content(sorted_branches(repository, count: 20, sort_by: :name))
end
it 'sorts the branches by oldest updated' do
@ -56,10 +33,7 @@ describe 'Branches' do
click_button "Last updated" # Open sorting dropdown
click_link "Oldest updated"
sorted = repository.branches_sorted_by(:updated_asc).first(20).map do |branch|
Regexp.escape(branch.name)
end
expect(page).to have_content(/#{sorted.join(".*")}/)
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
@ -99,28 +73,6 @@ describe 'Branches' do
expect(find('.all-branches')).to have_selector('li', count: 0)
end
end
describe 'Delete protected branch' do
before do
project.add_user(user, :master)
visit project_protected_branches_path(project)
set_protected_branch_name('fix')
click_on "Protect"
within(".protected-branches-list") { expect(page).to have_content('fix') }
expect(ProtectedBranch.count).to eq(1)
project.add_user(user, :developer)
end
it 'does not allow devleoper to removes protected branch', js: true do
visit project_branches_path(project)
fill_in 'branch-search', with: 'fix'
find('#branch-search').native.send_keys(:enter)
expect(page).to have_css('.btn-remove.disabled')
end
end
end
context 'logged in as master' do
@ -136,37 +88,6 @@ describe 'Branches' do
expect(page).to have_content("Protected branches can be managed in project settings")
end
end
describe 'Delete protected branch' do
before do
visit project_protected_branches_path(project)
set_protected_branch_name('fix')
click_on "Protect"
within(".protected-branches-list") { expect(page).to have_content('fix') }
expect(ProtectedBranch.count).to eq(1)
end
it 'removes branch after modal confirmation', js: true 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)
page.find('[data-target="#modal-delete-branch"]').trigger(:click)
expect(page).to have_css('.js-delete-branch[disabled]')
fill_in 'delete_branch_input', with: 'fix'
click_link 'Delete protected branch'
fill_in 'branch-search', with: 'fix'
find('#branch-search').native.send_keys(:enter)
expect(page).to have_content('No branches to show')
end
end
end
context 'logged out' do
@ -180,4 +101,13 @@ describe 'Branches' do
end
end
end
def sorted_branches(repository, count:, sort_by:)
sorted_branches =
repository.branches_sorted_by(sort_by).first(count).map do |branch|
Regexp.escape(branch.name)
end
Regexp.new(sorted_branches.join('.*'))
end
end

View File

@ -1,11 +1,148 @@
require 'spec_helper'
feature 'Protected Branches', js: true do
let(:user) { create(:user, :admin) }
feature 'Protected Branches', :js do
let(:user) { create(:user) }
let(:admin) { create(:admin) }
let(:project) { create(:project, :repository) }
before do
sign_in(user)
context 'logged in as developer' do
before do
project.add_developer(user)
sign_in(user)
end
describe 'Delete protected branch' do
before do
create(:protected_branch, project: project, name: 'fix')
expect(ProtectedBranch.count).to eq(1)
end
it 'does not allow developer to removes protected branch' do
visit project_branches_path(project)
fill_in 'branch-search', with: 'fix'
find('#branch-search').native.send_keys(:enter)
expect(page).to have_css('.btn-remove.disabled')
end
end
end
context 'logged in as master' do
before do
project.add_master(user)
sign_in(user)
end
describe 'Delete protected branch' do
before do
create(:protected_branch, project: project, name: 'fix')
expect(ProtectedBranch.count).to eq(1)
end
it 'removes branch after modal confirmation' 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)
page.find('[data-target="#modal-delete-branch"]').trigger(:click)
expect(page).to have_css('.js-delete-branch[disabled]')
fill_in 'delete_branch_input', with: 'fix'
click_link 'Delete protected branch'
fill_in 'branch-search', with: 'fix'
find('#branch-search').native.send_keys(:enter)
expect(page).to have_content('No branches to show')
end
end
end
context 'logged in as admin' do
before do
sign_in(admin)
end
describe "explicit protected branches" do
it "allows creating explicit protected branches" do
visit project_protected_branches_path(project)
set_protected_branch_name('some-branch')
click_on "Protect"
within(".protected-branches-list") { expect(page).to have_content('some-branch') }
expect(ProtectedBranch.count).to eq(1)
expect(ProtectedBranch.last.name).to eq('some-branch')
end
it "displays the last commit on the matching branch if it exists" do
commit = create(:commit, project: project)
project.repository.add_branch(admin, 'some-branch', commit.id)
visit project_protected_branches_path(project)
set_protected_branch_name('some-branch')
click_on "Protect"
within(".protected-branches-list") { expect(page).to have_content(commit.id[0..7]) }
end
it "displays an error message if the named branch does not exist" do
visit project_protected_branches_path(project)
set_protected_branch_name('some-branch')
click_on "Protect"
within(".protected-branches-list") { expect(page).to have_content('branch was removed') }
end
end
describe "wildcard protected branches" do
it "allows creating protected branches with a wildcard" do
visit project_protected_branches_path(project)
set_protected_branch_name('*-stable')
click_on "Protect"
within(".protected-branches-list") { expect(page).to have_content('*-stable') }
expect(ProtectedBranch.count).to eq(1)
expect(ProtectedBranch.last.name).to eq('*-stable')
end
it "displays the number of matching branches" do
project.repository.add_branch(admin, 'production-stable', 'master')
project.repository.add_branch(admin, 'staging-stable', 'master')
visit project_protected_branches_path(project)
set_protected_branch_name('*-stable')
click_on "Protect"
within(".protected-branches-list") { expect(page).to have_content("2 matching branches") }
end
it "displays all the branches matching the wildcard" do
project.repository.add_branch(admin, 'production-stable', 'master')
project.repository.add_branch(admin, 'staging-stable', 'master')
project.repository.add_branch(admin, 'development', 'master')
visit project_protected_branches_path(project)
set_protected_branch_name('*-stable')
click_on "Protect"
visit project_protected_branches_path(project)
click_on "2 matching branches"
within(".protected-branches-list") do
expect(page).to have_content("production-stable")
expect(page).to have_content("staging-stable")
expect(page).not_to have_content("development")
end
end
end
describe "access control" do
include_examples "protected branches > access control > CE"
end
end
def set_protected_branch_name(branch_name)
@ -13,81 +150,4 @@ feature 'Protected Branches', js: true do
find(".dropdown-input-field").set(branch_name)
click_on("Create wildcard #{branch_name}")
end
describe "explicit protected branches" do
it "allows creating explicit protected branches" do
visit project_protected_branches_path(project)
set_protected_branch_name('some-branch')
click_on "Protect"
within(".protected-branches-list") { expect(page).to have_content('some-branch') }
expect(ProtectedBranch.count).to eq(1)
expect(ProtectedBranch.last.name).to eq('some-branch')
end
it "displays the last commit on the matching branch if it exists" do
commit = create(:commit, project: project)
project.repository.add_branch(user, 'some-branch', commit.id)
visit project_protected_branches_path(project)
set_protected_branch_name('some-branch')
click_on "Protect"
within(".protected-branches-list") { expect(page).to have_content(commit.id[0..7]) }
end
it "displays an error message if the named branch does not exist" do
visit project_protected_branches_path(project)
set_protected_branch_name('some-branch')
click_on "Protect"
within(".protected-branches-list") { expect(page).to have_content('branch was removed') }
end
end
describe "wildcard protected branches" do
it "allows creating protected branches with a wildcard" do
visit project_protected_branches_path(project)
set_protected_branch_name('*-stable')
click_on "Protect"
within(".protected-branches-list") { expect(page).to have_content('*-stable') }
expect(ProtectedBranch.count).to eq(1)
expect(ProtectedBranch.last.name).to eq('*-stable')
end
it "displays the number of matching branches" do
project.repository.add_branch(user, 'production-stable', 'master')
project.repository.add_branch(user, 'staging-stable', 'master')
visit project_protected_branches_path(project)
set_protected_branch_name('*-stable')
click_on "Protect"
within(".protected-branches-list") { expect(page).to have_content("2 matching branches") }
end
it "displays all the branches matching the wildcard" do
project.repository.add_branch(user, 'production-stable', 'master')
project.repository.add_branch(user, 'staging-stable', 'master')
project.repository.add_branch(user, 'development', 'master')
visit project_protected_branches_path(project)
set_protected_branch_name('*-stable')
click_on "Protect"
visit project_protected_branches_path(project)
click_on "2 matching branches"
within(".protected-branches-list") do
expect(page).to have_content("production-stable")
expect(page).to have_content("staging-stable")
expect(page).not_to have_content("development")
end
end
end
describe "access control" do
include_examples "protected branches > access control > CE"
end
end