Commit 5ff1764a authored by Andy Soiron's avatar Andy Soiron

Merge branch 'api_kaminari_count_with_limit_cleanup' into 'master'

Cleanup api_kaminari_count_with_limit

See merge request gitlab-org/gitlab!81084
parents 8cac7158 4dd097fc
---
name: api_kaminari_count_with_limit
introduced_by_url: https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/23931
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/353077
milestone: '11.8'
type: ops
group: group::integrations
default_enabled: true
...@@ -89,22 +89,24 @@ Example response: ...@@ -89,22 +89,24 @@ Example response:
```json ```json
[ [
{ {
"name": "api_kaminari_count_with_limit", "name": "geo_pages_deployment_replication",
"introduced_by_url": "https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/23931", "introduced_by_url": "https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68662",
"rollout_issue_url": null, "rollout_issue_url": "https://gitlab.com/gitlab-org/gitlab/-/issues/337676",
"milestone": "11.8", "milestone": "14.3",
"type": "ops", "log_state_changes": null,
"group": "group::ecosystem", "type": "development",
"group": "group::geo",
"default_enabled": true "default_enabled": true
}, },
{ {
"name": "marginalia", "name": "analytics_devops_adoption_codeowners",
"introduced_by_url": null, "introduced_by_url": "https://gitlab.com/gitlab-org/gitlab/-/merge_requests/59874",
"rollout_issue_url": null, "rollout_issue_url": "https://gitlab.com/gitlab-org/gitlab/-/issues/328542",
"milestone": null, "milestone": "13.12",
"type": "ops", "log_state_changes": null,
"group": null, "type": "development",
"default_enabled": false "group": "group::optimize",
"default_enabled": true
} }
] ]
``` ```
......
...@@ -27,7 +27,6 @@ module Gitlab ...@@ -27,7 +27,6 @@ module Gitlab
end end
return pagination_data unless pagination_data.is_a?(ActiveRecord::Relation) return pagination_data unless pagination_data.is_a?(ActiveRecord::Relation)
return pagination_data unless Feature.enabled?(:api_kaminari_count_with_limit, type: :ops, default_enabled: :yaml)
limited_total_count = pagination_data.total_count_with_limit limited_total_count = pagination_data.total_count_with_limit
if limited_total_count > Kaminari::ActiveRecordRelationMethods::MAX_COUNT_LIMIT if limited_total_count > Kaminari::ActiveRecordRelationMethods::MAX_COUNT_LIMIT
......
...@@ -66,70 +66,50 @@ RSpec.describe Gitlab::Pagination::OffsetPagination do ...@@ -66,70 +66,50 @@ RSpec.describe Gitlab::Pagination::OffsetPagination do
let(:query) { base_query.merge(page: 1, per_page: 2) } let(:query) { base_query.merge(page: 1, per_page: 2) }
context 'when the api_kaminari_count_with_limit feature flag is unset' do context 'when resources count is less than MAX_COUNT_LIMIT' 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 before do
stub_feature_flags(api_kaminari_count_with_limit: false) stub_const("::Kaminari::ActiveRecordRelationMethods::MAX_COUNT_LIMIT", 4)
end end
it_behaves_like 'paginated response' it_behaves_like 'paginated response'
it_behaves_like 'response with pagination headers' it_behaves_like 'response with pagination headers'
end end
context 'when the api_kaminari_count_with_limit feature flag is enabled' do context 'when resources count is more than MAX_COUNT_LIMIT' do
before do before do
stub_feature_flags(api_kaminari_count_with_limit: true) stub_const("::Kaminari::ActiveRecordRelationMethods::MAX_COUNT_LIMIT", 2)
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 end
context 'when resources count is more than MAX_COUNT_LIMIT' do it_behaves_like 'paginated response'
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
it 'does not return the total headers when excluding them' do it 'does not return the X-Total and X-Total-Pages headers' do
expect_no_header('X-Total') expect_no_header('X-Total')
expect_no_header('X-Total-Pages') expect_no_header('X-Total-Pages')
expect_header('X-Per-Page', '2') expect_header('X-Per-Page', '2')
expect_header('X-Page', '1') expect_header('X-Page', '1')
expect_header('X-Next-Page', '2')
expect_header('X-Prev-Page', '')
paginator.paginate(resource, exclude_total_headers: true) 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
it 'does not return the total headers when excluding them' do
expect_no_header('X-Total')
expect_no_header('X-Total-Pages')
expect_header('X-Per-Page', '2')
expect_header('X-Page', '1')
paginator.paginate(resource, exclude_total_headers: true)
end
context 'when resource already paginated' do context 'when resource already paginated' do
let(:resource) { Project.all.page(1).per(1) } let(:resource) { Project.all.page(1).per(1) }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment