Skip to content

Conversation

@Boy132
Copy link
Member

@Boy132 Boy132 commented Dec 16, 2025

Closes #2007

@Boy132 Boy132 self-assigned this Dec 16, 2025
@coderabbitai
Copy link

coderabbitai bot commented Dec 16, 2025

📝 Walkthrough

Walkthrough

Three server event listeners now compute the recipient user's locale (defaulting to 'en') and pass that locale into translation calls for notification titles, bodies, and action labels so translations use the receiving user's language.

Changes

Cohort / File(s) Change Summary
Notification listeners
app/Listeners/Server/ServerInstalledListener.php, app/Listeners/Server/SubUserAddedListener.php, app/Listeners/Server/SubUserRemovedListener.php
Each listener now derives $locale from the recipient user's language (fallback 'en') and supplies locale: $locale or $locale as the locale argument to trans() calls for notification title, body, and action label translations. Existing control flow and notification delivery remain unchanged.

Pre-merge checks

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: using recipient language for database notifications, which directly addresses the issue of notifications being sent in the wrong language.
Description check ✅ Passed The description references the linked issue (#2007) which documents the problem of notifications being in the wrong language for recipients.
Linked Issues check ✅ Passed The code changes implement locale-aware translations across three listener files (ServerInstalledListener, SubUserAddedListener, SubUserRemovedListener) by extracting the recipient's language preference and passing it to all notification translation calls, directly addressing issue #2007.
Out of Scope Changes check ✅ Passed All changes are focused on adding locale parameters to notification translations in listener files, with no unrelated modifications to control flow, error handling, or other areas.

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.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (3)
app/Listeners/Server/SubUserAddedListener.php (1)

21-22: Standardize locale parameter style for consistency.

The translation calls mix named parameter syntax (locale: $locale on lines 21 and 26) with positional parameter syntax ($locale as the third argument on line 22). While both are valid, using a consistent style improves readability.

Apply this diff to standardize on named parameters:

-            ->body(trans('notifications.user_added.body', ['server' => $event->subuser->server->name], $locale))
+            ->body(trans('notifications.user_added.body', ['server' => $event->subuser->server->name], locale: $locale))

Also applies to: 26-26

app/Listeners/Server/SubUserRemovedListener.php (1)

16-17: Standardize locale parameter style for consistency.

The translation calls mix named parameter syntax (line 16) with positional parameter syntax (line 17). Using a consistent style improves readability.

Apply this diff to standardize on named parameters:

-            ->body(trans('notifications.user_removed.body', ['server' => $event->server->name], $locale))
+            ->body(trans('notifications.user_removed.body', ['server' => $event->server->name], locale: $locale))
app/Listeners/Server/ServerInstalledListener.php (1)

21-22: Standardize locale parameter style for consistency.

The translation calls mix named parameter syntax (locale: $locale on lines 21 and 26) with positional parameter syntax ($locale as the third argument on line 22). Using a consistent style improves readability.

Apply this diff to standardize on named parameters:

-            ->body(trans('server/setting.server_info.server_name', ['name' => $event->server->name], $locale))
+            ->body(trans('server/setting.server_info.server_name', ['name' => $event->server->name], locale: $locale))

Also applies to: 26-26

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 20e105c and ac10469.

📒 Files selected for processing (3)
  • app/Listeners/Server/ServerInstalledListener.php (1 hunks)
  • app/Listeners/Server/SubUserAddedListener.php (1 hunks)
  • app/Listeners/Server/SubUserRemovedListener.php (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: notAreYouScared
Repo: pelican-dev/panel PR: 1671
File: lang/en/server/database.php:22-25
Timestamp: 2025-09-06T20:20:48.706Z
Learning: In the pelican-dev/panel project, translation workflows for non-English languages are handled in separate PRs and processes, so missing translations in other locales should be ignored during code reviews.
🔇 Additional comments (3)
app/Listeners/Server/SubUserAddedListener.php (1)

18-19: LGTM!

The locale extraction correctly retrieves the recipient user's language preference and provides a sensible default fallback.

app/Listeners/Server/SubUserRemovedListener.php (1)

13-14: LGTM!

The locale extraction correctly retrieves the recipient user's language preference with an appropriate default.

app/Listeners/Server/ServerInstalledListener.php (1)

17-18: LGTM!

The locale extraction correctly retrieves the recipient user's language preference with an appropriate default.

@Boy132 Boy132 merged commit 5a47948 into main Dec 17, 2025
25 checks passed
@Boy132 Boy132 deleted the boy132/fix-database-notification-lang branch December 17, 2025 19:34
@github-actions github-actions bot locked and limited conversation to collaborators Dec 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Notification in different language

3 participants