diff --git a/backend/app/main.py b/backend/app/main.py index d737397..eea033b 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -1,6 +1,6 @@ from fastapi import FastAPI -from app.routers import auth, formations, identity, library, roster, teams, whiteboard +from app.routers import auth, formations, identity, library, roster, suggestions, teams, whiteboard app = FastAPI(title="Patterns of Play API") app.include_router(auth.router) @@ -8,6 +8,7 @@ app.include_router(whiteboard.router) app.include_router(library.router) app.include_router(roster.router) +app.include_router(suggestions.router) app.include_router(formations.router) app.include_router(identity.router) diff --git a/backend/app/routers/roster.py b/backend/app/routers/roster.py index 2f92be2..830fb87 100644 --- a/backend/app/routers/roster.py +++ b/backend/app/routers/roster.py @@ -77,9 +77,40 @@ def _player_to_out( dwr=player.dwr, # type: ignore[arg-type] attributes=_attrs_to_schema(attrs), is_you=player.user_id is not None and player.user_id == caller_user_id, + playstyle_note=player.playstyle_note, ) +def _claim_matching_row(scope: TeamScope, players: list[Player], ctx: CurrentMembership) -> None: + """T-041: no "claim your row" surface is designed (PNGs 24/25/27 show + none), so a player's account links to their roster row implicitly + instead of through a new UI. Players are added to the roster by name + before they ever log in (Brief step 19 CRUD is coach-only), so on a + player's own GET /api/roster, if this team has exactly one row that is + both unclaimed (user_id IS NULL) and name-matches their display_name + (case/whitespace-insensitive), and they hold no other claimed row on + this team yet (doc 03 section 2: "a player user maps to at most one + roster entry per team"), that row becomes theirs. + + This is the minimal linkage the suggestion flow (Brief step 22) needs + to know which row is "your own profile": a player can only submit a + suggestion, and only sees the composer/pending card, on a row they + have claimed this way. A name that matches zero or more than one row + leaves every row unclaimed exactly as before (T-033's documented gap), + so this never guesses. Idempotent and safe to call on every request: + it is a no-op once a row is claimed or when no unique match exists. + """ + if ctx.role_on_team != "player": + return + if any(p.user_id == ctx.user.id for p in players): + return + target = ctx.user.display_name.strip().casefold() + matches = [p for p in players if p.user_id is None and p.name.strip().casefold() == target] + if len(matches) == 1: + matches[0].user_id = ctx.user.id + scope.commit() + + def _compute_fit_warnings(players: list[Player], clash: RoleClash | None) -> list[FitWarningOut]: """Doc 03 section 3: "a roster flank where the wide player has AWR high and DWR low, and the fullback or wingback behind them on the same side @@ -180,6 +211,7 @@ def get_roster( db: Session = Depends(get_db), ) -> dict: players = scope.query(Player).order_by(Player.jersey_number.asc().nulls_last()).all() + _claim_matching_row(scope, players, ctx) attrs_by_player = _attrs_by_player(scope) roles = _role_map(db) player_outs = [ diff --git a/backend/app/routers/suggestions.py b/backend/app/routers/suggestions.py new file mode 100644 index 0000000..cb4494e --- /dev/null +++ b/backend/app/routers/suggestions.py @@ -0,0 +1,190 @@ +"""Player playstyle suggestion flow (doc 03 section 3 playstyle_suggestions; +Brief step 22, PNG 24/25/27; T-041). + +README roles table: "Suggest own playstyle: Not applicable (coach); Yes +(player): free text on own profile, then pending coach review; coach sees a +gold badge on the row and an Approve / Dismiss card. Approve merges the note +into the profile." Every route depends on get_team_scope (team_id always +comes from the caller's own membership, never a client-supplied field, +CLAUDE.md rule 4) and enforces role at the API, not just the UI (CLAUDE.md +rule 5): only a player may submit a suggestion, and only against their own +linked roster row (app/routers/roster.py's claim-by-name-match); only a +coach may list the team's pending queue, approve, or dismiss. A player +calling either of the coach-only endpoints gets 403, proven in +backend/tests/test_suggestions_routes.py. +""" + +from fastapi import APIRouter, Depends, HTTPException, status + +from app.deps import CurrentMembership, get_current_membership, require_role_on_team +from app.models import Player, PlaystyleSuggestion +from app.models._util import utcnow +from app.schemas import SuggestionCreateRequest, SuggestionOut +from app.scoped import TeamScope, get_team_scope + +router = APIRouter(prefix="/api/roster", tags=["suggestions"]) + + +def _suggestion_to_out(suggestion: PlaystyleSuggestion, player_name: str) -> SuggestionOut: + return SuggestionOut( + id=suggestion.id, + player_id=suggestion.player_id, + player_name=player_name, + author_user_id=suggestion.author_user_id, + text=suggestion.text, + status=suggestion.status, # type: ignore[arg-type] + created_at=suggestion.created_at, + reviewed_at=suggestion.reviewed_at, + ) + + +# --------------------------------------------------------------------------- +# Player-facing: submit and read back the suggestions for one roster row. +# --------------------------------------------------------------------------- + + +@router.post( + "/players/{player_id}/suggestions", + response_model=SuggestionOut, + status_code=status.HTTP_201_CREATED, +) +def submit_suggestion( + player_id: int, + payload: SuggestionCreateRequest, + ctx: CurrentMembership = Depends(require_role_on_team("player")), + scope: TeamScope = Depends(get_team_scope), +) -> SuggestionOut: + # require_role_on_team("player") already 403s a coach caller (README: + # "Suggest own playstyle" is "Not applicable" for coaches). + player = scope.get(Player, player_id) + if player is None: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Player not found") + if player.user_id != ctx.user.id: + # README: "free text on own profile" -- never against a teammate's + # row, even one on the same team. + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="Forbidden") + + existing_pending = ( + scope.query(PlaystyleSuggestion) + .filter( + PlaystyleSuggestion.player_id == player_id, + PlaystyleSuggestion.status == "pending", + ) + .first() + ) + if existing_pending is not None: + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail="A suggestion is already pending coach review", + ) + + suggestion = PlaystyleSuggestion( + player_id=player_id, + author_user_id=ctx.user.id, + text=payload.text, + status="pending", + ) + scope.add(suggestion) + scope.commit() + scope.refresh(suggestion) + return _suggestion_to_out(suggestion, player.name) + + +@router.get("/players/{player_id}/suggestions", response_model=list[SuggestionOut]) +def list_player_suggestions( + player_id: int, + ctx: CurrentMembership = Depends(get_current_membership), + scope: TeamScope = Depends(get_team_scope), +) -> list[SuggestionOut]: + player = scope.get(Player, player_id) + if player is None: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Player not found") + if ctx.role_on_team != "coach" and player.user_id != ctx.user.id: + # A player may read back their own suggestion history; a coach may + # read any row on their team (needed to render the review card). + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="Forbidden") + + rows = ( + scope.query(PlaystyleSuggestion) + .filter(PlaystyleSuggestion.player_id == player_id) + .order_by(PlaystyleSuggestion.created_at.desc()) + .all() + ) + return [_suggestion_to_out(row, player.name) for row in rows] + + +# --------------------------------------------------------------------------- +# Coach-facing: the team-wide pending queue, approve, dismiss. All three +# 403 a player caller (README: fit warnings and suggestion review are +# coach-only capabilities; CLAUDE.md rule 5). +# --------------------------------------------------------------------------- + + +@router.get("/suggestions/pending", response_model=list[SuggestionOut]) +def list_pending_suggestions( + ctx: CurrentMembership = Depends(require_role_on_team("coach")), + scope: TeamScope = Depends(get_team_scope), +) -> list[SuggestionOut]: + rows = ( + scope.query(PlaystyleSuggestion) + .filter(PlaystyleSuggestion.status == "pending") + .order_by(PlaystyleSuggestion.created_at.desc()) + .all() + ) + player_ids = {row.player_id for row in rows} + players = { + p.id: p for p in scope.query(Player).filter(Player.id.in_(player_ids)).all() + } + return [_suggestion_to_out(row, players[row.player_id].name) for row in rows] + + +def _pending_or_409(scope: TeamScope, suggestion_id: int) -> PlaystyleSuggestion: + suggestion = scope.get(PlaystyleSuggestion, suggestion_id) + if suggestion is None: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Suggestion not found") + if suggestion.status != "pending": + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, detail="Suggestion already reviewed" + ) + return suggestion + + +@router.post("/suggestions/{suggestion_id}/approve", response_model=SuggestionOut) +def approve_suggestion( + suggestion_id: int, + ctx: CurrentMembership = Depends(require_role_on_team("coach")), + scope: TeamScope = Depends(get_team_scope), +) -> SuggestionOut: + suggestion = _pending_or_409(scope, suggestion_id) + player = scope.get(Player, suggestion.player_id) + if player is None: # pragma: no cover - FK guarantees this in practice + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Player not found") + + suggestion.status = "approved" + suggestion.reviewed_at = utcnow() + suggestion.reviewed_by = ctx.user.id + # README: "Approve merges the note into the profile." + player.playstyle_note = suggestion.text + scope.commit() + scope.refresh(suggestion) + return _suggestion_to_out(suggestion, player.name) + + +@router.post("/suggestions/{suggestion_id}/dismiss", response_model=SuggestionOut) +def dismiss_suggestion( + suggestion_id: int, + ctx: CurrentMembership = Depends(require_role_on_team("coach")), + scope: TeamScope = Depends(get_team_scope), +) -> SuggestionOut: + suggestion = _pending_or_409(scope, suggestion_id) + player = scope.get(Player, suggestion.player_id) + player_name = player.name if player is not None else "" + + # README: "dismiss clears it" -- status flips with no merge into + # playstyle_note, so the profile is unchanged. + suggestion.status = "dismissed" + suggestion.reviewed_at = utcnow() + suggestion.reviewed_by = ctx.user.id + scope.commit() + scope.refresh(suggestion) + return _suggestion_to_out(suggestion, player_name) diff --git a/backend/app/schemas.py b/backend/app/schemas.py index ea1e08c..db0ad32 100644 --- a/backend/app/schemas.py +++ b/backend/app/schemas.py @@ -297,10 +297,15 @@ class PlayerOut(BaseModel): dwr: WorkRate attributes: PlayerAttributesIn # True when this row belongs to the calling user (README roles table: - # player's "own row marked (you)"). Always false until a roster row is - # claimed by a player account, which is out of this ticket's scope - # (see T-033 final report). + # player's "own row marked (you)"). Set by app/routers/roster.py's + # claim-by-name-match (T-041; see that module's docstring) once a + # player's display_name uniquely matches an unclaimed row. is_you: bool + # doc 03 section 3: "approved text merges into players.playstyle_note". + # Visible to both roles (it is part of the player's profile, not + # coach-only data like fit_warnings/receipts): README "Approve merges + # the note into the profile." + playstyle_note: str | None = None class FitWarningOut(BaseModel): @@ -336,6 +341,44 @@ class CoachRosterOut(RosterOut): fit_warnings: list[FitWarningOut] +# --------------------------------------------------------------------------- +# Playstyle suggestions (doc 03 section 3 playstyle_suggestions; Brief step +# 22, PNG 24/25/27; T-041). README roles table: a player suggests a change +# to their own playstyle as free text; it sits "pending coach review" until +# a coach approves (merging the text into players.playstyle_note above) or +# dismisses it (clearing it with no merge). +# --------------------------------------------------------------------------- + +SuggestionStatus = Literal["pending", "approved", "dismissed"] + + +class SuggestionCreateRequest(BaseModel): + """No player_id, author_user_id, team_id, or status field on purpose + (CLAUDE.md rule 4 / doc 03 4.2 author-stamping precedent): player_id + comes from the path, author_user_id and team_id are stamped server-side + from the session, and a freshly submitted suggestion is always + 'pending'.""" + + model_config = ConfigDict(extra="forbid") + + text: str = Field(min_length=1, max_length=2000) + + +class SuggestionOut(BaseModel): + id: int + player_id: int + # Resolved server-side (same pattern as PlayerOut.role_name / + # SavedPatternOut.author_label) so the frontend never re-derives it and + # the coach's pending-review list can render a name without a second + # round trip per row. + player_name: str + author_user_id: int + text: str + status: SuggestionStatus + created_at: datetime + reviewed_at: datetime | None + + # --------------------------------------------------------------------------- # Formations, keystones, rondo map (doc 03 section 5, Bible 4/3G.2; Brief # step 18; T-032). Library-world content like LibraryItemOut above: no diff --git a/backend/tests/test_suggestions_routes.py b/backend/tests/test_suggestions_routes.py new file mode 100644 index 0000000..2cb13ac --- /dev/null +++ b/backend/tests/test_suggestions_routes.py @@ -0,0 +1,351 @@ +"""Playstyle suggestion routes (doc 03 section 3 playstyle_suggestions; +Brief step 22, PNG 24/25/27; T-041). Covers the Roles-and-sessions DoD line +verbatim: "Suggestion flow round-trips: player submits, sees pending; coach +approves; note appears merged on the profile; dismiss clears it." Plus API +permission enforcement in both directions (CLAUDE.md rule 5: a player token +calling a coach-only endpoint 403s) and cross-team isolation. + +Player row linkage: app/routers/roster.py claims a roster row for a player +the first time they GET /api/roster, if their display_name uniquely matches +an unclaimed row's name (see that module's _claim_matching_row docstring). +Every test here that needs a player to act on "their own profile" registers +the player with a display_name equal to the coach-created roster row's +name, then calls GET /api/roster once to trigger the claim, exactly as the +frontend's page-load fetch does. +""" + +import importlib.util +import pathlib +import sys + +import pytest +from fastapi.testclient import TestClient + +from app.main import app + +REPO_ROOT = pathlib.Path(__file__).resolve().parents[2] + + +def _run_seed_loader() -> None: + spec = importlib.util.spec_from_file_location( + "pop_seed_script_suggestion_tests", REPO_ROOT / "scripts" / "seed.py" + ) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = module + spec.loader.exec_module(module) + assert module.main() == 0 + + +@pytest.fixture(autouse=True) +def _seed_library_content() -> None: + _run_seed_loader() + + +@pytest.fixture +def client() -> TestClient: + return TestClient(app) + + +def _register(client: TestClient, *, email: str, role: str, display_name: str): + return client.post( + "/api/auth/register", + json={ + "email": email, + "password": "correct-horse-battery", + "display_name": display_name, + "role": role, + }, + ) + + +def _coach_with_team(email: str = "coach@example.com", name: str = "Coach Test") -> TestClient: + c = TestClient(app) + _register(c, email=email, role="coach", display_name=name) + c.post("/api/teams", json={"name": f"Team for {email}"}) + return c + + +def _player_on_team(coach: TestClient, email: str, name: str) -> TestClient: + join_code = coach.get("/api/teams/current").json()["join_code"] + p = TestClient(app) + _register(p, email=email, role="player", display_name=name) + p.post("/api/teams/join", json={"join_code": join_code}) + return p + + +_ATTRS = { + "pace": 3, + "passing_range": 3, + "carrying_1v1": 3, + "positional_discipline": 3, + "aerial_physical": 3, + "pressing_engine": 3, +} + + +def _player_body(**overrides: object) -> dict: + base: dict = { + "name": "New Player", + "jersey_number": 10, + "preferred_foot": "R", + "role_code": None, + "flank": None, + "awr": "med", + "dwr": "med", + "attributes": _ATTRS, + } + base.update(overrides) + return base + + +def _add_player(coach: TestClient, name: str) -> int: + resp = coach.post("/api/roster/players", json=_player_body(name=name)) + assert resp.status_code == 201 + return resp.json()["id"] + + +def _claim(player: TestClient) -> None: + """Mirrors the frontend's page-load fetch: GET /api/roster is what + triggers app/routers/roster.py's claim-by-name-match.""" + resp = player.get("/api/roster") + assert resp.status_code == 200 + + +def _own_player_id(player: TestClient) -> int: + body = player.get("/api/roster").json() + mine = [p for p in body["players"] if p["is_you"]] + assert len(mine) == 1, "expected exactly one claimed row" + return mine[0]["id"] + + +# --------------------------------------------------------------------------- +# The round trip itself (Roles and sessions DoD line, verbatim) +# --------------------------------------------------------------------------- + + +def test_player_submits_suggestion_and_sees_it_pending(client: TestClient) -> None: + coach = _coach_with_team() + _add_player(coach, "Maya K.") + player = _player_on_team(coach, email="maya@example.com", name="Maya K.") + _claim(player) + player_id = _own_player_id(player) + + created = player.post( + f"/api/roster/players/{player_id}/suggestions", + json={"text": "Could we try me as a touchline winger for a session."}, + ) + assert created.status_code == 201 + body = created.json() + assert body["status"] == "pending" + assert body["player_id"] == player_id + assert body["text"] == "Could we try me as a touchline winger for a session." + + # "sees pending": the player reads their own suggestion back as pending. + mine = player.get(f"/api/roster/players/{player_id}/suggestions") + assert mine.status_code == 200 + assert [s["status"] for s in mine.json()] == ["pending"] + + +def test_coach_approves_and_the_note_appears_merged_on_the_profile(client: TestClient) -> None: + coach = _coach_with_team() + _add_player(coach, "Maya K.") + player = _player_on_team(coach, email="maya@example.com", name="Maya K.") + _claim(player) + player_id = _own_player_id(player) + + submitted = player.post( + f"/api/roster/players/{player_id}/suggestions", + json={"text": "Wants to cut inside onto her strong foot more often."}, + ) + suggestion_id = submitted.json()["id"] + + # Coach sees the gold-badge queue before approving. + pending = coach.get("/api/roster/suggestions/pending") + assert pending.status_code == 200 + assert [s["id"] for s in pending.json()] == [suggestion_id] + assert pending.json()[0]["player_name"] == "Maya K." + + approved = coach.post(f"/api/roster/suggestions/{suggestion_id}/approve") + assert approved.status_code == 200 + assert approved.json()["status"] == "approved" + + # "note appears merged on the profile" -- visible to both roles, and + # the pending queue clears. + coach_roster = coach.get("/api/roster").json() + player_roster = player.get("/api/roster").json() + coach_row = next(p for p in coach_roster["players"] if p["id"] == player_id) + player_row = next(p for p in player_roster["players"] if p["id"] == player_id) + assert coach_row["playstyle_note"] == "Wants to cut inside onto her strong foot more often." + assert player_row["playstyle_note"] == "Wants to cut inside onto her strong foot more often." + assert coach.get("/api/roster/suggestions/pending").json() == [] + + +def test_coach_dismiss_clears_it_without_merging_a_note(client: TestClient) -> None: + coach = _coach_with_team() + _add_player(coach, "Maya K.") + player = _player_on_team(coach, email="maya@example.com", name="Maya K.") + _claim(player) + player_id = _own_player_id(player) + + submitted = player.post( + f"/api/roster/players/{player_id}/suggestions", json={"text": "Try me at fullback."} + ) + suggestion_id = submitted.json()["id"] + + dismissed = coach.post(f"/api/roster/suggestions/{suggestion_id}/dismiss") + assert dismissed.status_code == 200 + assert dismissed.json()["status"] == "dismissed" + + # "dismiss clears it": no note merged, and the pending queue is empty. + row = next( + p for p in coach.get("/api/roster").json()["players"] if p["id"] == player_id + ) + assert row["playstyle_note"] is None + assert coach.get("/api/roster/suggestions/pending").json() == [] + + # Clearing lets the player submit a fresh suggestion (no lingering + # pending row blocking them). + resubmitted = player.post( + f"/api/roster/players/{player_id}/suggestions", json={"text": "Try me at winger instead."} + ) + assert resubmitted.status_code == 201 + + +# --------------------------------------------------------------------------- +# Permission enforcement, both directions (CLAUDE.md rule 5) +# --------------------------------------------------------------------------- + + +def test_coach_cannot_submit_a_suggestion(client: TestClient) -> None: + coach = _coach_with_team() + player_id = _add_player(coach, "Maya K.") + + resp = coach.post( + f"/api/roster/players/{player_id}/suggestions", json={"text": "Not applicable."} + ) + assert resp.status_code == 403 + + +def test_player_cannot_submit_for_a_teammates_row(client: TestClient) -> None: + coach = _coach_with_team() + _add_player(coach, "Maya K.") + other_id = _add_player(coach, "Alex B.") + player = _player_on_team(coach, email="maya@example.com", name="Maya K.") + _claim(player) + + resp = player.post( + f"/api/roster/players/{other_id}/suggestions", json={"text": "Not my row."} + ) + assert resp.status_code == 403 + + +def test_unclaimed_player_gets_403_without_ever_calling_get_roster_first( + client: TestClient, +) -> None: + """Submission requires a prior claim (own-row linkage); a player who + never triggered the claim (e.g. named differently from every roster + row) is forbidden from every player_id on the team, proving the check + is real ownership, not merely "any player on my team".""" + coach = _coach_with_team() + player_id = _add_player(coach, "Maya K.") + player = _player_on_team(coach, email="someone@example.com", name="Someone Else") + _claim(player) # no name match: nothing gets claimed + + resp = player.post( + f"/api/roster/players/{player_id}/suggestions", json={"text": "x"} + ) + assert resp.status_code == 403 + + +def test_second_pending_suggestion_is_rejected(client: TestClient) -> None: + coach = _coach_with_team() + _add_player(coach, "Maya K.") + player = _player_on_team(coach, email="maya@example.com", name="Maya K.") + _claim(player) + player_id = _own_player_id(player) + + first = player.post( + f"/api/roster/players/{player_id}/suggestions", json={"text": "First."} + ) + assert first.status_code == 201 + second = player.post( + f"/api/roster/players/{player_id}/suggestions", json={"text": "Second."} + ) + assert second.status_code == 409 + + +def test_player_gets_403_listing_the_pending_queue(client: TestClient) -> None: + coach = _coach_with_team() + _add_player(coach, "Maya K.") + player = _player_on_team(coach, email="maya@example.com", name="Maya K.") + _claim(player) + + assert player.get("/api/roster/suggestions/pending").status_code == 403 + + +def test_player_gets_403_approving_or_dismissing(client: TestClient) -> None: + coach = _coach_with_team() + _add_player(coach, "Maya K.") + player = _player_on_team(coach, email="maya@example.com", name="Maya K.") + _claim(player) + player_id = _own_player_id(player) + + submitted = player.post( + f"/api/roster/players/{player_id}/suggestions", json={"text": "x"} + ) + suggestion_id = submitted.json()["id"] + + # CLAUDE.md rule 5 / ticket instruction: "player calling delete or + # receipt endpoints gets 403 with a test proving it" -- same shape + # applies to the coach-only approve/dismiss review controls here. + assert player.post(f"/api/roster/suggestions/{suggestion_id}/approve").status_code == 403 + assert player.post(f"/api/roster/suggestions/{suggestion_id}/dismiss").status_code == 403 + + +def test_client_cannot_forge_player_id_author_or_status_in_the_body(client: TestClient) -> None: + """SuggestionCreateRequest schema (extra='forbid') has only `text`, so a + forged player_id/author_user_id/team_id/status field is rejected by + Pydantic before the route body ever runs (same convention as + test_roster_routes.py's team_id/user_id forgery test).""" + coach = _coach_with_team() + _add_player(coach, "Maya K.") + player = _player_on_team(coach, email="maya@example.com", name="Maya K.") + _claim(player) + player_id = _own_player_id(player) + + resp = player.post( + f"/api/roster/players/{player_id}/suggestions", + json={"text": "x", "status": "approved", "author_user_id": 9999, "team_id": 9999}, + ) + assert resp.status_code == 422 + + +# --------------------------------------------------------------------------- +# Cross-team isolation (CLAUDE.md rule 4 / Platform DoD: "a cross-team read +# attempt in tests returns nothing") +# --------------------------------------------------------------------------- + + +def test_cross_team_coach_cannot_see_or_review_another_teams_suggestion( + client: TestClient, +) -> None: + coach_a = _coach_with_team(email="coach-a@example.com", name="Coach A") + _add_player(coach_a, "Maya K.") + player_a = _player_on_team(coach_a, email="maya@example.com", name="Maya K.") + _claim(player_a) + player_id = _own_player_id(player_a) + suggestion_id = player_a.post( + f"/api/roster/players/{player_id}/suggestions", json={"text": "x"} + ).json()["id"] + + coach_b = _coach_with_team(email="coach-b@example.com", name="Coach B") + _add_player(coach_b, "Someone Else") + + # Doesn't leak into team B's pending queue. + assert coach_b.get("/api/roster/suggestions/pending").json() == [] + # Cross-team read/action attempts return nothing rather than another + # team's data (TeamScope.get scopes by team_id). + assert coach_b.get(f"/api/roster/players/{player_id}/suggestions").status_code == 404 + assert coach_b.post(f"/api/roster/suggestions/{suggestion_id}/approve").status_code == 404 + assert coach_b.post(f"/api/roster/suggestions/{suggestion_id}/dismiss").status_code == 404 diff --git a/docs/agent/BACKLOG.md b/docs/agent/BACKLOG.md index 7f5e199..f3d8773 100644 --- a/docs/agent/BACKLOG.md +++ b/docs/agent/BACKLOG.md @@ -11,7 +11,7 @@ Model: sonnet default; opus = hard ticket, never downgrade. | T-004 | Scoped query layer + full schema from doc 03 + Alembic chain from zero + cross-team read test returns nothing | 4, 5 | platform | sonnet | T-001 | T-002 | done | | T-010 | Seed files: transcribe Bible per doc 03 §4-6 (12 patterns, 8 deliveries, 3 rotations, 6 formations+keystones, rondo 5 zones, 6 archetypes+pass-risk, 4 animated + 2 static ref teams, detail-only slots, cult corner, roles, synergies) | 6 | content-seeder | sonnet | T-004 | T-020 | done | | T-011 | Em-dash transform pass + CI copy scan + seed validator (required fields, blurb ≤25 words, banned identity phrases, slot refs resolve) | 7, 8 | content-seeder | sonnet | T-010 | T-020 | done | -| T-012 | Founder decision 2026-07-16: identities age_hint column (amend doc 03, Alembic migration after T-041's, backfill from Bible 8.2.4, validator + seed update) | founder | content-seeder | sonnet | T-010, T-041 | T-043 | todo | +| T-012 | Founder decision 2026-07-16: identities age_hint column (amend doc 03, Alembic migration, backfill from Bible 8.2.4, validator + seed update) | founder | content-seeder | sonnet | T-010, T-041 | T-043 | doing | | T-020 | Board core: pitch canvas, landscape model coords, token drag 60fps @23 tokens, portrait mapping (left=y, top=100-x) with lossless round-trip unit test FIRST | 9, 10 | board-engineer | opus | T-001 | T-010 | done | | T-021 | Lane graph: suggested/confirmed/blocked states, two independent thresholds, live recompute during drag, interception dot | 11, 12 | board-engineer | opus | T-020 | T-011 | done | | T-022 | Zones + animation player (declarative specs AND raw keyframes, ball waypoints chase bound player) + recorder (all tokens incl. opponents + ball) | 13, 14, 15 | board-engineer | opus | T-021 | none | done | diff --git a/e2e/suggestions.spec.ts b/e2e/suggestions.spec.ts new file mode 100644 index 0000000..ea5b80a --- /dev/null +++ b/e2e/suggestions.spec.ts @@ -0,0 +1,283 @@ +// Playstyle suggestion flow (Brief step 22, PNG 24/25/27; T-041). Runs +// under both Playwright projects (mobile portrait, desktop landscape) per +// playwright.config.ts. Covers the Roles-and-sessions DoD line verbatim: +// "Suggestion flow round-trips: player submits, sees pending; coach +// approves; note appears merged on the profile; dismiss clears it." +// plus the role check contract (skill: verify-ui) that approve/dismiss are +// absent from a player's DOM, not merely hidden, and a three-theme pass. +// +// Player row linkage: backend/app/routers/roster.py claims a roster row +// for a player the first time they GET /api/roster (RosterPage's own +// page-load fetch), if their display_name uniquely matches an unclaimed +// row's name. Every test here registers the player with a displayName +// equal to the coach-created roster row's name so the claim fires and the +// player's own row is identifiable ("(you)"). + +import { test, expect, assertCleanPage, registerCoach, registerPlayer } from "./fixtures"; +import type { Locator, Page } from "@playwright/test"; + +function trackIssues(page: Page) { + const issues = { consoleErrors: [] as string[], failedRequests: [] as string[], serverErrors: [] as string[] }; + page.on("console", (m) => m.type() === "error" && issues.consoleErrors.push(m.text())); + page.on("requestfailed", (r) => issues.failedRequests.push(`${r.method()} ${r.url()}`)); + page.on("response", (r) => r.status() >= 500 && issues.serverErrors.push(`${r.status()} ${r.url()}`)); + return issues; +} + +// Same rationale as e2e/roster.spec.ts's robustClick: Chromium's mobile +// touch emulation shrinks the visual viewport once a text input focuses +// and never restores it, which can misdirect a plain coordinate click on +// this form-heavy page. Dispatching the event targets the element +// directly instead. +async function robustClick(locator: Locator) { + await locator.scrollIntoViewIfNeeded(); + await locator.dispatchEvent("click"); +} + +async function goToRoster(page: Page) { + await robustClick(page.getByTestId("nav-roster")); + await expect(page.getByTestId("nav-roster")).toHaveAttribute("aria-current", "page"); + await expect(page.getByRole("heading", { name: "Roster" })).toBeVisible(); +} + +async function addPlayer( + page: Page, + opts: { + name: string; + jersey: string; + roleCode: string; + flank: "left" | "right" | "center"; + awr: "low" | "med" | "high"; + dwr: "low" | "med" | "high"; + } +) { + await robustClick(page.getByTestId("roster-add-player")); + await page.getByTestId("player-name").fill(opts.name); + await page.getByTestId("player-jersey").fill(opts.jersey); + await page.getByTestId("player-role").selectOption(opts.roleCode); + await page.getByTestId("player-flank").selectOption(opts.flank); + await page.getByTestId("player-awr").selectOption(opts.awr); + await page.getByTestId("player-dwr").selectOption(opts.dwr); + await robustClick(page.getByTestId("player-save")); + await expect(page.getByTestId("player-save")).toHaveCount(0); +} + +const SUGGESTION_TEXT = + "Could we try me as a touchline winger for a session, cutting inside onto my strong foot."; +const SECOND_SUGGESTION_TEXT = "Maybe drop me deeper next session to link up the midfield."; + +test.describe("suggestions: full round trip (Brief step 22 DoD)", () => { + test("player submits, sees pending; coach approves; note merges on the profile; dismiss clears it", async ({ + browser, + }) => { + const coachContext = await browser.newContext(); + const coachPage = await coachContext.newPage(); + const coachIssues = trackIssues(coachPage); + const { joinCode } = await registerCoach(coachPage, { displayName: "Coach Suggest" }); + await goToRoster(coachPage); + await addPlayer(coachPage, { + name: "Maya K.", + jersey: "7", + roleCode: "inside_forward", + flank: "right", + awr: "high", + dwr: "low", + }); + + const playerContext = await browser.newContext(); + const playerPage = await playerContext.newPage(); + const playerIssues = trackIssues(playerPage); + await registerPlayer(playerPage, joinCode, { displayName: "Maya K." }); + await goToRoster(playerPage); + + const ownRow = playerPage.getByTestId(/roster-row-\d+/).filter({ hasText: "Maya K." }); + // Claim-by-name-match (backend/app/routers/roster.py) fired on this + // page's GET /api/roster load: the row is marked "(you)" in the list. + await expect(ownRow).toContainText("(you)"); + await robustClick(ownRow); + + // DoD: "player submits, sees pending". + await expect(playerPage.getByTestId("suggestion-composer")).toBeVisible(); + await playerPage.getByTestId("suggestion-text").fill(SUGGESTION_TEXT); + await robustClick(playerPage.getByTestId("suggestion-send")); + await expect(playerPage.getByTestId("suggestion-pending-card")).toContainText(SUGGESTION_TEXT); + await expect(playerPage.getByTestId("suggestion-composer")).toHaveCount(0); + + // Coach sees the gold badge on the row and the review card (README: + // "coach sees a gold badge on the row and an Approve / Dismiss card"). + await coachPage.reload(); + await goToRoster(coachPage); + const coachRow = coachPage.getByTestId(/roster-row-\d+/).filter({ hasText: "Maya K." }); + await expect(coachRow.locator(".suggestion-badge")).toHaveCount(1); + await robustClick(coachRow); + await expect(coachPage.getByTestId("suggestion-review-card")).toContainText(SUGGESTION_TEXT); + await expect(coachPage.getByTestId("suggestion-review-card")).toContainText("Maya K."); + + // DoD: "coach approves; note appears merged on the profile". + await robustClick(coachPage.getByTestId("suggestion-approve")); + await expect(coachPage.getByTestId("suggestion-review-card")).toHaveCount(0); + await expect(coachPage.locator(".suggestion-badge")).toHaveCount(0); + await expect(coachPage.getByTestId("playstyle-note")).toContainText(SUGGESTION_TEXT); + + // The player's own view reflects the merge and the cleared pending + // state, not just the coach's. + await playerPage.reload(); + await goToRoster(playerPage); + await robustClick(playerPage.getByTestId(/roster-row-\d+/).filter({ hasText: "Maya K." })); + await expect(playerPage.getByTestId("playstyle-note")).toContainText(SUGGESTION_TEXT); + await expect(playerPage.getByTestId("suggestion-pending-card")).toHaveCount(0); + await expect(playerPage.getByTestId("suggestion-composer")).toBeVisible(); + + // DoD: "dismiss clears it". A second suggestion (allowed now that the + // first has been reviewed), reviewed the other way. + await playerPage.getByTestId("suggestion-text").fill(SECOND_SUGGESTION_TEXT); + await robustClick(playerPage.getByTestId("suggestion-send")); + await expect(playerPage.getByTestId("suggestion-pending-card")).toContainText( + SECOND_SUGGESTION_TEXT + ); + + await coachPage.reload(); + await goToRoster(coachPage); + await robustClick(coachPage.getByTestId(/roster-row-\d+/).filter({ hasText: "Maya K." })); + await expect(coachPage.getByTestId("suggestion-review-card")).toContainText( + SECOND_SUGGESTION_TEXT + ); + await robustClick(coachPage.getByTestId("suggestion-dismiss")); + await expect(coachPage.getByTestId("suggestion-review-card")).toHaveCount(0); + await expect(coachPage.locator(".suggestion-badge")).toHaveCount(0); + // The earlier approved note is untouched by the dismiss (dismiss never + // merges into the profile). + await expect(coachPage.getByTestId("playstyle-note")).toContainText(SUGGESTION_TEXT); + + await playerPage.reload(); + await goToRoster(playerPage); + await robustClick(playerPage.getByTestId(/roster-row-\d+/).filter({ hasText: "Maya K." })); + await expect(playerPage.getByTestId("suggestion-pending-card")).toHaveCount(0); + await expect(playerPage.getByTestId("suggestion-composer")).toBeVisible(); + await expect(playerPage.getByTestId("playstyle-note")).toContainText(SUGGESTION_TEXT); + await expect(playerPage.getByTestId("playstyle-note")).not.toContainText(SECOND_SUGGESTION_TEXT); + + await assertCleanPage(coachPage, coachIssues); + await assertCleanPage(playerPage, playerIssues); + + await coachContext.close(); + await playerContext.close(); + }); +}); + +test.describe("suggestions: approve/dismiss are coach-only, absent from a player's DOM", () => { + test("player never sees approve or dismiss, even reviewing their own pending suggestion", async ({ + browser, + }) => { + const coachContext = await browser.newContext(); + const coachPage = await coachContext.newPage(); + const coachIssues = trackIssues(coachPage); + const { joinCode } = await registerCoach(coachPage, { displayName: "Coach Absent" }); + await goToRoster(coachPage); + await addPlayer(coachPage, { + name: "Alex B.", + jersey: "9", + roleCode: "target_man", + flank: "center", + awr: "med", + dwr: "med", + }); + + const playerContext = await browser.newContext(); + const playerPage = await playerContext.newPage(); + const playerIssues = trackIssues(playerPage); + await registerPlayer(playerPage, joinCode, { displayName: "Alex B." }); + await goToRoster(playerPage); + await robustClick(playerPage.getByTestId(/roster-row-\d+/).filter({ hasText: "Alex B." })); + await playerPage + .getByTestId("suggestion-text") + .fill("Try me on the shoulder of the last defender more often."); + await robustClick(playerPage.getByTestId("suggestion-send")); + await expect(playerPage.getByTestId("suggestion-pending-card")).toBeVisible(); + + // Role check contract (skill: verify-ui): coach-only elements absent + // from the DOM for a player, not merely hidden. + await expect(playerPage.getByTestId("suggestion-approve")).toHaveCount(0); + await expect(playerPage.getByTestId("suggestion-dismiss")).toHaveCount(0); + await expect(playerPage.getByTestId("suggestion-review-card")).toHaveCount(0); + await expect(playerPage.locator(".suggestion-badge")).toHaveCount(0); + + await assertCleanPage(coachPage, coachIssues); + await assertCleanPage(playerPage, playerIssues); + + await coachContext.close(); + await playerContext.close(); + }); +}); + +test.describe("suggestions: matches across all three themes", () => { + test("the composer/pending/review cards use the theme's gold accent border", async ({ + browser, + }) => { + const coachContext = await browser.newContext(); + const coachPage = await coachContext.newPage(); + const coachIssues = trackIssues(coachPage); + const { joinCode } = await registerCoach(coachPage, { displayName: "Coach Theme" }); + await goToRoster(coachPage); + await addPlayer(coachPage, { + name: "Maya K.", + jersey: "7", + roleCode: "inside_forward", + flank: "right", + awr: "high", + dwr: "low", + }); + + const playerContext = await browser.newContext(); + const playerPage = await playerContext.newPage(); + const playerIssues = trackIssues(playerPage); + await registerPlayer(playerPage, joinCode, { displayName: "Maya K." }); + await goToRoster(playerPage); + await robustClick(playerPage.getByTestId(/roster-row-\d+/).filter({ hasText: "Maya K." })); + await playerPage.getByTestId("suggestion-text").fill(SUGGESTION_TEXT); + await robustClick(playerPage.getByTestId("suggestion-send")); + await expect(playerPage.getByTestId("suggestion-pending-card")).toBeVisible(); + + await coachPage.reload(); + await goToRoster(coachPage); + await robustClick(coachPage.getByTestId(/roster-row-\d+/).filter({ hasText: "Maya K." })); + await expect(coachPage.getByTestId("suggestion-review-card")).toBeVisible(); + + const seenReviewBorder = new Set(); + const seenPendingBorder = new Set(); + + for (const theme of ["pitch", "dark", "board"] as const) { + await robustClick(coachPage.getByTestId(`theme-switch-${theme}`)); + await expect(coachPage.locator("html")).toHaveAttribute("data-theme", theme); + seenReviewBorder.add( + await coachPage + .getByTestId("suggestion-review-card") + .evaluate((el) => getComputedStyle(el).borderColor) + ); + + await robustClick(playerPage.getByTestId(`theme-switch-${theme}`)); + await expect(playerPage.locator("html")).toHaveAttribute("data-theme", theme); + seenPendingBorder.add( + await playerPage + .getByTestId("suggestion-pending-card") + .evaluate((el) => getComputedStyle(el).borderColor) + ); + } + + // Every theme actually painted a distinct token value: proves the + // cards read the accent CSS variable per theme rather than a + // hardcoded color (same evidence shape as e2e/roster.spec.ts's own + // three-theme test). + expect(seenReviewBorder.size).toBe(3); + expect(seenPendingBorder.size).toBe(3); + + await robustClick(coachPage.getByTestId("suggestion-approve")); + await expect(coachPage.getByTestId("playstyle-note")).toBeVisible(); + + await assertCleanPage(coachPage, coachIssues); + await assertCleanPage(playerPage, playerIssues); + + await coachContext.close(); + await playerContext.close(); + }); +}); diff --git a/frontend/src/pages/RosterPage.css b/frontend/src/pages/RosterPage.css index 5061635..50a2072 100644 --- a/frontend/src/pages/RosterPage.css +++ b/frontend/src/pages/RosterPage.css @@ -115,6 +115,20 @@ color: var(--text-secondary); font-size: 12px; } + +/* Coach-only "gold badge on the row" for a pending playstyle suggestion + (README roles table; Brief step 22). Gold is the only interactive + color, but this is a status dot, not a control; kept small and + inline like .roster-you rather than a full pill. */ +.suggestion-badge { + display: inline-block; + width: 7px; + height: 7px; + margin-left: 6px; + border-radius: 50%; + background: var(--accent); + vertical-align: middle; +} .roster-row-role { font-size: 11px; color: var(--text-secondary); @@ -324,6 +338,71 @@ cursor: default; } +/* Playstyle suggestion flow (PNG 24/25/27; Brief step 22): the composer, + the player's own pending card, and the coach's review card all share + this gold-bordered card shape, gold being the design system's one + interactive/highlight color. */ +.suggestion-card { + border: 1px solid var(--accent); + border-radius: var(--radius); + padding: 12px 14px; + margin-top: 4px; +} +.suggestion-card-label { + font-size: 11px; + letter-spacing: 0.05em; + text-transform: uppercase; + color: var(--text-secondary); + margin: 0 0 8px; +} +.suggestion-card-hint { + font-size: 12px; + color: var(--text-secondary); + margin: 0 0 10px; + line-height: 1.45; +} +.suggestion-card-text { + font-size: 13px; + color: var(--text-primary); + margin: 0; + line-height: 1.45; +} +.suggestion-composer textarea { + width: 100%; + min-height: 64px; + box-sizing: border-box; + font: inherit; + font-size: 13px; + padding: 8px 10px; + border-radius: 6px; + border: 1px solid var(--border); + background: var(--bg); + color: var(--text-primary); + resize: vertical; + margin-bottom: 10px; +} +.suggestion-composer button[type="submit"], +.suggestion-card-actions button[data-testid="suggestion-approve"] { + font: inherit; + font-size: 13px; + padding: 7px 16px; + border-radius: 999px; + border: 1px solid var(--accent); + background: var(--accent); + color: var(--accent-ink, #1b1b1b); + cursor: pointer; +} +.suggestion-composer button[type="submit"]:disabled, +.suggestion-card-actions button:disabled { + opacity: 0.6; + cursor: default; +} +.suggestion-card-actions { + display: flex; + gap: 10px; + margin-top: 10px; +} + /* Phone: single column, list above detail (design README: grids stack). */ @media (max-width: 700px) { .roster-layout { diff --git a/frontend/src/pages/RosterPage.tsx b/frontend/src/pages/RosterPage.tsx index 59bb48b..b111443 100644 --- a/frontend/src/pages/RosterPage.tsx +++ b/frontend/src/pages/RosterPage.tsx @@ -12,10 +12,15 @@ import type { FormEvent } from "react"; import { ATTRIBUTE_KEYS, ATTRIBUTE_LABELS, + approveSuggestion, createPlayer, deletePlayer, + dismissSuggestion, + fetchPendingSuggestions, + fetchPlayerSuggestions, fetchRoleCatalog, fetchRoster, + submitSuggestion, updatePlayer, type AttributeKey, type FitWarningWire, @@ -26,6 +31,7 @@ import { type PreferredFoot, type Role, type RoleCatalogWire, + type SuggestionWire, type WorkRate, } from "../rosterApi"; import { ApiError } from "../api"; @@ -110,6 +116,16 @@ export function RosterPage({ role }: { role: Role }) { const [saveError, setSaveError] = useState(null); const [saving, setSaving] = useState(false); + // Playstyle suggestion flow (Brief step 22, PNG 24/25/27; T-041). + // pendingSuggestions is the coach-only team-wide queue (README: "coach + // sees a gold badge on the row"); ownPendingSuggestion is a player's own + // latest pending submission for whichever profile they're viewing. + const [pendingSuggestions, setPendingSuggestions] = useState([]); + const [ownPendingSuggestion, setOwnPendingSuggestion] = useState(null); + const [suggestionText, setSuggestionText] = useState(""); + const [suggestionSaving, setSuggestionSaving] = useState(false); + const [suggestionError, setSuggestionError] = useState(null); + // Re-reads just the roster (not the role catalog, not the page loading // flag) after a create/update/delete: fit warnings can shift with any // roster edit, and re-fetching keeps them correct without duplicating @@ -147,6 +163,64 @@ export function RosterPage({ role }: { role: Role }) { [players, selectedId] ); + const isCoach = role === "coach"; + + const pendingPlayerIds = useMemo( + () => new Set(pendingSuggestions.map((s) => s.player_id)), + [pendingSuggestions] + ); + + const pendingSuggestionForSelected = useMemo( + () => + selectedPlayer + ? pendingSuggestions.find((s) => s.player_id === selectedPlayer.id) ?? null + : null, + [pendingSuggestions, selectedPlayer] + ); + + // Coach-only team-wide pending queue: backs the roster row badge and, + // filtered by selectedPlayer below, the review card. 403s for a player + // caller (README: suggestion review is coach-only), so this never runs + // for one. + const refreshPendingSuggestions = useCallback(async () => { + if (!isCoach) return; + try { + setPendingSuggestions(await fetchPendingSuggestions()); + } catch { + // Non-fatal: the roster itself already loaded; leave the queue as-is + // rather than failing the whole page over a secondary fetch. + } + }, [isCoach]); + + useEffect(() => { + refreshPendingSuggestions(); + }, [refreshPendingSuggestions]); + + // A player's own suggestion history, only ever fetched for their own + // claimed row (README: "free text on own profile"). Re-runs whenever the + // selection changes so switching away from, then back to, your own row + // reflects the latest state. + useEffect(() => { + setSuggestionError(null); + setSuggestionText(""); + if (isCoach || !selectedPlayer?.is_you) { + setOwnPendingSuggestion(null); + return; + } + let cancelled = false; + fetchPlayerSuggestions(selectedPlayer.id) + .then((rows) => { + if (cancelled) return; + setOwnPendingSuggestion(rows.find((r) => r.status === "pending") ?? null); + }) + .catch(() => { + if (!cancelled) setOwnPendingSuggestion(null); + }); + return () => { + cancelled = true; + }; + }, [isCoach, selectedPlayer]); + const rolesByPosition = useMemo(() => { const grouped = new Map(); for (const r of roles) { @@ -225,7 +299,60 @@ export function RosterPage({ role }: { role: Role }) { setForm((prev) => ({ ...prev, attributes: { ...prev.attributes, [key]: value } })); } - const isCoach = role === "coach"; + // Brief step 22 DoD: "player submits, sees pending". Only reachable for a + // player viewing their own claimed row (see PlayerDetail below), so + // selectedPlayer here is always the caller's own row when this runs. + async function handleSubmitSuggestion(event: FormEvent) { + event.preventDefault(); + if (!selectedPlayer) return; + setSuggestionSaving(true); + setSuggestionError(null); + try { + const created = await submitSuggestion(selectedPlayer.id, suggestionText.trim()); + setOwnPendingSuggestion(created); + setSuggestionText(""); + } catch (err) { + setSuggestionError( + err instanceof ApiError ? err.message : "Could not send suggestion, try again." + ); + } finally { + setSuggestionSaving(false); + } + } + + // "coach approves; note appears merged on the profile": re-reads the + // roster so the merged playstyle_note and the cleared queue both land. + async function handleApproveSuggestion(suggestionId: number) { + setSuggestionSaving(true); + setSuggestionError(null); + try { + await approveSuggestion(suggestionId); + await Promise.all([refreshRoster(), refreshPendingSuggestions()]); + } catch (err) { + setSuggestionError( + err instanceof ApiError ? err.message : "Could not approve, try again." + ); + } finally { + setSuggestionSaving(false); + } + } + + // "dismiss clears it": no roster re-read needed (no note is merged), just + // the queue. + async function handleDismissSuggestion(suggestionId: number) { + setSuggestionSaving(true); + setSuggestionError(null); + try { + await dismissSuggestion(suggestionId); + await refreshPendingSuggestions(); + } catch (err) { + setSuggestionError( + err instanceof ApiError ? err.message : "Could not dismiss, try again." + ); + } finally { + setSuggestionSaving(false); + } + } return (
@@ -290,6 +417,20 @@ export function RosterPage({ role }: { role: Role }) { {player.name} {player.is_you && (you)} + {/* README: "coach sees a gold badge on the row" + for a pending playstyle suggestion. Absent + from the DOM for a player, not just hidden: + pendingSuggestions is only ever populated for + a coach caller (refreshPendingSuggestions + above). */} + {isCoach && pendingPlayerIds.has(player.id) && ( + {player.role_name ?? "Unassigned"} @@ -318,6 +459,15 @@ export function RosterPage({ role }: { role: Role }) { onEdit={() => startEdit(selectedPlayer)} onDelete={handleDelete} saving={saving} + ownPendingSuggestion={ownPendingSuggestion} + suggestionText={suggestionText} + onSuggestionTextChange={setSuggestionText} + onSubmitSuggestion={handleSubmitSuggestion} + pendingSuggestion={pendingSuggestionForSelected} + onApproveSuggestion={handleApproveSuggestion} + onDismissSuggestion={handleDismissSuggestion} + suggestionSaving={suggestionSaving} + suggestionError={suggestionError} /> )} {mode === "view" && !selectedPlayer && ( @@ -350,12 +500,30 @@ function PlayerDetail({ onEdit, onDelete, saving, + ownPendingSuggestion, + suggestionText, + onSuggestionTextChange, + onSubmitSuggestion, + pendingSuggestion, + onApproveSuggestion, + onDismissSuggestion, + suggestionSaving, + suggestionError, }: { player: PlayerWire; isCoach: boolean; onEdit: () => void; onDelete: () => void; saving: boolean; + ownPendingSuggestion: SuggestionWire | null; + suggestionText: string; + onSuggestionTextChange: (value: string) => void; + onSubmitSuggestion: (event: FormEvent) => void; + pendingSuggestion: SuggestionWire | null; + onApproveSuggestion: (suggestionId: number) => void; + onDismissSuggestion: (suggestionId: number) => void; + suggestionSaving: boolean; + suggestionError: string | null; }) { return (
@@ -384,6 +552,17 @@ function PlayerDetail({
{player.role_description &&

{player.role_description}

} + {/* Approved playstyle suggestion, merged onto the profile (doc 03 + section 3; Brief step 22 DoD: "note appears merged on the + profile"). Visible to both roles, same as the rest of the + profile. */} + {player.playstyle_note && ( +
+

Playstyle note

+

{player.playstyle_note}

+
+ )} +

Work rates (attacking / defensive) @@ -414,6 +593,91 @@ function PlayerDetail({

))} + + {/* Brief step 22 / PNG 24-25: a player suggests a change to their own + playstyle, then sees it pending. Only ever rendered for a player + viewing their own claimed row: absent from the DOM entirely for a + coach (this whole block) and absent for a player viewing a + teammate's row (player.is_you false there). */} + {!isCoach && player.is_you && ( + <> + {ownPendingSuggestion ? ( +
+

Your suggestion, pending coach review

+

“{ownPendingSuggestion.text}”

+
+ ) : ( +
+

Suggest a change to your playstyle

+

+ Tell your coach how you see your game: role, runs, what you want to work on. The + coach reviews before anything changes. +

+