Merge pull request 'Discord picker: per-bot permission status + owner's preferred bot (#225 S4) — merge after S3' (#250) from feat/225-s4-picker-ui into main

drb-c2-core/app/routers/discord_checks.py

@@ -87,6 +87,16 @@ def _channel_state(rows: list[dict], enabled: bool) -> str:
     return "no_bot"
 
 
+def _checked_at(snaps: dict) -> str:
+    """When Discord was last actually asked: the OLDEST cached snapshot behind
+    this answer (a denied result is cached for 10 min), so the picker's
+    "Permissions checked N ago" never calls a stale answer fresh (server-26#225
+    S4). No snapshot (check off, no member bots) = now."""
+    times = [s.fetched_at for s in snaps.values() if s is not None and getattr(s, "fetched_at", None)]
+    ts = min(times) if times else time.time()
+    return datetime.fromtimestamp(ts, timezone.utc).isoformat()
+
+
 async def voice_check(*, guild_id: str, org_id: Optional[str], node_id: Optional[str],
                       channels: list[dict], channel_id: Optional[str], refresh: bool,
                       with_token_ids: bool) -> dict:
@@ -171,7 +181,7 @@ async def voice_check(*, guild_id: str, org_id: Optional[str], node_id: Optional
         "guild_id": guild_id,
         "node_id": node_id,
         "enabled": enabled,
-        "checked_at": datetime.now(timezone.utc).isoformat(),
+        "checked_at": _checked_at(snaps),
         "refreshed": refresh,
         "channels": out_channels,
         "invite": invite,

drb-c2-core/app/routers/tokens.py

@@ -1,5 +1,5 @@
 import uuid
-from fastapi import APIRouter, HTTPException, Depends
+from fastapi import APIRouter, HTTPException, Depends, Query
 from pydantic import BaseModel
 from typing import Optional
 from app.internal import firestore as fstore
@@ -12,6 +12,7 @@ from app.internal.auth import (
     get_role,
     require_admin_token,
     require_firebase_token,
+    resolve_org_scope,
 )
 from app.internal.logger import logger
 
@@ -101,6 +102,16 @@ async def flush_tokens(_: dict = Depends(require_admin_token)):
     return {"released": released}
 
 
+def _forget_cached_system(system_id: str) -> None:
+    """start_stream reads systems through fstore.doc_get_cached (5 min TTL):
+    drop the entry so a new preference applies to the next start instead of
+    up to five minutes later (server-26#225 S4)."""
+    cache = getattr(fstore, "_doc_cache", None)
+    path = getattr(fstore, "_path", None)
+    if isinstance(cache, dict) and callable(path):
+        cache.pop(f"{path('systems')}/{system_id}", None)
+
+
 def _may_edit_system(decoded: dict, system: Optional[dict]) -> bool:
     """Platform admin: any system. Org owner: only their own org's systems —
     a shared system (no org_id) serves every org, so a preference there is
@@ -167,6 +178,7 @@ async def set_preferred_system(
                 raise HTTPException(403, "That system is shared by every organization; only a platform admin "
                                          "can change its radio bot.")
             await fstore.doc_set("systems", old_id, {"preferred_token_id": None})
+            _forget_cached_system(old_id)
         await fstore.doc_set("bot_tokens", token_id, {"preferred_for_system_id": None})
         await write_audit(uid, email, "discord.token.prefer", details={
             "token_id": token_id, "system_id": None, "previous_system_id": old_id or None})
@@ -183,6 +195,7 @@ async def set_preferred_system(
         old_sys = await fstore.doc_get("systems", old_sys_id)
         if old_sys and old_sys.get("preferred_token_id") == token_id and _may_edit_system(decoded, old_sys):
             await fstore.doc_set("systems", old_sys_id, {"preferred_token_id": None})
+            _forget_cached_system(old_sys_id)
     old_tok_id = system_doc.get("preferred_token_id")
     if old_tok_id and old_tok_id != token_id:
         old_tok = await fstore.doc_get("bot_tokens", old_tok_id)
@@ -190,6 +203,7 @@ async def set_preferred_system(
             await fstore.doc_set("bot_tokens", old_tok_id, {"preferred_for_system_id": None})
 
     await fstore.doc_set("systems", system_id, {"preferred_token_id": token_id})
+    _forget_cached_system(system_id)
     await fstore.doc_set("bot_tokens", token_id, {"preferred_for_system_id": system_id})
     await write_audit(uid, email, "discord.token.prefer", details={
         "token_id": token_id, "system_id": system_id, "previous_token_id": old_tok_id,
@@ -197,6 +211,52 @@ async def set_preferred_system(
     return {"ok": True, "preferred_for_system_id": system_id}
 
 
+@router.get("/org/preferred")
+async def org_preferred_bots(
+    org_id: Optional[str] = Query(None, description="Platform-admin only: act for this org."),
+    decoded: dict = Depends(require_firebase_token),
+):
+    """What an org owner needs to choose a preferred radio bot per system
+    (server-26#225 S4, owner decision Q2): the org's own systems, each with
+    its preferred bot, and the org's own bring-your-own bots — the only bots
+    an owner may choose (PUT /tokens/{id}/prefer/{system_id}). GET /tokens
+    stays platform-admin only; this returns no pool bots and no token strings.
+    A pool bot a platform admin chose for one of these systems is flagged,
+    not named."""
+    if get_role(decoded) != "admin" and get_org_role(decoded) != "owner":
+        raise HTTPException(403, "Only the organization owner or a platform admin can choose a system's radio bot.")
+    org = resolve_org_scope(decoded, org_id)
+    systems = sorted(await fstore.collection_list("systems", org_id=org),
+                     key=lambda s: str(s.get("name") or s.get("system_id") or "").lower())
+    bots = sorted((t for t in await fstore.collection_list("bot_tokens", org_id=org) if t.get("token_id")),
+                  key=lambda t: str(t.get("name") or "").lower())
+    names = {t["token_id"]: t.get("name") or t["token_id"] for t in bots}
+    out_systems = []
+    for s in systems:
+        if not s.get("system_id"):
+            continue
+        pref = s.get("preferred_token_id")
+        out_systems.append({
+            "system_id": s["system_id"],
+            "name": s.get("name") or s["system_id"],
+            "preferred_token_id": pref if pref in names else None,
+            "preferred_is_platform_bot": bool(pref) and pref not in names,
+            "preferred_name": names.get(pref) if pref in names else None,
+        })
+    return {
+        "org_id": org,
+        "systems": out_systems,
+        "bots": [{
+            "token_id": t["token_id"],
+            "name": names[t["token_id"]],
+            "valid": t.get("valid"),
+            "server_count": len(t["guild_ids"]) if isinstance(t.get("guild_ids"), list) else None,
+            "preferred_for_system_id": t.get("preferred_for_system_id"),
+            "invite_url": leases.invite_url(t.get("application_id")),
+        } for t in bots],
+    }
+
+
 @router.delete("/{token_id}", status_code=204)
 async def delete_token(token_id: str, _: dict = Depends(require_admin_token)):
     existing = await fstore.doc_get("bot_tokens", token_id)

drb-c2-core/tests/test_tokens_org_preferred.py

@@ -0,0 +1,113 @@
+"""
+Org-owner "radio bot per system" card (server-26#225 S4, owner decision Q2):
+GET /tokens/org/preferred lists the owner's own systems and own BYO bots (no
+pool bots, no token strings); a preference set through
+PUT /tokens/{id}/prefer/{system} (S3) applies to the very next start, not after
+the 5-minute systems cache.
+"""
+import pytest
+from fastapi.testclient import TestClient
+
+from app.internal import firestore as fstore
+from app.internal.auth import require_firebase_token, require_service_or_firebase_token
+from app.main import app
+from tests.fake_firestore import FakeFirestore
+
+client = TestClient(app)
+ORG, OTHER = "org-a", "org-b"
+OWNER = {"uid": "u-own", "org_id": ORG, "org_role": "owner"}
+MEMBER = {"uid": "u-mem", "org_id": ORG, "org_role": "member", "role": "operator"}
+STRANGER = {"uid": "u-x", "org_id": OTHER, "org_role": "owner"}
+ADMIN_NO_ORG = {"uid": "u-admin", "role": "admin"}
+
+
+@pytest.fixture
+def fs(monkeypatch):
+    fake = FakeFirestore.install(monkeypatch)
+    fake.put("systems", "s-a", {"system_id": "s-a", "name": "County P25", "org_id": ORG})
+    fake.put("systems", "s-a2", {"system_id": "s-a2", "name": "City Fire", "org_id": ORG})
+    fake.put("systems", "s-b", {"system_id": "s-b", "name": "Other County", "org_id": OTHER})
+    fake.put("systems", "s-shared", {"system_id": "s-shared", "name": "Shared", "org_id": None})
+    fake.put("bot_tokens", "byo-a", {"token_id": "byo-a", "name": "Alpha Radio", "token": "secret-a",
+                                     "org_id": ORG, "valid": True, "guild_ids": ["100", "101"],
+                                     "application_id": "555"})
+    fake.put("bot_tokens", "byo-a-bad", {"token_id": "byo-a-bad", "name": "Broken", "token": "secret-bad",
+                                         "org_id": ORG, "valid": False})
+    fake.put("bot_tokens", "byo-b", {"token_id": "byo-b", "name": "Bravo Radio", "token": "secret-b",
+                                     "org_id": OTHER, "valid": True})
+    fake.put("bot_tokens", "pool-1", {"token_id": "pool-1", "name": "DRB Radio 1", "token": "secret-p",
+                                      "org_id": None, "valid": True})
+    yield fake
+    app.dependency_overrides.clear()
+
+
+def as_(who):
+    # /tokens is mounted behind require_service_or_firebase_token (main.py).
+    app.dependency_overrides[require_service_or_firebase_token] = lambda: who
+    app.dependency_overrides[require_firebase_token] = lambda: who
+
+
+def test_owner_lists_own_systems_and_own_bots_only(fs):
+    as_(OWNER)
+    r = client.get("/tokens/org/preferred")
+    assert r.status_code == 200, r.text
+    body = r.json()
+    assert body["org_id"] == ORG
+    assert [s["system_id"] for s in body["systems"]] == ["s-a2", "s-a"]       # own systems, by name
+    assert {b["token_id"] for b in body["bots"]} == {"byo-a", "byo-a-bad"}   # no pool, no other org
+    alpha = next(b for b in body["bots"] if b["token_id"] == "byo-a")
+    assert alpha["server_count"] == 2 and alpha["invite_url"] and alpha["valid"] is True
+    assert "secret" not in repr(body)                                        # never a token string
+
+
+def test_member_is_refused(fs):
+    as_(MEMBER)
+    assert client.get("/tokens/org/preferred").status_code == 403
+
+
+def test_admin_without_org_gets_403_not_every_org(fs):
+    as_(ADMIN_NO_ORG)
+    assert client.get("/tokens/org/preferred").status_code == 403
+    # With ?org_id= a platform admin may act for that org (resolve_org_scope).
+    body = client.get(f"/tokens/org/preferred?org_id={OTHER}").json()
+    assert [s["system_id"] for s in body["systems"]] == ["s-b"]
+
+
+def test_non_admin_org_override_is_ignored(fs):
+    as_(STRANGER)
+    body = client.get(f"/tokens/org/preferred?org_id={ORG}").json()
+    assert body["org_id"] == OTHER
+    assert [s["system_id"] for s in body["systems"]] == ["s-b"]
+    assert [b["token_id"] for b in body["bots"]] == ["byo-b"]
+
+
+def test_platform_pick_on_owner_system_is_flagged_not_named(fs):
+    fs.put("systems", "s-a", {"system_id": "s-a", "name": "County P25", "org_id": ORG,
+                              "preferred_token_id": "pool-1"})
+    as_(OWNER)
+    s = next(x for x in client.get("/tokens/org/preferred").json()["systems"] if x["system_id"] == "s-a")
+    assert s["preferred_token_id"] is None and s["preferred_name"] is None
+    assert s["preferred_is_platform_bot"] is True
+    assert "DRB Radio 1" not in repr(s)
+
+
+def test_owner_set_then_list_round_trip_and_cache_is_dropped(fs):
+    stale = (1e18, {"system_id": "s-a", "org_id": ORG})      # what start_stream would read for 5 min
+    fstore._doc_cache[f"{fstore._path('systems')}/s-a"] = stale
+    fstore._doc_cache[f"{fstore._path('systems')}/s-a2"] = stale
+    as_(OWNER)
+    assert client.put("/tokens/byo-a/prefer/s-a").status_code == 200
+    assert f"{fstore._path('systems')}/s-a" not in fstore._doc_cache
+    s = next(x for x in client.get("/tokens/org/preferred").json()["systems"] if x["system_id"] == "s-a")
+    assert s["preferred_token_id"] == "byo-a" and s["preferred_name"] == "Alpha Radio"
+
+    # Moving the bot to another system drops both systems' cached copies.
+    fstore._doc_cache[f"{fstore._path('systems')}/s-a"] = stale
+    assert client.put("/tokens/byo-a/prefer/s-a2").status_code == 200
+    assert f"{fstore._path('systems')}/s-a" not in fstore._doc_cache
+    assert f"{fstore._path('systems')}/s-a2" not in fstore._doc_cache
+
+    fstore._doc_cache[f"{fstore._path('systems')}/s-a2"] = stale
+    assert client.put("/tokens/byo-a/prefer/_none").status_code == 200
+    assert f"{fstore._path('systems')}/s-a2" not in fstore._doc_cache
+    assert fs.get("systems", "s-a2")["preferred_token_id"] is None

drb-c2-core/tests/test_voice_check_age.py

@@ -0,0 +1,37 @@
+"""
+voice-check's ``checked_at`` is when Discord was last asked — the oldest
+cached snapshot behind the answer — not the time of the HTTP response, so the
+picker's "Permissions checked N ago" footer never calls a 10-minute-old
+denied result fresh (server-26#225 S4).
+"""
+import time
+from datetime import datetime
+
+from app.internal import discord_rest
+from app.routers import discord_checks as checks
+from tests.test_discord_perm_check import GUILD, OWNER, _snap, _token, fs, run  # noqa: F401  (fs = fixture)
+
+
+def _age(out) -> float:
+    return time.time() - datetime.fromisoformat(out["checked_at"]).timestamp()
+
+
+def test_checked_at_is_the_oldest_snapshot_not_now(fs, monkeypatch):
+    _token(fs, "t1"); _token(fs, "t2")
+    ages = {"t1": 30, "t2": 420}
+
+    async def _old(token_doc, guild_id, *, refresh=False):
+        s = _snap(token_doc["token_id"])
+        s.fetched_at = time.time() - (0 if refresh else ages[token_doc["token_id"]])
+        return s
+    monkeypatch.setattr(discord_rest, "guild_snapshot", _old)
+
+    out = run(checks.node_voice_check("n1", GUILD, refresh=0, channel_id=None, decoded=OWNER))
+    assert 415 <= _age(out) <= 430
+    out = run(checks.node_voice_check("n1", GUILD, refresh=1, channel_id=None, decoded=OWNER))
+    assert _age(out) < 5
+
+
+def test_checked_at_without_snapshots_is_now(fs):
+    out = run(checks.node_voice_check("n1", GUILD, refresh=0, channel_id=None, decoded=OWNER))  # no bots
+    assert _age(out) < 5