feat: client detail view page with tabbed relation managers#539
Open
nielsdrost7 wants to merge 4 commits into
Open
feat: client detail view page with tabbed relation managers#539nielsdrost7 wants to merge 4 commits into
nielsdrost7 wants to merge 4 commits into
Conversation
New ViewRelation page (Modules/Clients/.../Pages/ViewRelation.php): - Extends Filament\Resources\Pages\ViewRecord - Header actions: Create Invoice (#217), Create Quote (#218), Edit - Registered as 'view' => ViewRelation::route('/{record}') in RelationResource Infolist on RelationResource (client summary section): - 9-field 3-column grid: company_name, trading_name, relation_number, relation_type (badge), relation_status (badge), registered_at, coc_number, vat_number, currency_code Five RelationManagers (tabs on the view page): - InvoicesRelationManager: invoice list + Create Invoice header action - QuotesRelationManager: quote list + Create Quote header action - ExpensesRelationManager: expense list (vendor expenses for this client) - TasksRelationManager: task list - ProjectsRelationManager: project list All five relation managers use existing hasMany relationships already defined on the Relation model (invoices, quotes, expenses, tasks, projects). RelationsTable: add ViewAction row button + import Action/ViewAction/ InvoiceResource/QuoteResource + Create Invoice and Create Quote row actions (the visible entry points from the client list for #191 and #192). Fixes #221 Fixes #217 Fixes #218 https://claude.ai/code/session_01L9apN3AW7b5h7ypmBA5pUg
Add EditAction to the ViewRelation header (opens edit form). Add conditional DeleteAction that is hidden when the client has any linked invoices, quotes, expenses, tasks, or projects (#220). Service-layer guard in RelationService throws RelationHasLinkedRecordsException as a backstop for direct API calls. https://claude.ai/code/session_01L9apN3AW7b5h7ypmBA5pUg
nielsdrost7
force-pushed
the
feature/221-client-detail-view
branch
from
05c4a6f to
7e8d61c
Compare
Tests verify: - ViewRelation page renders without errors - RelationService::deleteRelation() throws RelationHasLinkedRecordsException when the client has linked invoices or quotes - Clients without linked records can be deleted successfully https://claude.ai/code/session_01L9apN3AW7b5h7ypmBA5pUg
This was referenced Jun 14, 2026
Contributor
📝 WalkthroughWalkthroughAdds a ChangesRelation View Page & Linked-Record Guard
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
📋 Issue PlannerLet us write the prompt for your AI agent so you can ship faster (with fewer bugs). View plan for ticket: ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (1)
Modules/Clients/Tests/Feature/ViewRelationTest.php (1)
45-48: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winExercise the delete flow through
ViewRelation, not justRelationService.These cases currently prove the service guard, but they do not verify the page behavior added in this PR: the delete header action wiring, exception handling, and user-facing outcome on
ViewRelation. A broken page action would still pass this suite.Also applies to: 58-61, 70-74
🤖 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/Clients/Tests/Feature/ViewRelationTest.php` around lines 45 - 48, The current tests only exercise RelationService::deleteRelation() directly, so they miss the ViewRelation page behavior introduced in this PR. Update the affected cases to drive the delete flow through ViewRelation, asserting the delete header action wiring, exception handling, and resulting user-facing outcome on the page rather than only expecting RelationHasLinkedRecordsException from the service. Use ViewRelation and its delete action entry points to locate the page-level flow, and keep the existing service guard coverage only as secondary support 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.
Inline comments:
In `@Modules/Clients/Filament/Company/Resources/Relations/Pages/ViewRelation.php`:
- Around line 23-35: The quick-create actions in ViewRelation are using an
unsupported create route on InvoiceResource and QuoteResource, so they cannot
open the intended prefilled forms. Update the Action::make('create_invoice') and
Action::make('create_quote') URL generation to use the resources’ available
index-based contract/navigation pattern instead of getUrl('create', ...), and
make sure the quote action passes prospect_id rather than customer_id to match
QuoteResource linkage and the feature requirement.
In
`@Modules/Clients/Filament/Company/Resources/Relations/RelationManagers/ExpensesRelationManager.php`:
- Around line 13-21: The ExpensesRelationManager table currently defines only
columns, so the relation tab is missing the same row actions available on the
expense list page. Update the table() method in ExpensesRelationManager to also
register the appropriate row actions from the expense list implementation,
keeping the existing columns intact and reusing the same action definitions so
the relation tab behavior matches the main expenses page.
In
`@Modules/Clients/Filament/Company/Resources/Relations/RelationManagers/InvoicesRelationManager.php`:
- Around line 31-35: The create_invoice action in InvoicesRelationManager is
generating a URL to a page that InvoiceResource does not register, so update it
to use a valid InvoiceResource page or add the missing create page to the
resource. Check the InvoiceResource::getUrl('create', ...) call and the
InvoicesRelationManager action setup, then either switch to an exposed page such
as index with the appropriate parameters or extend InvoiceResource to include a
create page that matches this link.
- Around line 16-36: The InvoicesRelationManager::table() setup is missing the
invoice list-page row actions, so the tab cannot behave like the main invoice
table. Update this manager to include the same row action definitions used by
the invoice resource/list page, and ensure the delete action respects the
paid-invoice restriction already enforced elsewhere. Keep the existing columns
and header action, but add the appropriate rowActions() configuration so the tab
matches the module’s invoice behavior.
In
`@Modules/Clients/Filament/Company/Resources/Relations/RelationManagers/ProjectsRelationManager.php`:
- Around line 13-22: The ProjectsRelationManager table currently defines only
columns, so the project detail tab is missing the same row actions available on
the project list page. Update the table() method in ProjectsRelationManager to
include the existing project actions by reusing the appropriate action
definitions from the project resource/list-page (for example, the table actions
or row actions already used elsewhere), and ensure the relation manager exposes
them alongside the current columns.
In
`@Modules/Clients/Filament/Company/Resources/Relations/RelationManagers/QuotesRelationManager.php`:
- Around line 16-36: The QuotesRelationManager table currently defines only
columns and a header action, so the quote tab is missing the row-level actions
from the main quote list. Update the table configuration in
QuotesRelationManager::table to include the same row actions used by the
QuoteResource table (or the relevant action set for this module) so each quote
row can expose the required operations directly in the tab.
- Around line 31-35: The create_quote action is using the wrong parameter and
route target: update the URL in QuotesRelationManager::create_quote to pass the
relation’s prospect identifier instead of customer_id, and point it to a route
that actually exists on QuoteResource since only index is exposed. Make the
change in the Action::make('create_quote') chain so the generated URL matches
the QuoteResource navigation and the quote-prospect linkage.
In
`@Modules/Clients/Filament/Company/Resources/Relations/RelationManagers/TasksRelationManager.php`:
- Around line 13-22: The TasksRelationManager::table configuration only defines
columns, so the task tab is missing the row actions expected to match the task
module list page. Update this table definition to include the same row action
setup used by the task module’s list page, using the relevant action definitions
on the Table configuration so each task row exposes the required operations.
In `@Modules/Clients/Filament/Company/Resources/Relations/RelationResource.php`:
- Around line 71-79: getRelations() in RelationResource currently returns every
relation manager unconditionally, so disabled modules still show up. Update
RelationResource::getRelations() to build the array conditionally by checking
module availability before adding each manager, and gate
InvoicesRelationManager, QuotesRelationManager, ExpensesRelationManager,
TasksRelationManager, and ProjectsRelationManager individually so only enabled
modules are returned.
- Around line 82-87: RelationResource currently exposes an EditAction from
ViewRelation but getPages() only registers index and view, so the edit route is
missing. Update the RelationResource::getPages() mapping to include an edit page
route for the Relation model (using the existing EditRelation page class if
present), or remove the EditAction from ViewRelation so it no longer targets a
nonexistent route.
In
`@Modules/Clients/Filament/Company/Resources/Relations/Tables/RelationsTable.php`:
- Around line 95-107: The row-level quick-create actions in RelationsTable are
pointing at unsupported resource routes, so update the action URLs to use the
correct create targets exposed by InvoiceResource and QuoteResource. Fix the
create_quote action to pass the proper prospect_id parameter instead of
customer_id, and verify the create_invoice action uses the correct
resource/route signature so the Action::make('create_invoice') and
Action::make('create_quote') callbacks resolve successfully.
In `@Modules/Clients/Services/RelationService.php`:
- Around line 107-113: Make relation deletion use a single guarded path through
RelationService::deleteRelation() so linked-record checks cannot be bypassed.
Move the guard inside the transaction and lock the Relation row before
re-checking and deleting, then ensure both RelationsTable::DeleteBulkAction and
EditRelation’s default header delete action route through the same service
method (or an equivalent model observer) instead of deleting directly.
In `@Modules/Clients/Tests/Feature/ViewRelationTest.php`:
- Around line 14-19: Add the missing PHPUnit group annotations to
ViewRelationTest so it matches the surrounding feature test organization. Update
the render-related test in this class to use the smoke group, and tag the
delete-related test cases with the crud group, using the existing test methods
in ViewRelationTest as the place to add the #[Group(...)] attributes.
---
Nitpick comments:
In `@Modules/Clients/Tests/Feature/ViewRelationTest.php`:
- Around line 45-48: The current tests only exercise
RelationService::deleteRelation() directly, so they miss the ViewRelation page
behavior introduced in this PR. Update the affected cases to drive the delete
flow through ViewRelation, asserting the delete header action wiring, exception
handling, and resulting user-facing outcome on the page rather than only
expecting RelationHasLinkedRecordsException from the service. Use ViewRelation
and its delete action entry points to locate the page-level flow, and keep the
existing service guard coverage only as secondary support if needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ee1effed-938f-4a67-b265-70eb5b65b290
📒 Files selected for processing (12)
Modules/Clients/Exceptions/RelationHasLinkedRecordsException.phpModules/Clients/Filament/Company/Resources/Relations/Pages/ViewRelation.phpModules/Clients/Filament/Company/Resources/Relations/RelationManagers/ExpensesRelationManager.phpModules/Clients/Filament/Company/Resources/Relations/RelationManagers/InvoicesRelationManager.phpModules/Clients/Filament/Company/Resources/Relations/RelationManagers/ProjectsRelationManager.phpModules/Clients/Filament/Company/Resources/Relations/RelationManagers/QuotesRelationManager.phpModules/Clients/Filament/Company/Resources/Relations/RelationManagers/TasksRelationManager.phpModules/Clients/Filament/Company/Resources/Relations/RelationResource.phpModules/Clients/Filament/Company/Resources/Relations/Tables/RelationsTable.phpModules/Clients/Services/RelationService.phpModules/Clients/Tests/Feature/ViewRelationTest.phpresources/lang/en/ip.php
Comment on lines
+23
to
+35
| Action::make('create_invoice') | ||
| ->label(trans('ip.create_invoice')) | ||
| ->icon('heroicon-o-document-plus') | ||
| ->url(fn (): string => InvoiceResource::getUrl('create', [ | ||
| 'customer_id' => $this->getRecord()->id, | ||
| ])), | ||
|
|
||
| Action::make('create_quote') | ||
| ->label(trans('ip.create_quote')) | ||
| ->icon('heroicon-o-document-text') | ||
| ->url(fn (): string => QuoteResource::getUrl('create', [ | ||
| 'customer_id' => $this->getRecord()->id, | ||
| ])), |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
These quick-create actions cannot open the intended prefilled forms.
InvoiceResource and QuoteResource only expose index in the supplied resource contracts, so getUrl('create', ...) cannot resolve here. The quote action also needs prospect_id, not customer_id, to match the quote linkage and the feature requirement.
🤖 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/Clients/Filament/Company/Resources/Relations/Pages/ViewRelation.php`
around lines 23 - 35, The quick-create actions in ViewRelation are using an
unsupported create route on InvoiceResource and QuoteResource, so they cannot
open the intended prefilled forms. Update the Action::make('create_invoice') and
Action::make('create_quote') URL generation to use the resources’ available
index-based contract/navigation pattern instead of getUrl('create', ...), and
make sure the quote action passes prospect_id rather than customer_id to match
QuoteResource linkage and the feature requirement.
Comment on lines
+13
to
+21
| public function table(Table $table): Table | ||
| { | ||
| return $table | ||
| ->columns([ | ||
| TextColumn::make('expense_number')->sortable()->searchable(), | ||
| TextColumn::make('expensed_at')->date()->sortable(), | ||
| TextColumn::make('description')->limit(40)->searchable(), | ||
| TextColumn::make('expense_amount')->money()->sortable(), | ||
| ]); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Expense tab omits the corresponding row actions.
This manager only renders columns. The feature requirement for these relation tabs is to carry over the same row actions as the expense list page.
🤖 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/Clients/Filament/Company/Resources/Relations/RelationManagers/ExpensesRelationManager.php`
around lines 13 - 21, The ExpensesRelationManager table currently defines only
columns, so the relation tab is missing the same row actions available on the
expense list page. Update the table() method in ExpensesRelationManager to also
register the appropriate row actions from the expense list implementation,
keeping the existing columns intact and reusing the same action definitions so
the relation tab behavior matches the main expenses page.
Comment on lines
+16
to
+36
| public function table(Table $table): Table | ||
| { | ||
| return $table | ||
| ->columns([ | ||
| TextColumn::make('invoice_number')->sortable()->searchable(), | ||
| TextColumn::make('invoice_status') | ||
| ->badge() | ||
| ->formatStateUsing(fn ($state) => ($state instanceof InvoiceStatus ? $state : InvoiceStatus::tryFrom($state))?->label()) | ||
| ->color(fn ($state) => ($state instanceof InvoiceStatus ? $state : InvoiceStatus::tryFrom($state))?->color() ?? 'secondary') | ||
| ->sortable(), | ||
| TextColumn::make('invoiced_at')->date()->sortable(), | ||
| TextColumn::make('invoice_due_at')->date()->sortable(), | ||
| TextColumn::make('invoice_total')->money()->sortable(), | ||
| ]) | ||
| ->headerActions([ | ||
| Action::make('create_invoice') | ||
| ->label(trans('ip.create_invoice')) | ||
| ->url(fn (): string => InvoiceResource::getUrl('create', [ | ||
| 'customer_id' => $this->getOwnerRecord()->id, | ||
| ])), | ||
| ]); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Invoice tab is missing the module's row actions.
This manager only defines columns and a header action, so it cannot match the invoice list-page row actions or enforce the paid-invoice delete behavior required for this tab.
🤖 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/Clients/Filament/Company/Resources/Relations/RelationManagers/InvoicesRelationManager.php`
around lines 16 - 36, The InvoicesRelationManager::table() setup is missing the
invoice list-page row actions, so the tab cannot behave like the main invoice
table. Update this manager to include the same row action definitions used by
the invoice resource/list page, and ensure the delete action respects the
paid-invoice restriction already enforced elsewhere. Keep the existing columns
and header action, but add the appropriate rowActions() configuration so the tab
matches the module’s invoice behavior.
Comment on lines
+31
to
+35
| Action::make('create_invoice') | ||
| ->label(trans('ip.create_invoice')) | ||
| ->url(fn (): string => InvoiceResource::getUrl('create', [ | ||
| 'customer_id' => $this->getOwnerRecord()->id, | ||
| ])), |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
This action links to a route the invoice resource does not expose.
Modules/Invoices/Filament/Company/Resources/Invoices/InvoiceResource.php only registers index, so InvoiceResource::getUrl('create', ...) has no matching page here.
🤖 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/Clients/Filament/Company/Resources/Relations/RelationManagers/InvoicesRelationManager.php`
around lines 31 - 35, The create_invoice action in InvoicesRelationManager is
generating a URL to a page that InvoiceResource does not register, so update it
to use a valid InvoiceResource page or add the missing create page to the
resource. Check the InvoiceResource::getUrl('create', ...) call and the
InvoicesRelationManager action setup, then either switch to an exposed page such
as index with the appropriate parameters or extend InvoiceResource to include a
create page that matches this link.
Comment on lines
+13
to
+22
| public function table(Table $table): Table | ||
| { | ||
| return $table | ||
| ->columns([ | ||
| TextColumn::make('project_number')->sortable()->searchable(), | ||
| TextColumn::make('project_name')->limit(40)->sortable()->searchable(), | ||
| TextColumn::make('project_status')->sortable()->badge(), | ||
| TextColumn::make('start_at')->date()->sortable(), | ||
| TextColumn::make('end_at')->date()->sortable(), | ||
| ]); |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Project tab omits the corresponding row actions.
This relation manager only defines columns, so it does not satisfy the requirement to reuse the project list-page actions in the detail tab.
🤖 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/Clients/Filament/Company/Resources/Relations/RelationManagers/ProjectsRelationManager.php`
around lines 13 - 22, The ProjectsRelationManager table currently defines only
columns, so the project detail tab is missing the same row actions available on
the project list page. Update the table() method in ProjectsRelationManager to
include the existing project actions by reusing the appropriate action
definitions from the project resource/list-page (for example, the table actions
or row actions already used elsewhere), and ensure the relation manager exposes
them alongside the current columns.
Comment on lines
71
to
+79
| public static function getRelations(): array | ||
| { | ||
| return []; | ||
| return [ | ||
| InvoicesRelationManager::class, | ||
| QuotesRelationManager::class, | ||
| ExpensesRelationManager::class, | ||
| TasksRelationManager::class, | ||
| ProjectsRelationManager::class, | ||
| ]; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Gate these relation managers by module availability.
getRelations() always returns all five tabs, but the requirement is to show only enabled modules. Disabled modules will still appear from this resource.
🤖 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/Clients/Filament/Company/Resources/Relations/RelationResource.php`
around lines 71 - 79, getRelations() in RelationResource currently returns every
relation manager unconditionally, so disabled modules still show up. Update
RelationResource::getRelations() to build the array conditionally by checking
module availability before adding each manager, and gate
InvoicesRelationManager, QuotesRelationManager, ExpensesRelationManager,
TasksRelationManager, and ProjectsRelationManager individually so only enabled
modules are returned.
Comment on lines
82
to
87
| public static function getPages(): array | ||
| { | ||
| return [ | ||
| 'index' => ListRelations::route('/'), | ||
| 'view' => ViewRelation::route('/{record}'), | ||
| ]; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep outline Modules/Clients/Filament/Company/Resources/Relations --view expanded
rg -n "class EditRelation|route\('/\{record\}/edit'\)|EditAction::make" Modules/Clients/Filament/Company/Resources/RelationsRepository: InvoicePlane/InvoicePlane-v2
Length of output: 536
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,180p' Modules/Clients/Filament/Company/Resources/Relations/RelationResource.php
printf '\n---\n'
sed -n '1,200p' Modules/Clients/Filament/Company/Resources/Relations/Pages/ViewRelation.php
printf '\n---\n'
sed -n '1,160p' Modules/Clients/Filament/Company/Resources/Relations/Pages/EditRelation.phpRepository: InvoicePlane/InvoicePlane-v2
Length of output: 8500
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' Modules/Core/Filament/Company/Resources/BaseResource.php
printf '\n---\n'
rg -n "function getPages\(|class .*Resource extends BaseResource|EditRelation::route|view' => ViewRelation::route|edit' => EditRelation::route" Modules/Clients/Filament/Company/Resources Modules/Core/Filament/Company/ResourcesRepository: InvoicePlane/InvoicePlane-v2
Length of output: 2754
Add the missing edit page route
Modules/Clients/Filament/Company/Resources/Relations/RelationResource.php only registers index and view, but ViewRelation still exposes EditAction::make(). Add an edit page to getPages() or remove the header action so it doesn’t point at a missing route.
🤖 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/Clients/Filament/Company/Resources/Relations/RelationResource.php`
around lines 82 - 87, RelationResource currently exposes an EditAction from
ViewRelation but getPages() only registers index and view, so the edit route is
missing. Update the RelationResource::getPages() mapping to include an edit page
route for the Relation model (using the existing EditRelation page class if
present), or remove the EditAction from ViewRelation so it no longer targets a
nonexistent route.
Comment on lines
+95
to
+107
| ViewAction::make('view'), | ||
| Action::make('create_invoice') | ||
| ->label(trans('ip.create_invoice')) | ||
| ->icon('heroicon-o-document-plus') | ||
| ->url(fn (Relation $record): string => InvoiceResource::getUrl('create', [ | ||
| 'customer_id' => $record->id, | ||
| ])), | ||
| Action::make('create_quote') | ||
| ->label(trans('ip.create_quote')) | ||
| ->icon('heroicon-o-document-text') | ||
| ->url(fn (Relation $record): string => QuoteResource::getUrl('create', [ | ||
| 'customer_id' => $record->id, | ||
| ])), |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
The row-level quick-create actions are wired to the wrong targets.
Both actions call getUrl('create') on resources that only expose index, so they cannot resolve as written. The quote action also passes customer_id instead of the required prospect_id.
🤖 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/Clients/Filament/Company/Resources/Relations/Tables/RelationsTable.php`
around lines 95 - 107, The row-level quick-create actions in RelationsTable are
pointing at unsupported resource routes, so update the action URLs to use the
correct create targets exposed by InvoiceResource and QuoteResource. Fix the
create_quote action to pass the proper prospect_id parameter instead of
customer_id, and verify the create_invoice action uses the correct
resource/route signature so the Action::make('create_invoice') and
Action::make('create_quote') callbacks resolve successfully.
Comment on lines
107
to
113
| public function deleteRelation(Relation $relation): Relation | ||
| { | ||
| $this->guardAgainstLinkedRecords($relation); | ||
|
|
||
| DB::beginTransaction(); | ||
| try { | ||
| $relation->delete(); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify relation delete paths delegate to RelationService::deleteRelation()
# or to a shared model-level deletion guard.
rg -n -C3 'DeleteBulkAction::make|DeleteAction::make|->delete\(' Modules/ClientsRepository: InvoicePlane/InvoicePlane-v2
Length of output: 7653
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== RelationService outline ==\n'
ast-grep outline Modules/Clients/Services/RelationService.php --view expanded || true
printf '\n== Relation model references ==\n'
rg -n -C3 'guardAgainstLinkedRecords|delet|linked records|linkedRecords|RelationObserver|observe\(' Modules/Clients Modules/Core | sed -n '1,240p'
printf '\n== Relations table and page actions ==\n'
rg -n -C3 'DeleteBulkAction::make|DeleteAction::make|->action\(|->hidden\(|->visible\(' Modules/Clients/Filament/Company/Resources/RelationsRepository: InvoicePlane/InvoicePlane-v2
Length of output: 23720
Make relation deletion go through one guarded path.
Modules/Clients/Services/RelationService.php:109-113checks linked records before the transaction, so a concurrent insert can slip in; lock the row inside the transaction before re-checking and deleting.Modules/Clients/Filament/Company/Resources/Relations/Tables/RelationsTable.php:123-124still uses plainDeleteBulkAction::make(), andModules/Clients/Filament/Company/Resources/Relations/Pages/EditRelation.php:42uses the default header delete action, so both can bypassRelationService::deleteRelation()unless they’re routed through the same guard or a model observer.
🤖 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/Clients/Services/RelationService.php` around lines 107 - 113, Make
relation deletion use a single guarded path through
RelationService::deleteRelation() so linked-record checks cannot be bypassed.
Move the guard inside the transaction and lock the Relation row before
re-checking and deleting, then ensure both RelationsTable::DeleteBulkAction and
EditRelation’s default header delete action route through the same service
method (or an equivalent model observer) instead of deleting directly.
Comment on lines
+14
to
+19
| use PHPUnit\Framework\Attributes\CoversClass; | ||
| use PHPUnit\Framework\Attributes\Test; | ||
|
|
||
| #[CoversClass(ViewRelation::class)] | ||
| #[CoversClass(RelationService::class)] | ||
| class ViewRelationTest extends AbstractCompanyPanelTestCase |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== File around the class declaration =="
sed -n '1,120p' Modules/Clients/Tests/Feature/ViewRelationTest.php
echo
echo "== Search for #[Group] usage in similar tests =="
rg -n --glob 'Modules/**/Tests/**/*.php' '#\[Group\(' Modules | head -n 80
echo
echo "== Search for test attributes and base test cases in this module =="
rg -n --glob 'Modules/Clients/Tests/**/*.php' 'AbstractCompanyPanelTestCase|AbstractAdminPanelTestCase|#\[Test\]|#\[Group\(' Modules/Clients/TestsRepository: InvoicePlane/InvoicePlane-v2
Length of output: 13480
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== File metadata =="
wc -l Modules/Clients/Tests/Feature/ViewRelationTest.php
echo
echo "== File contents with line numbers =="
cat -n Modules/Clients/Tests/Feature/ViewRelationTest.php | sed -n '1,140p'Repository: InvoicePlane/InvoicePlane-v2
Length of output: 3311
🏁 Script executed:
#!/bin/bash
set -euo pipefail
wc -l Modules/Clients/Tests/Feature/ViewRelationTest.php
echo '---'
cat -n Modules/Clients/Tests/Feature/ViewRelationTest.php | sed -n '1,140p'
echo '---'
rg -n --glob 'Modules/**/Tests/**/*.php' '#\[Group\(' Modules | head -n 80
echo '---'
rg -n --glob 'Modules/Clients/Tests/**/*.php' 'AbstractCompanyPanelTestCase|AbstractAdminPanelTestCase|#\[Test\]|#\[Group\(' Modules/Clients/TestsRepository: InvoicePlane/InvoicePlane-v2
Length of output: 13866
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== File contents =="
cat -n Modules/Clients/Tests/Feature/ViewRelationTest.php | sed -n '1,140p'
echo
echo "== Group attribute usage in tests =="
rg -n --glob 'Modules/**/Tests/**/*.php' '#\[Group\(' Modules | head -n 120
echo
echo "== Similar company-panel feature tests =="
rg -n --glob 'Modules/Clients/Tests/**/*.php' 'AbstractCompanyPanelTestCase|#\[Test\]|#\[Group\(' Modules/Clients/TestsRepository: InvoicePlane/InvoicePlane-v2
Length of output: 16963
Add test group annotations. This class has no #[Group(...)] tags, while the surrounding feature tests use them for suite organization. Tag the render test as smoke and the delete cases as crud to match the existing convention.
🤖 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/Clients/Tests/Feature/ViewRelationTest.php` around lines 14 - 19, Add
the missing PHPUnit group annotations to ViewRelationTest so it matches the
surrounding feature test organization. Update the render-related test in this
class to use the smoke group, and tag the delete-related test cases with the
crud group, using the existing test methods in ViewRelationTest as the place to
add the #[Group(...)] attributes.
Source: Coding guidelines
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #221
Closes #217
Closes #218
What was missing
Clients had no detail/view page. The list was the only entry point, with only Edit and Delete actions.
Changes
ViewRelation page (
Pages/ViewRelation.php)Filament\Resources\Pages\ViewRecord'view' => ViewRelation::route('/{record}')in RelationResourceInfolist on RelationResource
9-field client summary in a 3-column grid:
company_name,trading_name,relation_number,relation_type(badge),relation_status(badge),registered_at,coc_number,vat_number,currency_codeFive RelationManagers (tabs)
InvoicesRelationManagerinvoices(customer_id)QuotesRelationManagerquotes(prospect_id)ExpensesRelationManagerexpenses(vendor_id)TasksRelationManagertasks(customer_id)ProjectsRelationManagerprojects(customer_id)All relationships were already defined on the
Relationmodel — no model changes needed.RelationsTable
ViewActionrow button (navigates to the view page)Action::make('create_invoice')andAction::make('create_quote')row actions pointing to the create pages (from PRs [Clients]: Create a quote for a specific client from the client list #191 and [Clients]: Create an invoice for a specific client from the client list #192) with?customer_id=pre-fillCloses #191
Closes #192
Summary by CodeRabbit
New Features
Bug Fixes
Tests