summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/helpers/dashboard_helper.rb13
-rw-r--r--app/views/admin/dashboard/index.html.haml37
-rw-r--r--changelogs/unreleased/24082-double-escaping-in-tableflip-quick-action.yml5
-rw-r--r--changelogs/unreleased/dz-improve-admin-features.yml5
-rw-r--r--doc/api/audit_events.md2
-rw-r--r--lib/api/helpers/pagination.rb249
-rw-r--r--lib/gitlab/pagination/base.rb32
-rw-r--r--lib/gitlab/pagination/offset_pagination.rb77
-rw-r--r--lib/gitlab/quick_actions/issuable_actions.rb2
-rw-r--r--lib/gitlab/serializer/pagination.rb5
-rw-r--r--locale/gitlab.pot10
-rw-r--r--spec/helpers/dashboard_helper_spec.rb61
-rw-r--r--spec/lib/api/helpers/pagination_spec.rb399
-rw-r--r--spec/lib/gitlab/pagination/offset_pagination_spec.rb215
14 files changed, 439 insertions, 673 deletions
diff --git a/app/helpers/dashboard_helper.rb b/app/helpers/dashboard_helper.rb
index 518cb7c9714..679622897aa 100644
--- a/app/helpers/dashboard_helper.rb
+++ b/app/helpers/dashboard_helper.rb
@@ -27,16 +27,25 @@ module DashboardHelper
false
end
- def feature_entry(title, href: nil, enabled: true)
+ def feature_entry(title, href: nil, enabled: true, doc_href: nil)
enabled_text = enabled ? 'on' : 'off'
label = "#{title}: status #{enabled_text}"
link_or_title = href && enabled ? tag.a(title, href: href) : title
tag.p(aria: { label: label }) do
concat(link_or_title)
+
concat(tag.span(class: ['light', 'float-right']) do
- concat(boolean_to_icon(enabled))
+ boolean_to_icon(enabled)
end)
+
+ if doc_href.present?
+ link_to_doc = link_to(sprite_icon('question', size: 16), doc_href,
+ class: 'prepend-left-5', title: _('Documentation'),
+ target: '_blank', rel: 'noopener noreferrer')
+
+ concat(link_to_doc)
+ end
end
end
diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml
index 41147950c40..e5a3c0df9bf 100644
--- a/app/views/admin/dashboard/index.html.haml
+++ b/app/views/admin/dashboard/index.html.haml
@@ -41,17 +41,38 @@
.info-well
.well-segment.admin-well.admin-well-features
%h4 Features
- = feature_entry(_('Sign up'), href: admin_application_settings_path(anchor: 'js-signup-settings'), enabled: allow_signup?)
- = feature_entry(_('LDAP'), enabled: Gitlab.config.ldap.enabled)
- = feature_entry(_('Gravatar'), href: admin_application_settings_path(anchor: 'js-account-settings'), enabled: gravatar_enabled?)
- = feature_entry(_('OmniAuth'), href: admin_application_settings_path(anchor: 'js-signin-settings'), enabled: Gitlab::Auth.omniauth_enabled?)
- = feature_entry(_('Reply by email'), enabled: Gitlab::IncomingEmail.enabled?)
+ = feature_entry(_('Sign up'),
+ href: admin_application_settings_path(anchor: 'js-signup-settings'),
+ enabled: allow_signup?)
+
+ = feature_entry(_('LDAP'),
+ enabled: Gitlab.config.ldap.enabled)
+
+ = feature_entry(_('Gravatar'),
+ href: admin_application_settings_path(anchor: 'js-account-settings'),
+ enabled: gravatar_enabled?)
+
+ = feature_entry(_('OmniAuth'),
+ href: admin_application_settings_path(anchor: 'js-signin-settings'),
+ enabled: Gitlab::Auth.omniauth_enabled?)
+
+ = feature_entry(_('Reply by email'),
+ enabled: Gitlab::IncomingEmail.enabled?)
= render_if_exists 'admin/dashboard/elastic_and_geo'
- = feature_entry(_('Container Registry'), href: ci_cd_admin_application_settings_path(anchor: 'js-registry-settings'), enabled: Gitlab.config.registry.enabled)
- = feature_entry(_('Gitlab Pages'), href: help_instance_configuration_url, enabled: Gitlab.config.pages.enabled)
- = feature_entry(_('Shared Runners'), href: admin_runners_path, enabled: Gitlab.config.gitlab_ci.shared_runners_enabled)
+ = feature_entry(_('Container Registry'),
+ href: ci_cd_admin_application_settings_path(anchor: 'js-registry-settings'),
+ enabled: Gitlab.config.registry.enabled,
+ doc_href: help_page_path('user/packages/container_registry/index'))
+
+ = feature_entry(_('Gitlab Pages'),
+ enabled: Gitlab.config.pages.enabled,
+ doc_href: help_instance_configuration_url)
+
+ = feature_entry(_('Shared Runners'),
+ href: admin_runners_path,
+ enabled: Gitlab.config.gitlab_ci.shared_runners_enabled)
.col-md-4
.info-well
.well-segment.admin-well
diff --git a/changelogs/unreleased/24082-double-escaping-in-tableflip-quick-action.yml b/changelogs/unreleased/24082-double-escaping-in-tableflip-quick-action.yml
new file mode 100644
index 00000000000..b7a2a41c93e
--- /dev/null
+++ b/changelogs/unreleased/24082-double-escaping-in-tableflip-quick-action.yml
@@ -0,0 +1,5 @@
+---
+title: Fix double escaping in /tableflip quick action
+merge_request: 19271
+author: Brian T
+type: fixed
diff --git a/changelogs/unreleased/dz-improve-admin-features.yml b/changelogs/unreleased/dz-improve-admin-features.yml
new file mode 100644
index 00000000000..a15b593d04e
--- /dev/null
+++ b/changelogs/unreleased/dz-improve-admin-features.yml
@@ -0,0 +1,5 @@
+---
+title: Improve admin dashboard features
+merge_request: 18666
+author:
+type: changed
diff --git a/doc/api/audit_events.md b/doc/api/audit_events.md
index aca221cf990..0b8351062e5 100644
--- a/doc/api/audit_events.md
+++ b/doc/api/audit_events.md
@@ -15,7 +15,7 @@ GET /audit_events
| `created_after` | string | no | Return audit events created on or after the given time. Format: ISO 8601 YYYY-MM-DDTHH:MM:SSZ |
| `created_before` | string | no | Return audit events created on or before the given time. Format: ISO 8601 YYYY-MM-DDTHH:MM:SSZ |
| `entity_type` | string | no | Return audit events for the given entity type. Valid values are: `User`, `Group`, or `Project`. |
-| `entity_id` | boolean | no | Return audit events for the given entity ID. Requires `entity_type` attribute to be present. |
+| `entity_id` | integer | no | Return audit events for the given entity ID. Requires `entity_type` attribute to be present. |
By default, `GET` requests return 20 results at a time because the API results
are paginated.
diff --git a/lib/api/helpers/pagination.rb b/lib/api/helpers/pagination.rb
index 71bbc218f94..9c5b355e823 100644
--- a/lib/api/helpers/pagination.rb
+++ b/lib/api/helpers/pagination.rb
@@ -4,254 +4,7 @@ module API
module Helpers
module Pagination
def paginate(relation)
- strategy = if params[:pagination] == 'keyset' && Feature.enabled?('api_keyset_pagination')
- KeysetPaginationStrategy
- else
- DefaultPaginationStrategy
- end
-
- strategy.new(self).paginate(relation)
- end
-
- class Base
- private
-
- def per_page
- @per_page ||= params[:per_page]
- end
-
- def base_request_uri
- @base_request_uri ||= URI.parse(request.url).tap do |uri|
- uri.host = Gitlab.config.gitlab.host
- uri.port = Gitlab.config.gitlab.port
- end
- end
-
- def build_page_url(query_params:)
- base_request_uri.tap do |uri|
- uri.query = query_params
- end.to_s
- end
-
- def page_href(next_page_params = {})
- query_params = params.merge(**next_page_params, per_page: per_page).to_query
-
- build_page_url(query_params: query_params)
- end
- end
-
- class KeysetPaginationInfo
- attr_reader :relation, :request_context
-
- def initialize(relation, request_context)
- # This is because it's rather complex to support multiple values with possibly different sort directions
- # (and we don't need this in the API)
- if relation.order_values.size > 1
- raise "Pagination only supports ordering by a single column." \
- "The following columns were given: #{relation.order_values.map { |v| v.expr.name }}"
- end
-
- @relation = relation
- @request_context = request_context
- end
-
- def fields
- keys.zip(values).reject { |_, v| v.nil? }.to_h
- end
-
- def column_for_order_by(relation)
- relation.order_values.first&.expr&.name
- end
-
- # Sort direction (`:asc` or `:desc`)
- def sort
- @sort ||= if order_by_primary_key?
- # Default order is by id DESC
- :desc
- else
- # API defaults to DESC order if param `sort` not present
- request_context.params[:sort]&.to_sym || :desc
- end
- end
-
- # Do we only sort by primary key?
- def order_by_primary_key?
- keys.size == 1 && keys.first == primary_key
- end
-
- def primary_key
- relation.model.primary_key.to_sym
- end
-
- def sort_ascending?
- sort == :asc
- end
-
- # Build hash of request parameters for a given record (relevant to pagination)
- def params_for(record)
- return {} unless record
-
- keys.each_with_object({}) do |key, h|
- h["ks_prev_#{key}".to_sym] = record.attributes[key.to_s]
- end
- end
-
- private
-
- # All values present in request parameters that correspond to #keys.
- def values
- @values ||= keys.map do |key|
- request_context.params["ks_prev_#{key}".to_sym]
- end
- end
-
- # All keys relevant to pagination.
- # This always includes the primary key. Optionally, the `order_by` key is prepended.
- def keys
- @keys ||= [column_for_order_by(relation), primary_key].compact.uniq
- end
- end
-
- class KeysetPaginationStrategy < Base
- attr_reader :request_context
- delegate :params, :header, :request, to: :request_context
-
- def initialize(request_context)
- @request_context = request_context
- end
-
- # rubocop: disable CodeReuse/ActiveRecord
- def paginate(relation)
- pagination = KeysetPaginationInfo.new(relation, request_context)
-
- paged_relation = relation.limit(per_page)
-
- if conds = conditions(pagination)
- paged_relation = paged_relation.where(*conds)
- end
-
- # In all cases: sort by primary key (possibly in addition to another sort column)
- paged_relation = paged_relation.order(pagination.primary_key => pagination.sort)
-
- add_default_pagination_headers
-
- if last_record = paged_relation.last
- next_page_params = pagination.params_for(last_record)
- add_navigation_links(next_page_params)
- end
-
- paged_relation
- end
- # rubocop: enable CodeReuse/ActiveRecord
-
- private
-
- def conditions(pagination)
- fields = pagination.fields
-
- return if fields.empty?
-
- placeholder = fields.map { '?' }
-
- comp = if pagination.sort_ascending?
- '>'
- else
- '<'
- end
-
- [
- # Row value comparison:
- # (A, B) < (a, b) <=> (A < a) OR (A = a AND B < b)
- # <=> A <= a AND ((A < a) OR (A = a AND B < b))
- "(#{fields.keys.join(',')}) #{comp} (#{placeholder.join(',')})",
- *fields.values
- ]
- end
-
- def add_default_pagination_headers
- header 'X-Per-Page', per_page.to_s
- end
-
- def add_navigation_links(next_page_params)
- header 'X-Next-Page', page_href(next_page_params)
- header 'Link', link_for('next', next_page_params)
- end
-
- def link_for(rel, next_page_params)
- %(<#{page_href(next_page_params)}>; rel="#{rel}")
- end
- end
-
- class DefaultPaginationStrategy < Base
- attr_reader :request_context
- delegate :params, :header, :request, to: :request_context
-
- def initialize(request_context)
- @request_context = request_context
- end
-
- def paginate(relation)
- paginate_with_limit_optimization(add_default_order(relation)).tap do |data|
- add_pagination_headers(data)
- end
- end
-
- private
-
- def paginate_with_limit_optimization(relation)
- pagination_data = relation.page(params[:page]).per(params[:per_page])
- return pagination_data unless pagination_data.is_a?(ActiveRecord::Relation)
- return pagination_data unless Feature.enabled?(:api_kaminari_count_with_limit)
-
- limited_total_count = pagination_data.total_count_with_limit
- if limited_total_count > Kaminari::ActiveRecordRelationMethods::MAX_COUNT_LIMIT
- # The call to `total_count_with_limit` memoizes `@arel` because of a call to `references_eager_loaded_tables?`
- # We need to call `reset` because `without_count` relies on `@arel` being unmemoized
- pagination_data.reset.without_count
- else
- pagination_data
- end
- end
-
- def add_default_order(relation)
- if relation.is_a?(ActiveRecord::Relation) && relation.order_values.empty?
- relation = relation.order(:id) # rubocop: disable CodeReuse/ActiveRecord
- end
-
- relation
- end
-
- def add_pagination_headers(paginated_data)
- header 'X-Per-Page', paginated_data.limit_value.to_s
- header 'X-Page', paginated_data.current_page.to_s
- header 'X-Next-Page', paginated_data.next_page.to_s
- header 'X-Prev-Page', paginated_data.prev_page.to_s
- header 'Link', pagination_links(paginated_data)
-
- return if data_without_counts?(paginated_data)
-
- header 'X-Total', paginated_data.total_count.to_s
- header 'X-Total-Pages', total_pages(paginated_data).to_s
- end
-
- def pagination_links(paginated_data)
- [].tap do |links|
- links << %(<#{page_href(page: paginated_data.prev_page)}>; rel="prev") if paginated_data.prev_page
- links << %(<#{page_href(page: paginated_data.next_page)}>; rel="next") if paginated_data.next_page
- links << %(<#{page_href(page: 1)}>; rel="first")
-
- links << %(<#{page_href(page: total_pages(paginated_data))}>; rel="last") unless data_without_counts?(paginated_data)
- end.join(', ')
- end
-
- def total_pages(paginated_data)
- # Ensure there is in total at least 1 page
- [paginated_data.total_pages, 1].max
- end
-
- def data_without_counts?(paginated_data)
- paginated_data.is_a?(Kaminari::PaginatableWithoutCount)
- end
+ ::Gitlab::Pagination::OffsetPagination.new(self).paginate(relation)
end
end
end
diff --git a/lib/gitlab/pagination/base.rb b/lib/gitlab/pagination/base.rb
new file mode 100644
index 00000000000..90fa1f8d1ec
--- /dev/null
+++ b/lib/gitlab/pagination/base.rb
@@ -0,0 +1,32 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Pagination
+ class Base
+ private
+
+ def per_page
+ @per_page ||= params[:per_page]
+ end
+
+ def base_request_uri
+ @base_request_uri ||= URI.parse(request.url).tap do |uri|
+ uri.host = Gitlab.config.gitlab.host
+ uri.port = Gitlab.config.gitlab.port
+ end
+ end
+
+ def build_page_url(query_params:)
+ base_request_uri.tap do |uri|
+ uri.query = query_params
+ end.to_s
+ end
+
+ def page_href(next_page_params = {})
+ query_params = params.merge(**next_page_params, per_page: per_page).to_query
+
+ build_page_url(query_params: query_params)
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/pagination/offset_pagination.rb b/lib/gitlab/pagination/offset_pagination.rb
new file mode 100644
index 00000000000..bf31f252a6b
--- /dev/null
+++ b/lib/gitlab/pagination/offset_pagination.rb
@@ -0,0 +1,77 @@
+# frozen_string_literal: true
+
+module Gitlab
+ module Pagination
+ class OffsetPagination < Base
+ attr_reader :request_context
+ delegate :params, :header, :request, to: :request_context
+
+ def initialize(request_context)
+ @request_context = request_context
+ end
+
+ def paginate(relation)
+ paginate_with_limit_optimization(add_default_order(relation)).tap do |data|
+ add_pagination_headers(data)
+ end
+ end
+
+ private
+
+ def paginate_with_limit_optimization(relation)
+ pagination_data = relation.page(params[:page]).per(params[:per_page])
+ return pagination_data unless pagination_data.is_a?(ActiveRecord::Relation)
+ return pagination_data unless Feature.enabled?(:api_kaminari_count_with_limit)
+
+ limited_total_count = pagination_data.total_count_with_limit
+ if limited_total_count > Kaminari::ActiveRecordRelationMethods::MAX_COUNT_LIMIT
+ # The call to `total_count_with_limit` memoizes `@arel` because of a call to `references_eager_loaded_tables?`
+ # We need to call `reset` because `without_count` relies on `@arel` being unmemoized
+ pagination_data.reset.without_count
+ else
+ pagination_data
+ end
+ end
+
+ def add_default_order(relation)
+ if relation.is_a?(ActiveRecord::Relation) && relation.order_values.empty?
+ relation = relation.order(:id) # rubocop: disable CodeReuse/ActiveRecord
+ end
+
+ relation
+ end
+
+ def add_pagination_headers(paginated_data)
+ header 'X-Per-Page', paginated_data.limit_value.to_s
+ header 'X-Page', paginated_data.current_page.to_s
+ header 'X-Next-Page', paginated_data.next_page.to_s
+ header 'X-Prev-Page', paginated_data.prev_page.to_s
+ header 'Link', pagination_links(paginated_data)
+
+ return if data_without_counts?(paginated_data)
+
+ header 'X-Total', paginated_data.total_count.to_s
+ header 'X-Total-Pages', total_pages(paginated_data).to_s
+ end
+
+ def pagination_links(paginated_data)
+ [].tap do |links|
+ links << %(<#{page_href(page: paginated_data.prev_page)}>; rel="prev") if paginated_data.prev_page
+ links << %(<#{page_href(page: paginated_data.next_page)}>; rel="next") if paginated_data.next_page
+ links << %(<#{page_href(page: 1)}>; rel="first")
+
+ links << %(<#{page_href(page: total_pages(paginated_data))}>; rel="last") unless data_without_counts?(paginated_data)
+ end.join(', ')
+ end
+
+ def total_pages(paginated_data)
+ # Ensure there is in total at least 1 page
+ [paginated_data.total_pages, 1].max
+ end
+
+ def data_without_counts?(paginated_data)
+ paginated_data.is_a?(Kaminari::PaginatableWithoutCount)
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/quick_actions/issuable_actions.rb b/lib/gitlab/quick_actions/issuable_actions.rb
index 340ec75c5f1..942f90e8040 100644
--- a/lib/gitlab/quick_actions/issuable_actions.rb
+++ b/lib/gitlab/quick_actions/issuable_actions.rb
@@ -234,7 +234,7 @@ module Gitlab
"#{comment} #{SHRUG}"
end
- desc _("Append the comment with %{TABLEFLIP}") % { tableflip: TABLEFLIP }
+ desc _("Append the comment with %{tableflip}") % { tableflip: TABLEFLIP }
params '<Comment>'
types Issuable
substitution :tableflip do |comment|
diff --git a/lib/gitlab/serializer/pagination.rb b/lib/gitlab/serializer/pagination.rb
index eb242cc7c20..bb7571dd66a 100644
--- a/lib/gitlab/serializer/pagination.rb
+++ b/lib/gitlab/serializer/pagination.rb
@@ -4,7 +4,6 @@ module Gitlab
module Serializer
class Pagination
InvalidResourceError = Class.new(StandardError)
- include ::API::Helpers::Pagination
def initialize(request, response)
@request = request
@@ -13,13 +12,13 @@ module Gitlab
def paginate(resource)
if resource.respond_to?(:page)
- super(resource)
+ ::Gitlab::Pagination::OffsetPagination.new(self).paginate(resource)
else
raise InvalidResourceError
end
end
- # Methods needed by `API::Helpers::Pagination`
+ # Methods needed by `Gitlab::Pagination::OffsetPagination`
#
attr_reader :request
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index dd02a55d4c9..9e19792f52e 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -1761,10 +1761,10 @@ msgstr ""
msgid "Appearance was successfully updated."
msgstr ""
-msgid "Append the comment with %{TABLEFLIP}"
+msgid "Append the comment with %{shrug}"
msgstr ""
-msgid "Append the comment with %{shrug}"
+msgid "Append the comment with %{tableflip}"
msgstr ""
msgid "Application"
@@ -5742,6 +5742,9 @@ msgstr ""
msgid "Dockerfile"
msgstr ""
+msgid "Documentation"
+msgstr ""
+
msgid "Documentation for popular identity providers"
msgstr ""
@@ -10502,6 +10505,9 @@ msgstr ""
msgid "Metrics|There was an error fetching the environments data, please try again"
msgstr ""
+msgid "Metrics|There was an error fetching the logs, please try again"
+msgstr ""
+
msgid "Metrics|There was an error getting deployment information."
msgstr ""
diff --git a/spec/helpers/dashboard_helper_spec.rb b/spec/helpers/dashboard_helper_spec.rb
index c899c2d9853..8a4ea33ac7c 100644
--- a/spec/helpers/dashboard_helper_spec.rb
+++ b/spec/helpers/dashboard_helper_spec.rb
@@ -25,39 +25,62 @@ describe DashboardHelper do
end
describe '#feature_entry' do
- context 'when implicitly enabled' do
- it 'considers feature enabled by default' do
- entry = feature_entry('Demo', href: 'demo.link')
+ shared_examples "a feature is enabled" do
+ it { is_expected.to include('<p aria-label="Demo: status on">') }
+ end
+
+ shared_examples "a feature is disabled" do
+ it { is_expected.to include('<p aria-label="Demo: status off">') }
+ end
- expect(entry).to include('<p aria-label="Demo: status on">')
- expect(entry).to include('<a href="demo.link">Demo</a>')
+ shared_examples "a feature without link" do
+ it do
+ is_expected.not_to have_link('Demo')
+ is_expected.not_to have_link('Documentation')
end
end
+ shared_examples "a feature with configuration" do
+ it { is_expected.to have_link('Demo', href: 'demo.link') }
+ end
+
+ shared_examples "a feature with documentation" do
+ it { is_expected.to have_link('Documentation', href: 'doc.link') }
+ end
+
+ context 'when implicitly enabled' do
+ subject { feature_entry('Demo') }
+
+ it_behaves_like 'a feature is enabled'
+ end
+
context 'when explicitly enabled' do
- it 'returns a link' do
- entry = feature_entry('Demo', href: 'demo.link', enabled: true)
+ context 'without links' do
+ subject { feature_entry('Demo', enabled: true) }
- expect(entry).to include('<p aria-label="Demo: status on">')
- expect(entry).to include('<a href="demo.link">Demo</a>')
+ it_behaves_like 'a feature is enabled'
+ it_behaves_like 'a feature without link'
end
- it 'returns text if href is not provided' do
- entry = feature_entry('Demo', enabled: true)
+ context 'with configure link' do
+ subject { feature_entry('Demo', href: 'demo.link', enabled: true) }
- expect(entry).to include('<p aria-label="Demo: status on">')
- expect(entry).not_to match(/<a[^>]+>/)
+ it_behaves_like 'a feature with configuration'
+ end
+
+ context 'with configure and documentation links' do
+ subject { feature_entry('Demo', href: 'demo.link', doc_href: 'doc.link', enabled: true) }
+
+ it_behaves_like 'a feature with configuration'
+ it_behaves_like 'a feature with documentation'
end
end
context 'when disabled' do
- it 'returns text without link' do
- entry = feature_entry('Demo', href: 'demo.link', enabled: false)
+ subject { feature_entry('Demo', href: 'demo.link', enabled: false) }
- expect(entry).to include('<p aria-label="Demo: status off">')
- expect(entry).not_to match(/<a[^>]+>/)
- expect(entry).to include('Demo')
- end
+ it_behaves_like 'a feature is disabled'
+ it_behaves_like 'a feature without link'
end
end
diff --git a/spec/lib/api/helpers/pagination_spec.rb b/spec/lib/api/helpers/pagination_spec.rb
index b57adb46385..040ff1a8ebe 100644
--- a/spec/lib/api/helpers/pagination_spec.rb
+++ b/spec/lib/api/helpers/pagination_spec.rb
@@ -3,399 +3,20 @@
require 'spec_helper'
describe API::Helpers::Pagination do
- let(:resource) { Project.all }
- let(:custom_port) { 8080 }
- let(:incoming_api_projects_url) { "#{Gitlab.config.gitlab.url}:#{custom_port}/api/v4/projects" }
+ subject { Class.new.include(described_class).new }
- before do
- stub_config_setting(port: custom_port)
- end
-
- subject do
- Class.new.include(described_class).new
- end
-
- describe '#paginate (keyset pagination)' do
- let(:value) { spy('return value') }
- let(:base_query) do
- {
- pagination: 'keyset',
- foo: 'bar',
- bar: 'baz'
- }
- end
- let(:query) { base_query }
-
- before do
- allow(subject).to receive(:header).and_return(value)
- allow(subject).to receive(:params).and_return(query)
- allow(subject).to receive(:request).and_return(double(url: "#{incoming_api_projects_url}?#{query.to_query}"))
- end
-
- context 'when resource can be paginated' do
- let!(:projects) do
- [
- create(:project, name: 'One'),
- create(:project, name: 'Two'),
- create(:project, name: 'Three')
- ].sort_by { |e| -e.id } # sort by id desc (this is the default sort order for the API)
- end
-
- describe 'first page' do
- let(:query) { base_query.merge(per_page: 2) }
-
- it 'returns appropriate amount of resources' do
- expect(subject.paginate(resource).count).to eq 2
- end
-
- it 'returns the first two records (by id desc)' do
- expect(subject.paginate(resource)).to eq(projects[0..1])
- end
-
- it 'adds appropriate headers' do
- expect_header('X-Per-Page', '2')
- expect_header('X-Next-Page', "#{incoming_api_projects_url}?#{query.merge(ks_prev_id: projects[1].id).to_query}")
-
- expect_header('Link', anything) do |_key, val|
- expect(val).to include('rel="next"')
- end
-
- subject.paginate(resource)
- end
- end
-
- describe 'second page' do
- let(:query) { base_query.merge(per_page: 2, ks_prev_id: projects[1].id) }
-
- it 'returns appropriate amount of resources' do
- expect(subject.paginate(resource).count).to eq 1
- end
-
- it 'returns the third record' do
- expect(subject.paginate(resource)).to eq(projects[2..2])
- end
-
- it 'adds appropriate headers' do
- expect_header('X-Per-Page', '2')
- expect_header('X-Next-Page', "#{incoming_api_projects_url}?#{query.merge(ks_prev_id: projects[2].id).to_query}")
-
- expect_header('Link', anything) do |_key, val|
- expect(val).to include('rel="next"')
- end
-
- subject.paginate(resource)
- end
- end
-
- describe 'third page' do
- let(:query) { base_query.merge(per_page: 2, ks_prev_id: projects[2].id) }
-
- it 'returns appropriate amount of resources' do
- expect(subject.paginate(resource).count).to eq 0
- end
-
- it 'adds appropriate headers' do
- expect_header('X-Per-Page', '2')
- expect_no_header('X-Next-Page')
- expect(subject).not_to receive(:header).with('Link')
-
- subject.paginate(resource)
- end
- end
-
- context 'if order' do
- context 'is not present' do
- let(:query) { base_query.merge(per_page: 2) }
-
- it 'is not present it adds default order(:id) desc' do
- resource.order_values = []
-
- paginated_relation = subject.paginate(resource)
-
- expect(resource.order_values).to be_empty
- expect(paginated_relation.order_values).to be_present
- expect(paginated_relation.order_values.size).to eq(1)
- expect(paginated_relation.order_values.first).to be_descending
- expect(paginated_relation.order_values.first.expr.name).to eq 'id'
- end
- end
-
- context 'is present' do
- let(:resource) { Project.all.order(name: :desc) }
- let!(:projects) do
- [
- create(:project, name: 'One'),
- create(:project, name: 'Two'),
- create(:project, name: 'Three'),
- create(:project, name: 'Three'), # Note the duplicate name
- create(:project, name: 'Four'),
- create(:project, name: 'Five'),
- create(:project, name: 'Six')
- ]
-
- # if we sort this by name descending, id descending, this yields:
- # {
- # 2 => "Two",
- # 4 => "Three",
- # 3 => "Three",
- # 7 => "Six",
- # 1 => "One",
- # 5 => "Four",
- # 6 => "Five"
- # }
- #
- # (key is the id)
- end
-
- it 'also orders by primary key' do
- paginated_relation = subject.paginate(resource)
-
- expect(paginated_relation.order_values).to be_present
- expect(paginated_relation.order_values.size).to eq(2)
- expect(paginated_relation.order_values.first).to be_descending
- expect(paginated_relation.order_values.first.expr.name).to eq 'name'
- expect(paginated_relation.order_values.second).to be_descending
- expect(paginated_relation.order_values.second.expr.name).to eq 'id'
- end
-
- it 'returns the right records (first page)' do
- result = subject.paginate(resource)
-
- expect(result.first).to eq(projects[1])
- expect(result.second).to eq(projects[3])
- end
-
- describe 'second page' do
- let(:query) { base_query.merge(ks_prev_id: projects[3].id, ks_prev_name: projects[3].name, per_page: 2) }
-
- it 'returns the right records (second page)' do
- result = subject.paginate(resource)
-
- expect(result.first).to eq(projects[2])
- expect(result.second).to eq(projects[6])
- end
-
- it 'returns the right link to the next page' do
- expect_header('X-Per-Page', '2')
- expect_header('X-Next-Page', "#{incoming_api_projects_url}?#{query.merge(ks_prev_id: projects[6].id, ks_prev_name: projects[6].name).to_query}")
- expect_header('Link', anything) do |_key, val|
- expect(val).to include('rel="next"')
- end
-
- subject.paginate(resource)
- end
- end
-
- describe 'third page' do
- let(:query) { base_query.merge(ks_prev_id: projects[6].id, ks_prev_name: projects[6].name, per_page: 5) }
-
- it 'returns the right records (third page), note increased per_page' do
- result = subject.paginate(resource)
-
- expect(result.size).to eq(3)
- expect(result.first).to eq(projects[0])
- expect(result.second).to eq(projects[4])
- expect(result.last).to eq(projects[5])
- end
- end
- end
- end
- end
- end
-
- describe '#paginate (default offset-based pagination)' do
- let(:value) { spy('return value') }
- let(:base_query) { { foo: 'bar', bar: 'baz' } }
- let(:query) { base_query }
-
- before do
- allow(subject).to receive(:header).and_return(value)
- allow(subject).to receive(:params).and_return(query)
- allow(subject).to receive(:request).and_return(double(url: "#{incoming_api_projects_url}?#{query.to_query}"))
- end
-
- context 'when resource can be paginated' do
- before do
- create_list(:project, 3)
- end
-
- describe 'first page' do
- shared_examples 'response with pagination headers' do
- it 'adds appropriate headers' do
- expect_header('X-Total', '3')
- expect_header('X-Total-Pages', '2')
- expect_header('X-Per-Page', '2')
- expect_header('X-Page', '1')
- expect_header('X-Next-Page', '2')
- expect_header('X-Prev-Page', '')
-
- expect_header('Link', anything) do |_key, val|
- expect(val).to include(%Q(<#{incoming_api_projects_url}?#{query.merge(page: 1).to_query}>; rel="first"))
- expect(val).to include(%Q(<#{incoming_api_projects_url}?#{query.merge(page: 2).to_query}>; rel="last"))
- expect(val).to include(%Q(<#{incoming_api_projects_url}?#{query.merge(page: 2).to_query}>; rel="next"))
- expect(val).not_to include('rel="prev"')
- end
-
- subject.paginate(resource)
- end
- end
-
- shared_examples 'paginated response' do
- it 'returns appropriate amount of resources' do
- expect(subject.paginate(resource).count).to eq 2
- end
-
- it 'executes only one SELECT COUNT query' do
- expect { subject.paginate(resource) }.to make_queries_matching(/SELECT COUNT/, 1)
- end
- end
-
- let(:query) { base_query.merge(page: 1, per_page: 2) }
-
- context 'when the api_kaminari_count_with_limit feature flag is unset' do
- it_behaves_like 'paginated response'
- it_behaves_like 'response with pagination headers'
- end
-
- context 'when the api_kaminari_count_with_limit feature flag is disabled' do
- before do
- stub_feature_flags(api_kaminari_count_with_limit: false)
- end
-
- it_behaves_like 'paginated response'
- it_behaves_like 'response with pagination headers'
- end
-
- context 'when the api_kaminari_count_with_limit feature flag is enabled' do
- before do
- stub_feature_flags(api_kaminari_count_with_limit: true)
- end
-
- context 'when resources count is less than MAX_COUNT_LIMIT' do
- before do
- stub_const("::Kaminari::ActiveRecordRelationMethods::MAX_COUNT_LIMIT", 4)
- end
-
- it_behaves_like 'paginated response'
- it_behaves_like 'response with pagination headers'
- end
-
- context 'when resources count is more than MAX_COUNT_LIMIT' do
- before do
- stub_const("::Kaminari::ActiveRecordRelationMethods::MAX_COUNT_LIMIT", 2)
- end
-
- it_behaves_like 'paginated response'
-
- it 'does not return the X-Total and X-Total-Pages headers' do
- expect_no_header('X-Total')
- expect_no_header('X-Total-Pages')
- expect_header('X-Per-Page', '2')
- expect_header('X-Page', '1')
- expect_header('X-Next-Page', '2')
- expect_header('X-Prev-Page', '')
+ describe '#paginate' do
+ let(:relation) { double("relation") }
+ let(:offset_pagination) { double("offset pagination") }
+ let(:expected_result) { double("result") }
- expect_header('Link', anything) do |_key, val|
- expect(val).to include(%Q(<#{incoming_api_projects_url}?#{query.merge(page: 1).to_query}>; rel="first"))
- expect(val).to include(%Q(<#{incoming_api_projects_url}?#{query.merge(page: 2).to_query}>; rel="next"))
- expect(val).not_to include('rel="last"')
- expect(val).not_to include('rel="prev"')
- end
+ it 'delegates to OffsetPagination' do
+ expect(::Gitlab::Pagination::OffsetPagination).to receive(:new).with(subject).and_return(offset_pagination)
+ expect(offset_pagination).to receive(:paginate).with(relation).and_return(expected_result)
- subject.paginate(resource)
- end
- end
- end
- end
+ result = subject.paginate(relation)
- describe 'second page' do
- let(:query) { base_query.merge(page: 2, per_page: 2) }
-
- it 'returns appropriate amount of resources' do
- expect(subject.paginate(resource).count).to eq 1
- end
-
- it 'adds appropriate headers' do
- expect_header('X-Total', '3')
- expect_header('X-Total-Pages', '2')
- expect_header('X-Per-Page', '2')
- expect_header('X-Page', '2')
- expect_header('X-Next-Page', '')
- expect_header('X-Prev-Page', '1')
-
- expect_header('Link', anything) do |_key, val|
- expect(val).to include(%Q(<#{incoming_api_projects_url}?#{query.merge(page: 1).to_query}>; rel="first"))
- expect(val).to include(%Q(<#{incoming_api_projects_url}?#{query.merge(page: 2).to_query}>; rel="last"))
- expect(val).to include(%Q(<#{incoming_api_projects_url}?#{query.merge(page: 1).to_query}>; rel="prev"))
- expect(val).not_to include('rel="next"')
- end
-
- subject.paginate(resource)
- end
- end
-
- context 'if order' do
- it 'is not present it adds default order(:id) if no order is present' do
- resource.order_values = []
-
- paginated_relation = subject.paginate(resource)
-
- expect(resource.order_values).to be_empty
- expect(paginated_relation.order_values).to be_present
- expect(paginated_relation.order_values.first).to be_ascending
- expect(paginated_relation.order_values.first.expr.name).to eq 'id'
- end
-
- it 'is present it does not add anything' do
- paginated_relation = subject.paginate(resource.order(created_at: :desc))
-
- expect(paginated_relation.order_values).to be_present
- expect(paginated_relation.order_values.first).to be_descending
- expect(paginated_relation.order_values.first.expr.name).to eq 'created_at'
- end
- end
+ expect(result).to eq(expected_result)
end
-
- context 'when resource empty' do
- describe 'first page' do
- let(:query) { base_query.merge(page: 1, per_page: 2) }
-
- it 'returns appropriate amount of resources' do
- expect(subject.paginate(resource).count).to eq 0
- end
-
- it 'adds appropriate headers' do
- expect_header('X-Total', '0')
- expect_header('X-Total-Pages', '1')
- expect_header('X-Per-Page', '2')
- expect_header('X-Page', '1')
- expect_header('X-Next-Page', '')
- expect_header('X-Prev-Page', '')
-
- expect_header('Link', anything) do |_key, val|
- expect(val).to include(%Q(<#{incoming_api_projects_url}?#{query.merge(page: 1).to_query}>; rel="first"))
- expect(val).to include(%Q(<#{incoming_api_projects_url}?#{query.merge(page: 1).to_query}>; rel="last"))
- expect(val).not_to include('rel="prev"')
- expect(val).not_to include('rel="next"')
- expect(val).not_to include('page=0')
- end
-
- subject.paginate(resource)
- end
- end
- end
- end
-
- def expect_header(*args, &block)
- expect(subject).to receive(:header).with(*args, &block)
- end
-
- def expect_no_header(*args, &block)
- expect(subject).not_to receive(:header).with(*args)
- end
-
- def expect_message(method)
- expect(subject).to receive(method)
- .at_least(:once).and_return(value)
end
end
diff --git a/spec/lib/gitlab/pagination/offset_pagination_spec.rb b/spec/lib/gitlab/pagination/offset_pagination_spec.rb
new file mode 100644
index 00000000000..9c7dd385726
--- /dev/null
+++ b/spec/lib/gitlab/pagination/offset_pagination_spec.rb
@@ -0,0 +1,215 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Pagination::OffsetPagination do
+ let(:resource) { Project.all }
+ let(:custom_port) { 8080 }
+ let(:incoming_api_projects_url) { "#{Gitlab.config.gitlab.url}:#{custom_port}/api/v4/projects" }
+
+ before do
+ stub_config_setting(port: custom_port)
+ end
+
+ let(:request_context) { double("request_context") }
+
+ subject do
+ described_class.new(request_context)
+ end
+
+ describe '#paginate' do
+ let(:value) { spy('return value') }
+ let(:base_query) { { foo: 'bar', bar: 'baz' } }
+ let(:query) { base_query }
+
+ before do
+ allow(request_context).to receive(:header).and_return(value)
+ allow(request_context).to receive(:params).and_return(query)
+ allow(request_context).to receive(:request).and_return(double(url: "#{incoming_api_projects_url}?#{query.to_query}"))
+ end
+
+ context 'when resource can be paginated' do
+ before do
+ create_list(:project, 3)
+ end
+
+ describe 'first page' do
+ shared_examples 'response with pagination headers' do
+ it 'adds appropriate headers' do
+ expect_header('X-Total', '3')
+ expect_header('X-Total-Pages', '2')
+ expect_header('X-Per-Page', '2')
+ expect_header('X-Page', '1')
+ expect_header('X-Next-Page', '2')
+ expect_header('X-Prev-Page', '')
+
+ expect_header('Link', anything) do |_key, val|
+ expect(val).to include(%Q(<#{incoming_api_projects_url}?#{query.merge(page: 1).to_query}>; rel="first"))
+ expect(val).to include(%Q(<#{incoming_api_projects_url}?#{query.merge(page: 2).to_query}>; rel="last"))
+ expect(val).to include(%Q(<#{incoming_api_projects_url}?#{query.merge(page: 2).to_query}>; rel="next"))
+ expect(val).not_to include('rel="prev"')
+ end
+
+ subject.paginate(resource)
+ end
+ end
+
+ shared_examples 'paginated response' do
+ it 'returns appropriate amount of resources' do
+ expect(subject.paginate(resource).count).to eq 2
+ end
+
+ it 'executes only one SELECT COUNT query' do
+ expect { subject.paginate(resource) }.to make_queries_matching(/SELECT COUNT/, 1)
+ end
+ end
+
+ let(:query) { base_query.merge(page: 1, per_page: 2) }
+
+ context 'when the api_kaminari_count_with_limit feature flag is unset' do
+ it_behaves_like 'paginated response'
+ it_behaves_like 'response with pagination headers'
+ end
+
+ context 'when the api_kaminari_count_with_limit feature flag is disabled' do
+ before do
+ stub_feature_flags(api_kaminari_count_with_limit: false)
+ end
+
+ it_behaves_like 'paginated response'
+ it_behaves_like 'response with pagination headers'
+ end
+
+ context 'when the api_kaminari_count_with_limit feature flag is enabled' do
+ before do
+ stub_feature_flags(api_kaminari_count_with_limit: true)
+ end
+
+ context 'when resources count is less than MAX_COUNT_LIMIT' do
+ before do
+ stub_const("::Kaminari::ActiveRecordRelationMethods::MAX_COUNT_LIMIT", 4)
+ end
+
+ it_behaves_like 'paginated response'
+ it_behaves_like 'response with pagination headers'
+ end
+
+ context 'when resources count is more than MAX_COUNT_LIMIT' do
+ before do
+ stub_const("::Kaminari::ActiveRecordRelationMethods::MAX_COUNT_LIMIT", 2)
+ end
+
+ it_behaves_like 'paginated response'
+
+ it 'does not return the X-Total and X-Total-Pages headers' do
+ expect_no_header('X-Total')
+ expect_no_header('X-Total-Pages')
+ expect_header('X-Per-Page', '2')
+ expect_header('X-Page', '1')
+ expect_header('X-Next-Page', '2')
+ expect_header('X-Prev-Page', '')
+
+ expect_header('Link', anything) do |_key, val|
+ expect(val).to include(%Q(<#{incoming_api_projects_url}?#{query.merge(page: 1).to_query}>; rel="first"))
+ expect(val).to include(%Q(<#{incoming_api_projects_url}?#{query.merge(page: 2).to_query}>; rel="next"))
+ expect(val).not_to include('rel="last"')
+ expect(val).not_to include('rel="prev"')
+ end
+
+ subject.paginate(resource)
+ end
+ end
+ end
+ end
+
+ describe 'second page' do
+ let(:query) { base_query.merge(page: 2, per_page: 2) }
+
+ it 'returns appropriate amount of resources' do
+ expect(subject.paginate(resource).count).to eq 1
+ end
+
+ it 'adds appropriate headers' do
+ expect_header('X-Total', '3')
+ expect_header('X-Total-Pages', '2')
+ expect_header('X-Per-Page', '2')
+ expect_header('X-Page', '2')
+ expect_header('X-Next-Page', '')
+ expect_header('X-Prev-Page', '1')
+
+ expect_header('Link', anything) do |_key, val|
+ expect(val).to include(%Q(<#{incoming_api_projects_url}?#{query.merge(page: 1).to_query}>; rel="first"))
+ expect(val).to include(%Q(<#{incoming_api_projects_url}?#{query.merge(page: 2).to_query}>; rel="last"))
+ expect(val).to include(%Q(<#{incoming_api_projects_url}?#{query.merge(page: 1).to_query}>; rel="prev"))
+ expect(val).not_to include('rel="next"')
+ end
+
+ subject.paginate(resource)
+ end
+ end
+
+ context 'if order' do
+ it 'is not present it adds default order(:id) if no order is present' do
+ resource.order_values = []
+
+ paginated_relation = subject.paginate(resource)
+
+ expect(resource.order_values).to be_empty
+ expect(paginated_relation.order_values).to be_present
+ expect(paginated_relation.order_values.first).to be_ascending
+ expect(paginated_relation.order_values.first.expr.name).to eq 'id'
+ end
+
+ it 'is present it does not add anything' do
+ paginated_relation = subject.paginate(resource.order(created_at: :desc))
+
+ expect(paginated_relation.order_values).to be_present
+ expect(paginated_relation.order_values.first).to be_descending
+ expect(paginated_relation.order_values.first.expr.name).to eq 'created_at'
+ end
+ end
+ end
+
+ context 'when resource empty' do
+ describe 'first page' do
+ let(:query) { base_query.merge(page: 1, per_page: 2) }
+
+ it 'returns appropriate amount of resources' do
+ expect(subject.paginate(resource).count).to eq 0
+ end
+
+ it 'adds appropriate headers' do
+ expect_header('X-Total', '0')
+ expect_header('X-Total-Pages', '1')
+ expect_header('X-Per-Page', '2')
+ expect_header('X-Page', '1')
+ expect_header('X-Next-Page', '')
+ expect_header('X-Prev-Page', '')
+
+ expect_header('Link', anything) do |_key, val|
+ expect(val).to include(%Q(<#{incoming_api_projects_url}?#{query.merge(page: 1).to_query}>; rel="first"))
+ expect(val).to include(%Q(<#{incoming_api_projects_url}?#{query.merge(page: 1).to_query}>; rel="last"))
+ expect(val).not_to include('rel="prev"')
+ expect(val).not_to include('rel="next"')
+ expect(val).not_to include('page=0')
+ end
+
+ subject.paginate(resource)
+ end
+ end
+ end
+ end
+
+ def expect_header(*args, &block)
+ expect(subject).to receive(:header).with(*args, &block)
+ end
+
+ def expect_no_header(*args, &block)
+ expect(subject).not_to receive(:header).with(*args)
+ end
+
+ def expect_message(method)
+ expect(subject).to receive(method)
+ .at_least(:once).and_return(value)
+ end
+end