Skip to content

Commit

Permalink
fix: non-facet aggregations use correct countQuery to get total numbe…
Browse files Browse the repository at this point in the history
…r of documents.

Fixes issues where `useFacet` cannot be true (e.g. use of `$search` in a pipeline) returning only a single page of results as seen aravindnc#60
  • Loading branch information
MattLicense committed Jul 17, 2024
1 parent 01fe8c1 commit 05bebfd
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 8 deletions.
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,5 @@ typings/
# next.js build output
.next

dist
dist
.idea/
14 changes: 7 additions & 7 deletions lib/mongoose-aggregate-paginate.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,9 @@ function aggregatePaginate(query, options, callback) {
}

function constructPipelines() {
let cleanedPipeline = pipeline.filter((stage) => stage !== PREPAGINATION_PLACEHOLDER);
let cleanedPipeline = pipeline.filter(
(stage) => stage !== PREPAGINATION_PLACEHOLDER
);

const countPipeline = [...cleanedPipeline, { $count: "count" }];

Expand All @@ -122,7 +124,6 @@ function aggregatePaginate(query, options, callback) {
return [cleanedPipeline, countPipeline];
}


let promise;
if (options.useFacet && !options.countQuery) {
let [pipeline, countPipeline] = constructPipelines();
Expand All @@ -135,14 +136,13 @@ function aggregatePaginate(query, options, callback) {
promise = q
.facet({
docs: pipeline,
count: countPipeline
count: countPipeline,
})
.then(([{ docs, count }]) => [docs, count]);
} else {
const [pipeline, countPipeline] = constructPipelines();

const [pipeline] = constructPipelines();

const countQuery = options.countQuery ? options.countQuery : this.aggregate(pipeline);
const countQuery = options.countQuery ? options.countQuery : countPipeline;

if (allowDiskUse) {
countQuery.allowDiskUse(true);
Expand Down Expand Up @@ -236,4 +236,4 @@ function aggregatePaginate(query, options, callback) {

module.exports = aggregatePaginate;

module.exports.PREPAGINATION_PLACEHOLDER = PREPAGINATION_PLACEHOLDER;
module.exports.PREPAGINATION_PLACEHOLDER = PREPAGINATION_PLACEHOLDER;
39 changes: 39 additions & 0 deletions tests/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,45 @@ describe("mongoose-paginate", function () {
expect(result.pageCount).to.equal(5);
});
});

it("without facet", () => {
// part of the reason for this test is due to working the usage of $search in the aggregation pipeline which is
// not supported with facets. The building of a query without facets previously had a bug that meant the pagination
// details were based on a single page of results, rather than the full count of results from a pipeline. While
// this is a basic test it verifies the pagination still returns the correct result while not using facets.
var aggregate = Book.aggregate([
{
$match: {
title: {
$in: [/Book/i],
},
},
},
{
$sort: {
date: 1,
},
},
]);
var options = {
limit: 10,
page: 2,
useFacet: false,
};
return Book.aggregatePaginate(aggregate, options).then((result) => {
expect(result.docs).to.have.length(10);
expect(result.docs[0].title).to.equal("Book #41");
expect(result.totalDocs).to.equal(100);
expect(result.limit).to.equal(10);
expect(result.page).to.equal(2);
expect(result.pagingCounter).to.equal(41);
expect(result.hasPrevPage).to.equal(true);
expect(result.hasNextPage).to.equal(true);
expect(result.prevPage).to.equal(1);
expect(result.nextPage).to.equal(3);
expect(result.totalPages).to.equal(10);
});
});
});

after(async function () {
Expand Down

0 comments on commit 05bebfd

Please sign in to comment.