Refactor GitHub team handling to use dynamic role mapping and remove hardcoded values

Author: ChrisLovering

README.md

@@ -29,7 +29,6 @@ These environment variables are required to work on the relevant cog.
 | KING_ARTHUR_CLOUDFLARE_TOKEN          | Zones                 | A token for the Cloudflare API used for the Cloudflare commands in King Arthur \* | Required                  |
 | KING_ARTHUR_GITHUB_ORG                | GitHubManagement      | The github organisation to fetch teams from                                       | python-discord            |
 | KING_ARTHUR_GITHUB_TOKEN              | GitHubManagement      | The github token used to manage the GitHub organisation                           | Required                  |
-| KING_ARTHUR_GITHUB_TEAM               | GitHubManagement      | The slug of the GitHub team to add new members to                                 | staff                     |
 | KING_ARTHUR_GRAFANA_URL               | GrafanaLDAPTeamSync   | The URL to the grafana instance to manage teams                                   | https://grafana.pydis.wtf |
 | KING_ARTHUR_GRAFANA_TOKEN             | GrafanaLDAPTeamSync   | The grafana token used to sync teams with LDAP                                    | Required                  |
 | KING_ARTHUR_YOUTUBE_API_KEY           | Motivation            | The YouTube API key to fetch missions with                                        | Required                  |

arthur/config.py

@@ -23,7 +23,6 @@ class Config(
     grafana_token: pydantic.SecretStr | None = None
     github_token: pydantic.SecretStr | None = None
     github_org: str = "python-discord"
-    github_team: str = "staff"
 
     devops_role: int = 409416496733880320
     helpers_role: int = 267630620367257601

arthur/constants.py

@@ -10,17 +10,18 @@ class LDAPGroupMapping(TypedDict):
     github_team_slug: str
 
 
-# Users are only checked for enrollment if they have this role. This doesn't grant them any
-# permissions, it is for performance to avoid iterating roles for every other user in the guild.
-LDAP_BASE_STAFF_ROLE = 267630620367257601
-
 # This is a mapping of LDAP groups to Discord role IDs and GitHub team IDs. It is used to determine
 # which users should be eligible for LDAP enrollment.
 LDAP_ROLE_MAPPING: dict[str, LDAPGroupMapping] = {
+    "helpers": {"discord_role_id": 267630620367257601, "github_team_slug": "helpers"},
     "devops": {"discord_role_id": 409416496733880320, "github_team_slug": "devops"},
     "administrators": {"discord_role_id": 267628507062992896, "github_team_slug": "admins"},
     "moderators": {"discord_role_id": 267629731250176001, "github_team_slug": "moderators"},
     "coredevs": {"discord_role_id": 587606783669829632, "github_team_slug": "core-developers"},
     "events": {"discord_role_id": 787816728474288181, "github_team_slug": "events"},
     "directors": {"discord_role_id": 267627879762755584, "github_team_slug": "directors"},
 }
+
+# Users are only checked for enrollment if they have this role. This doesn't grant them any
+# permissions, it is for performance to avoid iterating roles for every other user in the guild.
+HELPER_ROLE_ID = LDAP_ROLE_MAPPING["helpers"]["discord_role_id"]

arthur/exts/directory/ldap.py

@@ -12,7 +12,7 @@
 
 from arthur.apis.directory import freeipa, keycloak, ldap
 from arthur.config import CONFIG
-from arthur.constants import LDAP_BASE_STAFF_ROLE, LDAP_ROLE_MAPPING
+from arthur.constants import HELPER_ROLE_ID, LDAP_ROLE_MAPPING
 from arthur.log import logger
 
 if TYPE_CHECKING:
@@ -126,7 +126,7 @@ async def generate_creds(self, interaction: discord.Interaction, _button: ui.But
         """Generate credentials for the user."""
         user = interaction.user
 
-        if LDAP_BASE_STAFF_ROLE not in [role.id for role in user.roles]:
+        if HELPER_ROLE_ID not in [role.id for role in user.roles]:
             await interaction.response.send_message(
                 "You are not eligible for LDAP enrollment.", ephemeral=True
             )
@@ -265,7 +265,7 @@ async def on_member_update(self, before: discord.Member, after: discord.Member)
         before_roles = {role.id for role in before.roles}
         after_roles = {role.id for role in after.roles}
 
-        if LDAP_BASE_STAFF_ROLE in before_roles or LDAP_BASE_STAFF_ROLE in after_roles:
+        if HELPER_ROLE_ID in before_roles or HELPER_ROLE_ID in after_roles:
             await self.sync_users()
 
     async def bootstrap(self, user: discord.Member) -> tuple[BootstrapType, str, str | None]:
@@ -375,7 +375,7 @@ async def get_user_diff(
 
         enrolled_roles = {mapping["discord_role_id"] for mapping in LDAP_ROLE_MAPPING.values()}
 
-        base_role = guild.get_role(LDAP_BASE_STAFF_ROLE)
+        base_role = guild.get_role(HELPER_ROLE_ID)
 
         diff = []
         missing_emp = [user for user in users if user.employee_number is None]

arthur/exts/github/management.py

@@ -6,6 +6,7 @@
 
 from arthur.apis.github import GitHubError, add_member_to_team, remove_org_member
 from arthur.config import CONFIG
+from arthur.constants import LDAP_ROLE_MAPPING
 
 if TYPE_CHECKING:
     from arthur.bot import KingArthurTheTerrible
@@ -35,7 +36,7 @@ async def github(self, ctx: Context) -> None:
     async def add_team_member(self, ctx: Context, username: str) -> None:
         """Add a user to the default GitHub team."""
         try:
-            await add_member_to_team(username, CONFIG.github_team)
+            await add_member_to_team(username, LDAP_ROLE_MAPPING["helpers"]["github_team_slug"])
             await ctx.send(f":white_check_mark: Successfully invited {username} to the staff team.")
         except GitHubError as e:
             await ctx.send(f":x: Failed to add {username} to the staff team: {e}")