-
Notifications
You must be signed in to change notification settings - Fork 230
Bug 1535976 - Fix input name in the create-secret form #2680
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
Conversation
|
the bug says webhook secret key is not being validated, I dont see how this fixes that issue? |
|
@jwforres so since we are not doing any kind of validation of the secret key, but I've noticed that the secret name was not validated properly, since the input name was different from the name that that was checked on the form (and for some other input fields as well). |
|
ok i know on the original set of changes i said not to worry about the validation, but now we can probably look into it as a follow-on here. webhook secret key needs to be a valid URL path segment. Thats probably the only validation we should try to do. |
|
@jwforres PTAL |
| $scope.secretReferenceValidation = { | ||
| pattern: /^[a-zA-Z0-9\-_]+$/, | ||
| minLength: 8, | ||
| description: 'Secret reference key must consist of lower-case, upper-case letters, numbers, bash and underscore.' |
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.
dash not bash, also we use oxford commas here
"dash, and underscore"
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.
always forgetting about the commas. We dont use int with 'and' ... sorry
| </div> | ||
| <div class="has-error" ng-show="secretForm.webhookSecretKey.$error.minlength && secretForm.webhookSecretKey.$touched"> | ||
| <span class="help-block"> | ||
| Minimum character count is {{secretReferenceValidation.minLength}}. |
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 you got this message from the env editor, but I prefer what we say in other places which is something like:
The key must have at least 8 characters.
…mes in the create-secret form
|
@jwforres comments addressed. added dist as well. |
|
/lgtm |
|
Automatic merge from submit-queue. |
Fixed wrong
inputnames in thecreate-secret.html@jwforres PTAL