Display localized lobby notice text

[ocepkejepintu-droid]

Summary

• use optional lobby notice i18n metadata when present, falling back to the server text for older messages
• add the default English login.error.banned message used by Add localizable metadata to ban notices server#1083
• cover the metadata fallback path without requiring the unreleased commons model fields at compile time

Context

Companion work for FAForever/server#1083 and FAForever/faf-java-commons#277. This keeps the client backward-compatible with the current commons release while allowing newer notice payloads to render through the client translation bundle once the commons/server pieces land.

Issue: FAForever/server#640

Verification

• JAVA_HOME=/Users/yoseph/Codexperiment/.jdks/jdk-25.0.3+9/Contents/Home ./gradlew test — 1518 tests passed, 33 skipped
• JAVA_HOME=/Users/yoseph/Codexperiment/.jdks/jdk-25.0.3+9/Contents/Home ./gradlew test --tests com.faforever.client.remote.ServerAccessorTest.testLocalizedNoticeTextUsesI18nMetadataWhenPresent --tests com.faforever.client.remote.ServerAccessorTest.testLocalizedNoticeTextSupportsMetadataOnlyNotices — 2 focused tests passed
• git diff --check

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR adds reflective i18n localization support for server notices. FafServerAccessor.onNotice now delegates to a new helper method that safely extracts optional getI18nKey() and getI18nArgs() metadata from the notice object using reflection, then resolves the localized text via i18n.getOrDefault(), falling back to the original text when metadata is absent. A new login.error.banned message key is added, and a unit test verifies the localization path.

Changes

Server Notice Localization

Layer / File(s)	Summary
Imports and Dependencies src/main/java/com/faforever/client/remote/FafServerAccessor.java	Added imports for java.lang.reflect.Method and Collection to support reflection-based metadata extraction.
Localization Helper Methods src/main/java/com/faforever/client/remote/FafServerAccessor.java	Added getLocalizedNoticeText(...) which resolves i18n key and arguments via reflection and returns localized text with fallback. Added getOptionalNoticeValue(...) as a generic reflection utility for safe method invocation with exception handling.
Integration into onNotice src/main/java/com/faforever/client/remote/FafServerAccessor.java	Modified onNotice to call getLocalizedNoticeText(noticeMessage, noticeMessage.getText()) instead of directly using the raw message text.
Message Bundle src/main/resources/i18n/messages.properties	Added login.error.banned localization entry with placeholders for ban expiry and reason, including HTML line-break formatting.
Tests and Test Helpers src/test/java/com/faforever/client/remote/ServerAccessorTest.java	Added private LocalizableNoticeInfo record and unit test testLocalizedNoticeTextUsesI18nMetadataWhenPresent to verify that localization uses i18n metadata when available on the notice object.

Sequence Diagram

sequenceDiagram
  participant onNotice
  participant getLocalizedNoticeText
  participant getOptionalNoticeValue
  participant i18n
  onNotice->>getLocalizedNoticeText: noticeMessage, fallbackText
  getLocalizedNoticeText->>getOptionalNoticeValue: noticeMessage, "getI18nKey"
  getOptionalNoticeValue-->>getLocalizedNoticeText: Optional i18nKey
  alt i18nKey present
    getLocalizedNoticeText->>getOptionalNoticeValue: noticeMessage, "getI18nArgs"
    getOptionalNoticeValue-->>getLocalizedNoticeText: Optional i18nArgs
    getLocalizedNoticeText->>i18n: getOrDefault(key, args, fallback)
    i18n-->>getLocalizedNoticeText: localizedText
  else i18nKey absent
    getLocalizedNoticeText-->>getLocalizedNoticeText: use fallbackText
  end
  getLocalizedNoticeText-->>onNotice: finalText

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A notice arrives, we check with care,
Is there i18n metadata to share?
Reflect and resolve with fallback in hand,
Localized messages across every land!
Banned or blessed, the message rings clear. 📢

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% 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
Title check	✅ Passed	The pull request title directly corresponds to the main change: enabling localized display of lobby notice text by using i18n metadata.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/main/java/com/faforever/client/remote/FafServerAccessor.java (1)

283-300: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Null-text guard blocks the new i18n metadata path.

At Line 283, returning early on noticeMessage.getText() == null skips getLocalizedNoticeText(...), so metadata-only notices won’t render.

Proposed fix

-    if (noticeMessage.getText() == null) {
-      return;
-    }
+    String localizedText = getLocalizedNoticeText(noticeMessage, noticeMessage.getText());
+    if (localizedText == null) {
+      return;
+    }
@@
-    notificationService.addNotification(
-        new ServerNotification(i18n.get("messageFromServer"), getLocalizedNoticeText(noticeMessage, noticeMessage.getText()), severity,
-                               Collections.singletonList(new DismissAction(i18n))));
+    notificationService.addNotification(
+        new ServerNotification(i18n.get("messageFromServer"), localizedText, severity,
+                               Collections.singletonList(new DismissAction(i18n))));

🤖 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/main/java/com/faforever/client/remote/FafServerAccessor.java` around
lines 283 - 300, Remove the early return that bails out when
noticeMessage.getText() == null so metadata-only notices are allowed through;
instead let getLocalizedNoticeText(noticeMessage, noticeMessage.getText()) be
called even when getText() is null (or replace the guard with a more specific
check that only returns when there is neither text nor any metadata), then
continue to compute severity and call notificationService.addNotification(new
ServerNotification(..., getLocalizedNoticeText(...), ...)) as before (update any
null-handling inside getLocalizedNoticeText if needed).

🤖 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.

Outside diff comments:
In `@src/main/java/com/faforever/client/remote/FafServerAccessor.java`:
- Around line 283-300: Remove the early return that bails out when
noticeMessage.getText() == null so metadata-only notices are allowed through;
instead let getLocalizedNoticeText(noticeMessage, noticeMessage.getText()) be
called even when getText() is null (or replace the guard with a more specific
check that only returns when there is neither text nor any metadata), then
continue to compute severity and call notificationService.addNotification(new
ServerNotification(..., getLocalizedNoticeText(...), ...)) as before (update any
null-handling inside getLocalizedNoticeText if needed).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b21100b5-7759-45d5-920a-a1792834e5b6

📥 Commits

Reviewing files that changed from the base of the PR and between 1fb72ca and efd9717.

📒 Files selected for processing (3)

• src/main/java/com/faforever/client/remote/FafServerAccessor.java
• src/main/resources/i18n/messages.properties
• src/test/java/com/faforever/client/remote/ServerAccessorTest.java

[ocepkejepintu-droid]

Addressed the null-text edge case from the review in 12f92cb: localized metadata is now resolved before deciding whether to drop a notice, so metadata-only notices can still render.

I also ran the focused tests locally with JDK 25:

./gradlew test --tests com.faforever.client.remote.ServerAccessorTest.testLocalizedNoticeTextUsesI18nMetadataWhenPresent --tests com.faforever.client.remote.ServerAccessorTest.testLocalizedNoticeTextSupportsMetadataOnlyNotices

Result: 2 tests passed. Remote Codacy and CodeRabbit are green again.

[Sheikah45]

The way this is using reflection communicates to me this is a none serious pull request that doesnt understand what it is doing. So I am not going to take the time to review it.