Skip to content

Add localizable fields to ban notices#1090

Open
2ws2khc2zh-rgb wants to merge 2 commits into
FAForever:developfrom
2ws2khc2zh-rgb:bounty/localizable-ban-notice
Open

Add localizable fields to ban notices#1090
2ws2khc2zh-rgb wants to merge 2 commits into
FAForever:developfrom
2ws2khc2zh-rgb:bounty/localizable-ban-notice

Conversation

@2ws2khc2zh-rgb

@2ws2khc2zh-rgb 2ws2khc2zh-rgb commented Jun 11, 2026

Copy link
Copy Markdown

Summary

  • Adds structured localization fields to ban notice messages.
  • Keeps the existing text fallback for backwards compatibility with current clients.
  • Includes the ban reason and ISO-formatted expiry time so clients can render localized ban text themselves.

Validation

  • python -m py_compile server\exceptions.py server\lobbyconnection.py tests\integration_tests\test_login.py
  • python -m pytest tests\integration_tests\test_login.py -k "ban" could not be run locally because pytest is not installed in this environment.

Closes #640

Summary by CodeRabbit

  • Bug Fixes
    • Server ban notifications now include structured details: localization support, ban reason, and ban expiration timestamp, giving users clearer, actionable context when access is restricted.

@coderabbitai

coderabbitai Bot commented Jun 11, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c97da700-28c2-46b3-a9ba-ccf62b0b757f

📥 Commits

Reviewing files that changed from the base of the PR and between 1cbf9b2 and b324978.

📒 Files selected for processing (1)
  • server/exceptions.py

📝 Walkthrough

Walkthrough

The PR replaces hardcoded ban text with a structured, localizable notice payload: BanError.to_notice() returns the notice dict (including localization key, ban reason, and ISO expiry); the lobby connection sends that payload and integration tests were updated to expect the new fields.

Changes

Ban notice localization

Layer / File(s) Summary
BanError notice contract
server/exceptions.py
BanError.to_notice() method returns a structured dict containing the notice command/style, rendered ban HTML text, localization_key, ban_reason, and ISO-formatted ban_expires_at.
Exception handler integration and test validation
server/lobbyconnection.py, tests/integration_tests/test_login.py
The ban exception handler calls to_notice() to send the structured payload; test_server_ban and test_server_ban_token now assert the additional localization_key, ban_reason, and ban_expires_at fields.

🎯 3 (Moderate) | ⏱️ ~20 minutes

"I thumped my foot upon the log,
A ban turned neat, not just a slog.
ISO dates all tucked in tight,
Keys for words to get translation right.
Hop, hop — notices now speak polite!" 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: adding localizable fields to ban notices, which is the core objective across all modified files.
Linked Issues check ✅ Passed The PR fulfills issue #640 requirements by adding ISO-formatted UTC expiry timestamp and structured ban reason/localization fields to replace English ban text strings.
Out of Scope Changes check ✅ Passed All changes are directly scoped to implementing localizable ban notice fields as specified in issue #640, with no unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@2ws2khc2zh-rgb

Copy link
Copy Markdown
Author

Thanks CodeRabbit. I added a docstring for the new BanError.to_notice() helper to address the docstring coverage warning.

Validation still passes with:

python -m py_compile server/exceptions.py server/lobbyconnection.py tests/integration_tests/test_login.py

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.

Replace ban text with localizable message

1 participant