From 09d25dc31d7569f5a5d90a8fab772e39e7bfc0bf Mon Sep 17 00:00:00 2001 From: Clemens Lang Date: Mon, 26 May 2025 13:39:51 +0200 Subject: [PATCH] Fix failures introduced by f859fb8 The change in f859fb8 assumed that GitHub would respond to API requests to /organizations/$org_name/team/$team_id, but it doesn't. The only supported URL seems to be /organizations/$org_id/team/$team_id, so we need to somehow query the teams using both their org and team IDs. Fortunately GitHub includes an "url" field in the response for listing teams that can be used to obtain the correct path. Use this field to determine the correct URL and adjust the test harness to match GitHub's actual behavior. --- runtests.py | 36 ++++++++++++++++++++++++++---------- tracext/github/__init__.py | 11 ++++++----- 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/runtests.py b/runtests.py index 77c6821..970c044 100755 --- a/runtests.py +++ b/runtests.py @@ -1519,17 +1519,19 @@ def test_006_normal_operation(self): '/orgs/%s/teams' % self.organization: [ { "id": 1, + "url": u"%sorganizations/14143/team/1" % self.tracd_env_debug.get('TRAC_GITHUB_API_URL'), "name": u"Justice League", "slug": u"justice-league" }, { "id": 12, + "url": u"%sorganizations/14143/team/12" % self.tracd_env_debug.get('TRAC_GITHUB_API_URL'), "name": u"The League of Extraordinary Gentlemen and Gentlewomen", "slug": u"gentlepeople" } ], - '/organizations/%s/team/1/members' % self.organization: team1members, - '/organizations/%s/team/12/members' % self.organization: team12members + '/organizations/14143/team/1/members': team1members, + '/organizations/14143/team/12/members': team12members }) with TracContext(self, env=self.tracd_env_debug, **self.trac_env): @@ -1702,22 +1704,25 @@ def test_012_hook_membership_event_delete_team(self): '/orgs/%s/teams' % self.organization: [ { "id": 1, + "url": u"%sorganizations/14143/team/1" % self.tracd_env_debug.get('TRAC_GITHUB_API_URL'), "name": u"Justice League", "slug": u"justice-league" }, { "id": 12, + "url": u"%sorganizations/14143/team/12" % self.tracd_env_debug.get('TRAC_GITHUB_API_URL'), "name": u"The League of Extraordinary Gentlemen and Gentlewomen", "slug": u"gentlepeople" } ], - '/organizations/%s/team/1/members' % self.organization: team1members, - '/organizations/%s/team/12/members' % self.organization: team12members + '/organizations/14143/team/1/members': team1members, + '/organizations/14143/team/12/members': team12members }) update = { "team": { "id": 1, + "url": u"%sorganizations/14143/team/1" % self.tracd_env_debug.get('TRAC_GITHUB_API_URL'), "name": u"Justice League", "deleted": True } @@ -1740,11 +1745,12 @@ def test_012_hook_membership_event_delete_team(self): '/orgs/%s/teams' % self.organization: [ { "id": 12, + "url": u"%sorganizations/14143/team/12" % self.tracd_env_debug.get('TRAC_GITHUB_API_URL'), "name": u"The League of Extraordinary Gentlemen and Gentlewomen", "slug": u"gentlepeople" } ], - '/organizations/%s/team/12/members' % self.organization: team12members + '/organizations/14143/team/12/members': team12members }) # Send the delete event @@ -1782,6 +1788,7 @@ def test_013_hook_membership_event_delete_nonexistant_team(self): update = { "team": { "id": 1, + "url": u"%sorganizations/14143/team/1" % self.tracd_env_debug.get('TRAC_GITHUB_API_URL'), "name": u"Justice League", "deleted": True } @@ -1809,6 +1816,7 @@ def test_014_hook_membership_event_add_team(self): update = { "team": { "id": 1, + "url": u"%sorganizations/14143/team/1" % self.tracd_env_debug.get('TRAC_GITHUB_API_URL'), "name": u"Justice League", "slug": u"justice-league" } @@ -1825,11 +1833,12 @@ def test_014_hook_membership_event_add_team(self): '/orgs/%s/teams' % self.organization: [ { "id": 1, + "url": u"%sorganizations/14143/team/1" % self.tracd_env_debug.get('TRAC_GITHUB_API_URL'), "name": u"Justice League", "slug": u"justice-league" }, ], - '/organizations/%s/team/1/members' % self.organization: team1members, + '/organizations/14143/team/1/members': team1members, }) # Send the update event @@ -1868,16 +1877,18 @@ def test_015_hook_membership_event_add_member(self): '/orgs/%s/teams' % self.organization: [ { "id": 1, + "url": u"%sorganizations/14143/team/1" % self.tracd_env_debug.get('TRAC_GITHUB_API_URL'), "name": u"Justice League", "slug": u"justice-league" }, ], - '/organizations/%s/team/1/members' % self.organization: list(team1members) + '/organizations/14143/team/1/members': list(team1members) }) update = { "team": { "id": 1, + "url": u"%sorganizations/14143/team/1" % self.tracd_env_debug.get('TRAC_GITHUB_API_URL'), "name": u"Justice League", "slug": u"justice-league" } @@ -1890,11 +1901,12 @@ def test_015_hook_membership_event_add_member(self): '/orgs/%s/teams' % self.organization: [ { "id": 1, + "url": u"%sorganizations/14143/team/1" % self.tracd_env_debug.get('TRAC_GITHUB_API_URL'), "name": u"Justice League", "slug": u"justice-league" }, ], - '/organizations/%s/team/1/members' % self.organization: list(team1members) + '/organizations/14143/team/1/members': list(team1members) }) # Send the update event @@ -1933,16 +1945,18 @@ def test_016_hook_membership_event_remove_member(self): '/orgs/%s/teams' % self.organization: [ { "id": 1, + "url": u"%sorganizations/14143/team/1" % self.tracd_env_debug.get('TRAC_GITHUB_API_URL'), "name": u"Justice League", "slug": u"justice-league" }, ], - '/organizations/%s/team/1/members' % self.organization: list(team1members) + '/organizations/14143/team/1/members': list(team1members) }) update = { "team": { "id": 1, + "url": u"%sorganizations/14143/team/1" % self.tracd_env_debug.get('TRAC_GITHUB_API_URL'), "name": u"Justice League", "slug": u"justice-league" } @@ -1955,11 +1969,12 @@ def test_016_hook_membership_event_remove_member(self): '/orgs/%s/teams' % self.organization: [ { "id": 1, + "url": u"%sorganizations/14143/team/1" % self.tracd_env_debug.get('TRAC_GITHUB_API_URL'), "name": u"Justice League", "slug": u"justice-league" }, ], - '/organizations/%s/team/1/members' % self.organization: list(team1members) + '/organizations/14143/team/1/members': list(team1members) }) # Send the update event @@ -2072,6 +2087,7 @@ def test_021_hook_membership_event_api_failure(self): update = { "team": { "id": 1, + "url": u"%sorganizations/14143/team/1" % self.tracd_env_debug.get('TRAC_GITHUB_API_URL'), "name": u"Justice League", "deleted": True } diff --git a/tracext/github/__init__.py b/tracext/github/__init__.py index fe9dc6f..e1b254d 100644 --- a/tracext/github/__init__.py +++ b/tracext/github/__init__.py @@ -464,23 +464,23 @@ class GitHubTeam(GitHubUserCollection): get a list of GitHub login names that are part of this group. To access the full name of this team, use the `fullname()` method. """ - def __init__(self, api, env, org, teamid, slug): # pylint: disable=too-many-arguments + def __init__(self, api, env, org, url, slug): # pylint: disable=too-many-arguments """ Create a new team. :param api: the `GitHubGroupsProvider` providing API access :param env: the `TracEnvironment` context used to cache results :param org: the name of the organization of the team - :param teamid: the GitHub team ID of the team + :param url: the GitHub API URL of the team :param slug: the GitHub team shortname in URL representation """ - self._teamid = teamid + self._url = url self._orgid = org fullname = '-'.join(['github', org, slug]) super(GitHubTeam, self).__init__(api, env, fullname) def _apicall_parameters(self): - return ("organizations/{}/team/{}/members", self._orgid, self._teamid) + return ("{}/members", self._url) #class GitHubOrgMembers(GitHubUserCollection): # """ @@ -527,7 +527,8 @@ def _apicall_parameters(self): return ("orgs/{}/teams", self._org) def _apiresult_postprocess(self, json_obj): - return {team['slug']: team['id'] for team in json_obj} + github_api_url = os.environ.get("TRAC_GITHUB_API_URL", "https://api.github.com/").rstrip('/') + '/' + return {team['slug']: team['url'][len(github_api_url):] for team in json_obj} def _apiresult_error(self): return {}