Skip to content

Commit

Permalink
Add max_per_page value to grape swagger docs
Browse files Browse the repository at this point in the history
If a max_per_page value is added to grape then it is show in the docs.
This avoids confusing if someone tries to pass in a value greater than
max_per_page as it will return an error so the user can correct
themself. Note this may be a breaking change for some people.
  • Loading branch information
Owen Davies committed Apr 25, 2018
1 parent 3bf4feb commit a0b3e6b
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 6 deletions.
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,15 @@ class MoviesAPI < Grape::API
get :cast do
paginate Movie.find(params[:id]).actors
end

desc "Return one movie's awards, paginated"
# Enforce max_per_page value will add the alowed values
# to the swagger docs, and cause grape to return an error
# if outside that range
paginate per_page: 10, max_per_page: 200, enforce_max_per_page: true
get :awards do
paginate Movie.find(params[:id]).awards
end
end
end
```
Expand Down
13 changes: 9 additions & 4 deletions lib/grape/pagination.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,16 @@ def paginate(collection)
def self.paginate(options = {})
route_setting :per_page, options[:per_page]
route_setting :max_per_page, options[:max_per_page]

enforce_max_per_page = options[:max_per_page] && options[:enforce_max_per_page]
per_page_values = enforce_max_per_page ? 0..options[:max_per_page] : nil

params do
optional :page, :type => Integer, :default => 1,
:desc => 'Page of results to fetch.'
optional :per_page, :type => Integer,
:desc => 'Number of results to return per page.'
optional :page, :type => Integer, :default => 1,
:desc => 'Page of results to fetch.'
optional :per_page, :type => Integer,

This comment has been minimized.

Copy link
@voxdei

voxdei Jun 25, 2018

is there a specific reason not to supply default here ?

something like

optional :per_page, :type => Integer, default: options[:per_page]
                                :desc => 'Number of results to return per page.'

during the documentation generation we end up with blank input and it would be nice to see some defaut value instead

capture d ecran 2018-06-25 11 37 15

if there is a way to do it, please let me know

This comment has been minimized.

Copy link
@davidcelis

davidcelis Jun 25, 2018

Owner

Added this in 289716e; I frequently cut releases, so check back later!

:desc => 'Number of results to return per page.',
:values => per_page_values
end
end
end
Expand Down
22 changes: 21 additions & 1 deletion spec/grape_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,35 @@
it_behaves_like 'an endpoint with a middle page'
end

context 'with a max_per_page setting' do
context 'without a max_per_page setting' do
before { get '/numbers', :count => 100, :per_page => 30 }

it 'should list all numbers within per page in the response body' do
body = '[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30]'

expect(last_response.body).to eq(body)
end
end

context 'with a max_per_page setting not enforced' do
before { get '/numbers_with_max_per_page', :count => 100, :per_page => 30 }

it 'should not go above the max_per_page_limit' do
body = '[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25]'

expect(last_response.body).to eq(body)
end
end

context 'with a max_per_page setting enforced' do
before { get '/numbers_with_enforced_max_per_page', :count => 100, :per_page => 30 }

it 'should not allow value above the max_per_page_limit' do
body = '{"error":"per_page does not have a valid value"}'

expect(last_response.body).to eq(body)
end
end
end

context 'with custom response headers' do
Expand Down
20 changes: 19 additions & 1 deletion spec/support/numbers_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ class NumbersAPI < Grape::API
format :json

desc 'Return some paginated set of numbers'
paginate :per_page => 10, :max_per_page => 25
paginate :per_page => 10
params do
requires :count, :type => Integer
optional :with_headers, :default => false, :type => Boolean
Expand All @@ -20,4 +20,22 @@ class NumbersAPI < Grape::API

paginate (1..params[:count]).to_a
end

desc 'Return some paginated set of numbers with max_per_page'
paginate :per_page => 10, :max_per_page => 25
params do
requires :count, :type => Integer
end
get :numbers_with_max_per_page do
paginate (1..params[:count]).to_a
end

desc 'Return some paginated set of numbers with max_per_page enforced'
paginate :per_page => 10, :max_per_page => 25, :enforce_max_per_page => true
params do
requires :count, :type => Integer
end
get :numbers_with_enforced_max_per_page do
paginate (1..params[:count]).to_a
end
end

0 comments on commit a0b3e6b

Please sign in to comment.