From 9c48b9398323450a3cae83b8ed33e32d7ef0925f Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Mon, 5 Sep 2016 18:46:00 +1200 Subject: [PATCH] BUG Ensure changes in class write to an instance of the new class, not the old one Fixes #1210 Requires https://github.com/silverstripe/silverstripe-framework/pull/5950 --- code/Controllers/CMSMain.php | 8 ++---- tests/controller/CMSMainTest.php | 45 ++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/code/Controllers/CMSMain.php b/code/Controllers/CMSMain.php index 6b3758e9..53441e9b 100644 --- a/code/Controllers/CMSMain.php +++ b/code/Controllers/CMSMain.php @@ -1006,12 +1006,8 @@ class CMSMain extends LeftAndMain implements CurrentPageIdentifier, PermissionPr // Update the class instance if necessary if(isset($data['ClassName']) && $data['ClassName'] != $record->ClassName) { - $newClassName = $record->ClassName; - // The records originally saved attribute was overwritten by $form->saveInto($record) before. - // This is necessary for newClassInstance() to work as expected, and trigger change detection - // on the ClassName attribute - $record->setClassName($data['ClassName']); - // Replace $record with a new instance + // Replace $record with a new instance of the new class + $newClassName = $data['ClassName']; $record = $record->newClassInstance($newClassName); } diff --git a/tests/controller/CMSMainTest.php b/tests/controller/CMSMainTest.php index ccd6ef1a..06d29345 100644 --- a/tests/controller/CMSMainTest.php +++ b/tests/controller/CMSMainTest.php @@ -2,6 +2,7 @@ use SilverStripe\ORM\DB; use SilverStripe\ORM\DataObject; +use SilverStripe\ORM\ValidationException; use SilverStripe\ORM\Versioning\Versioned; use SilverStripe\ORM\HiddenClass; use SilverStripe\CMS\Controllers\CMSMain; @@ -540,14 +541,58 @@ class CMSMainTest extends FunctionalTest { $this->assertContains("delete", $exemptActions); $this->assertContains("unpublish", $exemptActions); } + + /** + * Test that changed classes save with the correct class name + */ + public function testChangeClass() { + $this->logInWithPermission('ADMIN'); + $cms = new CMSMain(); + $page = new CMSMainTest_ClassA(); + $page->Title = 'Class A'; + $page->write(); + + $form = $cms->getEditForm($page->ID); + $form->loadDataFrom(['ClassName' => 'CMSMainTest_ClassB']); + $result = $cms->save([ + 'ID' => $page->ID, + 'ClassName' => 'CMSMainTest_ClassB' + ], $form); + $this->assertEquals(200, $result->getStatusCode()); + + $newPage = SiteTree::get()->byID($page->ID); + + $this->assertInstanceOf('CMSMainTest_ClassB', $newPage); + $this->assertEquals('CMSMainTest_ClassB', $newPage->ClassName); + $this->assertEquals('Class A', $newPage->Title); + + } } class CMSMainTest_ClassA extends Page implements TestOnly { private static $allowed_children = array('CMSMainTest_ClassB'); + + protected function onBeforeWrite() + { + parent::onBeforeWrite(); + + if ($this->ClassName !== __CLASS__) { + throw new ValidationException("Class saved with incorrect ClassName"); + } + } } class CMSMainTest_ClassB extends Page implements TestOnly { + protected function onBeforeWrite() + { + parent::onBeforeWrite(); + + if ($this->ClassName !== __CLASS__) { + throw new ValidationException("Class saved with incorrect ClassName"); + } + } + } class CMSMainTest_NotRoot extends Page implements TestOnly {