From bba85470549623f01e86da8c666730179e13932f 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 | 121 +++++++++++++++++-- tests/OrderableRowsThroughTest.yml | 30 +++++ tests/OrderableRowsThroughVersionedTest.php | 126 ++++++++++++++++++++ tests/Stub/StubParent.php | 19 +-- tests/Stub/ThroughBelongs.php | 15 +++ tests/Stub/ThroughDefiner.php | 23 ++++ tests/Stub/ThroughIntermediary.php | 21 ++++ 8 files changed, 346 insertions(+), 32 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 13d6e60..c0c9e87 100755 --- a/src/GridFieldOrderableRows.php +++ b/src/GridFieldOrderableRows.php @@ -148,6 +148,8 @@ class GridFieldOrderableRows extends RequestHandler implements * 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) { @@ -497,7 +499,7 @@ class GridFieldOrderableRows extends RequestHandler implements */ protected function executeReorder(GridField $grid, $sortedIDs) { - if (!is_array($sortedIDs)) { + if (!is_array($sortedIDs) || empty($sortedIDs)) { return false; } $sortField = $this->getSortField(); @@ -544,6 +546,7 @@ class GridFieldOrderableRows extends RequestHandler implements $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(); @@ -587,19 +590,19 @@ class GridFieldOrderableRows extends RequestHandler implements // - 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); } @@ -716,12 +719,12 @@ class GridFieldOrderableRows extends RequestHandler implements } 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)) { @@ -743,7 +746,7 @@ class GridFieldOrderableRows extends RequestHandler implements * 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 d49674d..2f1a54e 100644 --- a/tests/GridFieldOrderableRowsTest.php +++ b/tests/GridFieldOrderableRowsTest.php @@ -12,16 +12,22 @@ use Symbiote\GridFieldExtensions\Tests\Stub\StubOrdered; use Symbiote\GridFieldExtensions\Tests\Stub\StubParent; use Symbiote\GridFieldExtensions\Tests\Stub\StubSubclass; 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. */ class GridFieldOrderableRowsTest extends SapphireTest { - - protected $usesDatabase = true; - - protected static $fixture_file = 'GridFieldOrderableRowsTest.yml'; + /** + * @var string + */ + protected static $fixture_file = [ + 'GridFieldOrderableRowsTest.yml', + 'OrderableRowsThroughTest.yml' + ]; protected static $extra_dataobjects = [ StubParent::class, @@ -29,27 +35,44 @@ class GridFieldOrderableRowsTest extends SapphireTest StubSubclass::class, StubUnorderable::class, StubOrderableChild::class, + StubOrderedVersioned::class, + StubSubclassOrderedVersioned::class, + ThroughDefiner::class, + ThroughIntermediary::class, + ThroughBelongs::class, ]; - public function testReorderItems() + public function reorderItemsProvider() { - $orderable = new GridFieldOrderableRows('ManyManySort'); + return [ + [StubParent::class . '.parent', 'MyManyMany', 'ManyManySort'], + [ThroughDefiner::class . '.DefinerOne', 'Belongings', 'Sort'], + ]; + } + + /** + * @dataProvider reorderItemsProvider + */ + public function testReorderItems($fixtureID, $relationName, $sortName) + { + $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 @@ -61,7 +84,7 @@ class GridFieldOrderableRowsTest extends SapphireTest $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); } @@ -125,5 +148,77 @@ class GridFieldOrderableRowsTest extends SapphireTest 'StubParent_MyManyMany', $orderable->setSortField('ManyManySort')->getSortTable($parent->MyManyMany()) ); + + $this->assertEquals( + 'StubOrderedVersioned', + $orderable->setSortField('Sort')->getSortTable($parent->MyHasManySubclassOrderedVersioned()) + ); + } + + public function testReorderItemsSubclassVersioned() + { + $orderable = new GridFieldOrderableRows('Sort'); + $reflection = new ReflectionMethod($orderable, 'executeReorder'); + $reflection->setAccessible(true); + + $parent = $this->objFromFixture(StubParent::class, 'parent-subclass-ordered-versioned'); + + // make sure all items are published + foreach ($parent->MyHasManySubclassOrderedVersioned() as $item) { + $item->publishRecursive(); + } + + // there should be no difference between stages at this point + $differenceFound = false; + foreach ($parent->MyHasManySubclassOrderedVersioned() as $item) { + /** @var StubSubclassOrderedVersioned|Versioned $item */ + if ($item->stagesDiffer()) { + $this->fail('Unexpected difference found on stages'); + } + } + + // reorder items + $config = new GridFieldConfig_RelationEditor(); + $config->addComponent($orderable); + + $grid = new GridField( + 'TestField', + 'TestField', + $parent->MyHasManySubclassOrderedVersioned()->sort('Sort', 'ASC'), + $config + ); + + $originalOrder = $parent->MyHasManySubclassOrderedVersioned() + ->sort('Sort', 'ASC') + ->column('ID'); + + $desiredOrder = []; + + // Make order non-contiguous, and 1-based + foreach (array_reverse($originalOrder) as $index => $id) { + $desiredOrder[$index * 2 + 1] = $id; + } + + $this->assertNotEquals($originalOrder, $desiredOrder); + + $reflection->invoke($orderable, $grid, $desiredOrder); + + $newOrder = $parent->MyHasManySubclassOrderedVersioned() + ->sort('Sort', 'ASC') + ->map('Sort', 'ID') + ->toArray(); + + $this->assertEquals($desiredOrder, $newOrder); + + // reorder should have been handled as versioned - there should be a difference between stages now + $differenceFound = false; + foreach ($parent->MyHasManySubclassOrderedVersioned() as $item) { + if ($item->stagesDiffer()) { + $differenceFound = true; + break; + } + } + + $this->assertTrue($differenceFound); } } diff --git a/tests/OrderableRowsThroughTest.yml b/tests/OrderableRowsThroughTest.yml new file mode 100644 index 0000000..a0e11ea --- /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 0000000..694da01 --- /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 7d60a11..8904318 100644 --- a/tests/Stub/StubParent.php +++ b/tests/Stub/StubParent.php @@ -7,18 +7,19 @@ use SilverStripe\ORM\DataObject; class StubParent extends DataObject implements TestOnly { - private static $has_many = array( + private static $has_many = [ 'MyHasMany' => StubOrdered::class, - 'MyHasManySubclass' => StubSubclass::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 0000000..3c6cb07 --- /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 0000000..62ab202 --- /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 0000000..12a48e1 --- /dev/null +++ b/tests/Stub/ThroughIntermediary.php @@ -0,0 +1,21 @@ + DBInt::class, + ]; + + private static $has_one = [ + 'Defining' => ThroughDefiner::class, + 'Belonging' => ThroughBelongs::class, + ]; +}