Refactor adding members to Grafana team to its own function

This also adds dataclasses to describe return types for clearer code.

1 files changed, 54 insertions, 18 deletions

diff --git a/arthur/exts/grafana/team_sync.py b/arthur/exts/grafana/team_sync.py
index 501f891..169bef8 100644
--- a/arthur/exts/grafana/team_sync.py
+++ b/arthur/exts/grafana/team_sync.py
@@ -1,3 +1,5 @@
+from dataclasses import dataclass
+
 import aiohttp
 import discord
 from discord.ext import commands, tasks
@@ -7,6 +9,22 @@ from arthur.apis import github, grafana
 from arthur.bot import KingArthur
 
 
+@dataclass(frozen=True)
+class MissingMembers:
+    """Number of members that were missing from the Grafana team, and how many could be added."""
+
+    count: int
+    successfully_added: int
+
+
+@dataclass(frozen=True)
+class SyncFigures:
+    """Figures related to a single sync members task run."""
+
+    added: MissingMembers
+    removed: int
+
+
 class GrafanaTeamSync(commands.Cog):
     """
     Update Grafana team membership to match Github team membership.
@@ -19,22 +37,14 @@ class GrafanaTeamSync(commands.Cog):
         self.bot = bot
         self.sync_github_grafana_teams.start()
 
-    async def _sync_teams(self, team: dict[str, str]) -> tuple[int, int]:
+    async def _add_missing_members(
+        self, grafana_team_id: int, github_team_members: set[str], grafana_team_members: set[str]
+    ) -> MissingMembers:
         """
-        Ensure members in github are present in grafana teams.
+        Adds members to the Grafana team if they're in the Github team and not already present.
 
-        Return the number of members missing from teh grafana team, and the number of members added.
+        Returns the number of missing members, and the number of members it could actually add.
         """
-        github_team_members = {
-            member["login"]
-            for member in await github.list_team_members(team["name"], self.bot.http_session)
-        }
-        grafana_team_members = {
-            member["login"]
-            for member in await grafana.list_team_members(team["id"], self.bot.http_session)
-            if member.get("auth_module") == "oauth_github"
-        }
-
         missing_members = github_team_members - grafana_team_members
         grafana_users = await grafana.get_all_users(self.bot.http_session)
         added_members = set()
@@ -43,11 +53,36 @@ class GrafanaTeamSync(commands.Cog):
                 continue
             await grafana.add_user_to_team(
                 grafana_user["userId"],
-                team["id"],
+                grafana_team_id,
                 self.bot.http_session,
             )
             added_members.add(grafana_user["login"])
-        return len(missing_members), len(added_members)
+        return MissingMembers(count=len(missing_members), successfully_added=len(added_members))
+
+    async def _sync_teams(self, team: dict[str, str]) -> SyncFigures:
+        """
+        Ensure members in Github are present in Grafana teams.
+
+        Return the number of members missing from the Grafana team, and the number of members added.
+        """
+        github_team_members = {
+            member["login"]
+            for member in await github.list_team_members(team["name"], self.bot.http_session)
+        }
+        grafana_team_members = {
+            member["login"]
+            for member in await grafana.list_team_members(team["id"], self.bot.http_session)
+            if member.get("auth_module") == "oauth_github"
+        }
+
+        added_members = await self._add_missing_members(
+            team["id"],
+            github_team_members,
+            grafana_team_members,
+        )
+        removed_members = 0  # TODO Actually remove members who shouldn't be present.
+
+        return SyncFigures(added=added_members, removed=removed_members)
 
     @tasks.loop(hours=12)
     async def sync_github_grafana_teams(self, channel: discord.TextChannel | None = None) -> None:
@@ -56,16 +91,17 @@ class GrafanaTeamSync(commands.Cog):
         for team in grafana_teams:
             logger.debug(f"Processing {team['name']}")
             try:
-                missing, added = await self._sync_teams(team)
+                figures = await self._sync_teams(team)
             except aiohttp.ClientResponseError as e:
                 logger.error(e)
                 if channel:
                     await channel.send(e)
                 continue
 
-            if channel and missing:
+            if channel:
                 await channel.send(
-                    f"Found {missing} users not in the {team['name']} grafana team, added {added} of them."
+                    f"Found {figures.added.count} users not in the {team['name']} Grafana team, added {figures.added.successfully_added} of them. "
+                    f"{figures.removed} members were found in the Grafana team who shouldn't be, and were removed."
                 )
 
     @sync_github_grafana_teams.error