Add controller concern for paginated collections65988-optimize-snippet-listings

We had similar code in a few places to redirect to the last page if
the given page number is out of range. This unifies the handling in a
new controller concern and adds usage of it in all snippet listings.

7 files changed, 49 insertions, 44 deletions

diff --git a/app/controllers/concerns/issuable_collections.rb b/app/controllers/concerns/issuable_collections.rb
index 8ea77b994de..88044cf7557 100644
--- a/app/controllers/concerns/issuable_collections.rb
+++ b/app/controllers/concerns/issuable_collections.rb
@@ -2,6 +2,7 @@
 
 module IssuableCollections
   extend ActiveSupport::Concern
+  include PaginatedCollection
   include SortingHelper
   include SortingPreference
   include Gitlab::IssuableMetadata
@@ -17,8 +18,11 @@ module IssuableCollections
   def set_issuables_index
     @issuables = issuables_collection
 
-    set_pagination
-    return if redirect_out_of_range(@total_pages)
+    unless pagination_disabled?
+      set_pagination
+
+      return if redirect_out_of_range(@issuables, @total_pages)
+    end
 
     if params[:label_name].present? && @project
       labels_params = { project_id: @project.id, title: params[:label_name] }
@@ -38,12 +42,10 @@ module IssuableCollections
   end
 
   def set_pagination
-    return if pagination_disabled?
-
     @issuables          = @issuables.page(params[:page])
     @issuables          = per_page_for_relative_position if params[:sort] == 'relative_position'
     @issuable_meta_data = issuable_meta_data(@issuables, collection_type, current_user)
-    @total_pages        = issuable_page_count
+    @total_pages        = issuable_page_count(@issuables)
   end
   # rubocop:enable Gitlab/ModuleWithInstanceVariables
 
@@ -57,20 +59,8 @@ module IssuableCollections
   end
   # rubocop: enable CodeReuse/ActiveRecord
 
-  def redirect_out_of_range(total_pages)
-    return false if total_pages.nil? || total_pages.zero?
-
-    out_of_range = @issuables.current_page > total_pages # rubocop:disable Gitlab/ModuleWithInstanceVariables
-
-    if out_of_range
-      redirect_to(url_for(safe_params.merge(page: total_pages, only_path: true)))
-    end
-
-    out_of_range
-  end
-
-  def issuable_page_count
-    page_count_for_relation(@issuables, finder.row_count) # rubocop:disable Gitlab/ModuleWithInstanceVariables
+  def issuable_page_count(relation)
+    page_count_for_relation(relation, finder.row_count)
   end
 
   def page_count_for_relation(relation, row_count)
diff --git a/app/controllers/concerns/paginated_collection.rb b/app/controllers/concerns/paginated_collection.rb
new file mode 100644
index 00000000000..be84215a9e2
--- /dev/null
+++ b/app/controllers/concerns/paginated_collection.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+module PaginatedCollection
+  extend ActiveSupport::Concern
+
+  private
+
+  def redirect_out_of_range(collection, total_pages = collection.total_pages)
+    return false if total_pages.zero?
+
+    out_of_range = collection.current_page > total_pages
+
+    if out_of_range
+      redirect_to(url_for(safe_params.merge(page: total_pages, only_path: true)))
+    end
+
+    out_of_range
+  end
+end
diff --git a/app/controllers/dashboard/snippets_controller.rb b/app/controllers/dashboard/snippets_controller.rb
index 382f54c6c00..6feade3df03 100644
--- a/app/controllers/dashboard/snippets_controller.rb
+++ b/app/controllers/dashboard/snippets_controller.rb
@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 class Dashboard::SnippetsController < Dashboard::ApplicationController
+  include PaginatedCollection
   include Gitlab::NoteableMetadata
 
   skip_cross_project_access_check :index
@@ -11,6 +12,8 @@ class Dashboard::SnippetsController < Dashboard::ApplicationController
       .page(params[:page])
       .inc_author
 
+    return if redirect_out_of_range(@snippets)
+
     @noteable_meta_data = noteable_meta_data(@snippets, 'Snippet')
   end
 end
diff --git a/app/controllers/dashboard/todos_controller.rb b/app/controllers/dashboard/todos_controller.rb
index 8f6fcb362d2..940d1482611 100644
--- a/app/controllers/dashboard/todos_controller.rb
+++ b/app/controllers/dashboard/todos_controller.rb
@@ -2,6 +2,7 @@
 
 class Dashboard::TodosController < Dashboard::ApplicationController
   include ActionView::Helpers::NumberHelper
