From ff6abacc86910a77208b70010a28726a8e180edf Mon Sep 17 00:00:00 2001 From: Marie <51697796+ijreilly@users.noreply.github.com> Date: Wed, 3 Apr 2024 17:17:23 +0200 Subject: [PATCH] [feat] Enable deletion of custom fields in workspace (#4780) **Context** cf. feature request [#4597](https://github.com/twentyhq/twenty/issues/4597) Enables deletion of custom fields that aren't active nor of type relation Also 1. renamed a misnamed file 2. deleted redundant hook BeforeDeleteOneField as it seemed best to move the logic to the resolver instead **How was it tested?** Did not write unit tests as code is to be migrated (discussed with @Weiko). Locally tested. --------- Co-authored-by: Marie Stoppa --- .../field-metadata/dtos/delete-field.input.ts | 9 +++ .../field-metadata/dtos/field-metadata.dto.ts | 3 - .../field-metadata/field-metadata.module.ts | 2 +- .../field-metadata/field-metadata.resolver.ts | 45 ++++++++++- .../field-metadata/field-metadata.service.ts | 77 +++++++++++++++++++ .../hooks/before-delete-one-field.hook.ts | 56 -------------- .../dtos/relation-metadata.dto.ts | 2 +- ....ts => before-delete-one-relation.hook.ts} | 0 8 files changed, 132 insertions(+), 62 deletions(-) create mode 100644 packages/twenty-server/src/engine/metadata-modules/field-metadata/dtos/delete-field.input.ts delete mode 100644 packages/twenty-server/src/engine/metadata-modules/field-metadata/hooks/before-delete-one-field.hook.ts rename packages/twenty-server/src/engine/metadata-modules/relation-metadata/hooks/{before-delete-one-field.hook.ts => before-delete-one-relation.hook.ts} (100%) diff --git a/packages/twenty-server/src/engine/metadata-modules/field-metadata/dtos/delete-field.input.ts b/packages/twenty-server/src/engine/metadata-modules/field-metadata/dtos/delete-field.input.ts new file mode 100644 index 000000000..8a217d769 --- /dev/null +++ b/packages/twenty-server/src/engine/metadata-modules/field-metadata/dtos/delete-field.input.ts @@ -0,0 +1,9 @@ +import { InputType, ID } from '@nestjs/graphql'; + +import { IDField } from '@ptc-org/nestjs-query-graphql'; + +@InputType() +export class DeleteOneFieldInput { + @IDField(() => ID, { description: 'The id of the field to delete.' }) + id!: string; +} diff --git a/packages/twenty-server/src/engine/metadata-modules/field-metadata/dtos/field-metadata.dto.ts b/packages/twenty-server/src/engine/metadata-modules/field-metadata/dtos/field-metadata.dto.ts index b901fca7a..97082a5e5 100644 --- a/packages/twenty-server/src/engine/metadata-modules/field-metadata/dtos/field-metadata.dto.ts +++ b/packages/twenty-server/src/engine/metadata-modules/field-metadata/dtos/field-metadata.dto.ts @@ -9,7 +9,6 @@ import { import { GraphQLJSON } from 'graphql-type-json'; import { Authorize, - BeforeDeleteOne, FilterableField, IDField, QueryOptions, @@ -31,7 +30,6 @@ import { FieldMetadataDefaultValue } from 'src/engine/metadata-modules/field-met import { RelationMetadataDTO } from 'src/engine/metadata-modules/relation-metadata/dtos/relation-metadata.dto'; import { FieldMetadataType } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity'; -import { BeforeDeleteOneField } from 'src/engine/metadata-modules/field-metadata/hooks/before-delete-one-field.hook'; import { IsFieldMetadataDefaultValue } from 'src/engine/metadata-modules/field-metadata/validators/is-field-metadata-default-value.validator'; import { IsFieldMetadataOptions } from 'src/engine/metadata-modules/field-metadata/validators/is-field-metadata-options.validator'; import { IsValidMetadataName } from 'src/engine/decorators/metadata/is-valid-metadata-name.decorator'; @@ -52,7 +50,6 @@ registerEnumType(FieldMetadataType, { disableSort: true, maxResultsSize: 1000, }) -@BeforeDeleteOne(BeforeDeleteOneField) @Relation('toRelationMetadata', () => RelationMetadataDTO, { nullable: true, }) diff --git a/packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.module.ts b/packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.module.ts index 0104a59b6..9dc578cf1 100644 --- a/packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.module.ts +++ b/packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.module.ts @@ -61,7 +61,7 @@ import { UpdateFieldInput } from './dtos/update-field.input'; one: { disabled: true }, many: { disabled: true }, }, - delete: { many: { disabled: true } }, + delete: { disabled: true }, guards: [JwtAuthGuard], }, ], diff --git a/packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.resolver.ts b/packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.resolver.ts index 99d145a3d..77deab4ef 100644 --- a/packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.resolver.ts +++ b/packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.resolver.ts @@ -1,4 +1,8 @@ -import { UseGuards } from '@nestjs/common'; +import { + BadRequestException, + UnauthorizedException, + UseGuards, +} from '@nestjs/common'; import { Args, Mutation, @@ -11,9 +15,11 @@ import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity'; import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorator'; import { JwtAuthGuard } from 'src/engine/guards/jwt.auth.guard'; import { CreateOneFieldMetadataInput } from 'src/engine/metadata-modules/field-metadata/dtos/create-field.input'; +import { DeleteOneFieldInput } from 'src/engine/metadata-modules/field-metadata/dtos/delete-field.input'; import { FieldMetadataDTO } from 'src/engine/metadata-modules/field-metadata/dtos/field-metadata.dto'; import { RelationDefinitionDTO } from 'src/engine/metadata-modules/field-metadata/dtos/relation-definition.dto'; import { UpdateOneFieldMetadataInput } from 'src/engine/metadata-modules/field-metadata/dtos/update-field.input'; +import { FieldMetadataType } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity'; import { FieldMetadataService } from 'src/engine/metadata-modules/field-metadata/field-metadata.service'; @UseGuards(JwtAuthGuard) @@ -43,6 +49,43 @@ export class FieldMetadataResolver { }); } + @Mutation(() => FieldMetadataDTO) + async deleteOneField( + @Args('input') input: DeleteOneFieldInput, + @AuthWorkspace() { id: workspaceId }: Workspace, + ) { + if (!workspaceId) { + throw new UnauthorizedException(); + } + + const fieldMetadata = + await this.fieldMetadataService.findOneWithinWorkspace(workspaceId, { + where: { + id: input.id.toString(), + }, + }); + + if (!fieldMetadata) { + throw new BadRequestException('Field does not exist'); + } + + if (!fieldMetadata.isCustom) { + throw new BadRequestException("Standard Fields can't be deleted"); + } + + if (fieldMetadata.isActive) { + throw new BadRequestException("Active fields can't be deleted"); + } + + if (fieldMetadata.type === FieldMetadataType.RELATION) { + throw new BadRequestException( + "Relation fields can't be deleted, you need to delete the RelationMetadata instead", + ); + } + + return this.fieldMetadataService.deleteOneField(input, workspaceId); + } + @ResolveField(() => RelationDefinitionDTO, { nullable: true }) async relationDefinition( @Parent() fieldMetadata: FieldMetadataDTO, diff --git a/packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.service.ts b/packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.service.ts index b0bbb0f1a..f090e1d64 100644 --- a/packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.service.ts +++ b/packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.service.ts @@ -16,6 +16,7 @@ import { ObjectMetadataService } from 'src/engine/metadata-modules/object-metada import { CreateFieldInput } from 'src/engine/metadata-modules/field-metadata/dtos/create-field.input'; import { WorkspaceMigrationColumnActionType, + WorkspaceMigrationColumnDrop, WorkspaceMigrationTableAction, } from 'src/engine/metadata-modules/workspace-migration/workspace-migration.entity'; import { generateTargetColumnMap } from 'src/engine/metadata-modules/field-metadata/utils/generate-target-column-map.util'; @@ -35,6 +36,8 @@ import { RelationMetadataEntity, RelationMetadataType, } from 'src/engine/metadata-modules/relation-metadata/relation-metadata.entity'; +import { DeleteOneFieldInput } from 'src/engine/metadata-modules/field-metadata/dtos/delete-field.input'; +import { computeCustomName } from 'src/engine/utils/compute-custom-name.util'; import { FieldMetadataEntity, @@ -358,6 +361,80 @@ export class FieldMetadataService extends TypeOrmQueryService { + const queryRunner = this.metadataDataSource.createQueryRunner(); + + await queryRunner.connect(); + await queryRunner.startTransaction(); // transaction not safe as a different queryRunner is used within workspaceMigrationRunnerService + + try { + const fieldMetadataRepository = + queryRunner.manager.getRepository>( + FieldMetadataEntity, + ); + + const fieldMetadata = await fieldMetadataRepository.findOne({ + where: { + id: input.id, + workspaceId: workspaceId, + }, + }); + + if (!fieldMetadata) { + throw new NotFoundException('Field does not exist'); + } + + const objectMetadata = + await this.objectMetadataService.findOneWithinWorkspace(workspaceId, { + where: { + id: fieldMetadata?.objectMetadataId, + }, + }); + + if (!objectMetadata) { + throw new NotFoundException('Object does not exist'); + } + + await fieldMetadataRepository.delete(fieldMetadata.id); + + await this.workspaceMigrationService.createCustomMigration( + generateMigrationName(`delete-${fieldMetadata.name}`), + workspaceId, + [ + { + name: computeObjectTargetTable(objectMetadata), + action: 'alter', + columns: [ + { + action: WorkspaceMigrationColumnActionType.DROP, + columnName: computeCustomName( + fieldMetadata.name, + fieldMetadata.isCustom, + ), + } satisfies WorkspaceMigrationColumnDrop, + ], + } satisfies WorkspaceMigrationTableAction, + ], + ); + + await this.workspaceMigrationRunnerService.executeMigrationFromPendingMigrations( + workspaceId, + ); + + await queryRunner.commitTransaction(); + + return fieldMetadata; + } catch (error) { + await queryRunner.rollbackTransaction(); + throw error; + } finally { + await queryRunner.release(); + } + } + public async findOneOrFail( id: string, options?: FindOneOptions, diff --git a/packages/twenty-server/src/engine/metadata-modules/field-metadata/hooks/before-delete-one-field.hook.ts b/packages/twenty-server/src/engine/metadata-modules/field-metadata/hooks/before-delete-one-field.hook.ts deleted file mode 100644 index a9f65528b..000000000 --- a/packages/twenty-server/src/engine/metadata-modules/field-metadata/hooks/before-delete-one-field.hook.ts +++ /dev/null @@ -1,56 +0,0 @@ -import { - BadRequestException, - Injectable, - UnauthorizedException, -} from '@nestjs/common'; - -import { - BeforeDeleteOneHook, - DeleteOneInputType, -} from '@ptc-org/nestjs-query-graphql'; - -import { FieldMetadataType } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity'; -import { FieldMetadataService } from 'src/engine/metadata-modules/field-metadata/field-metadata.service'; - -@Injectable() -export class BeforeDeleteOneField implements BeforeDeleteOneHook { - constructor(readonly fieldMetadataService: FieldMetadataService) {} - - async run( - instance: DeleteOneInputType, - context: any, - ): Promise { - const workspaceId = context?.req?.user?.workspace?.id; - - if (!workspaceId) { - throw new UnauthorizedException(); - } - - const fieldMetadata = - await this.fieldMetadataService.findOneWithinWorkspace(workspaceId, { - where: { - id: instance.id.toString(), - }, - }); - - if (!fieldMetadata) { - throw new BadRequestException('Field does not exist'); - } - - if (!fieldMetadata.isCustom) { - throw new BadRequestException("Standard Fields can't be deleted"); - } - - if (fieldMetadata.isActive) { - throw new BadRequestException("Active fields can't be deleted"); - } - - if (fieldMetadata.type === FieldMetadataType.RELATION) { - throw new BadRequestException( - "Relation fields can't be deleted, you need to delete the RelationMetadata instead", - ); - } - - return instance; - } -} diff --git a/packages/twenty-server/src/engine/metadata-modules/relation-metadata/dtos/relation-metadata.dto.ts b/packages/twenty-server/src/engine/metadata-modules/relation-metadata/dtos/relation-metadata.dto.ts index bdb669a22..9e58d9486 100644 --- a/packages/twenty-server/src/engine/metadata-modules/relation-metadata/dtos/relation-metadata.dto.ts +++ b/packages/twenty-server/src/engine/metadata-modules/relation-metadata/dtos/relation-metadata.dto.ts @@ -17,7 +17,7 @@ import { import { ObjectMetadataDTO } from 'src/engine/metadata-modules/object-metadata/dtos/object-metadata.dto'; import { RelationMetadataType } from 'src/engine/metadata-modules/relation-metadata/relation-metadata.entity'; -import { BeforeDeleteOneRelation } from 'src/engine/metadata-modules/relation-metadata/hooks/before-delete-one-field.hook'; +import { BeforeDeleteOneRelation } from 'src/engine/metadata-modules/relation-metadata/hooks/before-delete-one-relation.hook'; registerEnumType(RelationMetadataType, { name: 'RelationMetadataType', diff --git a/packages/twenty-server/src/engine/metadata-modules/relation-metadata/hooks/before-delete-one-field.hook.ts b/packages/twenty-server/src/engine/metadata-modules/relation-metadata/hooks/before-delete-one-relation.hook.ts similarity index 100% rename from packages/twenty-server/src/engine/metadata-modules/relation-metadata/hooks/before-delete-one-field.hook.ts rename to packages/twenty-server/src/engine/metadata-modules/relation-metadata/hooks/before-delete-one-relation.hook.ts