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

3 files changed, 50 insertions, 1 deletions

diff --git a/lib/gitlab/gitaly_client/ref.rb b/lib/gitlab/gitaly_client/ref.rb
index f4786f28a3a..2d61992f595 100644
--- a/lib/gitlab/gitaly_client/ref.rb
+++ b/lib/gitlab/gitaly_client/ref.rb
@@ -59,6 +59,8 @@ module Gitlab
       end
 
       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)
         raise ArgumentError, "Invalid sort_by key `#{sort_by}`" unless enum_value
         enum_value
diff --git a/spec/features/projects/branches_spec.rb b/spec/features/projects/branches_spec.rb
index 8694366de35..0050864d305 100644
--- a/spec/features/projects/branches_spec.rb
+++ b/spec/features/projects/branches_spec.rb
@@ -21,10 +21,48 @@ describe 'Branches', feature: true do
       it 'shows all the branches' do
         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")
       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
         control_count = ActiveRecord::QueryRecorder.new { visit namespace_project_branches_path(project.namespace, project) }.count
 
diff --git a/spec/lib/gitlab/gitaly_client/ref_spec.rb b/spec/lib/gitlab/gitaly_client/ref_spec.rb
index 42dba2ff874..8ad39a02b93 100644
--- a/spec/lib/gitlab/gitaly_client/ref_spec.rb
+++ b/spec/lib/gitlab/gitaly_client/ref_spec.rb
@@ -69,6 +69,15 @@ describe Gitlab::GitalyClient::Ref do
       client.local_branches(sort_by: 'updated_desc')
     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
       expect { client.local_branches(sort_by: 'invalid_sort') }.to raise_error(ArgumentError)
     end