From 1ad8c05fbce1cd93bbcb7d49fb76ab90e54142b8 Mon Sep 17 00:00:00 2001 From: Marie <51697796+ijreilly@users.noreply.github.com> Date: Thu, 30 Oct 2025 11:14:56 +0100 Subject: [PATCH] groupBy fix + typeMapper fix (#15433) groupBy fix: when a group's dimension value is NULL, we need to adapt the raw sql (stage: NULL -> stage IS NULL) typeMapper fix: a graphql type should be made non-nullable if was indicated so + does not have a default value. our check on not having a default value was limited to having a null defaultValue instead of having a null or undefined defaultValue. This is a breaking change, but all the queries that were providing a null value for these args were not functioning anyway, and luckily in the FE we declared all queries adding a `!` already. --- .../group-by/services/group-by-with-records.service.ts | 8 ++++++-- .../services/type-mapper.service.ts | 3 ++- .../group-by-with-records-resolver.integration-spec.ts | 8 ++++---- .../graphql/suites/upsert/upsert.integration-spec.ts | 2 +- .../graphql/utils/delete-many-operation-factory.util.ts | 2 +- .../graphql/utils/destroy-many-operation-factory.util.ts | 2 +- .../graphql/utils/update-many-operation-factory.util.ts | 4 ++-- .../graphql/utils/update-one-operation-factory.util.ts | 2 +- 8 files changed, 18 insertions(+), 13 deletions(-) diff --git a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/group-by/services/group-by-with-records.service.ts b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/group-by/services/group-by-with-records.service.ts index 917d7d04202..efad027c27b 100644 --- a/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/group-by/services/group-by-with-records.service.ts +++ b/packages/twenty-server/src/engine/api/graphql/graphql-query-runner/group-by/services/group-by-with-records.service.ts @@ -1,8 +1,8 @@ import { Inject, Injectable } from '@nestjs/common'; -import { isDefined } from 'class-validator'; import isEmpty from 'lodash.isempty'; import { ObjectRecord } from 'twenty-shared/types'; +import { isDefined } from 'twenty-shared/utils'; import { type ObjectLiteral } from 'typeorm'; import { ObjectRecordOrderBy } from 'src/engine/api/graphql/workspace-query-builder/interfaces/object-record.interface'; @@ -221,8 +221,12 @@ export class GroupByWithRecordsService { const conditions = groupByDefinitions .map((def, defIndex) => { const paramName = `groupValue_${groupIndex}_${defIndex}`; + const paramValue = group[def.alias]; - queryBuilder.setParameter(paramName, group[def.alias]); + if (!isDefined(paramValue)) { + return `${def.expression} IS NULL`; + } + queryBuilder.setParameter(paramName, paramValue); return `${def.expression} = :${paramName}`; }) diff --git a/packages/twenty-server/src/engine/api/graphql/workspace-schema-builder/services/type-mapper.service.ts b/packages/twenty-server/src/engine/api/graphql/workspace-schema-builder/services/type-mapper.service.ts index e999851507d..ce6ba980550 100644 --- a/packages/twenty-server/src/engine/api/graphql/workspace-schema-builder/services/type-mapper.service.ts +++ b/packages/twenty-server/src/engine/api/graphql/workspace-schema-builder/services/type-mapper.service.ts @@ -15,6 +15,7 @@ import { } from 'graphql'; import GraphQLJSON from 'graphql-type-json'; import { FieldMetadataType } from 'twenty-shared/types'; +import { isDefined } from 'twenty-shared/utils'; import { FieldMetadataDefaultValue } from 'src/engine/metadata-modules/field-metadata/interfaces/field-metadata-default-value.interface'; import { @@ -204,7 +205,7 @@ export class TypeMapperService { ); } - if (options.nullable === false && options.defaultValue === null) { + if (options.nullable === false && !isDefined(options.defaultValue)) { graphqlType = new GraphQLNonNull(graphqlType) as unknown as T; } diff --git a/packages/twenty-server/test/integration/graphql/suites/group-by-with-records-resolver.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/group-by-with-records-resolver.integration-spec.ts index dc87e4275bc..d0fb88ddb25 100644 --- a/packages/twenty-server/test/integration/graphql/suites/group-by-with-records-resolver.integration-spec.ts +++ b/packages/twenty-server/test/integration/graphql/suites/group-by-with-records-resolver.integration-spec.ts @@ -170,7 +170,7 @@ describe('basic group-by with records', () => { const response = await makeGraphqlAPIRequest({ query: gql` query OpportunitiesGroupBy( - $groupBy: [OpportunityGroupByInput!] + $groupBy: [OpportunityGroupByInput!]! $filter: OpportunityFilterInput ) { opportunitiesGroupBy(groupBy: $groupBy, filter: $filter) { @@ -293,7 +293,7 @@ describe('basic group-by with records', () => { const response = await makeGraphqlAPIRequest({ query: gql` query OpportunitiesGroupBy( - $groupBy: [OpportunityGroupByInput!] + $groupBy: [OpportunityGroupByInput!]! $filter: OpportunityFilterInput ) { opportunitiesGroupBy(groupBy: $groupBy, filter: $filter) { @@ -374,7 +374,7 @@ describe('basic group-by with records', () => { const response = await makeGraphqlAPIRequest({ query: gql` query CompaniesGroupBy( - $groupBy: [CompanyGroupByInput!] + $groupBy: [CompanyGroupByInput!]! $filter: CompanyFilterInput ) { companiesGroupBy(groupBy: $groupBy, filter: $filter) { @@ -464,7 +464,7 @@ describe('basic group-by with records', () => { return { query: gql` query OpportunitiesGroupBy( - $groupBy: [OpportunityGroupByInput!] + $groupBy: [OpportunityGroupByInput!]! $filter: OpportunityFilterInput $orderByForRecords: [OpportunityOrderByInput!] ) { diff --git a/packages/twenty-server/test/integration/graphql/suites/upsert/upsert.integration-spec.ts b/packages/twenty-server/test/integration/graphql/suites/upsert/upsert.integration-spec.ts index 7db4d86fd8e..fdad0a79042 100644 --- a/packages/twenty-server/test/integration/graphql/suites/upsert/upsert.integration-spec.ts +++ b/packages/twenty-server/test/integration/graphql/suites/upsert/upsert.integration-spec.ts @@ -8,7 +8,7 @@ import { FieldMetadataType } from 'twenty-shared/types'; const createRecordsQuery = gql` mutation CreateRecords( - $data: [TestRecordObjectCreateInput!] + $data: [TestRecordObjectCreateInput!]! $upsert: Boolean ) { createTestRecordObjects(data: $data, upsert: $upsert) { diff --git a/packages/twenty-server/test/integration/graphql/utils/delete-many-operation-factory.util.ts b/packages/twenty-server/test/integration/graphql/utils/delete-many-operation-factory.util.ts index 726b154f32e..c62adaa182e 100644 --- a/packages/twenty-server/test/integration/graphql/utils/delete-many-operation-factory.util.ts +++ b/packages/twenty-server/test/integration/graphql/utils/delete-many-operation-factory.util.ts @@ -16,7 +16,7 @@ export const deleteManyOperationFactory = ({ }: DeleteManyOperationFactoryParams) => ({ query: gql` mutation Delete${capitalize(objectMetadataPluralName)}( - $filter: ${capitalize(objectMetadataSingularName)}FilterInput + $filter: ${capitalize(objectMetadataSingularName)}FilterInput! ) { delete${capitalize(objectMetadataPluralName)}(filter: $filter) { ${gqlFields} diff --git a/packages/twenty-server/test/integration/graphql/utils/destroy-many-operation-factory.util.ts b/packages/twenty-server/test/integration/graphql/utils/destroy-many-operation-factory.util.ts index 4cf05e1993e..cd3ba4b4815 100644 --- a/packages/twenty-server/test/integration/graphql/utils/destroy-many-operation-factory.util.ts +++ b/packages/twenty-server/test/integration/graphql/utils/destroy-many-operation-factory.util.ts @@ -16,7 +16,7 @@ export const destroyManyOperationFactory = ({ }: DestroyManyOperationFactoryParams) => ({ query: gql` mutation Destroy${capitalize(objectMetadataPluralName)}( - $filter: ${capitalize(objectMetadataSingularName)}FilterInput + $filter: ${capitalize(objectMetadataSingularName)}FilterInput! ) { destroy${capitalize(objectMetadataPluralName)}(filter: $filter) { ${gqlFields} diff --git a/packages/twenty-server/test/integration/graphql/utils/update-many-operation-factory.util.ts b/packages/twenty-server/test/integration/graphql/utils/update-many-operation-factory.util.ts index 49133c4cbd7..e3467a1a1df 100644 --- a/packages/twenty-server/test/integration/graphql/utils/update-many-operation-factory.util.ts +++ b/packages/twenty-server/test/integration/graphql/utils/update-many-operation-factory.util.ts @@ -18,8 +18,8 @@ export const updateManyOperationFactory = ({ }: UpdateManyOperationFactoryParams) => ({ query: gql` mutation Update${capitalize(objectMetadataPluralName)}( - $data: ${capitalize(objectMetadataSingularName)}UpdateInput - $filter: ${capitalize(objectMetadataSingularName)}FilterInput + $data: ${capitalize(objectMetadataSingularName)}UpdateInput! + $filter: ${capitalize(objectMetadataSingularName)}FilterInput! ) { update${capitalize(objectMetadataPluralName)}(data: $data, filter: $filter) { ${gqlFields} diff --git a/packages/twenty-server/test/integration/graphql/utils/update-one-operation-factory.util.ts b/packages/twenty-server/test/integration/graphql/utils/update-one-operation-factory.util.ts index dffaec05b0a..84e637b3cc0 100644 --- a/packages/twenty-server/test/integration/graphql/utils/update-one-operation-factory.util.ts +++ b/packages/twenty-server/test/integration/graphql/utils/update-one-operation-factory.util.ts @@ -15,7 +15,7 @@ export const updateOneOperationFactory = ({ recordId, }: UpdateOneOperationFactoryParams) => ({ query: gql` - mutation Update${capitalize(objectMetadataSingularName)}($${objectMetadataSingularName}Id: UUID, $data: ${capitalize(objectMetadataSingularName)}UpdateInput) { + mutation Update${capitalize(objectMetadataSingularName)}($${objectMetadataSingularName}Id: UUID!, $data: ${capitalize(objectMetadataSingularName)}UpdateInput!) { update${capitalize(objectMetadataSingularName)}(id: $${objectMetadataSingularName}Id, data: $data) { ${gqlFields} }