Merge pull request #665 from DirectoryTree/BUG-663

Bug 663 - orWhereNotHas / orFilter + whereNotHas unexpected behaviour
DirectoryTree · Jun 17, 2024 · 518fd4c · 518fd4c
2 parents 523a54e + 6a4ca2c
commit 518fd4c
Show file tree

Hide file tree

Showing 2 changed files with 57 additions and 16 deletions.
diff --git a/src/Testing/EmulatesQueries.php b/src/Testing/EmulatesQueries.php
@@ -81,21 +81,16 @@ public function getEloquentQuery(): EloquentBuilder
     /**
      * Create a new nested query builder with the given state.
      */
-    public function newNestedInstance(Closure $closure = null, string $state = 'and'): static
+    public function newNestedInstance(?Closure $closure = null, string $state = 'and'): static
     {
         $query = $this->newInstance()->nested()->setNestedQueryState($state);
 
         if ($closure) {
-            $closure($query);
+            $this->query->where(function (EloquentBuilder $nested) use ($closure, $query) {
+                $closure($query->setEloquentQuery($nested));
+            });
         }
 
-        // Here we will merge the constraints from the nested
-        // query instance to make sure any bindings are
-        // carried over that were applied to it.
-        $this->query->mergeConstraintsFrom(
-            $query->getEloquentQuery()
-        );
-
         return $query;
     }
 
@@ -157,18 +152,22 @@ public function addFilter($type, array $bindings): static
     {
         $relationMethod = $this->determineRelationMethod($type, $bindings);
 
+        $operator = $bindings['operator'];
+
         // If the relation method is "not has", we will flip it
         // to a "has" filter and change the relation method
         // so database results are retrieved properly.
         if (in_array($relationMethod, ['whereDoesntHave', 'orWhereDoesntHave'])) {
-            $bindings['operator'] = '*';
+            $operator = '*';
         }
 
-        $this->query->{$relationMethod}('attributes', function ($query) use ($bindings) {
+        $this->query->{$relationMethod}('attributes', function ($query) use ($bindings, $operator) {
+            $field = $this->normalizeAttributeName($bindings['field']);
+
             $this->addFilterToDatabaseQuery(
                 $query,
-                $this->normalizeAttributeName($bindings['field']),
-                $bindings['operator'],
+                $field,
+                $operator,
                 $bindings['value']
             );
         });
@@ -195,9 +194,9 @@ protected function determineRelationMethod(string $type, array $bindings): strin
             && $this->nestedState = 'or'
             && $this->fieldIsUsedMultipleTimes($type, $bindings['field'])
         ) {
-            $method = $method == 'whereDoesntHave' ?
-                'orWhereDoesntHave' :
-                'orWhereHas';
+            $method = $method == 'whereDoesntHave'
+                ? 'orWhereDoesntHave'
+                : 'orWhereHas';
         }
 
         return $method;

diff --git a/tests/Feature/Emulator/EmulatedModelQueryTest.php b/tests/Feature/Emulator/EmulatedModelQueryTest.php
@@ -8,6 +8,7 @@
 use LdapRecord\Laravel\Testing\LdapObject;
 use LdapRecord\Laravel\Tests\TestCase;
 use LdapRecord\Models\ActiveDirectory\Group;
+use LdapRecord\Models\ActiveDirectory\OrganizationalUnit;
 use LdapRecord\Models\ActiveDirectory\User;
 use LdapRecord\Models\Entry;
 use LdapRecord\Models\Relations\HasManyIn;
@@ -732,6 +733,47 @@ public function test_domain_scoping()
         $this->assertFalse($alphaUser->is($bravo->first()));
         $this->assertFalse($bravoUser->is($alpha->first()));
     }
+
+    public function test_nested_or_filter()
+    {
+        DirectoryEmulator::setup('default');
+
+        // Create some other models to ensure they are not returned.
+        $customers = OrganizationalUnit::create(['ou' => 'Customers']);
+
+        $customer = (new OrganizationalUnit)->inside($customers);
+        $customer->fill(['ou' => 'Customer']);
+        $customer->save();
+
+        $users = (new OrganizationalUnit)->inside($customer);
+        $users->fill(['ou' => 'Users']);
+        $users->save();
+
+        $userWithMatchingAttribute = User::create([
+            'cn' => 'John',
+            'msExchRecipientTypeDetails' => [1],
+        ]);
+
+        $userWithoutAttribute = User::create([
+            'cn' => 'Jane',
+        ]);
+
+        $userWithoutMatchingAttribute = User::create([
+            'cn' => 'Bob',
+            'msExchRecipientTypeDetails' => [2],
+        ]);
+
+        $results = User::query()->orFilter(function ($query) {
+            $query->where('msExchRecipientTypeDetails', 1);
+            $query->whereNotHas('msExchRecipientTypeDetails');
+        })->get();
+
+        $this->assertCount(2, $results);
+
+        $this->assertTrue($results->contains($userWithoutAttribute));
+        $this->assertTrue($results->contains($userWithMatchingAttribute));
+        $this->assertTrue($results->doesntContain($userWithoutMatchingAttribute));
+    }
 }
 
 class TestModelStub extends Entry