Skip to content
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

Return added team membership instance #2956

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 12 additions & 5 deletions github/Team.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
# Copyright 2024 Andrii Kezikov <[email protected]> #
# Copyright 2024 Enrico Minack <[email protected]> #
# Copyright 2024 Jirka Borovec <[email protected]> #
# Copyright 2024 Jonas Maurus <[email protected]> #
# #
# This file is part of PyGithub. #
# http://pygithub.readthedocs.io/ #
Expand Down Expand Up @@ -200,7 +201,7 @@ def add_to_members(self, member: NamedUser) -> None:
assert isinstance(member, github.NamedUser.NamedUser), member
headers, data = self._requester.requestJsonAndCheck("PUT", f"{self.url}/members/{member._identity}")

def add_membership(self, member: NamedUser, role: Opt[str] = NotSet) -> None:
def add_membership(self, member: NamedUser, role: Opt[str] = NotSet) -> Membership:
"""
:calls: `PUT /teams/{id}/memberships/{user} <https://docs.github.com/en/rest/reference/teams>`_
"""
Expand All @@ -218,6 +219,7 @@ def add_membership(self, member: NamedUser, role: Opt[str] = NotSet) -> None:
headers, data = self._requester.requestJsonAndCheck(
"PUT", f"{self.url}/memberships/{member._identity}", input=put_parameters
)
return github.Membership.Membership(self._requester, headers, data, completed=True)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Happy to approve this change, but can you extend the tests in tests/Team.py? Tests already call add_membership, so simply assert some attributes of the returned Membership instance.


def get_team_membership(self, member: str | NamedUser) -> Membership:
"""
Expand Down Expand Up @@ -298,11 +300,15 @@ def update_team_repository(self, repo: Repository, permission: str) -> bool:
)
return status == 204

def delete(self) -> None:
def delete(self) -> bool:
"""
:calls: `DELETE /teams/{id} <https://docs.github.com/en/rest/reference/teams#delete-a-team>`_
"""
headers, data = self._requester.requestJsonAndCheck("DELETE", self.url)
status, headers, data = self._requester.requestJson(
"DELETE",
self.url,
)
return status == 204
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is bad practise: #2760


def edit(
self,
Expand Down Expand Up @@ -411,12 +417,13 @@ def has_in_repos(self, repo: Repository) -> bool:
status, headers, data = self._requester.requestJson("GET", f"{self.url}/repos/{repo._identity}")
return status == 204

def remove_membership(self, member: NamedUser) -> None:
def remove_membership(self, member: NamedUser) -> bool:
"""
:calls: `DELETE /teams/{team_id}/memberships/{username} <https://docs.github.com/en/rest/reference/teams#remove-team-membership-for-a-user>`_
"""
assert isinstance(member, github.NamedUser.NamedUser), member
headers, data = self._requester.requestJsonAndCheck("DELETE", f"{self.url}/memberships/{member._identity}")
status, headers, data = self._requester.requestJson("DELETE", f"{self.url}/memberships/{member._identity}")
return status == 204
Copy link
Collaborator

Choose a reason for hiding this comment

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

same here


def remove_from_members(self, member: NamedUser) -> None:
"""
Expand Down