Merge pull request #374 from mixmaxhq/jordins/PG-686-fix-pagination-error

fix: pagination error when paginatedField is not _id

Author: jordins

src/find.ts

@@ -60,6 +60,11 @@ export default async function findWithPagination(
   const removePaginatedFieldInResponse =
     params.fields && !params.fields[params.paginatedField || '_id'];
 
+  // Check if _id was in the original projection before sanitizeParams modifies it.
+  // We'll need this later to determine if _id should be stripped from results.
+  const originalFieldsIncludedId = params.fields && params.fields._id === 1;
+  const paginatedField = params.paginatedField || '_id';
+
   let response;
   if (params.sortCaseInsensitive) {
     // For case-insensitive sorting, we need to work with an aggregation:
@@ -112,6 +117,14 @@ export default async function findWithPagination(
     response.results = _.map(response.results, (result) => _.omit(result, params.paginatedField));
   }
 
+  // When using secondary sort (paginatedField !== '_id'), sanitizeParams adds _id to the projection
+  // for cursor encoding. Remove it from results if the user didn't originally request it.
+  const shouldRemoveIdFromResponse =
+    params.fields && paginatedField !== '_id' && !originalFieldsIncludedId;
+  if (shouldRemoveIdFromResponse) {
+    response.results = _.map(response.results, (result) => _.omit(result, '_id'));
+  }
+
   return response;
 }
 

src/utils/sanitizeParams.ts

@@ -102,6 +102,13 @@ export default async function sanitizeParams(
     if (!params.fields[params.paginatedField]) {
       params.fields[params.paginatedField] = 1;
     }
+
+    // When using secondary sort (paginatedField !== '_id'), we need _id for cursor encoding.
+    // Cursors are encoded as [paginatedFieldValue, _id] tuples (see encodePaginationTokens in query.ts).
+    // Without _id, the cursor would be encoded as a string, breaking pagination on subsequent pages.
+    if (shouldSecondarySortOnId && !params.fields._id) {
+      params.fields._id = 1;
+    }
   }
 
   return params;

test/find.test.ts

@@ -1390,6 +1390,56 @@ describe('find', () => {
       expect(res.hasNext).toBe(true);
     });
 
+    it('pagination works with custom paginatedField and projection without _id', async () => {
+      const collection = t.db.collection('test_paging_custom_fields');
+
+      // First page with projection that doesn't include _id
+      const firstPage = await paging.find(collection, {
+        limit: 2,
+        fields: {
+          counter: 1,
+        },
+        paginatedField: 'timestamp',
+      });
+
+      expect(firstPage.results.length).toEqual(2);
+      expect(firstPage.results[0].counter).toEqual(6);
+      expect(firstPage.results[1].counter).toEqual(5);
+      expect(firstPage.results[0]._id).toBe(undefined); // _id should not be in results
+      expect(firstPage.results[0].timestamp).toBe(undefined); // timestamp should not be in results
+      expect(firstPage.hasNext).toBe(true);
+
+      // Second page - this is where the bug occurs
+      const secondPage = await paging.find(collection, {
+        limit: 2,
+        fields: {
+          counter: 1,
+        },
+        paginatedField: 'timestamp',
+        next: firstPage.next,
+      });
+
+      expect(secondPage.results.length).toEqual(2);
+      expect(secondPage.results[0].counter).toEqual(4);
+      expect(secondPage.results[1].counter).toEqual(3);
+      expect(secondPage.hasNext).toBe(true);
+
+      // Third page
+      const thirdPage = await paging.find(collection, {
+        limit: 2,
+        fields: {
+          counter: 1,
+        },
+        paginatedField: 'timestamp',
+        next: secondPage.next,
+      });
+
+      expect(thirdPage.results.length).toEqual(2);
+      expect(thirdPage.results[0].counter).toEqual(2);
+      expect(thirdPage.results[1].counter).toEqual(1);
+      expect(thirdPage.hasNext).toBe(false);
+    });
+
     it('does not overwrite $or used in a query with next/previous', async () => {
       const collection = t.db.collection('test_paging_custom_fields');
       // First page of 2