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.

Sign up for GitHub

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

Jump to bottom
Merged
nejch merged 1 commit into from
May 14, 2021
Merged

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

nejch merged 1 commit into from
May 14, 2021

Conversation

@Shkurupii
Copy link

@Shkurupii Shkurupii commented Mar 15, 2021 •
edited by nejch
Loading

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
Closed
@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.

JohnVillalovos
JohnVillalovos reviewed May 6, 2021
View reviewed changes
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 •
edited
Loading

@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
nejch
nejch approved these changes May 6, 2021
View reviewed changes
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

JohnVillalovos commented May 14, 2021

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
@nejch nejch mentioned this pull request Jun 30, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JohnVillalovos JohnVillalovos JohnVillalovos left review comments

@nejch nejch nejch approved these changes

Assignees

@JohnVillalovos JohnVillalovos

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

3 participants

@Shkurupii @nejch @JohnVillalovos