diff --git a/model/Versioned.php b/model/Versioned.php index a49f1076f..eb95bd42e 100644 --- a/model/Versioned.php +++ b/model/Versioned.php @@ -5,6 +5,8 @@ * allowing you to rollback changes and view history. An example of this is * the pages used in the CMS. * + * @property int $Version + * * @package framework * @subpackage model */ @@ -487,12 +489,12 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { foreach($versionedTables as $child) { if($table === $child) break; // only need subclasses - + // Select all orphaned version records $orphanedQuery = SQLSelect::create() ->selectField("\"{$table}_versions\".\"ID\"") ->setFrom("\"{$table}_versions\""); - + // If we have a parent table limit orphaned records // to only those that exist in this if(DB::get_schema()->hasTable("{$child}_versions")) { @@ -568,7 +570,7 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { */ protected function augmentWriteVersioned(&$manipulation, $table, $recordID) { $baseDataClass = ClassInfo::baseDataClass($table); - + // Set up a new entry in (table)_versions $newManipulation = array( "command" => "insert", @@ -606,15 +608,17 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { } $nextVersion = $nextVersion ?: 1; - // Add the version number to this data - $manipulation[$table]['fields']['Version'] = $nextVersion; - $newManipulation['fields']['Version'] = $nextVersion; - - // Write AuthorID for baseclass if($table === $baseDataClass) { + // Write AuthorID for baseclass $userID = (Member::currentUser()) ? Member::currentUser()->ID : 0; $newManipulation['fields']['AuthorID'] = $userID; + + // Update main table version if not previously known + $manipulation[$table]['fields']['Version'] = $nextVersion; } + + // Update _versions table manipulation + $newManipulation['fields']['Version'] = $nextVersion; $manipulation["{$table}_versions"] = $newManipulation; } @@ -641,6 +645,19 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { public function augmentWrite(&$manipulation) { + // get Version number from base data table on write + $version = null; + $baseDataClass = ClassInfo::baseDataClass($this->owner->class); + if(isset($manipulation[$baseDataClass]['fields'])) { + if ($this->migratingVersion) { + $manipulation[$baseDataClass]['fields']['Version'] = $this->migratingVersion; + } + if (isset($manipulation[$baseDataClass]['fields']['Version'])) { + $version = $manipulation[$baseDataClass]['fields']['Version']; + } + } + + // Update all tables $tables = array_keys($manipulation); foreach($tables as $table) { @@ -649,20 +666,15 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { unset($manipulation[$table]); continue; } - + // Get ID field - $id = $manipulation[$table]['id'] ? $manipulation[$table]['id'] : $manipulation[$table]['fields']['ID']; + $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($this->migratingVersion) { - $manipulation[$table]['fields']['Version'] = $this->migratingVersion; - } - - $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']); @@ -672,7 +684,7 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { $this->augmentWriteVersioned($manipulation, $table, $id); } - // For base classes of versioned data objects + // Remove "Version" column from subclasses of baseDataClass if(!$this->hasVersionField($table)) { unset($manipulation[$table]['fields']['Version']); } @@ -805,56 +817,64 @@ class Versioned extends DataExtension implements TemplateGlobalProvider { /** * Move a database record from one stage to the other. * - * @param fromStage Place to copy from. Can be either a stage name or a version number. - * @param toStage Place to copy to. Must be a stage name. - * @param createNewVersion Set this to true to create a new version number. By default, the existing version - * number will be copied over. + * @param int|string $fromStage Place to copy from. Can be either a stage name or a version number. + * @param string $toStage Place to copy to. Must be a stage name. + * @param bool $createNewVersion Set this to true to create a new version number. + * By default, the existing version number will be copied over. */ public function publish($fromStage, $toStage, $createNewVersion = false) { $this->owner->extend('onBeforeVersionedPublish', $fromStage, $toStage, $createNewVersion); - $baseClass = $this->owner->class; - while( ($p = get_parent_class($baseClass)) != "DataObject") $baseClass = $p; + $baseClass = ClassInfo::baseDataClass($this->owner->class); $extTable = $this->extendWithSuffix($baseClass); + /** @var Versioned|DataObject $from */ if(is_numeric($fromStage)) { $from = Versioned::get_version($baseClass, $this->owner->ID, $fromStage); } else { $this->owner->flushCache(); - $from = Versioned::get_one_by_stage($baseClass, $fromStage, "\"{$baseClass}\".\"ID\"={$this->owner->ID}"); + $from = Versioned::get_one_by_stage($baseClass, $fromStage, array( + "\"{$baseClass}\".\"ID\" = ?" => $this->owner->ID + )); + } + if(!$from) { + user_error("Can't find {$this->owner->class}/{$this->owner->ID} in stage {$fromStage}", E_USER_WARNING); + return; } - $publisherID = isset(Member::currentUser()->ID) ? Member::currentUser()->ID : 0; - if($from) { - $from->forceChange(); - if($createNewVersion) { - $latest = self::get_latest_version($baseClass, $this->owner->ID); - $this->owner->Version = $latest->Version + 1; - } else { - $from->migrateVersion($from->Version); - } + // Set version of new record + $from->forceChange(); + if($createNewVersion) { + // Clear version to be automatically created on write + $from->Version = null; + } else { + $from->migrateVersion($from->Version); // Mark this version as having been published at some stage + $publisherID = isset(Member::currentUser()->ID) ? Member::currentUser()->ID : 0; DB::prepared_query("UPDATE \"{$extTable}_versions\" SET \"WasPublished\" = ?, \"PublisherID\" = ? WHERE \"RecordID\" = ? AND \"Version\" = ?", array(1, $publisherID, $from->ID, $from->Version) ); - - $oldMode = Versioned::get_reading_mode(); - Versioned::reading_stage($toStage); - - $conn = DB::get_conn(); - if(method_exists($conn, 'allowPrimaryKeyEditing')) $conn->allowPrimaryKeyEditing($baseClass, true); - $from->write(); - if(method_exists($conn, 'allowPrimaryKeyEditing')) $conn->allowPrimaryKeyEditing($baseClass, false); - - $from->destroy(); - - Versioned::set_reading_mode($oldMode); - } else { - user_error("Can't find {$this->owner->URLSegment}/{$this->owner->ID} in stage $fromStage", E_USER_WARNING); } + + // Change to new stage, write, and revert state + $oldMode = Versioned::get_reading_mode(); + Versioned::reading_stage($toStage); + + $conn = DB::get_conn(); + if(method_exists($conn, 'allowPrimaryKeyEditing')) { + $conn->allowPrimaryKeyEditing($baseClass, true); + $from->write(); + $conn->allowPrimaryKeyEditing($baseClass, false); + } else { + $from->write(); + } + + $from->destroy(); + + Versioned::set_reading_mode($oldMode); } /** diff --git a/tests/model/VersionedTest.php b/tests/model/VersionedTest.php index c2b4c42a7..0e205cd43 100644 --- a/tests/model/VersionedTest.php +++ b/tests/model/VersionedTest.php @@ -31,7 +31,7 @@ class VersionedTest extends SapphireTest { 'VersionedTest_WithIndexes_Live' => array('value' => false, 'message' => 'Unique indexes are no longer unique in _Live table'), ); - + // Test each table's performance foreach ($tableExpectations as $tableName => $expectation) { $indexes = DB::get_schema()->indexList($tableName); @@ -196,15 +196,37 @@ class VersionedTest extends SapphireTest { $page1 = $this->objFromFixture('VersionedTest_DataObject', 'page1'); $page1->Content = 'orig'; $page1->write(); - $oldVersion = $page1->Version; + $firstVersion = $page1->Version; $page1->publish('Stage', 'Live', false); - $this->assertEquals($oldVersion, $page1->Version, 'publish() with $createNewVersion=FALSE'); + $this->assertEquals( + $firstVersion, + $page1->Version, + 'publish() with $createNewVersion=FALSE does not create a new version' + ); $page1->Content = 'changed'; $page1->write(); - $oldVersion = $page1->Version; + $secondVersion = $page1->Version; + $this->assertTrue($firstVersion < $secondVersion, 'write creates new version'); + $page1->publish('Stage', 'Live', true); - $this->assertTrue($oldVersion < $page1->Version, 'publish() with $createNewVersion=TRUE'); + $thirdVersion = Versioned::get_latest_version('VersionedTest_DataObject', $page1->ID)->Version; + $liveVersion = Versioned::get_versionnumber_by_stage('VersionedTest_DataObject', 'Live', $page1->ID); + $stageVersion = Versioned::get_versionnumber_by_stage('VersionedTest_DataObject', 'Stage', $page1->ID); + $this->assertTrue( + $secondVersion < $thirdVersion, + 'publish() with $createNewVersion=TRUE creates a new version' + ); + $this->assertEquals( + $liveVersion, + $thirdVersion, + 'publish() with $createNewVersion=TRUE publishes to live' + ); + $this->assertEquals( + $stageVersion, + $secondVersion, + 'publish() with $createNewVersion=TRUE does not affect stage' + ); } public function testRollbackTo() { @@ -220,10 +242,11 @@ class VersionedTest extends SapphireTest { $changedVersion = $page1->Version; $page1->doRollbackTo($origVersion); - $page1 = Versioned::get_one_by_stage('VersionedTest_DataObject', 'Stage', - sprintf('"VersionedTest_DataObject"."ID" = %d', $page1->ID)); + $page1 = Versioned::get_one_by_stage('VersionedTest_DataObject', 'Stage', array( + '"VersionedTest_DataObject"."ID" = ?' => $page1->ID + )); - $this->assertTrue($page1->Version > $changedVersion, 'Create a new higher version number'); + $this->assertTrue($page1->Version == $changedVersion + 1, 'Create a new higher version number'); $this->assertEquals('orig', $page1->Content, 'Copies the content from the old version'); // check db entries