fix: enforce the user to pass in property with id suffix for morph relations in Rest API (#18335)
REST API allowed users to pass in targetOpportunity, targetPerson, targetCompany etc when trying to create a noteTarget or a taskTarget. The request went through, we got back a 201, the record was created, but the relationship was never established since the FK was empty in the database. This PR enforces users to send in the property with the `Id` suffix for consistency. So, the user sends in targetOpportunityId, targetPersonId, targetCompanyId etc. <p align="center"> <img width="854" height="480" alt="image" src="https://github.com/user-attachments/assets/ed50a623-68d4-4266-baf1-e94a657c3fd4" /> </p> If the users try to send without the "Id" suffix, they get an error explaining what to do. <p align="center"> <img width="854" height="480" alt="image" src="https://github.com/user-attachments/assets/8bab399b-7a04-4b6a-86b5-6f0e0b1ecd5d" /> </p> Additionally, the documentation itself contains the correct property names. <p align="center"> <img width="854" height="480" alt="image" src="https://github.com/user-attachments/assets/83e51cd6-8ef7-4a4d-8696-ab37cc4a9dd6" /> </p> Finally, the filters also enforce this "Id" suffix convention in the GET request. <p align="center"> <img width="854" height="480" alt="image" src="https://github.com/user-attachments/assets/168a2f09-1242-40fa-bd84-1f7d9c60357c" /> </p> Edit: Updated error messages after the screenshots were taken to make them a little generic. Secondly, this PR also fixes the issue of morph relation ids and objects not appearing in the response (when depth is 1).
This commit is contained in:
+1
-1
@@ -161,7 +161,7 @@ export const fieldMetadataConfigByFieldName: Record<
|
||||
isNullable: true,
|
||||
},
|
||||
[joinColumnNameForManyToOneMorphRelationField1]: {
|
||||
name: joinColumnNameForManyToOneMorphRelationField1,
|
||||
name: joinColumnNameForManyToOneMorphRelationField1.replace(/Id$/, ''),
|
||||
type: FieldMetadataType.MORPH_RELATION,
|
||||
isNullable: true,
|
||||
settings: {
|
||||
|
||||
+3
-1
@@ -53,7 +53,9 @@ export const successfulInputsByFieldMetadataType: {
|
||||
},
|
||||
{
|
||||
input: { [joinColumnNameForManyToOneMorphRelationField1]: null },
|
||||
expectedOutput: { [joinColumnNameForManyToOneMorphRelationField1]: null },
|
||||
expectedOutput: {
|
||||
[joinColumnNameForManyToOneMorphRelationField1]: null,
|
||||
},
|
||||
},
|
||||
],
|
||||
[FieldMetadataType.RAW_JSON]: [
|
||||
|
||||
+1
-1
@@ -51,7 +51,7 @@ describe('DataArgProcessorService', () => {
|
||||
|
||||
byUniversalIdentifier[universalId] = {
|
||||
id: fieldId,
|
||||
name: fieldName,
|
||||
name: config.name,
|
||||
type: config.type ?? FieldMetadataType.TEXT,
|
||||
isNullable: config.isNullable ?? true,
|
||||
objectMetadataId: 'object-id',
|
||||
|
||||
+25
-7
@@ -15,6 +15,7 @@ import {
|
||||
} from 'twenty-shared/utils';
|
||||
|
||||
import { transformActorField } from 'src/engine/api/common/common-args-processors/data-arg-processor/transformer-utils/transform-actor-field.util';
|
||||
import { isRelationNestedOperation } from 'src/engine/api/common/common-args-processors/data-arg-processor/utils/is-relation-nested-operation.util';
|
||||
import { transformAddressField } from 'src/engine/api/common/common-args-processors/data-arg-processor/transformer-utils/transform-address-field.util';
|
||||
import { transformArrayField } from 'src/engine/api/common/common-args-processors/data-arg-processor/transformer-utils/transform-array-field.util';
|
||||
import { transformCurrencyField } from 'src/engine/api/common/common-args-processors/data-arg-processor/transformer-utils/transform-currency-field.util';
|
||||
@@ -57,7 +58,9 @@ import { transformRichTextV2Value } from 'src/engine/core-modules/record-transfo
|
||||
import { WorkspaceNotFoundDefaultError } from 'src/engine/core-modules/workspace/workspace.exception';
|
||||
import { FlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/types/flat-entity-maps.type';
|
||||
import { findFlatEntityByIdInFlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/utils/find-flat-entity-by-id-in-flat-entity-maps.util';
|
||||
import { computeMorphOrRelationFieldJoinColumnName } from 'src/engine/metadata-modules/field-metadata/utils/compute-morph-or-relation-field-join-column-name.util';
|
||||
import { FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/types/flat-field-metadata.type';
|
||||
import { isFlatFieldMetadataOfType } from 'src/engine/metadata-modules/flat-field-metadata/utils/is-flat-field-metadata-of-type.util';
|
||||
import { buildFieldMapsFromFlatObjectMetadata } from 'src/engine/metadata-modules/flat-field-metadata/utils/build-field-maps-from-flat-object-metadata.util';
|
||||
import { FlatObjectMetadata } from 'src/engine/metadata-modules/flat-object-metadata/types/flat-object-metadata.type';
|
||||
|
||||
@@ -224,13 +227,11 @@ export class DataArgProcessorService {
|
||||
}
|
||||
case FieldMetadataType.RELATION:
|
||||
case FieldMetadataType.MORPH_RELATION: {
|
||||
const fieldMetadataRelationSettings =
|
||||
fieldMetadata.settings as FieldMetadataSettingsMapping['RELATION'];
|
||||
const relationSettings = fieldMetadata.settings as
|
||||
| FieldMetadataSettingsMapping['RELATION']
|
||||
| FieldMetadataSettingsMapping['MORPH_RELATION'];
|
||||
|
||||
if (
|
||||
fieldMetadataRelationSettings.relationType ===
|
||||
RelationType.ONE_TO_MANY
|
||||
) {
|
||||
if (relationSettings.relationType === RelationType.ONE_TO_MANY) {
|
||||
throw new CommonQueryRunnerException(
|
||||
`One-to-many relation ${key} field does not support write operations.`,
|
||||
CommonQueryRunnerExceptionCode.INVALID_ARGS_DATA,
|
||||
@@ -238,10 +239,27 @@ export class DataArgProcessorService {
|
||||
);
|
||||
}
|
||||
|
||||
if (key === fieldMetadataRelationSettings.joinColumnName) {
|
||||
const joinColumnName = isFlatFieldMetadataOfType(
|
||||
fieldMetadata,
|
||||
FieldMetadataType.MORPH_RELATION,
|
||||
)
|
||||
? computeMorphOrRelationFieldJoinColumnName({
|
||||
name: fieldMetadata.name,
|
||||
})
|
||||
: relationSettings.joinColumnName;
|
||||
|
||||
if (key === joinColumnName) {
|
||||
return validateUUIDFieldOrThrow(value, key);
|
||||
}
|
||||
|
||||
if (isDefined(joinColumnName) && !isRelationNestedOperation(value)) {
|
||||
throw new CommonQueryRunnerException(
|
||||
`Relation "${key}" requires connect or disconnect operation`,
|
||||
CommonQueryRunnerExceptionCode.INVALID_ARGS_DATA,
|
||||
{ userFriendlyMessage: STANDARD_ERROR_MESSAGE },
|
||||
);
|
||||
}
|
||||
|
||||
return value;
|
||||
}
|
||||
case FieldMetadataType.PHONES: {
|
||||
|
||||
+18
@@ -0,0 +1,18 @@
|
||||
import { isObject } from '@sniptt/guards';
|
||||
import { RELATION_NESTED_QUERY_KEYWORDS } from 'twenty-shared/constants';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
|
||||
const { CONNECT, DISCONNECT } = RELATION_NESTED_QUERY_KEYWORDS;
|
||||
|
||||
export const isRelationNestedOperation = (value: unknown): boolean => {
|
||||
if (!isObject(value)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const obj = value as Record<string, unknown>;
|
||||
|
||||
return (
|
||||
(CONNECT in obj && isDefined(obj[CONNECT])) ||
|
||||
(DISCONNECT in obj && isDefined(obj[DISCONNECT]))
|
||||
);
|
||||
};
|
||||
+4
-1
@@ -92,7 +92,10 @@ export class CommonSelectFieldsHelper {
|
||||
flatEntityId: fieldId,
|
||||
});
|
||||
|
||||
if (!isFlatFieldMetadataOfType(flatField, FieldMetadataType.RELATION)) {
|
||||
if (
|
||||
!isFlatFieldMetadataOfType(flatField, FieldMetadataType.RELATION) &&
|
||||
!isFlatFieldMetadataOfType(flatField, FieldMetadataType.MORPH_RELATION)
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
|
||||
|
||||
+2
-2
@@ -523,7 +523,7 @@ describe('getAllSelectableFields', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('should include MANY_TO_ONE relations without joinColumnName as regular fields', () => {
|
||||
it('should compute join column name for MANY_TO_ONE relations without joinColumnName in settings', () => {
|
||||
const nameField = createMockField({
|
||||
id: 'field-1',
|
||||
name: 'name',
|
||||
@@ -555,7 +555,7 @@ describe('getAllSelectableFields', () => {
|
||||
|
||||
expect(result).toEqual({
|
||||
name: true,
|
||||
company: true, // MANY_TO_ONE without joinColumnName are included as regular fields
|
||||
companyId: true,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
+12
-5
@@ -8,9 +8,9 @@ import {
|
||||
type RestrictedFieldsPermissions,
|
||||
compositeTypeDefinitions,
|
||||
} from 'twenty-shared/types';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
|
||||
import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util';
|
||||
import { computeMorphOrRelationFieldJoinColumnName } from 'src/engine/metadata-modules/field-metadata/utils/compute-morph-or-relation-field-join-column-name.util';
|
||||
import { type FlatEntityMaps } from 'src/engine/metadata-modules/flat-entity/types/flat-entity-maps.type';
|
||||
import { findFlatEntityByIdInFlatEntityMapsOrThrow } from 'src/engine/metadata-modules/flat-entity/utils/find-flat-entity-by-id-in-flat-entity-maps-or-throw.util';
|
||||
import { type FlatFieldMetadata } from 'src/engine/metadata-modules/flat-field-metadata/types/flat-field-metadata.type';
|
||||
@@ -80,11 +80,18 @@ export const getAllSelectableFields = ({
|
||||
|
||||
result[flatField.name] = compositeFields;
|
||||
} else if (
|
||||
isFlatFieldMetadataOfType(flatField, FieldMetadataType.RELATION) &&
|
||||
flatField.settings.relationType === RelationType.MANY_TO_ONE &&
|
||||
isDefined(flatField.settings.joinColumnName)
|
||||
(isFlatFieldMetadataOfType(flatField, FieldMetadataType.RELATION) ||
|
||||
isFlatFieldMetadataOfType(
|
||||
flatField,
|
||||
FieldMetadataType.MORPH_RELATION,
|
||||
)) &&
|
||||
flatField.settings.relationType === RelationType.MANY_TO_ONE
|
||||
) {
|
||||
result[flatField.settings.joinColumnName] = true;
|
||||
const joinColumnName = computeMorphOrRelationFieldJoinColumnName({
|
||||
name: flatField.name,
|
||||
});
|
||||
|
||||
result[joinColumnName] = true;
|
||||
} else {
|
||||
result[flatField.name] = true;
|
||||
}
|
||||
|
||||
+4
-3
@@ -436,11 +436,12 @@ describe('RestToCommonSelectedFieldsHandler', () => {
|
||||
|
||||
expect(result).toEqual({
|
||||
name: true,
|
||||
companyId: true,
|
||||
company: {
|
||||
name: true,
|
||||
people: {
|
||||
name: true,
|
||||
company: true, // Nested relation at depth 2 shows as boolean
|
||||
companyId: true,
|
||||
},
|
||||
},
|
||||
});
|
||||
@@ -573,12 +574,12 @@ describe('RestToCommonSelectedFieldsHandler', () => {
|
||||
depth: 1,
|
||||
});
|
||||
|
||||
// For noteTarget, only note and task relations are included
|
||||
// but company is a MANY_TO_ONE so it's included as a boolean
|
||||
expect(result).toEqual({
|
||||
noteId: true,
|
||||
note: {
|
||||
title: true,
|
||||
},
|
||||
companyId: true,
|
||||
company: {
|
||||
name: true,
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user