Commit 4f20ad1b authored by pbair's avatar pbair

Improve error message and minor clean up

Improve error message to display the type of relation the request has
failed for, to be clear to users that limitations are specific to the
type of results requested
parent dbb70c6c
...@@ -11,7 +11,7 @@ module API ...@@ -11,7 +11,7 @@ module API
end end
end end
def paginator(relation, request_scope) def paginator(relation, request_scope = nil)
return keyset_paginator(relation) if keyset_pagination_enabled? return keyset_paginator(relation) if keyset_pagination_enabled?
offset_paginator(relation, request_scope) offset_paginator(relation, request_scope)
...@@ -31,8 +31,9 @@ module API ...@@ -31,8 +31,9 @@ module API
def offset_paginator(relation, request_scope) def offset_paginator(relation, request_scope)
offset_limit = limit_for_scope(request_scope) offset_limit = limit_for_scope(request_scope)
if Gitlab::Pagination::Keyset.available_for_type?(relation) && offset_limit_exceeded?(offset_limit) if Gitlab::Pagination::Keyset.available_for_type?(relation) && offset_limit_exceeded?(offset_limit)
return error!("Offset pagination has a maximum allowed offset of #{offset_limit}, " \ return error!("Offset pagination has a maximum allowed offset of #{offset_limit} " \
"remaining records can be retrieved using keyset pagination.", 405) "for requests that return objects of type #{relation.klass}. " \
"Remaining records can be retrieved using keyset pagination.", 405)
end end
Gitlab::Pagination::OffsetPagination.new(self) Gitlab::Pagination::OffsetPagination.new(self)
......
...@@ -3,8 +3,12 @@ ...@@ -3,8 +3,12 @@
module Gitlab module Gitlab
module Pagination module Pagination
module Keyset module Keyset
SUPPORTED_TYPES = [
Project
].freeze
def self.available_for_type?(relation) def self.available_for_type?(relation)
relation.klass == Project SUPPORTED_TYPES.include?(relation.klass)
end end
def self.available?(request_context, relation) def self.available?(request_context, relation)
......
...@@ -6,7 +6,7 @@ describe API::Helpers::PaginationStrategies do ...@@ -6,7 +6,7 @@ describe API::Helpers::PaginationStrategies do
subject { Class.new.include(described_class).new } subject { Class.new.include(described_class).new }
let(:expected_result) { double("result") } let(:expected_result) { double("result") }
let(:relation) { double("relation") } let(:relation) { double("relation", klass: "SomeClass") }
let(:params) { {} } let(:params) { {} }
before do before do
...@@ -90,7 +90,7 @@ describe API::Helpers::PaginationStrategies do ...@@ -90,7 +90,7 @@ describe API::Helpers::PaginationStrategies do
it 'renders a 405 error' do it 'renders a 405 error' do
expect(subject).to receive(:error!).with(/maximum allowed offset/, 405) expect(subject).to receive(:error!).with(/maximum allowed offset/, 405)
subject.paginator(relation, nil) subject.paginator(relation)
end end
end end
...@@ -100,7 +100,7 @@ describe API::Helpers::PaginationStrategies do ...@@ -100,7 +100,7 @@ describe API::Helpers::PaginationStrategies do
it 'delegates to OffsetPagination' do it 'delegates to OffsetPagination' do
expect(Gitlab::Pagination::OffsetPagination).to receive(:new).with(subject).and_return(paginator) expect(Gitlab::Pagination::OffsetPagination).to receive(:new).with(subject).and_return(paginator)
expect(subject.paginator(relation, nil)).to eq(paginator) expect(subject.paginator(relation)).to eq(paginator)
end end
end end
end end
...@@ -116,7 +116,7 @@ describe API::Helpers::PaginationStrategies do ...@@ -116,7 +116,7 @@ describe API::Helpers::PaginationStrategies do
it 'delegates to OffsetPagination' do it 'delegates to OffsetPagination' do
expect(Gitlab::Pagination::OffsetPagination).to receive(:new).with(subject).and_return(paginator) expect(Gitlab::Pagination::OffsetPagination).to receive(:new).with(subject).and_return(paginator)
expect(subject.paginator(relation, nil)).to eq(paginator) expect(subject.paginator(relation)).to eq(paginator)
end end
end end
end end
...@@ -138,7 +138,7 @@ describe API::Helpers::PaginationStrategies do ...@@ -138,7 +138,7 @@ describe API::Helpers::PaginationStrategies do
end end
it 'delegates to Pager' do it 'delegates to Pager' do
expect(subject.paginator(relation, nil)).to eq(pager) expect(subject.paginator(relation)).to eq(pager)
end end
end end
...@@ -150,7 +150,7 @@ describe API::Helpers::PaginationStrategies do ...@@ -150,7 +150,7 @@ describe API::Helpers::PaginationStrategies do
it 'renders a 501 error' do it 'renders a 501 error' do
expect(subject).to receive(:error!).with(/not yet available/, 405) expect(subject).to receive(:error!).with(/not yet available/, 405)
subject.paginator(relation, nil) subject.paginator(relation)
end end
end end
end end
......
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