Skip to content

Commit

Permalink
Fix field creation (twentyhq#7547)
Browse files Browse the repository at this point in the history
In [this](twentyhq#7522) and
[this](twentyhq#7543) PR we introduced
the impossibility to save a field that would be non nullable but without
a default value.
The check is actually called on the input while the defaultValue is
added by the service on a "built" fieldMetadata to create or save. So
far all fields created from the app it currently fails as both
isNullable and defaultValue are undefined so falsy at that stage.
  • Loading branch information
ijreilly authored and harshit078 committed Oct 14, 2024
1 parent 7b0b921 commit 4058b63
Showing 1 changed file with 29 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -159,14 +159,7 @@ export class FieldMetadataService extends TypeOrmQueryService<FieldMetadataEntit
);
}

this.validateFieldMetadataInput<CreateFieldInput>(
fieldMetadataInput.type,
fieldMetadataInput,
objectMetadata,
);

console.time('createOne save');
const createdFieldMetadata = await fieldMetadataRepository.save({
const fieldMetadataForCreate = {
id: v4(),
createdAt: new Date(),
updatedAt: new Date(),
Expand All @@ -187,7 +180,18 @@ export class FieldMetadataService extends TypeOrmQueryService<FieldMetadataEntit
: undefined,
isActive: true,
isCustom: true,
});
};

this.validateFieldMetadata<CreateFieldInput>(
fieldMetadataForCreate.type,
fieldMetadataForCreate,
objectMetadata,
);

console.time('createOne save');
const createdFieldMetadata = await fieldMetadataRepository.save(
fieldMetadataForCreate,
);

console.timeEnd('createOne save');

Expand Down Expand Up @@ -394,12 +398,6 @@ export class FieldMetadataService extends TypeOrmQueryService<FieldMetadataEntit
}
}

this.validateFieldMetadataInput<UpdateFieldInput>(
existingFieldMetadata.type,
fieldMetadataInput,
objectMetadata,
);

const updatableFieldInput =
existingFieldMetadata.isCustom === false
? this.buildUpdatableStandardFieldInput(
Expand All @@ -408,21 +406,21 @@ export class FieldMetadataService extends TypeOrmQueryService<FieldMetadataEntit
)
: fieldMetadataInput;

// We're running field update under a transaction, so we can rollback if migration fails
await fieldMetadataRepository.update(id, {
const fieldMetadataForUpdate = {
...updatableFieldInput,
defaultValue:
// Todo: we handle default value for all field types.
![
FieldMetadataType.SELECT,
FieldMetadataType.MULTI_SELECT,
FieldMetadataType.BOOLEAN,
].includes(existingFieldMetadata.type)
? existingFieldMetadata.defaultValue
: updatableFieldInput.defaultValue !== null
? updatableFieldInput.defaultValue
: null,
});
defaultValue: isDefined(updatableFieldInput.defaultValue)
? updatableFieldInput.defaultValue
: existingFieldMetadata.defaultValue,
};

this.validateFieldMetadata<UpdateFieldInput>(
existingFieldMetadata.type,
fieldMetadataForUpdate,
objectMetadata,
);

// We're running field update under a transaction, so we can rollback if migration fails
await fieldMetadataRepository.update(id, fieldMetadataForUpdate);

const updatedFieldMetadata = await fieldMetadataRepository.findOne({
where: { id },
Expand Down Expand Up @@ -710,9 +708,7 @@ export class FieldMetadataService extends TypeOrmQueryService<FieldMetadataEntit
}
}

private validateFieldMetadataInput<
T extends UpdateFieldInput | CreateFieldInput,
>(
private validateFieldMetadata<T extends UpdateFieldInput | CreateFieldInput>(
fieldMetadataType: FieldMetadataType,
fieldMetadataInput: T,
objectMetadata: ObjectMetadataEntity,
Expand Down Expand Up @@ -746,7 +742,7 @@ export class FieldMetadataService extends TypeOrmQueryService<FieldMetadataEntit
}
}

if (!fieldMetadataInput.isNullable) {
if (fieldMetadataInput.isNullable === false) {
if (!isDefined(fieldMetadataInput.defaultValue)) {
throw new FieldMetadataException(
'Default value is required for non nullable fields',
Expand Down

0 comments on commit 4058b63

Please sign in to comment.