summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorThong Kuah <tkuah@gitlab.com>2019-09-11 04:48:21 +0000
committerThong Kuah <tkuah@gitlab.com>2019-09-11 04:48:21 +0000
commit28292d516abb33aeaf7e5bfaf94679d1317bd284 (patch)
tree0acb8ae2d6904b8d779230df7a170f2cd0c2b6bf
parentbb7bbcf7b667197e71f696cc17d8c08677629e8d (diff)
parentf1926b321deb8b922dead991fb4d8bea42699f9f (diff)
downloadgitlab-ce-28292d516abb33aeaf7e5bfaf94679d1317bd284.tar.gz
Merge branch '65988-optimize-snippet-listings' into 'master'
Optimize queries for snippet listings See merge request gitlab-org/gitlab-ce!32576
-rw-r--r--app/controllers/concerns/issuable_collections.rb28
-rw-r--r--app/controllers/concerns/paginated_collection.rb19
-rw-r--r--app/controllers/dashboard/snippets_controller.rb17
-rw-r--r--app/controllers/dashboard/todos_controller.rb32
-rw-r--r--app/controllers/explore/snippets_controller.rb13
-rw-r--r--app/controllers/projects/snippets_controller.rb19
-rw-r--r--app/controllers/snippets_controller.rb10
-rw-r--r--app/controllers/users_controller.rb12
-rw-r--r--app/models/concerns/noteable.rb4
-rw-r--r--app/models/snippet.rb1
-rw-r--r--app/views/shared/snippets/_list.html.haml7
-rw-r--r--app/views/shared/snippets/_snippet.html.haml6
-rw-r--r--changelogs/unreleased/65988-optimize-snippet-listings.yml5
-rw-r--r--lib/gitlab/issuable_metadata.rb4
-rw-r--r--lib/gitlab/noteable_metadata.rb33
-rw-r--r--spec/controllers/dashboard/snippets_controller_spec.rb21
-rw-r--r--spec/controllers/dashboard/todos_controller_spec.rb24
-rw-r--r--spec/controllers/explore/snippets_controller_spec.rb15
-rw-r--r--spec/controllers/projects/issues_controller_spec.rb54
-rw-r--r--spec/controllers/projects/snippets_controller_spec.rb32
-rw-r--r--spec/controllers/snippets_controller_spec.rb9
-rw-r--r--spec/lib/gitlab/noteable_metadata_spec.rb29
-rw-r--r--spec/support/shared_examples/controllers/paginated_collection_shared_examples.rb30
23 files changed, 262 insertions, 162 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 161c22046f9..6feade3df03 100644
--- a/app/controllers/dashboard/snippets_controller.rb
+++ b/app/controllers/dashboard/snippets_controller.rb
@@ -1,14 +1,19 @@
# frozen_string_literal: true
class Dashboard::SnippetsController < Dashboard::ApplicationController
+ include PaginatedCollection
+ include Gitlab::NoteableMetadata
+
skip_cross_project_access_check :index
def index
- @snippets = SnippetsFinder.new(
- current_user,
- author: current_user,
- scope: params[:scope]
- ).execute
- @snippets = @snippets.page(params[:page])
+ @snippets = SnippetsFinder.new(current_user, author: current_user, scope: params[:scope])
+ .execute
+ .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 76ed142c939..d4c6aae2ca8 100644
--- a/app/controllers/explore/snippets_controller.rb
+++ b/app/controllers/explore/snippets_controller.rb
@@ -1,8 +1,17 @@
# frozen_string_literal: true
class Explore::SnippetsController < Explore::ApplicationController
+ include PaginatedCollection
+ include Gitlab::NoteableMetadata
+
def index
- @snippets = SnippetsFinder.new(current_user).execute
- @snippets = @snippets.page(params[:page])
+ @snippets = SnippetsFinder.new(current_user)
+ .execute
+ .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 59f948959d6..dbd11c8ddc8 100644
--- a/app/controllers/projects/snippets_controller.rb
+++ b/app/controllers/projects/snippets_controller.rb
@@ -6,6 +6,8 @@ class Projects::SnippetsController < Projects::ApplicationController
include SpammableActions
include SnippetsActions
include RendersBlob
+ include PaginatedCollection
+ include Gitlab::NoteableMetadata
skip_before_action :verify_authenticity_token,
if: -> { action_name == 'show' && js_request? }
@@ -28,15 +30,14 @@ class Projects::SnippetsController < Projects::ApplicationController
respond_to :html
def index
- @snippets = SnippetsFinder.new(
- current_user,
- project: @project,
- scope: params[:scope]
- ).execute
- @snippets = @snippets.page(params[:page])
- if @snippets.out_of_range? && @snippets.total_pages != 0
- redirect_to project_snippets_path(@project, page: @snippets.total_pages)
- end
+ @snippets = SnippetsFinder.new(current_user, project: @project, scope: params[:scope])
+ .execute
+ .page(params[:page])
+ .inc_author
+
+ return if redirect_out_of_range(@snippets)
+
+ @noteable_meta_data = noteable_meta_data(@snippets, 'Snippet')
end
def new
diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb
index 869655e9550..5805d068e21 100644
--- a/app/controllers/snippets_controller.rb
+++ b/app/controllers/snippets_controller.rb
@@ -7,6 +7,8 @@ class SnippetsController < ApplicationController
include SnippetsActions
include RendersBlob
include PreviewMarkdown
+ include PaginatedCollection
+ include Gitlab::NoteableMetadata
skip_before_action :verify_authenticity_token,
if: -> { action_name == 'show' && js_request? }
@@ -32,7 +34,13 @@ class SnippetsController < ApplicationController
@user = UserFinder.new(params[:username]).find_by_username!
@snippets = SnippetsFinder.new(current_user, author: @user, scope: params[:scope])
- .execute.page(params[:page])
+ .execute
+ .page(params[:page])
+ .inc_author
+
+ return if redirect_out_of_range(@snippets)
+
+ @noteable_meta_data = noteable_meta_data(@snippets, 'Snippet')
render 'index'
else
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 91e0efcf45f..e38d4073de3 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -4,6 +4,7 @@ class UsersController < ApplicationController
include RoutableActions
include RendersMemberAccess
include ControllerWithCrossProjectAccessCheck
+ include Gitlab::NoteableMetadata
requires_cross_project_access show: false,
groups: false,
@@ -165,11 +166,12 @@ class UsersController < ApplicationController
end
def load_snippets
- @snippets = SnippetsFinder.new(
- current_user,
- author: user,
- scope: params[:scope]
- ).execute.page(params[:page])
+ @snippets = SnippetsFinder.new(current_user, author: user, scope: params[:scope])
+ .execute
+ .page(params[:page])
+ .inc_author
+
+ @noteable_meta_data = noteable_meta_data(@snippets, 'Snippet')
end
def build_canonical_path(user)
diff --git a/app/models/concerns/noteable.rb b/app/models/concerns/noteable.rb
index 6a44bc7c401..b3e4df730b4 100644
--- a/app/models/concerns/noteable.rb
+++ b/app/models/concerns/noteable.rb
@@ -3,6 +3,10 @@
module Noteable
extend ActiveSupport::Concern
+ # This object is used to gather noteable meta data for list displays
+ # avoiding n+1 queries and improving performance.
+ NoteableMeta = Struct.new(:user_notes_count)
+
class_methods do
# `Noteable` class names that support replying to individual notes.
def replyable_types
diff --git a/app/models/snippet.rb b/app/models/snippet.rb
index 00931457344..b2fca65b9e0 100644
--- a/app/models/snippet.rb
+++ b/app/models/snippet.rb
@@ -55,6 +55,7 @@ class Snippet < ApplicationRecord
scope :are_public, -> { where(visibility_level: Snippet::PUBLIC) }
scope :public_and_internal, -> { where(visibility_level: [Snippet::PUBLIC, Snippet::INTERNAL]) }
scope :fresh, -> { order("created_at DESC") }
+ scope :inc_author, -> { includes(:author) }
scope :inc_relations_for_view, -> { includes(author: :status) }
participant :author
diff --git a/app/views/shared/snippets/_list.html.haml b/app/views/shared/snippets/_list.html.haml
index 5d2152eb411..766f48fff3d 100644
--- a/app/views/shared/snippets/_list.html.haml
+++ b/app/views/shared/snippets/_list.html.haml
@@ -1,12 +1,11 @@
- remote = local_assigns.fetch(:remote, false)
- link_project = local_assigns.fetch(:link_project, false)
-- if @snippets.exists?
+- if @snippets.to_a.empty?
+ .nothing-here-block= s_("SnippetsEmptyState|No snippets found")
+- else
.snippets-list-holder
%ul.content-list
= render partial: 'shared/snippets/snippet', collection: @snippets, locals: { link_project: link_project }
= paginate @snippets, theme: 'gitlab', remote: remote
-
-- else
- .nothing-here-block= s_("SnippetsEmptyState|No snippets found")
diff --git a/app/views/shared/snippets/_snippet.html.haml b/app/views/shared/snippets/_snippet.html.haml
index 42af97bc6af..0ef626868a2 100644
--- a/app/views/shared/snippets/_snippet.html.haml
+++ b/app/views/shared/snippets/_snippet.html.haml
@@ -1,4 +1,5 @@
- link_project = local_assigns.fetch(:link_project, false)
+- notes_count = @noteable_meta_data[snippet.id].user_notes_count
%li.snippet-row
= image_tag avatar_icon_for_user(snippet.author), class: "avatar s40 d-none d-sm-block", alt: ''
@@ -12,10 +13,9 @@
%ul.controls
%li
- - note_count = snippet.notes.user.count
- = link_to reliable_snippet_path(snippet, anchor: 'notes'), class: ('no-comments' if note_count.zero?) do
+ = link_to reliable_snippet_path(snippet, anchor: 'notes'), class: ('no-comments' if notes_count.zero?) do
= icon('comments')
- = note_count
+ = notes_count
%li
%span.sr-only
= visibility_level_label(snippet.visibility_level)
diff --git a/changelogs/unreleased/65988-optimize-snippet-listings.yml b/changelogs/unreleased/65988-optimize-snippet-listings.yml
new file mode 100644
index 00000000000..186a83cf9f3
--- /dev/null
+++ b/changelogs/unreleased/65988-optimize-snippet-listings.yml
@@ -0,0 +1,5 @@
+---
+title: Optimize queries for snippet listings
+merge_request: 32576
+author:
+type: performance
diff --git a/lib/gitlab/issuable_metadata.rb b/lib/gitlab/issuable_metadata.rb
index be73bcd5506..6f760751b0f 100644
--- a/lib/gitlab/issuable_metadata.rb
+++ b/lib/gitlab/issuable_metadata.rb
@@ -19,7 +19,7 @@ module Gitlab
return {} if issuable_ids.empty?
- issuable_note_count = ::Note.count_for_collection(issuable_ids, collection_type)
+ issuable_notes_count = ::Note.count_for_collection(issuable_ids, collection_type)
issuable_votes_count = ::AwardEmoji.votes_for_collection(issuable_ids, collection_type)
issuable_merge_requests_count =
if collection_type == 'Issue'
@@ -31,7 +31,7 @@ module Gitlab
issuable_ids.each_with_object({}) do |id, issuable_meta|
downvotes = issuable_votes_count.find { |votes| votes.awardable_id == id && votes.downvote? }
upvotes = issuable_votes_count.find { |votes| votes.awardable_id == id && votes.upvote? }
- notes = issuable_note_count.find { |notes| notes.noteable_id == id }
+ notes = issuable_notes_count.find { |notes| notes.noteable_id == id }
merge_requests = issuable_merge_requests_count.find { |mr| mr.first == id }
issuable_meta[id] = ::Issuable::IssuableMeta.new(
diff --git a/lib/gitlab/noteable_metadata.rb b/lib/gitlab/noteable_metadata.rb
new file mode 100644
index 00000000000..f3f8933b81f
--- /dev/null
+++ b/lib/gitlab/noteable_metadata.rb
@@ -0,0 +1,33 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module NoteableMetadata
+ def noteable_meta_data(noteable_collection, collection_type)
+ # ActiveRecord uses Object#extend for null relations.
+ if !(noteable_collection.singleton_class < ActiveRecord::NullRelation) &&
+ noteable_collection.respond_to?(:limit_value) &&
+ noteable_collection.limit_value.nil?
+
+ raise 'Collection must have a limit applied for preloading meta-data'
+ end
+
+ # map has to be used here since using pluck or select will
+ # throw an error when ordering noteables which inserts
+ # a new order into the collection.
+ # We cannot use reorder to not mess up the paginated collection.
+ noteable_ids = noteable_collection.map(&:id)
+
+ return {} if noteable_ids.empty?
+
+ noteable_notes_count = ::Note.count_for_collection(noteable_ids, collection_type)
+
+ noteable_ids.each_with_object({}) do |id, noteable_meta|
+ notes = noteable_notes_count.find { |notes| notes.noteable_id == id }
+
+ noteable_meta[id] = ::Noteable::NoteableMeta.new(
+ notes.try(:count).to_i
+ )
+ end
+ end
+ end
+end
diff --git a/spec/controllers/dashboard/snippets_controller_spec.rb b/spec/controllers/dashboard/snippets_controller_spec.rb
new file mode 100644
index 00000000000..2d839094d34
--- /dev/null
+++ b/spec/controllers/dashboard/snippets_controller_spec.rb
@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Dashboard::SnippetsController do
+ let(:user) { create(:user) }
+
+ before do
+ sign_in(user)
+ end
+
+ describe 'GET #index' do
+ it_behaves_like 'paginated collection' do
+ let(:collection) { Snippet.all }
+
+ before do
+ create(:personal_snippet, :public, author: user)
+ end
+ end
+ end
+end
diff --git a/spec/controllers/dashboard/todos_controller_spec.rb b/spec/controllers/dashboard/todos_controller_spec.rb
index 9a3fbfaac51..c5af04f72ee 100644
--- a/spec/controllers/dashboard/todos_controller_spec.rb
+++ b/spec/controllers/dashboard/todos_controller_spec.rb
@@ -82,35 +82,15 @@ describe Dashboard::TodosController do
end
end
- context 'when using pagination' do
- let(:last_page) { user.todos.page.total_pages }
+ it_behaves_like 'paginated collection' do
let!(:issues) { create_list(:issue, 3, project: project, assignees: [user]) }
+ let(:collection) { user.todos }
before do
issues.each { |issue| todo_service.new_issue(issue, user) }
allow(Kaminari.config).to receive(:default_per_page).and_return(2)
end
- it 'redirects to last_page if page number is larger than number of pages' do
- get :index, params: { page: (last_page + 1).to_param }
-
- expect(response).to redirect_to(dashboard_todos_path(page: last_page))
- end
-
- it 'goes to the correct page' do
- get :index, params: { page: last_page }
-
- expect(assigns(:todos).current_page).to eq(last_page)
- expect(response).to have_gitlab_http_status(200)
- end
-
- it 'does not redirect to external sites when provided a host field' do
- external_host = "www.example.com"
- get :index, params: { page: (last_page + 1).to_param, host: external_host }
-
- expect(response).to redirect_to(dashboard_todos_path(page: last_page))
- end
-
context 'when providing no filters' do
it 'does not perform a query to get the page count, but gets that from the user' do
allow(controller).to receive(:current_user).and_return(user)
diff --git a/spec/controllers/explore/snippets_controller_spec.rb b/spec/controllers/explore/snippets_controller_spec.rb
new file mode 100644
index 00000000000..fa659c6df7f
--- /dev/null
+++ b/spec/controllers/explore/snippets_controller_spec.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Explore::SnippetsController do
+ describe 'GET #index' do
+ it_behaves_like 'paginated collection' do
+ let(:collection) { Snippet.all }
+
+ before do
+ create(:personal_snippet, :public)
+ end
+ end
+ end
+end
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 608131dcbc8..367bd641f5d 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -71,9 +71,16 @@ describe Projects::IssuesController do
end
end
- context 'with page param' do
- let(:last_page) { project.issues.page.total_pages }
+ it_behaves_like 'paginated collection' do
let!(:issue_list) { create_list(:issue, 2, project: project) }
+ let(:collection) { project.issues }
+ let(:params) do
+ {
+ namespace_id: project.namespace.to_param,
+ project_id: project,
+ state: 'opened'
+ }
+ end
before do
sign_in(user)
@@ -81,51 +88,10 @@ describe Projects::IssuesController do
allow(Kaminari.config).to receive(:default_per_page).and_return(1)
end
- it 'redirects to last_page if page number is larger than number of pages' do
- get :index,
- params: {
- namespace_id: project.namespace.to_param,
- project_id: project,
- page: (last_page + 1).to_param
- }
-
- expect(response).to redirect_to(namespace_project_issues_path(page: last_page, state: controller.params[:state], scope: controller.params[:scope]))
- end
-
- it 'redirects to specified page' do
- get :index,
- params: {
- namespace_id: project.namespace.to_param,
- project_id: project,
- page: last_page.to_param
- }
-
- expect(assigns(:issues).current_page).to eq(last_page)
- expect(response).to have_gitlab_http_status(200)
- end
-
- it 'does not redirect to external sites when provided a host field' do
- external_host = "www.example.com"
- get :index,
- params: {
- namespace_id: project.namespace.to_param,
- project_id: project,
- page: (last_page + 1).to_param,
- host: external_host
- }
-
- expect(response).to redirect_to(namespace_project_issues_path(page: last_page, state: controller.params[:state], scope: controller.params[:scope]))
- end
-
it 'does not use pagination if disabled' do
allow(controller).to receive(:pagination_disabled?).and_return(true)
- get :index,
- params: {
- namespace_id: project.namespace.to_param,
- project_id: project,
- page: (last_page + 1).to_param
- }
+ get :index, params: params.merge(page: last_page + 1)
expect(response).to have_gitlab_http_status(200)
expect(assigns(:issues).size).to eq(2)
diff --git a/spec/controllers/projects/snippets_controller_spec.rb b/spec/controllers/projects/snippets_controller_spec.rb
index 9b5d7317c11..b13534b9088 100644
--- a/spec/controllers/projects/snippets_controller_spec.rb
+++ b/spec/controllers/projects/snippets_controller_spec.rb
@@ -13,31 +13,17 @@ describe Projects::SnippetsController do
end
describe 'GET #index' do
- context 'when page param' do
- let(:last_page) { project.snippets.page.total_pages }
- let!(:project_snippet) { create(:project_snippet, :public, project: project, author: user) }
-
- it 'redirects to last_page if page number is larger than number of pages' do
- get :index,
- params: {
- namespace_id: project.namespace,
- project_id: project,
- page: (last_page + 1).to_param
- }
-
- expect(response).to redirect_to(namespace_project_snippets_path(page: last_page))
+ it_behaves_like 'paginated collection' do
+ let(:collection) { project.snippets }
+ let(:params) do
+ {
+ namespace_id: project.namespace,
+ project_id: project
+ }
end
- it 'redirects to specified page' do
- get :index,
- params: {
- namespace_id: project.namespace,
- project_id: project,
- page: last_page.to_param
- }
-
- expect(assigns(:snippets).current_page).to eq(last_page)
- expect(response).to have_gitlab_http_status(200)
+ before do
+ create(:project_snippet, :public, project: project, author: user)
end
end
diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb
index b0092bc8994..1b3a8965342 100644
--- a/spec/controllers/snippets_controller_spec.rb
+++ b/spec/controllers/snippets_controller_spec.rb
@@ -9,6 +9,15 @@ describe SnippetsController do
let(:user) { create(:user) }
context 'when username parameter is present' do
+ it_behaves_like 'paginated collection' do
+ let(:collection) { Snippet.all }
+ let(:params) { { username: user.username } }
+
+ before do
+ create(:personal_snippet, :public, author: user)
+ end
+ end
+
it 'renders snippets of a user when username is present' do
get :index, params: { username: user.username }
diff --git a/spec/lib/gitlab/noteable_metadata_spec.rb b/spec/lib/gitlab/noteable_metadata_spec.rb
new file mode 100644
index 00000000000..b12a1825f04
--- /dev/null
+++ b/spec/lib/gitlab/noteable_metadata_spec.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::NoteableMetadata do
+ subject { Class.new { include Gitlab::NoteableMetadata }.new }
+
+ it 'returns an empty Hash if an empty collection is provided' do
+ expect(subject.noteable_meta_data(Snippet.none, 'Snippet')).to eq({})
+ end
+
+ it 'raises an error when given a collection with no limit' do
+ expect { subject.noteable_meta_data(Snippet.all, 'Snippet') }.to raise_error(/must have a limit/)
+ end
+
+ context 'snippets' do
+ let!(:snippet) { create(:personal_snippet) }
+ let!(:other_snippet) { create(:personal_snippet) }
+ let!(:note) { create(:note, noteable: snippet) }
+
+ it 'aggregates stats on snippets' do
+ data = subject.noteable_meta_data(Snippet.all.limit(10), 'Snippet')
+
+ expect(data.count).to eq(2)
+ expect(data[snippet.id].user_notes_count).to eq(1)
+ expect(data[other_snippet.id].user_notes_count).to eq(0)
+ end
+ end
+end
diff --git a/spec/support/shared_examples/controllers/paginated_collection_shared_examples.rb b/spec/support/shared_examples/controllers/paginated_collection_shared_examples.rb
new file mode 100644
index 00000000000..bd84bd1093f
--- /dev/null
+++ b/spec/support/shared_examples/controllers/paginated_collection_shared_examples.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+shared_examples 'paginated collection' do
+ let(:collection) { nil }
+ let(:last_page) { collection.page.total_pages }
+ let(:action) { :index }
+ let(:params) { {} }
+
+ it 'renders a page number that is not ouf of range' do
+ get action, params: params.merge(page: last_page)
+
+ expect(response).to have_gitlab_http_status(200)
+ end
+
+ it 'redirects to last_page if page number is larger than number of pages' do
+ get action, params: params.merge(page: last_page + 1)
+
+ expect(response).to redirect_to(params.merge(page: last_page))
+ end
+
+ it 'does not redirect to external sites when provided a host field' do
+ external_host = 'www.example.com'
+
+ get action, params: params.merge(page: last_page + 1, host: external_host)
+
+ expect(response).to redirect_to(params.merge(page: last_page))
+ end
+end