Skip to content

Commit

Permalink
Prevent out of bounds pages from returning a next link.
Browse files Browse the repository at this point in the history
Also makes sure the prev link is a real page.

Self link is unmodified, it will return a link to the current page
even if it doesn't exist. Seemed to best conform to JSON:API spec.
  • Loading branch information
bcaplan committed Oct 11, 2017
1 parent 4d7c245 commit 36b7960
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ Features:
Fixes:

- [#2022](https://github.com/rails-api/active_model_serializers/pull/2022) Mutation of ActiveModelSerializers::Model now changes the attributes. Originally in [#1984](https://github.com/rails-api/active_model_serializers/pull/1984). (@bf4)
- [#2171](https://github.com/rails-api/active_model_serializers/pull/2171) Prevent the `next` link from getting populated in JSON:API when an out of bounds page is requested. (@bcaplan)
- [#2200](https://github.com/rails-api/active_model_serializers/pull/2200) Fix deserialization of polymorphic relationships. (@dennis95stumm)

Misc:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,16 @@ def last_page_url

def prev_page_url
return nil if collection.current_page == FIRST_PAGE
url_for_page(collection.current_page - FIRST_PAGE)

if collection.current_page > collection.total_pages
url_for_page(collection.total_pages)
else
url_for_page(collection.current_page - FIRST_PAGE)
end
end

def next_page_url
return nil if collection.total_pages == 0 || collection.current_page == collection.total_pages
return nil if collection.total_pages == 0 || collection.current_page >= collection.total_pages
url_for_page(collection.next_page)
end

Expand Down
31 changes: 31 additions & 0 deletions test/adapter/json_api/pagination_links_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,18 @@ def last_page_links
}
end

def out_of_bounds_page_links
{
links: {
self: "#{URI}?page%5Bnumber%5D=4&page%5Bsize%5D=2",
first: "#{URI}?page%5Bnumber%5D=1&page%5Bsize%5D=2",
prev: "#{URI}?page%5Bnumber%5D=3&page%5Bsize%5D=2",
next: nil,
last: "#{URI}?page%5Bnumber%5D=3&page%5Bsize%5D=2"
}
}
end

def expected_response_when_unpaginatable
data
end
Expand Down Expand Up @@ -120,6 +132,13 @@ def expected_response_with_last_page_pagination_links
end
end

def expected_response_with_out_of_bounds_page_pagination_links
{}.tap do |hash|
hash[:data] = []
hash.merge! out_of_bounds_page_links
end
end

def expected_response_with_empty_collection_pagination_links
{}.tap do |hash|
hash[:data] = []
Expand Down Expand Up @@ -174,6 +193,18 @@ def test_last_page_pagination_links_using_will_paginate
assert_equal expected_response_with_last_page_pagination_links, adapter.serializable_hash
end

def test_out_of_bounds_page_pagination_links_using_kaminari
adapter = load_adapter(using_kaminari(4), mock_request)

assert_equal expected_response_with_out_of_bounds_page_pagination_links, adapter.serializable_hash
end

def test_out_of_bounds_page_pagination_links_using_will_paginate
adapter = load_adapter(using_will_paginate(4), mock_request)

assert_equal expected_response_with_out_of_bounds_page_pagination_links, adapter.serializable_hash
end

def test_not_showing_pagination_links
adapter = load_adapter(@array, mock_request)

Expand Down

0 comments on commit 36b7960

Please sign in to comment.