Skip to content

{Graph} az ad user get-member-groups: Use /getMemberGroups API #22337

Merged
jiasli merged 3 commits intoAzure:betafrom
jiasli:graph-member
May 11, 2022
Merged

{Graph} az ad user get-member-groups: Use /getMemberGroups API #22337
jiasli merged 3 commits intoAzure:betafrom
jiasli:graph-member

Conversation

@jiasli
Copy link
Copy Markdown
Member

@jiasli jiasli commented May 10, 2022

During the development, az ad user get-member-groups uses incorrect API /memberOf:

def get_user_member_groups(client, upn_or_object_id, security_enabled_only=False):
if not is_guid(upn_or_object_id):
upn_or_object_id = client.user_get(upn_or_object_id)["id"]
results = client.user_member_groups_get(id=upn_or_object_id)
if security_enabled_only:
results = [res for res in results if res["securityEnabled"]]
return [{'objectId': x["id"], 'displayName': x["displayName"]} for x in results]

Instead, the correct API is /getMemberGroups, like what az ad group get-member-groups uses:

def get_member_groups(client, object_id, security_enabled_only):
"""Get a collection of object IDs of groups of which the specified group is a member."""
body = {
"securityEnabledOnly": security_enabled_only
}
return client.directory_object_get_member_groups(id=object_id, body=body)

This PR depends on #22318

@ghost ghost requested a review from yonzhan May 10, 2022 05:30
@ghost ghost added the Auto-Assign Auto assign by bot label May 10, 2022
@ghost ghost requested a review from wangzelin007 May 10, 2022 05:31
@ghost ghost assigned jiasli May 10, 2022
@ghost ghost added this to the May 2022 (2022-05-24) - For Build milestone May 10, 2022
@ghost ghost added the Graph (doesn't work with label-triggered comments; use Graph.Microsoft instead) az ad label May 10, 2022
@yonzhan
Copy link
Copy Markdown
Collaborator

yonzhan commented May 10, 2022

Graph

def delete_application(client, identifier):
object_id = _resolve_application(client, identifier)
client.application_delete(id=object_id)
client.application_delete(object_id)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the function takes positional argument, the best practice is to pass the argument as positional argument too, instead of as a keyword argument.

return result

def user_get(self, id):
def user_get(self, id_or_upn):
Copy link
Copy Markdown
Member Author

@jiasli jiasli May 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We honor the API's name:

GET /users/{id | userPrincipalName}

c.argument('account_enabled', arg_type=get_three_state_flag(), help='enable the user account')
c.argument('password', help='user password')
c.argument('upn_or_object_id', options_list=['--id', c.deprecate(target='--upn-or-object-id', redirect='--id', hide=True)], help='The object ID or principal name of the user for which to get information')
c.argument('upn_or_object_id', options_list=['--id'],
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--upn-or-object-id has been deprecated very long ago (#9297), now we remove it.

@jiasli jiasli merged commit a146a88 into Azure:beta May 11, 2022
@jiasli jiasli deleted the graph-member branch May 11, 2022 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Auto-Assign Auto assign by bot Graph (doesn't work with label-triggered comments; use Graph.Microsoft instead) az ad Microsoft Graph

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants