ENHANCEMENT Allowing diffs of has_one relationship in DataDifferencer (AIR-24)

model/DataDifferencer.php

@@ -76,8 +76,12 @@ class DataDifferencer extends ViewableData {
 			$fields = array_keys($this->toRecord->getAllFields());
 		}
 		
+		$hasOnes = $this->fromRecord->has_one();
+		
+		// Loop through properties
 		foreach($fields as $field) {
 			if(in_array($field, $this->ignoredFields)) continue;
+			if(in_array($field, array_keys($hasOnes))) continue;
 			
 			if(!$this->fromRecord) {
 				$diffed->setField($field, "<ins>" . $this->toRecord->$field . "</ins>");
@@ -86,6 +90,43 @@ class DataDifferencer extends ViewableData {
 			}
 		}
 		
+		// Loop through has_one
+		foreach($hasOnes as $relName => $relSpec) {
+			if(in_array($relName, $this->ignoredFields)) continue;
+			
+			$relField = "{$relName}ID";
+			$relObjTo = $this->toRecord->$relName();
+			
+			if(!$this->fromRecord) {
+				if($relObjTo) {
+					if($relObjTo instanceof Image) {
+						// Using relation name instead of database column name, because of FileField etc.
+						// TODO Use CMSThumbnail instead to limit max size, blocked by DataDifferencerTest and GC
+						// not playing nice with mocked images
+						$diffed->setField($relName, "<ins>" . $relObjTo->getTag() . "</ins>");
+					} else {
+						$diffed->setField($relField, "<ins>" . $relObjTo->Title() . "</ins>");
+					}
+				}
+			} else if($this->fromRecord->$relField != $this->toRecord->$relField) {
+				$relObjFrom = $this->fromRecord->$relName();
+				if($relObjFrom instanceof Image) {
+					// TODO Use CMSThumbnail (see above)
+					$diffed->setField(
+						// Using relation name instead of database column name, because of FileField etc.
+						$relName, 
+						Diff::compareHTML($relObjFrom->getTag(), $relObjTo->getTag())
+					);
+				} else {
+					$diffed->setField(
+						$relField, 
+						Diff::compareHTML($relObjFrom->getTitle(), $relObjTo->getTitle())
+					);
+				}
+				
+			}
+		}
+		
 		return $diffed;
 	}
 	

tests/model/DataDifferencerTest.php

@@ -8,20 +8,44 @@ class DataDifferencerTest extends SapphireTest {
 	
 	static $fixture_file = 'sapphire/tests/model/DataDifferencerTest.yml';
 	
-	protected $extraDataObjects = array('DataDifferencerTest_Object');
+	protected $extraDataObjects = array(
+		'DataDifferencerTest_Object',
+		'DataDifferencerTest_HasOneRelationObject',
+		'DataDifferencerTest_MockImage',
+	);
 	
 	function testArrayValues() {
 		$obj1 = $this->objFromFixture('DataDifferencerTest_Object', 'obj1');
 		// create a new version
 		$obj1->Choices = array('a');
 		$obj1->write();
-		$obj1v1 = Versioned::get_version('DataDifferencerTest_Object', $obj1->ID, 1);
+		$obj1v1 = Versioned::get_version('DataDifferencerTest_Object', $obj1->ID, $obj1->Version-1);
-		$obj1v2 = Versioned::get_version('DataDifferencerTest_Object', $obj1->ID, 2);
+		$obj1v2 = Versioned::get_version('DataDifferencerTest_Object', $obj1->ID, $obj1->Version);
 		$differ = new DataDifferencer($obj1v1, $obj1v2);
 		$obj1Diff = $differ->diffedData();
 		// TODO Using getter would split up field again, bug only caused by simulating
 		// an array-based value in the first place.
-		$this->assertContains('<ins>a</ins>  <del>a,b</del>', $obj1Diff->getField('Choices'));
+		$this->assertContains('<ins>a</ins><del>a,b</del>', $obj1Diff->getField('Choices'));
+	}
+	
+	function testHasOnes() {
+		$obj1 = $this->objFromFixture('DataDifferencerTest_Object', 'obj1');
+		$image1 = $this->objFromFixture('DataDifferencerTest_MockImage', 'image1');
+		$image2 = $this->objFromFixture('DataDifferencerTest_MockImage', 'image2');
+		$relobj1 = $this->objFromFixture('DataDifferencerTest_HasOneRelationObject', 'relobj1');
+		$relobj2 = $this->objFromFixture('DataDifferencerTest_HasOneRelationObject', 'relobj2');
+
+		// create a new version
+		$obj1->ImageID = $image2->ID;
+		$obj1->HasOneRelationID = $relobj2->ID;
+		$obj1->write();
+		$obj1v1 = Versioned::get_version('DataDifferencerTest_Object', $obj1->ID, $obj1->Version-1);
+		$obj1v2 = Versioned::get_version('DataDifferencerTest_Object', $obj1->ID, $obj1->Version);
+		$differ = new DataDifferencer($obj1v1, $obj1v2);
+		$obj1Diff = $differ->diffedData();
+		$this->assertContains($image1->Filename, $obj1Diff->getField('Image'));
+		$this->assertContains($image2->Filename, $obj1Diff->getField('Image'));
+		$this->assertContains('<ins>obj2</ins><del>obj1</del>', $obj1Diff->getField('HasOneRelationID'));
 	}
 }
 
@@ -33,6 +57,11 @@ class DataDifferencerTest_Object extends DataObject implements TestOnly {
 		'Choices' => "Varchar",
 	);
 	
+	static $has_one = array(
+		'Image' => 'DataDifferencerTest_MockImage',
+		'HasOneRelation' => 'DataDifferencerTest_HasOneRelationObject'
+	);
+	
 	function getCMSFields() {
 		$fields = parent::getCMSFields();
 		$choices = array(
@@ -54,4 +83,24 @@ class DataDifferencerTest_Object extends DataObject implements TestOnly {
 		$this->setField('Choices', (is_array($val)) ? implode(',', $val) : $val);
 	}
 	
+}
+
+class DataDifferencerTest_HasOneRelationObject extends DataObject implements TestOnly {
+	
+	static $db = array(
+		'Title' => 'Varchar'
+	);
+	
+	static $has_many = array(
+		'Objects' => 'DataDifferencerTest_Object'
+	);
+}
+
+class DataDifferencerTest_MockImage extends Image implements TestOnly {
+	function generateFormattedImage($format, $arg1 = null, $arg2 = null) {
+		$cacheFile = $this->cacheFilename($format, $arg1, $arg2);
+		$gd = new GD(Director::baseFolder()."/" . $this->Filename);
+		// Skip aktual generation
+		return $gd;
+	}
 }

tests/model/DataDifferencerTest.yml

@@ -1,3 +1,15 @@
+DataDifferencerTest_MockImage:
+   image1:
+      Filename: sapphire/tests/model/testimages/test_image.png
+   image2:
+      Filename: sapphire/tests/model/testimages/test.image.with.dots.png
+DataDifferencerTest_HasOneRelationObject:
+   relobj1:
+      Title: obj1
+   relobj2:
+      Title: obj2
 DataDifferencerTest_Object:
    obj1:
-      Choices: a,b
+      Choices: a,b
+      Image: =>DataDifferencerTest_MockImage.image1
+      HasOneRelation: =>DataDifferencerTest_HasOneRelationObject.relobj1