feat: create RelationMetadataV2 #9691
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR introduces RelationMetadataV2 to smoothly transition from 'from/to' to 'source/target' naming in relation metadata, with significant structural changes to support improved relationship handling between objects.
- Added new
RelationMetadataV2Entitywith source/target naming and proper TypeORM decorators in/packages/twenty-server/src/engine/metadata-modules/relation-metadata-v2/relation-metadata-v2.entity.ts - Created migration
1737037997641-createRelationMetadataV2.tsto set up new table structure with appropriate constraints and foreign keys - Implemented comprehensive
RelationMetadataV2Servicewith CRUD operations, validation, and workspace-specific metadata handling - Added proper exception handling through
RelationMetadataV2Exceptionand GraphQL error mapping - Note:
relation-to-delete-v2.tsstill uses old 'from/to' naming convention and needs updating
19 file(s) reviewed, 32 comment(s)
Edit PR Review Bot Settings | Greptile
| @OneToOne( | ||
| () => RelationMetadataV2Entity, | ||
| (relation: RelationMetadataV2Entity) => relation.sourceFieldMetadata, | ||
| ) | ||
| sourceRelationMetadata: Relation<RelationMetadataV2Entity>; |
There was a problem hiding this comment.
logic: Missing @joincolumn decorator for OneToOne relationship. This could cause issues with foreign key mapping.
| @OneToOne( | ||
| () => RelationMetadataV2Entity, | ||
| (relation: RelationMetadataV2Entity) => relation.targetFieldMetadata, | ||
| ) | ||
| targetRelationMetadata: Relation<RelationMetadataV2Entity>; |
There was a problem hiding this comment.
logic: Missing @joincolumn decorator for OneToOne relationship. This could cause issues with foreign key mapping.
|
|
||
| public async up(queryRunner: QueryRunner): Promise<void> { | ||
| await queryRunner.query(`CREATE TYPE "metadata"."relationMetadataV2_ondeleteaction_enum" AS ENUM('CASCADE', 'RESTRICT', 'SET_NULL', 'NO_ACTION')`); | ||
| await queryRunner.query(`CREATE TABLE "metadata"."relationMetadataV2" ("id" uuid NOT NULL DEFAULT uuid_generate_v4(), "relationType" character varying NOT NULL, "onDeleteAction" "metadata"."relationMetadataV2_ondeleteaction_enum" NOT NULL DEFAULT 'SET_NULL', "workspaceId" uuid NOT NULL, "sourceObjectMetadataId" uuid NOT NULL, "targetObjectMetadataId" uuid NOT NULL, "sourceFieldMetadataId" uuid NOT NULL, "targetFieldMetadataId" uuid NOT NULL, "createdAt" TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(), "updatedAt" TIMESTAMP WITH TIME ZONE NOT NULL DEFAULT now(), CONSTRAINT "REL_c73d1e4657f79a6ec346e00916" UNIQUE ("sourceFieldMetadataId"), CONSTRAINT "REL_74afbb10c407038ec790eb5f8e" UNIQUE ("targetFieldMetadataId"), CONSTRAINT "PK_46973b622a2907b5a9a2d3f3db8" PRIMARY KEY ("id"))`); |
There was a problem hiding this comment.
style: relationType is defined as character varying without length limit - consider adding a maximum length constraint for data integrity
| await queryRunner.query(`ALTER TABLE "metadata"."relationMetadataV2" ADD CONSTRAINT "FK_c73d1e4657f79a6ec346e00916b" FOREIGN KEY ("sourceFieldMetadataId") REFERENCES "metadata"."fieldMetadata"("id") ON DELETE NO ACTION ON UPDATE NO ACTION`); | ||
| await queryRunner.query(`ALTER TABLE "metadata"."relationMetadataV2" ADD CONSTRAINT "FK_74afbb10c407038ec790eb5f8e5" FOREIGN KEY ("targetFieldMetadataId") REFERENCES "metadata"."fieldMetadata"("id") ON DELETE NO ACTION ON UPDATE NO ACTION`); |
There was a problem hiding this comment.
logic: sourceFieldMetadataId and targetFieldMetadataId have NO ACTION on delete - this could lead to orphaned relations if fields are deleted. Consider using CASCADE
| @HideField() | ||
| workspaceId: string; |
There was a problem hiding this comment.
logic: workspaceId lacks validation decorators (@IsUUID, @isnotempty) despite being required in the service
| export type RelationToDeleteV2 = { | ||
| id: string; | ||
| fromFieldMetadataId: string; | ||
| toFieldMetadataId: string; | ||
| fromFieldMetadataName: string; | ||
| toFieldMetadataName: string; | ||
| fromObjectMetadataId: string; | ||
| toObjectMetadataId: string; | ||
| fromObjectName: string; | ||
| toObjectName: string; | ||
| toFieldMetadataIsCustom: boolean; | ||
| toObjectMetadataIsCustom: boolean; | ||
| direction: string; | ||
| }; |
There was a problem hiding this comment.
logic: All 'from/to' prefixes should be renamed to 'source/target' to match the PR requirements
| toObjectName: string; | ||
| toFieldMetadataIsCustom: boolean; | ||
| toObjectMetadataIsCustom: boolean; | ||
| direction: string; |
There was a problem hiding this comment.
style: The 'direction' field type should be an enum or union type to restrict possible values
| toFieldMetadataIsCustom: boolean; | ||
| toObjectMetadataIsCustom: boolean; |
There was a problem hiding this comment.
logic: Only checking 'to' side for custom flags - should also check 'from/source' side for consistency
| case RelationMetadataV2ExceptionCode.FOREIGN_KEY_NOT_FOUND: | ||
| default: | ||
| throw new InternalServerError(error.message); |
There was a problem hiding this comment.
logic: FOREIGN_KEY_NOT_FOUND case falls through to default. Consider handling it separately if it needs different error handling than InternalServerError.
| } | ||
| } | ||
|
|
||
| throw error; |
There was a problem hiding this comment.
style: Re-throwing raw errors could expose sensitive information. Consider wrapping non-RelationMetadataV2Exceptions in InternalServerError.
This PR is creating
RelationMetadataV2in order to refactor relations smoothly.Fix twentyhq/core-team-issues#237