From 52002c37fc4d346054e57054fc94ef7d7bd1e54f Mon Sep 17 00:00:00 2001 From: Rene Arumetsa Date: Fri, 28 Aug 2026 14:01:22 +0300 Subject: [PATCH 1/2] feat(members): assign team roles from the tournament registration sheet Match members by Discord username against the lineup nicknames in the separate registration spreadsheet (TEAM_SHEET_ID) and give each their team's role. One team per person: switching teams removes the old team role, and a team with no Discord role yet is auto-created. Only role names present in the sheet are ever touched, so organisation/field/base roles are never at risk; the feature is a no-op when TEAM_SHEET_ID is unset. The sheet is not a single table (merged rows, stacked CS2/LoL sections with different layouts), so it is parsed via raw-row scanning rather than get_all_records. Citizenship markers like "(EST)" are used only as player delimiters and discarded. Co-Authored-By: Claude Opus 4.8 (1M context) --- bot.py | 6 + commands/dev_member_commands.py | 6 + config.py | 3 + core/member_sync.py | 37 +++++- core/sheets.py | 161 +++++++++++++++++++++++ strings/member.py | 1 + tests/test_team_sync.py | 225 ++++++++++++++++++++++++++++++++ 7 files changed, 438 insertions(+), 1 deletion(-) create mode 100644 tests/test_team_sync.py diff --git a/bot.py b/bot.py index b0f8a7e..241e1b6 100644 --- a/bot.py +++ b/bot.py @@ -420,6 +420,12 @@ async def on_ready(): log.info("Loaded %d member rows from Google Sheets", len(data)) except Exception as e: log.error("Failed to load sheet on startup: %s", e) + try: + rosters = await sheets.refresh_teams() + if rosters: + log.info("Loaded %d teams from the registration sheet", len(rosters)) + except Exception as e: + log.error("Failed to load team sheet on startup: %s", e) # Sync slash commands to the guild only; wipe any leftover global registrations tree.copy_global_to(guild=GUILD_OBJ) diff --git a/commands/dev_member_commands.py b/commands/dev_member_commands.py index 4cedbd7..c60d4e7 100644 --- a/commands/dev_member_commands.py +++ b/commands/dev_member_commands.py @@ -182,6 +182,10 @@ def register_dev_member_commands( except Exception as e: await interaction.followup.send(S.ERR["sheet_error"].format(error=e), ephemeral=True) return + try: + await sheets.refresh_teams() + except Exception as e: + log.warning("/check: team sheet refresh failed, using stale team cache: %s", e) ids_filled = 0 for row in data: @@ -233,6 +237,8 @@ def register_dev_member_commands( parts.append(S.CHECK_UI["detail_nickname"]) if result.roles_added: parts.append(S.CHECK_UI["detail_roles_added"].format(roles=", ".join(result.roles_added))) + if result.roles_removed: + parts.append(S.CHECK_UI["detail_roles_removed"].format(roles=", ".join(result.roles_removed))) details.append(S.CHECK_UI["detail_changed"].format(name=member.display_name, parts=", ".join(parts))) else: already_ok += 1 diff --git a/config.py b/config.py index be14bd8..37435ad 100644 --- a/config.py +++ b/config.py @@ -23,6 +23,9 @@ DISCORD_TOKEN = ( ) or _LEGACY_DISCORD_TOKEN SHEET_ID = os.getenv("SHEET_ID") +# Separate spreadsheet holding the tournament team registrations (Team Name + +# lineup of Discord usernames). Optional: when unset, team-role sync is a no-op. +TEAM_SHEET_ID = os.getenv("TEAM_SHEET_ID") GOOGLE_CREDS_PATH = os.getenv("GOOGLE_CREDS_PATH", "credentials.json") _LEGACY_GUILD_ID = _env_int("GUILD_ID", 0) diff --git a/core/member_sync.py b/core/member_sync.py index ff3dd69..47eb4a1 100644 --- a/core/member_sync.py +++ b/core/member_sync.py @@ -199,9 +199,35 @@ async def sync_member( else: result.errors.append(f"Baasrolli ID {rid} ei leitud serverist") + # --- Team role (from the separate registration spreadsheet) -------------- + # Matched by Discord username. One team per person: switching teams removes + # the previous team's role; a team with no Discord role yet is auto-created. + # Only role NAMES that appear in the team sheet are ever touched here, so + # organisation/field/base roles are never at risk. When TEAM_SHEET_ID is + # unset the caches are empty and this whole block is a no-op. + team_name = sheets.get_team_for_username(member.name) + all_teams = sheets.all_team_names() + if team_name: + team_role = discord.utils.get(guild.roles, name=team_name) + if team_role is None: + try: + team_role = await guild.create_role(name=team_name, reason="Team sync: uus tiim") + log.info("Created team role %r for %s", team_name, member) + except discord.Forbidden: + result.errors.append(f"Tiimirolli '{team_name}' loomiseks puudub õigus") + team_role = None + except discord.HTTPException as e: + result.errors.append(f"Tiimirolli '{team_name}' loomine ebaõnnestus: {e}") + team_role = None + if team_role is not None: + desired_roles.append(team_role) + + # Team roles the member has but is no longer registered for (left/switched). + to_remove = [r for r in member.roles if r.name in all_teams and r.name != team_name] + # Roles to add (desired but member doesn't have) to_add = [r for r in desired_roles if r not in member.roles] - # (we currently only ADD the desired roles, not remove extras - safe default) + # (outside of team roles we only ADD, never remove extras - safe default) if to_add: try: @@ -212,6 +238,15 @@ async def sync_member( except discord.HTTPException as e: result.errors.append(f"Rolli viga kasutajale {member}: {e}") + if to_remove: + try: + await member.remove_roles(*to_remove, reason="Team sync: tiim vahetus") + result.roles_removed = [r.name for r in to_remove] + except discord.Forbidden: + log.debug("No permission to remove roles for %s (likely admin), skipping", member) + except discord.HTTPException as e: + result.errors.append(f"Rolli eemaldamise viga kasutajale {member}: {e}") + # --- Birthday check --- birthday_str = str(row.get("Sünnipäev", "")).strip() if not _is_placeholder(birthday_str): diff --git a/core/sheets.py b/core/sheets.py index cdcbd28..530bc8e 100644 --- a/core/sheets.py +++ b/core/sheets.py @@ -8,6 +8,7 @@ Pure-cache helpers (get_cache, find_*) remain sync. import asyncio import logging +import re import gspread from google.oauth2.service_account import Credentials @@ -250,3 +251,163 @@ def _add_new_member_row_sync(username: str, discord_id: int) -> None: async def add_new_member_row(username: str, discord_id: int) -> None: """Append a new row pre-filled with Discord username and User ID (non-blocking).""" await asyncio.to_thread(_add_new_member_row_sync, username, discord_id) + + +# =========================================================================== +# Team registration sheet (a SEPARATE spreadsheet, config.TEAM_SHEET_ID) +# --------------------------------------------------------------------------- +# Unlike the member roster this sheet is NOT a single clean table: it stacks +# several game sections (CS2, LoL, ...) - each with merged title/description +# rows, its own header row, and a block of team rows - across one or more tabs. +# So we read raw cell values (get_all_values) and scan for header rows rather +# than relying on get_all_records, which requires one rectangular table. +# +# Each team's players live in one "Lineup" cell, comma-joined, every nickname +# suffixed with a citizenship marker like "(EST)". Those nicknames ARE the +# players' Discord usernames; the citizenship is used only as a delimiter +# (a nickname may itself contain commas) and then discarded. +# =========================================================================== + +# Matches a citizenship marker such as "(EST)" / "(LAT)". Used to split a +# lineup cell into individual players, then thrown away. +_CITIZENSHIP_RE = re.compile(r"\(\s*[A-Za-z]{2,4}\s*\)") + +_TEAM_NAME_HEADER = "team name" +_LINEUP_HEADER_PREFIX = "lineup" + +# Team-sheet caches (mirrors the member-roster cache above) +_team_roster: dict[str, list[str]] = {} # team name -> [Discord username, ...] +_team_by_username: dict[str, str] = {} # normalized username -> team name +_team_names: set[str] = set() # universe of all team names + + +def parse_lineup(cell: str) -> list[str]: + """Split one 'Lineup' cell into player nicknames (Discord usernames). + + Players are delimited by their trailing citizenship marker, e.g. + 'TFT (EST), nqmm (EST), sn1rk (EST)' -> ['TFT', 'nqmm', 'sn1rk'] + Splitting on the marker (not on commas) keeps nicknames that themselves + contain commas or semicolons intact. Falls back to comma-splitting when a + cell carries no citizenship markers at all. + """ + cell = str(cell).strip() + if not cell: + return [] + names: list[str] = [] + last = 0 + matched = False + for m in _CITIZENSHIP_RE.finditer(cell): + matched = True + chunk = cell[last:m.start()].strip().strip(",;").strip() + if chunk: + names.append(chunk) + last = m.end() + if not matched: + return [p.strip() for p in cell.split(",") if p.strip()] + tail = cell[last:].strip().strip(",;").strip() # stray name after last marker + if tail: + names.append(tail) + return names + + +def _find_col(row: list, matches) -> int | None: + for idx, cell in enumerate(row): + if matches(str(cell)): + return idx + return None + + +def _cell(row: list, idx: int) -> str: + return str(row[idx]) if 0 <= idx < len(row) else "" + + +def parse_team_rosters(rows: list[list]) -> dict[str, list[str]]: + """Extract {team_name: [nickname, ...]} from a tab's raw rows. + + Scans for every header row that has both a 'Team Name' and a 'Lineup...' + column, then reads the rows beneath it (using that section's own column + positions) until the team-name column goes blank or a new header appears. + Handles multiple stacked sections with differing layouts in one tab. + """ + rosters: dict[str, list[str]] = {} + i, n = 0, len(rows) + while i < n: + name_col = _find_col(rows[i], lambda c: c.strip().lower() == _TEAM_NAME_HEADER) + lineup_col = _find_col(rows[i], lambda c: c.strip().lower().startswith(_LINEUP_HEADER_PREFIX)) + if name_col is None or lineup_col is None: + i += 1 + continue + i += 1 # move past the header into the data block + while i < n: + team = _cell(rows[i], name_col).strip() + if not team or team.lower() == _TEAM_NAME_HEADER: + break # blank team-name (or a new header) ends this section + players = parse_lineup(_cell(rows[i], lineup_col)) + if players: + rosters.setdefault(team, []).extend(players) + i += 1 + return rosters + + +def build_username_index(rosters: dict[str, list[str]]) -> dict[str, str]: + """Invert {team: [usernames]} into {normalized username: team}. + + If the same username appears on two teams the last one wins and a warning + is logged (a person is expected to be on exactly one team). + """ + index: dict[str, str] = {} + for team, players in rosters.items(): + for player in players: + key = player.strip().lower() + if not key: + continue + if key in index and index[key] != team: + log.warning( + "Player %r appears on both %r and %r; using %r", + player, index[key], team, team, + ) + index[key] = team + return index + + +def _refresh_teams_sync() -> dict[str, list[str]]: + global _team_roster, _team_by_username, _team_names + creds = Credentials.from_service_account_file(config.GOOGLE_CREDS_PATH, scopes=SCOPES) + client = gspread.authorize(creds) + spreadsheet = client.open_by_key(config.TEAM_SHEET_ID) + + rosters: dict[str, list[str]] = {} + for ws in spreadsheet.worksheets(): + for team, players in parse_team_rosters(ws.get_all_values()).items(): + rosters.setdefault(team, []).extend(players) + + _team_roster = rosters + _team_by_username = build_username_index(rosters) + _team_names = set(rosters) + return rosters + + +async def refresh_teams() -> dict[str, list[str]]: + """Reload the team registration sheet into the in-memory team caches. + + No-op returning {} when TEAM_SHEET_ID is not configured, so the whole + feature can be left switched off without touching sync behaviour. + """ + if not config.TEAM_SHEET_ID: + return {} + return await asyncio.to_thread(_refresh_teams_sync) + + +def get_team_for_username(username: str) -> str | None: + """Return the team a Discord username is registered on, or None.""" + return _team_by_username.get(str(username).strip().lower()) + + +def all_team_names() -> set[str]: + """Every team name known from the registration sheet (the role universe).""" + return set(_team_names) + + +def get_team_rosters() -> dict[str, list[str]]: + """Current {team: [usernames]} cache (mainly for diagnostics/tests).""" + return _team_roster diff --git a/strings/member.py b/strings/member.py index edcfbbb..b86e3e5 100644 --- a/strings/member.py +++ b/strings/member.py @@ -91,6 +91,7 @@ CHECK_UI: dict[str, str] = { "detail_error": "⚠️ {error}", "detail_nickname": "hüüdnimi", "detail_roles_added": "+rollid: {roles}", + "detail_roles_removed": "-rollid: {roles}", "detail_changed": "🔧 **{name}**: {parts}", "ids_filled": "\n🔑 Täideti **{count}** puuduvat kasutaja ID-d.", } diff --git a/tests/test_team_sync.py b/tests/test_team_sync.py new file mode 100644 index 0000000..9c2b178 --- /dev/null +++ b/tests/test_team_sync.py @@ -0,0 +1,225 @@ +"""Tests for team-role sync from the tournament registration sheet. + +Covers the risky parsing (turning a messy, multi-section, merged-cell sheet +into {team: [discord usernames]}) and the add/remove/auto-create behaviour of +sync_member, using lightweight fakes for discord + the sheets cache. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).parent.parent)) + +import config # noqa: E402 +from core import member_sync, sheets # noqa: E402 +from tests.conftest import run # noqa: E402 + + +# --- parse_lineup ---------------------------------------------------------- + +def test_parse_lineup_strips_citizenship_and_splits(): + cell = "TFT (EST), nqmm (EST), sn1rk (EST), Kevka (EST), Kalatexx (EST)" + assert sheets.parse_lineup(cell) == ["TFT", "nqmm", "sn1rk", "Kevka", "Kalatexx"] + + +def test_parse_lineup_keeps_names_containing_commas_and_semicolons(): + # A single player's descriptive name contains a ';' - must stay one name. + cell = "Onu Klaus ; vahepeal ka mõni teine tegelane (EST), Lurban (EST)" + assert sheets.parse_lineup(cell) == [ + "Onu Klaus ; vahepeal ka mõni teine tegelane", + "Lurban", + ] + + +def test_parse_lineup_handles_odd_usernames(): + cell = "-acc +vac (EST), m (EST), M1X3RRRRRR (EST)" + assert sheets.parse_lineup(cell) == ["-acc +vac", "m", "M1X3RRRRRR"] + + +def test_parse_lineup_mixed_citizenship(): + cell = "imp (LAT), milteg (EST), Freesies (EST)" + assert sheets.parse_lineup(cell) == ["imp", "milteg", "Freesies"] + + +def test_parse_lineup_empty(): + assert sheets.parse_lineup("") == [] + assert sheets.parse_lineup(" ") == [] + + +def test_parse_lineup_fallback_without_citizenship(): + assert sheets.parse_lineup("alice, bob") == ["alice", "bob"] + + +# --- parse_team_rosters (multi-section sheet) ------------------------------ + +# Mirrors the real sheet: merged title rows, a header row, team rows, a blank +# separator, then a SECOND section with a different column count. +SHEET_ROWS = [ + ["", "", "", "", "", "", ""], + ["name", "members", "vrs ranking", "registration_date", "game", "", ""], + ["[merged] TipiLAN 2026 CS2 Registration Log"] + [""] * 6, + ["No", "Team Name", "Lineup (nickname, citizenship)", "VRS Ranking", + "Registration Timestamp", "Status", "Participation confirmed?"], + ["1", "Piirivalvurid", "TFT (EST), nqmm (EST)", "N/A", "01.05.2026 15:07", "Confirmed", "Yes"], + ["2", "GENESIS", "kapa (EST), neaQ (EST)", "N/A", "01.05.2026 15:24", "Confirmed", "Yes"], + ["", "", "", "", "", "", ""], + ["[merged] TipiLAN 2026 LoL Registration Log"] + [""] * 4, + ["No", "Team Name", "Lineup (nickname, citizenship)", + "Registration Timestamp (dd.mm.yyyy hh:mm)", "Confirmation Status"], + ["1", "Pushing 30s", "Onu Klaus (EST), Lurban (EST)", "01.05.2026 21:40", ""], + ["", "", "", "", ""], +] + + +def test_parse_team_rosters_multiple_sections(): + rosters = sheets.parse_team_rosters(SHEET_ROWS) + assert rosters == { + "Piirivalvurid": ["TFT", "nqmm"], + "GENESIS": ["kapa", "neaQ"], + "Pushing 30s": ["Onu Klaus", "Lurban"], + } + + +def test_parse_team_rosters_ignores_non_table_content(): + # No header row anywhere -> nothing extracted, no crash. + assert sheets.parse_team_rosters([["just", "some", "prose"], ["more"]]) == {} + + +def test_build_username_index_is_case_insensitive(): + index = sheets.build_username_index({"GENESIS": ["Kapa", "neaQ"]}) + assert index == {"kapa": "GENESIS", "neaq": "GENESIS"} + + +# --- sync_member team behaviour ------------------------------------------- + +class FakeRole: + def __init__(self, rid: int, name: str): + self.id = rid + self.name = name + + def __eq__(self, other): + return isinstance(other, FakeRole) and other.id == self.id + + def __hash__(self): + return hash(self.id) + + +class FakeMember: + def __init__(self, uid: int, name: str, roles): + self.id = uid + self.name = name + self.nick = None + self.roles = list(roles) + + async def edit(self, nick=None): + self.nick = nick + + async def add_roles(self, *roles, reason=None): + self.roles.extend(roles) + + async def remove_roles(self, *roles, reason=None): + self.roles = [r for r in self.roles if r not in roles] + + +class FakeGuild: + def __init__(self, roles): + self.roles = list(roles) + self._next = 9000 + self.created: list[str] = [] + + def get_role(self, rid): + return next((r for r in self.roles if r.id == rid), None) + + async def create_role(self, name, reason=None): + self._next += 1 + role = FakeRole(self._next, name) + self.roles.append(role) + self.created.append(name) + return role + + +@pytest.fixture +def team_env(monkeypatch): + """Base roles present in the guild; no organisation/field/birthday noise.""" + base_roles = [FakeRole(rid, f"base-{rid}") for rid in config.BASE_ROLE_IDS] + + def make_row(name): + # User ID + Discord match the member so no sheet writes fire; everything + # else is a placeholder so only the team role logic is exercised. + return { + "User ID": "1", "Discord": name, "Nimi": "-", + "Organisatsioon": "-", "Valdkond": "-", "Roll": "-", "Sünnipäev": "-", + } + + monkeypatch.setattr(sheets, "find_member", lambda uid, name: make_row(name)) + return base_roles + + +def test_sync_creates_missing_team_role_and_removes_old_one(team_env, monkeypatch): + old_team = FakeRole(1, "OldTeam") + keeper = FakeRole(2, "Member") # not a team role - must be left alone + member = FakeMember(1, "tft", roles=[old_team, keeper]) + guild = FakeGuild(team_env + [old_team, keeper]) + + monkeypatch.setattr(sheets, "get_team_for_username", + lambda n: "GENESIS" if n.lower() == "tft" else None) + monkeypatch.setattr(sheets, "all_team_names", lambda: {"GENESIS", "OldTeam"}) + + result = run(member_sync.sync_member(member, guild)) + + assert "GENESIS" in guild.created # auto-created the missing role + assert "GENESIS" in result.roles_added + assert result.roles_removed == ["OldTeam"] # left their previous team + role_names = {r.name for r in member.roles} + assert "GENESIS" in role_names + assert "OldTeam" not in role_names + assert "Member" in role_names # unrelated role untouched + + +def test_sync_uses_existing_team_role(team_env, monkeypatch): + genesis = FakeRole(3, "GENESIS") + member = FakeMember(1, "kapa", roles=[]) + guild = FakeGuild(team_env + [genesis]) + + monkeypatch.setattr(sheets, "get_team_for_username", lambda n: "GENESIS") + monkeypatch.setattr(sheets, "all_team_names", lambda: {"GENESIS"}) + + result = run(member_sync.sync_member(member, guild)) + + assert guild.created == [] # did NOT create a duplicate + assert "GENESIS" in result.roles_added + assert genesis in member.roles + + +def test_sync_strips_team_role_when_not_registered(team_env, monkeypatch): + old_team = FakeRole(1, "OldTeam") + member = FakeMember(1, "ghost", roles=[old_team]) + guild = FakeGuild(team_env + [old_team]) + + monkeypatch.setattr(sheets, "get_team_for_username", lambda n: None) + monkeypatch.setattr(sheets, "all_team_names", lambda: {"OldTeam"}) + + result = run(member_sync.sync_member(member, guild)) + + assert result.roles_removed == ["OldTeam"] + assert old_team not in member.roles + + +def test_sync_no_team_sheet_is_noop(team_env, monkeypatch): + keeper = FakeRole(2, "Member") + member = FakeMember(1, "someone", roles=[keeper]) + guild = FakeGuild(team_env + [keeper]) + + # Empty caches = feature switched off. + monkeypatch.setattr(sheets, "get_team_for_username", lambda n: None) + monkeypatch.setattr(sheets, "all_team_names", lambda: set()) + + result = run(member_sync.sync_member(member, guild)) + + assert result.roles_removed == [] + assert guild.created == [] + assert keeper in member.roles From ce1ed289043e1f063ef5e70be23c046ef7f81cc6 Mon Sep 17 00:00:00 2001 From: Rene Arumetsa Date: Fri, 28 Aug 2026 15:04:23 +0300 Subject: [PATCH 2/2] refactor(teams): move team-role sync to the economy/community bot The team-role feature was aimed at the wrong bot. Tournament participants live in the economy/community guild, but team-role sync had been bolted onto the dev bot's roster sync (sync_member), which bails out for anyone missing from the internal member sheet - so it could never reach its actual audience. Decouple it: keep the (roster-independent) team-sheet parsing, pull the team wiring off the dev/member-sync path, and re-home it on the economy bot. - core/member_sync: revert sync_member to add-only (drop team block + SyncResult.roles_removed); add roster-independent sync_team_role and a whole-guild sync_all_team_roles returning a reporting summary. Still only ever touches role NAMES present in the team sheet. - commands/economy_team_commands: new admin-only /teamsync command. - bot.py: hourly team_sync_hourly task (economy-only, no-op unless TEAM_SHEET_ID is set; first tick at boot covers startup load); register /teamsync under the economy profile; drop the dev-side startup load. - commands/dev_member_commands: /check no longer refreshes teams or reports removed roles. - strings + .env.example: TEAMSYNC_UI, CMD[teamsync], document TEAM_SHEET_ID. - tests: retarget sync tests to sync_team_role; add sync_all_team_roles case. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01R6JZkyszyDFuFtk25WBbcR --- .env.example | 5 + bot.py | 52 ++++++++-- commands/dev_member_commands.py | 6 -- commands/economy_team_commands.py | 87 ++++++++++++++++ core/member_sync.py | 159 +++++++++++++++++++++++------- strings/__init__.py | 2 + strings/commands.py | 1 + strings/member.py | 21 +++- tests/test_team_sync.py | 101 ++++++++++--------- 9 files changed, 331 insertions(+), 103 deletions(-) create mode 100644 commands/economy_team_commands.py diff --git a/.env.example b/.env.example index ca588c6..0851cee 100644 --- a/.env.example +++ b/.env.example @@ -9,6 +9,11 @@ DISCORD_TOKEN= # Google Sheets spreadsheet ID (the long string in the sheet URL) SHEET_ID=your-google-sheet-id-here +# Separate spreadsheet holding tournament team registrations (Team Name + lineup +# of Discord usernames). Optional; drives /teamsync + hourly team-role sync on +# the economy/community bot. Leave unset to disable team-role sync entirely. +TEAM_SHEET_ID= + # Path to Google service account credentials JSON GOOGLE_CREDS_PATH=credentials.json diff --git a/bot.py b/bot.py index 241e1b6..3411f44 100644 --- a/bot.py +++ b/bot.py @@ -23,7 +23,7 @@ import config import strings as S from core import economy, pb_client, sheets from core.admin import is_bot_admin -from core.member_sync import SyncResult +from core.member_sync import SyncResult, sync_all_team_roles from commands.dev_member_commands import register_dev_member_commands from commands.dev_member_runtime import handle_member_join, run_birthday_daily from commands.economy_admin_commands import register_economy_admin_commands @@ -35,6 +35,7 @@ from commands.economy_prestige_commands import register_prestige_commands from commands.economy_quests_commands import register_economy_quests_commands from commands.economy_profile_commands import register_economy_profile_commands from commands.economy_support_commands import register_economy_support_commands +from commands.economy_team_commands import register_economy_team_commands from commands.ops_channel_commands import register_ops_channel_commands from commands.ops_admin_commands import register_ops_admin_commands from commands.info_commands import register_info_commands @@ -336,6 +337,40 @@ async def before_birthday_daily(): await bot.wait_until_ready() +@tasks.loop(hours=1) +async def team_sync_hourly(): + """Reload the tournament registration sheet and re-apply team roles. + + Economy profile only (the tournament players live in the community guild). + Runs the first iteration immediately on start, so this also covers the + initial load at boot. No-op when TEAM_SHEET_ID is unset. + """ + if IS_DEV_PROFILE or not config.TEAM_SHEET_ID: + return + try: + rosters = await sheets.refresh_teams() + except Exception as e: + log.error("team_sync_hourly: failed to load team sheet: %s", e) + return + if not rosters: + return + guild = bot.get_guild(config.GUILD_ID) + if guild is None: + log.warning("team_sync_hourly: guild %s not found", config.GUILD_ID) + return + summary = await sync_all_team_roles(guild, log) + if summary.assigned or summary.removed or summary.created: + log.info( + "team_sync_hourly: assigned=%d, removed=%d, created=%d, errors=%d", + summary.assigned, summary.removed, len(summary.created), len(summary.errors), + ) + + +@team_sync_hourly.before_loop +async def before_team_sync_hourly(): + await bot.wait_until_ready() + + # --------------------------------------------------------------------------- # Rotating rich presence # --------------------------------------------------------------------------- @@ -420,12 +455,6 @@ async def on_ready(): log.info("Loaded %d member rows from Google Sheets", len(data)) except Exception as e: log.error("Failed to load sheet on startup: %s", e) - try: - rosters = await sheets.refresh_teams() - if rosters: - log.info("Loaded %d teams from the registration sheet", len(rosters)) - except Exception as e: - log.error("Failed to load team sheet on startup: %s", e) # Sync slash commands to the guild only; wipe any leftover global registrations tree.copy_global_to(guild=GUILD_OBJ) @@ -439,6 +468,11 @@ async def on_ready(): birthday_daily.start() log.info("Birthday daily task started (fires 09:00 Tallinn time)") + # Start hourly tournament team-role sync (economy/community guild) + if not IS_DEV_PROFILE and config.TEAM_SHEET_ID and not team_sync_hourly.is_running(): + team_sync_hourly.start() + log.info("Team-role sync task started (hourly, from the registration sheet)") + # Start rotating rich presence if not _rotate_presence.is_running(): _rotate_presence.start() @@ -496,6 +530,10 @@ if IS_DEV_PROFILE: has_announced_today=_has_announced_today, mark_announced_today=_mark_announced_today, ) +else: + # Tournament team-role sync lives on the economy/community bot, where the + # registered players actually are (see commands/economy_team_commands.py). + register_economy_team_commands(tree, bot, log) register_ops_admin_commands( tree, diff --git a/commands/dev_member_commands.py b/commands/dev_member_commands.py index c60d4e7..4cedbd7 100644 --- a/commands/dev_member_commands.py +++ b/commands/dev_member_commands.py @@ -182,10 +182,6 @@ def register_dev_member_commands( except Exception as e: await interaction.followup.send(S.ERR["sheet_error"].format(error=e), ephemeral=True) return - try: - await sheets.refresh_teams() - except Exception as e: - log.warning("/check: team sheet refresh failed, using stale team cache: %s", e) ids_filled = 0 for row in data: @@ -237,8 +233,6 @@ def register_dev_member_commands( parts.append(S.CHECK_UI["detail_nickname"]) if result.roles_added: parts.append(S.CHECK_UI["detail_roles_added"].format(roles=", ".join(result.roles_added))) - if result.roles_removed: - parts.append(S.CHECK_UI["detail_roles_removed"].format(roles=", ".join(result.roles_removed))) details.append(S.CHECK_UI["detail_changed"].format(name=member.display_name, parts=", ".join(parts))) else: already_ok += 1 diff --git a/commands/economy_team_commands.py b/commands/economy_team_commands.py new file mode 100644 index 0000000..6559c7b --- /dev/null +++ b/commands/economy_team_commands.py @@ -0,0 +1,87 @@ +"""Tournament team-role sync for the economy/community guild. + +The tournament participants live in the *economy* (community) guild, not the +internal dev guild, so team-role assignment runs here rather than as part of the +member-roster sync in :mod:`commands.dev_member_commands`. This is deliberately +roster-INDEPENDENT: it matches Discord usernames straight against the separate +registration spreadsheet (``TEAM_SHEET_ID``) and never touches the member sheet. +""" + +from __future__ import annotations + +import logging + +import discord +from discord import app_commands + +from core import sheets +from core.admin import bot_admin_check +from core.member_sync import sync_all_team_roles +import strings as S + + +def register_economy_team_commands( + tree: app_commands.CommandTree, + bot: discord.Client, + log: logging.Logger, +) -> None: + @tree.command(name="teamsync", description=S.CMD["teamsync"]) + @app_commands.guild_only() + @bot_admin_check() + async def cmd_teamsync(interaction: discord.Interaction): + await interaction.response.defer(ephemeral=True) + + guild = interaction.guild + if guild is None: + await interaction.followup.send(S.ERR["guild_only"], ephemeral=True) + return + + try: + rosters = await sheets.refresh_teams() + except Exception as e: + await interaction.followup.send( + S.TEAMSYNC_UI["refresh_error"].format(error=e), ephemeral=True + ) + return + if not rosters: + await interaction.followup.send(S.TEAMSYNC_UI["disabled"], ephemeral=True) + return + + summary = await sync_all_team_roles(guild, log) + await interaction.followup.send(_format_summary(summary), ephemeral=True) + log.info( + "/teamsync - scanned=%d, assigned=%d, removed=%d, created=%d, errors=%d", + summary.scanned, + summary.assigned, + summary.removed, + len(summary.created), + len(summary.errors), + ) + + +def _format_summary(summary) -> str: + lines = [ + S.TEAMSYNC_UI["done"], + S.TEAMSYNC_UI["scanned"].format(count=summary.scanned), + S.TEAMSYNC_UI["assigned"].format(count=summary.assigned), + S.TEAMSYNC_UI["removed"].format(count=summary.removed), + ] + if summary.created: + # A team can be created only once, but the same role could surface for + # several members in one run - de-dupe for the report. + unique = list(dict.fromkeys(summary.created)) + lines.append(S.TEAMSYNC_UI["created"].format(roles=", ".join(unique))) + if summary.errors: + lines.append(S.TEAMSYNC_UI["errors"].format(count=len(summary.errors))) + + text = "\n".join(lines) + + if summary.changes: + shown = summary.changes[:20] + text += "\n\n" + S.TEAMSYNC_UI["changes_header"] + "\n" + "\n".join(shown) + if len(summary.changes) > 20: + text += "\n" + S.TEAMSYNC_UI["changes_more"].format(count=len(summary.changes) - 20) + else: + text += "\n\n" + S.TEAMSYNC_UI["no_changes"] + + return text diff --git a/core/member_sync.py b/core/member_sync.py index 47eb4a1..0c89117 100644 --- a/core/member_sync.py +++ b/core/member_sync.py @@ -48,7 +48,6 @@ class SyncResult: """Tracks what happened during a sync operation.""" nickname_changed: bool = False roles_added: list[str] = field(default_factory=list) - roles_removed: list[str] = field(default_factory=list) birthday_soon: bool = False birthday_today: bool = False not_found: bool = False @@ -57,7 +56,31 @@ class SyncResult: @property def changed(self) -> bool: - return self.nickname_changed or self.roles_added or self.roles_removed + return self.nickname_changed or self.roles_added + + +@dataclass +class TeamSyncResult: + """What happened when syncing one member's tournament team role.""" + added: str | None = None # team role name granted, if any + removed: list[str] = field(default_factory=list) # stale team roles taken away + created: str | None = None # team role name auto-created in the guild, if any + errors: list[str] = field(default_factory=list) + + @property + def changed(self) -> bool: + return bool(self.added or self.removed) + + +@dataclass +class TeamSyncSummary: + """Aggregate outcome of a whole-guild team-role sync.""" + scanned: int = 0 + assigned: int = 0 + removed: int = 0 + created: list[str] = field(default_factory=list) + changes: list[str] = field(default_factory=list) # human-readable per-member lines + errors: list[str] = field(default_factory=list) def _format_nickname(full_name: str) -> str: @@ -199,35 +222,9 @@ async def sync_member( else: result.errors.append(f"Baasrolli ID {rid} ei leitud serverist") - # --- Team role (from the separate registration spreadsheet) -------------- - # Matched by Discord username. One team per person: switching teams removes - # the previous team's role; a team with no Discord role yet is auto-created. - # Only role NAMES that appear in the team sheet are ever touched here, so - # organisation/field/base roles are never at risk. When TEAM_SHEET_ID is - # unset the caches are empty and this whole block is a no-op. - team_name = sheets.get_team_for_username(member.name) - all_teams = sheets.all_team_names() - if team_name: - team_role = discord.utils.get(guild.roles, name=team_name) - if team_role is None: - try: - team_role = await guild.create_role(name=team_name, reason="Team sync: uus tiim") - log.info("Created team role %r for %s", team_name, member) - except discord.Forbidden: - result.errors.append(f"Tiimirolli '{team_name}' loomiseks puudub õigus") - team_role = None - except discord.HTTPException as e: - result.errors.append(f"Tiimirolli '{team_name}' loomine ebaõnnestus: {e}") - team_role = None - if team_role is not None: - desired_roles.append(team_role) - - # Team roles the member has but is no longer registered for (left/switched). - to_remove = [r for r in member.roles if r.name in all_teams and r.name != team_name] - # Roles to add (desired but member doesn't have) to_add = [r for r in desired_roles if r not in member.roles] - # (outside of team roles we only ADD, never remove extras - safe default) + # (we currently only ADD the desired roles, not remove extras - safe default) if to_add: try: @@ -238,15 +235,6 @@ async def sync_member( except discord.HTTPException as e: result.errors.append(f"Rolli viga kasutajale {member}: {e}") - if to_remove: - try: - await member.remove_roles(*to_remove, reason="Team sync: tiim vahetus") - result.roles_removed = [r.name for r in to_remove] - except discord.Forbidden: - log.debug("No permission to remove roles for %s (likely admin), skipping", member) - except discord.HTTPException as e: - result.errors.append(f"Rolli eemaldamise viga kasutajale {member}: {e}") - # --- Birthday check --- birthday_str = str(row.get("Sünnipäev", "")).strip() if not _is_placeholder(birthday_str): @@ -259,6 +247,101 @@ async def sync_member( return result +async def sync_team_role( + member: discord.Member, + guild: discord.Guild, +) -> TeamSyncResult: + """Give one member their tournament team role from the registration sheet. + + Roster-INDEPENDENT: unlike :func:`sync_member` this does not touch the + internal member sheet at all. It matches the member's Discord username + against the team sheet caches (populated by ``sheets.refresh_teams``) and: + + * grants the role for the team they're registered on (auto-creating that + role in the guild when it does not exist yet); + * removes any *other* team role they still carry (left / switched teams). + + Only role NAMES present in the team sheet are ever added or removed, so no + unrelated role is ever at risk. When ``TEAM_SHEET_ID`` is unset the caches + are empty and this is a no-op returning an unchanged result. + """ + result = TeamSyncResult() + + team_name = sheets.get_team_for_username(member.name) + all_teams = sheets.all_team_names() + if not all_teams: + return result # feature switched off (no team sheet loaded) + + desired: discord.Role | None = None + if team_name: + desired = discord.utils.get(guild.roles, name=team_name) + if desired is None: + try: + desired = await guild.create_role(name=team_name, reason="Team sync: uus tiim") + result.created = team_name + log.info("Created team role %r for %s", team_name, member) + except discord.Forbidden: + result.errors.append(f"Tiimirolli '{team_name}' loomiseks puudub õigus") + except discord.HTTPException as e: + result.errors.append(f"Tiimirolli '{team_name}' loomine ebaõnnestus: {e}") + + # Team roles held but no longer registered for (switched teams / dropped out). + to_remove = [r for r in member.roles if r.name in all_teams and r.name != team_name] + + if desired is not None and desired not in member.roles: + try: + await member.add_roles(desired, reason="Team sync") + result.added = desired.name + except discord.Forbidden: + log.debug("No permission to add team role for %s, skipping", member) + except discord.HTTPException as e: + result.errors.append(f"Tiimirolli viga kasutajale {member}: {e}") + + if to_remove: + try: + await member.remove_roles(*to_remove, reason="Team sync: tiim vahetus") + result.removed = [r.name for r in to_remove] + except discord.Forbidden: + log.debug("No permission to remove team roles for %s, skipping", member) + except discord.HTTPException as e: + result.errors.append(f"Tiimirolli eemaldamise viga kasutajale {member}: {e}") + + return result + + +async def sync_all_team_roles( + guild: discord.Guild, + log: logging.Logger = log, +) -> TeamSyncSummary: + """Run :func:`sync_team_role` for every human member of ``guild``. + + Assumes the team caches are already fresh (caller runs ``refresh_teams`` + first). Returns an aggregate summary for reporting. + """ + summary = TeamSyncSummary() + for member in guild.members: + if member.bot: + continue + summary.scanned += 1 + res = await sync_team_role(member, guild) + if res.created: + summary.created.append(res.created) + if res.errors: + summary.errors.extend(res.errors) + if res.added: + summary.assigned += 1 + if res.removed: + summary.removed += len(res.removed) + if res.changed: + bits: list[str] = [] + if res.added: + bits.append(f"+{res.added}") + if res.removed: + bits.append("-" + ", -".join(res.removed)) + summary.changes.append(f"{member.display_name}: {', '.join(bits)}") + return summary + + async def announce_birthday( member: discord.Member, bot: discord.Client, diff --git a/strings/__init__.py b/strings/__init__.py index 4c65c3a..7841026 100644 --- a/strings/__init__.py +++ b/strings/__init__.py @@ -44,6 +44,7 @@ from .member import ( BIRTHDAY_UI, BIRTHDAY_MONTHS, CHECK_UI, + TEAMSYNC_UI, ) from .economy import ( @@ -139,6 +140,7 @@ __all__ = [ 'BIRTHDAY_UI', 'BIRTHDAY_MONTHS', 'CHECK_UI', + 'TEAMSYNC_UI', 'WORK_JOBS', 'BEG_LINES', 'BEG_JAIL_LINES', diff --git a/strings/commands.py b/strings/commands.py index e3a9a28..7c9ce03 100644 --- a/strings/commands.py +++ b/strings/commands.py @@ -24,6 +24,7 @@ CMD: dict[str, str] = { "check": "Laadi andmed, täida ID'd ja sünkroniseeri kõik liikmed", "sync": "Sünkroniseeri käsklused Discordi serveriga", "member": "Näita liikme andmeid tabelist", + "teamsync": "[Admin] Sünkroniseeri tiimirollid registreerimistabelist", "restart": "Tee taaskäivitus botile", "shutdown": "Lülita bot välja (ilma taaskäivituseta)", "pause": "Peata / jätka kõik käsklused (hooldusrežiim)", diff --git a/strings/member.py b/strings/member.py index b86e3e5..a604887 100644 --- a/strings/member.py +++ b/strings/member.py @@ -11,6 +11,7 @@ __all__ = [ 'BIRTHDAY_UI', 'BIRTHDAY_MONTHS', 'CHECK_UI', + 'TEAMSYNC_UI', ] # --------------------------------------------------------------------------- @@ -91,7 +92,25 @@ CHECK_UI: dict[str, str] = { "detail_error": "⚠️ {error}", "detail_nickname": "hüüdnimi", "detail_roles_added": "+rollid: {roles}", - "detail_roles_removed": "-rollid: {roles}", "detail_changed": "🔧 **{name}**: {parts}", "ids_filled": "\n🔑 Täideti **{count}** puuduvat kasutaja ID-d.", } + + +# --------------------------------------------------------------------------- +# /teamsync UI strings (tournament team-role sync from the registration sheet) +# --------------------------------------------------------------------------- + +TEAMSYNC_UI: dict[str, str] = { + "disabled": "⚠️ Tiimide sünkroonimine on välja lülitatud (TEAM_SHEET_ID puudub).", + "refresh_error": "⚠️ Registreerimistabeli laadimine ebaõnnestus: {error}", + "done": "**Tiimide sünkroonimine lõpetatud!**", + "scanned": "👥 Kontrollitud liikmeid: {count}", + "assigned": "✅ Tiimirolle antud: {count}", + "removed": "➖ Tiimirolle eemaldatud: {count}", + "created": "🆕 Loodud uusi tiimirolle: {roles}", + "errors": "⚠️ Vead: {count}", + "no_changes": "✨ Kõik tiimirollid olid juba korras.", + "changes_header": "**Muudatused:**", + "changes_more": "... ja {count} rohkem", +} diff --git a/tests/test_team_sync.py b/tests/test_team_sync.py index 9c2b178..85f781e 100644 --- a/tests/test_team_sync.py +++ b/tests/test_team_sync.py @@ -1,8 +1,9 @@ """Tests for team-role sync from the tournament registration sheet. Covers the risky parsing (turning a messy, multi-section, merged-cell sheet -into {team: [discord usernames]}) and the add/remove/auto-create behaviour of -sync_member, using lightweight fakes for discord + the sheets cache. +into {team: [discord usernames]}) and the roster-independent add/remove/ +auto-create behaviour of sync_team_role, using lightweight fakes for discord ++ the sheets cache. """ from __future__ import annotations @@ -10,11 +11,8 @@ from __future__ import annotations import sys from pathlib import Path -import pytest - sys.path.insert(0, str(Path(__file__).parent.parent)) -import config # noqa: E402 from core import member_sync, sheets # noqa: E402 from tests.conftest import run # noqa: E402 @@ -94,7 +92,7 @@ def test_build_username_index_is_case_insensitive(): assert index == {"kapa": "GENESIS", "neaq": "GENESIS"} -# --- sync_member team behaviour ------------------------------------------- +# --- sync_team_role behaviour (roster-independent) ------------------------- class FakeRole: def __init__(self, rid: int, name: str): @@ -109,15 +107,13 @@ class FakeRole: class FakeMember: - def __init__(self, uid: int, name: str, roles): + def __init__(self, uid: int, name: str, roles, bot: bool = False): self.id = uid self.name = name - self.nick = None + self.display_name = name + self.bot = bot self.roles = list(roles) - async def edit(self, nick=None): - self.nick = nick - async def add_roles(self, *roles, reason=None): self.roles.extend(roles) @@ -126,14 +122,12 @@ class FakeMember: class FakeGuild: - def __init__(self, roles): + def __init__(self, roles, members=None): self.roles = list(roles) + self.members = list(members or []) self._next = 9000 self.created: list[str] = [] - def get_role(self, rid): - return next((r for r in self.roles if r.id == rid), None) - async def create_role(self, name, reason=None): self._next += 1 role = FakeRole(self._next, name) @@ -142,84 +136,89 @@ class FakeGuild: return role -@pytest.fixture -def team_env(monkeypatch): - """Base roles present in the guild; no organisation/field/birthday noise.""" - base_roles = [FakeRole(rid, f"base-{rid}") for rid in config.BASE_ROLE_IDS] - - def make_row(name): - # User ID + Discord match the member so no sheet writes fire; everything - # else is a placeholder so only the team role logic is exercised. - return { - "User ID": "1", "Discord": name, "Nimi": "-", - "Organisatsioon": "-", "Valdkond": "-", "Roll": "-", "Sünnipäev": "-", - } - - monkeypatch.setattr(sheets, "find_member", lambda uid, name: make_row(name)) - return base_roles - - -def test_sync_creates_missing_team_role_and_removes_old_one(team_env, monkeypatch): +def test_sync_creates_missing_team_role_and_removes_old_one(monkeypatch): old_team = FakeRole(1, "OldTeam") keeper = FakeRole(2, "Member") # not a team role - must be left alone member = FakeMember(1, "tft", roles=[old_team, keeper]) - guild = FakeGuild(team_env + [old_team, keeper]) + guild = FakeGuild([old_team, keeper]) monkeypatch.setattr(sheets, "get_team_for_username", lambda n: "GENESIS" if n.lower() == "tft" else None) monkeypatch.setattr(sheets, "all_team_names", lambda: {"GENESIS", "OldTeam"}) - result = run(member_sync.sync_member(member, guild)) + result = run(member_sync.sync_team_role(member, guild)) - assert "GENESIS" in guild.created # auto-created the missing role - assert "GENESIS" in result.roles_added - assert result.roles_removed == ["OldTeam"] # left their previous team + assert result.created == "GENESIS" # auto-created the missing role + assert "GENESIS" in guild.created + assert result.added == "GENESIS" + assert result.removed == ["OldTeam"] # left their previous team role_names = {r.name for r in member.roles} assert "GENESIS" in role_names assert "OldTeam" not in role_names assert "Member" in role_names # unrelated role untouched -def test_sync_uses_existing_team_role(team_env, monkeypatch): +def test_sync_uses_existing_team_role(monkeypatch): genesis = FakeRole(3, "GENESIS") member = FakeMember(1, "kapa", roles=[]) - guild = FakeGuild(team_env + [genesis]) + guild = FakeGuild([genesis]) monkeypatch.setattr(sheets, "get_team_for_username", lambda n: "GENESIS") monkeypatch.setattr(sheets, "all_team_names", lambda: {"GENESIS"}) - result = run(member_sync.sync_member(member, guild)) + result = run(member_sync.sync_team_role(member, guild)) assert guild.created == [] # did NOT create a duplicate - assert "GENESIS" in result.roles_added + assert result.created is None + assert result.added == "GENESIS" assert genesis in member.roles -def test_sync_strips_team_role_when_not_registered(team_env, monkeypatch): +def test_sync_strips_team_role_when_not_registered(monkeypatch): old_team = FakeRole(1, "OldTeam") member = FakeMember(1, "ghost", roles=[old_team]) - guild = FakeGuild(team_env + [old_team]) + guild = FakeGuild([old_team]) monkeypatch.setattr(sheets, "get_team_for_username", lambda n: None) monkeypatch.setattr(sheets, "all_team_names", lambda: {"OldTeam"}) - result = run(member_sync.sync_member(member, guild)) + result = run(member_sync.sync_team_role(member, guild)) - assert result.roles_removed == ["OldTeam"] + assert result.removed == ["OldTeam"] + assert result.added is None assert old_team not in member.roles -def test_sync_no_team_sheet_is_noop(team_env, monkeypatch): +def test_sync_no_team_sheet_is_noop(monkeypatch): keeper = FakeRole(2, "Member") member = FakeMember(1, "someone", roles=[keeper]) - guild = FakeGuild(team_env + [keeper]) + guild = FakeGuild([keeper]) - # Empty caches = feature switched off. + # Empty caches = feature switched off: no removals even of a stale team role. monkeypatch.setattr(sheets, "get_team_for_username", lambda n: None) monkeypatch.setattr(sheets, "all_team_names", lambda: set()) - result = run(member_sync.sync_member(member, guild)) + result = run(member_sync.sync_team_role(member, guild)) - assert result.roles_removed == [] + assert result.removed == [] assert guild.created == [] assert keeper in member.roles + + +def test_sync_all_team_roles_aggregates_and_skips_bots(monkeypatch): + genesis = FakeRole(3, "GENESIS") + m1 = FakeMember(1, "kapa", roles=[]) # will get GENESIS + m2 = FakeMember(2, "nobody", roles=[]) # not registered, unchanged + bot_member = FakeMember(3, "botto", roles=[], bot=True) # skipped + guild = FakeGuild([genesis], members=[m1, m2, bot_member]) + + teams = {"kapa": "GENESIS"} + monkeypatch.setattr(sheets, "get_team_for_username", lambda n: teams.get(n.lower())) + monkeypatch.setattr(sheets, "all_team_names", lambda: {"GENESIS"}) + + summary = run(member_sync.sync_all_team_roles(guild)) + + assert summary.scanned == 2 # bot excluded + assert summary.assigned == 1 + assert summary.removed == 0 + assert summary.changes == ["kapa: +GENESIS"]