fix: scope null relationship groups without bypassing filters (#20563)

* test: reproduce filters leaking into `null` relationship group scope

* fix: scope `null` relationship groups without leaking filters

* fix: scope empty relationship filters without leaking filters

* fix: scope import relationship resolution columns

* Refactor query logic in SelectFilter

* Delete tests/src/Actions/Imports/ImportColumnTest.php

---------

Co-authored-by: Dan Harrin <git@danharrin.com>

packages/actions/src/Imports/ImportColumn.php

@@ -7,6 +7,7 @@ use Filament\Forms\Components\Select;
 use Filament\Support\Components\Component;
 use Filament\Support\Services\RelationshipJoiner;
 use Illuminate\Contracts\Support\Htmlable;
+use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\Collection;
 use Illuminate\Database\Eloquent\Model;
 use Illuminate\Database\Eloquent\Relations\BelongsTo;
@@ -467,18 +468,20 @@ class ImportColumn extends Component
 
         $resolveUsing = Arr::wrap($resolveUsing);
 
-        $isFirst = true;
+        $relationshipQuery->where(function (Builder $query) use ($resolveUsing, $state): void {
+            $isFirst = true;
 
-        foreach ($resolveUsing as $columnToResolve) {
-            $whereClause = $isFirst ? 'where' : 'orWhere';
+            foreach ($resolveUsing as $columnToResolve) {
+                $whereClause = $isFirst ? 'where' : 'orWhere';
 
-            $relationshipQuery->{$whereClause}(
-                $columnToResolve,
-                $state,
-            );
+                $query->{$whereClause}(
+                    $columnToResolve,
+                    $state,
+                );
 
-            $isFirst = false;
-        }
+                $isFirst = false;
+            }
+        });
 
         return $this->resolvedRelatedRecords[$state] = $relationshipQuery->first();
     }
@@ -522,18 +525,20 @@ class ImportColumn extends Component
 
         $resolveUsing = Arr::wrap($resolveUsing);
 
-        $isFirst = true;
+        $relationshipQuery->where(function (Builder $query) use ($resolveUsing, $state): void {
+            $isFirst = true;
 
-        foreach ($resolveUsing as $columnToResolve) {
-            $whereClause = $isFirst ? 'whereIn' : 'orWhereIn';
+            foreach ($resolveUsing as $columnToResolve) {
+                $whereClause = $isFirst ? 'whereIn' : 'orWhereIn';
 
-            $relationshipQuery->{$whereClause}(
-                $columnToResolve,
-                $state,
-            );
+                $query->{$whereClause}(
+                    $columnToResolve,
+                    $state,
+                );
 
-            $isFirst = false;
-        }
+                $isFirst = false;
+            }
+        });
 
         return $this->resolvedRelatedRecords[$encodedState] = $relationshipQuery->get();
     }

packages/tables/src/Filters/SelectFilter.php

@@ -229,9 +229,11 @@ class SelectFilter extends BaseFilter
             in_array(static::EMPTY_RELATIONSHIP_OPTION_KEY, Arr::wrap($values))
         ) {
             if (filled($filteredValues)) {
-                $query
-                    ->where(fn (Builder $query) => $applyRelationshipScope($query))
-                    ->orWhereDoesntHave($this->getRelationshipName());
+                $query->where(function (Builder $query) use ($applyRelationshipScope): void {
+                    $query
+                        ->where(fn (Builder $query) => $applyRelationshipScope($query))
+                        ->orWhereDoesntHave($this->getRelationshipName());
+                });
             } else {
                 $query->whereDoesntHave($this->getRelationshipName());
             }

packages/tables/src/Grouping/Group.php

@@ -408,10 +408,14 @@ class Group extends Component
         }
 
         if ($relationshipName = $this->getRelationshipName()) {
-            return $query->whereHas(
-                $relationshipName,
-                fn (EloquentBuilder $query) => $this->applyDefaultScopeToQuery($query, $this->getRelationshipAttribute(), $key),
-            )->when(blank($key), fn (EloquentBuilder $query) => $query->orWhereDoesntHave($relationshipName));
+            return $query->where(
+                fn (EloquentBuilder $query) => $query
+                    ->whereHas(
+                        $relationshipName,
+                        fn (EloquentBuilder $query) => $this->applyDefaultScopeToQuery($query, $this->getRelationshipAttribute(), $key),
+                    )
+                    ->when(blank($key), fn (EloquentBuilder $query) => $query->orWhereDoesntHave($relationshipName)),
+            );
         }
 
         return $this->applyDefaultScopeToQuery($query, $column, $key);

tests/src/Tables/Filters/SelectFilterTest.php

@@ -144,6 +144,20 @@ describe('filtering records', function (): void {
             ->assertCanSeeTableRecords($postsWithAuthor1->merge($postsWithoutAuthor))
             ->assertCanNotSeeTableRecords($postsWithAuthor2);
     });
+
+    it('does not allow an empty relationship option to bypass another filter', function (): void {
+        $author = User::factory()->create();
+
+        $matchingPostWithAuthor = Post::factory()->create(['author_id' => $author->getKey(), 'rating' => 1]);
+        $matchingPostWithoutAuthor = Post::factory()->create(['author_id' => null, 'rating' => 1]);
+        $filteredOutPostWithoutAuthor = Post::factory()->create(['author_id' => null, 'rating' => 5]);
+
+        livewire(TestTableWithMultipleEmptyRelationshipFilter::class)
+            ->filterTable('rating', 1)
+            ->filterTable('author', ['__empty', $author->getKey()])
+            ->assertCanSeeTableRecords([$matchingPostWithAuthor, $matchingPostWithoutAuthor])
+            ->assertCanNotSeeTableRecords([$filteredOutPostWithoutAuthor]);
+    });
 });
 
 class TestTableWithSelectFilter extends Component implements HasActions, HasSchemas, Tables\Contracts\HasTable
@@ -222,6 +236,11 @@ class TestTableWithMultipleEmptyRelationshipFilter extends Component implements
                 Tables\Columns\TextColumn::make('author.name'),
             ])
             ->filters([
+                SelectFilter::make('rating')
+                    ->options([
+                        1 => '1 Star',
+                        5 => '5 Stars',
+                    ]),
                 SelectFilter::make('author')
                     ->relationship('author', 'name', hasEmptyOption: true)
                     ->multiple(),

tests/src/Tables/GroupingTest.php

@@ -642,6 +642,26 @@ it('returns an empty array for a non-existent group in array tables', function (
         });
 });
 
+it('only includes filtered records in `getGroupedSelectableTableRecordKeys()` for a `null` relationship group', function (): void {
+    $user = User::factory()->create();
+
+    $publishedPostWithoutAuthor = Post::factory()->create(['author_id' => null, 'is_published' => true]);
+    Post::factory()->create(['author_id' => null, 'is_published' => false]);
+    Post::factory()->create(['author_id' => $user->id, 'is_published' => true]);
+
+    livewire(PostsTable::class)
+        ->set('tableGrouping', 'author.name')
+        ->filterTable('is_published')
+        ->tap(function (Testable $testable) use ($publishedPostWithoutAuthor): void {
+            /** @var PostsTable $livewire */
+            $livewire = $testable->instance();
+
+            $keys = $livewire->getGroupedSelectableTableRecordKeys(null);
+
+            expect($keys)->toBe([(string) $publishedPostWithoutAuthor->id]);
+        });
+});
+
 it('can set `collapsible()` and get with `isCollapsible()`', function (): void {
     expect(Group::make('status')->collapsible()->isCollapsible())->toBeTrue();
 });