aravindnc/mongoose-paginate-v2

pagination: false does not fully ignore limit

Closed this issue · 13 comments

wjteo commented

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?

@ypicard @a-kriya Are you saying that if we set pagination:false it still returns data with limit ? Can you please confirm.

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 : ]

Please update the package to v1.3.18 and let me know if the issue persists.

@a-kriya @ypicard

@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!

I've already published to npm couple of hours back.
@wjteo @a-kriya @ypicard
Thanks for pointing out the issue. Really appreciated.

@aravindnc Thank you for the quick update!
I suppose this ticket can be ✅