feat: implement generic many-to-many junction relation support #16820
Conversation
- Add junctionTargetRelationFieldIds to field metadata settings - Backend validation for junction target relation field IDs - Settings UI to configure junction relations (advanced mode only) - Frontend hooks for junction relation CRUD operations - Display junction target objects instead of junction records - Proper GraphQL field generation for nested junction targets
|
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:28618 This environment will automatically shut down when the PR is closed or after 5 hours. |
…eldMetadataItemRelation
…kspace-entity-manager.spec.ts
- Consolidated all junction-related utilities into junction/ subdirectory - Each file now has a single export for better tree-shaking - Added barrel export for clean imports - Extracted field categorization logic into reusable utility
… early returns, clean up debug code
…r -> polymorphicOwner)
… from search tests
…elPayload utility, use joinColumnName from settings
| objectMetadataItems: ObjectMetadataItem[]; | ||
| }; | ||
|
|
||
| // Build GraphQL fields for a regular RELATION target field |
There was a problem hiding this comment.
Done - removed the comment.
| objectNameSingular, | ||
| }); | ||
|
|
||
| // Order must match the rendering order in RecordFieldList |
There was a problem hiding this comment.
Done - removed the comment.
| boxedRelationFieldMetadataItems, | ||
| inlineFieldMetadataItems: allInlineFieldMetadataItems, | ||
| // Activity targets need special rendering with ActivityTargetsInlineCell | ||
| inlineRelationFieldMetadataItems: activityTargetFields, |
There was a problem hiding this comment.
Maybe rename to something with LegacyActivityTarget or something like that so we understand it's just that and will be migrated
There was a problem hiding this comment.
Done! Renamed inlineRelationFieldMetadataItems to legacyActivityTargetFieldMetadataItems to make it clear this is specifically for noteTargets/taskTargets that will be migrated.
| boxedRelationFields: FieldMetadataItem[]; | ||
| }; | ||
|
|
||
| // Check if field is a hardcoded activity target relation (noteTargets/taskTargets) |
There was a problem hiding this comment.
Remove comment (not helpful/needed)
There was a problem hiding this comment.
Done - removed the comment.
| (objectNameSingular === CoreObjectNameSingular.Task && | ||
| fieldMetadataItem.name === 'taskTargets'); | ||
|
|
||
| // Check if user has read permission for the relation's target object(s) |
There was a problem hiding this comment.
Remove comment (not helpful/needed)
There was a problem hiding this comment.
Done - removed the comment.
| .getLoadable(currentWorkspaceState) | ||
| .getValue(); | ||
|
|
||
| const isJunctionRelationsEnabled = |
There was a problem hiding this comment.
I'm not sure the feature flag check is needed here? Maybe we can get rid of currentWorkspace / this check, because this happens in "open input" and it's too late, it means the field is already shown the the user anyway if we arrived there
There was a problem hiding this comment.
[AI Reply] Good question! The feature flag check IS still needed here because while the field is shown in the UI (as a regular relation), the junction behavior (opening MultipleRecordPicker with checkbox selection) should only activate if the flag is enabled. Without this check, users would get the junction interaction but the backend wouldn't support it. However, I can see how this might be confusing - in a future iteration when the flag is removed, this check can be cleaned up.
| )?.value ?? false; | ||
|
|
||
| // Get the actual field metadata item with saved settings from the database | ||
| const { fieldMetadataItem } = getFieldMetadataItemById({ |
There was a problem hiding this comment.
Do we really need this? Why don't we have settings on fieldDefinition?
There was a problem hiding this comment.
Great catch! You're right - settings IS already on fieldDefinition.metadata.settings (populated by formatFieldMetadataItemAsFieldDefinition). I've removed the redundant getFieldMetadataItemById call and now use fieldDefinition.metadata.settings directly. Much cleaner!
| objectMetadataItems, | ||
| }); | ||
|
|
||
| // Check for metadata-driven junction relation (many-to-many through junction) |
There was a problem hiding this comment.
Remove these two comments
There was a problem hiding this comment.
Done - removed both comments.
| .getLoadable(objectMetadataItemsState) | ||
| .getValue(); | ||
|
|
||
| // Get the actual field metadata item with saved settings from the database |
There was a problem hiding this comment.
Same question as before, do we really need this? Setting not available on fieldDefinition?
There was a problem hiding this comment.
Fixed! Same issue - now using fieldDefinition.metadata.settings directly instead of fetching separately.
| fieldDefinition?.metadata.relationObjectMetadataNameSingular; | ||
|
|
||
| // Get the actual field metadata item with saved settings | ||
| const { fieldMetadataItem } = getFieldMetadataItemById({ |
There was a problem hiding this comment.
Again (x3) do we really need this? Definition does not have settings?
There was a problem hiding this comment.
Fixed! Same pattern - now using fieldDefinition.metadata.settings directly instead of fetching separately.
| }); | ||
|
|
||
| createRecordPayload[`${gqlField}Id`] = recordId; | ||
| (createRecordPayload as Record<string, unknown>)[`${gqlField}Id`] = |
There was a problem hiding this comment.
Why do we need this ugly casting? Let's keep it clean as it was
There was a problem hiding this comment.
Fixed! The createRecordPayload is of type Record<string, unknown> so the assignment works directly without casting. Removed the ugly cast.
| >(); | ||
| const isMobile = useIsMobile(); | ||
| const isJunctionRelationsEnabled = useIsFeatureEnabled( | ||
| 'IS_JUNCTION_RELATIONS_ENABLED' as FeatureFlagKey, |
There was a problem hiding this comment.
Casting needed? Can we take the flag from graphql for example instead?
There was a problem hiding this comment.
Fixed! Now using the FeatureFlagKey enum directly: FeatureFlagKey.IS_JUNCTION_RELATIONS_ENABLED instead of casting a string.
| }); | ||
| const isJunctionRelation = hasJunctionConfig(fieldMetadataItem?.settings); | ||
|
|
||
| // Get junction config for display |
There was a problem hiding this comment.
Done - removed the comment.
| .filter(isDefined)} | ||
| {targetRecordsWithMetadata.map(({ record, objectMetadata }) => ( | ||
| <RecordChip | ||
| key={record.id as string} |
There was a problem hiding this comment.
Why cast? Please review ALL cast we introduced in this PR and everytime solve the ROOT CAUSE that led to the type issue, avoiding casting.
There was a problem hiding this comment.
Fixed! The cast was unnecessary since ObjectRecord has id: string from BaseObjectRecord. Removed the cast - now just uses record.id directly.
- Remove unnecessary comments throughout the codebase - Rename inlineRelationFieldMetadataItems to legacyActivityTargetFieldMetadataItems for clarity - Use fieldDefinition.metadata.settings instead of redundant getFieldMetadataItemById calls - Remove type castings by fixing root causes (proper Pick<> types) - Use FeatureFlagKey enum instead of string casting
…ame Pet Care Agreements to Cared For Pets - Pass sourceObjectMetadataId in RelationFromManyFieldDisplay for proper source field identification - Rename field label from 'Pet Care Agreements' to 'Cared For Pets' to make it clear we're showing Pets, not agreements
… in junction Add New When creating a junction record via 'Add New', the sourceJoinColumnName was being read directly from sourceField.settings.joinColumnName. For morph source fields (like caretaker on PetCareAgreement), this doesn't work because the join column depends on the actual source object type (Person vs Company). Now the code computes the correct join column name dynamically using computeMorphRelationFieldName when the source field is a MORPH_RELATION.
…ionRelationGqlFields, lint errors in twenty-shared
![sentry[bot]](https://avatars.githubusercontent.com/u/1396951?s=60&v=4){kind=small}
| ? [...currentFieldValue, junctionRecordForStore] | ||
| : [junctionRecordForStore]; | ||
|
|
||
| return { | ||
| ...currentRecord, | ||
| [fieldName]: updatedJunctionRecords, | ||
| }; | ||
| }); | ||
|
|
||
| await createJunctionRecord(newJunctionRecordForApi); | ||
| } |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
6 issues found across 71 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/twenty-front/src/modules/object-record/record-inline-cell/components/RecordInlineCellValue.tsx">
<violation number="1" location="packages/twenty-front/src/modules/object-record/record-inline-cell/components/RecordInlineCellValue.tsx:45">
P2: The `onClick` handler is not guarded by `readonly` state. While visual cues (cursor, edit button) correctly hide when readonly, clicking the cell will still trigger `onOpenEditMode`. Consider conditionally applying the handler.</violation>
</file>
<file name="packages/twenty-front/src/modules/object-record/utils/buildRecordLabelPayload.ts">
<violation number="1" location="packages/twenty-front/src/modules/object-record/utils/buildRecordLabelPayload.ts:22">
P2: Whitespace handling bug in name parsing. Input with leading/trailing spaces or multiple spaces between words will incorrectly parse firstName/lastName (e.g., `' John Doe '` gives `firstName=''` instead of `'John'`). Consider trimming and splitting on whitespace pattern.</violation>
</file>
<file name="packages/twenty-front/src/modules/object-record/record-field/ui/hooks/useUpdateJunctionRelationFromCell.ts">
<violation number="1" location="packages/twenty-front/src/modules/object-record/record-field/ui/hooks/useUpdateJunctionRelationFromCell.ts:234">
P2: Missing error handling for optimistic update rollback. If `createJunctionRecord` fails, the Recoil store will retain the stale junction record with no cleanup. Consider wrapping in try/catch to rollback the store update on failure, similar to how `useCreateOneRecord` handles errors internally.</violation>
</file>
<file name="packages/twenty-front/src/modules/settings/data-model/fields/forms/morph-relation/components/SettingsDataModelFieldRelationJunctionForm.tsx">
<violation number="1" location="packages/twenty-front/src/modules/settings/data-model/fields/forms/morph-relation/components/SettingsDataModelFieldRelationJunctionForm.tsx:178">
P2: The `targetObjectLabel` always uses `regularRelationFields[0]` instead of the actually selected field. When multiple regular relation fields exist and the user selects one other than the first, the description will show the wrong target object label. Should find the selected field using `junctionTargetFieldId`.</violation>
</file>
<file name="packages/twenty-front/src/pages/settings/data-model/SettingsObjectFieldEdit.tsx">
<violation number="1" location="packages/twenty-front/src/pages/settings/data-model/SettingsObjectFieldEdit.tsx:129">
P1: Changing from `values` to `defaultValues` breaks form reactivity. Since `fieldMetadataItem` data loads asynchronously from Recoil state, the form will initialize with fallback values (`'Icon'`, `''`, `true`, etc.) and won't update when actual data arrives. This can cause users to see incorrect values and potentially submit wrong data.
Consider keeping `values` for data that loads asynchronously, or use `reset()` in a `useEffect` when the data changes.</violation>
</file>
<file name="packages/twenty-front/src/modules/object-record/record-field-list/utils/categorizeRelationFields.ts">
<violation number="1" location="packages/twenty-front/src/modules/object-record/record-field-list/utils/categorizeRelationFields.ts:46">
P2: Empty `morphRelations` array causes `.every()` to return `true` (vacuous truth), potentially granting unintended read access. Consider adding a length check to ensure there are actual morph relations to verify permissions against.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| mode: 'onTouched', | ||
| resolver: zodResolver(settingsFieldFormSchema()), | ||
| values: { | ||
| defaultValues: { |
There was a problem hiding this comment.
P1: Changing from values to defaultValues breaks form reactivity. Since fieldMetadataItem data loads asynchronously from Recoil state, the form will initialize with fallback values ('Icon', '', true, etc.) and won't update when actual data arrives. This can cause users to see incorrect values and potentially submit wrong data.
Consider keeping values for data that loads asynchronously, or use reset() in a useEffect when the data changes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-front/src/pages/settings/data-model/SettingsObjectFieldEdit.tsx, line 129:
<comment>Changing from `values` to `defaultValues` breaks form reactivity. Since `fieldMetadataItem` data loads asynchronously from Recoil state, the form will initialize with fallback values (`'Icon'`, `''`, `true`, etc.) and won't update when actual data arrives. This can cause users to see incorrect values and potentially submit wrong data.
Consider keeping `values` for data that loads asynchronously, or use `reset()` in a `useEffect` when the data changes.</comment>
<file context>
@@ -108,20 +108,34 @@ export const SettingsObjectFieldEdit = () => {
mode: 'onTouched',
resolver: zodResolver(settingsFieldFormSchema()),
- values: {
+ defaultValues: {
icon: fieldMetadataItem?.icon ?? 'Icon',
type: fieldMetadataItem?.type as SettingsFieldType,
</file context>
| <RecordInlineCellDisplayMode isHovered={false}> | ||
| <RecordInlineCellDisplayMode | ||
| isHovered={isFocused} | ||
| onClick={onOpenEditMode} |
There was a problem hiding this comment.
P2: The onClick handler is not guarded by readonly state. While visual cues (cursor, edit button) correctly hide when readonly, clicking the cell will still trigger onOpenEditMode. Consider conditionally applying the handler.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-front/src/modules/object-record/record-inline-cell/components/RecordInlineCellValue.tsx, line 45:
<comment>The `onClick` handler is not guarded by `readonly` state. While visual cues (cursor, edit button) correctly hide when readonly, clicking the cell will still trigger `onOpenEditMode`. Consider conditionally applying the handler.</comment>
<file context>
@@ -29,15 +30,20 @@ const StyledClickableContainer = styled.div<{
- <RecordInlineCellDisplayMode isHovered={false}>
+ <RecordInlineCellDisplayMode
+ isHovered={isFocused}
+ onClick={onOpenEditMode}
+ >
<FieldDisplay />
</file context>
| onClick={onOpenEditMode} | |
| onClick={!readonly ? onOpenEditMode : undefined} |
| getLabelIdentifierFieldMetadataItem(objectMetadataItem); | ||
|
|
||
| if (labelIdentifierField?.type === FieldMetadataType.FULL_NAME) { | ||
| const words = searchInput?.split(' ') ?? []; |
There was a problem hiding this comment.
P2: Whitespace handling bug in name parsing. Input with leading/trailing spaces or multiple spaces between words will incorrectly parse firstName/lastName (e.g., ' John Doe ' gives firstName='' instead of 'John'). Consider trimming and splitting on whitespace pattern.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-front/src/modules/object-record/utils/buildRecordLabelPayload.ts, line 22:
<comment>Whitespace handling bug in name parsing. Input with leading/trailing spaces or multiple spaces between words will incorrectly parse firstName/lastName (e.g., `' John Doe '` gives `firstName=''` instead of `'John'`). Consider trimming and splitting on whitespace pattern.</comment>
<file context>
@@ -0,0 +1,34 @@
+ getLabelIdentifierFieldMetadataItem(objectMetadataItem);
+
+ if (labelIdentifierField?.type === FieldMetadataType.FULL_NAME) {
+ const words = searchInput?.split(' ') ?? [];
+ const hasMultipleWords = words.length > 1;
+
</file context>
| }; | ||
| }); | ||
|
|
||
| await createJunctionRecord(newJunctionRecordForApi); |
There was a problem hiding this comment.
P2: Missing error handling for optimistic update rollback. If createJunctionRecord fails, the Recoil store will retain the stale junction record with no cleanup. Consider wrapping in try/catch to rollback the store update on failure, similar to how useCreateOneRecord handles errors internally.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-front/src/modules/object-record/record-field/ui/hooks/useUpdateJunctionRelationFromCell.ts, line 234:
<comment>Missing error handling for optimistic update rollback. If `createJunctionRecord` fails, the Recoil store will retain the stale junction record with no cleanup. Consider wrapping in try/catch to rollback the store update on failure, similar to how `useCreateOneRecord` handles errors internally.</comment>
<file context>
@@ -0,0 +1,259 @@
+ };
+ });
+
+ await createJunctionRecord(newJunctionRecordForApi);
+ }
+ },
</file context>
| const isMorphSelected = selectedOption?.type === 'morph'; | ||
| const targetObjectLabel = isMorphSelected | ||
| ? t`linked records` | ||
| : ((regularRelationFields[0]?.relation?.targetObjectMetadata |
There was a problem hiding this comment.
P2: The targetObjectLabel always uses regularRelationFields[0] instead of the actually selected field. When multiple regular relation fields exist and the user selects one other than the first, the description will show the wrong target object label. Should find the selected field using junctionTargetFieldId.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-front/src/modules/settings/data-model/fields/forms/morph-relation/components/SettingsDataModelFieldRelationJunctionForm.tsx, line 178:
<comment>The `targetObjectLabel` always uses `regularRelationFields[0]` instead of the actually selected field. When multiple regular relation fields exist and the user selects one other than the first, the description will show the wrong target object label. Should find the selected field using `junctionTargetFieldId`.</comment>
<file context>
@@ -0,0 +1,231 @@
+ const isMorphSelected = selectedOption?.type === 'morph';
+ const targetObjectLabel = isMorphSelected
+ ? t`linked records`
+ : ((regularRelationFields[0]?.relation?.targetObjectMetadata
+ ? objectMetadataItems.find(
+ (item) =>
</file context>
| fieldMetadataItem.relation?.targetObjectMetadata.id, | ||
| ).canReadObjectRecords; | ||
|
|
||
| const canReadMorphRelation = fieldMetadataItem?.morphRelations?.every( |
There was a problem hiding this comment.
P2: Empty morphRelations array causes .every() to return true (vacuous truth), potentially granting unintended read access. Consider adding a length check to ensure there are actual morph relations to verify permissions against.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-front/src/modules/object-record/record-field-list/utils/categorizeRelationFields.ts, line 46:
<comment>Empty `morphRelations` array causes `.every()` to return `true` (vacuous truth), potentially granting unintended read access. Consider adding a length check to ensure there are actual morph relations to verify permissions against.</comment>
<file context>
@@ -0,0 +1,92 @@
+ fieldMetadataItem.relation?.targetObjectMetadata.id,
+ ).canReadObjectRecords;
+
+ const canReadMorphRelation = fieldMetadataItem?.morphRelations?.every(
+ (morphRelation) =>
+ isDefined(morphRelation.targetObjectMetadata.id) &&
</file context>
- Split junction utility functions into separate files for better modularity and clarity. - Update import paths to reflect the new structure. - Remove unnecessary comments and streamline code for improved readability.
There was a problem hiding this comment.
1 issue found across 35 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/twenty-front/src/modules/object-record/record-field/ui/utils/junction/isObjectWithId.ts">
<violation number="1" location="packages/twenty-front/src/modules/object-record/record-field/ui/utils/junction/isObjectWithId.ts:4">
P2: Type guard narrowing is incomplete - `ObjectRecord` requires both `id` and `__typename`, but only `id` is validated. This could cause runtime errors if consuming code accesses `__typename` after this check. Consider either:
1. Adding `'__typename' in value && typeof value.__typename === 'string'` to the check, or
2. Using a more accurate return type like `value is { id: string; [key: string]: unknown }`</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| import { type ObjectRecord } from '@/object-record/types/ObjectRecord'; | ||
| import { isDefined } from 'twenty-shared/utils'; | ||
|
|
||
| export const isObjectWithId = (value: unknown): value is ObjectRecord => |
There was a problem hiding this comment.
P2: Type guard narrowing is incomplete - ObjectRecord requires both id and __typename, but only id is validated. This could cause runtime errors if consuming code accesses __typename after this check. Consider either:
- Adding
'__typename' in value && typeof value.__typename === 'string'to the check, or - Using a more accurate return type like
value is { id: string; [key: string]: unknown }
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-front/src/modules/object-record/record-field/ui/utils/junction/isObjectWithId.ts, line 4:
<comment>Type guard narrowing is incomplete - `ObjectRecord` requires both `id` and `__typename`, but only `id` is validated. This could cause runtime errors if consuming code accesses `__typename` after this check. Consider either:
1. Adding `'__typename' in value && typeof value.__typename === 'string'` to the check, or
2. Using a more accurate return type like `value is { id: string; [key: string]: unknown }`</comment>
<file context>
@@ -0,0 +1,8 @@
+import { type ObjectRecord } from '@/object-record/types/ObjectRecord';
+import { isDefined } from 'twenty-shared/utils';
+
+export const isObjectWithId = (value: unknown): value is ObjectRecord =>
+ isDefined(value) &&
+ typeof value === 'object' &&
</file context>
…lationOneToManyFieldInput - Update the RelationOneToManyFieldInput component to ensure objectMetadataNameSingular is not null by providing a default empty string. This prevents potential runtime errors when the value is undefined.
|
|
||
| await createTargetRecord(targetPayload); | ||
|
|
||
| const newJunctionId = v4(); | ||
| const createdJunction = await createJunctionRecord({ | ||
| id: newJunctionId, | ||
| [sourceJoinColumnName]: recordId, | ||
| [targetJoinColumnName]: newTargetId, | ||
| }); | ||
|
|
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Overview
This PR implements generic many-to-many relation support through junction tables (also known as associative entities or join tables). This replaces the need for hardcoded taskTarget/noteTarget logic and provides a flexible foundation for modeling complex entity relationships.
Architecture
Data Model
Many-to-many relationships are implemented using a junction object pattern:
The junction object (PetRocket) has:
MANY_TO_ONErelation to Pet (the source)MANY_TO_ONErelation to Rocket (the target)The source object (Pet) has a
ONE_TO_MANYrelation pointing to the junction, with field settings that specify which target field to follow.Field Settings Schema
Junction configuration is stored in
FieldMetadataRelationSettings:Two configuration modes:
junctionTargetFieldId- References a specificRELATIONfield on the junctionjunctionTargetMorphId- References amorphIdgroup for polymorphic targets (e.g., link to Person OR Company)GraphQL Query Generation
When a junction relation is detected, the GraphQL fields are generated to fetch the nested target:
For polymorphic junction targets:
Frontend Architecture
Display Flow
hasJunctionConfig()checks if field has junction settingsgetJunctionConfig()resolves junction object metadata and target fieldsextractTargetRecordsFromJunction()extracts target records from junction recordsEdit Flow
Picker Opening: Initializes the multi-record picker with:
Selection Handling: Manages create/delete of junction records:
Key Trade-offs
Backend Changes
MANY_TO_ONErelationFieldMetadataRelationSettingstype with junction fieldsHow to Test
IS_JUNCTION_RELATIONS_ENABLEDfeature flagScreen.Recording.2026-01-07.at.13.01.47.mov