diff --git a/src/GridFieldOrderableRows.php b/src/GridFieldOrderableRows.php index 2d251096..492d96f6 100755 --- a/src/GridFieldOrderableRows.php +++ b/src/GridFieldOrderableRows.php @@ -2,6 +2,7 @@ namespace Symbiote\GridFieldExtensions; +use Exception; use SilverStripe\Control\Controller; use SilverStripe\Control\RequestHandler; use SilverStripe\Core\ClassInfo; @@ -17,9 +18,11 @@ use SilverStripe\ORM\DataList; use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DataObjectInterface; +use SilverStripe\ORM\DataObjectSchema; use SilverStripe\ORM\DB; use SilverStripe\ORM\ManyManyList; -use SilverStripe\ORM\Map; +use SilverStripe\ORM\ManyManyThroughList; +use SilverStripe\ORM\ManyManyThroughQueryManipulator; use SilverStripe\ORM\SS_List; use SilverStripe\ORM\FieldType\DBDatetime; use SilverStripe\Versioned\Versioned; @@ -141,6 +144,18 @@ public function getExtraSortFields() return $this->extraSortFields; } + /** + * 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) + { + return $list instanceof ManyManyList || $list instanceof ManyManyThroughList; + } + /** * Sets extra sort fields to apply before the sort field. * @@ -183,12 +198,19 @@ public function validateSortField(SS_List $list) { $field = $this->getSortField(); + // Check extra fields on many many relation types if ($list instanceof ManyManyList) { $extra = $list->getExtraFields(); if ($extra && array_key_exists($field, $extra)) { return; } + } elseif ($list instanceof ManyManyThroughList) { + $manipulator = $this->getManyManyInspector($list); + $fieldTable = DataObject::getSchema()->tableForField($manipulator->getJoinClass(), $field); + if ($fieldTable) { + return; + } } $classes = ClassInfo::dataClassesFor($list->dataClass()); @@ -199,7 +221,7 @@ public function validateSortField(SS_List $list) } } - throw new \Exception("Couldn't find the sort field '" . $field . "'"); + throw new Exception("Couldn't find the sort field '" . $field . "'"); } /** @@ -217,6 +239,8 @@ public function getSortTable(SS_List $list) if ($extra && array_key_exists($field, $extra)) { return $table; } + } elseif ($list instanceof ManyManyThroughList) { + return $this->getManyManyInspector($list)->getJoinAlias(); } $classes = ClassInfo::dataClassesFor($list->dataClass()); foreach ($classes as $class) { @@ -224,7 +248,7 @@ public function getSortTable(SS_List $list) return DataObject::getSchema()->tableName($class); } } - throw new \Exception("Couldn't find the sort field '$field'"); + throw new Exception("Couldn't find the sort field '$field'"); } public function getURLHandlers($grid) @@ -340,9 +364,10 @@ public function handleReorder($grid, $request) } $list = $grid->getList(); $modelClass = $grid->getModelClass(); - if ($list instanceof ManyManyList && !singleton($modelClass)->canView()) { + $isManyMany = $this->isManyMany($list); + if ($isManyMany && !singleton($modelClass)->canView()) { $this->httpError(403); - } elseif (!($list instanceof ManyManyList) && !singleton($modelClass)->canEdit()) { + } elseif (!$isManyMany && !singleton($modelClass)->canEdit()) { $this->httpError(403); } @@ -364,10 +389,10 @@ public function handleReorder($grid, $request) } /** - * Get mapping of sort value to ID from posted data + * Get mapping of sort value to item ID from posted data (gridfield list state), ordered by sort value. * * @param array $data Raw posted data - * @return array + * @return array [sortIndex => recordID] */ protected function getSortedIDs($data) { @@ -470,10 +495,10 @@ 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; } - $field = $this->getSortField(); + $sortField = $this->getSortField(); $sortterm = ''; if ($this->extraSortFields) { @@ -486,7 +511,7 @@ protected function executeReorder(GridField $grid, $sortedIDs) } } $list = $grid->getList(); - $sortterm .= '"'.$this->getSortTable($list).'"."'.$field.'"'; + $sortterm .= '"'.$this->getSortTable($list).'"."'.$sortField.'"'; $items = $list->filter('ID', $sortedIDs)->sort($sortterm); // Ensure that each provided ID corresponded to an actual object. @@ -507,11 +532,22 @@ protected function executeReorder(GridField $grid, $sortedIDs) if (isset($record->_SortColumn0)) { $current[$record->ID] = $record->_SortColumn0; } else { - $current[$record->ID] = $record->$field; + $current[$record->ID] = $record->$sortField; } } + } elseif ($items instanceof ManyManyThroughList) { + $manipulator = $this->getManyManyInspector($list); + $joinClass = $manipulator->getJoinClass(); + $fromRelationName = $manipulator->getForeignKey(); + $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(); } else { - $current = $items->map('ID', $field)->toArray(); + $current = $items->map('ID', $sortField)->toArray(); } // Perform the actual re-ordering. @@ -519,35 +555,51 @@ protected function executeReorder(GridField $grid, $sortedIDs) return true; } + /** + * @param SS_List $list + * @param array $values **UNUSED** [listItemID => currentSortValue]; + * @param array $sortedIDs [newSortValue => listItemID] + */ protected function reorderItems($list, array $values, array $sortedIDs) { + // setup $sortField = $this->getSortField(); - /** @var SS_List $map */ - $map = $list->map('ID', $sortField); - //fix for versions of SS that return inconsistent types for `map` function - if ($map instanceof Map) { - $map = $map->toArray(); - } + $class = $list->dataClass(); + // The problem is that $sortedIDs is a list of the _related_ item IDs, which causes trouble + // with ManyManyThrough, where we need the ID of the _join_ item in order to set the value. + $itemToSortReference = ($list instanceof ManyManyThroughList) ? 'getJoin' : 'Me'; + $currentSortList = $list->map('ID', $itemToSortReference)->toArray(); - // If not a ManyManyList and using versioning, detect it. + // sanity check. $this->validateSortField($list); - $isVersioned = false; - $class = $list->dataClass(); + $isVersioned = false; // check if sort column is present on the model provided by dataClass() and if it's versioned // cases: // Model has sort column and is versioned - handle as versioned // Model has sort column and is NOT versioned - handle as NOT versioned // Model doesn't have sort column because sort column is on ManyManyList - handle as NOT versioned - - // try to match table name, note that we have to cover the case where the table which has the sort column - // belongs to ancestor of the object which is populating the list - $classes = ClassInfo::ancestry($class, true); - foreach ($classes as $currentClass) { - if (DataObject::getSchema()->tableName($currentClass) == $this->getSortTable($list)) { - $isVersioned = $class::has_extension(Versioned::class); - break; + // Model doesn't have sort column because sort column is on ManyManyThroughList... + // - Related item is not versioned: + // - Through object is versioned: THROW an error. + // - Through object is NOT versioned: handle as NOT versioned + // - Related item is versioned... + // - Through object is versioned: handle as versioned + // - Through object is NOT versioned: THROW an error. + 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 (!$this->isManyMany($list)) { + $isVersioned = $class::create()->hasExtension(Versioned::class); } // Loop through each item, and update the sort values which do not @@ -556,22 +608,22 @@ protected function reorderItems($list, array $values, array $sortedIDs) $sortTable = $this->getSortTable($list); $now = DBDatetime::now()->Rfc2822(); $additionalSQL = ''; - $baseTable = DataObject::getSchema()->baseDataTable($list->dataClass()); + $baseTable = DataObject::getSchema()->baseDataTable($class); $isBaseTable = ($baseTable == $sortTable); if (!$list instanceof ManyManyList && $isBaseTable) { $additionalSQL = ", \"LastEdited\" = '$now'"; } - foreach ($sortedIDs as $sortValue => $id) { - if ($map[$id] != $sortValue) { + foreach ($sortedIDs as $newSortValue => $targetRecordID) { + if ($currentSortList[$targetRecordID]->$sortField != $newSortValue) { DB::query(sprintf( 'UPDATE "%s" SET "%s" = %d%s WHERE %s', $sortTable, $sortField, - $sortValue, + $newSortValue, $additionalSQL, - $this->getSortTableClauseForIds($list, $id) + $this->getSortTableClauseForIds($list, $targetRecordID) )); if (!$isBaseTable && !$list instanceof ManyManyList) { @@ -579,20 +631,22 @@ protected function reorderItems($list, array $values, array $sortedIDs) 'UPDATE "%s" SET "LastEdited" = \'%s\' WHERE %s', $baseTable, $now, - $this->getSortTableClauseForIds($list, $id) + $this->getSortTableClauseForIds($list, $targetRecordID) )); } } } } else { // For versioned objects, modify them with the ORM so that the - // *_versions table is updated. This ensures re-ordering works + // *_Versions table is updated. This ensures re-ordering works // similar to the SiteTree where you change the position, and then // you go into the record and publish it. - foreach ($sortedIDs as $sortValue => $id) { - if ($map[$id] != $sortValue) { - $record = $class::get()->byID($id); - $record->$sortField = $sortValue; + foreach ($sortedIDs as $newSortValue => $targetRecordID) { + // either the list data class (has_many, (belongs_)many_many) + // or the intermediary join class (many_many through) + $record = $currentSortList[$targetRecordID]; + if ($record->$sortField != $newSortValue) { + $record->$sortField = $newSortValue; $record->write(); } } @@ -629,7 +683,7 @@ protected function populateSortValues(DataList $list) $this->getSortTableClauseForIds($list, $id) )); - if (!$isBaseTable && !$list instanceof ManyManyList) { + if (!$isBaseTable && !$this->isManyMany($list)) { DB::query(sprintf( 'UPDATE "%s" SET "LastEdited" = \'%s\' WHERE %s', $baseTable, @@ -640,6 +694,18 @@ protected function populateSortValues(DataList $list) } } + /** + * Forms a WHERE clause for the table the sort column is defined on. + * e.g. ID = 5 + * e.g. ID IN(5, 8, 10) + * e.g. SortOrder = 5 AND RelatedThing.ID = 3 + * e.g. SortOrder IN(5, 8, 10) AND RelatedThing.ID = 3 + * + * @param DataList $list + * @param int|string|array $ids a single number, or array of numbers + * + * @return string + */ protected function getSortTableClauseForIds(DataList $list, $ids) { if (is_array($ids)) { @@ -648,10 +714,13 @@ protected function getSortTableClauseForIds(DataList $list, $ids) $value = '= ' . (int) $ids; } - if ($list instanceof ManyManyList) { - $extra = $list->getExtraFields(); - $key = $list->getLocalKey(); - $foreignKey = $list->getForeignKey(); + if ($this->isManyMany($list)) { + $introspector = $this->getManyManyInspector($list); + $extra = $list instanceof ManyManyList ? + $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)) { @@ -667,4 +736,27 @@ protected function getSortTableClauseForIds(DataList $list, $ids) return "\"ID\" $value"; } + + /** + * A ManyManyList defines functions such as getLocalKey, however on ManyManyThroughList + * these functions are moved to ManyManyThroughQueryManipulator, but otherwise retain + * the same signature. + * + * @param ManyManyList|ManyManyThroughList $list + * + * @return ManyManyList|ManyManyThroughQueryManipulator + */ + protected function getManyManyInspector($list) + { + $inspector = $list; + if ($list instanceof ManyManyThroughList) { + foreach ($list->dataQuery()->getDataQueryManipulators() as $manipulator) { + if ($manipulator instanceof ManyManyThroughQueryManipulator) { + $inspector = $manipulator; + break; + } + } + } + return $inspector; + } } 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, + ]; +}