feat(bans): show the ban reason to the banned player#4659
Conversation
Completes the player-facing half of the account-bans feature. The infra API already returns the active ban on GET /users/@me (category, player-facing reason, expiry); until now the game ignored it and a banned player only saw the WebSocket close string "Account Banned". - ApiSchemas: add `ban` to UserMeResponseSchema (top-level, optional/nullable so an older API is treated as "no ban"). category kept as a string so an unrecognised value degrades gracefully. - New <banned-modal>: reads the `userMeResponse` event (like the marketing toast), and when a ban is present opens an <o-modal> showing the localized category, the reason, and "banned until <date>" / "permanent". Opens once; the player can close it (the server still enforces the ban). - en.json: ban_notice.* strings incl. a label per ban_category. Tests: schema (ban present/permanent/null/malformed) and the component (temp/permanent/no-ban/@me-failed/unknown-category fallback). Full suite green (2099); tsc, ESLint, Prettier, translation-sync and en.json-sorted all pass. Depends on the infra @me `ban` field (already deployed); the game server is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughA new ban schema, English translations, and ChangesBanned player notice
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant UserMeResponse
participant BannedModal
participant translateText
participant o-modal
UserMeResponse->>BannedModal: dispatch userMeResponse with ban details
BannedModal->>translateText: resolve localized notice text
BannedModal->>o-modal: render ban content
BannedModal->>o-modal: open once
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)resources/lang/en.jsonTraceback (most recent call last): 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/client/components/BannedModal.ts (2)
8-8: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify the
Bantype.Ahoy! We only need to use
NonNullableonce because it automatically removes bothnullandundefinedfrom the type for us.✨ Proposed fix
-type Ban = NonNullable<NonNullable<UserMeResponse["ban"]>>; +type Ban = NonNullable<UserMeResponse["ban"]>;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/client/components/BannedModal.ts` at line 8, Update the Ban type alias to apply NonNullable only once to UserMeResponse["ban"], removing the redundant nested wrapper while preserving the resulting non-null ban type.
70-89: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse Tailwind CSS classes instead of inline styles.
Ahoy! I noticed we are using inline styles for this modal. Since we use Tailwind CSS 4 for our UI, let's switch to Tailwind classes to keep our styling clean, idiomatic, and easy to maintain.
✨ Proposed fix
- <div style="max-width: 32rem; line-height: 1.5;"> - <p style="font-weight: 600; font-size: 1.05rem; margin: 0 0 0.5rem;"> + <div class="max-w-lg leading-normal"> + <p class="font-semibold text-lg mb-2"> ${this.categoryLabel(ban.category)} </p> ${ban.reason - ? html`<p style="margin: 0 0 0.5rem;"> + ? html`<p class="mb-2"> ${translateText("ban_notice.reason", { reason: ban.reason })} </p>` : null} - <p style="margin: 0 0 0.75rem;"> + <p class="mb-3"> ${ban.expiresAt ? translateText("ban_notice.until", { date: new Date(ban.expiresAt).toLocaleString(), }) : translateText("ban_notice.permanent")} </p> - <p style="margin: 0; opacity: 0.8;"> + <p class="m-0 opacity-80"> ${translateText("ban_notice.appeal")} </p> </div>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/client/components/BannedModal.ts` around lines 70 - 89, Replace the inline style attributes in the modal content rendered by BannedModal with equivalent Tailwind CSS utility classes, preserving the existing layout, typography, spacing, and opacity for the container and all paragraphs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/client/components/BannedModal.ts`:
- Around line 28-32: Update onUserMeResponse so the no-ban path (detail ===
false or missing detail.ban) clears both this.ban and this.opened before
returning. Preserve the existing assignment of detail.ban for banned users.
---
Nitpick comments:
In `@src/client/components/BannedModal.ts`:
- Line 8: Update the Ban type alias to apply NonNullable only once to
UserMeResponse["ban"], removing the redundant nested wrapper while preserving
the resulting non-null ban type.
- Around line 70-89: Replace the inline style attributes in the modal content
rendered by BannedModal with equivalent Tailwind CSS utility classes, preserving
the existing layout, typography, spacing, and opacity for the container and all
paragraphs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 311ca3de-f493-46d6-871d-916f63f3b91e
📒 Files selected for processing (7)
index.htmlresources/lang/en.jsonsrc/client/Main.tssrc/client/components/BannedModal.tssrc/core/ApiSchemas.tstests/ApiSchemas.test.tstests/client/BannedModal.test.ts
github-project-automation
Bot
moved this from Triage
to Development
in OpenFront Release Management
Addresses review: the no-ban path now resets ban + opened, so an unban that lands mid-session clears the modal (and a later re-ban reopens it). Test added.
2 participants
Why
Completes the player-facing half of the account-bans feature (infra side: openfrontio/infra bans PRs). The API already returns the caller's active ban on
GET /users/@me:…but the game ignored it. A banned player only ever saw the WebSocket close string "Account Banned" — none of the category, reason, or expiry we now store reached them. This surfaces it.
What
ApiSchemas— addbantoUserMeResponseSchema(top-level,.nullable().optional()so an older API is treated as "no ban").categoryis kept as a plain string so a value a newer server introduces degrades gracefully instead of failing the whole@meparse.New
<banned-modal>— reads theuserMeResponsedocument event (same pattern as<marketing-consent-toast>), and when a ban is present opens an<o-modal>showing:ban_notice.category.*),It opens once; the player can close it (the server still enforces the ban — this is informational).
en.json—ban_notice.*strings, including a label for each of the 12ban_categoryvalues.Wired into
Main.ts+index.htmlbeside the marketing toast.Testing
tests/ApiSchemas.test.ts): active temp ban, permanent (null reason/expiry), null / missing (no ban), and a malformed ban with no category.tests/client/BannedModal.test.ts): not-banned → renders nothing;@mefailed → nothing; temp ban shows category + reason + lift date; permanent ban shows "permanent" and omits the reason line; an unknown category falls back to the generic ToS label.tsc --noEmit, ESLint, Prettier, the translation-sync test, and the en.json-sorted test all pass.Notes
bannedtoken); this only adds the reason display, which depends solely on the already-deployed@me.banfield.other, no reason, permanent) render as "Terms of Service violation — This ban is permanent."🤖 Generated with Claude Code