diff --git a/docs/en/02_Developer_Guides/00_Model/10_Versioning.md b/docs/en/02_Developer_Guides/00_Model/10_Versioning.md index b344df6d2..e7c40799a 100644 --- a/docs/en/02_Developer_Guides/00_Model/10_Versioning.md +++ b/docs/en/02_Developer_Guides/00_Model/10_Versioning.md @@ -27,6 +27,11 @@ The extension is automatically applied to `SiteTree` class. For more information [Extending](../extending) and the [Configuration](../configuration) documentation. +
+Versioning only works if you are adding the extension to the base class. That is, the first subclass +of `DataObject`. Adding this extension to children of the base class will have unpredictable behaviour. +
+ ## Database Structure Depending on how many stages you configured, two or more new tables will be created for your records. In the above, this diff --git a/model/Versioned.php b/model/Versioned.php index e089d4feb..a49f1076f 100644 --- a/model/Versioned.php +++ b/model/Versioned.php @@ -652,26 +652,31 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { // Get ID field $id = $manipulation[$table]['id'] ? $manipulation[$table]['id'] : $manipulation[$table]['fields']['ID']; - if(!$id) user_error("Couldn't find ID in " . var_export($manipulation[$table], true), E_USER_ERROR); + if(!$id) { + user_error("Couldn't find ID in " . var_export($manipulation[$table], true), E_USER_ERROR); + } if($this->migratingVersion) { $manipulation[$table]['fields']['Version'] = $this->migratingVersion; } - // If we haven't got a version #, then we're creating a new version. - // Otherwise, we're just copying a version to another table - if(empty($manipulation[$table]['fields']['Version'])) { + $version = isset($manipulation[$table]['fields']['Version']) + ? $manipulation[$table]['fields']['Version'] + : null; + if($version < 0 || $this->_nextWriteWithoutVersion) { + // Putting a Version of -1 is a signal to leave the version table alone, despite their being no version + unset($manipulation[$table]['fields']['Version']); + } elseif(empty($version)) { + // If we haven't got a version #, then we're creating a new version. + // Otherwise, we're just copying a version to another table $this->augmentWriteVersioned($manipulation, $table, $id); } - // Putting a Version of -1 is a signal to leave the version table alone, despite their being no version - if($manipulation[$table]['fields']['Version'] < 0 || $this->_nextWriteWithoutVersion) { + // For base classes of versioned data objects + if(!$this->hasVersionField($table)) { unset($manipulation[$table]['fields']['Version']); } - // For base classes of versioned data objects - if(!$this->hasVersionField($table)) unset($manipulation[$table]['fields']['Version']); - // Grab a version number - it should be the same across all tables. if(isset($manipulation[$table]['fields']['Version'])) { $thisVersion = $manipulation[$table]['fields']['Version']; diff --git a/tests/model/DataObjectLazyLoadingTest.php b/tests/model/DataObjectLazyLoadingTest.php index 79db4d151..cf6c7b8a7 100644 --- a/tests/model/DataObjectLazyLoadingTest.php +++ b/tests/model/DataObjectLazyLoadingTest.php @@ -13,6 +13,7 @@ class DataObjectLazyLoadingTest extends SapphireTest { // These are all defined in DataObjectTest.php and VersionedTest.php protected $extraDataObjects = array( + // From DataObjectTest 'DataObjectTest_Team', 'DataObjectTest_Fixture', 'DataObjectTest_SubTeam', @@ -24,8 +25,24 @@ class DataObjectLazyLoadingTest extends SapphireTest { 'DataObjectTest_TeamComment', 'DataObjectTest_EquipmentCompany', 'DataObjectTest_SubEquipmentCompany', + 'DataObjectTest\NamespacedClass', + 'DataObjectTest\RelationClass', + 'DataObjectTest_ExtendedTeamComment', + 'DataObjectTest_Company', + 'DataObjectTest_Staff', + 'DataObjectTest_CEO', + 'DataObjectTest_Fan', + 'DataObjectTest_Play', + 'DataObjectTest_Ploy', + 'DataObjectTest_Bogey', + // From VersionedTest 'VersionedTest_DataObject', 'VersionedTest_Subclass', + 'VersionedTest_AnotherSubclass', + 'VersionedTest_RelatedWithoutVersion', + 'VersionedTest_SingleStage', + 'VersionedTest_WithIndexes', + // From DataObjectLazyLoadingTest 'VersionedLazy_DataObject', 'VersionedLazySub_DataObject', ); diff --git a/tests/model/VersionedTest.php b/tests/model/VersionedTest.php index 989b71be0..c2b4c42a7 100644 --- a/tests/model/VersionedTest.php +++ b/tests/model/VersionedTest.php @@ -154,13 +154,13 @@ class VersionedTest extends SapphireTest { "\"VersionedTest_DataObject\".\"ID\" ASC"); // Check that page 3 has gone $this->assertNotNull($remainingPages); - $this->assertEquals(array("Page 1", "Page 2"), $remainingPages->column('Title')); + $this->assertEquals(array("Page 1", "Page 2", "Subclass Page 1"), $remainingPages->column('Title')); // Get all including deleted $allPages = Versioned::get_including_deleted("VersionedTest_DataObject", "\"ParentID\" = 0", "\"VersionedTest_DataObject\".\"ID\" ASC"); // Check that page 3 is still there - $this->assertEquals(array("Page 1", "Page 2", "Page 3"), $allPages->column('Title')); + $this->assertEquals(array("Page 1", "Page 2", "Page 3", "Subclass Page 1"), $allPages->column('Title')); // Check that the returned pages have the correct IDs $this->assertEquals($allPageIDs, $allPages->column('ID')); @@ -169,7 +169,7 @@ class VersionedTest extends SapphireTest { Versioned::reading_stage("Live"); $allPages = Versioned::get_including_deleted("VersionedTest_DataObject", "\"ParentID\" = 0", "\"VersionedTest_DataObject\".\"ID\" ASC"); - $this->assertEquals(array("Page 1", "Page 2", "Page 3"), $allPages->column('Title')); + $this->assertEquals(array("Page 1", "Page 2", "Page 3", "Subclass Page 1"), $allPages->column('Title')); // Check that the returned pages still have the correct IDs $this->assertEquals($allPageIDs, $allPages->column('ID')); @@ -208,7 +208,7 @@ class VersionedTest extends SapphireTest { } public function testRollbackTo() { - $page1 = $this->objFromFixture('VersionedTest_DataObject', 'page1'); + $page1 = $this->objFromFixture('VersionedTest_AnotherSubclass', 'subclass1'); $page1->Content = 'orig'; $page1->write(); $page1->publish('Stage', 'Live'); @@ -225,6 +225,17 @@ class VersionedTest extends SapphireTest { $this->assertTrue($page1->Version > $changedVersion, 'Create a new higher version number'); $this->assertEquals('orig', $page1->Content, 'Copies the content from the old version'); + + // check db entries + $version = DB::prepared_query("SELECT MAX(\"Version\") FROM \"VersionedTest_DataObject_versions\" WHERE \"RecordID\" = ?", + array($page1->ID) + )->value(); + $this->assertEquals($page1->Version, $version, 'Correct entry in VersionedTest_DataObject_versions'); + + $version = DB::prepared_query("SELECT MAX(\"Version\") FROM \"VersionedTest_AnotherSubclass_versions\" WHERE \"RecordID\" = ?", + array($page1->ID) + )->value(); + $this->assertEquals($page1->Version, $version, 'Correct entry in VersionedTest_AnotherSubclass_versions'); } public function testDeleteFromStage() { @@ -318,6 +329,7 @@ class VersionedTest extends SapphireTest { $noversion = new DataObject(); $versioned = new VersionedTest_DataObject(); $versionedSub = new VersionedTest_Subclass(); + $versionedAno = new VersionedTest_AnotherSubclass(); $versionField = new VersionedTest_UnversionedWithField(); $this->assertFalse( @@ -329,8 +341,14 @@ class VersionedTest extends SapphireTest { 'The versioned ext adds an Int version field.' ); $this->assertEquals( - 'Int', $versionedSub->hasOwnTableDatabaseField('Version'), - 'Sub-classes of a versioned model have a Version field.' + null, + $versionedSub->hasOwnTableDatabaseField('Version'), + 'Sub-classes of a versioned model don\'t have a Version field.' + ); + $this->assertEquals( + null, + $versionedAno->hasOwnTableDatabaseField('Version'), + 'Sub-classes of a versioned model don\'t have a Version field.' ); $this->assertEquals( 'Varchar', $versionField->hasOwnTableDatabaseField('Version'), @@ -818,10 +836,6 @@ class VersionedTest_Subclass extends VersionedTest_DataObject implements TestOnl private static $db = array( "ExtraField" => "Varchar", ); - - private static $extensions = array( - "Versioned('Stage', 'Live')" - ); } /** diff --git a/tests/model/VersionedTest.yml b/tests/model/VersionedTest.yml index 0d050c5a8..1c01b70a4 100644 --- a/tests/model/VersionedTest.yml +++ b/tests/model/VersionedTest.yml @@ -1,19 +1,24 @@ VersionedTest_DataObject: - page1: - Title: Page 1 - page2: - Title: Page 2 - page3: - Title: Page 3 - page2a: - Parent: =>VersionedTest_DataObject.page2 - Title: Page 2a - page2b: - Parent: =>VersionedTest_DataObject.page2 - Title: Page 2b - page3a: - Parent: =>VersionedTest_DataObject.page3 - Title: Page 3a - page3b: - Parent: =>VersionedTest_DataObject.page3 - Title: Page 3b + page1: + Title: Page 1 + page2: + Title: Page 2 + page3: + Title: Page 3 + page2a: + Parent: =>VersionedTest_DataObject.page2 + Title: Page 2a + page2b: + Parent: =>VersionedTest_DataObject.page2 + Title: Page 2b + page3a: + Parent: =>VersionedTest_DataObject.page3 + Title: Page 3a + page3b: + Parent: =>VersionedTest_DataObject.page3 + Title: Page 3b + +VersionedTest_AnotherSubclass: + subclass1: + Title: 'Subclass Page 1' + AnotherField: 'Bob'