New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
pagination-plugin and fetch options on count fix #1914
pagination-plugin and fetch options on count fix #1914
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, but it needs tests to verify the correct behavior of the fix and to ensure it doesn't get re-introduced in the future.
I hope I make correct tests, I try if the count lost standard options and keep custom options |
Nice work! I'll review this properly soon. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me and all tests pass.
Great !! Have you a release date for this PR integration in bookshelf ? I realy need this in my current development |
I will most likely make it to the next version (0.14.0) which will be released soon. |
Introduction
Make the pagination plugin fully compatible with others bookshelf plugins
Motivation
In my project, I use the plugin bookshelf-paranoia to use the soft delete feature. I have a problem with pagination.
If I want to show soft deleted items using pagination, the
fetchPaginate
function return correctly the models but don't return correct count.After resarch, it appears the count part don't use passed fetch options except the transacting parameter.
Proposed solution
I propose to use others fetch options on count part omit all default fetch params (withRelated, ...) to avoid problems