diff --git a/src/Models/BaseElement.php b/src/Models/BaseElement.php index 3b86e210..d4ec0a6e 100644 --- a/src/Models/BaseElement.php +++ b/src/Models/BaseElement.php @@ -5,6 +5,7 @@ use DNADesign\Elemental\Controllers\ElementController; use DNADesign\Elemental\Forms\TextCheckboxGroupField; use DNADesign\Elemental\ORM\FieldType\DBObjectType; +use DNADesign\Elemental\Services\ReorderElements; use DNADesign\Elemental\TopPage\DataExtension; use Exception; use SilverStripe\CMS\Controllers\CMSPageEditController; @@ -1278,4 +1279,10 @@ public static function getGraphQLTypeName(): string } return str_replace('\\', '_', static::class); } + + public function onAfterPublish() + { + $reorderer = Injector::inst()->create(ReorderElements::class, $this); + $reorderer->publishSortOrder(); + } } diff --git a/src/Services/ReorderElements.php b/src/Services/ReorderElements.php index 936587e0..9e28be11 100644 --- a/src/Services/ReorderElements.php +++ b/src/Services/ReorderElements.php @@ -6,6 +6,7 @@ use InvalidArgumentException; use SilverStripe\Core\Convert; use SilverStripe\ORM\DataObject; +use SilverStripe\ORM\DB; use SilverStripe\ORM\Queries\SQLUpdate; use SilverStripe\Versioned\Versioned; @@ -61,7 +62,8 @@ public function setElement(BaseElement $element) } /** - * Set the ordering of Elements in relation to sibling Elements in the parent {@see ElementalArea} + * Set the ordering of Elements in relation to sibling Elements in the parent {@see ElementalArea}. + * This only affects the sort order in draft. * * @param int $elementToBeAfterID ID of the Element to be ordered after */ @@ -97,34 +99,23 @@ public function reorder($elementToBeAfterID = 0) // We are updating records with SQL queries to avoid the ORM triggering the creation of new versions // for each element that is affected by this reordering. $baseTableName = Convert::raw2sql(DataObject::getSchema()->tableName(BaseElement::class)); - - // Update both the draft and live versions of the records - $tableNames = [$baseTableName]; - if (BaseElement::has_extension(Versioned::class)) { - /** @var BaseElement&Versioned $element */ - $tableNames[] = $element->stageTable($baseTableName, Versioned::LIVE); + $tableName = sprintf('"%s"', $baseTableName); + + if ($sortAfterPosition < $currentPosition) { + $operator = '+'; + $filter = "$tableName.\"Sort\" > $sortAfterPosition AND $tableName.\"Sort\" < $currentPosition"; + $newBlockPosition = $sortAfterPosition + 1; + } else { + $operator = '-'; + $filter = "$tableName.\"Sort\" <= $sortAfterPosition AND $tableName.\"Sort\" > $currentPosition"; + $newBlockPosition = $sortAfterPosition; } - foreach ($tableNames as $tableName) { - $tableName = sprintf('"%s"', $tableName); - - if ($sortAfterPosition < $currentPosition) { - $operator = '+'; - $filter = "$tableName.\"Sort\" > $sortAfterPosition AND $tableName.\"Sort\" < $currentPosition"; - $newBlockPosition = $sortAfterPosition + 1; - } else { - $operator = '-'; - $filter = "$tableName.\"Sort\" <= $sortAfterPosition AND $tableName.\"Sort\" > $currentPosition"; - $newBlockPosition = $sortAfterPosition; - } - - $query = SQLUpdate::create() - ->setTable("$tableName") - ->assignSQL('"Sort"', "$tableName.\"Sort\" $operator 1") - ->addWhere([$filter, "$tableName.\"ParentID\"" => $parentId]); - - $query->execute(); - } + $query = SQLUpdate::create() + ->setTable("$tableName") + ->assignSQL('"Sort"', "$tableName.\"Sort\" $operator 1") + ->addWhere([$filter, "$tableName.\"ParentID\"" => $parentId]); + $query->execute(); // Now use the ORM to write a new version of the record that we are directly reordering $element->Sort = $newBlockPosition; @@ -132,4 +123,21 @@ public function reorder($elementToBeAfterID = 0) return $element; } + + /** + * Force live sort order to match stage sort order + */ + public function publishSortOrder() + { + $baseTableName = Convert::raw2sql(DataObject::getSchema()->tableName(BaseElement::class)); + $live = Versioned::LIVE; + $sql = sprintf( + 'UPDATE "%2$s" + SET "Sort" = (SELECT "%1$s"."Sort" FROM "%1$s" WHERE "%2$s"."ID" = "%1$s"."ID") + WHERE EXISTS (SELECT "%1$s"."Sort" FROM "%1$s" WHERE "%2$s"."ID" = "%1$s"."ID") AND "ParentID" = ?', + $baseTableName, + "{$baseTableName}_{$live}" + ); + DB::prepared_query($sql, [$this->getElement()->ParentID]); + } } diff --git a/tests/Services/ReorderElementsTest.php b/tests/Services/ReorderElementsTest.php index 66a8c014..319be73f 100644 --- a/tests/Services/ReorderElementsTest.php +++ b/tests/Services/ReorderElementsTest.php @@ -2,9 +2,11 @@ namespace DNADesign\Elemental\Tests\Services; +use DNADesign\Elemental\Models\BaseElement; use DNADesign\Elemental\Services\ReorderElements; use DNADesign\Elemental\Tests\Src\TestElement; use SilverStripe\Dev\SapphireTest; +use SilverStripe\Versioned\Versioned; class ReorderElementsTest extends SapphireTest { @@ -19,28 +21,49 @@ class ReorderElementsTest extends SapphireTest */ public function testReorder() { + foreach (BaseElement::get() as $toPublish) { + $toPublish->publishSingle(); + } $element = $this->objFromFixture(TestElement::class, 'element1'); $reorderService = new ReorderElements($element); $reorderService->reorder(4); - $this->assertIdsInOrder([2, 3, 4, 1], 'The Element should be last in the list'); + $this->assertIdsInOrder([2, 3, 4, 1], [1, 2, 3, 4], 'The Element should be last in the list (draft only)'); + $element->publishSingle(); + $this->assertIdsInOrder([2, 3, 4, 1], [2, 3, 4, 1], 'The sort order should be published correctly'); $reorderService->reorder(0); - $this->assertIdsInOrder([1, 2, 3, 4], 'The Element should be first in the list'); + $this->assertIdsInOrder([1, 2, 3, 4], [2, 3, 4, 1], 'The Element should be first in the list (draft only)'); + $element->publishSingle(); + $this->assertIdsInOrder([1, 2, 3, 4], [1, 2, 3, 4], 'The sort order should be published correctly'); $reorderService->reorder(2); - $this->assertIdsInOrder([2, 1, 3, 4], 'The Element should be second in the list'); + $this->assertIdsInOrder([2, 1, 3, 4], [1, 2, 3, 4], 'The Element should be second in the list (draft only)'); + $element->publishSingle(); + $this->assertIdsInOrder([2, 1, 3, 4], [2, 1, 3, 4], 'The sort order should be published correctly'); $reorderService->reorder(); - $this->assertIdsInOrder([1, 2, 3, 4], 'The Element should be first in the list'); + $this->assertIdsInOrder([1, 2, 3, 4], [2, 1, 3, 4], 'The Element should be first in the list (draft only)'); + $element->publishSingle(); + $this->assertIdsInOrder([1, 2, 3, 4], [1, 2, 3, 4], 'The sort order should be published correctly'); } - protected function assertIdsInOrder($ids, $message = null) + protected function assertIdsInOrder($draftIds, $publishedIDs, $message = null) { - $actualIDs = TestElement::get()->sort('Sort')->column('ID'); + // Check draft + Versioned::withVersionedMode(function () use ($draftIds, $message) { + Versioned::set_stage(Versioned::DRAFT); + $actualIDs = TestElement::get()->sort('Sort')->column('ID'); - // Ideally this should be assertSame, but sometimes the database - // returns IDs as strings instead of integers (e.g. "1" instead of 1). - $this->assertEquals($ids, $actualIDs, $message); + // Ideally this should be assertSame, but sometimes the database + // returns IDs as strings instead of integers (e.g. "1" instead of 1). + $this->assertEquals($draftIds, $actualIDs, $message); + }); + // Check published + Versioned::withVersionedMode(function () use ($publishedIDs, $message) { + Versioned::set_stage(Versioned::LIVE); + $actualIDs = TestElement::get()->sort('Sort')->column('ID'); + $this->assertEquals($publishedIDs, $actualIDs, $message); + }); } }