mirror of
https://github.com/filamentphp/filament.git
synced 2026-09-24 15:42:09 +08:00
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 <git@danharrin.com>
This commit is contained in:
co-authored by
Dan Harrin
parent
71ebff1c3e
commit
669ae00934
@@ -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}.*",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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()) {
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
}
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
}
|
||||
|
||||
+27
@@ -0,0 +1,27 @@
|
||||
<?php
|
||||
|
||||
namespace Filament\Tests\Fixtures\Resources\Tickets\RelationManagers;
|
||||
|
||||
use Filament\Resources\RelationManagers\RelationManager;
|
||||
use Filament\Tables\Columns\Summarizers\Count;
|
||||
use Filament\Tables\Columns\Summarizers\Sum;
|
||||
use Filament\Tables\Columns\TextColumn;
|
||||
use Filament\Tables\Table;
|
||||
|
||||
class DepartmentsWithMixedSummaryRelationManager extends RelationManager
|
||||
{
|
||||
protected static string $relationship = 'departments';
|
||||
|
||||
public function table(Table $table): Table
|
||||
{
|
||||
return $table
|
||||
->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')),
|
||||
]);
|
||||
}
|
||||
}
|
||||
+27
@@ -0,0 +1,27 @@
|
||||
<?php
|
||||
|
||||
namespace Filament\Tests\Fixtures\Resources\Tickets\RelationManagers;
|
||||
|
||||
use Filament\Resources\RelationManagers\RelationManager;
|
||||
use Filament\Tables\Columns\Summarizers\Sum;
|
||||
use Filament\Tables\Columns\TextColumn;
|
||||
use Filament\Tables\Table;
|
||||
|
||||
class DepartmentsWithPivotSummaryRelationManager extends RelationManager
|
||||
{
|
||||
protected static string $relationship = 'departments';
|
||||
|
||||
public function table(Table $table): Table
|
||||
{
|
||||
return $table
|
||||
->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')),
|
||||
]);
|
||||
}
|
||||
}
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user