fix: incoherency allowed characters tableName/columnName (+ improved regex & docs) #4173
Conversation
|
integrated #4172 |
svandenhoek
changed the title
There was a problem hiding this comment.
Looks good. I think some minor revisions to the error message could help make it clearer for end users. Perhaps something like this:
Table names must start with a letter followed by one or more letters, numbers or underscores. Names cannot exceed 31 characters. Spaces are also allowed as long as they do not occur immediately before or after an underscore.
We could also have a few examples: My table, myTable, my_table, My table 2, etc.
| @@ -0,0 +1,30 @@ | |||
| import { test, expect } from '@playwright/test'; | |||
|
|
|||
| let regexErrorMessage='Table name must start with a letter, followed by letters/underscores/spaces/numbers (though no underscore preceded/followed by a space) and with a maximum of 31 characters, i.e. ^(?!.* _|.*_ )[a-zA-Z][a-zA-Z0-9 _]{0,30}$' | |||
There was a problem hiding this comment.
Unless the message is reassigned, const should be used here instead of let.
| @@ -178,7 +179,7 @@ export default { | |||
| ) { | |||
| return undefined; | |||
| } else { | |||
| return "Table name must start with a letter, followed by letters, underscores, a space or numbers, i.e. [a-zA-Z][a-zA-Z0-9_]*. Maximum length: 31 characters"; | |||
| return "Table name must start with a letter, followed by letters/underscores/spaces/numbers (though no underscore preceded/followed by a space) and with a maximum of 31 characters, i.e. ^(?!.* _|.*_ )[a-zA-Z][a-zA-Z0-9 _]{0,30}$"; | |||
There was a problem hiding this comment.
Should we put the regex pattern in the error message? It is helpful for developers, but I'm not sure if many end users (data managers, lab personnel, etc.) would understand this.
There was a problem hiding this comment.
Fair point. I left it in as they were present already, though for a user (without IT background) it indeed can just cause confusion.
| if (!this.column.name.match(/^[a-zA-Z][a-zA-Z0-9_ ]+$/)) { | ||
| return "Name should start with letter, followed by letter, number, whitespace or underscore ([a-zA-Z][a-zA-Z0-9_ ]*)"; | ||
| if (!this.column.name.match(constants.COLUMN_NAME_REGEX)) { | ||
| return "Name should start with a letter, followed by letters/underscores/spaces/numbers (though no underscore preceded/followed by a space), i.e. ^(?!.* _|.*_ )[a-zA-Z][a-zA-Z0-9 _]*$"; |
| ) { | ||
| return "Name is required and can only contain 'azAZ_ '"; | ||
| return "Name is required and must start with a letter, followed by letters/underscores/spaces/numbers (though no underscore preceded/followed by a space) and with a maximum of 31 characters, i.e. ^(?!.* _|.*_ )[a-zA-Z][a-zA-Z0-9 _]{0,30}$"; |
|
|
What are the main changes you did:
how to test:
todo: