From b65d82c2740b4531b1b1efc0c1005208a28959fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Malfait?= Date: Wed, 3 Apr 2024 18:30:12 +0200 Subject: [PATCH] Force explicit deletion behavior for relations (#4775) We've seen a few cascading errors (e.g. comment.activityId would be non nullable but cascade behavior is set to "set null"). I think it's safer if we have to explicitly chose the deletion behavior it every time. Especially since Postgres default to "No action" while we defaulted to "Set Null", which is confusing. In the future we will most likely introduce a second param `onSoftDelete` in the decorator --- .../utils/convert-on-delete-action-to-on-delete.util.ts | 4 ---- .../decorators/relation-metadata.decorator.ts | 2 -- .../interfaces/reflect-relation-metadata.interface.ts | 2 +- .../standard-objects/activity.object-metadata.ts | 9 ++++++++- .../company/standard-objects/company.object-metadata.ts | 2 ++ .../standard-objects/opportunity.object-metadata.ts | 2 ++ .../person/standard-objects/person.object-metadata.ts | 3 +++ .../view/standard-objects/view.object-metadata.ts | 8 +++++++- .../standard-objects/workspace-member.object-metadata.ts | 8 ++++++++ 9 files changed, 31 insertions(+), 9 deletions(-) diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-migration-runner/utils/convert-on-delete-action-to-on-delete.util.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-migration-runner/utils/convert-on-delete-action-to-on-delete.util.ts index 5bdb4e08f2..73e36ba678 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-migration-runner/utils/convert-on-delete-action-to-on-delete.util.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-migration-runner/utils/convert-on-delete-action-to-on-delete.util.ts @@ -3,10 +3,6 @@ import { RelationOnDeleteAction } from 'src/engine/metadata-modules/relation-met export const convertOnDeleteActionToOnDelete = ( onDeleteAction: RelationOnDeleteAction | undefined, ): 'CASCADE' | 'SET NULL' | 'RESTRICT' | 'NO ACTION' | undefined => { - if (!onDeleteAction) { - return 'SET NULL'; - } - switch (onDeleteAction) { case 'CASCADE': return 'CASCADE'; diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/decorators/relation-metadata.decorator.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/decorators/relation-metadata.decorator.ts index 3307f6093b..2981b4f9c9 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/decorators/relation-metadata.decorator.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/decorators/relation-metadata.decorator.ts @@ -6,7 +6,6 @@ import { } from 'src/engine/workspace-manager/workspace-sync-metadata/interfaces/reflect-relation-metadata.interface'; import { TypedReflect } from 'src/utils/typed-reflect'; -import { RelationOnDeleteAction } from 'src/engine/metadata-modules/relation-metadata/relation-metadata.entity'; export function RelationMetadata( params: RelationMetadataDecoratorParams, @@ -27,7 +26,6 @@ export function RelationMetadata( target, fieldKey, ...params, - onDelete: params.onDelete ?? RelationOnDeleteAction.SET_NULL, gate, } satisfies ReflectRelationMetadata, ], diff --git a/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/interfaces/reflect-relation-metadata.interface.ts b/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/interfaces/reflect-relation-metadata.interface.ts index 32282798f4..13f8c1a0d7 100644 --- a/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/interfaces/reflect-relation-metadata.interface.ts +++ b/packages/twenty-server/src/engine/workspace-manager/workspace-sync-metadata/interfaces/reflect-relation-metadata.interface.ts @@ -11,7 +11,7 @@ export interface RelationMetadataDecoratorParams { type: RelationMetadataType; inverseSideTarget: () => ObjectType; inverseSideFieldKey?: keyof T; - onDelete?: RelationOnDeleteAction; + onDelete: RelationOnDeleteAction; } export interface ReflectRelationMetadata diff --git a/packages/twenty-server/src/modules/activity/standard-objects/activity.object-metadata.ts b/packages/twenty-server/src/modules/activity/standard-objects/activity.object-metadata.ts index 452ff189ad..8a49be75f4 100644 --- a/packages/twenty-server/src/modules/activity/standard-objects/activity.object-metadata.ts +++ b/packages/twenty-server/src/modules/activity/standard-objects/activity.object-metadata.ts @@ -1,5 +1,8 @@ import { FieldMetadataType } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity'; -import { RelationMetadataType } from 'src/engine/metadata-modules/relation-metadata/relation-metadata.entity'; +import { + RelationMetadataType, + RelationOnDeleteAction, +} from 'src/engine/metadata-modules/relation-metadata/relation-metadata.entity'; import { activityStandardFieldIds } from 'src/engine/workspace-manager/workspace-sync-metadata/constants/standard-field-ids'; import { standardObjectIds } from 'src/engine/workspace-manager/workspace-sync-metadata/constants/standard-object-ids'; import { FieldMetadata } from 'src/engine/workspace-manager/workspace-sync-metadata/decorators/field-metadata.decorator'; @@ -91,6 +94,7 @@ export class ActivityObjectMetadata extends BaseObjectMetadata { @RelationMetadata({ type: RelationMetadataType.ONE_TO_MANY, inverseSideTarget: () => ActivityTargetObjectMetadata, + onDelete: RelationOnDeleteAction.SET_NULL, }) @IsNullable() activityTargets: ActivityTargetObjectMetadata[]; @@ -105,6 +109,7 @@ export class ActivityObjectMetadata extends BaseObjectMetadata { @RelationMetadata({ type: RelationMetadataType.ONE_TO_MANY, inverseSideTarget: () => AttachmentObjectMetadata, + onDelete: RelationOnDeleteAction.SET_NULL, }) @IsNullable() attachments: AttachmentObjectMetadata[]; @@ -119,6 +124,7 @@ export class ActivityObjectMetadata extends BaseObjectMetadata { @RelationMetadata({ type: RelationMetadataType.ONE_TO_MANY, inverseSideTarget: () => CommentObjectMetadata, + onDelete: RelationOnDeleteAction.CASCADE, }) @IsNullable() comments: CommentObjectMetadata[]; @@ -131,6 +137,7 @@ export class ActivityObjectMetadata extends BaseObjectMetadata { icon: 'IconUserCircle', joinColumn: 'authorId', }) + @IsNullable() author: WorkspaceMemberObjectMetadata; @FieldMetadata({ diff --git a/packages/twenty-server/src/modules/company/standard-objects/company.object-metadata.ts b/packages/twenty-server/src/modules/company/standard-objects/company.object-metadata.ts index af5db2ed7c..78ebe912ef 100644 --- a/packages/twenty-server/src/modules/company/standard-objects/company.object-metadata.ts +++ b/packages/twenty-server/src/modules/company/standard-objects/company.object-metadata.ts @@ -132,6 +132,7 @@ export class CompanyObjectMetadata extends BaseObjectMetadata { @RelationMetadata({ type: RelationMetadataType.ONE_TO_MANY, inverseSideTarget: () => PersonObjectMetadata, + onDelete: RelationOnDeleteAction.SET_NULL, }) @IsNullable() people: PersonObjectMetadata[]; @@ -173,6 +174,7 @@ export class CompanyObjectMetadata extends BaseObjectMetadata { @RelationMetadata({ type: RelationMetadataType.ONE_TO_MANY, inverseSideTarget: () => OpportunityObjectMetadata, + onDelete: RelationOnDeleteAction.SET_NULL, }) @IsNullable() opportunities: OpportunityObjectMetadata[]; diff --git a/packages/twenty-server/src/modules/opportunity/standard-objects/opportunity.object-metadata.ts b/packages/twenty-server/src/modules/opportunity/standard-objects/opportunity.object-metadata.ts index 2708cc2fab..c8353dd132 100644 --- a/packages/twenty-server/src/modules/opportunity/standard-objects/opportunity.object-metadata.ts +++ b/packages/twenty-server/src/modules/opportunity/standard-objects/opportunity.object-metadata.ts @@ -163,6 +163,7 @@ export class OpportunityObjectMetadata extends BaseObjectMetadata { @RelationMetadata({ type: RelationMetadataType.ONE_TO_MANY, inverseSideTarget: () => AttachmentObjectMetadata, + onDelete: RelationOnDeleteAction.CASCADE, }) @IsNullable() attachments: AttachmentObjectMetadata[]; @@ -177,6 +178,7 @@ export class OpportunityObjectMetadata extends BaseObjectMetadata { @RelationMetadata({ type: RelationMetadataType.ONE_TO_MANY, inverseSideTarget: () => EventObjectMetadata, + onDelete: RelationOnDeleteAction.SET_NULL, }) @IsNullable() events: EventObjectMetadata[]; diff --git a/packages/twenty-server/src/modules/person/standard-objects/person.object-metadata.ts b/packages/twenty-server/src/modules/person/standard-objects/person.object-metadata.ts index 85a4afe978..afed4319a4 100644 --- a/packages/twenty-server/src/modules/person/standard-objects/person.object-metadata.ts +++ b/packages/twenty-server/src/modules/person/standard-objects/person.object-metadata.ts @@ -142,6 +142,7 @@ export class PersonObjectMetadata extends BaseObjectMetadata { type: RelationMetadataType.ONE_TO_MANY, inverseSideTarget: () => OpportunityObjectMetadata, inverseSideFieldKey: 'pointOfContact', + onDelete: RelationOnDeleteAction.SET_NULL, }) pointOfContactForOpportunities: OpportunityObjectMetadata[]; @@ -199,6 +200,7 @@ export class PersonObjectMetadata extends BaseObjectMetadata { type: RelationMetadataType.ONE_TO_MANY, inverseSideTarget: () => MessageParticipantObjectMetadata, inverseSideFieldKey: 'person', + onDelete: RelationOnDeleteAction.SET_NULL, }) @IsSystem() messageParticipants: MessageParticipantObjectMetadata[]; @@ -213,6 +215,7 @@ export class PersonObjectMetadata extends BaseObjectMetadata { @RelationMetadata({ type: RelationMetadataType.ONE_TO_MANY, inverseSideTarget: () => CalendarEventAttendeeObjectMetadata, + onDelete: RelationOnDeleteAction.SET_NULL, }) @Gate({ featureFlag: 'IS_CALENDAR_ENABLED', diff --git a/packages/twenty-server/src/modules/view/standard-objects/view.object-metadata.ts b/packages/twenty-server/src/modules/view/standard-objects/view.object-metadata.ts index c644497b8f..91705f1ea0 100644 --- a/packages/twenty-server/src/modules/view/standard-objects/view.object-metadata.ts +++ b/packages/twenty-server/src/modules/view/standard-objects/view.object-metadata.ts @@ -1,5 +1,8 @@ import { FieldMetadataType } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity'; -import { RelationMetadataType } from 'src/engine/metadata-modules/relation-metadata/relation-metadata.entity'; +import { + RelationMetadataType, + RelationOnDeleteAction, +} from 'src/engine/metadata-modules/relation-metadata/relation-metadata.entity'; import { viewStandardFieldIds } from 'src/engine/workspace-manager/workspace-sync-metadata/constants/standard-field-ids'; import { standardObjectIds } from 'src/engine/workspace-manager/workspace-sync-metadata/constants/standard-object-ids'; import { FieldMetadata } from 'src/engine/workspace-manager/workspace-sync-metadata/decorators/field-metadata.decorator'; @@ -102,6 +105,7 @@ export class ViewObjectMetadata extends BaseObjectMetadata { @RelationMetadata({ type: RelationMetadataType.ONE_TO_MANY, inverseSideTarget: () => ViewFieldObjectMetadata, + onDelete: RelationOnDeleteAction.SET_NULL, }) @IsNullable() viewFields: ViewFieldObjectMetadata[]; @@ -116,6 +120,7 @@ export class ViewObjectMetadata extends BaseObjectMetadata { @RelationMetadata({ type: RelationMetadataType.ONE_TO_MANY, inverseSideTarget: () => ViewFilterObjectMetadata, + onDelete: RelationOnDeleteAction.SET_NULL, }) @IsNullable() viewFilters: ViewFilterObjectMetadata[]; @@ -130,6 +135,7 @@ export class ViewObjectMetadata extends BaseObjectMetadata { @RelationMetadata({ type: RelationMetadataType.ONE_TO_MANY, inverseSideTarget: () => ViewSortObjectMetadata, + onDelete: RelationOnDeleteAction.SET_NULL, }) @IsNullable() viewSorts: ViewSortObjectMetadata[]; diff --git a/packages/twenty-server/src/modules/workspace-member/standard-objects/workspace-member.object-metadata.ts b/packages/twenty-server/src/modules/workspace-member/standard-objects/workspace-member.object-metadata.ts index 86cd02db13..036bc2f6c9 100644 --- a/packages/twenty-server/src/modules/workspace-member/standard-objects/workspace-member.object-metadata.ts +++ b/packages/twenty-server/src/modules/workspace-member/standard-objects/workspace-member.object-metadata.ts @@ -102,6 +102,7 @@ export class WorkspaceMemberObjectMetadata extends BaseObjectMetadata { type: RelationMetadataType.ONE_TO_MANY, inverseSideTarget: () => ActivityObjectMetadata, inverseSideFieldKey: 'author', + onDelete: RelationOnDeleteAction.SET_NULL, }) authoredActivities: ActivityObjectMetadata[]; @@ -116,6 +117,7 @@ export class WorkspaceMemberObjectMetadata extends BaseObjectMetadata { type: RelationMetadataType.ONE_TO_MANY, inverseSideTarget: () => ActivityObjectMetadata, inverseSideFieldKey: 'assignee', + onDelete: RelationOnDeleteAction.SET_NULL, }) assignedActivities: ActivityObjectMetadata[]; @@ -144,6 +146,7 @@ export class WorkspaceMemberObjectMetadata extends BaseObjectMetadata { type: RelationMetadataType.ONE_TO_MANY, inverseSideTarget: () => CompanyObjectMetadata, inverseSideFieldKey: 'accountOwner', + onDelete: RelationOnDeleteAction.SET_NULL, }) accountOwnerForCompanies: CompanyObjectMetadata[]; @@ -158,6 +161,7 @@ export class WorkspaceMemberObjectMetadata extends BaseObjectMetadata { type: RelationMetadataType.ONE_TO_MANY, inverseSideTarget: () => AttachmentObjectMetadata, inverseSideFieldKey: 'author', + onDelete: RelationOnDeleteAction.SET_NULL, }) authoredAttachments: AttachmentObjectMetadata[]; @@ -172,6 +176,7 @@ export class WorkspaceMemberObjectMetadata extends BaseObjectMetadata { type: RelationMetadataType.ONE_TO_MANY, inverseSideTarget: () => CommentObjectMetadata, inverseSideFieldKey: 'author', + onDelete: RelationOnDeleteAction.SET_NULL, }) authoredComments: CommentObjectMetadata[]; @@ -201,6 +206,7 @@ export class WorkspaceMemberObjectMetadata extends BaseObjectMetadata { type: RelationMetadataType.ONE_TO_MANY, inverseSideTarget: () => MessageParticipantObjectMetadata, inverseSideFieldKey: 'workspaceMember', + onDelete: RelationOnDeleteAction.SET_NULL, }) messageParticipants: MessageParticipantObjectMetadata[]; @@ -215,6 +221,7 @@ export class WorkspaceMemberObjectMetadata extends BaseObjectMetadata { type: RelationMetadataType.ONE_TO_MANY, inverseSideTarget: () => BlocklistObjectMetadata, inverseSideFieldKey: 'workspaceMember', + onDelete: RelationOnDeleteAction.SET_NULL, }) blocklist: BlocklistObjectMetadata[]; @@ -229,6 +236,7 @@ export class WorkspaceMemberObjectMetadata extends BaseObjectMetadata { type: RelationMetadataType.ONE_TO_MANY, inverseSideTarget: () => CalendarEventAttendeeObjectMetadata, inverseSideFieldKey: 'workspaceMember', + onDelete: RelationOnDeleteAction.SET_NULL, }) @Gate({ featureFlag: 'IS_CALENDAR_ENABLED',