Unique fields - fixes (#13848)
- Enable update to unique for composite field with defaultValue different from default defaultValue on subfield not included in unique constraint - Enable update to unique for standard field + Disable update to non-unique for standard index - Fix typo Fixes https://github.com/twentyhq/core-team-issues/issues/1360
This commit is contained in:
+110
@@ -12,6 +12,9 @@ import { type UpdateFieldInput } from 'src/engine/metadata-modules/field-metadat
|
||||
import { type FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity';
|
||||
import { BeforeUpdateOneField } from 'src/engine/metadata-modules/field-metadata/hooks/before-update-one-field.hook';
|
||||
import { FieldMetadataService } from 'src/engine/metadata-modules/field-metadata/services/field-metadata.service';
|
||||
import { type IndexMetadataEntity } from 'src/engine/metadata-modules/index-metadata/index-metadata.entity';
|
||||
import { type ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity';
|
||||
import { ObjectMetadataService } from 'src/engine/metadata-modules/object-metadata/object-metadata.service';
|
||||
import { getMockFieldMetadataEntity } from 'src/utils/__test__/get-field-metadata-entity.mock';
|
||||
|
||||
jest.mock('@lingui/core', () => ({
|
||||
@@ -26,6 +29,7 @@ type UpdateFieldInputForTest = Omit<UpdateFieldInput, 'id' | 'workspaceId'>;
|
||||
describe('BeforeUpdateOneField', () => {
|
||||
let hook: BeforeUpdateOneField<UpdateFieldInput>;
|
||||
let fieldMetadataService: FieldMetadataService;
|
||||
let objectMetadataService: ObjectMetadataService;
|
||||
|
||||
const mockWorkspaceId = 'workspace-id';
|
||||
const mockFieldId = 'field-id';
|
||||
@@ -40,6 +44,12 @@ describe('BeforeUpdateOneField', () => {
|
||||
findOneWithinWorkspace: jest.fn(),
|
||||
},
|
||||
},
|
||||
{
|
||||
provide: ObjectMetadataService,
|
||||
useValue: {
|
||||
findOneWithinWorkspace: jest.fn(),
|
||||
},
|
||||
},
|
||||
],
|
||||
}).compile();
|
||||
|
||||
@@ -47,6 +57,9 @@ describe('BeforeUpdateOneField', () => {
|
||||
module.get<BeforeUpdateOneField<UpdateFieldInput>>(BeforeUpdateOneField);
|
||||
fieldMetadataService =
|
||||
module.get<FieldMetadataService>(FieldMetadataService);
|
||||
objectMetadataService = module.get<ObjectMetadataService>(
|
||||
ObjectMetadataService,
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -571,4 +584,101 @@ describe('BeforeUpdateOneField', () => {
|
||||
|
||||
expect(result).toEqual(expectedResult);
|
||||
});
|
||||
|
||||
it('should throw ValidationError if isUnique is updated for a standard field with a standard unique index', async () => {
|
||||
const mockField: Partial<FieldMetadataEntity> = {
|
||||
id: mockFieldId,
|
||||
isCustom: false,
|
||||
isUnique: true,
|
||||
};
|
||||
|
||||
jest
|
||||
.spyOn(fieldMetadataService, 'findOneWithinWorkspace')
|
||||
.mockResolvedValue(mockField as FieldMetadataEntity);
|
||||
|
||||
const mockObjectMetadata: Partial<ObjectMetadataEntity> = {
|
||||
id: mockWorkspaceId,
|
||||
indexMetadatas: [
|
||||
{
|
||||
isUnique: true,
|
||||
isCustom: false,
|
||||
indexFieldMetadatas: [{ fieldMetadataId: mockFieldId }],
|
||||
} as IndexMetadataEntity,
|
||||
],
|
||||
};
|
||||
|
||||
jest
|
||||
.spyOn(objectMetadataService, 'findOneWithinWorkspace')
|
||||
.mockResolvedValue(mockObjectMetadata as ObjectMetadataEntity);
|
||||
|
||||
const instance: UpdateOneInputType<UpdateFieldInputForTest> = {
|
||||
id: mockFieldId,
|
||||
update: {
|
||||
isUnique: false,
|
||||
},
|
||||
};
|
||||
|
||||
await expect(
|
||||
hook.run(instance as UpdateOneInputType<UpdateFieldInput>, {
|
||||
workspaceId: mockWorkspaceId,
|
||||
locale: undefined,
|
||||
}),
|
||||
).rejects.toThrow(ValidationError);
|
||||
});
|
||||
|
||||
it('should not throw ValidationError if isUnique is updated for a standard field without a standard unique index', async () => {
|
||||
const mockField: Partial<FieldMetadataEntity> = {
|
||||
id: mockFieldId,
|
||||
isCustom: false,
|
||||
isUnique: true,
|
||||
};
|
||||
|
||||
jest
|
||||
.spyOn(fieldMetadataService, 'findOneWithinWorkspace')
|
||||
.mockResolvedValue(mockField as FieldMetadataEntity);
|
||||
|
||||
const mockObjectMetadata: Partial<ObjectMetadataEntity> = {
|
||||
id: mockWorkspaceId,
|
||||
indexMetadatas: [
|
||||
{
|
||||
isUnique: false,
|
||||
isCustom: false,
|
||||
indexFieldMetadatas: [{ fieldMetadataId: mockFieldId }],
|
||||
} as IndexMetadataEntity,
|
||||
],
|
||||
};
|
||||
|
||||
jest
|
||||
.spyOn(objectMetadataService, 'findOneWithinWorkspace')
|
||||
.mockResolvedValue(mockObjectMetadata as ObjectMetadataEntity);
|
||||
|
||||
const instance: UpdateOneInputType<UpdateFieldInputForTest> = {
|
||||
id: mockFieldId,
|
||||
update: {
|
||||
isUnique: false,
|
||||
},
|
||||
};
|
||||
|
||||
jest
|
||||
.spyOn(fieldMetadataService, 'findOneWithinWorkspace')
|
||||
.mockResolvedValue(mockField as FieldMetadataEntity);
|
||||
|
||||
const result = await hook.run(
|
||||
instance as UpdateOneInputType<UpdateFieldInput>,
|
||||
{
|
||||
workspaceId: mockWorkspaceId,
|
||||
locale: undefined,
|
||||
},
|
||||
);
|
||||
|
||||
const expectedResult = {
|
||||
id: mockFieldId,
|
||||
update: {
|
||||
isUnique: false,
|
||||
standardOverrides: {},
|
||||
},
|
||||
};
|
||||
|
||||
expect(result).toEqual(expectedResult);
|
||||
});
|
||||
});
|
||||
|
||||
+52
-5
@@ -17,6 +17,7 @@ import { type FieldStandardOverridesDTO } from 'src/engine/metadata-modules/fiel
|
||||
import { type UpdateFieldInput } from 'src/engine/metadata-modules/field-metadata/dtos/update-field.input';
|
||||
import { type FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity';
|
||||
import { FieldMetadataService } from 'src/engine/metadata-modules/field-metadata/services/field-metadata.service';
|
||||
import { ObjectMetadataService } from 'src/engine/metadata-modules/object-metadata/object-metadata.service';
|
||||
|
||||
interface StandardFieldUpdate extends Partial<UpdateFieldInput> {
|
||||
standardOverrides?: FieldStandardOverridesDTO;
|
||||
@@ -26,7 +27,10 @@ interface StandardFieldUpdate extends Partial<UpdateFieldInput> {
|
||||
export class BeforeUpdateOneField<T extends UpdateFieldInput>
|
||||
implements BeforeUpdateOneHook<T>
|
||||
{
|
||||
constructor(readonly fieldMetadataService: FieldMetadataService) {}
|
||||
constructor(
|
||||
readonly fieldMetadataService: FieldMetadataService,
|
||||
readonly objectMetadataService: ObjectMetadataService,
|
||||
) {}
|
||||
|
||||
async run(
|
||||
instance: UpdateOneInputType<T>,
|
||||
@@ -45,7 +49,11 @@ export class BeforeUpdateOneField<T extends UpdateFieldInput>
|
||||
const fieldMetadata = await this.getFieldMetadata(instance, workspaceId);
|
||||
|
||||
if (!fieldMetadata.isCustom) {
|
||||
return this.handleStandardFieldUpdate(instance, fieldMetadata, locale);
|
||||
return await this.handleStandardFieldUpdate(
|
||||
instance,
|
||||
fieldMetadata,
|
||||
locale,
|
||||
);
|
||||
}
|
||||
|
||||
return instance;
|
||||
@@ -69,11 +77,11 @@ export class BeforeUpdateOneField<T extends UpdateFieldInput>
|
||||
return fieldMetadata;
|
||||
}
|
||||
|
||||
private handleStandardFieldUpdate(
|
||||
private async handleStandardFieldUpdate(
|
||||
instance: UpdateOneInputType<T>,
|
||||
fieldMetadata: FieldMetadataEntity,
|
||||
locale?: keyof typeof APP_LOCALES,
|
||||
): UpdateOneInputType<T> {
|
||||
): Promise<UpdateOneInputType<T>> {
|
||||
const update: StandardFieldUpdate = {};
|
||||
const updatableFields = [
|
||||
'isActive',
|
||||
@@ -81,6 +89,7 @@ export class BeforeUpdateOneField<T extends UpdateFieldInput>
|
||||
'options',
|
||||
'settings',
|
||||
'defaultValue',
|
||||
'isUnique',
|
||||
];
|
||||
const overridableFields = ['label', 'icon', 'description'];
|
||||
|
||||
@@ -91,7 +100,7 @@ export class BeforeUpdateOneField<T extends UpdateFieldInput>
|
||||
|
||||
if (nonUpdatableFields.length > 0) {
|
||||
throw new ValidationError(
|
||||
`Only isActive, isLabelSyncedWithName, label, icon, description and defaultValue fields can be updated for standard fields. Invalid fields: ${nonUpdatableFields.join(', ')}`,
|
||||
`Only isActive, isLabelSyncedWithName, label, icon, description, isUnique and defaultValue fields can be updated for standard fields. Invalid fields: ${nonUpdatableFields.join(', ')}`,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -106,6 +115,7 @@ export class BeforeUpdateOneField<T extends UpdateFieldInput>
|
||||
this.handleOptionsField(instance, update);
|
||||
this.handleSettingsField(instance, update);
|
||||
this.handleDefaultValueField(instance, update);
|
||||
await this.handleIsUniqueField(instance, fieldMetadata, update);
|
||||
|
||||
return {
|
||||
id: instance.id,
|
||||
@@ -113,6 +123,43 @@ export class BeforeUpdateOneField<T extends UpdateFieldInput>
|
||||
};
|
||||
}
|
||||
|
||||
private async handleIsUniqueField(
|
||||
instance: UpdateOneInputType<T>,
|
||||
fieldMetadata: FieldMetadataEntity,
|
||||
update: StandardFieldUpdate,
|
||||
) {
|
||||
if (!isDefined(instance.update.isUnique)) {
|
||||
return;
|
||||
}
|
||||
|
||||
const objectMetadata =
|
||||
await this.objectMetadataService.findOneWithinWorkspace(
|
||||
fieldMetadata.workspaceId,
|
||||
{
|
||||
where: {
|
||||
id: fieldMetadata.objectMetadataId,
|
||||
},
|
||||
},
|
||||
);
|
||||
|
||||
const hasStandardUniqueIndex = objectMetadata?.indexMetadatas.some(
|
||||
(index) =>
|
||||
index.isUnique &&
|
||||
!index.isCustom &&
|
||||
index.indexFieldMetadatas?.some(
|
||||
(field) => field.fieldMetadataId === fieldMetadata.id,
|
||||
),
|
||||
);
|
||||
|
||||
if (hasStandardUniqueIndex && instance.update.isUnique === false) {
|
||||
throw new ValidationError(
|
||||
'Unique standard field cannot be updated to non-unique.',
|
||||
);
|
||||
}
|
||||
|
||||
update.isUnique = instance.update.isUnique;
|
||||
}
|
||||
|
||||
private handleDefaultValueField(
|
||||
instance: UpdateOneInputType<T>,
|
||||
update: StandardFieldUpdate,
|
||||
|
||||
+1
@@ -20,6 +20,7 @@ export const buildUpdatableStandardFieldInput = (
|
||||
defaultValue: fieldMetadataInput.defaultValue,
|
||||
settings: fieldMetadataInput.settings,
|
||||
isLabelSyncedWithName: fieldMetadataInput.isLabelSyncedWithName,
|
||||
isUnique: fieldMetadataInput.isUnique,
|
||||
};
|
||||
|
||||
if ('standardOverrides' in fieldMetadataInput) {
|
||||
|
||||
+18
-3
@@ -1,10 +1,10 @@
|
||||
import { isDeepStrictEqual } from 'util';
|
||||
|
||||
import { type FieldMetadataType } from 'twenty-shared/types';
|
||||
|
||||
import { type FieldMetadataDefaultValue } from 'src/engine/metadata-modules/field-metadata/interfaces/field-metadata-default-value.interface';
|
||||
|
||||
import { compositeTypeDefinitions } from 'src/engine/metadata-modules/field-metadata/composite-types';
|
||||
import { generateDefaultValue } from 'src/engine/metadata-modules/field-metadata/utils/generate-default-value';
|
||||
import { isCompositeFieldMetadataType } from 'src/engine/metadata-modules/field-metadata/utils/is-composite-field-metadata-type.util';
|
||||
|
||||
export const isValidUniqueFieldDefaultValueCombination = ({
|
||||
defaultValue,
|
||||
@@ -15,7 +15,22 @@ export const isValidUniqueFieldDefaultValueCombination = ({
|
||||
isUnique: boolean;
|
||||
type: FieldMetadataType;
|
||||
}) => {
|
||||
if (!isUnique) return true;
|
||||
|
||||
const defaultDefaultValue = generateDefaultValue(type);
|
||||
|
||||
return !isUnique || isDeepStrictEqual(defaultValue, defaultDefaultValue);
|
||||
if (!isCompositeFieldMetadataType(type))
|
||||
return defaultValue === defaultDefaultValue;
|
||||
|
||||
const doUniquePropertiesHaveDefaultValues =
|
||||
compositeTypeDefinitions
|
||||
.get(type)
|
||||
?.properties.filter((property) => property.isIncludedInUniqueConstraint)
|
||||
.every(
|
||||
({ name }) =>
|
||||
(defaultValue as Record<string, string | null>)?.[name] ===
|
||||
(defaultDefaultValue as Record<string, string | null>)?.[name],
|
||||
) ?? false;
|
||||
|
||||
return doUniquePropertiesHaveDefaultValues;
|
||||
};
|
||||
|
||||
+5
-1
@@ -560,7 +560,11 @@ export class ObjectMetadataService extends TypeOrmQueryService<ObjectMetadataEnt
|
||||
options: FindOneOptions<ObjectMetadataEntity>,
|
||||
): Promise<ObjectMetadataEntity | null> {
|
||||
return this.objectMetadataRepository.findOne({
|
||||
relations: ['fields'],
|
||||
relations: [
|
||||
'fields',
|
||||
'indexMetadatas',
|
||||
'indexMetadatas.indexFieldMetadatas',
|
||||
],
|
||||
...options,
|
||||
where: {
|
||||
...options.where,
|
||||
|
||||
Reference in New Issue
Block a user