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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R6JZkyszyDFuFtk25WBbcR
This commit is contained in:
Rene Arumetsa
@@ -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"]
|
||||
|
||||
Reference in New Issue
Block a user