Merge pull request #586 from bethinkpl/fix-eagerLoadingAncestorsWithScope

Fix eager loading ancestors with scope

Author: lazychaser

src/BaseRelation.php

@@ -141,11 +141,6 @@ public function getResults()
      */
     public function addEagerConstraints(array $models)
     {
-        // The first model in the array is always the parent, so add the scope constraints based on that model.
-        // @link https://github.com/laravel/framework/pull/25240
-        // @link https://github.com/lazychaser/laravel-nestedset/issues/351
-        optional($models[0])->applyNestedSetScope($this->query);
-
         $this->query->whereNested(function (Builder $inner) use ($models) {
             // We will use this query in order to apply constraints to the
             // base query builder

src/NodeTrait.php

@@ -1005,7 +1005,8 @@ public function getPrevSibling(array $columns = [ '*' ])
     public function isDescendantOf(self $other)
     {
         return $this->getLft() > $other->getLft() &&
-            $this->getLft() < $other->getRgt();
+            $this->getLft() < $other->getRgt() &&
+            $this->isSameScope($other);
     }
 
     /**
@@ -1201,6 +1202,24 @@ protected function assertSameScope(self $node)
         }
     }
 
+    /**
+     * @param self $node
+     */
+    protected function isSameScope(self $node): bool
+    {
+        if ( ! $scoped = $this->getScopeAttributes()) {
+            return true;
+        }
+
+        foreach ($scoped as $attr) {
+            if ($this->getAttribute($attr) != $node->getAttribute($attr)) {
+                return false;
+            }
+        }
+
+        return true;
+    }
+
     /**
      * @param array|null $except
      *

src/QueryBuilder.php

@@ -87,8 +87,10 @@ public function whereIsRoot()
     public function whereAncestorOf($id, $andSelf = false, $boolean = 'and')
     {
         $keyName = $this->model->getTable() . '.' . $this->model->getKeyName();
+        $model = null;
 
         if (NestedSet::isNode($id)) {
+            $model = $id;
             $value = '?';
 
             $this->query->addBinding($id->getRgt());
@@ -108,7 +110,7 @@ public function whereAncestorOf($id, $andSelf = false, $boolean = 'and')
             $value = '('.$valueQuery->toSql().')';
         }
 
-        $this->query->whereNested(function ($inner) use ($value, $andSelf, $id, $keyName) {
+        $this->query->whereNested(function ($inner) use ($model, $value, $andSelf, $id, $keyName) {
             list($lft, $rgt) = $this->wrappedColumns();
             $wrappedTable = $this->query->getGrammar()->wrapTable($this->model->getTable());
 
@@ -117,9 +119,13 @@ public function whereAncestorOf($id, $andSelf = false, $boolean = 'and')
             if ( ! $andSelf) {
                 $inner->where($keyName, '<>', $id);
             }
+            if ($model !== null) {
+                // we apply scope only when Node was passed as $id.
+                // In other cases, according to docs, query should be scoped() before calling this method
+                $model->applyNestedSetScope($inner);
+            }
         }, $boolean);
 
-
         return $this;
     }
 
@@ -178,12 +184,13 @@ public function ancestorsAndSelf($id, array $columns = [ '*' ])
      * @param array $values
      * @param string $boolean
      * @param bool $not
+     * @param Query $query
      *
      * @return $this
      */
-    public function whereNodeBetween($values, $boolean = 'and', $not = false)
+    public function whereNodeBetween($values, $boolean = 'and', $not = false, $query = null)
     {
-        $this->query->whereBetween($this->model->getTable() . '.' . $this->model->getLftName(), $values, $boolean, $not);
+        ($query ?? $this->query)->whereBetween($this->model->getTable() . '.' . $this->model->getLftName(), $values, $boolean, $not);
 
         return $this;
     }
@@ -217,19 +224,26 @@ public function orWhereNodeBetween($values)
     public function whereDescendantOf($id, $boolean = 'and', $not = false,
                                       $andSelf = false
     ) {
-        if (NestedSet::isNode($id)) {
-            $data = $id->getBounds();
-        } else {
-            $data = $this->model->newNestedSetQuery()
-                                ->getPlainNodeData($id, true);
-        }
+        $this->query->whereNested(function (Query $inner) use ($id, $andSelf, $not) {
+            if (NestedSet::isNode($id)) {
+                $id->applyNestedSetScope($inner);
+                $data = $id->getBounds();
+            } else {
+                // we apply scope only when Node was passed as $id.
+                // In other cases, according to docs, query should be scoped() before calling this method
+                $data = $this->model->newNestedSetQuery()
+                    ->getPlainNodeData($id, true);
+            }
 
-        // Don't include the node
-        if ( ! $andSelf) {
-            ++$data[0];
-        }
+            // Don't include the node
+            if (!$andSelf) {
+                ++$data[0];
+            }
 
-        return $this->whereNodeBetween($data, $boolean, $not);
+            return $this->whereNodeBetween($data, 'and', $not, $inner);
+        }, $boolean);
+
+        return $this;
     }
 
     /**

tests/ScopedNodeTest.php

@@ -219,4 +219,20 @@ public function testInsertingBeforeAnotherScopeFails()
 
         $a->insertAfterNode($b);
     }
+
+    public function testEagerLoadingAncestorsWithScope()
+    {
+        $filteredNodes = MenuItem::where('title', 'menu item 3')->with(['ancestors'])->get();
+
+        $this->assertEquals(2, $filteredNodes->find(5)->ancestors[0]->id);
+        $this->assertEquals(4, $filteredNodes->find(6)->ancestors[0]->id);
+    }
+
+    public function testEagerLoadingDescendantsWithScope()
+    {
+        $filteredNodes = MenuItem::where('title', 'menu item 2')->with(['descendants'])->get();
+
+        $this->assertEquals(5, $filteredNodes->find(2)->descendants[0]->id);
+        $this->assertEquals(6, $filteredNodes->find(4)->descendants[0]->id);
+    }
 }