`jsonapi_pagination_meta` produces wrong values if called after calling `jsonapi_paginate`
lpagliari opened this issue ยท 4 comments
When using both jsonapi_paginate
and jsonapi_pagination_meta
to return pagination info, the order when each one is called matters.
Expected Behavior
Calling jsonapi_pagination_meta
after calling jsonapi_paginate
would produce the same results as calling them on the reverse order.
Actual Behavior
jsonapi_pagination_meta
is returning the wrong data if it is called after jsonapi_paginate
.
Steps to Reproduce the Problem
- Create 6 records of a model;
- On the controller, build your result by calling
jsonapi_pagination_meta
afterjsonapi_paginate
. For example:MySerializer.new( objects, links: jsonapi_pagination(objects), meta: jsonapi_pagination_meta(objects), )
- Call the index URL with params
?page[size]=2&page[number]=1
; - The result is:
instead of:
"meta": { "current": 3, "first": 1, "prev": 2 }, "links": { "self": "(...)?page[size]=2&page[number]=1", "current": "(...)?filter=&page[number]=1&page[size]=2&sort=", "next": "(...)?filter=&page[number]=2&page[size]=2&sort=", "last": "(...)?filter=&page[number]=3&page[size]=2&sort=" }
"meta": { "current": 1, "first": 2, "prev": 3 }, "links": { "self": "(...)?page[size]=2&page[number]=1", "current": "(...)?filter=&page[number]=1&page[size]=2&sort=", "next": "(...)?filter=&page[number]=2&page[size]=2&sort=", "last": "(...)?filter=&page[number]=3&page[size]=2&sort=" }
Note that the links
are correct, the error is on meta
.
More Details
The reason seems to be on the fact that we are changing the original page number param on jsonapi_paginate
, so when we call jsonapi_pagination_meta
the original page number is changed, thus returning the wrong meta info:
def jsonapi_pagination(resources)
(...)
pagination.each do |page_name, number|
original_params[:page][:number] = number # <== here is the issue.
# original_params != params, *but* original_params[:page] == params[:page]
links[page_name] = original_url + CGI.unescape(
original_params.to_query
)
end
links
end
The fix seems to be simple: just use a different object for the params. This means simply cloning original_params[:page]
:
*request.path_parameters.keys.map(&:to_s)
).to_unsafe_h.with_indifferent_access
- original_params[:page] ||= {}
+ original_params[:page] = original_params[:page].clone || {}
original_url = request.base_url + request.path + '?'
pagination.each do |page_name, number|
or marshaling the entire params
(to be safer):
original_params = params.except(
*request.path_parameters.keys.map(&:to_s)
).to_unsafe_h.with_indifferent_access
+ original_params = Marshal.load(Marshal.dump(original_params))
original_params[:page] ||= {}
original_url = request.base_url + request.path + '?'
Thanks Luiza! I'll take a look ๐โโ๏ธ
@lpagliari I'll be releasing the fix. Thanks for reporting this.
On another note, based on your example, please consider using the API to handle the pagination. Particularly, you don't need to pass over the links
and meta
to the serializer if you're using the module as part of your controller.
If you're not using Rails, I'd like to learn more about your setup to see if we can provide a similar to Rails integration.
The reasoning behind this is that the serialization library interface should be considered an internal API (I might switch it to something a bit more maintained)...
Wishing you a great end of the week!
I see, thanks @lpagliari
Let me know if you find other things we can improve ๐โโ๏ธ