From 786172745437913cc85de910cad12f412b71bdbb Mon Sep 17 00:00:00 2001 From: Dimitrij Denissenko Date: Thu, 8 Oct 2020 09:38:15 +0100 Subject: [PATCH] Grape v1.5 fixes + shared pagination parameters --- CHANGELOG.md | 14 ++- Gemfile.lock | 26 ++--- README.md | 6 +- lib/grape/kaminari.rb | 40 +++++--- lib/grape/kaminari/version.rb | 2 +- spec/kaminari_spec.rb | 181 +++++++++++++++++----------------- spec/paginate_helper_spec.rb | 12 ++- 7 files changed, 153 insertions(+), 128 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c1196ce..199fcf9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,14 +1,24 @@ -### 0.4.0 (Next) +### 0.5.0 (Next) #### Features * Your contribution here. -* [#56](https://github.com/bsm/grape-kaminari/pull/56): Introduce a CHANGELOG - [@dim](https://github.com/dim). #### Fixes * Your contribution here. +### 0.4.0 + +#### Features + +* [#57](https://github.com/bsm/grape-kaminari/pull/57): Introduce new params based API - [@dim](https://github.com/dim). +* [#56](https://github.com/bsm/grape-kaminari/pull/56): Introduce a CHANGELOG - [@dim](https://github.com/dim). + +#### Fixes + +* [#57](https://github.com/bsm/grape-kaminari/pull/57): Fix issues related to Grape v1.5 release - [@dim](https://github.com/dim). + ### 0.3.0 (2020/08/10) * [#54](https://github.com/bsm/grape-kaminari/pull/54): Inherit paginate helper - [@dim](https://github.com/dim). diff --git a/Gemfile.lock b/Gemfile.lock index ac1969a..5b2acb0 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,14 +1,14 @@ PATH remote: . specs: - grape-kaminari (0.3.0) + grape-kaminari (0.4.0) grape (>= 1.0, != 1.4.0) kaminari-grape GEM remote: https://rubygems.org/ specs: - activesupport (6.0.3.2) + activesupport (6.0.3.4) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 0.7, < 2) minitest (~> 5.1) @@ -29,7 +29,7 @@ GEM concurrent-ruby (~> 1.0) dry-equalizer (0.3.0) dry-inflector (0.2.0) - dry-logic (1.0.6) + dry-logic (1.0.8) concurrent-ruby (~> 1.0) dry-core (~> 0.2) dry-equalizer (~> 0.2) @@ -40,7 +40,7 @@ GEM dry-equalizer (~> 0.3) dry-inflector (~> 0.1, >= 0.1.2) dry-logic (~> 1.0, >= 1.0.2) - grape (1.3.3) + grape (1.5.0) activesupport builder dry-types (>= 1.1) @@ -53,13 +53,13 @@ GEM kaminari-grape (1.0.1) grape kaminari-core (~> 1.0) - minitest (5.14.1) + minitest (5.14.2) mustermann (1.1.1) ruby2_keywords (~> 0.0.1) mustermann-grape (1.0.1) mustermann (>= 1.0.0) parallel (1.19.2) - parser (2.7.1.4) + parser (2.7.2.0) ast (~> 2.4.1) rack (2.2.3) rack-accept (0.4.5) @@ -68,13 +68,13 @@ GEM rack (>= 1.0, < 3) rainbow (3.0.0) rake (13.0.1) - regexp_parser (1.7.1) + regexp_parser (1.8.1) rexml (3.2.4) rspec (3.9.0) rspec-core (~> 3.9.0) rspec-expectations (~> 3.9.0) rspec-mocks (~> 3.9.0) - rspec-core (3.9.2) + rspec-core (3.9.3) rspec-support (~> 3.9.3) rspec-expectations (3.9.2) diff-lcs (>= 1.2.0, < 2.0) @@ -83,17 +83,17 @@ GEM diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.9.0) rspec-support (3.9.3) - rubocop (0.89.0) + rubocop (0.92.0) parallel (~> 1.10) - parser (>= 2.7.1.1) + parser (>= 2.7.1.5) rainbow (>= 2.2.2, < 4.0) regexp_parser (>= 1.7) rexml - rubocop-ast (>= 0.1.0, < 1.0) + rubocop-ast (>= 0.5.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 1.4.0, < 2.0) - rubocop-ast (0.3.0) - parser (>= 2.7.1.4) + rubocop-ast (0.7.1) + parser (>= 2.7.1.5) ruby-progressbar (1.10.1) ruby2_keywords (0.0.2) thread_safe (0.3.6) diff --git a/README.md b/README.md index b8d9124..64eaf68 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,6 @@ class MyApi < Grape::API resource :posts do desc 'Return a list of posts.' - # Annotate action with `paginate`. # This will add three optional params: page, per_page, and offset # # You can optionally overwrite the default :per_page setting (10) @@ -45,8 +44,9 @@ class MyApi < Grape::API # You can disable the offset parameter from appearing in the API # documentation by setting it to false. # - paginate per_page: 20, max_per_page: 30, offset: 5 - + params do + use :pagination, per_page: 20, max_per_page: 30, offset: 5 + end get do posts = Post.where(...) diff --git a/lib/grape/kaminari.rb b/lib/grape/kaminari.rb index 3c15c97..6c41ebf 100644 --- a/lib/grape/kaminari.rb +++ b/lib/grape/kaminari.rb @@ -9,10 +9,30 @@ module Kaminari included do helpers HelperMethods - base_instance.extend DSLMethods end module HelperMethods # :nodoc: + extend Grape::API::Helpers + + params :pagination do |opts = {}| + opts.reverse_merge!( + per_page: ::Kaminari.config.default_per_page || 10, + max_per_page: ::Kaminari.config.max_per_page, + offset: 0, + ) + + optional :page, type: Integer, default: 1, + desc: 'Page offset to fetch.' + optional :per_page, type: Integer, default: opts[:per_page], + desc: 'Number of results to return per page.', + max_value: opts[:max_per_page] + + if opts[:offset].is_a?(Integer) + optional :offset, type: Integer, default: opts[:offset], + desc: 'Pad a number of results.' + end + end + def paginate(collection) collection.page(params[:page].to_i) .per(params[:per_page].to_i) @@ -30,24 +50,12 @@ def paginate(collection) end module DSLMethods # :nodoc: - def paginate(**options) - options.reverse_merge!( - per_page: ::Kaminari.config.default_per_page || 10, - max_per_page: ::Kaminari.config.max_per_page, - offset: 0, - ) + def paginate(opts = {}) params do - optional :page, type: Integer, default: 1, - desc: 'Page offset to fetch.' - optional :per_page, type: Integer, default: options[:per_page], - desc: 'Number of results to return per page.', - max_value: options[:max_per_page] - if options[:offset].is_a? Numeric - optional :offset, type: Integer, default: options[:offset], - desc: 'Pad a number of results.' - end + use(:pagination, opts) end end end + Grape::API::Instance.extend(DSLMethods) end end diff --git a/lib/grape/kaminari/version.rb b/lib/grape/kaminari/version.rb index dabf084..53b966b 100644 --- a/lib/grape/kaminari/version.rb +++ b/lib/grape/kaminari/version.rb @@ -1,5 +1,5 @@ module Grape module Kaminari - VERSION = '0.3.0'.freeze + VERSION = '0.4.0'.freeze end end diff --git a/spec/kaminari_spec.rb b/spec/kaminari_spec.rb index b81a78e..1b5e595 100644 --- a/spec/kaminari_spec.rb +++ b/spec/kaminari_spec.rb @@ -1,130 +1,131 @@ require 'spec_helper' -class UnPaginatedAPI < Grape::API - # Intentionally not including Grape::Kaminari -end - class PaginatedAPI < Grape::API include Grape::Kaminari end describe Grape::Kaminari do - describe 'unpaginated api' do - subject { Class.new(UnPaginatedAPI) } + subject { Class.new(PaginatedAPI) } - it 'raises an error' do - expect { subject.paginate }.to raise_error(NoMethodError, /undefined method `paginate' for/i) - end + def declared_params + subject.namespace_stackable(:declared_params).flatten end - describe 'default paginated api' do - subject { Class.new(PaginatedAPI) } - - it 'adds to declared parameters' do - subject.paginate - expect(subject.inheritable_setting.route[:declared_params]).to eq(%i[page per_page offset]) - end - - describe 'descriptions, validation, and defaults' do - let(:params) { subject.routes.first.params } - - before do - subject.paginate - subject.get('/') {} - end - - it 'does not require :page' do - expect(params['page'][:required]).to eq(false) - end - - it 'does not require :per_page' do - expect(params['per_page'][:required]).to eq(false) - end + it 'adds to declared parameters' do + subject.params { use :pagination } + expect(declared_params).to eq(%i[page per_page offset]) + end - it 'does not require :offset' do - expect(params['offset'][:required]).to eq(false) - end + it 'may exclude :offset' do + subject.params { use :pagination, offset: false } + expect(declared_params).to eq(%i[page per_page]) + end - it 'describes :page' do - expect(params['page'][:desc]).to eq('Page offset to fetch.') - end + it 'should support legacy declarations' do + subject.paginate + expect(declared_params).to eq(%i[page per_page offset]) + end - it 'describes :per_page' do - expect(params['per_page'][:desc]).to eq('Number of results to return per page.') - end + it 'should not stumble across repeated declarations' do + subject.paginate offset: false + subject.params do + optional :extra + end + expect(declared_params).to eq(%i[page per_page extra]) + end - it 'describes :offset' do - expect(params['offset'][:desc]).to eq('Pad a number of results.') - end + describe 'descriptions, validation, and defaults' do + let(:params) { subject.routes.first.params } - it 'validates :page as Integer' do - expect(params['page'][:type]).to eq('Integer') - end + before do + subject.params { use :pagination } + subject.get('/') {} + end - it 'validates :per_page as Integer' do - expect(params['per_page'][:type]).to eq('Integer') - end + it 'does not require :page' do + expect(params['page'][:required]).to eq(false) + end - it 'validates :offset as Integer' do - expect(params['offset'][:type]).to eq('Integer') - end + it 'does not require :per_page' do + expect(params['per_page'][:required]).to eq(false) + end - it 'defaults :page to 1' do - expect(params['page'][:default]).to eq(1) - end + it 'does not require :offset' do + expect(params['offset'][:required]).to eq(false) + end - it 'defaults :per_page to Kaminari.config.default_per_page' do - expect(params['per_page'][:default]).to eq(::Kaminari.config.default_per_page) - end + it 'describes :page' do + expect(params['page'][:desc]).to eq('Page offset to fetch.') + end - it 'defaults :offset to 0' do - expect(params['offset'][:default]).to eq(0) - end + it 'describes :per_page' do + expect(params['per_page'][:desc]).to eq('Number of results to return per page.') end - end - describe 'custom paginated api' do - subject { Class.new(PaginatedAPI) } - let(:app) { subject } - let(:params) { subject.routes.first.params } + it 'describes :offset' do + expect(params['offset'][:desc]).to eq('Pad a number of results.') + end - before do - subject.paginate per_page: 99, max_per_page: 999, offset: 9 - subject.get('/') {} + it 'validates :page as Integer' do + expect(params['page'][:type]).to eq('Integer') end - it 'defaults :per_page to customized value' do - expect(params['per_page'][:default]).to eq(99) + it 'validates :per_page as Integer' do + expect(params['per_page'][:type]).to eq('Integer') end - it 'succeeds when :per_page is within :max_value' do - get('/', page: 1, per_page: 999) - expect(last_response.status).to eq(200) + it 'validates :offset as Integer' do + expect(params['offset'][:type]).to eq('Integer') end - it 'ensures :per_page is within :max_value' do - get('/', page: 1, per_page: 1_000) - expect(last_response.status).to eq(400) - expect(last_response.body).to match(/per_page must be less than or equal 999/) + it 'defaults :page to 1' do + expect(params['page'][:default]).to eq(1) end - it 'ensures :per_page is numeric' do - get('/', page: 1, per_page: 'foo') - expect(last_response.status).to eq(400) - expect(last_response.body).to match(/per_page is invalid/) + it 'defaults :per_page to Kaminari.config.default_per_page' do + expect(params['per_page'][:default]).to eq(::Kaminari.config.default_per_page) end - it 'defaults :offset to customized value' do - expect(params['offset'][:default]).to eq(9) + it 'defaults :offset to 0' do + expect(params['offset'][:default]).to eq(0) end end +end - describe 'paginated api without :offset' do - subject { Class.new(PaginatedAPI) } +describe 'custom paginated api' do + subject { Class.new(PaginatedAPI) } + let(:app) { subject } + let(:params) { subject.routes.first.params } - it 'excludes :offset from declared params' do - subject.paginate offset: false - expect(subject.inheritable_setting.route[:declared_params]).not_to include(:offset) + before do + subject.params do + use :pagination, per_page: 99, max_per_page: 999, offset: 9 end + subject.get('/') {} + end + + it 'defaults :per_page to customized value' do + expect(params['per_page'][:default]).to eq(99) + end + + it 'succeeds when :per_page is within :max_value' do + get('/', page: 1, per_page: 999) + expect(last_response.status).to eq(200) + end + + it 'ensures :per_page is within :max_value' do + get('/', page: 1, per_page: 1_000) + expect(last_response.status).to eq(400) + expect(last_response.body).to match(/per_page must be less than or equal 999/) + end + + it 'ensures :per_page is numeric' do + get('/', page: 1, per_page: 'foo') + expect(last_response.status).to eq(400) + expect(last_response.body).to match(/per_page is invalid/) + end + + it 'defaults :offset to customized value' do + expect(params['offset'][:default]).to eq(9) end end diff --git a/spec/paginate_helper_spec.rb b/spec/paginate_helper_spec.rb index ef4d6ec..57b9ef5 100644 --- a/spec/paginate_helper_spec.rb +++ b/spec/paginate_helper_spec.rb @@ -4,18 +4,24 @@ class PaginatedAPI < Grape::API include Grape::Kaminari - paginate + params do + use :pagination + end get '' do paginate(Kaminari.paginate_array((1..10).to_a)) end - paginate offset: false + params do + use :pagination, offset: false + end get 'no-offset' do paginate(Kaminari.paginate_array((1..10).to_a)) end resource :sub do - paginate per_page: 2 + params do + use :pagination, per_page: 2 + end get '/' do paginate(Kaminari.paginate_array((1..10).to_a)) end