Skip to content

Commit

Permalink
[fix] Support non latin characters in schema names (#5063)
Browse files Browse the repository at this point in the history
Fixes #4943

## How was it tested?
Local (front + /metadata)
Unit tests for utils

---------

Co-authored-by: Weiko <[email protected]>
  • Loading branch information
ijreilly and Weiko authored Apr 23, 2024
1 parent 824786f commit ff39ba5
Show file tree
Hide file tree
Showing 16 changed files with 236 additions and 13 deletions.
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].*$/;

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 ]*$/;

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

0 comments on commit ff39ba5

Please sign in to comment.