Users command: Functionality to add/remove roles + Users user guide #283
Conversation
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ |
src/code42cli/cmds/users.py
Outdated
| @@ -60,6 +61,50 @@ def list_users(state, org_uid, role_name, active, inactive, format): | |||
| formatter.echo_formatted_dataframe(df) | |||
|
|
|||
|
|
|||
| @users.command() | |||
| @click.argument("username") | |||
There was a problem hiding this comment.
When commands have multiple arguments, we elect to use required options instead.
This is because then the order does not matter, like I don't have to use --help to see if the username goes first or the role name.
There was a problem hiding this comment.
Great, I added them as functions with flexible help text, assuming that they would be re-used in further expansions of the users command.
src/code42cli/cmds/users.py
Outdated
|
|
||
| def _add_user_role(sdk, role_name, username): | ||
| user_id = _get_user_id(sdk, username) | ||
| try: |
There was a problem hiding this comment.
I don't understand what this try / except is doing.
It looks like it catches an error just to raise it again.
Is there any technical difference of just letting the error raise naturally? If so, we should add a comment explaining what it is because it is not obvious.
There was a problem hiding this comment.
The try/except block isn't doing anything, actually - I will remove it.
We don't actually even need the roleID from _get_role_id to proceed. However, I like the error handling Cecilia had built for an invalid role name. It handles cases where an invalid role name was entered and provides the following error message:
Error: Role with name Fake_Role not found.
Otherwise, the error is:
Error: Problem making request to server.
Py42 doesn't really have a method for determining if a role name is valid.
There was a problem hiding this comment.
Might be worth adding a comment about that, or even extracting the error handling into a separate method
(still approved but thought I'd mention that)
There was a problem hiding this comment.
I added a comment. I had trouble extracting the error handling, since 95% of what that method doing is error handling. Extracting the error handling didn't add clarity or reduce redundancy.
…lp text modifications
This PR addresses a portion of Issue #259 by adding functionality to add and remove user roles to an individual user. This command takes a username and a user role name as two positional arguments.
Testing Procedure:
PR Checklist
Did you remember to do the below?