API CHANGE Publish ex-VirtualPage records when their page type changes, propagate type change to live (AIR-78) BUGFIX Remove extraneous database rows when changing a page from VirtualPage to another type (AIR-78)

This commit is contained in:
Ingo Schommer 2012-01-14 11:31:01 +01:00
parent 9101a75123
commit a570e2f2a0
2 changed files with 149 additions and 17 deletions

View File

@ -244,6 +244,38 @@ class VirtualPage extends Page {
$this->updateImageTracking();
}
}
// Check if page type has changed to a non-virtual page.
// Caution: Relies on the fact that the current instance is still of the old page type.
if($this->isChanged('ClassName', 2)) {
$changed = $this->getChangedFields();
$classBefore = $changed['ClassName']['before'];
$classAfter = $changed['ClassName']['after'];
if($classBefore != $classAfter) {
// Remove all database rows for the old page type to avoid inconsistent data retrieval.
// TODO This should apply to all page type changes, not only on VirtualPage - but needs
// more comprehensive testing as its a destructive operation
$removedTables = array_diff(ClassInfo::dataClassesFor($classBefore), ClassInfo::dataClassesFor($classAfter));
if($removedTables) foreach($removedTables as $removedTable) {
// Note: *_versions records are left intact
foreach(array('', 'Live') as $stage) {
if($stage) $removedTable = "{$removedTable}_{$stage}";
DB::query(sprintf('DELETE FROM "%s" WHERE "ID" = %d', $removedTable, $this->ID));
}
}
// Also publish the change immediately to avoid inconsistent behaviour between
// a non-virtual draft and a virtual live record (e.g. republishing the original record
// shouldn't republish the - now unrelated - changes on the ex-VirtualPage draft).
// Copies all stage fields to live as well.
$source = DataObject::get_one("SiteTree",sprintf('"SiteTree"."ID" = %d', $this->CopyContentFromID));
$this->copyFrom($source);
$this->publish('Stage', 'Live');
// Change reference on instance (as well as removing the underlying database tables)
$this->CopyContentFromID = 0;
}
}
}
function validate() {

View File

@ -6,8 +6,29 @@ class VirtualPageTest extends SapphireTest {
protected $extraDataObjects = array(
'VirtualPageTest_ClassA',
'VirtualPageTest_ClassB',
'VirtualPageTest_NotRoot',
'VirtualPageTest_VirtualPageSub',
);
protected $requiredExtensions = array(
'Page' => array('VirtualPageTest_PageExtension')
);
function setUp() {
parent::setUp();
$this->origInitiallyCopiedFields = VirtualPage::$initially_copied_fields;
VirtualPage::$initially_copied_fields[] = 'MyInitiallyCopiedField';
$this->origNonVirtualField = VirtualPage::$non_virtual_fields;
VirtualPage::$non_virtual_fields[] = 'MyNonVirtualField';
VirtualPage::$non_virtual_fields[] = 'MySharedNonVirtualField';
}
function tearDown() {
parent::tearDown();
VirtualPage::$initially_copied_fields = $this->origInitiallyCopiedFields;
VirtualPage::$non_virtual_fields = $this->origNonVirtualField;
}
/**
* Test that, after you update the source page of a virtual page, all the virtual pages
@ -355,11 +376,6 @@ class VirtualPageTest extends SapphireTest {
}
function testGetVirtualFields() {
$origInitiallyCopiedFields = VirtualPage::$initially_copied_fields;
VirtualPage::$initially_copied_fields[] = 'MyInitiallyCopiedField';
$origNonVirtualField = VirtualPage::$non_virtual_fields;
VirtualPage::$non_virtual_fields[] = 'MyNonVirtualField';
// Needs association with an original, otherwise will just return the "base" virtual fields
$page = new VirtualPageTest_ClassA();
$page->write();
@ -370,17 +386,9 @@ class VirtualPageTest extends SapphireTest {
$this->assertContains('MyVirtualField', $virtual->getVirtualFields());
$this->assertNotContains('MyNonVirtualField', $virtual->getVirtualFields());
$this->assertNotContains('MyInitiallyCopiedField', $virtual->getVirtualFields());
VirtualPage::$initially_copied_fields = $origInitiallyCopiedFields;
VirtualPage::$non_virtual_fields = $origNonVirtualField;
}
function testCopyFrom() {
$origInitiallyCopiedFields = VirtualPage::$initially_copied_fields;
VirtualPage::$initially_copied_fields[] = 'MyInitiallyCopiedField';
$origNonVirtualField = VirtualPage::$non_virtual_fields;
VirtualPage::$non_virtual_fields[] = 'MyNonVirtualField';
$original = new VirtualPageTest_ClassA();
$original->MyInitiallyCopiedField = 'original';
$original->MyVirtualField = 'original';
@ -416,9 +424,6 @@ class VirtualPageTest extends SapphireTest {
$virtual->MyInitiallyCopiedField,
'Fields listed in $initially_copied_fields are not copied on subsequent copyFrom() invocations'
);
VirtualPage::$initially_copied_fields = $origInitiallyCopiedFields;
VirtualPage::$non_virtual_fields = $origNonVirtualField;
}
function testWriteWithoutVersion() {
@ -494,6 +499,82 @@ class VirtualPageTest extends SapphireTest {
if(!$isDetected) $this->fail('Fails validation with $can_be_root=false');
}
function testPageTypeChangeDoesntKeepOrphanedVirtualPageRecord() {
$page = new SiteTree();
$page->write();
$page->publish('Stage', 'Live');
$virtual = new VirtualPageTest_VirtualPageSub();
$virtual->CopyContentFromID = $page->ID;
$virtual->write();
$virtual->publish('Stage', 'Live');
$nonVirtual = $virtual;
$nonVirtual->ClassName = 'VirtualPageTest_ClassA';
$nonVirtual->write(); // not publishing
$this->assertNotNull(
DB::query(sprintf('SELECT "ID" FROM "SiteTree" WHERE "ID" = %d', $nonVirtual->ID))->value(),
"Shared base database table entry exists after type change"
);
$this->assertNull(
DB::query(sprintf('SELECT "ID" FROM "VirtualPage" WHERE "ID" = %d', $nonVirtual->ID))->value(),
"Base database table entry no longer exists after type change"
);
$this->assertNull(
DB::query(sprintf('SELECT "ID" FROM "VirtualPageTest_VirtualPageSub" WHERE "ID" = %d', $nonVirtual->ID))->value(),
"Sub database table entry no longer exists after type change"
);
$this->assertNull(
DB::query(sprintf('SELECT "ID" FROM "VirtualPage_Live" WHERE "ID" = %d', $nonVirtual->ID))->value(),
"Base live database table entry no longer exists after type change"
);
$this->assertNull(
DB::query(sprintf('SELECT "ID" FROM "VirtualPageTest_VirtualPageSub_Live" WHERE "ID" = %d', $nonVirtual->ID))->value(),
"Sub live database table entry no longer exists after type change"
);
}
function testPageTypeChangePropagatesToLive() {
$page = new SiteTree();
$page->MySharedNonVirtualField = 'original';
$page->write();
$page->publish('Stage', 'Live');
$virtual = new VirtualPageTest_VirtualPageSub();
$virtual->CopyContentFromID = $page->ID;
$virtual->write();
$virtual->publish('Stage', 'Live');
$page->Title = 'original'; // 'Title' is a virtual field
// Publication would causes the virtual field to copy through onBeforeWrite(),
// but we want to test that it gets copied on class name change instead
$page->write();
$nonVirtual = $virtual;
$nonVirtual->ClassName = 'VirtualPageTest_ClassA';
$nonVirtual->MySharedNonVirtualField = 'changed on new type';
$nonVirtual->write(); // not publishing the page type change here
$this->assertEquals('original', $nonVirtual->Title,
'Copies virtual fields from original draft into new instance on type change '
);
$nonVirtualLive = Versioned::get_one_by_stage('SiteTree', 'Live', '"SiteTree_Live"."ID" = ' . $nonVirtual->ID);
$this->assertNotNull($nonVirtualLive);
$this->assertEquals('VirtualPageTest_ClassA', $nonVirtualLive->ClassName);
$this->assertEquals('changed on new type', $nonVirtualLive->MySharedNonVirtualField);
$page->MySharedNonVirtualField = 'changed only on original';
$page->write();
$page->publish('Stage', 'Live');
$nonVirtualLive = Versioned::get_one_by_stage('SiteTree', 'Live', '"SiteTree_Live"."ID" = ' . $nonVirtual->ID, false);
$this->assertEquals('changed on new type', $nonVirtualLive->MySharedNonVirtualField,
'No field copying from previous original after page type changed'
);
}
}
class VirtualPageTest_ClassA extends Page implements TestOnly {
@ -517,4 +598,23 @@ class VirtualPageTest_ClassC extends Page implements TestOnly {
class VirtualPageTest_NotRoot extends Page implements TestOnly {
static $can_be_root = false;
}
class VirtualPageTest_VirtualPageSub extends VirtualPage implements TestOnly {
static $db = array(
'MyProperty' => 'Varchar',
);
}
class VirtualPageTest_PageExtension extends DataExtension implements TestOnly {
function extraStatics() {
return array(
'db' => array(
// These fields are just on an extension to simulate shared properties between Page and VirtualPage.
// Not possible through direct $db definitions due to VirtualPage inheriting from Page, and Page being defined elsewhere.
'MySharedVirtualField' => 'Text',
'MySharedNonVirtualField' => 'Text',
)
);
}
}