+  include PaginatedCollection
 
   before_action :authorize_read_project!, only: :index
   before_action :authorize_read_group!, only: :index
@@ -12,7 +13,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController
     @todos = @todos.page(params[:page])
     @todos = @todos.with_entity_associations
 
-    return if redirect_out_of_range(@todos)
+    return if redirect_out_of_range(@todos, todos_page_count(@todos))
   end
 
   def destroy
@@ -82,28 +83,15 @@ class Dashboard::TodosController < Dashboard::ApplicationController
     }
   end
 
-  def todo_params
-    params.permit(:action_id, :author_id, :project_id, :type, :sort, :state, :group_id)
-  end
-
-  # rubocop: disable CodeReuse/ActiveRecord
-  def redirect_out_of_range(todos)
-    total_pages =
-      if todo_params.except(:sort, :page).empty?
-        (current_user.todos_pending_count.to_f / todos.limit_value).ceil
-      else
-        todos.total_pages
-      end
-
-    return false if total_pages.zero?
-
-    out_of_range = todos.current_page > total_pages
-
-    if out_of_range
-      redirect_to url_for(safe_params.merge(page: total_pages, only_path: true))
+  def todos_page_count(todos)
+    if todo_params.except(:sort, :page).empty? # rubocop: disable CodeReuse/ActiveRecord
+      (current_user.todos_pending_count.to_f / todos.limit_value).ceil
+    else
+      todos.total_pages
     end
+  end
 
-    out_of_range
+  def todo_params
+    params.permit(:action_id, :author_id, :project_id, :type, :sort, :state, :group_id)
   end
-  # rubocop: enable CodeReuse/ActiveRecord
 end
diff --git a/app/controllers/explore/snippets_controller.rb b/app/controllers/explore/snippets_controller.rb
index 6192b594593..d4c6aae2ca8 100644
--- a/app/controllers/explore/snippets_controller.rb
+++ b/app/controllers/explore/snippets_controller.rb
@@ -1,6 +1,7 @@
 # frozen_string_literal: true
 
 class Explore::SnippetsController < Explore::ApplicationController
+  include PaginatedCollection
   include Gitlab::NoteableMetadata
 
   def index
@@ -9,6 +10,8 @@ class Explore::SnippetsController < Explore::ApplicationController
       .page(params[:page])
       .inc_author
 
+    return if redirect_out_of_range(@snippets)
+
     @noteable_meta_data = noteable_meta_data(@snippets, 'Snippet')
   end
 end
diff --git a/app/controllers/projects/snippets_controller.rb b/app/controllers/projects/snippets_controller.rb
index c84e9fbc2d7..dbd11c8ddc8 100644
--- a/app/controllers/projects/snippets_controller.rb
+++ b/app/controllers/projects/snippets_controller.rb
@@ -6,6 +6,7 @@ class Projects::SnippetsController < Projects::ApplicationController
   include SpammableActions
   include SnippetsActions
   include RendersBlob
+  include PaginatedCollection
   include Gitlab::NoteableMetadata
 
   skip_before_action :verify_authenticity_token,
@@ -34,9 +35,7 @@ class Projects::SnippetsController < Projects::ApplicationController
       .page(params[:page])
       .inc_author
 
-    if @snippets.out_of_range? && @snippets.total_pages != 0
-      return redirect_to project_snippets_path(@project, page: @snippets.total_pages)
-    end
+    return if redirect_out_of_range(@snippets)
 
     @noteable_meta_data = noteable_meta_data(@snippets, 'Snippet')
   end
diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb
index c1e05b72b37..5805d068e21 100644
--- a/app/controllers/snippets_controller.rb
+++ b/app/controllers/snippets_controller.rb
@@ -7,6 +7,7 @@ class SnippetsController < ApplicationController
   include SnippetsActions
   include RendersBlob
   include PreviewMarkdown
+  include PaginatedCollection
   include Gitlab::NoteableMetadata
 
   skip_before_action :verify_authenticity_token,
@@ -37,6 +38,8 @@ class SnippetsController < ApplicationController
         .page(params[:page])
         .inc_author
 
+      return if redirect_out_of_range(@snippets)
+
       @noteable_meta_data = noteable_meta_data(@snippets, 'Snippet')
 
       render 'index'