Skip to content

Commit

Permalink
fix(gatsby): correct pagination logic (#32496)
Browse files Browse the repository at this point in the history 
* Revert "fix(gatsby): correct hasNextPage pagination info when resultOffset is provided (#32319)"

This reverts commit 9f8a580

* fix(gatsby): fixed pagination logic

* try to make tests less confusing
  • Loading branch information
@vladar
vladar committed Jul 23, 2021
1 parent c985fa8 commit 2dbe647
Show file tree
Hide file tree
Showing 3 changed files with 119 additions and 70 deletions.
79 changes: 30 additions & 49 deletions packages/gatsby/src/schema/__tests__/pagination.js
View file
@@ -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 }
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -112,33 +112,33 @@ 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,
hasNextPage: false,
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,
Expand All @@ -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 () => {
Expand All @@ -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)
Expand All @@ -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)
Expand Down
89 changes: 87 additions & 2 deletions packages/gatsby/src/schema/__tests__/queries.js
View file
Expand Up @@ -35,15 +35,16 @@ describe(`Query schema`, () => {
let schema
let schemaComposer

const runQuery = query =>
const runQuery = (query, variables) =>
graphql(
schema,
query,
undefined,
withResolverContext({
schema,
schemaComposer,
})
}),
variables
)

beforeAll(async () => {
Expand Down Expand Up @@ -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,
},
},
}
`)
})
})
})
21 changes: 2 additions & 19 deletions packages/gatsby/src/schema/resolvers.ts
View file
Expand Up @@ -75,7 +75,7 @@ export function findManyPaginated<TSource, TArgs>(
// 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,
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 2dbe647

Please sign in to comment.