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 transliteration for metadata + transliterate select options #5430

Merged
merged 11 commits into from
May 15, 2024

Conversation

ijreilly
Copy link
Contributor

@ijreilly ijreilly commented May 15, 2024

Context

Fixes #5403

Transliteration is now integrated to form validation through the schema. While it does not impede inputting an invalid value, it impedes submitting a form that will fail as the transliteration is not possible. Until then we were only performing the transliteration at save time in the front-end, but it's best to provide the information as soon as possible. Later we will add helpers to guide the user (eg "This name is not valid": #5428).


export const transliterateLabel = (label: string) =>
formatLabelOrThrows(label, OPTION_VALUE_VALID_STRING_PATTERN);

export const getOptionValueFromLabel = (label: string) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we don't want to throw here as this needs to be evaluated before save-time to generate the option value as we type; unlike in the object/fields forms where the transliteration only needs to be performed at save-time

@ijreilly ijreilly marked this pull request as ready for review May 15, 2024 15:03
@ijreilly ijreilly requested review from charlesBochet and thaisguigon and removed request for charlesBochet May 15, 2024 15:03
import { OPTION_VALUE_VALID_STRING_PATTERN } from '~/pages/settings/data-model/utils/constants.utils';
import { formatLabelOrThrows } from '~/pages/settings/data-model/utils/format-label.util';

export const transliterateLabel = (label: string) =>
Copy link
Member

Choose a reason for hiding this comment

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

remove this function and only use formatLabelOrThrows

OPTION_VALUE_VALID_STRING_PATTERN,
} from '~/pages/settings/data-model/utils/constants.utils';

describe('METADATA_NAME_VALID_STRING_PATTERN', () => {
Copy link
Member

Choose a reason for hiding this comment

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

instead of testing regex, introduce computeMetadataNameFromLabelOrThrow(), computeSelectOptionValueOrThrow

computeMetadataNameFromLabelOrThrow = (string) => transliterateAndValidFormatOrThrow(string, VALID_PATTERN)

rename: formatLabelOrThrow(string, VALID_PATTERN) ==> transliterateAndValidFormatOrThrow(string, VALID_PATTERN)

@@ -4,7 +4,7 @@ import {
ValidationArguments,
} from 'class-validator';

const graphQLEnumNameRegex = /^[_A-Za-z][_0-9A-Za-z]+$/;
const graphQLEnumNameRegex = /^[_A-Za-z][_0-9A-Za-z]*$/;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@magrinj are you ok with this update? One-char long names are not an issue with graphql

@charlesBochet charlesBochet merged commit 6338742 into main May 15, 2024
8 of 13 checks passed
@charlesBochet charlesBochet deleted the 5403-fix-transliteration branch May 15, 2024 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Transliteration] Cannot save a field, object, option with non-latin characters
2 participants