-
Notifications
You must be signed in to change notification settings - Fork 2.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: new relation resolver #9794
Conversation
200d488
to
056df8b
Compare
056df8b
to
dbc5bb8
Compare
822b226
to
80398b9
Compare
0771d0b
to
d733570
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This PR introduces a new relation resolver feature for managing workspace users, with a feature-flag controlled implementation for field metadata relations.
- Added new
FieldMetadataRelationService
in/packages/twenty-server/src/engine/metadata-modules/field-metadata/relation/field-metadata-relation.service.ts
for cached field metadata relation handling - Introduced
RelationDTO
in/dtos/relation.dto.ts
to define relation metadata structure with proper validation - Extended
IDataloaders
interface with newrelationLoader
indataloader.interface.ts
for efficient relation data loading - Added
IsNewRelationEnabled
feature flag (defaulting to false) for controlled rollout - Enhanced
FieldMetadataInterface
with new relation-specific properties for defining field and object relationships
11 file(s) reviewed, 9 comment(s)
Edit PR Review Bot Settings | Greptile
packages/twenty-server/src/engine/dataloaders/dataloader.service.ts
Outdated
Show resolved
Hide resolved
packages/twenty-server/src/engine/metadata-modules/field-metadata/dtos/relation.dto.ts
Outdated
Show resolved
Hide resolved
packages/twenty-server/src/engine/metadata-modules/field-metadata/dtos/relation.dto.ts
Show resolved
Hide resolved
packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.resolver.ts
Show resolved
Hide resolved
packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.resolver.ts
Outdated
Show resolved
Hide resolved
packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.resolver.ts
Show resolved
Hide resolved
...erver/src/engine/metadata-modules/field-metadata/relation/field-metadata-relation.service.ts
Show resolved
Hide resolved
...erver/src/engine/metadata-modules/field-metadata/relation/field-metadata-relation.service.ts
Show resolved
Hide resolved
...erver/src/engine/metadata-modules/field-metadata/relation/field-metadata-relation.service.ts
Outdated
Show resolved
Hide resolved
return { | ||
sourceObjectMetadata: cleanObjectMetadata( | ||
sourceObjectMetadata, | ||
) as ObjectMetadataEntity, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we avoid these as ? maybe use Pick to defined the type of sourceObjectMetadata
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tried many things and we don't really have an easy way for now, I think a big refactor should be made to drop ObjectMetadataInterface
and FieldMetadataInterface
, as they were created at first to create "fake/standard" object from typescript file.
|
||
import { ObjectMetadataItemWithFieldMaps } from 'src/engine/metadata-modules/types/object-metadata-item-with-field-maps'; | ||
|
||
export const cleanObjectMetadata = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not a big fan of this util!
Initial naming is: ObjectMetadataItem (the one we know). ObjectMetadataItemWithFieldMaps (the one with fieldsById, fieldByName...). So could we add it next to the tooling to generate the maps and name it: parseObjectMetadataFromObjectMetadataWithFieldMaps(objectMetadataItemWithFieldMaps: ...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does not seem to be fixed!
...erver/src/engine/metadata-modules/field-metadata/relation/field-metadata-relation.service.ts
Outdated
Show resolved
Hide resolved
packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.resolver.ts
Outdated
Show resolved
Hide resolved
packages/twenty-server/src/engine/dataloaders/dataloader.service.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still a comment to be fixed! (can be done in another PR). Also, shouldn't we also put the dataloader behind feature flag?
Fix #240