Allow for repo strings in all Team repo methods (#3356)

Harmonizes repo string support across all `Team` repo methods.
Increases code reuse, adds tests for str/Repository logic and use.

Fixes #3355.

Author: EnricoMi

github/Repository.py

@@ -1261,6 +1261,17 @@ def web_commit_signoff_required(self) -> bool:
         self._completeIfNotSet(self._web_commit_signoff_required)
         return self._web_commit_signoff_required.value
 
+    @staticmethod
+    def as_url_param(repo: str | Repository) -> str:
+        assert isinstance(repo, (str, github.Repository.Repository))
+        if isinstance(repo, github.Repository.Repository):
+            return repo._identity  # type: ignore
+        else:
+            # we expect exactly one slash in the repo name
+            assert len(repo.split("/")) == 2, repo
+            # do not quote the slash as this is expected to become part of URL path
+            return urllib.parse.quote(repo, safe="/")
+
     def add_to_collaborators(self, collaborator: str | NamedUser, permission: Opt[str] = NotSet) -> Invitation | None:
         """
         :calls: `PUT /repos/{owner}/{repo}/collaborators/{user} <https://docs.github.com/en/rest/collaborators/collaborators#add-a-repository-collaborator>`_

github/Team.py

@@ -301,26 +301,24 @@ def get_team_membership(self, member: str | NamedUser) -> Membership:
         headers, data = self._requester.requestJsonAndCheck("GET", f"{self.url}/memberships/{member}")
         return github.Membership.Membership(self._requester, headers, data, completed=True)
 
-    def add_to_repos(self, repo: Repository) -> None:
+    def add_to_repos(self, repo: str | Repository) -> None:
         """
         :calls: `PUT /teams/{id}/repos/{org}/{repo} <https://docs.github.com/en/rest/reference/teams>`_
         """
-        assert isinstance(repo, github.Repository.Repository), repo
-        headers, data = self._requester.requestJsonAndCheck("PUT", f"{self.url}/repos/{repo._identity}")
+        assert isinstance(repo, (str, github.Repository.Repository)), repo
+        headers, data = self._requester.requestJsonAndCheck(
+            "PUT", f"{self.url}/repos/{github.Repository.Repository.as_url_param(repo)}"
+        )
 
-    def get_repo_permission(self, repo: Repository) -> Permissions | None:
+    def get_repo_permission(self, repo: str | Repository) -> Permissions | None:
         """
         :calls: `GET /teams/{id}/repos/{org}/{repo} <https://docs.github.com/en/rest/reference/teams>`_
         """
-        assert isinstance(repo, github.Repository.Repository) or isinstance(repo, str), repo
-        if isinstance(repo, github.Repository.Repository):
-            repo = repo._identity  # type: ignore
-        else:
-            repo = urllib.parse.quote(repo, safe="")
+        assert isinstance(repo, (str, github.Repository.Repository)), repo
         try:
             headers, data = self._requester.requestJsonAndCheck(
                 "GET",
-                f"{self.url}/repos/{repo}",
+                f"{self.url}/repos/{github.Repository.Repository.as_url_param(repo)}",
                 headers={"Accept": Consts.teamRepositoryPermissions},
             )
             return github.Permissions.Permissions(self._requester, headers, data["permissions"])
@@ -332,38 +330,33 @@ def get_repo_permission(self, repo: Repository) -> Permissions | None:
         Team.set_repo_permission() is deprecated, use Team.update_team_repository() instead.
         """
     )
-    def set_repo_permission(self, repo: Repository, permission: str) -> None:
+    def set_repo_permission(self, repo: str | Repository, permission: str) -> None:
         """
         :calls: `PUT /teams/{id}/repos/{org}/{repo} <https://docs.github.com/en/rest/reference/teams>`_
         :param repo: :class:`github.Repository.Repository`
         :param permission: string
         :rtype: None
         """
-
-        assert isinstance(repo, github.Repository.Repository), repo
+        assert isinstance(repo, (str, github.Repository.Repository)), repo
         put_parameters = {
             "permission": permission,
         }
         headers, data = self._requester.requestJsonAndCheck(
-            "PUT", f"{self.url}/repos/{repo._identity}", input=put_parameters
+            "PUT", f"{self.url}/repos/{github.Repository.Repository.as_url_param(repo)}", input=put_parameters
         )
 
-    def update_team_repository(self, repo: Repository, permission: str) -> bool:
+    def update_team_repository(self, repo: str | Repository, permission: str) -> bool:
         """
         :calls: `PUT /orgs/{org}/teams/{team_slug}/repos/{owner}/{repo} <https://docs.github.com/en/rest/reference/teams#check-team-permissions-for-a-repository>`_
         """
-        assert isinstance(repo, github.Repository.Repository) or isinstance(repo, str), repo
+        assert isinstance(repo, (str, github.Repository.Repository)), repo
         assert isinstance(permission, str), permission
