Merge branch '34280-gitalyclient-sort_by_param-invalid-sort_by-key-recently_updated' into 'master'

Fix a bug where an invalid sort param value was passed to Gitaly

Closes #34280

See merge request !12534
This commit is contained in:
Rémy Coutable 2017-06-29 12:53:55 +00:00
commit 572fa2cb02
3 changed files with 50 additions and 1 deletions

View file

@ -59,6 +59,8 @@ module Gitlab
end end
def sort_by_param(sort_by) def sort_by_param(sort_by)
sort_by = 'name' if sort_by == 'name_asc'
enum_value = Gitaly::FindLocalBranchesRequest::SortBy.resolve(sort_by.upcase.to_sym) enum_value = Gitaly::FindLocalBranchesRequest::SortBy.resolve(sort_by.upcase.to_sym)
raise ArgumentError, "Invalid sort_by key `#{sort_by}`" unless enum_value raise ArgumentError, "Invalid sort_by key `#{sort_by}`" unless enum_value
enum_value enum_value

View file

@ -21,10 +21,48 @@ describe 'Branches', feature: true do
it 'shows all the branches' do it 'shows all the branches' do
visit namespace_project_branches_path(project.namespace, project) visit namespace_project_branches_path(project.namespace, project)
repository.branches { |branch| expect(page).to have_content("#{branch.name}") } repository.branches_sorted_by(:name).first(20).each do |branch|
expect(page).to have_content("#{branch.name}")
end
expect(page).to have_content("Protected branches can be managed in project settings") expect(page).to have_content("Protected branches can be managed in project settings")
end end
it 'sorts the branches by name' do
visit namespace_project_branches_path(project.namespace, project)
click_button "Name" # 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 namespace_project_branches_path(project.namespace, project)
click_button "Name" # 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(".*")}/)
end
it 'sorts the branches by oldest updated' do
visit namespace_project_branches_path(project.namespace, project)
click_button "Name" # 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(".*")}/)
end
it 'avoids a N+1 query in branches index' do it 'avoids a N+1 query in branches index' do
control_count = ActiveRecord::QueryRecorder.new { visit namespace_project_branches_path(project.namespace, project) }.count control_count = ActiveRecord::QueryRecorder.new { visit namespace_project_branches_path(project.namespace, project) }.count

View file

@ -69,6 +69,15 @@ describe Gitlab::GitalyClient::Ref do
client.local_branches(sort_by: 'updated_desc') client.local_branches(sort_by: 'updated_desc')
end end
it 'translates known mismatches on sort param values' do
expect_any_instance_of(Gitaly::Ref::Stub)
.to receive(:find_local_branches)
.with(gitaly_request_with_params(sort_by: :NAME), kind_of(Hash))
.and_return([])
client.local_branches(sort_by: 'name_asc')
end
it 'raises an argument error if an invalid sort_by parameter is passed' do it 'raises an argument error if an invalid sort_by parameter is passed' do
expect { client.local_branches(sort_by: 'invalid_sort') }.to raise_error(ArgumentError) expect { client.local_branches(sort_by: 'invalid_sort') }.to raise_error(ArgumentError)
end end