From b6130c4e11a492bfc28aca403c4882efe32fb24e Mon Sep 17 00:00:00 2001 From: Dylan Wagstaff Date: Mon, 25 Jun 2018 12:22:27 +1200 Subject: [PATCH] Add unit tests for new ManyManyThrough support The previous commit (9fa9ef89) added support for the new SilverStripe 4 feature of Many Many relationships through an intermediary object. After much head scratching and community testing, the solution was proven to work, however had no automated tests to confirm as such. This commit rectifies that by testing both versioned and unversioned DataObjects in a many_many through style relationship. Some minor tidy and comments were also added as per feedback on the functionality code changes. --- src/GridFieldOrderableRows.php | 23 ++-- tests/GridFieldOrderableRowsTest.php | 43 +++++-- tests/OrderableRowsThroughTest.yml | 30 +++++ tests/OrderableRowsThroughVersionedTest.php | 126 ++++++++++++++++++++ tests/Stub/StubParent.php | 16 +-- tests/Stub/ThroughBelongs.php | 15 +++ tests/Stub/ThroughDefiner.php | 23 ++++ tests/Stub/ThroughIntermediary.php | 21 ++++ 8 files changed, 268 insertions(+), 29 deletions(-) create mode 100644 tests/OrderableRowsThroughTest.yml create mode 100644 tests/OrderableRowsThroughVersionedTest.php create mode 100644 tests/Stub/ThroughBelongs.php create mode 100644 tests/Stub/ThroughDefiner.php create mode 100644 tests/Stub/ThroughIntermediary.php diff --git a/src/GridFieldOrderableRows.php b/src/GridFieldOrderableRows.php index dc92ee3b..492d96f6 100755 --- a/src/GridFieldOrderableRows.php +++ b/src/GridFieldOrderableRows.php @@ -148,6 +148,8 @@ public function getExtraSortFields() * Checks to see if the relationship list is for a type of many_many * * @param SS_List $list + * + * @return bool */ protected function isManyMany(SS_List $list) { @@ -493,7 +495,7 @@ public function handleSave(GridField $grid, DataObjectInterface $record) */ protected function executeReorder(GridField $grid, $sortedIDs) { - if (!is_array($sortedIDs)) { + if (!is_array($sortedIDs) || empty($sortedIDs)) { return false; } $sortField = $this->getSortField(); @@ -540,6 +542,7 @@ protected function executeReorder(GridField $grid, $sortedIDs) $toRelationName = $manipulator->getLocalKey(); $sortlist = DataList::create($joinClass)->filter([ $toRelationName => $items->column('ID'), + // first() is safe as there are earlier checks to ensure our list to sort is valid $fromRelationName => $items->first()->getJoin()->$fromRelationName, ]); $current = $sortlist->map($toRelationName, $sortField)->toArray(); @@ -583,19 +586,19 @@ protected function reorderItems($list, array $values, array $sortedIDs) // - Related item is versioned... // - Through object is versioned: handle as versioned // - Through object is NOT versioned: THROW an error. - $isManyMany = $this->isManyMany($list); - if ($isManyMany && $list instanceof ManyManyThroughList) { + if ($list instanceof ManyManyThroughList) { $listClassVersioned = $class::create()->hasExtension(Versioned::class); // We'll be updating the join class, not the relation class. $class = $this->getManyManyInspector($list)->getJoinClass(); $isVersioned = $class::create()->hasExtension(Versioned::class); + // If one side of the relationship is versioned and the other is not, throw an error. if ($listClassVersioned xor $isVersioned) { throw new Exception( 'ManyManyThrough cannot mismatch Versioning between join class and related class' ); } - } elseif (!$isManyMany) { + } elseif (!$this->isManyMany($list)) { $isVersioned = $class::create()->hasExtension(Versioned::class); } @@ -712,12 +715,12 @@ protected function getSortTableClauseForIds(DataList $list, $ids) } if ($this->isManyMany($list)) { - $intropector = $this->getManyManyInspector($list); + $introspector = $this->getManyManyInspector($list); $extra = $list instanceof ManyManyList ? - $intropector->getExtraFields() : - DataObjectSchema::create()->fieldSpecs($intropector->getJoinClass(), DataObjectSchema::DB_ONLY); - $key = $intropector->getLocalKey(); - $foreignKey = $intropector->getForeignKey(); + $introspector->getExtraFields() : + DataObjectSchema::create()->fieldSpecs($introspector->getJoinClass(), DataObjectSchema::DB_ONLY); + $key = $introspector->getLocalKey(); + $foreignKey = $introspector->getForeignKey(); $foreignID = (int) $list->getForeignID(); if ($extra && array_key_exists($this->getSortField(), $extra)) { @@ -739,7 +742,7 @@ protected function getSortTableClauseForIds(DataList $list, $ids) * these functions are moved to ManyManyThroughQueryManipulator, but otherwise retain * the same signature. * - * @param ManyManyList|ManyManyThroughList + * @param ManyManyList|ManyManyThroughList $list * * @return ManyManyList|ManyManyThroughQueryManipulator */ diff --git a/tests/GridFieldOrderableRowsTest.php b/tests/GridFieldOrderableRowsTest.php index 487d601f..024e7914 100644 --- a/tests/GridFieldOrderableRowsTest.php +++ b/tests/GridFieldOrderableRowsTest.php @@ -14,6 +14,9 @@ use Symbiote\GridFieldExtensions\Tests\Stub\StubSubclass; use Symbiote\GridFieldExtensions\Tests\Stub\StubSubclassOrderedVersioned; use Symbiote\GridFieldExtensions\Tests\Stub\StubUnorderable; +use Symbiote\GridFieldExtensions\Tests\Stub\ThroughDefiner; +use Symbiote\GridFieldExtensions\Tests\Stub\ThroughIntermediary; +use Symbiote\GridFieldExtensions\Tests\Stub\ThroughBelongs; /** * Tests for the {@link GridFieldOrderableRows} component. @@ -23,7 +26,10 @@ class GridFieldOrderableRowsTest extends SapphireTest /** * @var string */ - protected static $fixture_file = 'GridFieldOrderableRowsTest.yml'; + protected static $fixture_file = [ + 'GridFieldOrderableRowsTest.yml', + 'OrderableRowsThroughTest.yml' + ]; /** * @var array @@ -36,27 +42,42 @@ class GridFieldOrderableRowsTest extends SapphireTest StubOrderableChild::class, StubOrderedVersioned::class, StubSubclassOrderedVersioned::class, + ThroughDefiner::class, + ThroughIntermediary::class, + ThroughBelongs::class, ]; - public function testReorderItems() + public function reorderItemsProvider() + { + return [ + [StubParent::class . '.parent', 'MyManyMany', 'ManyManySort'], + [ThroughDefiner::class . '.DefinerOne', 'Belongings', 'Sort'], + ]; + } + + /** + * @dataProvider reorderItemsProvider + */ + public function testReorderItems($fixtureID, $relationName, $sortName) { - $orderable = new GridFieldOrderableRows('ManyManySort'); + $orderable = new GridFieldOrderableRows($sortName); $reflection = new ReflectionMethod($orderable, 'executeReorder'); $reflection->setAccessible(true); - $parent = $this->objFromFixture(StubParent::class, 'parent'); - $config = new GridFieldConfig_RelationEditor(); $config->addComponent($orderable); + list($parentClass, $parentInstanceID) = explode('.', $fixtureID); + $parent = $this->objFromFixture($parentClass, $parentInstanceID); + $grid = new GridField( - 'MyManyMany', - 'My Many Many', - $parent->MyManyMany()->sort('ManyManySort'), + $relationName, + 'Testing Many Many', + $parent->$relationName()->sort($sortName), $config ); - $originalOrder = $parent->MyManyMany()->sort('ManyManySort')->column('ID'); + $originalOrder = $parent->$relationName()->sort($sortName)->column('ID'); $desiredOrder = []; // Make order non-contiguous, and 1-based @@ -68,7 +89,7 @@ public function testReorderItems() $reflection->invoke($orderable, $grid, $desiredOrder); - $newOrder = $parent->MyManyMany()->sort('ManyManySort')->map('ManyManySort', 'ID')->toArray(); + $newOrder = $parent->$relationName()->sort($sortName)->map($sortName, 'ID')->toArray(); $this->assertEquals($desiredOrder, $newOrder); } @@ -157,7 +178,7 @@ public function testReorderItemsSubclassVersioned() foreach ($parent->MyHasManySubclassOrderedVersioned() as $item) { /** @var StubSubclassOrderedVersioned|Versioned $item */ if ($item->stagesDiffer()) { - $this->fail('Unexpected diference found on stages'); + $this->fail('Unexpected difference found on stages'); } } diff --git a/tests/OrderableRowsThroughTest.yml b/tests/OrderableRowsThroughTest.yml new file mode 100644 index 00000000..a0e11ea1 --- /dev/null +++ b/tests/OrderableRowsThroughTest.yml @@ -0,0 +1,30 @@ +Symbiote\GridFieldExtensions\Tests\Stub\ThroughDefiner: + DefinerOne: + DefinerTwo: + +Symbiote\GridFieldExtensions\Tests\Stub\ThroughBelongs: + BelongsOne: + BelongsTwo: + BelongsThree: + +Symbiote\GridFieldExtensions\Tests\Stub\ThroughIntermediary: + One: + Defining: =>Symbiote\GridFieldExtensions\Tests\Stub\ThroughDefiner.DefinerOne + Belonging: =>Symbiote\GridFieldExtensions\Tests\Stub\ThroughBelongs.BelongsOne + Sort: 3 + Two: + Defining: =>Symbiote\GridFieldExtensions\Tests\Stub\ThroughDefiner.DefinerOne + Belonging: =>Symbiote\GridFieldExtensions\Tests\Stub\ThroughBelongs.BelongsTwo + Sort: 2 + Three: + Defining: =>Symbiote\GridFieldExtensions\Tests\Stub\ThroughDefiner.DefinerOne + Belonging: =>Symbiote\GridFieldExtensions\Tests\Stub\ThroughBelongs.BelongsThree + Sort: 1 + Four: + Defining: =>Symbiote\GridFieldExtensions\Tests\Stub\ThroughDefiner.DefinerTwo + Belonging: =>Symbiote\GridFieldExtensions\Tests\Stub\ThroughBelongs.BelongsTwo + Sort: 1 + Five: + Defining: =>Symbiote\GridFieldExtensions\Tests\Stub\ThroughDefiner.DefinerTwo + Belonging: =>Symbiote\GridFieldExtensions\Tests\Stub\ThroughBelongs.BelongsThree + Sort: 2 diff --git a/tests/OrderableRowsThroughVersionedTest.php b/tests/OrderableRowsThroughVersionedTest.php new file mode 100644 index 00000000..694da01b --- /dev/null +++ b/tests/OrderableRowsThroughVersionedTest.php @@ -0,0 +1,126 @@ + [Versioned::class], + ThroughIntermediary::class => [Versioned::class], + ThroughBelongs::class => [Versioned::class], + ]; + + protected $originalReadingMode; + + protected function setUp() + { + parent::setUp(); + $this->orignalReadingMode = Versioned::get_reading_mode(); + } + + protected function tearDown() + { + Versioned::set_reading_mode($this->originalReadingMode); + unset($this->originalReadingMode); + parent::tearDown(); + } + + /** + * Basically the same as GridFieldOrderableRowsTest::testReorderItems + * but with some Versioned calls & checks mixed in. + */ + public function testReorderingSavesAndPublishes() + { + $parent = $this->objFromFixture(ThroughDefiner::class, 'DefinerOne'); + $relationName = 'Belongings'; + $sortName = 'Sort'; + + $orderable = new GridFieldOrderableRows($sortName); + $reflection = new ReflectionMethod($orderable, 'executeReorder'); + $reflection->setAccessible(true); + + $config = new GridFieldConfig_RelationEditor(); + $config->addComponent($orderable); + + // This test data is versioned - ensure we're all published + $parent->publishRecursive(); + // there should be no difference between stages at this point + foreach ($parent->$relationName() as $item) { + $this->assertFalse( + $item->getJoin()->stagesDiffer(), + 'No records should be different from their published versions' + ); + } + + $grid = new GridField( + 'Belongings', + 'Testing Many Many', + $parent->$relationName()->sort($sortName), + $config + ); + + $originalOrder = $parent->$relationName()->sort($sortName)->column('ID'); + // Ring (un)shift by one, e.g. 3,2,1 becomes 1,3,2. + // then string key our new order starting at 1 + $desiredOrder = array_values($originalOrder); + array_unshift($desiredOrder, array_pop($desiredOrder)); + $desiredOrder = array_combine( + range('1', count($desiredOrder)), + $desiredOrder + ); + $this->assertNotEquals($originalOrder, $desiredOrder); + + // Perform the reorder + $reflection->invoke($orderable, $grid, $desiredOrder); + + // Verify draft stage has reordered + Versioned::set_stage(Versioned::DRAFT); + $newOrder = $parent->$relationName()->sort($sortName)->map($sortName, 'ID')->toArray(); + $this->assertEquals($desiredOrder, $newOrder); + + // reorder should have been handled as versioned - there should be a difference between stages now + // by using a ring style shift every item should have a new sort (thus a new version). + $differenceFound = false; + foreach ($parent->$relationName() as $item) { + if ($item->getJoin()->stagesDiffer()) { + $differenceFound = true; + } + } + $this->assertTrue($differenceFound, 'All records should have changes in draft'); + + // Verify live stage has NOT reordered + Versioned::set_stage(Versioned::LIVE); + $sameOrder = $parent->$relationName()->sort($sortName)->column('ID'); + $this->assertEquals($originalOrder, $sameOrder); + + $parent->publishRecursive(); + + foreach ($parent->$relationName() as $item) { + $this->assertFalse( + $item->getJoin()->stagesDiffer(), + 'No records should be different from their published versions anymore' + ); + } + + $newLiveOrder = $parent->$relationName()->sort($sortName)->map($sortName, 'ID')->toArray(); + $this->assertEquals($desiredOrder, $newLiveOrder); + } +} diff --git a/tests/Stub/StubParent.php b/tests/Stub/StubParent.php index 4193c1cd..89043188 100644 --- a/tests/Stub/StubParent.php +++ b/tests/Stub/StubParent.php @@ -7,19 +7,19 @@ class StubParent extends DataObject implements TestOnly { - private static $has_many = array( + private static $has_many = [ 'MyHasMany' => StubOrdered::class, 'MyHasManySubclass' => StubSubclass::class, 'MyHasManySubclassOrderedVersioned' => StubSubclassOrderedVersioned::class, - ); + ]; - private static $many_many = array( - 'MyManyMany' => StubOrdered::class - ); + private static $many_many = [ + 'MyManyMany' => StubOrdered::class, + ]; - private static $many_many_extraFields = array( - 'MyManyMany' => array('ManyManySort' => 'Int') - ); + private static $many_many_extraFields = [ + 'MyManyMany' => ['ManyManySort' => 'Int'], + ]; private static $table_name = 'StubParent'; } diff --git a/tests/Stub/ThroughBelongs.php b/tests/Stub/ThroughBelongs.php new file mode 100644 index 00000000..3c6cb07d --- /dev/null +++ b/tests/Stub/ThroughBelongs.php @@ -0,0 +1,15 @@ + ThroughDefiner::class, + ]; +} diff --git a/tests/Stub/ThroughDefiner.php b/tests/Stub/ThroughDefiner.php new file mode 100644 index 00000000..62ab2023 --- /dev/null +++ b/tests/Stub/ThroughDefiner.php @@ -0,0 +1,23 @@ + [ + 'through' => ThroughIntermediary::class, + 'from' => 'Defining', + 'to' => 'Belonging', + ] + ]; + + private static $owns = [ + 'Belongings' + ]; +} diff --git a/tests/Stub/ThroughIntermediary.php b/tests/Stub/ThroughIntermediary.php new file mode 100644 index 00000000..12a48e10 --- /dev/null +++ b/tests/Stub/ThroughIntermediary.php @@ -0,0 +1,21 @@ + DBInt::class, + ]; + + private static $has_one = [ + 'Defining' => ThroughDefiner::class, + 'Belonging' => ThroughBelongs::class, + ]; +}