-        if isinstance(repo, github.Repository.Repository):
-            repo_url_param = repo._identity
-        else:
-            repo_url_param = urllib.parse.quote(repo, safe="")
         put_parameters = {
             "permission": permission,
         }
         status, _, _ = self._requester.requestJson(
             "PUT",
-            f"{self.organization.url}/teams/{self.slug}/repos/{repo_url_param}",
+            f"{self.organization.url}/teams/{self.slug}/repos/{github.Repository.Repository.as_url_param(repo)}",
             input=put_parameters,
         )
         return status == 204
@@ -473,12 +466,14 @@ def has_in_members(self, member: NamedUser) -> bool:
         status, headers, data = self._requester.requestJson("GET", f"{self.url}/members/{member._identity}")
         return status == 204
 
-    def has_in_repos(self, repo: Repository) -> bool:
+    def has_in_repos(self, repo: str | Repository) -> bool:
         """
         :calls: `GET /teams/{id}/repos/{owner}/{repo} <https://docs.github.com/en/rest/reference/teams>`_
         """
-        assert isinstance(repo, github.Repository.Repository), repo
-        status, headers, data = self._requester.requestJson("GET", f"{self.url}/repos/{repo._identity}")
+        assert isinstance(repo, (str, github.Repository.Repository)), repo
+        status, headers, data = self._requester.requestJson(
+            "GET", f"{self.url}/repos/{github.Repository.Repository.as_url_param(repo)}"
+        )
         return status == 204
 
     def remove_membership(self, member: NamedUser) -> None:
@@ -498,12 +493,14 @@ def remove_from_members(self, member: NamedUser) -> None:
         assert isinstance(member, github.NamedUser.NamedUser), member
         headers, data = self._requester.requestJsonAndCheck("DELETE", f"{self.url}/members/{member._identity}")
 
-    def remove_from_repos(self, repo: Repository) -> None:
+    def remove_from_repos(self, repo: str | Repository) -> None:
         """
         :calls: `DELETE /teams/{id}/repos/{owner}/{repo} <https://docs.github.com/en/rest/reference/teams>`_
         """
-        assert isinstance(repo, github.Repository.Repository), repo
-        headers, data = self._requester.requestJsonAndCheck("DELETE", f"{self.url}/repos/{repo._identity}")
+        assert isinstance(repo, (str, github.Repository.Repository)), repo
+        headers, data = self._requester.requestJsonAndCheck(
+            "DELETE", f"{self.url}/repos/{github.Repository.Repository.as_url_param(repo)}"
+        )
 
     def _useAttributes(self, attributes: dict[str, Any]) -> None:
         if "created_at" in attributes:  # pragma no branch

tests/Repository.py

@@ -104,6 +104,7 @@
 from unittest import mock
 
 import github
+import github.Repository
 
 from . import Framework
 
@@ -268,6 +269,15 @@ def testAttributes(self):
         self.assertEqual(self.repo.web_commit_signoff_required, False)
         self.assertEqual(self.repo.custom_properties, {})
 
+    def testAsUrlParam(self):
+        self.assertEqual(github.Repository.Repository.as_url_param(self.repo), "PyGithub/PyGithub")
+        self.assertEqual(github.Repository.Repository.as_url_param(self.repo._identity), "PyGithub/PyGithub")
+
+        for repo in ["repo", "repo/name/slash"]:
+            with self.assertRaises(AssertionError) as raisedexp:
+                github.Repository.Repository.as_url_param(repo)
+            self.assertEqual(raisedexp.exception.args, (repo,))
+
     def testEditWithoutArguments(self):
         self.repo.edit("PyGithub")
 

tests/Team.py

@@ -53,9 +53,13 @@
 
 import warnings
 from datetime import datetime, timezone
+from typing import TYPE_CHECKING
 
 from . import Framework
 
+if TYPE_CHECKING:
+    from github.Repository import Repository
+
 
 class Team(Framework.TestCase):
     def setUp(self):
@@ -162,8 +166,7 @@ def testUpdateTeamRepository(self):
         repo = self.org.get_repo("FatherBeaver")
         self.assertTrue(self.team.update_team_repository(repo, "admin"))
 
-    def testRepos(self):
-        repo = self.org.get_repo("FatherBeaver")
+    def doTestRepos(self, repo: str | Repository):
         self.assertListKeyEqual(self.team.get_repos(), None, [])
         self.assertFalse(self.team.has_in_repos(repo))
         self.assertIsNone(self.team.get_repo_permission(repo))
@@ -176,6 +179,13 @@ def testRepos(self):
         self.assertListKeyEqual(self.team.get_repos(), None, [])
         self.assertFalse(self.team.has_in_repos(repo))
 
+    def testRepos(self):
+        self.doTestRepos(self.org.get_repo("FatherBeaver"))
+
+    def testReposStr(self):
+        with self.replayData("Team.testRepos.txt"):
+            self.doTestRepos(self.org.get_repo("FatherBeaver")._identity)
+
     def testEditWithoutArguments(self):
         self.team.edit("Name edited by PyGithub")
         self.assertEqual(self.team.name, "Name edited by PyGithub")