Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] Support non latin characters in schema names #5063

Merged
merged 13 commits into from
Apr 23, 2024
Merged
3 changes: 3 additions & 0 deletions packages/twenty-front/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,8 @@
},
"msw": {
"workerDirectory": "public"
},
"dependencies": {
"transliteration": "^2.3.5"
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import toCamelCase from 'lodash.camelcase';
import toSnakeCase from 'lodash.snakecase';

import { Field, FieldMetadataType } from '~/generated-metadata/graphql';
import { formatMetadataLabelToMetadataNameOrThrows } from '~/pages/settings/data-model/utils/format-metadata-label-to-name.util';
import { isDefined } from '~/utils/isDefined';

import { FieldMetadataOption } from '../types/FieldMetadataOption';
Expand Down Expand Up @@ -64,7 +64,7 @@ export const formatFieldMetadataItemInput = (
description: input.description?.trim() ?? null,
icon: input.icon,
label: input.label.trim(),
name: toCamelCase(input.label.trim()),
name: formatMetadataLabelToMetadataNameOrThrows(input.label.trim()),
options: options?.map((option, index) => ({
color: option.color,
id: option.id,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const metadataLabelValidationPattern = /^[a-zA-Z][a-zA-Z0-9 ]*$/;
const metadataLabelValidationPattern = /^[^0-9].*$/;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ijreilly with this regex we don't exclude special characters which will also break in the BE for field creations

export const validateMetadataLabel = (value: string) =>
!!value.match(metadataLabelValidationPattern);
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import styled from '@emotion/styled';
import { z } from 'zod';

import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem';
import { validateMetadataLabel } from '@/object-metadata/utils/validateMetadataLabel';
import { objectMetadataItemSchema } from '@/object-metadata/validation-schemas/objectMetadataItemSchema';
import { IconPicker } from '@/ui/input/components/IconPicker';
import { TextArea } from '@/ui/input/components/TextArea';
Expand Down Expand Up @@ -92,7 +93,11 @@ export const SettingsDataModelObjectAboutForm = ({
label={label}
placeholder={placeholder}
value={value}
onChange={onChange}
onChange={(value) => {
if (!value || validateMetadataLabel(value)) {
onChange?.(value);
}
}}
disabled={disabled}
fullWidth
/>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import camelCase from 'lodash.camelcase';

import { objectMetadataItemSchema } from '@/object-metadata/validation-schemas/objectMetadataItemSchema';
import { CreateObjectInput } from '~/generated-metadata/graphql';
import { formatMetadataLabelToMetadataNameOrThrows } from '~/pages/settings/data-model/utils/format-metadata-label-to-name.util';

export const settingsCreateObjectInputSchema = objectMetadataItemSchema
.pick({
Expand All @@ -12,6 +11,8 @@ export const settingsCreateObjectInputSchema = objectMetadataItemSchema
})
.transform<CreateObjectInput>((value) => ({
...value,
nameSingular: camelCase(value.labelSingular),
namePlural: camelCase(value.labelPlural),
nameSingular: formatMetadataLabelToMetadataNameOrThrows(
value.labelSingular,
),
namePlural: formatMetadataLabelToMetadataNameOrThrows(value.labelPlural),
}));
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import camelCase from 'lodash.camelcase';

import { objectMetadataItemSchema } from '@/object-metadata/validation-schemas/objectMetadataItemSchema';
import { UpdateObjectInput } from '~/generated-metadata/graphql';
import { formatMetadataLabelToMetadataNameOrThrows } from '~/pages/settings/data-model/utils/format-metadata-label-to-name.util';

export const settingsUpdateObjectInputSchema = objectMetadataItemSchema
.pick({
Expand All @@ -17,7 +16,9 @@ export const settingsUpdateObjectInputSchema = objectMetadataItemSchema
.transform<UpdateObjectInput>((value) => ({
...value,
nameSingular: value.labelSingular
? camelCase(value.labelSingular)
? formatMetadataLabelToMetadataNameOrThrows(value.labelSingular)
: undefined,
namePlural: value.labelPlural
? formatMetadataLabelToMetadataNameOrThrows(value.labelPlural)
: undefined,
namePlural: value.labelPlural ? camelCase(value.labelPlural) : undefined,
}));
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { formatMetadataLabelToMetadataNameOrThrows } from '~/pages/settings/data-model/utils/format-metadata-label-to-name.util';

const VALID_STRING_PATTERN = /^[a-zA-Z][a-zA-Z0-9 ]*$/;

describe('formatMetadataLabelToMetadataNameOrThrows', () => {
it('leaves strings unchanged if only latin characters', () => {
const input = 'testName';

expect(
formatMetadataLabelToMetadataNameOrThrows(input).match(
VALID_STRING_PATTERN,
)?.length,
).toBe(1);
expect(formatMetadataLabelToMetadataNameOrThrows(input)).toEqual(input);
});

it('leaves strings unchanged if only latin characters and digits', () => {
const input = 'testName123';

expect(
formatMetadataLabelToMetadataNameOrThrows(input).match(
VALID_STRING_PATTERN,
)?.length,
).toBe(1);
expect(formatMetadataLabelToMetadataNameOrThrows(input)).toEqual(input);
});

it('format strings with non latin characters', () => {
const input = 'בְרִבְרִ';
const expected = 'bRibRi';

expect(
formatMetadataLabelToMetadataNameOrThrows(input).match(
VALID_STRING_PATTERN,
)?.length,
).toBe(1);
expect(formatMetadataLabelToMetadataNameOrThrows(input)).toEqual(expected);
});

it('format strings with mixed characters', () => {
const input = 'aa2בְרִבְרִ';
const expected = 'aa2BRibRi';

expect(
formatMetadataLabelToMetadataNameOrThrows(input).match(
VALID_STRING_PATTERN,
)?.length,
).toBe(1);
expect(formatMetadataLabelToMetadataNameOrThrows(input)).toEqual(expected);
});

it('throws error if could not format', () => {
const input = '$$$***';

expect(() => formatMetadataLabelToMetadataNameOrThrows(input)).toThrow();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import toCamelCase from 'lodash.camelcase';
import { slugify, transliterate } from 'transliteration';

import { isDefined } from '~/utils/isDefined';

const VALID_STRING_PATTERN = /^[a-zA-Z][a-zA-Z0-9 ]*$/;

export const formatMetadataLabelToMetadataNameOrThrows = (
string: string,
): string => {
let formattedString = string;

if (isDefined(formattedString.match(VALID_STRING_PATTERN))) {
return toCamelCase(formattedString);
}

formattedString = toCamelCase(
slugify(transliterate(formattedString, { trim: true })),
);

if (!formattedString.match(VALID_STRING_PATTERN)) {
throw new Error(`"${string}" is not a valid name`);
}

return formattedString;
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { BadRequestException } from '@nestjs/common';

export class InvalidStringException extends BadRequestException {
constructor(string: string) {
const message = `String "${string}" is not valid`;

super(message);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ import {
import { DeleteOneFieldInput } from 'src/engine/metadata-modules/field-metadata/dtos/delete-field.input';
import { computeColumnName } from 'src/engine/metadata-modules/field-metadata/utils/compute-column-name.util';
import { assertMutationNotOnRemoteObject } from 'src/engine/metadata-modules/object-metadata/utils/assert-mutation-not-on-remote-object.util';
import { validateString } from 'src/engine/metadata-modules/remote-server/utils/validate-remote-server-input';
import { InvalidStringException } from 'src/engine/metadata-modules/errors/InvalidStringException';

import {
FieldMetadataEntity,
Expand Down Expand Up @@ -114,6 +116,8 @@ export class FieldMetadataService extends TypeOrmQueryService<FieldMetadataEntit
fieldMetadataInput.options = generateRatingOptions();
}

this.validateFieldMetadataInput<CreateFieldInput>(fieldMetadataInput);

const fieldAlreadyExists = await fieldMetadataRepository.findOne({
where: {
name: fieldMetadataInput.name,
Expand Down Expand Up @@ -293,6 +297,8 @@ export class FieldMetadataService extends TypeOrmQueryService<FieldMetadataEntit
}
}

this.validateFieldMetadataInput<UpdateFieldInput>(fieldMetadataInput);

const updatableFieldInput =
existingFieldMetadata.isCustom === false
? this.buildUpdatableStandardFieldInput(
Expand Down Expand Up @@ -533,4 +539,24 @@ export class FieldMetadataService extends TypeOrmQueryService<FieldMetadataEntit
};
}
}

private validateFieldMetadataInput<
T extends UpdateFieldInput | CreateFieldInput,
>(fieldMetadataInput: T): T {
if (fieldMetadataInput.name) {
try {
validateString(fieldMetadataInput.name);
} catch (error) {
if (error instanceof InvalidStringException) {
throw new BadRequestException(
`Characters used in name "${fieldMetadataInput.name}" are not supported`,
);
} else {
throw error;
}
}
}

return fieldMetadataInput;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ import {
FeatureFlagKeys,
} from 'src/engine/core-modules/feature-flag/feature-flag.entity';
import { DataSourceEntity } from 'src/engine/metadata-modules/data-source/data-source.entity';
import { validateObjectMetadataInput } from 'src/engine/metadata-modules/object-metadata/utils/validate-object-metadata-input.util';

import { ObjectMetadataEntity } from './object-metadata.entity';

Expand Down Expand Up @@ -237,6 +238,8 @@ export class ObjectMetadataService extends TypeOrmQueryService<ObjectMetadataEnt
objectMetadataInput.workspaceId,
);

validateObjectMetadataInput(objectMetadataInput);

if (
objectMetadataInput.nameSingular.toLowerCase() ===
objectMetadataInput.namePlural.toLowerCase()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { BadRequestException } from '@nestjs/common';

import { InvalidStringException } from 'src/engine/metadata-modules/errors/InvalidStringException';
import { CreateObjectInput } from 'src/engine/metadata-modules/object-metadata/dtos/create-object.input';
import { UpdateObjectInput } from 'src/engine/metadata-modules/object-metadata/dtos/update-object.input';
import { validateMetadataName } from 'src/engine/metadata-modules/utils/validate-metadata-name.utils';

export const validateObjectMetadataInput = <
T extends UpdateObjectInput | CreateObjectInput,
>(
objectMetadataInput: T,
): void => {
try {
if (objectMetadataInput.nameSingular) {
validateMetadataName(objectMetadataInput.nameSingular);
}

if (objectMetadataInput.namePlural) {
validateMetadataName(objectMetadataInput.namePlural);
}
} catch (error) {
if (error instanceof InvalidStringException) {
console.error(error.message);
throw new BadRequestException(
`Characters used in name "${objectMetadataInput.nameSingular}" or "${objectMetadataInput.namePlural}" are not supported`,
);
} else {
throw error;
}
}
};
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import {
import { ObjectMetadataEntity } from 'src/engine/metadata-modules/object-metadata/object-metadata.entity';
import { computeObjectTargetTable } from 'src/engine/utils/compute-object-target-table.util';
import { generateMigrationName } from 'src/engine/metadata-modules/workspace-migration/utils/generate-migration-name.util';
import { InvalidStringException } from 'src/engine/metadata-modules/errors/InvalidStringException';
import { validateMetadataName } from 'src/engine/metadata-modules/utils/validate-metadata-name.utils';

import {
RelationMetadataEntity,
Expand Down Expand Up @@ -51,6 +53,20 @@ export class RelationMetadataService extends TypeOrmQueryService<RelationMetadat
relationMetadataInput,
);

try {
validateMetadataName(relationMetadataInput.fromName);
validateMetadataName(relationMetadataInput.toName);
} catch (error) {
if (error instanceof InvalidStringException) {
console.error(error.message);
throw new BadRequestException(
`Characters used in name "${relationMetadataInput.fromName}" or "${relationMetadataInput.toName}" are not supported`,
);
} else {
throw error;
}
}

await this.validateCreateRelationMetadataInput(
relationMetadataInput,
objectMetadataMap,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import { InvalidStringException } from 'src/engine/metadata-modules/errors/InvalidStringException';
import { validateMetadataName } from 'src/engine/metadata-modules/utils/validate-metadata-name.utils';

describe('validateMetadataName', () => {
it('does not throw if string is valid', () => {
const input = 'testName';

expect(validateMetadataName(input)).not.toThrow;
});

it('throws error if string has non latin characters', () => {
const input = 'בְרִבְרִ';

expect(() => validateMetadataName(input)).toThrow(InvalidStringException);
});

it('throws error if starts with digits', () => {
const input = '123string';

expect(() => validateMetadataName(input)).toThrow(InvalidStringException);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { InvalidStringException } from 'src/engine/metadata-modules/errors/InvalidStringException';

const VALID_STRING_PATTERN = /^[a-zA-Z][a-zA-Z0-9 ]*$/;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the pattern here is not the same as the FE pattern


export const validateMetadataName = (string: string) => {
if (!string.match(VALID_STRING_PATTERN)) {
throw new InvalidStringException(string);
}
};
16 changes: 15 additions & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -45804,6 +45804,18 @@ __metadata:
languageName: node
linkType: hard

"transliteration@npm:^2.3.5":
version: 2.3.5
resolution: "transliteration@npm:2.3.5"
dependencies:
yargs: "npm:^17.5.1"
bin:
slugify: dist/bin/slugify
transliterate: dist/bin/transliterate
checksum: 68397225c2ca59b8e33206c65f905724e86b64460cbf90576d352dc2366e763ded97e2c7b8b1f140fb36a565d61a97c51080df9fa638e6b1769f6cb24f383756
languageName: node
linkType: hard

"traverse@npm:0.6.7":
version: 0.6.7
resolution: "traverse@npm:0.6.7"
Expand Down Expand Up @@ -46264,6 +46276,8 @@ __metadata:
"twenty-front@workspace:packages/twenty-front":
version: 0.0.0-use.local
resolution: "twenty-front@workspace:packages/twenty-front"
dependencies:
transliteration: "npm:^2.3.5"
languageName: unknown
linkType: soft

Expand Down Expand Up @@ -49452,7 +49466,7 @@ __metadata:
languageName: node
linkType: hard

"yargs@npm:^17.0.0, yargs@npm:^17.3.1, yargs@npm:^17.6.2, yargs@npm:^17.7.2":
"yargs@npm:^17.0.0, yargs@npm:^17.3.1, yargs@npm:^17.5.1, yargs@npm:^17.6.2, yargs@npm:^17.7.2":
version: 17.7.2
resolution: "yargs@npm:17.7.2"
dependencies:
Expand Down
Loading