Support defining paginator on resource #92
Conversation
|
|
||
| if JSONAPI.configuration.top_level_meta_include_page_count | ||
| data[:page_count] = page_count_for(data[:record_count]) | ||
| data[:page_count] = apply_pagination?(options) ? page_count_for(data[:record_count]) : 1 |
There was a problem hiding this comment.
If JSONAPI.configuration.top_level_meta_include_page_count is true but apply_pagination? is false, then page count should always equal 1
| def paginator | ||
| @paginator ||= paginator_klass.new(page_params) | ||
| @paginator ||= begin | ||
| paginator_klass = "#{resource_paginator_name}_paginator".classify.constantize |
There was a problem hiding this comment.
I changed this from being its own method to living inside paginator because a) it's not used anywhere else and b) I thought it might confusing to have paginator, paginator_klass, and resource_paginator_name methods.
| context 'with "page"' do | ||
| context 'when using "paged" paginator' do | ||
| before(:all) do | ||
| JSONAPI.configuration.default_paginator = :paged |
There was a problem hiding this comment.
Need to change the resource paginator instead of the default config because of the way @request.resource_klass._paginator works in Pagination. If @paginator has not been set on the resource, then it is set to the default config paginator. That's all good for the first set of paginator tests, but when it moves on to the next ones and updates the default paginator setting, @paginator for UserResource is still set to what it was previously.
So, in the course of running the "paged" tests, UserResource._paginator gets set to :paged, even if you never updated the UserResource directly & changed the default paginator setting. When it gets to the "offset" tests and you update the default paginator to "offset", then you'll get an error because UserResource._paginator is still set as :paged.
Basically, this change is solving an issue with the tests themselves, not an issue being raised by the code.
| end | ||
|
|
||
| describe '#count_pages_for' do | ||
| describe '#page_count_for' do |
There was a problem hiding this comment.
Changed this to match the corresponding method name
|
@phantomphildius Would be great if you could take a look as well! |
2 participants
Updates Pagination/Formatters to check the resource paginator instead of the default paginator. Currently, if you've set a paginator directly on your resource, it's ignored in favor of the default.
@tiagopog Could you please take a look at this for me? Let me know if you have any questions or if there's anything you'd like me to add / change.
Thanks!