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

@@ -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 ($list instanceof ManyManyThroughList) {
-        if ($isManyMany && $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() :
+                $introspector->getExtraFields() :
-                DataObjectSchema::create()->fieldSpecs($intropector->getJoinClass(), DataObjectSchema::DB_ONLY);
+                DataObjectSchema::create()->fieldSpecs($introspector->getJoinClass(), DataObjectSchema::DB_ONLY);
-            $key   = $intropector->getLocalKey();
+            $key   = $introspector->getLocalKey();
-            $foreignKey = $intropector->getForeignKey();
+            $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
      */

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;
+     * @var string
-
+     */
-    protected static $fixture_file = 'GridFieldOrderableRowsTest.yml';
+    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',
+            $relationName,
-            'My Many Many',
+            'Testing Many Many',
-            $parent->MyManyMany()->sort('ManyManySort'),
+            $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);
     }
 }

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

tests/OrderableRowsThroughVersionedTest.php

@@ -0,0 +1,126 @@
+<?php
+
+namespace Symbiote\GridFieldExtensions\Tests;
+
+use ReflectionMethod;
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\Versioned\Versioned;
+use SilverStripe\Forms\GridField\GridField;
+use SilverStripe\Forms\GridField\GridFieldConfig_RelationEditor;
+use Symbiote\GridFieldExtensions\Tests\Stub\ThroughDefiner;
+use Symbiote\GridFieldExtensions\Tests\Stub\ThroughIntermediary;
+use Symbiote\GridFieldExtensions\Tests\Stub\ThroughBelongs;
+use Symbiote\GridFieldExtensions\GridFieldOrderableRows;
+
+class OrderableRowsThroughTest extends SapphireTest
+{
+    protected static $fixture_file = 'OrderableRowsThroughTest.yml';
+
+    protected static $extra_dataobjects = [
+        ThroughDefiner::class,
+        ThroughIntermediary::class,
+        ThroughBelongs::class,
+    ];
+
+    protected static $required_extensions = [
+        ThroughDefiner::class => [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);
+    }
+}

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(
+    private static $many_many = [
-        'MyManyMany' => StubOrdered::class
+        'MyManyMany' => StubOrdered::class,
-    );
+    ];
 
-    private static $many_many_extraFields = array(
+    private static $many_many_extraFields = [
-        'MyManyMany' => array('ManyManySort' => 'Int')
+        'MyManyMany' => ['ManyManySort' => 'Int'],
-    );
+    ];
 
     private static $table_name = 'StubParent';
 }

tests/Stub/ThroughBelongs.php

@@ -0,0 +1,15 @@
+<?php
+
+namespace Symbiote\GridFieldExtensions\Tests\Stub;
+
+use SilverStripe\ORM\DataObject;
+use SilverStripe\Dev\TestOnly;
+
+class ThroughBelongs extends DataObject implements TestOnly
+{
+    private static $table_name = 'BelongsThrough';
+    
+    private static $belongs_many_many = [
+        'Definers' => ThroughDefiner::class,
+    ];
+}

tests/Stub/ThroughDefiner.php

@@ -0,0 +1,23 @@
+<?php
+
+namespace Symbiote\GridFieldExtensions\Tests\Stub;
+
+use SilverStripe\ORM\DataObject;
+use SilverStripe\Dev\TestOnly;
+
+class ThroughDefiner extends DataObject implements TestOnly
+{
+    private static $table_name = 'ManyThrough';
+    
+    private static $many_many = [
+        'Belongings' => [
+            'through' => ThroughIntermediary::class,
+            'from' => 'Defining',
+            'to' => 'Belonging',
+        ]
+    ];
+
+    private static $owns = [
+        'Belongings'
+    ];
+}

tests/Stub/ThroughIntermediary.php

@@ -0,0 +1,21 @@
+<?php
+
+namespace Symbiote\GridFieldExtensions\Tests\Stub;
+
+use SilverStripe\ORM\DataObject;
+use SilverStripe\ORM\FieldType\DBInt;
+use SilverStripe\Dev\TestOnly;
+
+class ThroughIntermediary extends DataObject implements TestOnly
+{
+    private static $table_name = 'IntermediaryThrough';
+
+    private static $db = [
+        'Sort' => DBInt::class,
+    ];
+    
+    private static $has_one = [
+        'Defining' => ThroughDefiner::class,
+        'Belonging' => ThroughBelongs::class,
+    ];
+}