I've found an issue in the way pagination params are added to generated URLs. It is easiest to describe in the context of your tests.
On this line of top_level_links_test.rb, you set a custom page_param to page[number]. And when you call Article.all on this line (currently 210), it works.
However, if you change that line to articles = Article.paginate(page: 1).all, then an extra page[] is wrapped around the param, so it becomes ?page[page[number]]=1 in the query string.
I've traced the issue to this line on query/builder.rb.
The problem is that we can't count on whether json_api_client is going to wrap page_param or per_page_param in page[]. I'd like it to be one way or the other.
I'm happy to fix it if you can give me direction as to which way you'd like it to go. It will likely be a breaking change either way. It should either always wrap page params with page[], or never. I'd lean towards always adding page[], since it is that way in the spec. However, we should just be consistent.
Thanks!
I've found an issue in the way pagination params are added to generated URLs. It is easiest to describe in the context of your tests.
On this line of top_level_links_test.rb, you set a custom
page_paramtopage[number]. And when you callArticle.allon this line (currently 210), it works.However, if you change that line to
articles = Article.paginate(page: 1).all, then an extrapage[]is wrapped around the param, so it becomes?page[page[number]]=1in the query string.I've traced the issue to this line on query/builder.rb.
The problem is that we can't count on whether
json_api_clientis going to wrappage_paramorper_page_paraminpage[]. I'd like it to be one way or the other.I'm happy to fix it if you can give me direction as to which way you'd like it to go. It will likely be a breaking change either way. It should either always wrap page params with
page[], or never. I'd lean towards always addingpage[], since it is that way in the spec. However, we should just be consistent.Thanks!