From c1f9403e45e636651010929b6113add34f8e6a8a Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 10 Aug 2017 15:01:38 +0200 Subject: [PATCH] Use Prev/Next pagination for exploring projects This changes the pagination of the "Explore" pages so they use a simpler pagination system that only shows "Prev" and "Next" buttons. This removes the need for getting the total number of rows to display, a process that can easily take up to 2 seconds when browsing through a large list of projects. Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/27390 --- Gemfile | 2 +- Gemfile.lock | 17 ++++++++++---- .../explore/projects_controller.rb | 11 +++++---- app/helpers/pagination_helper.rb | 21 +++++++++++++++++ .../kaminari/gitlab/_without_count.html.haml | 8 +++++++ app/views/shared/projects/_list.html.haml | 2 +- .../pagination-projects-explore.yml | 4 ++++ spec/helpers/pagination_helper_spec.rb | 23 +++++++++++++++++++ 8 files changed, 78 insertions(+), 10 deletions(-) create mode 100644 app/helpers/pagination_helper.rb create mode 100644 app/views/kaminari/gitlab/_without_count.html.haml create mode 100644 changelogs/unreleased/pagination-projects-explore.yml create mode 100644 spec/helpers/pagination_helper_spec.rb diff --git a/Gemfile b/Gemfile index a768fa428bf..93363edf66e 100644 --- a/Gemfile +++ b/Gemfile @@ -84,7 +84,7 @@ gem 'rack-cors', '~> 0.4.0', require: 'rack/cors' gem 'hashie-forbidden_attributes' # Pagination -gem 'kaminari', '~> 0.17.0' +gem 'kaminari', '~> 1.0' # HAML gem 'hamlit', '~> 2.6.1' diff --git a/Gemfile.lock b/Gemfile.lock index d443209349d..3e661d4e3ad 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -419,9 +419,18 @@ GEM json-schema (2.6.2) addressable (~> 2.3.8) jwt (1.5.6) - kaminari (0.17.0) - actionpack (>= 3.0.0) - activesupport (>= 3.0.0) + kaminari (1.0.1) + activesupport (>= 4.1.0) + kaminari-actionview (= 1.0.1) + kaminari-activerecord (= 1.0.1) + kaminari-core (= 1.0.1) + kaminari-actionview (1.0.1) + actionview + kaminari-core (= 1.0.1) + kaminari-activerecord (1.0.1) + activerecord + kaminari-core (= 1.0.1) + kaminari-core (1.0.1) kgio (2.10.0) knapsack (1.11.0) rake @@ -1011,7 +1020,7 @@ DEPENDENCIES jquery-rails (~> 4.1.0) json-schema (~> 2.6.2) jwt (~> 1.5.6) - kaminari (~> 0.17.0) + kaminari (~> 1.0) knapsack (~> 1.11.0) kubeclient (~> 2.2.0) letter_opener_web (~> 1.3.0) diff --git a/app/controllers/explore/projects_controller.rb b/app/controllers/explore/projects_controller.rb index 741879dee35..762c6ebf3a3 100644 --- a/app/controllers/explore/projects_controller.rb +++ b/app/controllers/explore/projects_controller.rb @@ -6,7 +6,7 @@ class Explore::ProjectsController < Explore::ApplicationController def index params[:sort] ||= 'latest_activity_desc' @sort = params[:sort] - @projects = load_projects.page(params[:page]) + @projects = load_projects respond_to do |format| format.html @@ -21,7 +21,7 @@ class Explore::ProjectsController < Explore::ApplicationController def trending params[:trending] = true @sort = params[:sort] - @projects = load_projects.page(params[:page]) + @projects = load_projects respond_to do |format| format.html @@ -34,7 +34,7 @@ class Explore::ProjectsController < Explore::ApplicationController end def starred - @projects = load_projects.reorder('star_count DESC').page(params[:page]) + @projects = load_projects.reorder('star_count DESC') respond_to do |format| format.html @@ -50,6 +50,9 @@ class Explore::ProjectsController < Explore::ApplicationController def load_projects ProjectsFinder.new(current_user: current_user, params: params) - .execute.includes(:route, namespace: :route) + .execute + .includes(:route, namespace: :route) + .page(params[:page]) + .without_count end end diff --git a/app/helpers/pagination_helper.rb b/app/helpers/pagination_helper.rb new file mode 100644 index 00000000000..83dd76a01dd --- /dev/null +++ b/app/helpers/pagination_helper.rb @@ -0,0 +1,21 @@ +module PaginationHelper + def paginate_collection(collection, remote: nil) + if collection.is_a?(Kaminari::PaginatableWithoutCount) + paginate_without_count(collection) + elsif collection.respond_to?(:total_pages) + paginate_with_count(collection, remote: remote) + end + end + + def paginate_without_count(collection) + render( + 'kaminari/gitlab/without_count', + previous_path: path_to_prev_page(collection), + next_path: path_to_next_page(collection) + ) + end + + def paginate_with_count(collection, remote: nil) + paginate(collection, remote: remote, theme: 'gitlab') + end +end diff --git a/app/views/kaminari/gitlab/_without_count.html.haml b/app/views/kaminari/gitlab/_without_count.html.haml new file mode 100644 index 00000000000..250029c4475 --- /dev/null +++ b/app/views/kaminari/gitlab/_without_count.html.haml @@ -0,0 +1,8 @@ +.gl-pagination + %ul.pagination.clearfix + - if previous_path + %li.prev + = link_to(t('views.pagination.previous'), previous_path, rel: 'prev') + - if next_path + %li.next + = link_to(t('views.pagination.next'), next_path, rel: 'next') diff --git a/app/views/shared/projects/_list.html.haml b/app/views/shared/projects/_list.html.haml index 914506bf0ce..0bedfea3502 100644 --- a/app/views/shared/projects/_list.html.haml +++ b/app/views/shared/projects/_list.html.haml @@ -23,6 +23,6 @@ = icon('lock fw', base: 'circle', class: 'fa-lg private-fork-icon') %strong= pluralize(@private_forks_count, 'private fork') %span  you have no access to. - = paginate(projects, remote: remote, theme: "gitlab") if projects.respond_to? :total_pages + = paginate_collection(projects, remote: remote) - else .nothing-here-block No projects found diff --git a/changelogs/unreleased/pagination-projects-explore.yml b/changelogs/unreleased/pagination-projects-explore.yml new file mode 100644 index 00000000000..dc9c4218793 --- /dev/null +++ b/changelogs/unreleased/pagination-projects-explore.yml @@ -0,0 +1,4 @@ +--- +title: Use Prev/Next pagination for exploring projects +merge_request: +author: diff --git a/spec/helpers/pagination_helper_spec.rb b/spec/helpers/pagination_helper_spec.rb new file mode 100644 index 00000000000..e235475fb47 --- /dev/null +++ b/spec/helpers/pagination_helper_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe PaginationHelper do + describe '#paginate_collection' do + let(:collection) { User.all.page(1) } + + it 'paginates a collection without using a COUNT' do + without_count = collection.without_count + + expect(helper).to receive(:paginate_without_count) + .with(without_count) + .and_call_original + + helper.paginate_collection(without_count) + end + + it 'paginates a collection using a COUNT' do + expect(helper).to receive(:paginate_with_count).and_call_original + + helper.paginate_collection(collection) + end + end +end