pagination: false does not fully ignore limit
Closed this issue · 13 comments
Not sure if this is a bug, or expected behaviour, I realised because of issue #19, when limit is set to undefined or 0, only metadata is returned.
However this is also true when pagination is set to false, i.e.
{limit:0, pagination:false}
or
{limit: undefined, pagination:false}
I didn't expect this behaviour because the documentation states "If pagination is set to false, it will return all docs without adding limit condition."
@aravindnc Seeing the same issue.
@aravindnc Yes I can confirm this is also the case for us. Any updates on this topic?
Hi,
The bug appears in the following scenario:
myModel.paginate({ pagination: false, limit: undefined })
This will return 0 documents when the expected behaviour would be to return all documents. Passing limit: undefined
should be ignored if pagination: false
is set.
I gave a quick look at the code and you can trigger the bug in your test suite here (tests/index.js
: line 472). I also found another weird behaviour when page: undefined
is passed to the .paginate
method when it should be ignored.
it('all data (without pagination)', function () {
var query = {
title: {
$in: [/Book/i],
},
};
var options = {
pagination: false,
limit: undefined, // -> This is our concern
// page: undefined -> This also breaks the test when I think it should be ignored if pagination=false is defined
};
...
@ypicard Thank you for the detailed comment.
Ideally you should not be passing limit parameter, if pagination is set as false which will return all the data.
Let me know if that works, otherwise I will add a patch to ignore limit/page parameter.
@aravindnc I'd say that the behavior does seem buggy, and why @wjteo filed this ticket and we all end up here : ]
@aravindnc Tested in the application and works well. Is there a default limit of 100, by the way?
@aravindnc Tested in the application and works well. Is there a default limit of 100, by the way?
Great!
Default limit is set as 10.
@aravindnc I meant to ask if there's also some limit to the # of docs even with pagination=false
?
Will you be publishing the change on NPM?
@aravindnc Looks good to me, thank you for your responsiveness!
@aravindnc Thank you for the quick update!
I suppose this ticket can be ✅