Merge branch 'fj-default-order-snippet-lists' into 'master'

Reorder snippets in lists using `updated_at` column

See merge request gitlab-org/gitlab!34393

app/controllers/concerns/snippets_actions.rb

@@ -8,6 +8,7 @@ module SnippetsActions
   include PaginatedCollection
   include Gitlab::NoteableMetadata
   include Snippets::SendBlob
+  include SnippetsSort
 
   included do
     skip_before_action :verify_authenticity_token,

app/controllers/concerns/snippets_sort.rb

+# frozen_string_literal: true
+
+module SnippetsSort
+  extend ActiveSupport::Concern
+
+  def sort_param
+    params[:sort].presence || 'updated_desc'
+  end
+end

app/controllers/dashboard/snippets_controller.rb

@@ -3,6 +3,7 @@
 class Dashboard::SnippetsController < Dashboard::ApplicationController
   include PaginatedCollection
   include Gitlab::NoteableMetadata
+  include SnippetsSort
 
   skip_cross_project_access_check :index
 
@@ -11,7 +12,7 @@ class Dashboard::SnippetsController < Dashboard::ApplicationController
       .new(current_user, author: current_user)
       .execute
 
-    @snippets = SnippetsFinder.new(current_user, author: current_user, scope: params[:scope])
+    @snippets = SnippetsFinder.new(current_user, author: current_user, scope: params[:scope], sort: sort_param)
       .execute
       .page(params[:page])
       .inc_author

app/controllers/projects/snippets_controller.rb

@@ -19,7 +19,7 @@ class Projects::SnippetsController < Projects::Snippets::ApplicationController
       .new(current_user, project: @project)
       .execute
 
-    @snippets = SnippetsFinder.new(current_user, project: @project, scope: params[:scope])
+    @snippets = SnippetsFinder.new(current_user, project: @project, scope: params[:scope], sort: sort_param)
       .execute
       .page(params[:page])
       .inc_author

app/controllers/snippets_controller.rb

@@ -21,7 +21,7 @@ class SnippetsController < Snippets::ApplicationController
     if params[:username].present?
       @user = UserFinder.new(params[:username]).find_by_username!
 
-      @snippets = SnippetsFinder.new(current_user, author: @user, scope: params[:scope])
+      @snippets = SnippetsFinder.new(current_user, author: @user, scope: params[:scope], sort: sort_param)
         .execute
         .page(params[:page])
         .inc_author

app/finders/snippets_finder.rb

@@ -43,7 +43,7 @@ class SnippetsFinder < UnionFinder
   include Gitlab::Utils::StrongMemoize
 
   attr_accessor :current_user, :params
-  delegate :explore, :only_personal, :only_project, :scope, to: :params
+  delegate :explore, :only_personal, :only_project, :scope, :sort, to: :params
 
   def initialize(current_user = nil, params = {})
     @current_user = current_user
@@ -69,7 +69,9 @@ class SnippetsFinder < UnionFinder
 
     items = init_collection
     items = by_ids(items)
-    items.with_optional_visibility(visibility_from_scope).fresh
+    items = items.with_optional_visibility(visibility_from_scope)
+
+    items.order_by(sort_param)
   end
 
   private
@@ -202,6 +204,10 @@ class SnippetsFinder < UnionFinder
       params[:project].is_a?(Project) ? params[:project] : Project.find_by_id(params[:project])
     end
   end
+
+  def sort_param
+    sort.presence || 'id_desc'
+  end
 end
 
 SnippetsFinder.prepend_if_ee('EE::SnippetsFinder')

changelogs/unreleased/fj-default-order-snippet-lists.yml

+---
+title: Reorder snippets in lists using `updated_at` column
+merge_request: 34393
+author: Dibyadarshi Dash @ddash2
+type: changed

spec/controllers/dashboard/snippets_controller_spec.rb

@@ -3,7 +3,7 @@
 require 'spec_helper'
 
 RSpec.describe Dashboard::SnippetsController do
-  let(:user) { create(:user) }
+  let_it_be(:user) { create(:user) }
 
   before do
     sign_in(user)
@@ -26,5 +26,7 @@ RSpec.describe Dashboard::SnippetsController do
 
       get :index
     end
+
+    it_behaves_like 'snippets sort order'
   end
 end

spec/controllers/projects/snippets_controller_spec.rb

@@ -15,14 +15,18 @@ RSpec.describe Projects::SnippetsController do
   end
 
   describe 'GET #index' do
