BUGFIX #4605 DataObject::newClassInstance() should repopulate it's defaults after changing to an instance of a different class, otherwise databases will complain of NULL values being written to columns that don't accept NULL values.

git-svn-id: svn://svn.silverstripe.com/silverstripe/open/modules/sapphire/trunk@88956 467b73ca-7a2a-4603-9d3b-597d59a354a9

core/model/DataObject.php

@@ -410,11 +410,16 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 	}
 
 	/**
-	 * Create a new instance of a different class from this object's record
+	 * Create a new instance of a different class from this object's record.
 	 * This is useful when dynamically changing the type of an instance. Specifically,
 	 * it ensures that the instance of the class is a match for the className of the
 	 * record. Don't set the {@link DataObject->class} or {@link DataObject->ClassName}
 	 * property manually before calling this method, as it will confuse change detection.
+	 * 
+	 * If the new class is different to the original class, defaults are populated again
+	 * because this will only occur automatically on instantiation of a DataObject if
+	 * there is no record, or the record has no ID. In this case, we do have an ID but
+	 * we still need to repopulate the defaults.
 	 *
 	 * @param string $newClassName The name of the new class
 	 *
@@ -425,12 +430,14 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 		$newInstance = new $newClassName(array_merge(
 			$this->record,
 			array(
-				'ClassName'=>$originalClass,
-				'RecordClassName'=>$originalClass,
+				'ClassName' => $originalClass,
+				'RecordClassName' => $originalClass,
 			)
 		));
-		if($newClassName != $this->ClassName) {
+		
+		if($newClassName != $originalClass) {
 			$newInstance->setClassName($newClassName);
+			$newInstance->populateDefaults();
 			$newInstance->forceChange();
 		}
 

core/model/RedirectorPage.php

@@ -16,6 +16,10 @@ class RedirectorPage extends Page {
 		"ExternalURL" => "Varchar(255)",
 	);
 	
+	static $defaults = array(
+		"RedirectionType" => "Internal"
+	);
+	
 	static $has_one = array(
 		"LinkTo" => "SiteTree",
 	);

core/model/Translatable.php

@@ -767,6 +767,7 @@ class Translatable extends DataObjectDecorator implements PermissionProvider {
 				if($translations) foreach($translations as $translation) {
 					$translation->setClassName($this->owner->ClassName);
 					$translation = $translation->newClassInstance($translation->ClassName);
+					$translation->populateDefaults();
 					$translation->forceChange();
 					$translation->write();
 				}

tests/DataObjectTest.php

@@ -641,9 +641,9 @@ class DataObjectTest extends SapphireTest {
 		$changedFields = $changedPage->getChangedFields();
 		
 		// Don't write the record, it will reset changed fields
-		
 		$this->assertType('RedirectorPage', $changedPage);
 		$this->assertEquals($changedPage->ClassName, 'RedirectorPage');
+		$this->assertEquals($changedPage->RedirectionType, 'Internal');
 		//$this->assertEquals($changedPage->RecordClassName, 'RedirectorPage');
 		$this->assertContains('ClassName', array_keys($changedFields));
 		$this->assertEquals($changedFields['ClassName']['before'], 'Page');

tests/model/TranslatableTest.php

@@ -684,8 +684,8 @@ class TranslatableTest extends FunctionalTest {
 		$translatedPageID = $translatedPage->ID;
 		
 		// change page type
-		$origPage->ClassName = 'RedirectorPage';
-		$origPage->write();
+		$newPage = $origPage->newClassInstance('RedirectorPage');
+		$newPage->write();
 		
 		// re-fetch original page with new instance
 		$origPageChanged = DataObject::get_by_id('RedirectorPage', $origPageID);