Skip to content

Commit b9c6dca

Browse files
committed
fix: pagination error when paginatedField is not _id
When paginatedField !== '_id', cursors must be encoded as [value, _id] tuples. Without _id in projection, cursors were encoded as strings, causing pagination to fail on subsequent pages with "op is not iterable" error.
1 parent 3e4da14 commit b9c6dca

File tree

3 files changed

+70
-0
lines changed

3 files changed

+70
-0
lines changed

src/find.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,11 @@ export default async function findWithPagination(
6060
const removePaginatedFieldInResponse =
6161
params.fields && !params.fields[params.paginatedField || '_id'];
6262

63+
// Check if _id was in the original projection before sanitizeParams modifies it.
64+
// We'll need this later to determine if _id should be stripped from results.
65+
const originalFieldsIncludedId = params.fields && params.fields._id === 1;
66+
const paginatedField = params.paginatedField || '_id';
67+
6368
let response;
6469
if (params.sortCaseInsensitive) {
6570
// For case-insensitive sorting, we need to work with an aggregation:
@@ -112,6 +117,14 @@ export default async function findWithPagination(
112117
response.results = _.map(response.results, (result) => _.omit(result, params.paginatedField));
113118
}
114119

120+
// When using secondary sort (paginatedField !== '_id'), sanitizeParams adds _id to the projection
121+
// for cursor encoding. Remove it from results if the user didn't originally request it.
122+
const shouldRemoveIdFromResponse =
123+
params.fields && paginatedField !== '_id' && !originalFieldsIncludedId;
124+
if (shouldRemoveIdFromResponse) {
125+
response.results = _.map(response.results, (result) => _.omit(result, '_id'));
126+
}
127+
115128
return response;
116129
}
117130

src/utils/sanitizeParams.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,13 @@ export default async function sanitizeParams(
102102
if (!params.fields[params.paginatedField]) {
103103
params.fields[params.paginatedField] = 1;
104104
}
105+
106+
// When using secondary sort (paginatedField !== '_id'), we need _id for cursor encoding.
107+
// Cursors are encoded as [paginatedFieldValue, _id] tuples (see encodePaginationTokens in query.ts).
108+
// Without _id, the cursor would be encoded as a string, breaking pagination on subsequent pages.
109+
if (shouldSecondarySortOnId && !params.fields._id) {
110+
params.fields._id = 1;
111+
}
105112
}
106113

107114
return params;

test/find.test.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1390,6 +1390,56 @@ describe('find', () => {
13901390
expect(res.hasNext).toBe(true);
13911391
});
13921392

1393+
it('pagination works with custom paginatedField and projection without _id', async () => {
1394+
const collection = t.db.collection('test_paging_custom_fields');
1395+
1396+
// First page with projection that doesn't include _id
1397+
const firstPage = await paging.find(collection, {
1398+
limit: 2,
1399+
fields: {
1400+
counter: 1,
1401+
},
1402+
paginatedField: 'timestamp',
1403+
});
1404+
1405+
expect(firstPage.results.length).toEqual(2);
1406+
expect(firstPage.results[0].counter).toEqual(6);
1407+
expect(firstPage.results[1].counter).toEqual(5);
1408+
expect(firstPage.results[0]._id).toBe(undefined); // _id should not be in results
1409+
expect(firstPage.results[0].timestamp).toBe(undefined); // timestamp should not be in results
1410+
expect(firstPage.hasNext).toBe(true);
1411+
1412+
// Second page - this is where the bug occurs
1413+
const secondPage = await paging.find(collection, {
1414+
limit: 2,
1415+
fields: {
1416+
counter: 1,
1417+
},
1418+
paginatedField: 'timestamp',
1419+
next: firstPage.next,
1420+
});
1421+
1422+
expect(secondPage.results.length).toEqual(2);
1423+
expect(secondPage.results[0].counter).toEqual(4);
1424+
expect(secondPage.results[1].counter).toEqual(3);
1425+
expect(secondPage.hasNext).toBe(true);
1426+
1427+
// Third page
1428+
const thirdPage = await paging.find(collection, {
1429+
limit: 2,
1430+
fields: {
1431+
counter: 1,
1432+
},
1433+
paginatedField: 'timestamp',
1434+
next: secondPage.next,
1435+
});
1436+
1437+
expect(thirdPage.results.length).toEqual(2);
1438+
expect(thirdPage.results[0].counter).toEqual(2);
1439+
expect(thirdPage.results[1].counter).toEqual(1);
1440+
expect(thirdPage.hasNext).toBe(false);
1441+
});
1442+
13931443
it('does not overwrite $or used in a query with next/previous', async () => {
13941444
const collection = t.db.collection('test_paging_custom_fields');
13951445
// First page of 2

0 commit comments

Comments
 (0)