ephraimbuddy commented on a change in pull request #18757:
URL: https://github.com/apache/airflow/pull/18757#discussion_r723284713
##########
File path: airflow/api_connexion/endpoints/user_endpoint.py
##########
@@ -124,9 +124,21 @@ def patch_user(username, update_mask=None):
if user is None:
detail = f"The User with username `{username}` was not found"
raise NotFound(title="User not found", detail=detail)
-
- # Get fields to update. 'username' is always excluded (and it's an error to
- # include it in update_maek).
+ # Check unique username
+ new_username = data.get('username')
+ if new_username:
+ usr = security_manager.find_user(username=new_username)
+ if usr and usr != user:
+ raise AlreadyExists(detail=f"The username `{new_username}` already
exists")
+
+ # Check unique email
+ email = data.get('email')
+ if email:
+ usr = security_manager.find_user(email=email)
+ if usr and usr != user:
+ raise AlreadyExists(detail=f"The email `{email}` already exists")
Review comment:
I would suggest we put these as required values at the expense of user
experience. Now, the post endpoint thinks that username/email are required and
doesn't consider first_name/last_name as required values. This means that we
can create users without first_name and last_name. The schema for users doesn't
have required fields, the same as the API spec. I'm proposing to make
first_name, last_name, username, and email required fields.
The patch endpoint is intended to be used to edit other users, not only the
user making the request. Therefore making the fields required may not actually
be at the expense of user experience but a normal thing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
