Fix cursor-based pagination with lexicographic ordering for composite fields (#12467)
# Fix cursor-based pagination with lexicographic ordering for composite
fields
## Bug
The existing cursor-based pagination implementation had a bug when
handling composite fields.
When paginating through results sorted by composite fields (like
`fullName` with sub-properties `firstName` and`lastName`), the WHERE
conditions generated for cursor positioning were incorrect, leading to
records being skipped.
The previous implementation was generating wrong WHERE conditions:
For example, when paginating with a cursor like `{ firstName: 'John',
lastName: 'Doe' }`, it would generate:
```sql
WHERE firstName > 'John' AND lastName > 'Doe'
```
This is incorrect because it would miss records like `{ firstName:
'John', lastName: 'Smith' }` which should be included in forward
pagination.
## Fix
Create a new util to use proper lexicographic order when sorting a
composite field.
---------
Co-authored-by: Charles Bochet <charlesBochet@users.noreply.github.com>
Co-authored-by: Charles Bochet <charles@twenty.com>
This commit is contained in:
Raphaël Bosi
@ -2,5 +2,6 @@ export const TEST_PERSON_1_ID = '777a8457-eb2d-40ac-a707-551b615b6980';
|
||||
export const TEST_PERSON_2_ID = '777a8457-eb2d-40ac-a707-551b615b6981';
|
||||
export const TEST_PERSON_3_ID = '777a8457-eb2d-40ac-a707-551b615b6982';
|
||||
export const TEST_PERSON_4_ID = '777a8457-eb2d-40ac-a707-551b615b6983';
|
||||
export const TEST_PERSON_5_ID = '777a8457-eb2d-40ac-a707-551b615b6984';
|
||||
export const NOT_EXISTING_TEST_PERSON_ID =
|
||||
'777a8457-eb2d-40ac-a707-551b615b6990';
|
||||
|
||||
@ -0,0 +1,263 @@
|
||||
import { PERSON_GQL_FIELDS } from 'test/integration/constants/person-gql-fields.constants';
|
||||
import {
|
||||
TEST_PERSON_1_ID,
|
||||
TEST_PERSON_2_ID,
|
||||
TEST_PERSON_3_ID,
|
||||
TEST_PERSON_4_ID,
|
||||
TEST_PERSON_5_ID,
|
||||
} from 'test/integration/constants/test-person-ids.constants';
|
||||
import { createOneOperationFactory } from 'test/integration/graphql/utils/create-one-operation-factory.util';
|
||||
import { findManyOperationFactory } from 'test/integration/graphql/utils/find-many-operation-factory.util';
|
||||
import { makeGraphqlAPIRequest } from 'test/integration/graphql/utils/make-graphql-api-request.util';
|
||||
import { deleteAllRecords } from 'test/integration/utils/delete-all-records';
|
||||
|
||||
describe('GraphQL People Pagination with Composite Field Sorting', () => {
|
||||
beforeAll(async () => {
|
||||
await deleteAllRecords('person');
|
||||
|
||||
const testPeople = [
|
||||
{
|
||||
id: TEST_PERSON_1_ID,
|
||||
firstName: 'Alice',
|
||||
lastName: 'Brown',
|
||||
},
|
||||
{
|
||||
id: TEST_PERSON_2_ID,
|
||||
firstName: 'Alice',
|
||||
lastName: 'Smith',
|
||||
},
|
||||
{
|
||||
id: TEST_PERSON_3_ID,
|
||||
firstName: 'Bob',
|
||||
lastName: 'Johnson',
|
||||
},
|
||||
{
|
||||
id: TEST_PERSON_4_ID,
|
||||
firstName: 'Bob',
|
||||
lastName: 'Williams',
|
||||
},
|
||||
{
|
||||
id: TEST_PERSON_5_ID,
|
||||
firstName: 'Charlie',
|
||||
lastName: 'Davis',
|
||||
},
|
||||
];
|
||||
|
||||
for (const person of testPeople) {
|
||||
const graphqlOperation = createOneOperationFactory({
|
||||
objectMetadataSingularName: 'person',
|
||||
gqlFields: PERSON_GQL_FIELDS,
|
||||
data: {
|
||||
id: person.id,
|
||||
name: {
|
||||
firstName: person.firstName,
|
||||
lastName: person.lastName,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
await makeGraphqlAPIRequest(graphqlOperation).expect(200);
|
||||
}
|
||||
});
|
||||
|
||||
it('should support pagination with fullName composite field in ascending order', async () => {
|
||||
const firstPageOperation = findManyOperationFactory({
|
||||
objectMetadataSingularName: 'person',
|
||||
objectMetadataPluralName: 'people',
|
||||
gqlFields: PERSON_GQL_FIELDS,
|
||||
orderBy: {
|
||||
name: {
|
||||
firstName: 'AscNullsLast',
|
||||
lastName: 'AscNullsLast',
|
||||
},
|
||||
},
|
||||
first: 2,
|
||||
});
|
||||
|
||||
const firstPageResponse =
|
||||
await makeGraphqlAPIRequest(firstPageOperation).expect(200);
|
||||
|
||||
const firstPagePeople = firstPageResponse.body.data.people.edges.map(
|
||||
(edge: any) => edge.node,
|
||||
);
|
||||
const firstPageCursor =
|
||||
firstPageResponse.body.data.people.pageInfo.endCursor;
|
||||
|
||||
expect(firstPagePeople).toHaveLength(2);
|
||||
expect(firstPageResponse.body.data.people.pageInfo.hasNextPage).toBe(true);
|
||||
|
||||
expect(firstPagePeople[0].name.firstName).toBe('Alice');
|
||||
expect(firstPagePeople[0].name.lastName).toBe('Brown');
|
||||
expect(firstPagePeople[1].name.firstName).toBe('Alice');
|
||||
expect(firstPagePeople[1].name.lastName).toBe('Smith');
|
||||
|
||||
const secondPageOperation = findManyOperationFactory({
|
||||
objectMetadataSingularName: 'person',
|
||||
objectMetadataPluralName: 'people',
|
||||
gqlFields: PERSON_GQL_FIELDS,
|
||||
orderBy: {
|
||||
name: {
|
||||
firstName: 'AscNullsLast',
|
||||
lastName: 'AscNullsLast',
|
||||
},
|
||||
},
|
||||
first: 2,
|
||||
after: firstPageCursor,
|
||||
});
|
||||
|
||||
const secondPageResponse =
|
||||
await makeGraphqlAPIRequest(secondPageOperation).expect(200);
|
||||
|
||||
const secondPagePeople = secondPageResponse.body.data.people.edges.map(
|
||||
(edge: any) => edge.node,
|
||||
);
|
||||
|
||||
expect(secondPagePeople).toHaveLength(2);
|
||||
|
||||
expect(secondPagePeople[0].name.firstName).toBe('Bob');
|
||||
expect(secondPagePeople[0].name.lastName).toBe('Johnson');
|
||||
expect(secondPagePeople[1].name.firstName).toBe('Bob');
|
||||
expect(secondPagePeople[1].name.lastName).toBe('Williams');
|
||||
|
||||
const firstPageIds = firstPagePeople.map((p: { id: string }) => p.id);
|
||||
const secondPageIds = secondPagePeople.map((p: { id: string }) => p.id);
|
||||
const intersection = firstPageIds.filter((id: string) =>
|
||||
secondPageIds.includes(id),
|
||||
);
|
||||
|
||||
expect(intersection).toHaveLength(0);
|
||||
|
||||
const thirdPageOperation = findManyOperationFactory({
|
||||
objectMetadataSingularName: 'person',
|
||||
objectMetadataPluralName: 'people',
|
||||
gqlFields: PERSON_GQL_FIELDS,
|
||||
orderBy: {
|
||||
name: {
|
||||
firstName: 'AscNullsLast',
|
||||
lastName: 'AscNullsLast',
|
||||
},
|
||||
},
|
||||
first: 2,
|
||||
after: secondPageResponse.body.data.people.pageInfo.endCursor,
|
||||
});
|
||||
|
||||
const thirdPageResponse =
|
||||
await makeGraphqlAPIRequest(thirdPageOperation).expect(200);
|
||||
|
||||
const thirdPagePeople = thirdPageResponse.body.data.people.edges.map(
|
||||
(edge: any) => edge.node,
|
||||
);
|
||||
|
||||
expect(thirdPagePeople).toHaveLength(1);
|
||||
expect(thirdPagePeople[0].name.firstName).toBe('Charlie');
|
||||
expect(thirdPagePeople[0].name.lastName).toBe('Davis');
|
||||
expect(thirdPageResponse.body.data.people.pageInfo.hasNextPage).toBe(false);
|
||||
});
|
||||
|
||||
it('should support cursor-based pagination with fullName in descending order', async () => {
|
||||
const firstPageOperation = findManyOperationFactory({
|
||||
objectMetadataSingularName: 'person',
|
||||
objectMetadataPluralName: 'people',
|
||||
gqlFields: PERSON_GQL_FIELDS,
|
||||
orderBy: {
|
||||
name: {
|
||||
firstName: 'DescNullsLast',
|
||||
lastName: 'DescNullsLast',
|
||||
},
|
||||
},
|
||||
first: 2,
|
||||
});
|
||||
|
||||
const firstPageResponse =
|
||||
await makeGraphqlAPIRequest(firstPageOperation).expect(200);
|
||||
|
||||
const firstPagePeople = firstPageResponse.body.data.people.edges.map(
|
||||
(edge: any) => edge.node,
|
||||
);
|
||||
|
||||
expect(firstPagePeople).toHaveLength(2);
|
||||
|
||||
expect(firstPagePeople[0].name.firstName).toBe('Charlie');
|
||||
expect(firstPagePeople[0].name.lastName).toBe('Davis');
|
||||
expect(firstPagePeople[1].name.firstName).toBe('Bob');
|
||||
expect(firstPagePeople[1].name.lastName).toBe('Williams');
|
||||
|
||||
const secondPageOperation = findManyOperationFactory({
|
||||
objectMetadataSingularName: 'person',
|
||||
objectMetadataPluralName: 'people',
|
||||
gqlFields: PERSON_GQL_FIELDS,
|
||||
orderBy: {
|
||||
name: {
|
||||
firstName: 'DescNullsLast',
|
||||
lastName: 'DescNullsLast',
|
||||
},
|
||||
},
|
||||
first: 2,
|
||||
after: firstPageResponse.body.data.people.pageInfo.endCursor,
|
||||
});
|
||||
|
||||
const secondPageResponse =
|
||||
await makeGraphqlAPIRequest(secondPageOperation).expect(200);
|
||||
|
||||
const secondPagePeople = secondPageResponse.body.data.people.edges.map(
|
||||
(edge: any) => edge.node,
|
||||
);
|
||||
|
||||
expect(secondPagePeople).toHaveLength(2);
|
||||
|
||||
expect(secondPagePeople[0].name.firstName).toBe('Bob');
|
||||
expect(secondPagePeople[0].name.lastName).toBe('Johnson');
|
||||
expect(secondPagePeople[1].name.firstName).toBe('Alice');
|
||||
expect(secondPagePeople[1].name.lastName).toBe('Smith');
|
||||
});
|
||||
|
||||
it('should support backward pagination with fullName composite field in ascending order', async () => {
|
||||
const allPeopleOperation = findManyOperationFactory({
|
||||
objectMetadataSingularName: 'person',
|
||||
objectMetadataPluralName: 'people',
|
||||
gqlFields: PERSON_GQL_FIELDS,
|
||||
orderBy: {
|
||||
name: {
|
||||
firstName: 'AscNullsLast',
|
||||
lastName: 'AscNullsLast',
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const allPeopleResponse =
|
||||
await makeGraphqlAPIRequest(allPeopleOperation).expect(200);
|
||||
|
||||
const allPeople = allPeopleResponse.body.data.people.edges.map(
|
||||
(edge: any) => edge.node,
|
||||
);
|
||||
const lastPersonCursor =
|
||||
allPeopleResponse.body.data.people.pageInfo.endCursor;
|
||||
|
||||
const backwardPageOperation = findManyOperationFactory({
|
||||
objectMetadataSingularName: 'person',
|
||||
objectMetadataPluralName: 'people',
|
||||
gqlFields: PERSON_GQL_FIELDS,
|
||||
orderBy: {
|
||||
name: {
|
||||
firstName: 'AscNullsLast',
|
||||
lastName: 'AscNullsLast',
|
||||
},
|
||||
},
|
||||
last: 2,
|
||||
before: lastPersonCursor,
|
||||
});
|
||||
|
||||
const backwardPageResponse = await makeGraphqlAPIRequest(
|
||||
backwardPageOperation,
|
||||
).expect(200);
|
||||
|
||||
const backwardPagePeople = backwardPageResponse.body.data.people.edges.map(
|
||||
(edge: any) => edge.node,
|
||||
);
|
||||
|
||||
expect(backwardPagePeople).toHaveLength(2);
|
||||
|
||||
expect(backwardPagePeople[0].id).toBe(allPeople.at(-2)?.id);
|
||||
expect(backwardPagePeople[1].id).toBe(allPeople.at(-3)?.id);
|
||||
});
|
||||
});
|
||||
@ -6,6 +6,12 @@ type FindManyOperationFactoryParams = {
|
||||
objectMetadataPluralName: string;
|
||||
gqlFields: string;
|
||||
filter?: object;
|
||||
orderBy?: object;
|
||||
limit?: number;
|
||||
after?: string;
|
||||
before?: string;
|
||||
first?: number;
|
||||
last?: number;
|
||||
};
|
||||
|
||||
export const findManyOperationFactory = ({
|
||||
@ -13,19 +19,38 @@ export const findManyOperationFactory = ({
|
||||
objectMetadataPluralName,
|
||||
gqlFields,
|
||||
filter = {},
|
||||
orderBy = {},
|
||||
limit,
|
||||
after,
|
||||
before,
|
||||
first,
|
||||
last,
|
||||
}: FindManyOperationFactoryParams) => ({
|
||||
query: gql`
|
||||
query ${capitalize(objectMetadataPluralName)}($filter: ${capitalize(objectMetadataSingularName)}FilterInput) {
|
||||
${objectMetadataPluralName}(filter: $filter) {
|
||||
query ${capitalize(objectMetadataPluralName)}($filter: ${capitalize(objectMetadataSingularName)}FilterInput, $orderBy: [${capitalize(objectMetadataSingularName)}OrderByInput], $limit: Int, $after: String, $before: String, $first: Int, $last: Int) {
|
||||
${objectMetadataPluralName}(filter: $filter, orderBy: $orderBy, limit: $limit, after: $after, before: $before, first: $first, last: $last) {
|
||||
edges {
|
||||
node {
|
||||
${gqlFields}
|
||||
}
|
||||
cursor
|
||||
}
|
||||
pageInfo {
|
||||
hasNextPage
|
||||
hasPreviousPage
|
||||
startCursor
|
||||
endCursor
|
||||
}
|
||||
}
|
||||
}
|
||||
`,
|
||||
variables: {
|
||||
filter,
|
||||
orderBy,
|
||||
limit,
|
||||
after,
|
||||
before,
|
||||
first,
|
||||
last,
|
||||
},
|
||||
});
|
||||
|
||||
@ -1,14 +1,14 @@
|
||||
import { TEST_COMPANY_1_ID } from 'test/integration/constants/test-company-ids.constants';
|
||||
import {
|
||||
TEST_PERSON_1_ID,
|
||||
TEST_PERSON_2_ID,
|
||||
TEST_PERSON_3_ID,
|
||||
TEST_PERSON_4_ID,
|
||||
} from 'test/integration/constants/test-person-ids.constants';
|
||||
import { makeRestAPIRequest } from 'test/integration/rest/utils/make-rest-api-request.util';
|
||||
import { generateRecordName } from 'test/integration/utils/generate-record-name';
|
||||
import { TEST_COMPANY_1_ID } from 'test/integration/constants/test-company-ids.constants';
|
||||
import { deleteAllRecords } from 'test/integration/utils/delete-all-records';
|
||||
import { TEST_PRIMARY_LINK_URL } from 'test/integration/constants/test-primary-link-url.constant';
|
||||
import { makeRestAPIRequest } from 'test/integration/rest/utils/make-rest-api-request.util';
|
||||
import { deleteAllRecords } from 'test/integration/utils/delete-all-records';
|
||||
import { generateRecordName } from 'test/integration/utils/generate-record-name';
|
||||
|
||||
describe('Core REST API Find Many endpoint', () => {
|
||||
const testPersonIds = [
|
||||
@ -262,6 +262,168 @@ describe('Core REST API Find Many endpoint', () => {
|
||||
expect(people).toEqual([...people].sort((a, b) => a.city - b.city));
|
||||
});
|
||||
|
||||
it('should should throw an error when trying to order by a composite field', async () => {
|
||||
await makeRestAPIRequest({
|
||||
method: 'get',
|
||||
path: '/people?order_by=name[AscNullsLast]',
|
||||
}).expect(400);
|
||||
});
|
||||
|
||||
// TODO: Uncomment this test when we support composite fields ordering in the rest api
|
||||
|
||||
// it('should support pagination with fullName composite field ordering', async () => {
|
||||
// await deleteAllRecords('person');
|
||||
|
||||
// const testPeople = [
|
||||
// {
|
||||
// id: TEST_PERSON_1_ID,
|
||||
// firstName: 'Alice',
|
||||
// lastName: 'Brown',
|
||||
// position: 0,
|
||||
// },
|
||||
// {
|
||||
// id: TEST_PERSON_2_ID,
|
||||
// firstName: 'Alice',
|
||||
// lastName: 'Smith',
|
||||
// position: 1,
|
||||
// },
|
||||
// {
|
||||
// id: TEST_PERSON_3_ID,
|
||||
// firstName: 'Bob',
|
||||
// lastName: 'Johnson',
|
||||
// position: 2,
|
||||
// },
|
||||
// {
|
||||
// id: TEST_PERSON_4_ID,
|
||||
// firstName: 'Bob',
|
||||
// lastName: 'Williams',
|
||||
// position: 3,
|
||||
// },
|
||||
// {
|
||||
// id: TEST_PERSON_5_ID,
|
||||
// firstName: 'Charlie',
|
||||
// lastName: 'Davis',
|
||||
// position: 4,
|
||||
// },
|
||||
// ];
|
||||
|
||||
// for (const person of testPeople) {
|
||||
// await makeRestAPIRequest({
|
||||
// method: 'post',
|
||||
// path: '/people',
|
||||
// body: {
|
||||
// id: person.id,
|
||||
// name: {
|
||||
// firstName: person.firstName,
|
||||
// lastName: person.lastName,
|
||||
// },
|
||||
// },
|
||||
// });
|
||||
// }
|
||||
|
||||
// const firstPageResponse = await makeRestAPIRequest({
|
||||
// method: 'get',
|
||||
// path: '/people?order_by=name[AscNullsLast]&limit=2',
|
||||
// }).expect(200);
|
||||
|
||||
// const firstPagePeople = firstPageResponse.body.data.people;
|
||||
// const firstPageCursor = firstPageResponse.body.pageInfo.endCursor;
|
||||
|
||||
// expect(firstPagePeople).toHaveLength(2);
|
||||
// expect(firstPageResponse.body.pageInfo.hasNextPage).toBe(true);
|
||||
|
||||
// expect(firstPagePeople[0].name.firstName).toBe('Alice');
|
||||
// expect(firstPagePeople[0].name.lastName).toBe('Brown');
|
||||
// expect(firstPagePeople[1].name.firstName).toBe('Alice');
|
||||
// expect(firstPagePeople[1].name.lastName).toBe('Smith');
|
||||
|
||||
// const secondPageResponse = await makeRestAPIRequest({
|
||||
// method: 'get',
|
||||
// path: `/people?order_by=name[AscNullsLast]&limit=2&starting_after=${firstPageCursor}`,
|
||||
// }).expect(200);
|
||||
|
||||
// const secondPagePeople = secondPageResponse.body.data.people;
|
||||
|
||||
// expect(secondPagePeople).toHaveLength(2);
|
||||
|
||||
// expect(secondPagePeople[0].name.firstName).toBe('Bob');
|
||||
// expect(secondPagePeople[0].name.lastName).toBe('Johnson');
|
||||
// expect(secondPagePeople[1].name.firstName).toBe('Bob');
|
||||
// expect(secondPagePeople[1].name.lastName).toBe('Williams');
|
||||
|
||||
// const firstPageIds = firstPagePeople.map((p: { id: string }) => p.id);
|
||||
// const secondPageIds = secondPagePeople.map((p: { id: string }) => p.id);
|
||||
// const intersection = firstPageIds.filter((id: string) =>
|
||||
// secondPageIds.includes(id),
|
||||
// );
|
||||
|
||||
// expect(intersection).toHaveLength(0);
|
||||
|
||||
// const thirdPageResponse = await makeRestAPIRequest({
|
||||
// method: 'get',
|
||||
// path: `/people?order_by=name[AscNullsLast]&limit=2&starting_after=${secondPageResponse.body.pageInfo.endCursor}`,
|
||||
// }).expect(200);
|
||||
|
||||
// const thirdPagePeople = thirdPageResponse.body.data.people;
|
||||
|
||||
// expect(thirdPagePeople).toHaveLength(1);
|
||||
// expect(thirdPagePeople[0].name.firstName).toBe('Charlie');
|
||||
// expect(thirdPagePeople[0].name.lastName).toBe('Davis');
|
||||
// expect(thirdPageResponse.body.pageInfo.hasNextPage).toBe(false);
|
||||
// });
|
||||
|
||||
// it('should support cursor-based pagination with fullName descending order', async () => {
|
||||
// const firstPageResponse = await makeRestAPIRequest({
|
||||
// method: 'get',
|
||||
// path: '/people?order_by=name[DescNullsLast]&limit=2',
|
||||
// }).expect(200);
|
||||
|
||||
// const firstPagePeople = firstPageResponse.body.data.people;
|
||||
|
||||
// expect(firstPagePeople).toHaveLength(2);
|
||||
|
||||
// expect(firstPagePeople[0].name.firstName).toBe('Charlie');
|
||||
// expect(firstPagePeople[0].name.lastName).toBe('Davis');
|
||||
// expect(firstPagePeople[1].name.firstName).toBe('Bob');
|
||||
// expect(firstPagePeople[1].name.lastName).toBe('Williams');
|
||||
|
||||
// const secondPageResponse = await makeRestAPIRequest({
|
||||
// method: 'get',
|
||||
// path: `/people?order_by=name[DescNullsLast]&limit=2&starting_after=${firstPageResponse.body.pageInfo.endCursor}`,
|
||||
// }).expect(200);
|
||||
|
||||
// const secondPagePeople = secondPageResponse.body.data.people;
|
||||
|
||||
// expect(secondPagePeople).toHaveLength(2);
|
||||
|
||||
// expect(secondPagePeople[0].name.firstName).toBe('Bob');
|
||||
// expect(secondPagePeople[0].name.lastName).toBe('Johnson');
|
||||
// expect(secondPagePeople[1].name.firstName).toBe('Alice');
|
||||
// expect(secondPagePeople[1].name.lastName).toBe('Smith');
|
||||
// });
|
||||
|
||||
// it('should support backward pagination with fullName composite field', async () => {
|
||||
// const allPeopleResponse = await makeRestAPIRequest({
|
||||
// method: 'get',
|
||||
// path: '/people?order_by=name.firstName[AscNullsLast],name.lastName[AscNullsLast]',
|
||||
// }).expect(200);
|
||||
|
||||
// const allPeople = allPeopleResponse.body.data.people;
|
||||
// const lastPersonCursor = allPeopleResponse.body.pageInfo.endCursor;
|
||||
|
||||
// const backwardPageResponse = await makeRestAPIRequest({
|
||||
// method: 'get',
|
||||
// path: `/people?order_by=name[AscNullsLast]&limit=2&ending_before=${lastPersonCursor}`,
|
||||
// }).expect(200);
|
||||
|
||||
// const backwardPagePeople = backwardPageResponse.body.data.people;
|
||||
|
||||
// expect(backwardPagePeople).toHaveLength(2);
|
||||
|
||||
// expect(backwardPagePeople[0].id).toBe(allPeople[allPeople.length - 3].id);
|
||||
// expect(backwardPagePeople[1].id).toBe(allPeople[allPeople.length - 2].id);
|
||||
// });
|
||||
|
||||
it('should support depth 0 parameter', async () => {
|
||||
const response = await makeRestAPIRequest({
|
||||
method: 'get',
|
||||
|
||||
Reference in New Issue
Block a user