diff --git a/model/DataDifferencer.php b/model/DataDifferencer.php index 793f992dc..71cd2a4f4 100644 --- a/model/DataDifferencer.php +++ b/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, "" . $this->toRecord->$field . ""); @@ -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, "" . $relObjTo->getTag() . ""); + } else { + $diffed->setField($relField, "" . $relObjTo->Title() . ""); + } + } + } 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; } diff --git a/tests/model/DataDifferencerTest.php b/tests/model/DataDifferencerTest.php index 7d80a18e7..98a589310 100644 --- a/tests/model/DataDifferencerTest.php +++ b/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); - $obj1v2 = Versioned::get_version('DataDifferencerTest_Object', $obj1->ID, 2); + $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(); // TODO Using getter would split up field again, bug only caused by simulating // an array-based value in the first place. - $this->assertContains('a a,b', $obj1Diff->getField('Choices')); + $this->assertContains('aa,b', $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('obj2obj1', $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; + } } \ No newline at end of file diff --git a/tests/model/DataDifferencerTest.yml b/tests/model/DataDifferencerTest.yml index 7da6dc710..528cf0088 100644 --- a/tests/model/DataDifferencerTest.yml +++ b/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 \ No newline at end of file + Choices: a,b + Image: =>DataDifferencerTest_MockImage.image1 + HasOneRelation: =>DataDifferencerTest_HasOneRelationObject.relobj1 \ No newline at end of file