Skip to content

Commit 9b3ff93

Browse files
author
Roman Snapko
authored
Fix pagination logic to always fill the first page (#1654)
<!-- Ensure the title clearly reflects what was changed. Provide a clear and concise description of the changes made. The PR should only contain the changes related to the issue, and no other unrelated changes. --> Fixes OPS-3092. https://www.loom.com/share/6113676dd8df4a3bb0ddbb5e0c88a3fb
1 parent bf90468 commit 9b3ff93

2 files changed

Lines changed: 142 additions & 2 deletions

File tree

packages/server/api/src/app/helper/pagination/paginator.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,22 @@ export default class Paginator<Entity extends ObjectLiteral> {
9191
public async paginate(
9292
builder: SelectQueryBuilder<Entity>,
9393
): Promise<PagingResult<Entity>> {
94-
const entities = await this.appendPagingQuery(builder).getMany();
95-
const hasMore = entities.length > this.limit;
94+
let entities = await this.appendPagingQuery(builder).getMany();
95+
let hasMore = entities.length > this.limit;
96+
97+
if (
98+
this.hasBeforeCursor() &&
99+
!this.hasAfterCursor() &&
100+
entities.length < this.limit
101+
) {
102+
this.afterCursor = null;
103+
this.beforeCursor = null;
104+
this.nextAfterCursor = null;
105+
this.nextBeforeCursor = null;
106+
107+
entities = await this.appendPagingQuery(builder).getMany();
108+
hasMore = entities.length > this.limit;
109+
}
96110

97111
if (hasMore) {
98112
entities.splice(entities.length - 1, 1);

packages/server/api/test/integration/helper/pagination/paginator.integration.test.ts

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,33 @@ const TestFlowVersionEntity = new EntitySchema({
7373
},
7474
});
7575

76+
const FOUR_RUNS_TEST_DATA = [
77+
{
78+
id: 'run1',
79+
created: '2025-01-01 08:51:00.880',
80+
projectId: 'proj1',
81+
status: 'SUCCEEDED',
82+
},
83+
{
84+
id: 'run2',
85+
created: '2025-01-01 08:51:00.852',
86+
projectId: 'proj1',
87+
status: 'RUNNING',
88+
},
89+
{
90+
id: 'run3',
91+
created: '2025-01-01 08:51:00.123',
92+
projectId: 'proj1',
93+
status: 'SUCCEEDED',
94+
},
95+
{
96+
id: 'run4',
97+
created: '2025-01-01 08:50:59.999',
98+
projectId: 'proj1',
99+
status: 'FAILED',
100+
},
101+
];
102+
76103
describe('Paginator Integration Tests', () => {
77104
let dataSource: DataSource;
78105

@@ -284,6 +311,105 @@ describe('Paginator Integration Tests', () => {
284311
});
285312

286313
describe('Edge Cases', () => {
314+
describe('refetch when backward result is shorter than limit', () => {
315+
test.each([3, 4])(
316+
'returns correct forward window with limit %i',
317+
async (limit) => {
318+
const testData = FOUR_RUNS_TEST_DATA;
319+
320+
for (const data of testData) {
321+
await dataSource
322+
.createQueryBuilder()
323+
.insert()
324+
.into('test_flow_runs')
325+
.values(data)
326+
.execute();
327+
}
328+
329+
const queryBase = () =>
330+
dataSource
331+
.createQueryBuilder(TestFlowRunEntity, 'fr')
332+
.where('fr.projectId = :projectId', { projectId: 'proj1' });
333+
334+
const paginator1 = new Paginator(TestFlowRunEntity);
335+
paginator1.setAlias('fr');
336+
paginator1.setOrder(Order.DESC);
337+
paginator1.setLimit(2);
338+
const page1 = await paginator1.paginate(queryBase());
339+
340+
const paginator2 = new Paginator(TestFlowRunEntity);
341+
paginator2.setAlias('fr');
342+
paginator2.setOrder(Order.DESC);
343+
paginator2.setLimit(2);
344+
paginator2.setAfterCursor(page1.cursor.afterCursor!);
345+
const page2 = await paginator2.paginate(queryBase());
346+
347+
const paginatorBack = new Paginator(TestFlowRunEntity);
348+
paginatorBack.setAlias('fr');
349+
paginatorBack.setOrder(Order.DESC);
350+
paginatorBack.setLimit(limit);
351+
paginatorBack.setBeforeCursor(page2.cursor.beforeCursor!);
352+
353+
const backPage = await paginatorBack.paginate(queryBase());
354+
355+
const allIds = ['run1', 'run2', 'run3', 'run4'];
356+
const expectedIds = allIds.slice(0, Math.min(limit, allIds.length));
357+
358+
expect(backPage.data.map((d) => d.id)).toEqual(expectedIds);
359+
},
360+
);
361+
362+
test.each([
363+
{ limit: 3, expectAfterDefined: true },
364+
{ limit: 4, expectAfterDefined: false },
365+
])(
366+
'sets expected cursors with limit $limit',
367+
async ({ limit, expectAfterDefined }) => {
368+
const testData = FOUR_RUNS_TEST_DATA;
369+
370+
for (const data of testData) {
371+
await dataSource
372+
.createQueryBuilder()
373+
.insert()
374+
.into('test_flow_runs')
375+
.values(data)
376+
.execute();
377+
}
378+
379+
const queryBase = () =>
380+
dataSource
381+
.createQueryBuilder(TestFlowRunEntity, 'fr')
382+
.where('fr.projectId = :projectId', { projectId: 'proj1' });
383+
384+
const paginator1 = new Paginator(TestFlowRunEntity);
385+
paginator1.setAlias('fr');
386+
paginator1.setOrder(Order.DESC);
387+
paginator1.setLimit(2);
388+
const page1 = await paginator1.paginate(queryBase());
389+
390+
const paginator2 = new Paginator(TestFlowRunEntity);
391+
paginator2.setAlias('fr');
392+
paginator2.setOrder(Order.DESC);
393+
paginator2.setLimit(2);
394+
paginator2.setAfterCursor(page1.cursor.afterCursor!);
395+
const page2 = await paginator2.paginate(queryBase());
396+
397+
const paginatorBack = new Paginator(TestFlowRunEntity);
398+
paginatorBack.setAlias('fr');
399+
paginatorBack.setOrder(Order.DESC);
400+
paginatorBack.setLimit(limit);
401+
paginatorBack.setBeforeCursor(page2.cursor.beforeCursor!);
402+
const backPage = await paginatorBack.paginate(queryBase());
403+
404+
if (expectAfterDefined) {
405+
expect(backPage.cursor.afterCursor).toBeDefined();
406+
} else {
407+
expect(backPage.cursor.afterCursor).toBeNull();
408+
}
409+
expect(backPage.cursor.beforeCursor).toBeNull();
410+
},
411+
);
412+
});
287413
test('should handle empty result set', async () => {
288414
const paginator = new Paginator(TestFlowRunEntity);
289415
paginator.setAlias('fr');

0 commit comments

Comments
 (0)