Add pluralization - #2520
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2520 +/- ##
=============================================
- Coverage 97.10% 97.06% -0.05%
Complexity 1949 1949
=============================================
Files 483 483
Lines 16656 16686 +30
Branches 2408 2418 +10
=============================================
+ Hits 16174 16196 +22
- Misses 482 490 +8 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
PILOS
 |
||||||||||||||||||||||||||||
| Project |
PILOS
|
| Branch Review |
add-pluralization
|
| Run status |
|
| Run duration | 08m 08s |
| Commit |
|
| Committer | Samuel Weirich |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
 |
0
|
 |
0
|
 |
0
|
 |
0
|
 |
635
|
| View all changes introduced in this branch ↗︎ | |
88ee280 to
bc9287a
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds frontend support for Laravel-style pluralization in localized strings and uses it to display a more informative “Too many requests” message based on the Retry-After header, alongside documentation updates explaining placeholders/pluralization.
Changes:
- Extend the custom
vue-i18nmessage compiler to support Laravel pluralization formats ({n},[n,m],*) and placeholder replacement with:placeholder. - Update the 429 “Too many requests” handler to compute a wait time in minutes and pass it into the translation.
- Update localization documentation and README localization section.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| resources/js/services/Api.js | Passes retry delay into the “too many requests” toast. |
| resources/js/i18n.js | Adds pluralization parsing and switches placeholder replacement to replaceAll. |
| README.md | Points contributors to localization documentation. |
| lang/en/app.php | Converts too_many_requests to a pluralizable string with :count. |
| docs/docs/administration/09-customisation/02-locales.md | Documents placeholders and pluralization format and contribution flow. |
Comments suppressed due to low confidence (1)
resources/js/i18n.js:50
getPluralizationattempts to match each plural segment from the start of the string, but Laravel-style plural strings are commonly written with spaces around the pipe (e.g."{1} ... | [2,*] ..."). In that casepartwill start with whitespace and the regex won’t match, causing the function to fall back to the full unparsed message.
Trim each part before matching.
for (const part of messageParts) {
const match = part.match(regex);
if (match) {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (6)
resources/js/i18n.js:25
- The message compiler mutates the captured
messagetemplate (pluralization + placeholder replacement). Because vue-i18n caches the compiled function per key, later translations can return stale text (e.g., first plural form/placeholder values “stick”), andreplaceAll(":n", ...)can also corrupt other placeholders like:name/:numberOfSelectedUsers.
Build a per-call rendered string and use a boundary-safe replacement so :n doesn’t match the start of longer placeholders.
// If ctx.values has n or count property, we have to handle pluralization
if (ctx.values["n"] !== undefined) {
message = getPluralization(message, ctx.values["n"]);
} else if (ctx.values["count"] !== undefined) {
message = getPluralization(message, ctx.values["count"]);
resources/js/views/RoomsView.vue:148
$t("rooms.auth_throttled", authThrottledFor)assumes the new pluralization format that uses:count, but the other locale files still use:try_again(e.g.lang/de/rooms.php,lang/fr/rooms.php, etc.). In those locales the placeholder will no longer be replaced and will show up in the UI.
To avoid breaking non-English locales, migrate rooms.auth_throttled in all locales to the new pluralization + :count syntax (or keep the placeholder key consistent across locales).
v-if="authThrottledFor > 0"
class="mt-1 text-red-500"
role="alert"
>
{{ $t("rooms.auth_throttled", authThrottledFor) }}
</p>
resources/js/views/Login.vue:210
t("auth.throttle", retryAfter)now relies on the pluralization/count-based frontend syntax, but the non-English locale files still use the old:secondsplaceholder (e.g.lang/de/auth.php,lang/fr/auth.php, etc.). With the current change those locales will render:secondsliterally.
Either migrate the other locales to the new :count + pluralization format, or keep the placeholder name consistent across locales.
const retryAfter = Number(error.response.headers["retry-after"]);
if (data.username) {
errors[id] = {
username: [t("auth.throttle", retryAfter)],
};
} else {
errors[id] = { email: [t("auth.throttle", retryAfter)] };
}
resources/js/components/RoomTabMembersBulkEditButton.vue:6
- These translations now pass a count for pluralization, but several non-English locale strings for the same keys still use the legacy
:numberOfSelectedUsersplaceholder (e.g.lang/de/rooms.php:210,:247, etc.). Those locales will render:numberOfSelectedUsersliterally after this change.
To prevent breaking existing locales, migrate the corresponding keys in lang/*/rooms.php to the new pluralization format using :count.
<Button
v-tooltip="$t('rooms.members.bulk_edit_user', props.userIds.length)"
data-test="room-members-bulk-edit-button"
:aria-label="$t('rooms.members.bulk_edit_user', props.userIds.length)"
:disabled="disabled"
resources/js/components/RoomTabMembersBulkDeleteButton.vue:6
- Same issue as the bulk edit button: non-English locale files still use
:numberOfSelectedUsersfor these keys (e.g.lang/de/rooms.php:212,:251,:253). With the new count-based pluralization calls, those locales will show the placeholder text instead of the selected member count.
Please migrate the affected locale strings to the new pluralization format using :count to keep all locales working.
<Button
v-tooltip="$t('rooms.members.bulk_remove_user', props.userIds.length)"
data-test="room-members-bulk-delete-button"
:aria-label="$t('rooms.members.bulk_remove_user', props.userIds.length)"
:disabled="disabled"
resources/js/components/RoomDetailsList.vue:85
meetings.participant_count_valuewas added only tolang/en/meetings.php. Other locales don’t define this key, so non-English users will see a missing-translation fallback for participant count.
Either add participant_count_value to all locales (preferred), or keep using the existing meetings.participant_count key until the other locales are updated.
<span>
{{
$t(
"meetings.participant_count_value",
props.room.last_meeting.usage.participant_count,
)
}}</span
Fixes #
Type
Checklist
Changes
Other information