[BUGFIX] ObjectMetadata item server validation (#10699)
# Introduction This PR contains several SNAPSHOT files explaining big + While refactoring the Object Model settings page in https://github.com/twentyhq/twenty/pull/10653, encountered a critical issue when submitting either one or both names with `""` empty string hard corrupting a workspace. This motivate this PR reviewing server side validation I feel like we could share zod schema between front and back ## Refactored server validation What to expect from Names: - Plural and singular have to be different ( case insensitive and trimmed check ) - Contains only a-z A-Z and 0-9 - Follows camelCase - Is not empty => Is not too short ( 1 ) - Is not too long ( 63 ) - Is case insensitive( fooBar and fOoBar now rejected ) What to expect from Labels: - Plural and singular have to be different ( case insensitive and trimmed check ) - Is not empty => Is not too short ( 1 ) - Is not too long ( 63 ) - Is case insensitive ( fooBar and fOoBar now rejected ) close https://github.com/twentyhq/twenty/issues/10694 ## Creation integrations tests Created new integrations tests, following [EachTesting](https://jestjs.io/docs/api#testeachtablename-fn-timeout) pattern and uses snapshot to assert errors message. These tests cover several failing use cases and started to implement ones for the happy path but object metadata item deletion is currently broken unless I'm mistaken @Weiko is on it ## Notes - [ ] As we've added new validation rules towards names and labels we should scan db in order to standardize existing values using either a migration command or manual check - [ ] Will review in an other PR the update path, adding integrations tests and so on
This commit is contained in:
Paul Rastoin
@ -0,0 +1,43 @@
|
||||
// Jest Snapshot v1, https://goo.gl/fbAQLP
|
||||
|
||||
exports[`Object metadata creation should fail when labelPlural contains only whitespace 1`] = `"Input is too short: """`;
|
||||
|
||||
exports[`Object metadata creation should fail when labelPlural exceeds maximum length 1`] = `"String "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" exceeds 63 characters limit"`;
|
||||
|
||||
exports[`Object metadata creation should fail when labelPlural is empty 1`] = `"Input is too short: """`;
|
||||
|
||||
exports[`Object metadata creation should fail when labelSingular contains only whitespace 1`] = `"Input is too short: """`;
|
||||
|
||||
exports[`Object metadata creation should fail when labelSingular exceeds maximum length 1`] = `"String "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" exceeds 63 characters limit"`;
|
||||
|
||||
exports[`Object metadata creation should fail when labelSingular is empty 1`] = `"Input is too short: """`;
|
||||
|
||||
exports[`Object metadata creation should fail when labels are identical 1`] = `"The singular and plural labels cannot be the same for an object"`;
|
||||
|
||||
exports[`Object metadata creation should fail when labels with whitespaces result to be identical 1`] = `"The singular and plural labels cannot be the same for an object"`;
|
||||
|
||||
exports[`Object metadata creation should fail when name exceeds maximum length 1`] = `"String "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" exceeds 63 characters limit"`;
|
||||
|
||||
exports[`Object metadata creation should fail when namePlural has invalid characters 1`] = `"String "μ" is not valid: must start with lowercase letter and contain only alphanumeric letters"`;
|
||||
|
||||
exports[`Object metadata creation should fail when namePlural is a reserved keyword 1`] = `"The name "users" is not available"`;
|
||||
|
||||
exports[`Object metadata creation should fail when namePlural is an empty string 1`] = `"Input is too short: """`;
|
||||
|
||||
exports[`Object metadata creation should fail when namePlural is not camelCased 1`] = `"Name should be in camelCase: Not_Camel_Case"`;
|
||||
|
||||
exports[`Object metadata creation should fail when nameSingular contains only one char and whitespaces 1`] = `"Name should be in camelCase: a a "`;
|
||||
|
||||
exports[`Object metadata creation should fail when nameSingular contains only whitespaces 1`] = `"Name should be in camelCase: "`;
|
||||
|
||||
exports[`Object metadata creation should fail when nameSingular has invalid characters 1`] = `"String "μ" is not valid: must start with lowercase letter and contain only alphanumeric letters"`;
|
||||
|
||||
exports[`Object metadata creation should fail when nameSingular is a reserved keyword 1`] = `"The name "user" is not available"`;
|
||||
|
||||
exports[`Object metadata creation should fail when nameSingular is an empty string 1`] = `"Input is too short: """`;
|
||||
|
||||
exports[`Object metadata creation should fail when nameSingular is not camelCased 1`] = `"Name should be in camelCase: Not_Camel_Case"`;
|
||||
|
||||
exports[`Object metadata creation should fail when names are identical 1`] = `"The singular and plural names cannot be the same for an object"`;
|
||||
|
||||
exports[`Object metadata creation should fail when names with whitespaces result to be identical 1`] = `"Name should be in camelCase: fooBar "`;
|
||||
@ -0,0 +1,137 @@
|
||||
import { getMockCreateObjectInput } from 'test/integration/utils/object-metadata/generate-mock-create-object-metadata-input';
|
||||
import { performFailingObjectMetadataCreation } from 'test/integration/utils/object-metadata/perform-failing-object-metadata-creation';
|
||||
import { EachTestingContext } from 'twenty-shared';
|
||||
|
||||
import { CreateObjectInput } from 'src/engine/metadata-modules/object-metadata/dtos/create-object.input';
|
||||
import { ErrorCode } from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
|
||||
|
||||
type CreateObjectInputPayload = Omit<
|
||||
CreateObjectInput,
|
||||
'workspaceId' | 'dataSourceId'
|
||||
>;
|
||||
|
||||
type CreateOneObjectMetadataItemTestingContext = EachTestingContext<
|
||||
Partial<CreateObjectInputPayload>
|
||||
>[];
|
||||
const failingNamesCreationTestsUseCase: CreateOneObjectMetadataItemTestingContext =
|
||||
[
|
||||
{
|
||||
title: 'when nameSingular has invalid characters',
|
||||
context: { nameSingular: 'μ' },
|
||||
},
|
||||
{
|
||||
title: 'when namePlural has invalid characters',
|
||||
context: { namePlural: 'μ' },
|
||||
},
|
||||
{
|
||||
title: 'when nameSingular is a reserved keyword',
|
||||
context: { nameSingular: 'user' },
|
||||
},
|
||||
{
|
||||
title: 'when namePlural is a reserved keyword',
|
||||
context: { namePlural: 'users' },
|
||||
},
|
||||
{
|
||||
title: 'when nameSingular is not camelCased',
|
||||
context: { nameSingular: 'Not_Camel_Case' },
|
||||
},
|
||||
{
|
||||
title: 'when namePlural is not camelCased',
|
||||
context: { namePlural: 'Not_Camel_Case' },
|
||||
},
|
||||
{
|
||||
title: 'when namePlural is an empty string',
|
||||
context: { namePlural: '' },
|
||||
},
|
||||
{
|
||||
title: 'when nameSingular is an empty string',
|
||||
context: { nameSingular: '' },
|
||||
},
|
||||
{
|
||||
title: 'when nameSingular contains only whitespaces',
|
||||
context: { nameSingular: ' ' },
|
||||
},
|
||||
{
|
||||
title: 'when nameSingular contains only one char and whitespaces',
|
||||
context: { nameSingular: ' a a ' },
|
||||
},
|
||||
{
|
||||
title: 'when name exceeds maximum length',
|
||||
context: { nameSingular: 'a'.repeat(64) },
|
||||
},
|
||||
{
|
||||
title: 'when names are identical',
|
||||
context: {
|
||||
nameSingular: 'fooBar',
|
||||
namePlural: 'fooBar',
|
||||
},
|
||||
},
|
||||
{
|
||||
title: 'when names with whitespaces result to be identical',
|
||||
context: {
|
||||
nameSingular: ' fooBar ',
|
||||
namePlural: 'fooBar',
|
||||
},
|
||||
},
|
||||
];
|
||||
|
||||
const failingLabelsCreationTestsUseCase: CreateOneObjectMetadataItemTestingContext =
|
||||
[
|
||||
{
|
||||
title: 'when labelSingular is empty',
|
||||
context: { labelSingular: '' },
|
||||
},
|
||||
{
|
||||
title: 'when labelPlural is empty',
|
||||
context: { labelPlural: '' },
|
||||
},
|
||||
{
|
||||
title: 'when labelSingular exceeds maximum length',
|
||||
context: { labelSingular: 'A'.repeat(64) },
|
||||
},
|
||||
{
|
||||
title: 'when labelPlural exceeds maximum length',
|
||||
context: { labelPlural: 'A'.repeat(64) },
|
||||
},
|
||||
{
|
||||
title: 'when labelSingular contains only whitespace',
|
||||
context: { labelSingular: ' ' },
|
||||
},
|
||||
{
|
||||
title: 'when labelPlural contains only whitespace',
|
||||
context: { labelPlural: ' ' },
|
||||
},
|
||||
{
|
||||
title: 'when labels are identical',
|
||||
context: {
|
||||
labelPlural: 'fooBar',
|
||||
labelSingular: 'fooBar',
|
||||
},
|
||||
},
|
||||
{
|
||||
title: 'when labels with whitespaces result to be identical',
|
||||
context: {
|
||||
labelPlural: ' fooBar ',
|
||||
labelSingular: 'fooBar',
|
||||
},
|
||||
},
|
||||
];
|
||||
|
||||
const allTestsUseCases = [
|
||||
...failingNamesCreationTestsUseCase,
|
||||
...failingLabelsCreationTestsUseCase,
|
||||
];
|
||||
|
||||
describe('Object metadata creation should fail', () => {
|
||||
it.each(allTestsUseCases)('$title', async ({ context }) => {
|
||||
const errors = await performFailingObjectMetadataCreation(
|
||||
getMockCreateObjectInput(context),
|
||||
);
|
||||
|
||||
expect(errors.length).toBe(1);
|
||||
const firstError = errors[0];
|
||||
|
||||
expect(firstError.extensions.code).toBe(ErrorCode.BAD_USER_INPUT);
|
||||
expect(firstError.message).toMatchSnapshot();
|
||||
});
|
||||
});
|
||||
@ -0,0 +1,38 @@
|
||||
import { deleteOneObjectMetadataItem } from 'test/integration/metadata/suites/object-metadata/utils/delete-one-object-metadata.util';
|
||||
import { getMockCreateObjectInput } from 'test/integration/utils/object-metadata/generate-mock-create-object-metadata-input';
|
||||
import { performObjectMetadataCreation } from 'test/integration/utils/object-metadata/perform-object-metadata-creation';
|
||||
import { EachTestingContext } from 'twenty-shared';
|
||||
|
||||
import { CreateObjectInput } from 'src/engine/metadata-modules/object-metadata/dtos/create-object.input';
|
||||
|
||||
type CreateObjectInputPayload = Omit<
|
||||
CreateObjectInput,
|
||||
'workspaceId' | 'dataSourceId'
|
||||
>;
|
||||
|
||||
type CreateOneObjectMetadataItemTestingContext = EachTestingContext<
|
||||
Partial<CreateObjectInputPayload>
|
||||
>[];
|
||||
const successfulObjectMetadataItemCreateOneUseCase: CreateOneObjectMetadataItemTestingContext =
|
||||
[
|
||||
{
|
||||
title: 'with basic input',
|
||||
context: {},
|
||||
},
|
||||
// TODO populate
|
||||
];
|
||||
|
||||
const allTestsUseCases = [...successfulObjectMetadataItemCreateOneUseCase];
|
||||
|
||||
describe('Object metadata creation should succeed', () => {
|
||||
it.each(allTestsUseCases)('$title', async ({ context }) => {
|
||||
const response = await performObjectMetadataCreation(
|
||||
getMockCreateObjectInput(context),
|
||||
);
|
||||
|
||||
expect(response.body.data.createOneObject.id).toBeDefined();
|
||||
await deleteOneObjectMetadataItem(
|
||||
response.body.data.createOneObject.id,
|
||||
).catch();
|
||||
});
|
||||
});
|
||||
@ -0,0 +1,15 @@
|
||||
import { CreateObjectInput } from 'src/engine/metadata-modules/object-metadata/dtos/create-object.input';
|
||||
|
||||
// TODO would tend to use faker
|
||||
export const getMockCreateObjectInput = (
|
||||
overrides?: Partial<Omit<CreateObjectInput, 'workspaceId' | 'dataSourceId'>>,
|
||||
) => ({
|
||||
namePlural: 'listings',
|
||||
nameSingular: 'listing',
|
||||
labelPlural: 'Listings',
|
||||
labelSingular: 'Listing',
|
||||
description: 'Listing object',
|
||||
icon: 'IconListNumbers',
|
||||
isLabelSyncedWithName: false,
|
||||
...overrides,
|
||||
});
|
||||
@ -0,0 +1,28 @@
|
||||
import { deleteOneObjectMetadataItem } from 'test/integration/metadata/suites/object-metadata/utils/delete-one-object-metadata.util';
|
||||
import { performObjectMetadataCreation } from 'test/integration/utils/object-metadata/perform-object-metadata-creation';
|
||||
import { isDefined } from 'twenty-shared';
|
||||
|
||||
import { CreateObjectInput } from 'src/engine/metadata-modules/object-metadata/dtos/create-object.input';
|
||||
|
||||
export const performFailingObjectMetadataCreation = async (
|
||||
objectInput: Omit<CreateObjectInput, 'workspaceId' | 'dataSourceId'>,
|
||||
) => {
|
||||
const response = await performObjectMetadataCreation(objectInput);
|
||||
|
||||
if (isDefined(response.body.data)) {
|
||||
try {
|
||||
const createdId = response.body.data.createOneObject.id;
|
||||
|
||||
await deleteOneObjectMetadataItem(createdId);
|
||||
} catch (e) {
|
||||
// eslint-disable-next-line no-console
|
||||
console.error(e);
|
||||
}
|
||||
expect(false).toEqual(
|
||||
'Object Metadata Item should have failed but did not',
|
||||
);
|
||||
}
|
||||
expect(response.body.errors.length).toBeGreaterThan(0);
|
||||
|
||||
return response.body.errors;
|
||||
};
|
||||
@ -0,0 +1,18 @@
|
||||
import { createOneObjectMetadataFactory } from 'test/integration/metadata/suites/object-metadata/utils/create-one-object-metadata-factory.util';
|
||||
import { makeMetadataAPIRequest } from 'test/integration/metadata/suites/utils/make-metadata-api-request.util';
|
||||
|
||||
import { CreateObjectInput } from 'src/engine/metadata-modules/object-metadata/dtos/create-object.input';
|
||||
|
||||
export const performObjectMetadataCreation = async (
|
||||
args: Omit<CreateObjectInput, 'workspaceId' | 'dataSourceId'>,
|
||||
) => {
|
||||
const graphqlOperation = createOneObjectMetadataFactory({
|
||||
input: { object: args },
|
||||
gqlFields: `
|
||||
id
|
||||
nameSingular
|
||||
`,
|
||||
});
|
||||
|
||||
return await makeMetadataAPIRequest(graphqlOperation);
|
||||
};
|
||||
Reference in New Issue
Block a user