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
updated regex for resource name validation and convert to kebabCase if name is not valid #9373
Conversation
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.
I like the function wrapping improvements 👍
@@ -46,12 +46,13 @@ describe('ValidationUtils', () => { | |||
|
|||
describe('createComponentName', () => { | |||
const invalidConvertedtoValidNamePair: { [key: string]: string } = { | |||
'0name': 'ocp-0name', |
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.
Need to bring this back (in some fashion) I think because you undid your regex changes.
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.
ah yes, if the source name starts with a number (i.e repo name)
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.
Have handled it, thanks.
|
||
return _.kebabCase(url.split('/').pop()); | ||
const kebabCaseStr = _.kebabCase(nameString); | ||
return nameString.match(/^\d/) ? `ocp-${kebabCaseStr}` : kebabCaseStr; |
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.
return nameString.match(/^\d/) ? `ocp-${kebabCaseStr}` : kebabCaseStr; | |
return nameString.match(/^\d/) || kebabCaseStr.match(/^\d/) ? `ocp-${kebabCaseStr}` : kebabCaseStr; |
Technically, a github reposistory name can start with a hypen in the begining.
So if you call the createComponentName('-2dotnetcore') it returns back 2-dotnetcore
which is invalid name, so we need to consider the kebabCaseStr
string as well, so that the output becomes ocp-2-donetcore
'invalid-Name': 'ocp-invalid-name', | ||
'0name': 'ocp-0-name', | ||
Name: 'name', | ||
'-name': 'name', |
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.
Based on the previouse comment, you could add the following test data to this list as well.
-2name': 'ocp-2-name
,
…f name is not valid
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: invincibleJai, karthikjeeyar The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Fixes:
https://issues.redhat.com/browse/ODC-6043
Analysis / Root cause:
Solution Description:
Screen shots / Gifs for design review:
Unit test coverage report:
Browser conformance: