Skip to content

Add localizable metadata to ban notice #1085

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dinhkien0701 wants to merge 3 commits into
base: develop
Choose a base branch
from
Open

Add localizable metadata to ban notice #1085

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0f1cbb5
Add localizable metadata to ban notice
dinhkien0701 May 12, 2026
6baab4c
Fix mypy arg type in QDataStream pack_message
dinhkien0701 May 12, 2026
7ab837e
Update Pipfile.lock hash for pipenv verify
dinhkien0701 May 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Pipfile.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 17 additions & 3 deletions server/exceptions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

from server.timing import datetime_now

BAN_NOTICE_I18N_KEY = "notice.ban"
BAN_APPEAL_EMAIL = "moderation@faforever.com"


class ClientError(Exception):
"""
Expand All @@ -31,12 +34,23 @@ def __init__(self, ban_expiry, ban_reason, *args, **kwargs):
self.ban_expiry = ban_expiry
self.ban_reason = ban_reason

def localization(self):
return {
"i18n_key": BAN_NOTICE_I18N_KEY,
"i18n_args": {
"duration": self._ban_duration_text(),
"reason": self.ban_reason,
"appeal_email": BAN_APPEAL_EMAIL
}
}
Comment on lines +37 to +45
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Include expires_at as an ISO timestamp instead of humanized duration text.

Issue #640 explicitly requires "timestamps are ISO-formatted in UTC (not timedeltas)" to enable client-side localization. The current implementation sends duration as pre-rendered English text (e.g., "for 3 hours" or "forever"), which prevents the client from localizing the duration into the user's language.

Replace duration with expires_at so the client can compute and localize the duration based on the user's locale and current time.

♻️ Proposed refactor 
 def localization(self):
+    ban_expiry_iso = None if self.ban_expiry.year > 9000 else self.ban_expiry.isoformat()
     return {
         "i18n_key": BAN_NOTICE_I18N_KEY,
         "i18n_args": {
-            "duration": self._ban_duration_text(),
+            "expires_at": ban_expiry_iso,
             "reason": self.ban_reason,
             "appeal_email": BAN_APPEAL_EMAIL
         }
     }

Note: Permanent bans (year > 9000) can use None or a sentinel value to indicate "forever" rather than a far-future timestamp.

🤖 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 `@server/exceptions.py` around lines 37 - 45, The localization payload
currently uses a pre-rendered "duration" string from _ban_duration_text();
replace that with an ISO8601 UTC timestamp field named expires_at (or null for
permanent bans) so clients can compute/localize durations. In the localization()
method, stop including "duration" and instead include "expires_at": compute the
ban end as a timezone-aware datetime in UTC (e.g., self.expires_at or derive
from existing ban end property), format it with
.astimezone(timezone.utc).isoformat() (or append 'Z' for UTC) and set None for
permanent/forever bans; keep other keys (BAN_NOTICE_I18N_KEY, ban_reason,
BAN_APPEAL_EMAIL) unchanged and remove calls to _ban_duration_text().


def message(self):
data = self.localization()["i18n_args"]
return (
f"You are banned from FAF {self._ban_duration_text()}. <br>"
f"Reason: <br>{self.ban_reason}<br><br>"
f"You are banned from FAF {data['duration']}. <br>"
f"Reason: <br>{data['reason']}<br><br>"
"<i>If you would like to appeal this ban, please send an email to: "
"moderation@faforever.com</i>"
f"{data['appeal_email']}</i>"
)

def _ban_duration_text(self):
Expand Down
10 changes: 7 additions & 3 deletions server/lobbyconnection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,12 +222,16 @@ async def on_message_received(self, message):
"text": e.message
})
except BanError as e:
await self.send({
ban_notice = {
"command": "notice",
"style": "error",
"text": e.message()
"text": e.message(),
**e.localization()
}
await self.send({
**ban_notice
})
await self.abort(e.message())
await self.abort(ban_notice["text"])
except ClientError as e:
self._logger.warning(
"ClientError[%s]: %s",
Expand Down
2 changes: 1 addition & 1 deletion server/protocol/qdatastream.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def pack_message(*args: str) -> bytes:
raise NotImplementedError("Only string serialization is supported")

msg += QDataStreamProtocol.pack_qstring(arg)
return QDataStreamProtocol.pack_block(msg)
return QDataStreamProtocol.pack_block(bytes(msg))

@staticmethod
def encode_message(message: dict) -> bytes:
Expand Down
16 changes: 14 additions & 2 deletions tests/integration_tests/test_login.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,13 @@ async def test_server_ban(lobby_server, user):
"You are banned from FAF forever. <br>Reason: <br>Test permanent ban"
"<br><br><i>If you would like to appeal this ban, please send an "
"email to: moderation@faforever.com</i>"
)
),
"i18n_key": "notice.ban",
"i18n_args": {
"duration": "forever",
"reason": "Test permanent ban",
"appeal_email": "moderation@faforever.com"
}
}


Expand Down Expand Up @@ -80,7 +86,13 @@ async def test_server_ban_token(lobby_server, user, jwk_priv_key, jwk_kid):
"You are banned from FAF forever. <br>Reason: <br>Test permanent ban"
"<br><br><i>If you would like to appeal this ban, please send an "
"email to: moderation@faforever.com</i>"
)
),
"i18n_key": "notice.ban",
"i18n_args": {
"duration": "forever",
"reason": "Test permanent ban",
"appeal_email": "moderation@faforever.com"
}
}


Expand Down
8 changes: 7 additions & 1 deletion tests/integration_tests/test_server.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -852,7 +852,13 @@ async def test_server_ban_prevents_hosting(lobby_server, database, command):
"You are banned from FAF forever. <br>Reason: <br>Test live ban<br>"
"<br><i>If you would like to appeal this ban, please send an email "
"to: moderation@faforever.com</i>"
)
),
"i18n_key": "notice.ban",
"i18n_args": {
"duration": "forever",
"reason": "Test live ban",
"appeal_email": "moderation@faforever.com"
}
}


Expand Down
Loading