From bed43e0ca7940ff2e393aba3b43dad7cdd04e386 Mon Sep 17 00:00:00 2001 From: Ciaro Vermeire Date: Fri, 12 Apr 2019 21:08:30 +0200 Subject: [PATCH 1/3] Require Laravel 5.7 and PHP 7.1 Due to some changes in the Laravel internals in regard with handling eager loading, upgrading the library to Laravel 5.7 is required. Also, nobody should be on PHP < 7 anyway. So now is a good time, if any... --- composer.json | 12 ++++++------ phpunit.xml | 1 - tests/NodeTest.php | 2 +- tests/ScopedNodeTest.php | 2 +- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/composer.json b/composer.json index c56fea5..ab4d192 100644 --- a/composer.json +++ b/composer.json @@ -1,6 +1,6 @@ { "name": "kalnoy/nestedset", - "description": "Nested Set Model for Laravel 4-5", + "description": "Nested Set Model for Laravel 5.7 and up", "keywords": ["laravel", "nested sets", "nsm", "database", "hierarchy"], "license": "MIT", @@ -12,10 +12,10 @@ ], "require": { - "php": ">=5.5.9", - "illuminate/support": "5.2 - 5.8", - "illuminate/database": "5.2 - 5.8", - "illuminate/events": "5.2 - 5.8" + "php": ">=7.1.3", + "illuminate/support": "~5.7.0|~5.8.0", + "illuminate/database": "~5.7.0|~5.8.0", + "illuminate/events": "~5.7.0|~5.8.0" }, "autoload": { @@ -25,7 +25,7 @@ }, "require-dev": { - "phpunit/phpunit": "4.8.*" + "phpunit/phpunit": "7.*" }, "minimum-stability": "dev", diff --git a/phpunit.xml b/phpunit.xml index 80d7085..9d5784e 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -8,7 +8,6 @@ convertWarningsToExceptions="true" processIsolation="false" stopOnFailure="true" - syntaxCheck="false" > diff --git a/tests/NodeTest.php b/tests/NodeTest.php index 6e23663..e29dadb 100644 --- a/tests/NodeTest.php +++ b/tests/NodeTest.php @@ -3,7 +3,7 @@ use Illuminate\Database\Capsule\Manager as Capsule; use Kalnoy\Nestedset\NestedSet; -class NodeTest extends PHPUnit_Framework_TestCase +class NodeTest extends PHPUnit\Framework\TestCase { public static function setUpBeforeClass() { diff --git a/tests/ScopedNodeTest.php b/tests/ScopedNodeTest.php index dc7ea4f..5fbe96f 100644 --- a/tests/ScopedNodeTest.php +++ b/tests/ScopedNodeTest.php @@ -3,7 +3,7 @@ use Illuminate\Database\Capsule\Manager as Capsule; use Kalnoy\Nestedset\NestedSet; -class ScopedNodeTest extends PHPUnit_Framework_TestCase +class ScopedNodeTest extends PHPUnit\Framework\TestCase { public static function setUpBeforeClass() { From 1f9bb95722cbc096af31fe51f99190b7cc54f466 Mon Sep 17 00:00:00 2001 From: Ciaro Vermeire Date: Fri, 12 Apr 2019 21:24:47 +0200 Subject: [PATCH 2/3] Fix eager loading issue ancestors/descendants relationships (fixes #339, #351, #329, #334, #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. --- phpunit.xml | 1 - src/AncestorsRelation.php | 3 ++- src/BaseRelation.php | 5 +++++ src/DescendantsRelation.php | 3 ++- src/NodeTrait.php | 4 ++-- tests/NodeTest.php | 6 ++++-- tests/ScopedNodeTest.php | 6 ++++-- 7 files changed, 19 insertions(+), 9 deletions(-) diff --git a/phpunit.xml b/phpunit.xml index 9d5784e..1e71a58 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -7,7 +7,6 @@ convertNoticesToExceptions="true" convertWarningsToExceptions="true" processIsolation="false" - stopOnFailure="true" > diff --git a/src/AncestorsRelation.php b/src/AncestorsRelation.php index 089e382..b59fba2 100644 --- a/src/AncestorsRelation.php +++ b/src/AncestorsRelation.php @@ -15,7 +15,8 @@ public function addConstraints() { if ( ! static::$constraints) return; - $this->query->whereAncestorOf($this->parent)->defaultOrder(); + $this->query->whereAncestorOf($this->parent) + ->applyNestedSetScope(); } /** diff --git a/src/BaseRelation.php b/src/BaseRelation.php index 6dbc064..386757d 100644 --- a/src/BaseRelation.php +++ b/src/BaseRelation.php @@ -154,6 +154,11 @@ 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 diff --git a/src/DescendantsRelation.php b/src/DescendantsRelation.php index 5240d3a..4c6457d 100644 --- a/src/DescendantsRelation.php +++ b/src/DescendantsRelation.php @@ -17,7 +17,8 @@ public function addConstraints() { if ( ! static::$constraints) return; - $this->query->whereDescendantOf($this->parent); + $this->query->whereDescendantOf($this->parent) + ->applyNestedSetScope(); } /** diff --git a/src/NodeTrait.php b/src/NodeTrait.php index 167c001..b921984 100644 --- a/src/NodeTrait.php +++ b/src/NodeTrait.php @@ -248,7 +248,7 @@ public function children() */ public function descendants() { - return new DescendantsRelation($this->newScopedQuery(), $this); + return new DescendantsRelation($this->newQuery(), $this); } /** @@ -337,7 +337,7 @@ public function prevNodes() */ public function ancestors() { - return new AncestorsRelation($this->newScopedQuery(), $this); + return new AncestorsRelation($this->newQuery(), $this); } /** diff --git a/tests/NodeTest.php b/tests/NodeTest.php index e29dadb..7c2fbae 100644 --- a/tests/NodeTest.php +++ b/tests/NodeTest.php @@ -870,7 +870,9 @@ public function testFlatTree() $this->assertEquals('galaxy', $tree[3]->name); } - public function testSeveralNodesModelWork() + // Commented, cause there is no assertion here and otherwise the test is marked as risky in PHPUnit 7. + // What's the purpose of this method? @todo: remove/update? + /*public function testSeveralNodesModelWork() { $category = new Category; @@ -883,7 +885,7 @@ public function testSeveralNodesModelWork() $duplicate->name = 'test'; $duplicate->saveAsRoot(); - } + }*/ public function testWhereIsLeaf() { diff --git a/tests/ScopedNodeTest.php b/tests/ScopedNodeTest.php index 5fbe96f..f1c6921 100644 --- a/tests/ScopedNodeTest.php +++ b/tests/ScopedNodeTest.php @@ -193,11 +193,13 @@ protected function assertOtherScopeNotAffected() $this->assertEquals(1, $node->getLft()); } - public function testRebuildsTree() + // Commented, cause there is no assertion here and otherwise the test is marked as risky in PHPUnit 7. + // What's the purpose of this method? @todo: remove/update? + /*public function testRebuildsTree() { $data = []; MenuItem::scoped([ 'menu_id' => 2 ])->rebuildTree($data); - } + }*/ /** * @expectedException LogicException From 10732f165b5d93ab1b1992681e81bb320e05704b Mon Sep 17 00:00:00 2001 From: Ciaro Vermeire Date: Sun, 21 Apr 2019 12:28:36 +0200 Subject: [PATCH 3/3] Split off v5 branch to indicate BC for Laravel 5.7 and 5.8 --- README.markdown | 3 ++- composer.json | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/README.markdown b/README.markdown index 8a3f253..77c313a 100644 --- a/README.markdown +++ b/README.markdown @@ -6,7 +6,8 @@ This is a Laravel 4-5 package for working with trees in relational databases. -* **Laravel 5.5, 5.6, 5.7, 5.8** is supported since v4.3 +* **Laravel 5.7, 5.8** is supported since v5 +* **Laravel 5.5, 5.6** is supported since v4.3 * **Laravel 5.2, 5.3, 5.4** is supported since v4 * **Laravel 5.1** is supported in v3 * **Laravel 4** is supported in v2 diff --git a/composer.json b/composer.json index ab4d192..e126663 100644 --- a/composer.json +++ b/composer.json @@ -33,7 +33,7 @@ "extra": { "branch-alias": { - "dev-master": "v4.2.x-dev" + "dev-master": "v5.0.x-dev" }, "laravel": {