From bcdc07273e67a5b269a40edfa760163205646e4c Mon Sep 17 00:00:00 2001 From: Marie <51697796+ijreilly@users.noreply.github.com> Date: Fri, 31 Oct 2025 15:13:49 +0100 Subject: [PATCH] Fix orderBy, groupBy, and orderByForRecords on foreignKey (#15480) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For the variables orderBy (for findMany and groupBy), groupBy (for groupBy) and orderByForRecords (for groupBy), we were wrongfully adding the foreign key field (eg: pointOfContact) by its joinColumnName (eg: pointOfContactId) as the variable key (eg. `orderBy: { pointOfContactId: "AscNullsFirst" } }`. That broke because then this key is used to identify the field in the parsers. This went unnoticed because this order / group option is not very interesting as it is limited to the id for now, but it s still better to have it work than crash! before (on findMany) flawn_order_by_pocId after (on findMany) image --- .../common-group-by-query-runner.service.ts | 20 +++++++++----- .../graphql-query-order.parser.ts | 27 ++++++++++++++----- .../utils/parse-group-by-args.util.ts | 3 ++- ...mat-column-name-for-relation-field.util.ts | 19 +++++++++++++ 4 files changed, 55 insertions(+), 14 deletions(-) create mode 100644 packages/twenty-server/src/engine/twenty-orm/utils/format-column-name-for-relation-field.util.ts diff --git a/packages/twenty-server/src/engine/api/common/common-query-runners/common-group-by-query-runner.service.ts b/packages/twenty-server/src/engine/api/common/common-query-runners/common-group-by-query-runner.service.ts index d84b46c79f4..19f43423e6d 100644 --- a/packages/twenty-server/src/engine/api/common/common-query-runners/common-group-by-query-runner.service.ts +++ b/packages/twenty-server/src/engine/api/common/common-query-runners/common-group-by-query-runner.service.ts @@ -38,6 +38,7 @@ import { parseGroupByArgs } from 'src/engine/api/graphql/graphql-query-runner/gr import { removeQuotes } from 'src/engine/api/graphql/graphql-query-runner/group-by/resolvers/utils/remove-quote.util'; import { GroupByWithRecordsService } from 'src/engine/api/graphql/graphql-query-runner/group-by/services/group-by-with-records.service'; import { ProcessAggregateHelper } from 'src/engine/api/graphql/graphql-query-runner/helpers/process-aggregate.helper'; +import { isFieldMetadataRelationOrMorphRelation } from 'src/engine/api/graphql/workspace-schema-builder/utils/is-field-metadata-relation-or-morph-relation.utils'; import { ObjectMetadataItemWithFieldMaps } from 'src/engine/metadata-modules/types/object-metadata-item-with-field-maps'; import { ObjectMetadataMaps } from 'src/engine/metadata-modules/types/object-metadata-maps'; import { ViewFilterGroupService } from 'src/engine/metadata-modules/view-filter-group/services/view-filter-group.service'; @@ -45,6 +46,7 @@ import { ViewFilterService } from 'src/engine/metadata-modules/view-filter/servi import { ViewEntity } from 'src/engine/metadata-modules/view/entities/view.entity'; import { ViewService } from 'src/engine/metadata-modules/view/services/view.service'; import { WorkspaceSelectQueryBuilder } from 'src/engine/twenty-orm/repository/workspace-select-query-builder'; +import { formatColumnNameForRelationField } from 'src/engine/twenty-orm/utils/format-column-name-for-relation-field.util'; import { formatColumnNamesFromCompositeFieldAndSubfields } from 'src/engine/twenty-orm/utils/format-column-names-from-composite-field-and-subfield.util'; @Injectable() @@ -104,12 +106,18 @@ export class CommonGroupByQueryRunnerService extends CommonBaseQueryRunnerServic ); const groupByDefinitions = groupByFields.map((groupByField) => { - const columnNameWithQuotes = `"${ - formatColumnNamesFromCompositeFieldAndSubfields( - groupByField.fieldMetadata.name, - groupByField.subFieldName ? [groupByField.subFieldName] : undefined, - )[0] - }"`; + const columnName = isFieldMetadataRelationOrMorphRelation( + groupByField.fieldMetadata, + ) + ? formatColumnNameForRelationField( + groupByField.fieldMetadata.name, + groupByField.fieldMetadata.settings, + ) + : formatColumnNamesFromCompositeFieldAndSubfields( + groupByField.fieldMetadata.name, + groupByField.subFieldName ? [groupByField.subFieldName] : undefined, + )[0]; + const columnNameWithQuotes = `"${columnName}"`; const alias = removeQuotes(columnNameWithQuotes) + (isGroupByDateField(groupByField) diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-order/graphql-query-order.parser.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-order/graphql-query-order.parser.ts index edd3b89ce13..2b9886e1844 100644 --- a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-order/graphql-query-order.parser.ts +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/graphql-query-parsers/graphql-query-order/graphql-query-order.parser.ts @@ -29,10 +29,12 @@ import { type AggregationField, getAvailableAggregationsFromObjectFields, } from 'src/engine/api/graphql/workspace-schema-builder/utils/get-available-aggregations-from-object-fields.util'; +import { isFieldMetadataRelationOrMorphRelation } from 'src/engine/api/graphql/workspace-schema-builder/utils/is-field-metadata-relation-or-morph-relation.utils'; import { UserInputError } from 'src/engine/core-modules/graphql/utils/graphql-errors.util'; import { type FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity'; import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util'; import { type ObjectMetadataItemWithFieldMaps } from 'src/engine/metadata-modules/types/object-metadata-item-with-field-maps'; +import { formatColumnNameForRelationField } from 'src/engine/twenty-orm/utils/format-column-name-for-relation-field.util'; import { formatColumnNamesFromCompositeFieldAndSubfields } from 'src/engine/twenty-orm/utils/format-column-names-from-composite-field-and-subfield.util'; export type OrderByCondition = { @@ -54,14 +56,16 @@ export class GraphqlQueryOrderFieldParser { ): Record { return orderBy.reduce( (acc, item) => { - Object.entries(item).forEach(([key, value]) => { - const fieldMetadataId = this.objectMetadataMapItem.fieldIdByName[key]; + Object.entries(item).forEach(([fieldName, orderByDirection]) => { + const fieldMetadataId = + this.objectMetadataMapItem.fieldIdByName[fieldName] || + this.objectMetadataMapItem.fieldIdByJoinColumnName[fieldName]; const fieldMetadata = this.objectMetadataMapItem.fieldsById[fieldMetadataId]; - if (!fieldMetadata || value === undefined) { + if (!fieldMetadata || orderByDirection === undefined) { throw new GraphqlQueryRunnerException( - `Field "${key}" does not exist or is not sortable`, + `Field "${fieldName}" does not exist or is not sortable`, GraphqlQueryRunnerExceptionCode.FIELD_NOT_FOUND, ); } @@ -69,7 +73,7 @@ export class GraphqlQueryOrderFieldParser { if (isCompositeFieldMetadataType(fieldMetadata.type)) { const compositeOrder = parseCompositeFieldForOrder( fieldMetadata, - value, + orderByDirection, objectNameSingular, isForwardPagination, ); @@ -79,9 +83,18 @@ export class GraphqlQueryOrderFieldParser { const orderByCasting = this.getOptionalOrderByCasting(fieldMetadata); - acc[`"${objectNameSingular}"."${key}"${orderByCasting}`] = + const columnName = isFieldMetadataRelationOrMorphRelation( + fieldMetadata, + ) + ? formatColumnNameForRelationField( + fieldMetadata.name, + fieldMetadata.settings, + ) + : fieldName; + + acc[`"${objectNameSingular}"."${columnName}"${orderByCasting}`] = convertOrderByToFindOptionsOrder( - value as OrderByDirection, + orderByDirection as OrderByDirection, isForwardPagination, ); } diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/group-by/resolvers/utils/parse-group-by-args.util.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/group-by/resolvers/utils/parse-group-by-args.util.ts index 7ebf07f566e..cdcbc6c2ec0 100644 --- a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/group-by/resolvers/utils/parse-group-by-args.util.ts +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/group-by/resolvers/utils/parse-group-by-args.util.ts @@ -60,7 +60,8 @@ export const parseGroupByArgs = ( } for (const fieldName of Object.keys(fieldNames)) { const fieldMetadataId = - objectMetadataItemWithFieldMaps.fieldIdByName[fieldName]; + objectMetadataItemWithFieldMaps.fieldIdByName[fieldName] || + objectMetadataItemWithFieldMaps.fieldIdByJoinColumnName[fieldName]; const fieldMetadata = objectMetadataItemWithFieldMaps.fieldsById[fieldMetadataId]; diff --git a/packages/twenty-server/src/engine/twenty-orm/utils/format-column-name-for-relation-field.util.ts b/packages/twenty-server/src/engine/twenty-orm/utils/format-column-name-for-relation-field.util.ts new file mode 100644 index 00000000000..dc650082e26 --- /dev/null +++ b/packages/twenty-server/src/engine/twenty-orm/utils/format-column-name-for-relation-field.util.ts @@ -0,0 +1,19 @@ +import { RelationType } from 'twenty-shared/types'; +import { isDefined } from 'twenty-shared/utils'; + +import { type FieldMetadataRelationSettings } from 'src/engine/metadata-modules/field-metadata/interfaces/field-metadata-settings.interface'; + +export const formatColumnNameForRelationField = ( + fieldName: string, + fieldMetadataSettings: FieldMetadataRelationSettings, +): string => { + if (fieldMetadataSettings.relationType === RelationType.MANY_TO_ONE) { + if (!isDefined(fieldMetadataSettings.joinColumnName)) { + throw new Error(`Join column name is not defined for field ${fieldName}`); + } + + return fieldMetadataSettings.joinColumnName; + } + + return fieldName; +};