From c6d370dbb4782b4a9851b202d231c6133fb12e4d Mon Sep 17 00:00:00 2001 From: GatsbyJS Bot Date: Mon, 26 Jul 2021 04:59:53 -0400 Subject: [PATCH] fix(gatsby): correct pagination logic (#32496) (#32507) * Revert "fix(gatsby): correct hasNextPage pagination info when resultOffset is provided (#32319)" This reverts commit 9f8a5802 * fix(gatsby): fixed pagination logic * try to make tests less confusing (cherry picked from commit 2dbe6477d88bf6e0138f247dc9dda1bd67a2a613) Co-authored-by: Vladimir Razuvaev --- .../gatsby/src/schema/__tests__/pagination.js | 79 +++++++--------- .../gatsby/src/schema/__tests__/queries.js | 89 ++++++++++++++++++- packages/gatsby/src/schema/resolvers.ts | 21 +---- 3 files changed, 119 insertions(+), 70 deletions(-) diff --git a/packages/gatsby/src/schema/__tests__/pagination.js b/packages/gatsby/src/schema/__tests__/pagination.js index 4518c0b520457..8f36768ef2898 100644 --- a/packages/gatsby/src/schema/__tests__/pagination.js +++ b/packages/gatsby/src/schema/__tests__/pagination.js @@ -1,8 +1,8 @@ const { paginate } = require(`../resolvers`) describe(`Paginate query results`, () => { - const nodes = [{ id: 1 }, { id: 2 }, { id: 3 }, { id: 4 }] - const results = { entries: nodes, totalCount: async () => nodes.length } + const slice = [{ id: 1 }, { id: 2 }, { id: 3 }, { id: 4 }] + const results = { entries: slice, totalCount: async () => 100 } it(`returns results`, async () => { const args = { limit: 1 } @@ -14,15 +14,15 @@ describe(`Paginate query results`, () => { const args = { limit: 3 } const next = paginate(results, args).edges.map(({ next }) => next) const prev = paginate(results, args).edges.map(({ previous }) => previous) - expect(next).toEqual([nodes[1], nodes[2], undefined]) - expect(prev).toEqual([undefined, nodes[0], nodes[1]]) + expect(next).toEqual([slice[1], slice[2], undefined]) + expect(prev).toEqual([undefined, slice[0], slice[1]]) }) it(`returns correct pagination info with limit only`, async () => { const args = { limit: 2 } const { pageInfo, totalCount } = paginate(results, args) expect(typeof totalCount).toBe(`function`) - expect(await totalCount()).toBe(4) + expect(await totalCount()).toBe(100) expect(pageInfo).toEqual({ currentPage: 1, @@ -34,15 +34,15 @@ describe(`Paginate query results`, () => { totalCount: expect.toBeFunction(), }) - expect(await pageInfo.pageCount()).toEqual(2) - expect(await pageInfo.totalCount()).toEqual(4) + expect(await pageInfo.pageCount()).toEqual(50) + expect(await pageInfo.totalCount()).toEqual(100) }) it(`returns correct pagination info with skip and limit`, async () => { const args = { skip: 1, limit: 2 } const { pageInfo, totalCount } = paginate(results, args) expect(typeof totalCount).toBe(`function`) - expect(await totalCount()).toBe(4) + expect(await totalCount()).toBe(100) expect(pageInfo).toEqual({ currentPage: 2, @@ -53,15 +53,15 @@ describe(`Paginate query results`, () => { perPage: 2, totalCount: expect.toBeFunction(), }) - expect(await pageInfo.pageCount()).toBe(3) - expect(await pageInfo.totalCount()).toBe(4) + expect(await pageInfo.pageCount()).toBe(51) + expect(await pageInfo.totalCount()).toBe(100) }) it(`returns correct pagination info with skip and limit`, async () => { const args = { skip: 2, limit: 2 } const { pageInfo, totalCount } = paginate(results, args) expect(typeof totalCount).toBe(`function`) - expect(await totalCount()).toBe(4) + expect(await totalCount()).toBe(100) expect(pageInfo).toEqual({ currentPage: 2, @@ -73,15 +73,15 @@ describe(`Paginate query results`, () => { totalCount: expect.toBeFunction(), }) - expect(await pageInfo.pageCount()).toEqual(2) - expect(await pageInfo.totalCount()).toEqual(4) + expect(await pageInfo.pageCount()).toEqual(50) + expect(await pageInfo.totalCount()).toEqual(100) }) it(`returns correct pagination info with skip only`, async () => { const args = { skip: 1 } const { pageInfo, totalCount } = paginate(results, args) expect(typeof totalCount).toBe(`function`) - expect(await totalCount()).toBe(4) + expect(await totalCount()).toBe(100) expect(pageInfo).toEqual({ currentPage: 2, @@ -93,14 +93,14 @@ describe(`Paginate query results`, () => { totalCount: expect.toBeFunction(), }) expect(await pageInfo.pageCount()).toEqual(2) - expect(await pageInfo.totalCount()).toEqual(4) + expect(await pageInfo.totalCount()).toEqual(100) }) it(`returns correct pagination info with skip > totalCount`, async () => { - const args = { skip: 10 } + const args = { skip: 101 } const { pageInfo, totalCount } = paginate(results, args) expect(typeof totalCount).toBe(`function`) - expect(await totalCount()).toBe(4) + expect(await totalCount()).toBe(100) expect(pageInfo).toEqual({ currentPage: 2, @@ -112,14 +112,14 @@ describe(`Paginate query results`, () => { totalCount: expect.toBeFunction(), }) expect(await pageInfo.pageCount()).toEqual(2) - expect(await pageInfo.totalCount()).toEqual(4) + expect(await pageInfo.totalCount()).toEqual(100) }) it(`returns correct pagination info with limit > totalCount`, async () => { - const args = { limit: 10 } + const args = { limit: 120 } const { pageInfo, totalCount } = paginate(results, args) expect(typeof totalCount).toBe(`function`) - expect(await totalCount()).toBe(4) + expect(await totalCount()).toBe(100) expect(pageInfo).toEqual({ currentPage: 1, @@ -127,18 +127,18 @@ describe(`Paginate query results`, () => { hasPreviousPage: false, itemCount: 4, pageCount: expect.toBeFunction(), - perPage: 10, + perPage: 120, totalCount: expect.toBeFunction(), }) expect(await pageInfo.pageCount()).toBe(1) - expect(await pageInfo.totalCount()).toBe(4) + expect(await pageInfo.totalCount()).toBe(100) }) it(`returns correct pagination info with skip and resultOffset`, async () => { const args = { skip: 2, resultOffset: 1 } const { pageInfo, totalCount } = paginate(results, args) expect(typeof totalCount).toBe(`function`) - expect(await totalCount()).toBe(4) + expect(await totalCount()).toBe(100) expect(pageInfo).toEqual({ currentPage: 2, @@ -150,45 +150,26 @@ describe(`Paginate query results`, () => { totalCount: expect.toBeFunction(), }) expect(await pageInfo.pageCount()).toEqual(2) - expect(await pageInfo.totalCount()).toEqual(4) + expect(await pageInfo.totalCount()).toEqual(100) }) it(`returns correct pagination info with skip, limit and resultOffset`, async () => { - const args = { skip: 1, limit: 2, resultOffset: 1 } - const { pageInfo, totalCount } = paginate(results, args) - expect(typeof totalCount).toBe(`function`) - expect(await totalCount()).toBe(4) - - expect(pageInfo).toEqual({ - currentPage: 2, - hasNextPage: true, - hasPreviousPage: true, - itemCount: 2, - pageCount: expect.toBeFunction(), - perPage: 2, - totalCount: expect.toBeFunction(), - }) - expect(await pageInfo.pageCount()).toEqual(3) - expect(await pageInfo.totalCount()).toEqual(4) - }) - - it(`returns correct pagination info with skip, limit and resultOffset on the last page`, async () => { const args = { skip: 2, limit: 2, resultOffset: 1 } const { pageInfo, totalCount } = paginate(results, args) expect(typeof totalCount).toBe(`function`) - expect(await totalCount()).toBe(4) + expect(await totalCount()).toBe(100) expect(pageInfo).toEqual({ currentPage: 2, - hasNextPage: false, + hasNextPage: true, hasPreviousPage: true, itemCount: 2, pageCount: expect.toBeFunction(), perPage: 2, totalCount: expect.toBeFunction(), }) - expect(await pageInfo.pageCount()).toEqual(2) - expect(await pageInfo.totalCount()).toEqual(4) + expect(await pageInfo.pageCount()).toEqual(50) + expect(await pageInfo.totalCount()).toEqual(100) }) it(`throws when resultOffset is greater than skip`, async () => { @@ -200,7 +181,7 @@ describe(`Paginate query results`, () => { it(`supports totalCount as function`, async () => { const args = { limit: 1 } - const results = { entries: nodes, totalCount: () => 1000 } + const results = { entries: slice, totalCount: () => 1000 } const { pageInfo, totalCount } = paginate(results, args) expect(await totalCount()).toEqual(1000) expect(await pageInfo.totalCount()).toEqual(1000) @@ -209,7 +190,7 @@ describe(`Paginate query results`, () => { it(`supports totalCount as async function`, async () => { const args = { limit: 1 } const results = { - entries: nodes, + entries: slice, totalCount: async () => Promise.resolve(1100), } const { pageInfo, totalCount } = paginate(results, args) diff --git a/packages/gatsby/src/schema/__tests__/queries.js b/packages/gatsby/src/schema/__tests__/queries.js index 99ca8d32e5f97..b8198249f4728 100644 --- a/packages/gatsby/src/schema/__tests__/queries.js +++ b/packages/gatsby/src/schema/__tests__/queries.js @@ -35,7 +35,7 @@ describe(`Query schema`, () => { let schema let schemaComposer - const runQuery = query => + const runQuery = (query, variables) => graphql( schema, query, @@ -43,7 +43,8 @@ describe(`Query schema`, () => { withResolverContext({ schema, schemaComposer, - }) + }), + variables ) beforeAll(async () => { @@ -1807,4 +1808,88 @@ describe(`Query schema`, () => { expect(results.data).toEqual(expected) }) }) + + describe(`with skip/limit`, () => { + const query = ` + query ($limit: Int!, $skip: Int!) { + allFile(limit: $limit, skip: $skip) { + totalCount + pageInfo { + currentPage + hasNextPage + hasPreviousPage + itemCount + pageCount + perPage + totalCount + } + } + } + ` + it(`return correct pagination info for the first page`, async () => { + const results = await runQuery(query, { limit: 1, skip: 0 }) + expect(results).toMatchInlineSnapshot(` + Object { + "data": Object { + "allFile": Object { + "pageInfo": Object { + "currentPage": 1, + "hasNextPage": true, + "hasPreviousPage": false, + "itemCount": 1, + "pageCount": 3, + "perPage": 1, + "totalCount": 3, + }, + "totalCount": 3, + }, + }, + } + `) + }) + + it(`return correct pagination info for the page in the middle`, async () => { + const results = await runQuery(query, { limit: 1, skip: 1 }) + expect(results).toMatchInlineSnapshot(` + Object { + "data": Object { + "allFile": Object { + "pageInfo": Object { + "currentPage": 2, + "hasNextPage": true, + "hasPreviousPage": true, + "itemCount": 1, + "pageCount": 3, + "perPage": 1, + "totalCount": 3, + }, + "totalCount": 3, + }, + }, + } + `) + }) + + it(`return correct pagination info for the last page`, async () => { + const results = await runQuery(query, { limit: 1, skip: 2 }) + expect(results).toMatchInlineSnapshot(` + Object { + "data": Object { + "allFile": Object { + "pageInfo": Object { + "currentPage": 3, + "hasNextPage": false, + "hasPreviousPage": true, + "itemCount": 1, + "pageCount": 3, + "perPage": 1, + "totalCount": 3, + }, + "totalCount": 3, + }, + }, + } + `) + }) + }) }) diff --git a/packages/gatsby/src/schema/resolvers.ts b/packages/gatsby/src/schema/resolvers.ts index b89e47d06f531..55c35e0135ae3 100644 --- a/packages/gatsby/src/schema/resolvers.ts +++ b/packages/gatsby/src/schema/resolvers.ts @@ -75,7 +75,7 @@ export function findManyPaginated( // Apply paddings for pagination // (for previous/next node and also to detect if there is a previous/next page) const skip = typeof args.skip === `number` ? Math.max(0, args.skip - 1) : 0 - const limit = typeof args.limit === `number` ? args.limit + 1 : undefined + const limit = typeof args.limit === `number` ? args.limit + 2 : undefined const extendedArgs = { ...args, @@ -288,24 +288,7 @@ export function paginate( } const currentPage = limit ? Math.ceil(skip / limit) + 1 : skip ? 2 : 1 const hasPreviousPage = currentPage > 1 - - let hasNextPage = false - // If limit is not defined, there will never be a next page. - if (limit) { - if (resultOffset > 0) { - // If resultOffset is greater than 0, we need to test if `allItems` contains - // items that should be skipped. - // - // This is represented if the `start` index offset is 0 or less. A start - // greater than 0 means `allItems` contains extra items that would come - // before the skipped items. - hasNextPage = start < 1 - } else { - // If the resultOffset is 0, we can test if `allItems` contains more items - // than the limit after removing the skipped items. - hasNextPage = allItems.length - start > limit - } - } + const hasNextPage = limit ? allItems.length - start > limit : false return { totalCount,