Merge pull request #1570 from bmorrall/fix-last-pagination-page-size

Fixed pagination issue with last page size

CHANGELOG.md

@@ -21,6 +21,7 @@ Features:
 - [#1340](https://github.com/rails-api/active_model_serializers/pull/1340) Add support for resource-level meta. (@beauby)
 
 Fixes:
+- [#1570](https://github.com/rails-api/active_model_serializers/pull/1570) Fixed pagination issue with last page size. (@bmorrall)
 - [#1516](https://github.com/rails-api/active_model_serializers/pull/1516) No longer return a nil href when only
   adding meta to a relationship link. (@groyoh)
 - [#1458](https://github.com/rails-api/active_model_serializers/pull/1458) Preserve the serializer

lib/active_model_serializers/adapter/json_api/pagination_links.rb

@@ -12,8 +12,9 @@ module ActiveModelSerializers
         end
 
         def serializable_hash(options = {})
+          per_page = collection.try(:per_page) || collection.try(:limit_value) || collection.size
           pages_from.each_with_object({}) do |(key, value), hash|
-            params = query_parameters.merge(page: { number: value, size: collection.size }).to_query
+            params = query_parameters.merge(page: { number: value, size: per_page }).to_query
 
             hash[key] = "#{url(options)}?#{params}"
           end

test/adapter/json_api/pagination_links_test.rb

@@ -15,7 +15,9 @@ module ActiveModelSerializers
           @array = [
             Profile.new({ id: 1, name: 'Name 1', description: 'Description 1', comments: 'Comments 1' }),
             Profile.new({ id: 2, name: 'Name 2', description: 'Description 2', comments: 'Comments 2' }),
-            Profile.new({ id: 3, name: 'Name 3', description: 'Description 3', comments: 'Comments 3' })
+            Profile.new({ id: 3, name: 'Name 3', description: 'Description 3', comments: 'Comments 3' }),
+            Profile.new({ id: 4, name: 'Name 4', description: 'Description 4', comments: 'Comments 4' }),
+            Profile.new({ id: 5, name: 'Name 5', description: 'Description 5', comments: 'Comments 5' })
           ]
         end
 
@@ -32,19 +34,21 @@ module ActiveModelSerializers
           ActiveModel::SerializableResource.new(paginated_collection, options)
         end
 
-        def using_kaminari
+        def using_kaminari(page = 2)
-          Kaminari.paginate_array(@array).page(2).per(1)
+          Kaminari.paginate_array(@array).page(page).per(2)
         end
 
-        def using_will_paginate
+        def using_will_paginate(page = 2)
-          @array.paginate(page: 2, per_page: 1)
+          @array.paginate(page: page, per_page: 2)
         end
 
         def data
           { data: [
               { id: '1', type: 'profiles', attributes: { name: 'Name 1', description: 'Description 1' } },
               { id: '2', type: 'profiles', attributes: { name: 'Name 2', description: 'Description 2' } },
-              { id: '3', type: 'profiles', attributes: { name: 'Name 3', description: 'Description 3' } }
+              { id: '3', type: 'profiles', attributes: { name: 'Name 3', description: 'Description 3' } },
+              { id: '4', type: 'profiles', attributes: { name: 'Name 4', description: 'Description 4' } },
+              { id: '5', type: 'profiles', attributes: { name: 'Name 5', description: 'Description 5' } }
             ]
           }
         end
@@ -52,11 +56,21 @@ module ActiveModelSerializers
         def links
           {
             links: {
-              self: "#{URI}?page%5Bnumber%5D=2&page%5Bsize%5D=1",
+              self: "#{URI}?page%5Bnumber%5D=2&page%5Bsize%5D=2",
-              first: "#{URI}?page%5Bnumber%5D=1&page%5Bsize%5D=1",
+              first: "#{URI}?page%5Bnumber%5D=1&page%5Bsize%5D=2",
-              prev: "#{URI}?page%5Bnumber%5D=1&page%5Bsize%5D=1",
+              prev: "#{URI}?page%5Bnumber%5D=1&page%5Bsize%5D=2",
-              next: "#{URI}?page%5Bnumber%5D=3&page%5Bsize%5D=1",
+              next: "#{URI}?page%5Bnumber%5D=3&page%5Bsize%5D=2",
-              last: "#{URI}?page%5Bnumber%5D=3&page%5Bsize%5D=1"
+              last: "#{URI}?page%5Bnumber%5D=3&page%5Bsize%5D=2"
+            }
+          }
+        end
+
+        def last_page_links
+          {
+            links: {
+              self: "#{URI}?page%5Bnumber%5D=3&page%5Bsize%5D=2",
+              first: "#{URI}?page%5Bnumber%5D=1&page%5Bsize%5D=2",
+              prev: "#{URI}?page%5Bnumber%5D=2&page%5Bsize%5D=2"
             }
           }
         end
@@ -67,7 +81,7 @@ module ActiveModelSerializers
 
         def expected_response_with_pagination_links
           {}.tap do |hash|
-            hash[:data] = [data.values.flatten.second]
+            hash[:data] = data.values.flatten[2..3]
             hash.merge! links
           end
         end
@@ -75,11 +89,18 @@ module ActiveModelSerializers
         def expected_response_with_pagination_links_and_additional_params
           new_links = links[:links].each_with_object({}) { |(key, value), hash| hash[key] = "#{value}&test=test" }
           {}.tap do |hash|
-            hash[:data] = [data.values.flatten.second]
+            hash[:data] = data.values.flatten[2..3]
             hash.merge! links: new_links
           end
         end
 
+        def expected_response_with_last_page_pagination_links
+          {}.tap do |hash|
+            hash[:data] = [data.values.flatten.last]
+            hash.merge! last_page_links
+          end
+        end
+
         def test_pagination_links_using_kaminari
           adapter = load_adapter(using_kaminari)
 
@@ -102,6 +123,20 @@ module ActiveModelSerializers
             adapter.serializable_hash(@options)
         end
 
+        def test_last_page_pagination_links_using_kaminari
+          adapter = load_adapter(using_kaminari(3))
+
+          mock_request
+          assert_equal expected_response_with_last_page_pagination_links, adapter.serializable_hash(@options)
+        end
+
+        def test_last_page_pagination_links_using_will_paginate
+          adapter = load_adapter(using_will_paginate(3))
+
+          mock_request
+          assert_equal expected_response_with_last_page_pagination_links, adapter.serializable_hash(@options)
+        end
+
         def test_not_showing_pagination_links
           adapter = load_adapter(@array)