Skip to content

feat: add feature to get inherited member for project/group #1376

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

Merged
merged 1 commit into from
May 14, 2021

Conversation

Shkurupii
Copy link

@Shkurupii Shkurupii commented Mar 15, 2021

Hi!

First of all excuse me for inconvenience. I failed my previous PR(#1187) branch during rebasing, so PR automatically were closed.

Briefly, PR about the same, feature #1133, and taking into account @nejch 's suggestion from (#1187)
Here I propose to introduce new custom managers ProjectMemberAllManager (/projects/:id/members/all/:user_id) and GroupMemberAllManager (/groups/:id/members/all/:user_id). Subsequently, in order to fetch a list or instance of members including inherited ones, you will need to:

project = gl.projects.get(project_id)
project.members_all.list()
project.members_all.get(member_id)
group = gl.groups.get(group_id)
group.members_all.list()
group.members_all.get(member_id)

@JacobHenner , just fyi.

Closes #1133

@nejch nejch mentioned this pull request Apr 28, 2021
@nejch
Copy link
Member

nejch commented May 6, 2021

@JohnVillalovos this helps get rid of a custom method that prevented users from using the endpoint fully (only list was supported, not get, and even that didn't work properly like ListMixin). My only silly thought in the initial PR was that all_members() sounded a bit more natural than members_all(), any opinions there? 😁

@Shkurupii thanks again and sorry for the delay, I will resolve the conflicts here so you don't have to rebase again. One caveat is I think this will not work in the CLI because of how subcommands are constructed from object names (this reuses GroupMember). I'll do some local tests to see if that can be avoided.

Copy link
Member

@JohnVillalovos JohnVillalovos left a comment

Choose a reason for hiding this comment

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

Forgot to push the "Submit review" button a week or so ago...

@JohnVillalovos
Copy link
Member

JohnVillalovos commented May 6, 2021

@JohnVillalovos this helps get rid of a custom method that prevented users from using the endpoint fully (only list was supported, not get, and even that didn't work properly like ListMixin). My only silly thought in the initial PR was that all_members() sounded a bit more natural than members_all(), any opinions there? 😁

@nejch

Based on how the API is I lean towards members_all().

FYI: I did a rebase of this a week or so ago. Might help you.

https://github.com/JohnVillalovos/python-gitlab/tree/jlvillal/feat-get-inherited-members

@nejch nejch force-pushed the feat-get-inherited-members branch from 83f8d03 to 4b716a3 Compare May 6, 2021 20:11
@nejch nejch force-pushed the feat-get-inherited-members branch from 4b716a3 to e444b39 Compare May 6, 2021 22:50
Copy link
Member

@nejch nejch left a comment

Choose a reason for hiding this comment

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

@Shkurupii thanks, I just rebased/squashed this and added a few more fixes that come from more recent typing and pep8 checks but I've kept you as author.

I'll let @JohnVillalovos merge though so I don't self-approve 😁

@nejch nejch merged commit f35c73e into python-gitlab:master May 14, 2021
@JohnVillalovos
Copy link
Member

I'll let @JohnVillalovos merge though so I don't self-approve 😁

Oh sorry! I must have missed this 😢

Thanks for merging!

@Shkurupii Shkurupii deleted the feat-get-inherited-members branch May 15, 2021 07:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

/projects/:id/members/all/:user_id not supported
3 participants