mukeshs/twenty_crm
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Fix sort with pagination and composite fields (#9150)

Browse Source 
Fixes https://github.com/twentyhq/twenty/issues/8863

## Description
This PR fixes an issue with cursor-based pagination when dealing with
composite fields (like `fullName`). Previously, the pagination direction
was incorrectly determined for composite fields because the code wasn't
properly handling nested object structures in the `orderBy` parameter.
Refactored the code accordingly.
This commit is contained in:
Weiko
2024-12-19 16:41:04 +01:00
committed by GitHub
parent ed56a68b7c
commit 3f58a41d2f
4 changed files with 228 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-order/graphql-query-order.parser.ts
View File

@ -48,7 +48,10 @@ export class GraphqlQueryOrderFieldParser {
Object.assign(acc, compositeOrder);
} else {
acc[`"${objectNameSingular}"."${key}"`] =
this.convertOrderByToFindOptionsOrder(value, isForwardPagination);
this.convertOrderByToFindOptionsOrder(
value as OrderByDirection,
isForwardPagination,
);
}
});

141
packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/__tests__/compute-cursor-arg-filter.spec.ts Normal file
View File

@ -0,0 +1,141 @@
import { OrderByDirection } from 'src/engine/api/graphql/workspace-query-builder/interfaces/object-record.interface';
import { GraphqlQueryRunnerException } from 'src/engine/api/graphql/graphql-query-runner/errors/graphql-query-runner.exception';
import { computeCursorArgFilter } from 'src/engine/api/graphql/graphql-query-runner/utils/compute-cursor-arg-filter';
import { FieldMetadataType } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity';
describe('computeCursorArgFilter', () => {
const mockFieldMetadataMap = {
name: {
type: FieldMetadataType.TEXT,
id: 'name-id',
name: 'name',
label: 'Name',
objectMetadataId: 'object-id',
},
age: {
type: FieldMetadataType.NUMBER,
id: 'age-id',
name: 'age',
label: 'Age',
objectMetadataId: 'object-id',
},
fullName: {
type: FieldMetadataType.FULL_NAME,
id: 'fullname-id',
name: 'fullName',
label: 'Full Name',
objectMetadataId: 'object-id',
},
};
describe('basic cursor filtering', () => {
it('should return empty array when cursor is empty', () => {
const result = computeCursorArgFilter({}, [], mockFieldMetadataMap, true);
expect(result).toEqual([]);
});
it('should compute forward pagination filter for single field', () => {
const cursor = { name: 'John' };
const orderBy = [{ name: OrderByDirection.AscNullsLast }];
const result = computeCursorArgFilter(
cursor,
orderBy,
mockFieldMetadataMap,
true,
);
expect(result).toEqual([{ name: { gt: 'John' } }]);
});
it('should compute backward pagination filter for single field', () => {
const cursor = { name: 'John' };
const orderBy = [{ name: OrderByDirection.AscNullsLast }];
const result = computeCursorArgFilter(
cursor,
orderBy,
mockFieldMetadataMap,
false,
);
expect(result).toEqual([{ name: { lt: 'John' } }]);
});
});
describe('multiple fields cursor filtering', () => {
it('should handle multiple cursor fields with forward pagination', () => {
const cursor = { name: 'John', age: 30 };
const orderBy = [
{ name: OrderByDirection.AscNullsLast },
{ age: OrderByDirection.DescNullsLast },
];
const result = computeCursorArgFilter(
cursor,
orderBy,
mockFieldMetadataMap,
true,
);
expect(result).toEqual([
{ name: { gt: 'John' } },
{ name: { eq: 'John' }, age: { lt: 30 } },
]);
});
});
describe('composite field handling', () => {
it('should handle fullName composite field', () => {
const cursor = {
fullName: { firstName: 'John', lastName: 'Doe' },
};
const orderBy = [
{
fullName: {
firstName: OrderByDirection.AscNullsLast,
lastName: OrderByDirection.AscNullsLast,
},
},
];
const result = computeCursorArgFilter(
cursor,
orderBy,
mockFieldMetadataMap,
true,
);
expect(result).toEqual([
{
fullName: {
firstName: { gt: 'John' },
lastName: { gt: 'Doe' },
},
},
]);
});
});
describe('error handling', () => {
it('should throw error for invalid field metadata', () => {
const cursor = { invalidField: 'value' };
const orderBy = [{ invalidField: OrderByDirection.AscNullsLast }];
expect(() =>
computeCursorArgFilter(cursor, orderBy, mockFieldMetadataMap, true),
).toThrow(GraphqlQueryRunnerException);
});
it('should throw error for missing orderBy entry', () => {
const cursor = { name: 'John' };
const orderBy = [{ age: OrderByDirection.AscNullsLast }];
expect(() =>
computeCursorArgFilter(cursor, orderBy, mockFieldMetadataMap, true),
).toThrow(GraphqlQueryRunnerException);
});
});
});

111
packages/twenty-server/src/engine/api/graphql/graphql-query-runner/utils/compute-cursor-arg-filter.ts
View File

@ -13,6 +13,42 @@ import { FieldMetadataType } from 'src/engine/metadata-modules/field-metadata/fi
import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util';
import { FieldMetadataMap } from 'src/engine/metadata-modules/types/field-metadata-map';
const computeOperator = (
isAscending: boolean,
isForwardPagination: boolean,
defaultOperator?: string,
): string => {
if (defaultOperator) return defaultOperator;
return isAscending
? isForwardPagination
? 'gt'
: 'lt'
: isForwardPagination
? 'lt'
: 'gt';
};
const validateAndGetOrderBy = (
key: string,
orderBy: ObjectRecordOrderBy,
): Record<string, any> => {
const keyOrderBy = orderBy.find((order) => key in order);
if (!keyOrderBy) {
throw new GraphqlQueryRunnerException(
'Invalid cursor',
GraphqlQueryRunnerExceptionCode.INVALID_CURSOR,
);
}
return keyOrderBy;
};
const isAscendingOrder = (direction: OrderByDirection): boolean =>
direction === OrderByDirection.AscNullsFirst ||
direction === OrderByDirection.AscNullsLast;
export const computeCursorArgFilter = (
cursor: Record<string, any>,
orderBy: ObjectRecordOrderBy,
@ -40,35 +76,22 @@ export const computeCursorArgFilter = (
cursorKeys[subConditionIndex],
cursorValues[subConditionIndex],
fieldMetadataMapByName,
orderBy,
isForwardPagination,
'eq',
),
};
}
const keyOrderBy = orderBy.find((order) => key in order);
if (!keyOrderBy) {
throw new GraphqlQueryRunnerException(
'Invalid cursor',
GraphqlQueryRunnerExceptionCode.INVALID_CURSOR,
);
}
const isAscending =
keyOrderBy[key] === OrderByDirection.AscNullsFirst ||
keyOrderBy[key] === OrderByDirection.AscNullsLast;
const operator = isAscending
? isForwardPagination
? 'gt'
: 'lt'
: isForwardPagination
? 'lt'
: 'gt';
return {
...whereCondition,
...buildWhereCondition(key, value, fieldMetadataMapByName, operator),
...buildWhereCondition(
key,
value,
fieldMetadataMapByName,
orderBy,
isForwardPagination,
),
} as ObjectRecordFilter;
});
};
@ -77,7 +100,9 @@ const buildWhereCondition = (
key: string,
value: any,
fieldMetadataMapByName: FieldMetadataMap,
operator: string,
orderBy: ObjectRecordOrderBy,
isForwardPagination: boolean,
operator?: string,
): Record<string, any> => {
const fieldMetadata = fieldMetadataMapByName[key];
@ -93,18 +118,30 @@ const buildWhereCondition = (
key,
value,
fieldMetadata.type,
orderBy,
isForwardPagination,
operator,
);
}
return { [key]: { [operator]: value } };
const keyOrderBy = validateAndGetOrderBy(key, orderBy);
const isAscending = isAscendingOrder(keyOrderBy[key]);
const computedOperator = computeOperator(
isAscending,
isForwardPagination,
operator,
);
return { [key]: { [computedOperator]: value } };
};
const buildCompositeWhereCondition = (
key: string,
value: any,
fieldType: FieldMetadataType,
operator: string,
orderBy: ObjectRecordOrderBy,
isForwardPagination: boolean,
operator?: string,
): Record<string, any> => {
const compositeType = compositeTypeDefinitions.get(fieldType);
@ -115,18 +152,30 @@ const buildCompositeWhereCondition = (
);
}
const keyOrderBy = validateAndGetOrderBy(key, orderBy);
const result: Record<string, any> = {};
compositeType.properties.forEach((property) => {
if (
property.type !== FieldMetadataType.RAW_JSON &&
value[property.name] !== undefined
property.type === FieldMetadataType.RAW_JSON ||
value[property.name] === undefined
) {
result[key] = {
...result[key],
[property.name]: { [operator]: value[property.name] },
};
return;
}
const isAscending = isAscendingOrder(keyOrderBy[key][property.name]);
const computedOperator = computeOperator(
isAscending,
isForwardPagination,
operator,
);
result[key] = {
...result[key],
[property.name]: {
[computedOperator]: value[property.name],
},
};
});
return result;

4
packages/twenty-server/src/engine/api/graphql/workspace-query-builder/interfaces/object-record.interface.ts
View File

@ -18,7 +18,9 @@ export enum OrderByDirection {
}
export type ObjectRecordOrderBy = Array<{
[Property in keyof ObjectRecord]?: OrderByDirection;
[Property in keyof ObjectRecord]?:
| OrderByDirection
| Record<string, OrderByDirection>;
}>;
export interface ObjectRecordDuplicateCriteria {