-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Conversation
packages/twenty-server/package.json
Outdated
@@ -30,6 +30,7 @@ | |||
"lodash.uniqby": "^4.7.0", | |||
"passport": "^0.7.0", | |||
"psl": "^1.9.0", | |||
"transliteration": "^2.3.5", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM, left a few comments! 👏
packages/twenty-front/src/pages/settings/data-model/utils/format-string.util.ts
Outdated
Show resolved
Hide resolved
packages/twenty-server/src/engine/metadata-modules/utils/validate-string.utils.ts
Outdated
Show resolved
Hide resolved
packages/twenty-server/src/engine/metadata-modules/utils/__tests__/validate-string.spec.ts
Outdated
Show resolved
Hide resolved
packages/twenty-server/src/engine/metadata-modules/field-metadata/field-metadata.service.ts
Outdated
Show resolved
Hide resolved
@@ -1,4 +1,4 @@ | |||
const metadataLabelValidationPattern = /^[a-zA-Z][a-zA-Z0-9 ]*$/; | |||
const metadataLabelValidationPattern = /^[^0-9].*$/; |
There was a problem hiding this comment.
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
|
||
const VALID_STRING_PATTERN = /^[a-zA-Z][a-zA-Z0-9 ]*$/; | ||
|
||
export const formatMetadataLabel = (string: string): string => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually maybe formatMetadataLabelToMetadataName is more explicit? Wdyt
@@ -0,0 +1,9 @@ | |||
import { InvalidStringException } from 'src/engine/metadata-modules/errors/InvalidStringException'; | |||
|
|||
const VALID_STRING_PATTERN = /^[a-zA-Z][a-zA-Z0-9 ]*$/; |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Good job! @ijreilly
Fixes twentyhq#4943 ## How was it tested? Local (front + /metadata) Unit tests for utils --------- Co-authored-by: Weiko <[email protected]>
Fixes #4943
How was it tested?
Local (front + /metadata)
Unit tests for utils