Skip to content

[PR] Repair stale Filament v5 auth translation keys on Login page#593

Merged
nielsdrost7 merged 2 commits into
developfrom
claude/filament-v5-translations-gd74fc
Jul 9, 2026
Merged

[PR] Repair stale Filament v5 auth translation keys on Login page#593
nielsdrost7 merged 2 commits into
developfrom
claude/filament-v5-translations-gd74fc

Conversation

@nielsdrost7

@nielsdrost7 nielsdrost7 commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

Summary

The custom login page (Modules/Core/Filament/Pages/Auth/Login.php) rendered raw translation keys instead of labels — e.g. filament-panels::pages/auth/login.form.email.label and filament-panels::pages/auth/login.form.password.label showed literally on the sign-in form.

Cause: Filament v5 extracted the auth pages into the filament/auth package and reorganised the login translations. The custom Login page still referenced v4 key paths (filament-panels::pages/auth/login.*), which no longer resolve under v5.

Changes

1. Restore the label repair (originally on fix/repair-stale-filament-translations, commits 4e69878 / 6f3b944, never merged into develop):

  • Drop the explicit ->label(trans('filament-panels::pages/auth/login.form.email.label')) / ...password.label overrides so the email/password fields fall back to Filament's own translated labels.

2. Make the rate-limit throttle notification upgrade-proof:

  • Replace the hand-built throttle Notification — which hardcoded the same fragile ...login.notifications.throttled.* translation keys — with a delegation to the base page's getRateLimitedNotification($exception). Filament resolves the correct keys for whatever version is installed, so this notification won't break again when Filament reshuffles its translation namespaces. This also removes the latent inconsistency in the original repair, where the throttle body was left pointing at the old key path.

Developer Checklist

  • Translations added or updated (if needed)
  • References a row in CHECKLIST.md — n/a (bug fix)
  • Includes appropriate test coverage — none added; change is a translation-key/label repair with no new logic
  • Follows service/DTO/transformer structure (no inline logic)
  • UI follows Filament & Livewire best practices
  • Ran php artisan test — not run in this environment (Composer deps not installed; install timed out)
  • Ran vendor/bin/pint — not run in this environment (Composer deps not installed)

Related Issues

  • Reported by a user: Filament v5 relocated some translation keys, breaking the sign-in form labels.

Notes for Reviewers

  • Verify the sign-in form renders proper "Email"/"Password" labels and no raw filament-panels::... keys.
  • getRateLimitedNotification() is the base Login page's built-in hook (stable across Filament v3–v5; the default authenticate() uses it internally), so delegating to it is the intended extension point rather than re-implementing the notification.
  • I could not run the test suite or Pint here — Composer install timed out in this environment. A local php artisan test + vendor/bin/pint pass before merge is recommended.

Filament v5 extracted the auth pages into the filament/auth package and
moved the login translations from `filament-panels::pages/auth/login.*`
to `filament-panels::auth/pages/login.*`. The custom Login page still
referenced the old keys, so the login form rendered raw translation keys
(e.g. `filament-panels::pages/auth/login.form.email.label`).

- Drop the explicit email/password `->label()` overrides so the fields
  fall back to Filament's translated labels.
- Repoint the rate-limit throttle notification title/body to the new
  `auth/pages/login` namespace path.

Restores the repair originally made on fix/repair-stale-filament-translations
(commits 4e69878, 6f3b944), which was never merged into develop, and also
fixes the throttle body key that the original repair left on the old path.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011MrHa717dy6aoU6Z67iWG2
@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Login.php's authenticate() method updates throttled notification translation keys from filament-panels::pages/auth/login to filament-panels::auth/pages/login namespaces. Separately, TextInput label() calls are removed from the email and password form components.

Changes

Login page updates

Layer / File(s) Summary
Translation key update and label removal
Modules/Core/Filament/Pages/Auth/Login.php
Throttled notification translation namespaces are switched to the auth/pages variant in authenticate(), and label() calls are removed from the email and password TextInput field configurations.

Estimated code review effort: 1 (Trivial) | ~3 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
Title check ✅ Passed The title accurately summarizes the main change: fixing stale Filament v5 auth translation keys on the login page.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/filament-v5-translations-gd74fc

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
Modules/Core/Filament/Pages/Auth/Login.php (1)

25-25: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Double trans() call for the same key can be simplified.

The body line calls trans('filament-panels::auth/pages/login.notifications.throttled') to check array_key_exists('body', ...), then calls trans() again with the full body key. Consider storing the result in a variable to avoid the duplicate lookup.

♻️ Proposed refactor
-                ->body(array_key_exists('body', trans('filament-panels::auth/pages/login.notifications.throttled') ?: []) ? trans('filament-panels::auth/pages/login.notifications.throttled.body', [
-                    'seconds' => $exception->secondsUntilAvailable,
-                    'minutes' => ceil($exception->secondsUntilAvailable / 60),
-                ]) : null)
+                ->body(($throttled = trans('filament-panels::auth/pages/login.notifications.throttled')) && array_key_exists('body', $throttled ?: []) ? trans('filament-panels::auth/pages/login.notifications.throttled.body', [
+                    'seconds' => $exception->secondsUntilAvailable,
+                    'minutes' => ceil($exception->secondsUntilAvailable / 60),
+                ]) : null)
🤖 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 `@Modules/Core/Filament/Pages/Auth/Login.php` at line 25, The throttled
notification body in Login::body is doing two trans() lookups for the same key,
so simplify it by storing the translated throttled message array in a local
variable and reusing it for both the array_key_exists check and the body value.
Update the logic in the login page’s throttled notification builder so the
lookup happens once, while keeping the existing behavior in the Login class
unchanged.
🤖 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.

Nitpick comments:
In `@Modules/Core/Filament/Pages/Auth/Login.php`:
- Line 25: The throttled notification body in Login::body is doing two trans()
lookups for the same key, so simplify it by storing the translated throttled
message array in a local variable and reusing it for both the array_key_exists
check and the body value. Update the logic in the login page’s throttled
notification builder so the lookup happens once, while keeping the existing
behavior in the Login class unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 766c735f-f402-4b39-8a8e-a3015c4a21fc

📥 Commits

Reviewing files that changed from the base of the PR and between dc79079 and 42fafcb.

📒 Files selected for processing (1)
  • Modules/Core/Filament/Pages/Auth/Login.php

Instead of hand-building the rate-limit notification with hardcoded
`filament-panels::...login.notifications.throttled.*` translation keys —
the very keys that broke on the v4 -> v5 upgrade — delegate to the base
Login page's getRateLimitedNotification(). Filament resolves the correct
keys for whatever version is installed, so this notification no longer
breaks when Filament reshuffles its translation namespaces.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011MrHa717dy6aoU6Z67iWG2
@nielsdrost7 nielsdrost7 merged commit ae7c593 into develop Jul 9, 2026
1 of 5 checks passed
@nielsdrost7 nielsdrost7 deleted the claude/filament-v5-translations-gd74fc branch July 9, 2026 17:55
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.

2 participants