From 559bbfcca8caf89460c21c3d9cc3f4769baae3bb Mon Sep 17 00:00:00 2001
From: Sean Harvey <sean@silverstripe.com>
Date: Wed, 14 Oct 2009 06:35:45 +0000
Subject: [PATCH] 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        | 15 +++++++++++----
 core/model/RedirectorPage.php    |  4 ++++
 core/model/Translatable.php      |  1 +
 tests/DataObjectTest.php         |  2 +-
 tests/model/TranslatableTest.php |  4 ++--
 5 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/core/model/DataObject.php b/core/model/DataObject.php
index e2290f452..ab4778cab 100755
--- a/core/model/DataObject.php
+++ b/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();
 		}
 
diff --git a/core/model/RedirectorPage.php b/core/model/RedirectorPage.php
index e0b6498f9..d6b4f36c9 100755
--- a/core/model/RedirectorPage.php
+++ b/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",
 	);
diff --git a/core/model/Translatable.php b/core/model/Translatable.php
index ede74a267..8696fed23 100755
--- a/core/model/Translatable.php
+++ b/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();
 				}
diff --git a/tests/DataObjectTest.php b/tests/DataObjectTest.php
index d6b8124e9..fa447ebb0 100755
--- a/tests/DataObjectTest.php
+++ b/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');
diff --git a/tests/model/TranslatableTest.php b/tests/model/TranslatableTest.php
index 1e73bae20..db9cd010d 100755
--- a/tests/model/TranslatableTest.php
+++ b/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);