From b221134ce19419d372399540f350a477751afe38 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Fri, 28 Sep 2018 17:38:38 +0200 Subject: [PATCH 1/3] FIX Orderable rows now respects actual MMTL sort orders instead of incrementing from SiteTree --- src/GridFieldOrderableRows.php | 67 ++++++++++++++++++++++++++-------- 1 file changed, 51 insertions(+), 16 deletions(-) diff --git a/src/GridFieldOrderableRows.php b/src/GridFieldOrderableRows.php index 57bb4fb..d8c5e0c 100755 --- a/src/GridFieldOrderableRows.php +++ b/src/GridFieldOrderableRows.php @@ -4,6 +4,8 @@ namespace Symbiote\GridFieldExtensions; use Exception; use SilverStripe\Control\Controller; +use SilverStripe\Control\HTTPRequest; +use SilverStripe\Control\HTTPResponse_Exception; use SilverStripe\Control\RequestHandler; use SilverStripe\Core\ClassInfo; use SilverStripe\Forms\GridField\GridField; @@ -20,11 +22,11 @@ use SilverStripe\ORM\DataObject; use SilverStripe\ORM\DataObjectInterface; use SilverStripe\ORM\DataObjectSchema; use SilverStripe\ORM\DB; +use SilverStripe\ORM\FieldType\DBDatetime; use SilverStripe\ORM\ManyManyList; use SilverStripe\ORM\ManyManyThroughList; use SilverStripe\ORM\ManyManyThroughQueryManipulator; use SilverStripe\ORM\SS_List; -use SilverStripe\ORM\FieldType\DBDatetime; use SilverStripe\Versioned\Versioned; use SilverStripe\View\ViewableData; @@ -295,7 +297,22 @@ class GridFieldOrderableRows extends RequestHandler implements $record->ID, $this->getSortField() ); - $sortField = new HiddenField($sortFieldName, false, $record->getField($this->getSortField())); + + // Default: Get the sort field directly from the current record + $currentSortValue = $record->getField($this->getSortField()); + + $list = $grid->getList(); + if ($list instanceof ManyManyThroughList) { + // In a many many through list we should get the current sort order from the relationship + // if it exists, not directly from the record + $throughListSorts = $this->getSortValuesFromManyManyThroughList($list, $this->getSortField()); + + if (array_key_exists($record->ID, $throughListSorts)) { + $currentSortValue = $throughListSorts[$record->ID]; + } + } + + $sortField = HiddenField::create($sortFieldName, false, $currentSortValue); $sortField->addExtraClass('ss-orderable-hidden-sort'); $sortField->setForm($grid->getForm()); @@ -345,17 +362,18 @@ class GridFieldOrderableRows extends RequestHandler implements $sortterm .= '"'.$this->getSortTable($list).'"."'.$this->getSortField().'"'; } return $list->sort($sortterm); - } else { - return $list; } + + return $list; } /** * Handles requests to reorder a set of IDs in a specific order. * * @param GridField $grid - * @param SS_HTTPRequest $request - * @return SS_HTTPResponse + * @param HTTPRequest $request + * @return string + * @throws HTTPResponse_Exception */ public function handleReorder($grid, $request) { @@ -536,16 +554,7 @@ class GridFieldOrderableRows extends RequestHandler implements } } } 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(); + $current = $this->getSortValuesFromManyManyThroughList($list, $sortField); } else { $current = $items->map('ID', $sortField)->toArray(); } @@ -759,4 +768,30 @@ class GridFieldOrderableRows extends RequestHandler implements } return $inspector; } + + /** + * Used to get sort orders from a many many through list relationship record, rather than the current + * record itself. + * + * @param ManyManyList|ManyManyThroughList $list + * @return int[] Sort orders for the + */ + protected function getSortValuesFromManyManyThroughList($list, $sortField) + { + $manipulator = $this->getManyManyInspector($list); + + // Find the foreign key name, ID and class to look up + $joinClass = $manipulator->getJoinClass(); + $fromRelationName = $manipulator->getForeignKey(); + $toRelationName = $manipulator->getLocalKey(); + + // Create a list of the MMTL relations + $sortlist = DataList::create($joinClass)->filter([ + $toRelationName => $list->column('ID'), + // first() is safe as there are earlier checks to ensure our list to sort is valid + $fromRelationName => $list->first()->getJoin()->$fromRelationName, + ]); + + return $sortlist->map($toRelationName, $sortField)->toArray(); + } } From 78c63c6725cca07882b720a5c9581da45b04d769 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Fri, 28 Sep 2018 17:59:44 +0200 Subject: [PATCH 2/3] Add test for getting the correct Sort order from MMTL items, fix incorrect test class name --- tests/GridFieldOrderableRowsTest.php | 34 +++++++++++++++++++++ tests/OrderableRowsThroughVersionedTest.php | 4 +-- tests/Stub/ThroughIntermediary.php | 7 ++--- 3 files changed, 39 insertions(+), 6 deletions(-) diff --git a/tests/GridFieldOrderableRowsTest.php b/tests/GridFieldOrderableRowsTest.php index 2ca98a9..dd71b07 100644 --- a/tests/GridFieldOrderableRowsTest.php +++ b/tests/GridFieldOrderableRowsTest.php @@ -88,6 +88,40 @@ class GridFieldOrderableRowsTest extends SapphireTest $this->assertEquals($desiredOrder, $newOrder); } + public function testManyManyThroughListSortOrdersAreUsedForInitialRender() + { + /** @var ThroughDefiner $record */ + $record = $this->objFromFixture(ThroughDefiner::class, 'DefinerOne'); + + $orderable = new GridFieldOrderableRows('Sort'); + $config = new GridFieldConfig_RelationEditor(); + $config->addComponent($orderable); + + $grid = new GridField( + 'Belongings', + 'Testing Many Many', + $record->Belongings()->sort('Sort'), + $config + ); + + // Get the first record, which would be the first one to have column contents generated + /** @var ThroughIntermediary $expected */ + $intermediary = $this->objFromFixture(ThroughIntermediary::class, 'One'); + + $result = $orderable->getColumnContent($grid, $record, 'irrelevant'); + + $this->assertContains( + 'Belongings[GridFieldEditableColumns][' . $record->ID . '][Sort]', + $result, + 'The field name is indexed under the record\'s ID' + ); + $this->assertContains( + 'value="' . $intermediary->Sort . '"', + $result, + 'The value comes from the MMTL intermediary Sort value' + ); + } + public function testSortableChildClass() { $orderable = new GridFieldOrderableRows('Sort'); diff --git a/tests/OrderableRowsThroughVersionedTest.php b/tests/OrderableRowsThroughVersionedTest.php index 694da01..e7e3cb1 100644 --- a/tests/OrderableRowsThroughVersionedTest.php +++ b/tests/OrderableRowsThroughVersionedTest.php @@ -12,7 +12,7 @@ use Symbiote\GridFieldExtensions\Tests\Stub\ThroughIntermediary; use Symbiote\GridFieldExtensions\Tests\Stub\ThroughBelongs; use Symbiote\GridFieldExtensions\GridFieldOrderableRows; -class OrderableRowsThroughTest extends SapphireTest +class OrderableRowsThroughVersionedTest extends SapphireTest { protected static $fixture_file = 'OrderableRowsThroughTest.yml'; @@ -105,7 +105,7 @@ class OrderableRowsThroughTest extends SapphireTest } } $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'); diff --git a/tests/Stub/ThroughIntermediary.php b/tests/Stub/ThroughIntermediary.php index 12a48e1..9477d61 100644 --- a/tests/Stub/ThroughIntermediary.php +++ b/tests/Stub/ThroughIntermediary.php @@ -2,18 +2,17 @@ namespace Symbiote\GridFieldExtensions\Tests\Stub; -use SilverStripe\ORM\DataObject; -use SilverStripe\ORM\FieldType\DBInt; use SilverStripe\Dev\TestOnly; +use SilverStripe\ORM\DataObject; class ThroughIntermediary extends DataObject implements TestOnly { private static $table_name = 'IntermediaryThrough'; private static $db = [ - 'Sort' => DBInt::class, + 'Sort' => 'Int', ]; - + private static $has_one = [ 'Defining' => ThroughDefiner::class, 'Belonging' => ThroughBelongs::class, From 968b80791834cfbac0163e77f4362161ef2b51e8 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Fri, 28 Sep 2018 18:24:50 +0200 Subject: [PATCH 3/3] Add SS 4.3.x to Travis matrix and move Postgres to 2.1.x-dev with SS 4.2 or newer --- .travis.yml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 74cc970..f0fbdb3 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,9 +9,11 @@ matrix: - php: 5.6 env: DB=MYSQL RECIPE_VERSION=1.0.x-dev PHPCS_TEST=1 PHPUNIT_TEST=1 - php: 7.0 - env: DB=PGSQL RECIPE_VERSION=1.1.x-dev PHPUNIT_TEST=1 + env: DB=MYSQL RECIPE_VERSION=1.1.x-dev PHPUNIT_TEST=1 - php: 7.1 - env: DB=MYSQL RECIPE_VERSION=4.2.x-dev PHPUNIT_COVERAGE_TEST=1 + env: DB=PGSQL RECIPE_VERSION=4.2.x-dev PHPUNIT_COVERAGE_TEST=1 + - php: 7.2 + env: DB=MYSQL RECIPE_VERSION=4.3.x-dev PHPUNIT_TEST=1 - php: 7.2 env: DB=MYSQL RECIPE_VERSION=4.x-dev PHPUNIT_TEST=1 @@ -21,7 +23,7 @@ before_script: - composer validate - composer require silverstripe/recipe-core "$RECIPE_VERSION" --no-update - - if [[ $DB == PGSQL ]]; then composer require silverstripe/postgresql:2.0.x-dev --no-update; fi + - if [[ $DB == PGSQL ]]; then composer require silverstripe/postgresql:2.1.x-dev --no-update; fi - composer install --prefer-dist --no-interaction --no-progress --no-suggest --optimize-autoloader --verbose --profile script: