Fix select default value not in options (#11622)
Also fixing a bunch of places where validation exceptions were not properly handled
This commit is contained in:
@ -11,6 +11,8 @@ import {
|
|||||||
} from 'class-validator';
|
} from 'class-validator';
|
||||||
import { FieldMetadataType } from 'twenty-shared/types';
|
import { FieldMetadataType } from 'twenty-shared/types';
|
||||||
|
|
||||||
|
import { FieldMetadataDefaultValue } from 'src/engine/metadata-modules/field-metadata/interfaces/field-metadata-default-value.interface';
|
||||||
|
import { FieldMetadataOptions } from 'src/engine/metadata-modules/field-metadata/interfaces/field-metadata-options.interface';
|
||||||
import { FieldMetadataSettings } from 'src/engine/metadata-modules/field-metadata/interfaces/field-metadata-settings.interface';
|
import { FieldMetadataSettings } from 'src/engine/metadata-modules/field-metadata/interfaces/field-metadata-settings.interface';
|
||||||
|
|
||||||
import {
|
import {
|
||||||
@ -86,4 +88,47 @@ export class FieldMetadataValidationService<
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async validateDefaultValueOrThrow({
|
||||||
|
fieldType,
|
||||||
|
defaultValue,
|
||||||
|
options,
|
||||||
|
}: {
|
||||||
|
fieldType: FieldMetadataType;
|
||||||
|
defaultValue: FieldMetadataDefaultValue<T>;
|
||||||
|
options: FieldMetadataOptions<T>;
|
||||||
|
}) {
|
||||||
|
if (
|
||||||
|
fieldType === FieldMetadataType.SELECT ||
|
||||||
|
fieldType === FieldMetadataType.MULTI_SELECT
|
||||||
|
) {
|
||||||
|
this.validateEnumDefaultValue(options, defaultValue);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private validateEnumDefaultValue(
|
||||||
|
options: FieldMetadataOptions<T>,
|
||||||
|
defaultValue: FieldMetadataDefaultValue<T>,
|
||||||
|
) {
|
||||||
|
if (typeof defaultValue === 'string') {
|
||||||
|
const formattedDefaultValue = defaultValue.replace(
|
||||||
|
/^['"](.*)['"]$/,
|
||||||
|
'$1',
|
||||||
|
);
|
||||||
|
|
||||||
|
const enumOptions = options.map((option) => option.value);
|
||||||
|
|
||||||
|
if (
|
||||||
|
enumOptions &&
|
||||||
|
(enumOptions.includes(formattedDefaultValue) ||
|
||||||
|
enumOptions.some((option) => option.to === formattedDefaultValue))
|
||||||
|
) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
throw new FieldMetadataException(
|
||||||
|
`Default value for existing options is invalid: ${defaultValue}`,
|
||||||
|
FieldMetadataExceptionCode.INVALID_FIELD_INPUT,
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@ -1,4 +1,4 @@
|
|||||||
import { UnauthorizedException, UseFilters, UseGuards } from '@nestjs/common';
|
import { UseFilters, UseGuards } from '@nestjs/common';
|
||||||
import {
|
import {
|
||||||
Args,
|
Args,
|
||||||
Context,
|
Context,
|
||||||
@ -12,7 +12,10 @@ import { FieldMetadataType } from 'twenty-shared/types';
|
|||||||
|
|
||||||
import { FeatureFlagKey } from 'src/engine/core-modules/feature-flag/enums/feature-flag-key.enum';
|
import { FeatureFlagKey } from 'src/engine/core-modules/feature-flag/enums/feature-flag-key.enum';
|
||||||
import { FeatureFlagService } from 'src/engine/core-modules/feature-flag/services/feature-flag.service';
|
import { FeatureFlagService } from 'src/engine/core-modules/feature-flag/services/feature-flag.service';
|
||||||
import { ValidationError } from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
|
import {
|
||||||
|
ForbiddenError,
|
||||||
|
ValidationError,
|
||||||
|
} from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
|
||||||
import { I18nContext } from 'src/engine/core-modules/i18n/types/i18n-context.type';
|
import { I18nContext } from 'src/engine/core-modules/i18n/types/i18n-context.type';
|
||||||
import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity';
|
import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity';
|
||||||
import { IDataloaders } from 'src/engine/dataloaders/dataloader.interface';
|
import { IDataloaders } from 'src/engine/dataloaders/dataloader.interface';
|
||||||
@ -132,7 +135,7 @@ export class FieldMetadataResolver {
|
|||||||
@AuthWorkspace() { id: workspaceId }: Workspace,
|
@AuthWorkspace() { id: workspaceId }: Workspace,
|
||||||
) {
|
) {
|
||||||
if (!workspaceId) {
|
if (!workspaceId) {
|
||||||
throw new UnauthorizedException();
|
throw new ForbiddenError('Could not retrieve workspace ID');
|
||||||
}
|
}
|
||||||
|
|
||||||
const fieldMetadata =
|
const fieldMetadata =
|
||||||
|
|||||||
@ -213,7 +213,7 @@ export class FieldMetadataService extends TypeOrmQueryService<FieldMetadataEntit
|
|||||||
: existingFieldMetadata.defaultValue,
|
: existingFieldMetadata.defaultValue,
|
||||||
};
|
};
|
||||||
|
|
||||||
this.validateFieldMetadata<UpdateFieldInput>(
|
await this.validateFieldMetadata<UpdateFieldInput>(
|
||||||
existingFieldMetadata.type,
|
existingFieldMetadata.type,
|
||||||
fieldMetadataForUpdate,
|
fieldMetadataForUpdate,
|
||||||
objectMetadata,
|
objectMetadata,
|
||||||
@ -545,11 +545,13 @@ export class FieldMetadataService extends TypeOrmQueryService<FieldMetadataEntit
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
private validateFieldMetadata<T extends UpdateFieldInput | CreateFieldInput>(
|
private async validateFieldMetadata<
|
||||||
|
T extends UpdateFieldInput | CreateFieldInput,
|
||||||
|
>(
|
||||||
fieldMetadataType: FieldMetadataType,
|
fieldMetadataType: FieldMetadataType,
|
||||||
fieldMetadataInput: T,
|
fieldMetadataInput: T,
|
||||||
objectMetadata: ObjectMetadataEntity,
|
objectMetadata: ObjectMetadataEntity,
|
||||||
): T {
|
): Promise<T> {
|
||||||
if (fieldMetadataInput.name) {
|
if (fieldMetadataInput.name) {
|
||||||
try {
|
try {
|
||||||
validateMetadataNameOrThrow(fieldMetadataInput.name);
|
validateMetadataNameOrThrow(fieldMetadataInput.name);
|
||||||
@ -599,10 +601,17 @@ export class FieldMetadataService extends TypeOrmQueryService<FieldMetadataEntit
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
if (isDefined(fieldMetadataInput.defaultValue)) {
|
||||||
|
await this.fieldMetadataValidationService.validateDefaultValueOrThrow({
|
||||||
|
fieldType: fieldMetadataType,
|
||||||
|
options: fieldMetadataInput.options,
|
||||||
|
defaultValue: fieldMetadataInput.defaultValue ?? null,
|
||||||
|
});
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if (fieldMetadataInput.settings) {
|
if (fieldMetadataInput.settings) {
|
||||||
this.fieldMetadataValidationService.validateSettingsOrThrow({
|
await this.fieldMetadataValidationService.validateSettingsOrThrow({
|
||||||
fieldType: fieldMetadataType,
|
fieldType: fieldMetadataType,
|
||||||
settings: fieldMetadataInput.settings,
|
settings: fieldMetadataInput.settings,
|
||||||
});
|
});
|
||||||
@ -717,7 +726,7 @@ export class FieldMetadataService extends TypeOrmQueryService<FieldMetadataEntit
|
|||||||
const fieldMetadataForCreate =
|
const fieldMetadataForCreate =
|
||||||
this.prepareCustomFieldMetadata(fieldMetadataInput);
|
this.prepareCustomFieldMetadata(fieldMetadataInput);
|
||||||
|
|
||||||
this.validateFieldMetadata<CreateFieldInput>(
|
await this.validateFieldMetadata<CreateFieldInput>(
|
||||||
fieldMetadataForCreate.type,
|
fieldMetadataForCreate.type,
|
||||||
fieldMetadataForCreate,
|
fieldMetadataForCreate,
|
||||||
objectMetadata,
|
objectMetadata,
|
||||||
|
|||||||
@ -1,9 +1,12 @@
|
|||||||
import { BadRequestException, UnauthorizedException } from '@nestjs/common';
|
|
||||||
import { Test, TestingModule } from '@nestjs/testing';
|
import { Test, TestingModule } from '@nestjs/testing';
|
||||||
|
|
||||||
import { i18n } from '@lingui/core';
|
import { i18n } from '@lingui/core';
|
||||||
import { UpdateOneInputType } from '@ptc-org/nestjs-query-graphql';
|
import { UpdateOneInputType } from '@ptc-org/nestjs-query-graphql';
|
||||||
|
|
||||||
|
import {
|
||||||
|
ForbiddenError,
|
||||||
|
ValidationError,
|
||||||
|
} from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
|
||||||
import { UpdateFieldInput } from 'src/engine/metadata-modules/field-metadata/dtos/update-field.input';
|
import { UpdateFieldInput } from 'src/engine/metadata-modules/field-metadata/dtos/update-field.input';
|
||||||
import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity';
|
import { FieldMetadataEntity } from 'src/engine/metadata-modules/field-metadata/field-metadata.entity';
|
||||||
import { FieldMetadataService } from 'src/engine/metadata-modules/field-metadata/field-metadata.service';
|
import { FieldMetadataService } from 'src/engine/metadata-modules/field-metadata/field-metadata.service';
|
||||||
@ -48,7 +51,7 @@ describe('BeforeUpdateOneField', () => {
|
|||||||
jest.clearAllMocks();
|
jest.clearAllMocks();
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should throw UnauthorizedException if workspaceId is not provided', async () => {
|
it('should throw ForbiddenError if workspaceId is not provided', async () => {
|
||||||
const instance: UpdateOneInputType<UpdateFieldInputForTest> = {
|
const instance: UpdateOneInputType<UpdateFieldInputForTest> = {
|
||||||
id: mockFieldId,
|
id: mockFieldId,
|
||||||
update: {
|
update: {
|
||||||
@ -61,10 +64,10 @@ describe('BeforeUpdateOneField', () => {
|
|||||||
workspaceId: '',
|
workspaceId: '',
|
||||||
locale: undefined,
|
locale: undefined,
|
||||||
}),
|
}),
|
||||||
).rejects.toThrow(UnauthorizedException);
|
).rejects.toThrow(ForbiddenError);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should throw BadRequestException if field does not exist', async () => {
|
it('should throw ValidationError if field does not exist', async () => {
|
||||||
const instance: UpdateOneInputType<UpdateFieldInputForTest> = {
|
const instance: UpdateOneInputType<UpdateFieldInputForTest> = {
|
||||||
id: mockFieldId,
|
id: mockFieldId,
|
||||||
update: {
|
update: {
|
||||||
@ -81,7 +84,7 @@ describe('BeforeUpdateOneField', () => {
|
|||||||
workspaceId: mockWorkspaceId,
|
workspaceId: mockWorkspaceId,
|
||||||
locale: undefined,
|
locale: undefined,
|
||||||
}),
|
}),
|
||||||
).rejects.toThrow(BadRequestException);
|
).rejects.toThrow(ValidationError);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should not affect custom fields', async () => {
|
it('should not affect custom fields', async () => {
|
||||||
@ -113,7 +116,7 @@ describe('BeforeUpdateOneField', () => {
|
|||||||
expect(result).toEqual(instance);
|
expect(result).toEqual(instance);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should throw BadRequestException when trying to update non-updatable fields on standard fields', async () => {
|
it('should throw ValidationError when trying to update non-updatable fields on standard fields', async () => {
|
||||||
const instance: UpdateOneInputType<UpdateFieldInputForTest> = {
|
const instance: UpdateOneInputType<UpdateFieldInputForTest> = {
|
||||||
id: mockFieldId,
|
id: mockFieldId,
|
||||||
update: {
|
update: {
|
||||||
@ -135,10 +138,10 @@ describe('BeforeUpdateOneField', () => {
|
|||||||
workspaceId: mockWorkspaceId,
|
workspaceId: mockWorkspaceId,
|
||||||
locale: undefined,
|
locale: undefined,
|
||||||
}),
|
}),
|
||||||
).rejects.toThrow(BadRequestException);
|
).rejects.toThrow(ValidationError);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should throw BadRequestException when trying to update label when it is synced with name', async () => {
|
it('should throw ValidationError when trying to update label when it is synced with name', async () => {
|
||||||
const instance: UpdateOneInputType<UpdateFieldInputForTest> = {
|
const instance: UpdateOneInputType<UpdateFieldInputForTest> = {
|
||||||
id: mockFieldId,
|
id: mockFieldId,
|
||||||
update: {
|
update: {
|
||||||
@ -162,7 +165,7 @@ describe('BeforeUpdateOneField', () => {
|
|||||||
workspaceId: mockWorkspaceId,
|
workspaceId: mockWorkspaceId,
|
||||||
locale: undefined,
|
locale: undefined,
|
||||||
}),
|
}),
|
||||||
).rejects.toThrow(BadRequestException);
|
).rejects.toThrow(ValidationError);
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should handle isActive updates for standard fields', async () => {
|
it('should handle isActive updates for standard fields', async () => {
|
||||||
|
|||||||
@ -1,8 +1,4 @@
|
|||||||
import {
|
import { Injectable } from '@nestjs/common';
|
||||||
BadRequestException,
|
|
||||||
Injectable,
|
|
||||||
UnauthorizedException,
|
|
||||||
} from '@nestjs/common';
|
|
||||||
|
|
||||||
import { i18n } from '@lingui/core';
|
import { i18n } from '@lingui/core';
|
||||||
import {
|
import {
|
||||||
@ -12,6 +8,10 @@ import {
|
|||||||
import { APP_LOCALES, SOURCE_LOCALE } from 'twenty-shared/translations';
|
import { APP_LOCALES, SOURCE_LOCALE } from 'twenty-shared/translations';
|
||||||
import { isDefined } from 'twenty-shared/utils';
|
import { isDefined } from 'twenty-shared/utils';
|
||||||
|
|
||||||
|
import {
|
||||||
|
ForbiddenError,
|
||||||
|
ValidationError,
|
||||||
|
} from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
|
||||||
import { generateMessageId } from 'src/engine/core-modules/i18n/utils/generateMessageId';
|
import { generateMessageId } from 'src/engine/core-modules/i18n/utils/generateMessageId';
|
||||||
import { FieldStandardOverridesDTO } from 'src/engine/metadata-modules/field-metadata/dtos/field-standard-overrides.dto';
|
import { FieldStandardOverridesDTO } from 'src/engine/metadata-modules/field-metadata/dtos/field-standard-overrides.dto';
|
||||||
import { UpdateFieldInput } from 'src/engine/metadata-modules/field-metadata/dtos/update-field.input';
|
import { UpdateFieldInput } from 'src/engine/metadata-modules/field-metadata/dtos/update-field.input';
|
||||||
@ -39,7 +39,7 @@ export class BeforeUpdateOneField<T extends UpdateFieldInput>
|
|||||||
},
|
},
|
||||||
): Promise<UpdateOneInputType<T>> {
|
): Promise<UpdateOneInputType<T>> {
|
||||||
if (!workspaceId) {
|
if (!workspaceId) {
|
||||||
throw new UnauthorizedException();
|
throw new ForbiddenError('Could not retrieve workspace ID');
|
||||||
}
|
}
|
||||||
|
|
||||||
const fieldMetadata = await this.getFieldMetadata(instance, workspaceId);
|
const fieldMetadata = await this.getFieldMetadata(instance, workspaceId);
|
||||||
@ -63,7 +63,7 @@ export class BeforeUpdateOneField<T extends UpdateFieldInput>
|
|||||||
});
|
});
|
||||||
|
|
||||||
if (!fieldMetadata) {
|
if (!fieldMetadata) {
|
||||||
throw new BadRequestException('Field does not exist');
|
throw new ValidationError('Field does not exist');
|
||||||
}
|
}
|
||||||
|
|
||||||
return fieldMetadata;
|
return fieldMetadata;
|
||||||
@ -96,13 +96,13 @@ export class BeforeUpdateOneField<T extends UpdateFieldInput>
|
|||||||
instance.update.label !== fieldMetadata.label;
|
instance.update.label !== fieldMetadata.label;
|
||||||
|
|
||||||
if (isUpdatingLabelWhenSynced) {
|
if (isUpdatingLabelWhenSynced) {
|
||||||
throw new BadRequestException(
|
throw new ValidationError(
|
||||||
'Cannot update label when it is synced with name',
|
'Cannot update label when it is synced with name',
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
if (nonUpdatableFields.length > 0) {
|
if (nonUpdatableFields.length > 0) {
|
||||||
throw new BadRequestException(
|
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 and defaultValue fields can be updated for standard fields. Invalid fields: ${nonUpdatableFields.join(', ')}`,
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
@ -116,4 +116,59 @@ describe('updateOne', () => {
|
|||||||
);
|
);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('FieldMetadataService Enum Default Value Validation', () => {
|
||||||
|
it('should throw an error if the default value is not in the options', async () => {
|
||||||
|
const { data: listingObjectMetadata } = await createOneObjectMetadata({
|
||||||
|
input: {
|
||||||
|
labelSingular: LISTING_NAME_SINGULAR,
|
||||||
|
labelPlural: LISTING_NAME_PLURAL,
|
||||||
|
nameSingular: LISTING_NAME_SINGULAR,
|
||||||
|
namePlural: LISTING_NAME_PLURAL,
|
||||||
|
icon: 'IconBuildingSkyscraper',
|
||||||
|
isLabelSyncedWithName: true,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const { data: createdFieldMetadata } = await createOneFieldMetadata({
|
||||||
|
input: {
|
||||||
|
objectMetadataId: listingObjectMetadata.createOneObject.id,
|
||||||
|
type: FieldMetadataType.SELECT,
|
||||||
|
name: 'testName',
|
||||||
|
label: 'Test name',
|
||||||
|
isLabelSyncedWithName: true,
|
||||||
|
options: [
|
||||||
|
{
|
||||||
|
label: 'Option 1',
|
||||||
|
value: 'option1',
|
||||||
|
color: 'green',
|
||||||
|
position: 1,
|
||||||
|
},
|
||||||
|
],
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
const { errors } = await updateOneFieldMetadata({
|
||||||
|
input: {
|
||||||
|
idToUpdate: createdFieldMetadata.createOneField.id,
|
||||||
|
updatePayload: {
|
||||||
|
defaultValue: 'option2',
|
||||||
|
},
|
||||||
|
},
|
||||||
|
gqlFields: `
|
||||||
|
id
|
||||||
|
name
|
||||||
|
label
|
||||||
|
isLabelSyncedWithName
|
||||||
|
`,
|
||||||
|
expectToFail: true,
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(errors[0].message).toBe('Invalid default value "option2"');
|
||||||
|
|
||||||
|
await deleteOneObjectMetadata({
|
||||||
|
input: { idToDelete: listingObjectMetadata.createOneObject.id },
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user