Skip to content

🔍 Inspector: Fix missing manage_settings gating in ClubSettings - #508

Draft
jnnngs wants to merge 1 commit into
mainfrom
claude/gallant-mayer-2dt63p
Draft

jnnngs wants to merge 1 commit into
mainfrom
claude/gallant-mayer-2dt63p

Conversation

@jnnngs

@jnnngs jnnngs commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

What and why

ClubSettings.tsx rendered every settings section (image, general info, location & contact, opening hours, disciplines, membership, privacy, regional defaults) fully live and editable for any club moderator, even though the backend gates every one of those save endpoints on PermManageSettings (service/club.go: UpdateClub, UpdateImageURL, ReplaceOpeningHours, RegenerateJoinCode). A moderator promoted with only manage_members (the default promotion grant explicitly excludes manage_settings) would see a normal, live form and get a bare "Failed to save" toast with no explanation on every attempted save.

LeagueSettings.tsx — the structural sibling built to the same pattern — already fixed this exact bug, and CLAUDE.md documents the fix in detail ("A control the viewer's grant doesn't cover is drawn read-only, with the reason"). It just never made it across to the club side.

📋 Findings

Functional

  1. [HIGH] ClubSettings.tsx never gates any section on manage_settings — the fixed defect. permissionData (from getModeratorPermissions) was fetched at the page level but only used for MembersSection's canDelegate and the announcement composer's canSend; all 8 profile/settings sections rendered live regardless of the viewer's actual capability, silently 403'ing on save with no explanation. See "Fixed" below.
  2. [LOW] ScoreCardDetail.tsx's updateMutation.onError only shows a toast for ApiError (403 or other), with no else branch for a non-ApiError failure (e.g. a raw network TypeError from fetch). There is a fallback inline {updateMutation.isError && <p>Failed to save changes…</p>} message, so the user isn't left with literally zero feedback, but the toast path is inconsistent with ScoreEntry.tsx's equivalent handler, which always falls back to a generic toast. Not fixed — low impact given the existing inline fallback, and touching shared error-toast conventions felt like a larger, less clearly-scoped change than the primary fix.
  3. [LOW] contextChangePlan/score-card context switching, notification fan-out, gear-comparison opt-in — traced these flows end-to-end (backend service + frontend call sites) looking for the class of bug the brief calls out (region/permission gaps, stale closures, float money math, race conditions). All were consistent with CLAUDE.md's documented behaviour and already covered by existing tests; no defects found.

UX

  1. [MED] (mechanically part of finding feat: improve scorecard UX with selectable cells, score input panel, and click-to-X #1) — no CapabilityNote anywhere in ClubSettings.tsx prior to this fix, so a moderator with a partial grant had no way to tell a save failure from a permission gap vs. a real error.
  2. No other UX inconsistencies (loading/empty/error states, destructive-action confirmation, form validation timing) turned up in the traced flows — ConfirmDialog is used consistently for every destructive action across both settings pages, and no window.confirm/alert/prompt usage exists in the frontend.

🐛 Fixed

Ported LeagueSettings.tsx's canManage / CapabilityNote pattern to ClubSettings.tsx:

  • Added a canManage: boolean prop to ClubImageSection, GeneralInfoSection, LocationContactSection, OpeningHoursSection, DisciplinesSection, MembershipInfoSection, PrivacySection, RegionalSection.
  • Each section now disables its inputs/toggle buttons, hides its Save button, and shows a CapabilityNote ("Read-only — … needs the "Manage settings" permission. The club owner grants it from Members, below.") when the viewer lacks manage_settings.
  • ClubSettings() now computes canManageSettings = can(permissionData?.role, PERM.manageSettings) and passes it to all 8 sections — mirroring LeagueSettings.tsx exactly.
  • Added a disabled prop to the shared TagInput component (used by Disciplines' Distances/Facilities editors) so it can be gated too.

Added describe('ClubSettings capability gating', …) in ClubSettings.test.tsx (mirroring the existing LeagueSettings capability gating suite) covering both branches: read-only without manage_settings, editable with it.

Verified:

  • cd frontend && npm run check — clean
  • cd frontend && npm run lint — clean
  • cd frontend && npm test — 59 files / 367 tests pass (including the 2 new tests)
  • cd frontend && npm run build — clean (confirms tsc -b solution-file build mode)

⚠️ Risk/Follow-up

  • Purely additive/gating change to one page's UI — no API shape, route or permission model changes. Risk is limited to a moderator who previously had implicit-in-practice (if incidentally granted) access via a section that should have been gated now correctly seeing it read-only, which is the intended fix.
  • Did not run the Playwright e2e suite (not in the PR gate per CLAUDE.md, and this change doesn't touch score capture/leagues/events flows it covers).

❓ Questions

  • None — the defect and fix were unambiguous once the League/Club asymmetry was found.

📝 Noted, not fixed


🤖 Generated with Claude Code

https://claude.ai/code/session_016fa2ueKYTDTQCG2ZqKgzJa


Generated by Claude Code

Every club settings section (image, general info, location & contact,
opening hours, disciplines, membership, privacy, regional defaults) saves
through an endpoint the backend gates on manage_settings, but the frontend
never threaded the viewer's permissions through to disable them — any club
moderator saw fully live, editable forms and got an unexplained "Failed to
save" on every attempt if they lacked the capability. LeagueSettings.tsx
already fixed this exact bug for leagues; port the same canManage/
CapabilityNote pattern to its club counterpart.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016fa2ueKYTDTQCG2ZqKgzJa
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants