diff --git a/src/common/utils/pagination.spec.ts b/src/common/utils/pagination.spec.ts index 128432b5c..fd8366c7a 100644 --- a/src/common/utils/pagination.spec.ts +++ b/src/common/utils/pagination.spec.ts @@ -56,6 +56,24 @@ describe('AutoPaginatable', () => { expect(mockApiCall).not.toHaveBeenCalled(); }); + it('returns initial data without a request when there is no next page', async () => { + const initialData: List = { + object: 'list', + data: [{ id: '1' }, { id: '2' }], + listMetadata: { before: null, after: null }, + }; + + const paginatable = new AutoPaginatable( + initialData, + mockApiCall, + ); + + const result = await paginatable.autoPagination(); + + expect(result).toEqual(initialData.data); + expect(mockApiCall).not.toHaveBeenCalled(); + }); + it('paginates through all pages', async () => { const initialData: List = { object: 'list', @@ -88,6 +106,8 @@ describe('AutoPaginatable', () => { const result = await resultPromise; expect(result).toEqual([ + { id: '1' }, + { id: '2' }, { id: '3' }, { id: '4' }, { id: '5' }, @@ -97,7 +117,7 @@ describe('AutoPaginatable', () => { expect(mockApiCall).toHaveBeenCalledTimes(2); expect(mockApiCall).toHaveBeenNthCalledWith(1, { limit: 100, - after: undefined, + after: 'cursor1', }); expect(mockApiCall).toHaveBeenNthCalledWith(2, { limit: 100, @@ -116,11 +136,6 @@ describe('AutoPaginatable', () => { }; mockApiCall - .mockResolvedValueOnce({ - object: 'list', - data: [{ id: 'u11' }, { id: 'u12' }], - listMetadata: { before: 'u11', after: 'u12' }, - }) .mockResolvedValueOnce({ object: 'list', data: [{ id: 'u13' }, { id: 'u14' }], @@ -140,7 +155,7 @@ describe('AutoPaginatable', () => { const resultPromise = paginatable.autoPagination(); - await jest.advanceTimersByTimeAsync(700); + await jest.advanceTimersByTimeAsync(350); const result = await resultPromise; @@ -152,20 +167,16 @@ describe('AutoPaginatable', () => { { id: 'u15' }, ]); - expect(mockApiCall).toHaveBeenCalledTimes(3); + expect(mockApiCall).toHaveBeenCalledTimes(2); expect(mockApiCall).toHaveBeenNthCalledWith(1, { - limit: 100, - before: 'u21', - }); - expect(mockApiCall).toHaveBeenNthCalledWith(2, { limit: 100, after: 'u12', }); - expect(mockApiCall).toHaveBeenNthCalledWith(3, { + expect(mockApiCall).toHaveBeenNthCalledWith(2, { limit: 100, after: 'u14', }); - for (const call of mockApiCall.mock.calls.slice(1)) { + for (const call of mockApiCall.mock.calls) { expect(call[0]).not.toHaveProperty('before'); } }); @@ -233,7 +244,7 @@ describe('AutoPaginatable', () => { await resultPromise; expect(mockApiCall).toHaveBeenNthCalledWith(1, { - after: undefined, + after: 'cursor1', status: 'active', limit: 100, }); diff --git a/src/common/utils/pagination.ts b/src/common/utils/pagination.ts index dbb21c22b..2bd77c3df 100644 --- a/src/common/utils/pagination.ts +++ b/src/common/utils/pagination.ts @@ -24,23 +24,17 @@ export class AutoPaginatable< return this.list.listMetadata; } - private async *generatePages( - params: PaginationOptions, - ): AsyncGenerator { + private async *generatePages(after: string): AsyncGenerator { + // Follow-up pages always walk forward from the first page, so `before` is dropped const { before, ...options } = this.options; - const result = await this.apiCall({ - ...options, - limit: 100, - ...(params.before ? { before } : {}), - after: params.after, - }); + const result = await this.apiCall({ ...options, limit: 100, after }); yield result.data; if (result.listMetadata.after) { // Delay of 4rps to respect list users rate limits await new Promise((resolve) => setTimeout(resolve, 350)); - yield* this.generatePages({ after: result.listMetadata.after }); + yield* this.generatePages(result.listMetadata.after); } } @@ -49,16 +43,13 @@ export class AutoPaginatable< * Returns the first result if `options.limit` is passed to the first request. */ async autoPagination(): Promise { - if (this.options.limit) { + if (this.options.limit || !this.listMetadata.after) { return this.data; } - const results: ResourceType[] = []; + const results = [...this.data]; - for await (const page of this.generatePages({ - before: this.options.before, - after: this.options.after, - })) { + for await (const page of this.generatePages(this.listMetadata.after)) { results.push(...page); } diff --git a/src/feature-flags/feature-flags.spec.ts b/src/feature-flags/feature-flags.spec.ts index bd368d6ba..b810c00f4 100644 --- a/src/feature-flags/feature-flags.spec.ts +++ b/src/feature-flags/feature-flags.spec.ts @@ -353,7 +353,6 @@ describe('FeatureFlags', () => { }, }; fetchOnce(fixtureWithAfter); - fetchOnce(fixtureWithAfter); fetchOnce(listOrganizationFeatureFlagsFixture); const result = await workos.featureFlags.listOrganizationFeatureFlags({ @@ -362,12 +361,12 @@ describe('FeatureFlags', () => { await result.autoPagination(); - const thirdCallUrl = fetch.mock.calls[2][0]; - const thirdCallParams = Object.fromEntries( - new URL(String(thirdCallUrl)).searchParams, + const secondCallUrl = fetch.mock.calls[1][0]; + const secondCallParams = Object.fromEntries( + new URL(String(secondCallUrl)).searchParams, ); - expect(thirdCallParams).not.toHaveProperty('organizationId'); - expect(thirdCallParams).toHaveProperty('after', 'flag_next_page'); + expect(secondCallParams).not.toHaveProperty('organizationId'); + expect(secondCallParams).toHaveProperty('after', 'flag_next_page'); }); }); }); diff --git a/src/pipes/pipes-requests.spec.ts b/src/pipes/pipes-requests.spec.ts index 99a7910c6..1102b8bbc 100644 --- a/src/pipes/pipes-requests.spec.ts +++ b/src/pipes/pipes-requests.spec.ts @@ -42,7 +42,6 @@ beforeEach(() => fetch.resetMocks()); describe('Pipes request contracts', () => { it('preserves the ownership filter across automatic pagination', async () => { - fetchOnce(listDataIntegrationFixture); fetchOnce({ ...listDataIntegrationFixture, list_metadata: { before: null, after: 'next' }, @@ -60,7 +59,6 @@ describe('Pipes request contracts', () => { ), ).toEqual([ { ownership: 'organization', order: 'desc' }, - { ownership: 'organization', order: 'desc', limit: '100' }, { ownership: 'organization', order: 'desc', limit: '100', after: 'next' }, ]); });