Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(gatsby): correct pagination logic #32496

Merged
merged 3 commits into from Jul 23, 2021
Merged

fix(gatsby): correct pagination logic #32496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
74bed1a
Revert "fix(gatsby): correct hasNextPage pagination info when resultO…
vladar Jul 23, 2021
1d19c08
fix(gatsby): fixed pagination logic
vladar Jul 23, 2021
45bd8a4
try to make tests less confusing
vladar Jul 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
79 changes: 30 additions & 49 deletions packages/gatsby/src/schema/__tests__/pagination.js
View file
Open in desktop
@@ -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
Open in desktop
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
Open in desktop
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