+    let(:base_params) do
+      {
+        namespace_id: project.namespace,
+        project_id: project
+      }
+    end
+
+    subject { get :index, params: base_params }
+
     it_behaves_like 'paginated collection' do
       let(:collection) { project.snippets }
-      let(:params) do
-        {
-          namespace_id: project.namespace,
-          project_id: project
-        }
-      end
+      let(:params) { base_params }
 
       before do
         create(:project_snippet, :public, project: project, author: user)
@@ -35,7 +39,11 @@ RSpec.describe Projects::SnippetsController do
         .to receive(:new).with(nil, project: project)
         .and_return(service)
 
-      get :index, params: { namespace_id: project.namespace, project_id: project }
+      subject
+    end
+
+    it_behaves_like 'snippets sort order' do
+      let(:params) { base_params }
     end
 
     context 'when the project snippet is private' do
@@ -43,7 +51,7 @@ RSpec.describe Projects::SnippetsController do
 
       context 'when anonymous' do
         it 'does not include the private snippet' do
-          get :index, params: { namespace_id: project.namespace, project_id: project }
+          subject
 
           expect(assigns(:snippets)).not_to include(project_snippet)
           expect(response).to have_gitlab_http_status(:ok)
@@ -56,7 +64,7 @@ RSpec.describe Projects::SnippetsController do
         end
 
         it 'renders the snippet' do
-          get :index, params: { namespace_id: project.namespace, project_id: project }
+          subject
 
           expect(assigns(:snippets)).to include(project_snippet)
           expect(response).to have_gitlab_http_status(:ok)
@@ -69,7 +77,7 @@ RSpec.describe Projects::SnippetsController do
         end
 
         it 'renders the snippet' do
-          get :index, params: { namespace_id: project.namespace, project_id: project }
+          subject
 
           expect(assigns(:snippets)).to include(project_snippet)
           expect(response).to have_gitlab_http_status(:ok)

spec/controllers/snippets_controller_spec.rb

@@ -6,6 +6,8 @@ RSpec.describe SnippetsController do
   let_it_be(:user) { create(:user) }
 
   describe 'GET #index' do
+    let(:base_params) { { username: user.username } }
+
     context 'when username parameter is present' do
       it_behaves_like 'paginated collection' do
         let(:collection) { Snippet.all }
@@ -38,6 +40,10 @@ RSpec.describe SnippetsController do
         expect(response).to redirect_to(dashboard_snippets_path)
       end
     end
+
+    it_behaves_like 'snippets sort order' do
+      let(:params) { base_params }
+    end
   end
 
   describe 'GET #new' do

spec/finders/snippets_finder_spec.rb

@@ -295,6 +295,22 @@ RSpec.describe SnippetsFinder do
         expect(finder.execute).to be_empty
       end
     end
+
+    context 'no sort param is provided' do
+      it 'returns snippets sorted by id' do
+        snippets = described_class.new(admin).execute
+
+        expect(snippets.ids).to eq(Snippet.order_id_desc.ids)
+      end
+    end
+
+    context 'sort param is provided' do
+      it 'returns snippets sorted by sort param' do
+        snippets = described_class.new(admin, sort: 'updated_desc').execute
+
+        expect(snippets.ids).to eq(Snippet.order_updated_desc.ids)
+      end
+    end
   end
 
   it_behaves_like 'snippet visibility'

spec/support/shared_examples/controllers/snippets_sort_order_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'snippets sort order' do
+  let(:params) { {} }
+  let(:sort_argument) { {} }
+  let(:sort_params) { params.merge(sort_argument)}
+
+  before do
+    sign_in(user)
+
+    stub_snippet_counter
+  end
+
+  subject { get :index, params: sort_params }
+
+  context 'when no sort param is provided' do
+    it 'calls SnippetsFinder with updated_at sort option' do
+      expect(SnippetsFinder).to receive(:new).with(user,
+          hash_including(sort: 'updated_desc')).and_call_original
+
+      subject
+    end
+  end
+
+  context 'when sort param is provided' do
+    let(:order) { 'created_desc' }
+    let(:sort_argument) { { sort: order } }
+
+    it 'calls SnippetsFinder with the given sort param' do
+      expect(SnippetsFinder).to receive(:new).with(user,
+          hash_including(sort: order)).and_call_original
+
+      subject
+    end
+  end
+
+  def stub_snippet_counter
+    allow(Snippets::CountService)
+      .to receive(:new).and_return(double(:count_service, execute: {}))
+  end
+end