From 8e3f549b19bd4930f2fa80a7fa6a528cdc5e4011 Mon Sep 17 00:00:00 2001
From: Damian Mooyman <damian@silverstripe.com>
Date: Mon, 28 Sep 2015 17:22:44 +1300
Subject: [PATCH] BUG Fix regressions in CMS from db field changes

---
 code/model/VirtualPage.php      | 54 ++++++++++++++++++---------------
 tests/model/VirtualPageTest.php |  6 +++-
 tests/model/VirtualPageTest.yml | 31 ++++++++++---------
 3 files changed, 51 insertions(+), 40 deletions(-)

diff --git a/code/model/VirtualPage.php b/code/model/VirtualPage.php
index acd361a9..8d809a2f 100644
--- a/code/model/VirtualPage.php
+++ b/code/model/VirtualPage.php
@@ -16,6 +16,8 @@ class VirtualPage extends Page {
 	 * Note that anything in {@link self::config()->initially_copied_fields} is implicitly included in this list.
 	 */
 	private static $non_virtual_fields = array(
+		"ID",
+		"ClassName",
 		"SecurityTypeID",
 		"OwnerID",
 		"URLSegment",
@@ -53,26 +55,30 @@ class VirtualPage extends Page {
 		$nonVirtualFields = array_merge(self::config()->non_virtual_fields, self::config()->initially_copied_fields);
 		$record = $this->CopyContentFrom();
 
-		$allFields = $record->db();
-		if($hasOne = $record->hasOne()) foreach($hasOne as $link) $allFields[$link . 'ID'] = "Int";
 		$virtualFields = array();
-		foreach($allFields as $field => $type) {
-			if(!in_array($field, $nonVirtualFields)) $virtualFields[] = $field;
+		foreach($record->db() as $field => $type) {
+			if(!in_array($field, $nonVirtualFields)) {
+				$virtualFields[] = $field;
+			}
 		}
-
 		return $virtualFields;
 	}
 
 	/**
-	 * @return SiteTree Returns the linked page, or failing that, a new object.
+	 * Returns the linked page, or failing that, a new object.
+	 *
+	 * Always returns a non-empty object
+	 * 
+	 * @return SiteTree 
 	 */
 	public function CopyContentFrom() {
 		$copyContentFromID = $this->CopyContentFromID;
-		if(!$copyContentFromID) return new SiteTree();
+		if(!$copyContentFromID) {
+			return new SiteTree();
+		}
 		
 		if(!isset($this->components['CopyContentFrom'])) {
-			$this->components['CopyContentFrom'] = DataObject::get_by_id("SiteTree", 
-				$copyContentFromID);
+			$this->components['CopyContentFrom'] = DataObject::get_by_id("SiteTree", $copyContentFromID);
 
 			// Don't let VirtualPages point to other VirtualPages
 			if($this->components['CopyContentFrom'] instanceof VirtualPage) {
@@ -87,8 +93,11 @@ class VirtualPage extends Page {
 		
 		return $this->components['CopyContentFrom'] ? $this->components['CopyContentFrom'] : new SiteTree();
 	}
+	
 	public function setCopyContentFromID($val) {
-		if($val && DataObject::get_by_id('SiteTree', $val) instanceof VirtualPage) $val = 0;
+		if($val && DataObject::get_by_id('SiteTree', $val) instanceof VirtualPage) {
+			$val = 0;
+		}
 		return $this->setField("CopyContentFromID", $val);
 	}
  
@@ -440,13 +449,10 @@ class VirtualPage extends Page {
 	 * @return bool
 	 */
 	public function hasField($field) {
-		return (
-			array_key_exists($field, $this->record) 
-			|| $this->hasDatabaseField($field) 
-			|| array_key_exists($field, $this->db()) // Needed for composite fields
-			|| parent::hasMethod("get{$field}")
-			|| $this->CopyContentFrom()->hasField($field)
-		);
+		if(parent::hasField($field)) {
+			return true;
+		}
+		return $this->CopyContentFrom()->hasField($field);
 	}	
 	/**
 	 * Overwrite to also check for method on the original data object
@@ -455,8 +461,10 @@ class VirtualPage extends Page {
 	 * @return bool 
 	 */
 	public function hasMethod($method) {
-		if(parent::hasMethod($method)) return true;
-		return $this->copyContentFrom()->hasMethod($method);
+		if(parent::hasMethod($method)) {
+			return true;
+		}
+		return $this->CopyContentFrom()->hasMethod($method);
 	}
 
 	/**
@@ -467,11 +475,9 @@ class VirtualPage extends Page {
 	 * @return string
 	 */
 	public function castingHelper($field) {
-		if($this->copyContentFrom()) {
-			return $this->copyContentFrom()->castingHelper($field);
-		} else {
-			return parent::castingHelper($field);
-		}
+		return $this
+			->CopyContentFrom()
+			->castingHelper($field);
 	}
 
 }
diff --git a/tests/model/VirtualPageTest.php b/tests/model/VirtualPageTest.php
index 1a764a84..5103d553 100644
--- a/tests/model/VirtualPageTest.php
+++ b/tests/model/VirtualPageTest.php
@@ -6,8 +6,12 @@ class VirtualPageTest extends SapphireTest {
 	protected $extraDataObjects = array(
 		'VirtualPageTest_ClassA',
 		'VirtualPageTest_ClassB',
+		'VirtualPageTest_ClassC',
+		'VirtualPageTest_NotRoot',
+		'VirtualPageTest_PageExtension',
+		'VirtualPageTest_PageWithAllowedChildren',
+		'VirtualPageTest_TestDBField',
 		'VirtualPageTest_VirtualPageSub',
-		'VirtualPageTest_PageWithAllowedChildren'
 	);
 
 	protected $illegalExtensions = array(
diff --git a/tests/model/VirtualPageTest.yml b/tests/model/VirtualPageTest.yml
index 473f6caa..074d50a5 100644
--- a/tests/model/VirtualPageTest.yml
+++ b/tests/model/VirtualPageTest.yml
@@ -1,17 +1,18 @@
 Page:
-   master:
-      Title: My Page
-      MenuTitle: My Page Nav
-   master2:
-      Title: My Other Page
-      MenuTitle: My Other Page Nav
-   holder:
-      Title: Sub-pages
+  master:
+    Title: My Page
+    MenuTitle: My Page Nav
+  master2:
+    Title: My Other Page
+    MenuTitle: My Other Page Nav
+  holder:
+    Title: Sub-pages
 VirtualPage:
-   vp1:
-      CopyContentFrom: =>Page.master
-      Parent: =>Page.holder
-   vp2:
-      CopyContentFrom: =>Page.master
-      Parent: =>Page.holder
-      
\ No newline at end of file
+  vp1:
+    Title: vp1
+    CopyContentFrom: =>Page.master
+    Parent: =>Page.holder
+  vp2:
+    Title: vp2
+    CopyContentFrom: =>Page.master
+    Parent: =>Page.holder