Just develop to master#282
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>
…ts (#287) * Initial plan * Apply code review fixes from PR #282 Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: nielsdrost7 <47660417+nielsdrost7@users.noreply.github.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro 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 |
|
@nielsdrost7 I've opened a new pull request, #296, to work on those changes. Once the pull request is ready, I'll request review from you. |
Fixed 1 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
- Add missing /* Arrange */ comment to it_creates_a_quote_through_a_modal - Add missing /* Act */ and /* Assert */ comments to same method - Add missing /* Arrange */ comment to it_creates_a_quote - Remove duplicate /* Assert */ comment in it_only_lists_quotes_for_the_current_tenant - Rename widget_shows_only_current_tenant_quotes → it_shows_only_current_tenant_quotes_in_the_widget (noun-before-verb naming violation) - Split forbidden /* Act & Assert */ into separate /* Act */ and /* Assert */ in QuoteDuplicateNumberPreventionTest Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DqwRdtVHYbBGYkENv1D78R
…and document system permissions
…in-specific actions (Closes #489)
…r all 5 roles (Closes #490)
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
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
┌─────┬─────────────────────────────────────────────────────────────────────┬──────────────────────┐
│ # │ Issue │ Fix │
├─────┼─────────────────────────────────────────────────────────────────────┼──────────────────────┤
│ │ database/migrations/2025_06_03_142408_create_sessions_table.php │ Deleted the │
│ 1 │ duplicated Modules/Core's sessions migration — crashed │ duplicate │
│ │ RefreshDatabase and cascaded 300 failures │ │
├─────┼─────────────────────────────────────────────────────────────────────┼──────────────────────┤
│ │ UserService::updateUser(array $validatedInput, $userToUpdate) — │ Swapped to │
│ 2 │ parameter order was reversed from every caller (updateUser($record, │ updateUser(User │
│ │ $data)). Would throw "call to fill() on array" at runtime │ $user, array $data) │
├─────┼─────────────────────────────────────────────────────────────────────┼──────────────────────┤
│ 3 │ Invoice::mailQueue() declared return type Builder but returns │ Fixed to HasMany │
│ │ HasMany │ │
├─────┼─────────────────────────────────────────────────────────────────────┼──────────────────────┤
│ │ Invoice::activities(), attachments(), clientAttachments() all │ Removed all three │
│ 4 │ returned null. clientAttachments() also referenced non-existent │ dead methods │
│ │ $this->status_text and unqualified 'Attachment' string │ │
├─────┼─────────────────────────────────────────────────────────────────────┼──────────────────────┤
│ 5 │ Same dead methods in Quote │ Removed │
├─────┼─────────────────────────────────────────────────────────────────────┼──────────────────────┤
│ │ DeleteBulkAction on UsersTable had no super_admin guard — bulk │ Added │
│ 6 │ delete could wipe all super admins │ reject(super_admin) │
│ │ │ before delete │
├─────┼─────────────────────────────────────────────────────────────────────┼──────────────────────┤
│ 7 │ Modules/Quotes/Listeners/QuoteWasUpdatedListenerTest.php — test │ Moved to │
│ │ file in wrong directory, PHPUnit never found it │ Tests/Feature/ │
└─────┴─────────────────────────────────────────────────────────────────────┴──────────────────────┘
What was extracted to branches
┌─────────────────────────────────────────────────────────────┬────────────────────────────────────┐
│ Orphan │ Branch │
├─────────────────────────────────────────────────────────────┼────────────────────────────────────┤
│ app/Mason/Bricks/ + app/Mason/Collections/ (17 files, │ feature/130-report-builder │
│ survived a "remove" commit) │ (already existed) │
├─────────────────────────────────────────────────────────────┼────────────────────────────────────┤
│ Entire DocumentGroup subsystem (model, resource, service, │ │
│ factory, seeder, migration, observer, tests — 14 files, │ feature/document-groups-cleanup │
│ unregistered dead code, Numbering is the canonical │ (new) │
│ replacement) │ │
└─────────────────────────────────────────────────────────────┴────────────────────────────────────┘
Remaining known issues (not blocking tests, need dedicated work)
should now actually pass (duplicate-number tests) and need un-tagging. SettingsTest (9 tests) is
all-skipped due to a Filament 5 blade rendering issue.
crash at render time, but upload would silently do nothing.
$data['quote_total'] ?? 0 uses raw i Jump to bottom (ctrl+End) ↓ correctly uses the computed value.