Connect/Disconnect - Add Disconnect logic + Migration to query builders (insert/update) #13271
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:52425 This environment will automatically shut down when the PR is closed or after 5 hours. |
etiennejouan
force-pushed
the
ej/1149-disconnect
branch
from
c6bc905 to
5db99f3
Compare
etiennejouan
force-pushed
the
ej/1149-disconnect
branch
from
05bd25b to
6e68d18
Compare
etiennejouan
added
-PR: awaiting review
and removed
-PR: wip
-PR: awaiting author
labels
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR implements a comprehensive relation management system for GraphQL operations, adding disconnect functionality alongside existing connect capabilities. The changes enable users to connect and disconnect entity relationships through createMany (with upsert:true), updateOne, and updateMany resolvers.
The implementation introduces several key architectural improvements:
Core Infrastructure: A new RelationNestedQueries class centralizes the logic for processing both connect and disconnect operations, replacing scattered relation handling code across different services. This class follows a two-phase approach: preparation (during .set() or .values()) and execution (during .execute()).
Query Builder Enhancements: Both WorkspaceInsertQueryBuilder and WorkspaceUpdateQueryBuilder have been enhanced to support nested relation fields. The builders now accept QueryDeepPartialEntityWithNestedRelationFields types and process relation operations before executing the main query.
Type System Extensions: The type system has been expanded with new types like RelationConnectQueryFieldsByEntityIndex, RelationDisconnectQueryFieldsByEntityIndex, and DisconnectObject. The main entity type has been renamed from QueryDeepPartialEntityWithRelationConnect to QueryDeepPartialEntityWithNestedRelationFields to reflect the broader scope.
GraphQL Schema Updates: The schema builder now generates disconnect fields for Update input types alongside existing Create types. Input types now include isOneOf: true for proper GraphQL OneOf compliance when handling mutually exclusive connect/disconnect operations.
Utility Functions: New utilities like extractNestedRelationFieldsByEntityIndex parse and validate nested relation operations, while existing utilities like computeRelationConnectQueryConfigs have been refactored to accept pre-processed relation fields.
Testing: Comprehensive integration tests have been added in nested-relation-queries.integration-spec.ts, replacing the previous relation-connect.integration-spec.ts file with expanded coverage for all supported operations.
The changes maintain backward compatibility while providing a foundation for complex relationship management operations across the Twenty platform.
Confidence score: 3/5
• This PR introduces significant architectural changes with potential for runtime issues due to direct query builder mutations and complex relation processing logic
• The score reflects concerns about error handling robustness, potential race conditions in relation processing, and the complexity of the two-phase query execution approach
• Files requiring more attention: RelationNestedQueries class, query builder implementations, and the GraphQL schema factory changes
20 files reviewed, 6 comments
Comment on lines
+110
to
+116
| const queryBuilder = new WorkspaceSelectQueryBuilder( | ||
| this as unknown as WorkspaceSelectQueryBuilder<T>, | ||
| this.objectRecordsPermissions, | ||
| this.internalContext, | ||
| this.shouldBypassPermissionChecks, | ||
| this.authContext, | ||
| ); |
There was a problem hiding this comment.
logic: Creating WorkspaceSelectQueryBuilder with 'this as unknown as WorkspaceSelectQueryBuilder<T>' type assertion is unsafe
packages/twenty-server/src/engine/twenty-orm/relation-nested-queries/relation-nested-queries.ts
Outdated
Show resolved
Hide resolved
packages/twenty-server/src/engine/twenty-orm/relation-nested-queries/relation-nested-queries.ts
Outdated
Show resolved
Hide resolved
packages/twenty-server/src/engine/twenty-orm/relation-nested-queries/relation-nested-queries.ts
Show resolved
Hide resolved
...ty-server/src/engine/twenty-orm/utils/extract-nested-relation-fields-by-entity-index.util.ts
Show resolved
Hide resolved
packages/twenty-server/src/engine/twenty-orm/repository/workspace-update-query-builder.ts
Outdated
Show resolved
Hide resolved
etiennejouan
commented
Jul 23, 2025
| @@ -116,17 +150,41 @@ export class WorkspaceUpdateQueryBuilder< | |||
| }; | |||
| } | |||
|
|
|||
| override set(_values: QueryDeepPartialEntity<T>): this { | |||
| override set( | |||
There was a problem hiding this comment.
I don't know how to handle typing in update query builder here.
etiennejouan
changed the title
📊 API Changes ReportGraphQL Schema ChangesGraphQL Schema Changes[error] Error: Unable to read JSON file: /home/runner/work/twenty/twenty/main-schema-introspection.json: Not valid JSON content REST Metadata API Analysis ErrorError Output
|
Weiko
reviewed
Jul 23, 2025
.../engine/twenty-orm/entity-manager/types/relation-nested-query-fields-by-entity-index.type.ts
Show resolved
Hide resolved
| @@ -88,6 +107,25 @@ export class WorkspaceInsertQueryBuilder< | |||
| this.internalContext, | |||
| ); | |||
|
|
|||
| const queryBuilder = new WorkspaceSelectQueryBuilder( | |||
There was a problem hiding this comment.
Suggested change
| const queryBuilder = new WorkspaceSelectQueryBuilder( | |
| const nestedRelationQueryBuilder = new WorkspaceSelectQueryBuilder( |
| @@ -1462,118 +1446,4 @@ export class WorkspaceEntityManager extends EntityManager { | |||
| PermissionsExceptionCode.RAW_SQL_NOT_ALLOWED, | |||
| ); | |||
| } | |||
|
|
|||
| private async processRelationConnect<Entity extends ObjectLiteral>( | |||
There was a problem hiding this comment.
Do we want to implement the same logic for methods that don't rely on query builders?
save, recover, delete, destroy?
There was a problem hiding this comment.
I'll do it for the .save. Regarding the others, it does not seem useful as they are not create/update operations
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Context :
Large PR with 600+ test files. Enable connect and disconnect logic in createMany (upsert true) / updateOne / updateMany resolvers
Test :