-
Notifications
You must be signed in to change notification settings - Fork 17
added update user method #304
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
CLA Assistant Lite bot All contributors have signed the CLA ✍️ |
src/py42/services/users.py
Outdated
`REST Documentation <https://console.us.code42.com/apidocviewer/#User-put>`__ | ||
|
||
Args: | ||
user_id (int): An ID for the user. |
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.
This looks like the userUid
(via the REST documentation). In the users service, we still refer to user_uid
(and user_id
meaning the legacy id) since that will match the response, and won't be confused with other methods that use user_id
as a reference to the legacy id
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.
the REST endpoint can accept user_uid or user_id, I used user_id for consistency with the other methods in py42, would you like me to change it to user_uid instead?
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 think we tend to use user_uid
whenever possible, so yes I would prefer it here. The methods that are located near this method happen to use the legacy ID because of obligation, but other methods in this service use the user_uid
.
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.
Obligation or mistake, in case there are any that slipped through when not necessary
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.
Makes sense. I've updated it to use uid.
:class:`py42.response.Py42Response` | ||
""" | ||
|
||
uri = u"/api/User/{}?idType=uid".format(user_uid) |
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 think this is fine, but usually we let the params=
argument take care of query parameters
Description of Change
Currently, py42 can create a user, but cannot update a user that already exists. This change adds a method to allow updates to existing users.
Issues Resolved
n/a, this is a prerequesite for work on parts of code42/code42cli#259
Testing Procedure
sdk.users.update_user(user_id, username="[email protected]", email="[email protected]", first_name="new", last_name="name")
PR Checklist
Did you remember to do the below?