From 669ae009341237198d8f700a930bf78a1f50189c Mon Sep 17 00:00:00 2001 From: Craig Anderson Date: Fri, 3 Apr 2026 10:26:08 -0400 Subject: [PATCH] Fix Summarizer Issue with BelongsToMany Pivot Fields (#19625) * Allow Summarizer on BelongsToMany pivot columns * Allow fields that begin with "pivot." * Fix PHPStan error * PHPStan fixes * Add tests * cleanup and fix edge cases * Update Summarizer.php --------- Co-authored-by: Dan Harrin --- .../src/Columns/Summarizers/Summarizer.php | 28 +++++++++---- .../src/Concerns/CanSummarizeRecords.php | 32 ++++++++++++++- .../0013_create_department_ticket_table.php | 2 + tests/src/Fixtures/Models/Ticket.php | 4 +- ...rtmentsWithMixedSummaryRelationManager.php | 27 +++++++++++++ ...rtmentsWithPivotSummaryRelationManager.php | 27 +++++++++++++ .../Panels/Resources/RelationManagerTest.php | 40 +++++++++++++++++++ 7 files changed, 150 insertions(+), 10 deletions(-) create mode 100644 tests/src/Fixtures/Resources/Tickets/RelationManagers/DepartmentsWithMixedSummaryRelationManager.php create mode 100644 tests/src/Fixtures/Resources/Tickets/RelationManagers/DepartmentsWithPivotSummaryRelationManager.php diff --git a/packages/tables/src/Columns/Summarizers/Summarizer.php b/packages/tables/src/Columns/Summarizers/Summarizer.php index ccbb26421c..416dd9b45a 100644 --- a/packages/tables/src/Columns/Summarizers/Summarizer.php +++ b/packages/tables/src/Columns/Summarizers/Summarizer.php @@ -113,23 +113,35 @@ class Summarizer extends ViewComponent implements HasEmbeddedView return $relatedQuery; }, ); - } elseif ($query && str($attribute)->startsWith('pivot.')) { + } elseif ($query) { // https://github.com/filamentphp/filament/issues/12501 + // Handle pivot columns in `BelongsToMany` context. + // This handles two cases: + // 1. Columns defined as `pivot.quantity` (direct pivot access) + // 2. Columns defined as `quantity` in a `RelationManager` (implicit pivot column) - $pivotAttribute = (string) str($attribute) - ->after('pivot.') - ->prepend('pivot_'); + $pivotAttribute = str($attribute)->startsWith('pivot.') + ? (string) str($attribute)->after('pivot.')->prepend('pivot_') + : 'pivot_' . $attribute; $isPivotAttributeSelected = collect($query->getQuery()->getColumns()) ->contains(fn (string $column): bool => str($column)->endsWith(" as {$pivotAttribute}")); - $attribute = $isPivotAttributeSelected ? $pivotAttribute : $attribute; - - // Avoid duplicate columns in the subquery by selecting pivot columns individually. if ($isPivotAttributeSelected) { + $attribute = $pivotAttribute; + } + + // Remove the join table's wildcard to prevent duplicate column + // errors (e.g., both tables have `id`) when the query is used + // as a subquery in MySQL. This applies to all columns in a + // `BelongsToMany` context, not just pivot columns. + $hasPivotColumns = collect($query->getQuery()->getColumns()) + ->contains(fn (string $column): bool => str($column)->contains(' as pivot_')); + + if ($hasPivotColumns && ($joinTable = ($query->getQuery()->joins[0]->table ?? null))) { $query->getQuery()->columns = array_filter( $query->getQuery()->columns, - fn (mixed $column): bool => $column !== "{$query->getQuery()->joins[0]->table}.*", + fn (mixed $column): bool => ! is_string($column) || $column !== "{$joinTable}.*", ); } } diff --git a/packages/tables/src/Concerns/CanSummarizeRecords.php b/packages/tables/src/Concerns/CanSummarizeRecords.php index 1dbb570645..a53a9a8df4 100644 --- a/packages/tables/src/Concerns/CanSummarizeRecords.php +++ b/packages/tables/src/Concerns/CanSummarizeRecords.php @@ -35,6 +35,22 @@ trait CanSummarizeRecords $selects = []; + // https://github.com/filamentphp/filament/issues/19594 + // Check if we have pivot columns selected (`BelongsToMany` `RelationManager` context) + $hasPivotColumns = collect($query->getQuery()->getColumns()) + ->contains(fn (string $column): bool => str($column)->contains(' as pivot_')); + + // If we have pivot columns, remove the join table's wildcard to prevent + // duplicate column errors (e.g., both tables have `id`) when the query + // is used as a subquery in MySQL. Only the join table's wildcard is removed + // so that non-pivot columns from the related model remain accessible. + if ($hasPivotColumns && ($joinTable = ($query->getQuery()->joins[0]->table ?? null))) { + $query->getQuery()->columns = array_filter( + $query->getQuery()->columns, + fn (mixed $column): bool => ! is_string($column) || $column !== "{$joinTable}.*", + ); + } + foreach ($this->getTable()->getVisibleColumns() as $column) { $summarizers = $column->getSummarizers($query); @@ -46,7 +62,21 @@ trait CanSummarizeRecords continue; } - $qualifiedAttribute = $query->getModel()->qualifyColumn($column->getName()); + $columnName = $column->getName(); + + // https://github.com/filamentphp/filament/issues/19594 + // Check if this column is actually a pivot column by looking for its alias. + // Handle both `pivot.amount_total` (explicit) and `quantity` (implicit) column names. + $pivotAlias = str($columnName)->startsWith('pivot.') + ? (string) str($columnName)->after('pivot.')->prepend('pivot_') + : 'pivot_' . $columnName; + $isPivotColumn = $hasPivotColumns && collect($query->getQuery()->getColumns()) + ->contains(fn (string $col): bool => str($col)->endsWith(" as {$pivotAlias}")); + + // Use the pivot alias if this is a pivot column, otherwise qualify with the model's table + $qualifiedAttribute = $isPivotColumn + ? $pivotAlias + : $query->getModel()->qualifyColumn($columnName); foreach ($summarizers as $summarizer) { if ($summarizer->hasQueryModification()) { diff --git a/tests/database/migrations/0013_create_department_ticket_table.php b/tests/database/migrations/0013_create_department_ticket_table.php index 1b4349c8d3..c2bb02f376 100644 --- a/tests/database/migrations/0013_create_department_ticket_table.php +++ b/tests/database/migrations/0013_create_department_ticket_table.php @@ -12,6 +12,8 @@ return new class extends Migration $table->id(); $table->unsignedBigInteger('department_id'); $table->unsignedBigInteger('ticket_id'); + $table->unsignedInteger('quantity')->default(1); + $table->unsignedInteger('price')->default(0); $table->timestamps(); }); } diff --git a/tests/src/Fixtures/Models/Ticket.php b/tests/src/Fixtures/Models/Ticket.php index e1f7f9e358..0458ae1aab 100644 --- a/tests/src/Fixtures/Models/Ticket.php +++ b/tests/src/Fixtures/Models/Ticket.php @@ -20,6 +20,8 @@ class Ticket extends Model public function departments(): BelongsToMany { - return $this->belongsToMany(Department::class); + return $this->belongsToMany(Department::class) + ->withPivot(['quantity', 'price']) + ->withTimestamps(); } } diff --git a/tests/src/Fixtures/Resources/Tickets/RelationManagers/DepartmentsWithMixedSummaryRelationManager.php b/tests/src/Fixtures/Resources/Tickets/RelationManagers/DepartmentsWithMixedSummaryRelationManager.php new file mode 100644 index 0000000000..e571741312 --- /dev/null +++ b/tests/src/Fixtures/Resources/Tickets/RelationManagers/DepartmentsWithMixedSummaryRelationManager.php @@ -0,0 +1,27 @@ +columns([ + TextColumn::make('name') + ->summarize(Count::make('name_count')), + TextColumn::make('quantity') + ->summarize(Sum::make('quantity_sum')), + TextColumn::make('pivot.price') + ->summarize(Sum::make('price_sum')), + ]); + } +} diff --git a/tests/src/Fixtures/Resources/Tickets/RelationManagers/DepartmentsWithPivotSummaryRelationManager.php b/tests/src/Fixtures/Resources/Tickets/RelationManagers/DepartmentsWithPivotSummaryRelationManager.php new file mode 100644 index 0000000000..d34c669c09 --- /dev/null +++ b/tests/src/Fixtures/Resources/Tickets/RelationManagers/DepartmentsWithPivotSummaryRelationManager.php @@ -0,0 +1,27 @@ +columns([ + TextColumn::make('name'), + // Test implicit pivot column (just `quantity`) + TextColumn::make('quantity') + ->summarize(Sum::make('quantity_sum')), + // Test explicit pivot column (`pivot.price`) + TextColumn::make('pivot.price') + ->summarize(Sum::make('price_sum')), + ]); + } +} diff --git a/tests/src/Panels/Resources/RelationManagerTest.php b/tests/src/Panels/Resources/RelationManagerTest.php index e3c42a5415..df4ef75d36 100644 --- a/tests/src/Panels/Resources/RelationManagerTest.php +++ b/tests/src/Panels/Resources/RelationManagerTest.php @@ -20,6 +20,8 @@ use Filament\Tests\Fixtures\Resources\Tickets\Pages\EditTicket; use Filament\Tests\Fixtures\Resources\Tickets\RelationManagers\DepartmentsRelationManager; use Filament\Tests\Fixtures\Resources\Tickets\RelationManagers\DepartmentsRelationManagerWithTabs; use Filament\Tests\Fixtures\Resources\Tickets\RelationManagers\DepartmentsWithAttachTableSelectRelationManager; +use Filament\Tests\Fixtures\Resources\Tickets\RelationManagers\DepartmentsWithMixedSummaryRelationManager; +use Filament\Tests\Fixtures\Resources\Tickets\RelationManagers\DepartmentsWithPivotSummaryRelationManager; use Filament\Tests\Panels\Resources\TestCase; use Illuminate\Auth\Access\Response; use Illuminate\Support\Str; @@ -313,3 +315,41 @@ it('cannot access record for action after record no longer matches tab without ` ->mountTableAction(DeleteAction::class, $department) ->assertTableActionNotMounted(DeleteAction::class); }); + +// https://github.com/filamentphp/filament/issues/19594 +it('can summarize pivot columns in a `BelongsToMany` `RelationManager`', function (): void { + $ticket = Ticket::factory()->create(); + $departments = Department::factory()->count(3)->create(); + + // Attach departments with pivot data + $ticket->departments()->attach($departments[0], ['quantity' => 10, 'price' => 1000]); + $ticket->departments()->attach($departments[1], ['quantity' => 20, 'price' => 2000]); + $ticket->departments()->attach($departments[2], ['quantity' => 30, 'price' => 3000]); + + // Test both implicit (`quantity`) and explicit (`pivot.price`) pivot column summarizers + livewire(DepartmentsWithPivotSummaryRelationManager::class, [ + 'ownerRecord' => $ticket, + 'pageClass' => EditTicket::class, + ]) + ->assertSuccessful() + ->assertTableColumnSummarySet('quantity', 'quantity_sum', 60) // 10 + 20 + 30 + ->assertTableColumnSummarySet('pivot.price', 'price_sum', 6000); // 1000 + 2000 + 3000 +}); + +it('can summarize both pivot and non-pivot columns in a `BelongsToMany` `RelationManager`', function (): void { + $ticket = Ticket::factory()->create(); + $departments = Department::factory()->count(3)->create(); + + $ticket->departments()->attach($departments[0], ['quantity' => 10, 'price' => 1000]); + $ticket->departments()->attach($departments[1], ['quantity' => 20, 'price' => 2000]); + $ticket->departments()->attach($departments[2], ['quantity' => 30, 'price' => 3000]); + + livewire(DepartmentsWithMixedSummaryRelationManager::class, [ + 'ownerRecord' => $ticket, + 'pageClass' => EditTicket::class, + ]) + ->assertSuccessful() + ->assertTableColumnSummarySet('name', 'name_count', 3) + ->assertTableColumnSummarySet('quantity', 'quantity_sum', 60) + ->assertTableColumnSummarySet('pivot.price', 'price_sum', 6000); +});