From e9d89bb177392546ff0b68449475712fc59efb63 Mon Sep 17 00:00:00 2001 From: Steve Boyd Date: Wed, 3 Apr 2024 17:58:18 +1300 Subject: [PATCH] FIX Do not publish dataobject in requireDefaultRecords() --- src/Extensions/ElementalAreasExtension.php | 7 +-- .../ElementalAreasExtensionTest.php | 45 ++++++++++++++++--- tests/Src/TestVersionedDataObject.php | 29 ++++++++++++ 3 files changed, 69 insertions(+), 12 deletions(-) create mode 100644 tests/Src/TestVersionedDataObject.php diff --git a/src/Extensions/ElementalAreasExtension.php b/src/Extensions/ElementalAreasExtension.php index 19878e89..042326ae 100644 --- a/src/Extensions/ElementalAreasExtension.php +++ b/src/Extensions/ElementalAreasExtension.php @@ -15,6 +15,7 @@ use SilverStripe\Forms\LiteralField; use SilverStripe\ORM\DataExtension; use SilverStripe\ORM\DataObject; +use SilverStripe\ORM\RelatedData\StandardRelatedDataService; use SilverStripe\Versioned\Versioned; use SilverStripe\View\ViewableData; @@ -328,15 +329,9 @@ public function requireDefaultRecords() } } - $needsPublishing = ViewableData::has_extension($elementalObject, Versioned::class) - && $elementalObject->isPublished(); - /** @var ElementalAreasExtension $elementalObject */ $elementalObject->ensureElementalAreasExist($elementalAreas); $elementalObject->write(); - if ($needsPublishing) { - $elementalObject->publishRecursive(); - } } $this->owner->extend('onAfterRequireDefaultElementalRecords'); diff --git a/tests/Extensions/ElementalAreasExtensionTest.php b/tests/Extensions/ElementalAreasExtensionTest.php index 8b7983c2..486875a9 100644 --- a/tests/Extensions/ElementalAreasExtensionTest.php +++ b/tests/Extensions/ElementalAreasExtensionTest.php @@ -2,15 +2,18 @@ namespace DNADesign\Elemental\Tests\Extensions; -use DNADesign\Elemental\Extensions\ElementalAreasExtension; +use SilverStripe\Dev\SapphireTest; +use SilverStripe\CMS\Model\SiteTree; +use SilverStripe\Core\Config\Config; +use SilverStripe\Forms\LiteralField; +use DNADesign\Elemental\Models\ElementalArea; use DNADesign\Elemental\Models\ElementContent; use DNADesign\Elemental\Tests\Src\TestElement; -use DNADesign\Elemental\Tests\Src\TestUnusedElement; -use SilverStripe\Core\Config\Config; -use SilverStripe\CMS\Model\SiteTree; -use SilverStripe\Dev\SapphireTest; use SilverStripe\Forms\HTMLEditor\HTMLEditorField; -use SilverStripe\Forms\LiteralField; +use DNADesign\Elemental\Tests\Src\TestUnusedElement; +use DNADesign\Elemental\Tests\Src\TestVersionedDataObject; +use DNADesign\Elemental\Extensions\ElementalAreasExtension; +use SilverStripe\ORM\DB; class ElementalAreasExtensionTest extends SapphireTest { @@ -18,11 +21,15 @@ class ElementalAreasExtensionTest extends SapphireTest SiteTree::class => [ ElementalAreasExtension::class, ], + TestVersionedDataObject::class => [ + ElementalAreasExtension::class, + ], ]; protected static $extra_dataobjects = [ TestElement::class, TestUnusedElement::class, + TestVersionedDataObject::class, ]; protected function setUp(): void @@ -110,4 +117,30 @@ public function provideContentFieldPreservationSettings() [true, true, HTMLEditorField::class], ]; } + + public function testRequireDefaultRecords() + { + /** @var TestVersionedDataObject|ElementalAreasExtension|Versioned $object */ + $object = new TestVersionedDataObject(); + $object->Title = 'abc'; + $id = $object->write(); + $object->publishSingle(); + $object->Title = 'def'; + $object->write(); + $this->assertTrue($object->isModifiedOnDraft()); + $tableName = $object->getSchema()->tableName(TestVersionedDataObject::class); + $tableNameLive = $tableName . '_Live'; + // Explicitly set ElementalAreaID to null to simulate a fresh object that + // has the ElementalAreasExtension applied to it + // There's an onBeforeWrite() hook that will set ElementalAreaID so update DB directly + DB::query("UPDATE $tableName SET ElementalAreaID = NULL WHERE ID = $id"); + DB::query("UPDATE $tableNameLive SET ElementalAreaID = NULL WHERE ID = $id"); + $object->requireDefaultRecords(); + // refetch object to ensure we're not using a cached version + $object = TestVersionedDataObject::get()->byID($id); + // assert that an ElementalID was set on the object + $this->assertSame(ElementalArea::get()->max('ID'), $object->ElementalAreaID); + // assert that we didn't accidentally publish the modified object + $this->assertTrue($object->isModifiedOnDraft()); + } } diff --git a/tests/Src/TestVersionedDataObject.php b/tests/Src/TestVersionedDataObject.php new file mode 100644 index 00000000..69cbd751 --- /dev/null +++ b/tests/Src/TestVersionedDataObject.php @@ -0,0 +1,29 @@ + 'Varchar(255)', + ]; + + private static $has_one = [ + 'ElementalArea' => ElementalArea::class, + ]; + + private static $owns = [ + 'ElementalArea', + ]; +}