-
Notifications
You must be signed in to change notification settings - Fork 210
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
[POOL-589] Allow user creation with blank middle name. #1303
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.
@phiggins @blakestier Thanks for tracking this down. I've opened a PR for your branch so that it is easier to comment on.
In addition to the inline comments, you'll need to make sure your commits have the DCO Sign-Off. You can read about that here: https://github.com/chef/chef/blob/master/CONTRIBUTING.md#developer-certification-of-origin-dco
As @srenatus mentioned in chat, an alternative would be to officially disallow empty strings as input and then change the knife-opc
tool. I'd like to avoid more API changes than necessary for this change though.
@@ -74,6 +74,9 @@ | |||
%% Validation regex for various names | |||
-define(HUMAN_NAME_REGEX, "[[:word:][:digit:]!'. -]+"). | |||
|
|||
%% Validation regex for various names that can be empty. | |||
-define(OPTIONAL_NAME_REGEX, "[[:word:][:digit:]!'. -]*"). |
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.
Perhaps we should change HUMAN_NAME_REGEX
since first_name and last_name are also optional? The only non-optional field is display_name, but my guess is we have previously allowed empty strings there as well.
@@ -180,6 +180,26 @@ parse_binary_json_tests(Version) -> | |||
?assertEqual(lists:sort(MinValid), lists:sort(GotData)) | |||
end | |||
}, | |||
{?VD("Can create user with empty middle 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.
In addition to these unit tests, we will want to add oc-chef-pedant test cases for the /users API.
90152c7
to
f4c1b84
Compare
Signed-off-by: Pete Higgins <[email protected]>
f4c1b84
to
151de48
Compare
👍 Github won't let me approve this since I opened it. |
No description provided.