Skip to content

Commit 1f9bb95

Browse files
committed
Fix eager loading issue ancestors/descendants relationships (fixes lazychaser#339, lazychaser#351, lazychaser#329, lazychaser#334, lazychaser#271)
The way eager loading is handled in Laravel has changed since the original implementation of the NestedSet library. It was not possible anymore to apply the scope to eagerly loaded ancestors/descendants relationships. Also removed defaultOrder() from the relations so custom orders can be applied. Sorting should never be a part of a relation anyway, or it would be hard to customize.
1 parent bed43e0 commit 1f9bb95

File tree

7 files changed

+19
-9
lines changed

7 files changed

+19
-9
lines changed

phpunit.xml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77
convertNoticesToExceptions="true"
88
convertWarningsToExceptions="true"
99
processIsolation="false"
10-
stopOnFailure="true"
1110
>
1211
<testsuites>
1312
<testsuite name="Package Test Suite">

src/AncestorsRelation.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ public function addConstraints()
1515
{
1616
if ( ! static::$constraints) return;
1717

18-
$this->query->whereAncestorOf($this->parent)->defaultOrder();
18+
$this->query->whereAncestorOf($this->parent)
19+
->applyNestedSetScope();
1920
}
2021

2122
/**

src/BaseRelation.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,11 @@ public function getResults()
154154
*/
155155
public function addEagerConstraints(array $models)
156156
{
157+
// The first model in the array is always the parent, so add the scope constraints based on that model.
158+
// @link https://github.com/laravel/framework/pull/25240
159+
// @link https://github.com/lazychaser/laravel-nestedset/issues/351
160+
optional($models[0])->applyNestedSetScope($this->query);
161+
157162
$this->query->whereNested(function (Builder $inner) use ($models) {
158163
// We will use this query in order to apply constraints to the
159164
// base query builder

src/DescendantsRelation.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ public function addConstraints()
1717
{
1818
if ( ! static::$constraints) return;
1919

20-
$this->query->whereDescendantOf($this->parent);
20+
$this->query->whereDescendantOf($this->parent)
21+
->applyNestedSetScope();
2122
}
2223

2324
/**

src/NodeTrait.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ public function children()
248248
*/
249249
public function descendants()
250250
{
251-
return new DescendantsRelation($this->newScopedQuery(), $this);
251+
return new DescendantsRelation($this->newQuery(), $this);
252252
}
253253

254254
/**
@@ -337,7 +337,7 @@ public function prevNodes()
337337
*/
338338
public function ancestors()
339339
{
340-
return new AncestorsRelation($this->newScopedQuery(), $this);
340+
return new AncestorsRelation($this->newQuery(), $this);
341341
}
342342

343343
/**

tests/NodeTest.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -870,7 +870,9 @@ public function testFlatTree()
870870
$this->assertEquals('galaxy', $tree[3]->name);
871871
}
872872

873-
public function testSeveralNodesModelWork()
873+
// Commented, cause there is no assertion here and otherwise the test is marked as risky in PHPUnit 7.
874+
// What's the purpose of this method? @todo: remove/update?
875+
/*public function testSeveralNodesModelWork()
874876
{
875877
$category = new Category;
876878
@@ -883,7 +885,7 @@ public function testSeveralNodesModelWork()
883885
$duplicate->name = 'test';
884886
885887
$duplicate->saveAsRoot();
886-
}
888+
}*/
887889

888890
public function testWhereIsLeaf()
889891
{

tests/ScopedNodeTest.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -193,11 +193,13 @@ protected function assertOtherScopeNotAffected()
193193
$this->assertEquals(1, $node->getLft());
194194
}
195195

196-
public function testRebuildsTree()
196+
// Commented, cause there is no assertion here and otherwise the test is marked as risky in PHPUnit 7.
197+
// What's the purpose of this method? @todo: remove/update?
198+
/*public function testRebuildsTree()
197199
{
198200
$data = [];
199201
MenuItem::scoped([ 'menu_id' => 2 ])->rebuildTree($data);
200-
}
202+
}*/
201203

202204
/**
203205
* @expectedException LogicException

0 commit comments

Comments
 (0)