Fix deactivate objects impacts (#11185)

In this PR:
- Remove deactivated objects from ActivityTargetInlineCell record picker
- Prevent users to deactivate createdAt, updatedAt, deletedAt fields on
any objects

Still left:
- write unit tests on the assert utils
- write integration tests on field metadata service
- prevent users to deactivate createdAt, updatedAt, deletedAt on FE
This commit is contained in:
Charles Bochet
2025-03-26 20:45:46 +01:00
committed by GitHub
parent 90e884d33f
commit 5bd10d40cb
49 changed files with 861 additions and 531 deletions

View File

@ -15,4 +15,5 @@ export enum FieldMetadataExceptionCode {
INTERNAL_SERVER_ERROR = 'INTERNAL_SERVER_ERROR',
FIELD_METADATA_RELATION_NOT_ENABLED = 'FIELD_METADATA_RELATION_NOT_ENABLED',
FIELD_METADATA_RELATION_MALFORMED = 'FIELD_METADATA_RELATION_MALFORMED',
LABEL_IDENTIFIER_FIELD_METADATA_ID_NOT_FOUND = 'LABEL_IDENTIFIER_FIELD_METADATA_ID_NOT_FOUND',
}

View File

@ -30,6 +30,7 @@ import {
} from 'src/engine/metadata-modules/field-metadata/field-metadata.exception';
import { FieldMetadataRelatedRecordsService } from 'src/engine/metadata-modules/field-metadata/services/field-metadata-related-records.service';
import { assertDoesNotNullifyDefaultValueForNonNullableField } from 'src/engine/metadata-modules/field-metadata/utils/assert-does-not-nullify-default-value-for-non-nullable-field.util';
import { checkCanDeactivateFieldOrThrow } from 'src/engine/metadata-modules/field-metadata/utils/check-can-deactivate-field-or-throw';
import {
computeColumnName,
computeCompositeColumnName,
@ -154,6 +155,12 @@ export class FieldMetadataService extends TypeOrmQueryService<FieldMetadataEntit
);
}
if (!objectMetadata.labelIdentifierFieldMetadataId) {
throw new FieldMetadataException(
'Label identifier field metadata id does not exist',
FieldMetadataExceptionCode.LABEL_IDENTIFIER_FIELD_METADATA_ID_NOT_FOUND,
);
}
assertMutationNotOnRemoteObject(objectMetadata);
assertDoesNotNullifyDefaultValueForNonNullableField({
@ -161,18 +168,13 @@ export class FieldMetadataService extends TypeOrmQueryService<FieldMetadataEntit
defaultValueFromUpdate: fieldMetadataInput.defaultValue,
});
if (
objectMetadata.labelIdentifierFieldMetadataId ===
existingFieldMetadata.id &&
fieldMetadataInput.isActive === false
) {
throw new FieldMetadataException(
'Cannot deactivate label identifier field',
FieldMetadataExceptionCode.FIELD_MUTATION_NOT_ALLOWED,
);
}
if (fieldMetadataInput.isActive === false) {
checkCanDeactivateFieldOrThrow({
labelIdentifierFieldMetadataId:
objectMetadata.labelIdentifierFieldMetadataId,
existingFieldMetadata,
});
const viewsRepository =
await this.twentyORMGlobalManager.getRepositoryForWorkspace(
fieldMetadataInput.workspaceId,

View File

@ -30,6 +30,8 @@ export interface FieldMetadataInterface<
relationTargetObjectMetadataId?: string;
relationTargetObjectMetadata?: ObjectMetadataEntity;
isCustom?: boolean;
isSystem?: boolean;
isActive?: boolean;
generatedType?: 'STORED' | 'VIRTUAL';
asExpression?: string;
}

View File

@ -0,0 +1,11 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
exports[`checkCanDeactivateFieldOrThrow throws if trying to deactivate createdAt field 1`] = `"Cannot deactivate createdAt, updatedAt or deletedAt field"`;
exports[`checkCanDeactivateFieldOrThrow throws if trying to deactivate deletedAt field 1`] = `"Cannot deactivate createdAt, updatedAt or deletedAt field"`;
exports[`checkCanDeactivateFieldOrThrow throws if trying to deactivate label identifier field 1`] = `"Cannot deactivate label identifier field"`;
exports[`checkCanDeactivateFieldOrThrow throws if trying to deactivate system field 1`] = `"Cannot deactivate system field"`;
exports[`checkCanDeactivateFieldOrThrow throws if trying to deactivate updatedAt field 1`] = `"Cannot deactivate createdAt, updatedAt or deletedAt field"`;

View File

@ -0,0 +1,106 @@
import { EachTestingContext } from 'twenty-shared/testing';
import { checkCanDeactivateFieldOrThrow } from 'src/engine/metadata-modules/field-metadata/utils/check-can-deactivate-field-or-throw';
type CheckCanDeactivateFieldOrThrowTestContext = EachTestingContext<{
input: Parameters<typeof checkCanDeactivateFieldOrThrow>[0];
shouldNotThrow?: true;
}>;
const checkCanDeactivateFieldOrThrowTestCases: CheckCanDeactivateFieldOrThrowTestContext[] =
[
{
title: 'does not throw if nominal case',
context: {
input: {
labelIdentifierFieldMetadataId: 'fieldIdentifierId',
existingFieldMetadata: {
id: 'myFieldId',
isSystem: false,
name: 'myFieldName',
},
},
shouldNotThrow: true,
},
},
{
title: 'throws if trying to deactivate label identifier field',
context: {
input: {
labelIdentifierFieldMetadataId: 'fieldId',
existingFieldMetadata: {
id: 'fieldId',
isSystem: false,
name: 'name',
},
},
},
},
{
title: 'throws if trying to deactivate system field',
context: {
input: {
labelIdentifierFieldMetadataId: 'fieldIdentifierId',
existingFieldMetadata: {
id: 'systemFieldId',
isSystem: true,
name: 'systemField',
},
},
},
},
{
title: 'throws if trying to deactivate createdAt field',
context: {
input: {
labelIdentifierFieldMetadataId: 'fieldIdentifierId',
existingFieldMetadata: {
id: 'createdAtId',
isSystem: false,
name: 'createdAt',
},
},
},
},
{
title: 'throws if trying to deactivate updatedAt field',
context: {
input: {
labelIdentifierFieldMetadataId: 'fieldIdentifierId',
existingFieldMetadata: {
id: 'updatedAtId',
isSystem: false,
name: 'updatedAt',
},
},
},
},
{
title: 'throws if trying to deactivate deletedAt field',
context: {
input: {
labelIdentifierFieldMetadataId: 'fieldIdentifierId',
existingFieldMetadata: {
id: 'deletedAtId',
isSystem: false,
name: 'deletedAt',
},
},
},
},
];
describe('checkCanDeactivateFieldOrThrow', () => {
it.each(checkCanDeactivateFieldOrThrowTestCases)(
'$title',
({ context: { input, shouldNotThrow } }) => {
if (shouldNotThrow) {
expect(() => checkCanDeactivateFieldOrThrow(input)).not.toThrow();
} else {
expect(() =>
checkCanDeactivateFieldOrThrow(input),
).toThrowErrorMatchingSnapshot();
}
},
);
});

View File

@ -0,0 +1,42 @@
import { FieldMetadataInterface } from 'src/engine/metadata-modules/field-metadata/interfaces/field-metadata.interface';
import {
FieldMetadataException,
FieldMetadataExceptionCode,
} from 'src/engine/metadata-modules/field-metadata/field-metadata.exception';
type CheckCanDeactivateFieldOptions = {
labelIdentifierFieldMetadataId: string;
existingFieldMetadata: Pick<
FieldMetadataInterface,
'id' | 'isSystem' | 'name'
>;
};
export const checkCanDeactivateFieldOrThrow = ({
labelIdentifierFieldMetadataId,
existingFieldMetadata,
}: CheckCanDeactivateFieldOptions) => {
if (existingFieldMetadata.id === labelIdentifierFieldMetadataId) {
throw new FieldMetadataException(
'Cannot deactivate label identifier field',
FieldMetadataExceptionCode.INVALID_FIELD_INPUT,
);
}
if (existingFieldMetadata.isSystem === true) {
throw new FieldMetadataException(
'Cannot deactivate system field',
FieldMetadataExceptionCode.INVALID_FIELD_INPUT,
);
}
if (
['deletedAt', 'createdAt', 'updatedAt'].includes(existingFieldMetadata.name)
) {
throw new FieldMetadataException(
'Cannot deactivate createdAt, updatedAt or deletedAt field',
FieldMetadataExceptionCode.INVALID_FIELD_INPUT,
);
}
};

View File

@ -30,6 +30,7 @@ export const fieldMetadataGraphqlApiExceptionHandler = (error: Error) => {
case FieldMetadataExceptionCode.INTERNAL_SERVER_ERROR:
case FieldMetadataExceptionCode.FIELD_METADATA_RELATION_NOT_ENABLED:
case FieldMetadataExceptionCode.FIELD_METADATA_RELATION_MALFORMED:
case FieldMetadataExceptionCode.LABEL_IDENTIFIER_FIELD_METADATA_ID_NOT_FOUND:
default:
throw new InternalServerError(error.message);
}

View File

@ -1,4 +1,4 @@
import { getMockCreateObjectInput } from 'test/integration/utils/object-metadata/generate-mock-create-object-metadata-input';
import { getMockCreateObjectInput } from 'test/integration/metadata/suites/object-metadata/utils/generate-mock-create-object-metadata-input';
import { EachTestingContext } from 'twenty-shared/testing';
import { UpdateObjectPayload } from 'src/engine/metadata-modules/object-metadata/dtos/update-object.input';