mongoose-paginate-v2 icon indicating copy to clipboard operation
mongoose-paginate-v2 copied to clipboard

Incorrect interaction with 'mongoose-delete'

Open davidsierradz opened this issue 5 years ago • 8 comments

Describe the bug If I pass the Query object from findWithDeleted() method of dsanel/mongoose-delete: Mongoose Soft Delete Plugin to paginate I get a missmatch between the totalDocs and docs.

To Reproduce

  1. Check this gist: mongoose-paginate-v2 and mongoose-delete to reproduce.
  2. Save some documents, then soft-delete them.

From: console.log(await Kitten.findWithDeleted())

[ { deleted: false,
    _id: 5d35e342fb9f2304a3b6df13,
    name: 'Fluffy',
    __v: 0 },
  { deleted: false,
    _id: 5d35e3440489e404b23d8708,
    name: 'Fluffy',
    __v: 0 },
  { deleted: true,
    _id: 5d35e366fae0df04db3e80e1,
    name: 'Fluffy',
    __v: 0 },
  { deleted: true,
    _id: 5d35e36b99a4c304ec877ecb,
    name: 'Fluffy',
    __v: 0 },
  { deleted: true,
    _id: 5d35e389376a390514531488,
    name: 'Fluffy',
    __v: 0 },
  { deleted: true,
    _id: 5d35e3a0379d650536cd4053,
    name: 'Fluffy',
    __v: 0 } ]

From: console.log(await Kitten.paginate(Kitten.findWithDeleted()));

{ docs:
   [ { deleted: false,
       _id: 5d35e342fb9f2304a3b6df13,
       name: 'Fluffy',
       __v: 0 },
     { deleted: false,
       _id: 5d35e3440489e404b23d8708,
       name: 'Fluffy',
       __v: 0 } ],
  totalDocs: 6,
  limit: 10,
  offset: 0,
  hasPrevPage: false,
  hasNextPage: false,
  page: 1,
  totalPages: 1,
  pagingCounter: 1,
  prevPage: null,
  nextPage: null }

Expected behavior I expect all documents from the Query to appear in docs array, also totalDocs and docs.length from paginate should always be the same.

Desktop (please complete the following information):

  • OS: Linux
  • Node: 11

Thanks

Edit: My bad, I'm overwriting Mongoose's Model.find() in mongoose-delete configuration object (overrideMethods: ["find"]), maybe add an option in mongoose-paginate-v2 so we can pass a custom find method to use in mongoose-paginate-v2/index.js#L107?

const mQuery = options.customFind(query, projection, findOptions);

davidsierradz avatar Jul 22 '19 16:07 davidsierradz

@davidsierradz This is not a general use case I guess. So may have to create a new branch for this.

aravindnc avatar Jul 24 '19 02:07 aravindnc

Sure, I'm ok with a fork, but I think this shouldn't be a breaking change:

diff --git a/src/index.js b/src/index.js
index f9d52b8..b2dae57 100644
--- a/src/index.js
+++ b/src/index.js
@@ -39,6 +39,7 @@ const defaultOptions = {
   projection: {},
   select: '',
   options: {},
+  customFind: 'find',
 };
 
 function paginate(query, options, callback) {
@@ -57,6 +58,7 @@ function paginate(query, options, callback) {
     projection,
     select,
     sort,
+    customFind,
   } = options;
 
   const customLabels = {
@@ -104,7 +106,7 @@ function paginate(query, options, callback) {
   const countPromise = this.countDocuments(query).exec();
 
   if (limit) {
-    const mQuery = this.find(query, projection, findOptions);
+    const mQuery = this[customFind](query, projection, findOptions);
     mQuery.select(select);
     mQuery.sort(sort);
     mQuery.lean(lean);

Btw, awesome plugin, thanks!

davidsierradz avatar Jul 24 '19 12:07 davidsierradz

@davidsierradz You're the best! I will add it this to the package if similar request comes again.

aravindnc avatar Jul 24 '19 14:07 aravindnc

i would really appreciate the changes suggested by @davidsierradz !

simnado avatar Feb 18 '21 13:02 simnado

Would really appreciate these changes as well !

fawzisf avatar Feb 08 '22 10:02 fawzisf

Will be adding it this week.

aravindnc avatar Feb 08 '22 15:02 aravindnc

I did a quick try with mongoose-delete which seems to mess up other test cases. So I would appreciate it if someone can do a P.R with a test case. Otherwise, give me some time to work on this.

aravindnc avatar Feb 11 '22 14:02 aravindnc

@aravindnc any updates on this, sir?

ariakm25 avatar Jul 16 '22 08:07 ariakm25