From 2c54e33106ae1dfa2eb4d873769c2f17edd406ff Mon Sep 17 00:00:00 2001 From: Damian Mooyman Date: Wed, 9 Aug 2017 10:56:08 +1200 Subject: [PATCH] API Virtual pages now respect cascade_deletes on source page --- code/Model/SiteTree.php | 6 +- code/Model/VirtualPage.php | 2 +- tests/model/SiteTreeBrokenLinksTest.php | 100 ++++++++++-------------- tests/model/VirtualPageTest.php | 97 +++++++---------------- 4 files changed, 74 insertions(+), 131 deletions(-) diff --git a/code/Model/SiteTree.php b/code/Model/SiteTree.php index 754899f4f8..18da07a96b 100755 --- a/code/Model/SiteTree.php +++ b/code/Model/SiteTree.php @@ -193,13 +193,17 @@ class SiteTree extends DataObject implements PermissionProvider, i18nEntityProvi ); private static $has_many = array( - "VirtualPages" => "SilverStripe\\CMS\\Model\\VirtualPage.CopyContentFrom" + "VirtualPages" => VirtualPage::class . '.CopyContentFrom' ); private static $owned_by = array( "VirtualPages" ); + private static $cascade_deletes = [ + 'VirtualPages', + ]; + private static $casting = array( "Breadcrumbs" => "HTMLFragment", "LastEdited" => "Datetime", diff --git a/code/Model/VirtualPage.php b/code/Model/VirtualPage.php index fc47d9299b..2154610ee0 100644 --- a/code/Model/VirtualPage.php +++ b/code/Model/VirtualPage.php @@ -66,7 +66,7 @@ class VirtualPage extends Page ); private static $has_one = array( - "CopyContentFrom" => "SilverStripe\\CMS\\Model\\SiteTree", + "CopyContentFrom" => SiteTree::class, ); private static $owns = array( diff --git a/tests/model/SiteTreeBrokenLinksTest.php b/tests/model/SiteTreeBrokenLinksTest.php index 6e12d46cc0..179b3a92ef 100644 --- a/tests/model/SiteTreeBrokenLinksTest.php +++ b/tests/model/SiteTreeBrokenLinksTest.php @@ -37,6 +37,7 @@ public function tearDown() public function testBrokenLinksBetweenPages() { + /** @var Page $obj */ $obj = $this->objFromFixture('Page', 'content'); $obj->Content = 'this is a broken link'; @@ -50,6 +51,7 @@ public function testBrokenLinksBetweenPages() public function testBrokenAnchorBetweenPages() { + /** @var Page $obj */ $obj = $this->objFromFixture('Page', 'content'); $target = $this->objFromFixture('Page', 'about'); @@ -141,7 +143,6 @@ public function testDeletingMarksBackLinkedPagesAsBroken() $this->assertEquals(0, (int)$linkSrc->HasBrokenLink); // Delete page from draft - $linkDestID = $linkDest->ID; $linkDest->delete(); // Confirm draft has broken link @@ -156,10 +157,12 @@ public function testPublishingSourceBeforeDestHasBrokenLink() $this->logInWithPermission('ADMIN'); // Set up two draft pages with a link from content -> about + /** @var Page $linkDest */ $linkDest = $this->objFromFixture('Page', 'about'); // Ensure that it's not on the published site $linkDest->doUnpublish(); + /** @var Page $linkSrc */ $linkSrc = $this->objFromFixture('Page', 'content'); $linkSrc->Content = "

ID]\">about us

"; $linkSrc->write(); @@ -191,11 +194,6 @@ public function testRestoreFixesBrokenLinks() $p2->write(); $this->assertTrue($p2->publishRecursive()); - // Virtual pages are another - $vp = new VirtualPage(); - $vp->CopyContentFromID = $p->ID; - $vp->write(); - // Redirector links are a third $rp = new RedirectorPage(); $rp->Title = "redirector"; @@ -206,7 +204,6 @@ public function testRestoreFixesBrokenLinks() // Confirm that there are no broken links to begin with $this->assertFalse($p2->HasBrokenLink); - $this->assertFalse($vp->HasBrokenLink); $this->assertFalse($rp->HasBrokenLink); // Unpublishing doesn't affect broken state on live (draft is source of truth) @@ -218,14 +215,11 @@ public function testRestoreFixesBrokenLinks() // Delete the source page, confirm that the VP, RP and page 2 have broken links on draft $p->delete(); - $vp->flushCache(); - $vp = DataObject::get_by_id(SiteTree::class, $vp->ID); $p2->flushCache(); $p2 = DataObject::get_by_id(SiteTree::class, $p2->ID); $rp->flushCache(); $rp = DataObject::get_by_id(SiteTree::class, $rp->ID); $this->assertEquals(1, $p2->HasBrokenLink); - $this->assertEquals(1, $vp->HasBrokenLink); $this->assertEquals(1, $rp->HasBrokenLink); // Restore the page to stage, confirm that this fixes the links @@ -235,12 +229,9 @@ public function testRestoreFixesBrokenLinks() $p2->flushCache(); $p2 = DataObject::get_by_id(SiteTree::class, $p2->ID); - $vp->flushCache(); - $vp = DataObject::get_by_id(SiteTree::class, $vp->ID); $rp->flushCache(); $rp = DataObject::get_by_id(SiteTree::class, $rp->ID); $this->assertFalse((bool)$p2->HasBrokenLink); - $this->assertFalse((bool)$vp->HasBrokenLink); $this->assertFalse((bool)$rp->HasBrokenLink); // Publish and confirm that the p2 and RP broken links are fixed on published @@ -254,68 +245,57 @@ public function testRestoreFixesBrokenLinks() public function testRevertToLiveFixesBrokenLinks() { // Create page and virutal page - $p = new Page(); - $p->Title = "source"; - $p->write(); - $pageID = $p->ID; - $this->assertTrue($p->publishRecursive()); + $page = new Page(); + $page->Title = "source"; + $page->write(); + $pageID = $page->ID; + $this->assertTrue($page->publishRecursive()); // Content links are one kind of link to pages - $p2 = new Page(); - $p2->Title = "regular link"; - $p2->Content = "ID]\">test"; - $p2->write(); - $this->assertTrue($p2->publishRecursive()); - - // Virtual pages are another - $vp = new VirtualPage(); - $vp->CopyContentFromID = $p->ID; - $vp->write(); + $page2 = new Page(); + $page2->Title = "regular link"; + $page2->Content = "test"; + $page2->write(); + $this->assertTrue($page2->publishRecursive()); // Redirector links are a third - $rp = new RedirectorPage(); - $rp->Title = "redirector"; - $rp->LinkType = 'Internal'; - $rp->LinkToID = $p->ID; - $rp->write(); - $this->assertTrue($rp->publishRecursive()); + $redirectorPage = new RedirectorPage(); + $redirectorPage->Title = "redirector"; + $redirectorPage->LinkType = 'Internal'; + $redirectorPage->LinkToID = $page->ID; + $redirectorPage->write(); + $this->assertTrue($redirectorPage->publishRecursive()); // Confirm that there are no broken links to begin with - $this->assertFalse($p2->HasBrokenLink); - $this->assertFalse($vp->HasBrokenLink); - $this->assertFalse($rp->HasBrokenLink); + $this->assertFalse($page2->HasBrokenLink); + $this->assertFalse($redirectorPage->HasBrokenLink); // Delete from draft and confirm that broken links are marked - $pID = $p->ID; - $p->delete(); + $page->delete(); - $vp->flushCache(); - $vp = DataObject::get_by_id(SiteTree::class, $vp->ID); - $p2->flushCache(); - $p2 = DataObject::get_by_id(SiteTree::class, $p2->ID); - $rp->flushCache(); - $rp = DataObject::get_by_id(SiteTree::class, $rp->ID); - $this->assertEquals(1, $p2->HasBrokenLink); - $this->assertEquals(1, $vp->HasBrokenLink); - $this->assertEquals(1, $rp->HasBrokenLink); + $page2->flushCache(); + $page2 = DataObject::get_by_id(SiteTree::class, $page2->ID); + $redirectorPage->flushCache(); + $redirectorPage = DataObject::get_by_id(SiteTree::class, $redirectorPage->ID); + $this->assertEquals(1, $page2->HasBrokenLink); + $this->assertEquals(1, $redirectorPage->HasBrokenLink); // Call doRevertToLive and confirm that broken links are restored - $pLive = Versioned::get_one_by_stage(SiteTree::class, 'Live', '"SiteTree"."ID" = ' . $pID); - $pLive->doRevertToLive(); - - $p2->flushCache(); - $p2 = DataObject::get_by_id(SiteTree::class, $p2->ID); - $vp->flushCache(); - $vp = DataObject::get_by_id(SiteTree::class, $vp->ID); - $rp->flushCache(); - $rp = DataObject::get_by_id(SiteTree::class, $rp->ID); - $this->assertFalse((bool)$p2->HasBrokenLink); - $this->assertFalse((bool)$vp->HasBrokenLink); - $this->assertFalse((bool)$rp->HasBrokenLink); + /** @var Page $pageLive */ + $pageLive = Versioned::get_one_by_stage(SiteTree::class, 'Live', '"SiteTree"."ID" = ' . $pageID); + $pageLive->doRevertToLive(); + + $page2->flushCache(); + $page2 = DataObject::get_by_id(SiteTree::class, $page2->ID); + $redirectorPage->flushCache(); + $redirectorPage = DataObject::get_by_id(SiteTree::class, $redirectorPage->ID); + $this->assertFalse((bool)$page2->HasBrokenLink); + $this->assertFalse((bool)$redirectorPage->HasBrokenLink); } public function testBrokenAnchorLinksInAPage() { + /** @var Page $obj */ $obj = $this->objFromFixture('Page', 'content'); $origContent = $obj->Content; diff --git a/tests/model/VirtualPageTest.php b/tests/model/VirtualPageTest.php index 300d0d2879..9b7c325301 100644 --- a/tests/model/VirtualPageTest.php +++ b/tests/model/VirtualPageTest.php @@ -1,18 +1,16 @@ assertTrue($vp->canPublish()); } - public function testCanDeleteOrphanedVirtualPagesFromLive() - { - // An unpublished source page - $p = new Page(); - $p->Content = "test content"; - $p->write(); - $p->publishRecursive(); - $pID = $p->ID; - - $vp = new VirtualPage(); - $vp->CopyContentFromID = $p->ID; - $vp->write(); - $this->assertTrue($vp->canPublish()); - $this->assertTrue($vp->publishRecursive()); - - // Delete the source page semi-manually, without triggering - // the cascade publish back to the virtual page. - Versioned::set_stage(Versioned::LIVE); - $livePage = Versioned::get_by_stage(SiteTree::class, Versioned::LIVE)->byID($pID); - $livePage->delete(); - Versioned::set_stage(Versioned::DRAFT); - - // Confirm that we can unpublish, but not publish - $this->assertFalse($p->IsPublished(), 'Copied page has orphaned the virtual page on live'); - $this->assertTrue($vp->isPublished(), 'Virtual page remains on live'); - $this->assertTrue($vp->canUnpublish()); - $this->assertFalse($vp->canPublish()); - - // Confirm that the action really works - $this->assertTrue($vp->doUnpublish()); - $this->assertEquals( - 0, - DB::prepared_query( - "SELECT count(*) FROM \"SiteTree_Live\" WHERE \"ID\" = ?", - array($vp->ID) - )->value() - ); - } - public function testCanEdit() { $parentPage = $this->objFromFixture('Page', 'master3'); $virtualPage = $this->objFromFixture(VirtualPage::class, 'vp3'); - $bob = $this->objFromFixture('SilverStripe\\Security\\Member', 'bob'); - $andrew = $this->objFromFixture('SilverStripe\\Security\\Member', 'andrew'); + $bob = $this->objFromFixture(Member::class, 'bob'); + $andrew = $this->objFromFixture(Member::class, 'andrew'); // Bob can edit the mirrored page, but he shouldn't be able to edit the virtual page. $this->logInAs($bob); @@ -259,12 +218,13 @@ public function testCanEdit() public function testCanView() { + /** @var Page $parentPage */ $parentPage = $this->objFromFixture('Page', 'master3'); $parentPage->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE); $virtualPage = $this->objFromFixture(VirtualPage::class, 'vp3'); $virtualPage->copyVersionToStage(Versioned::DRAFT, Versioned::LIVE); - $cindy = $this->objFromFixture('SilverStripe\\Security\\Member', 'cindy'); - $alice = $this->objFromFixture('SilverStripe\\Security\\Member', 'alice'); + $cindy = $this->objFromFixture(Member::class, 'cindy'); + $alice = $this->objFromFixture(Member::class, 'alice'); // Cindy can see both pages $this->logInAs($cindy); @@ -336,6 +296,7 @@ public function testUnpublishingSourcePageOfAVirtualPageAlsoUnpublishesVirtualPa $vp = new VirtualPage(); $vp->CopyContentFromID = $p->ID; $vp->write(); + $vpID = $vp->ID; $this->assertTrue($vp->publishRecursive()); // All is fine, the virtual page doesn't have a broken link @@ -346,17 +307,17 @@ public function testUnpublishingSourcePageOfAVirtualPageAlsoUnpublishesVirtualPa // The draft VP still has the CopyContentFromID link $vp->flushCache(); - $vp = DataObject::get_by_id(SiteTree::class, $vp->ID); + $vp = DataObject::get_by_id(SiteTree::class, $vpID); $this->assertEquals($p->ID, $vp->CopyContentFromID); - $vpLive = Versioned::get_one_by_stage(SiteTree::class, Versioned::LIVE, '"SiteTree"."ID" = ' . $vp->ID); + $vpLive = Versioned::get_one_by_stage(SiteTree::class, Versioned::LIVE, '"SiteTree"."ID" = ' . $vpID); $this->assertNull($vpLive); - // Delete from draft, confirm that the virtual page has a broken link on the draft site + // Delete from draft, ensure virtual page deletion cascades $p->delete(); $vp->flushCache(); - $vp = DataObject::get_by_id(SiteTree::class, $vp->ID); - $this->assertEquals(1, $vp->HasBrokenLink); + $vp = DataObject::get_by_id(SiteTree::class, $vpID); + $this->assertNull($vp); } public function testDeletingFromLiveSourcePageOfAVirtualPageAlsoUnpublishesVirtualPage() @@ -369,29 +330,27 @@ public function testDeletingFromLiveSourcePageOfAVirtualPageAlsoUnpublishesVirtu $vp = new VirtualPage(); $vp->CopyContentFromID = $p->ID; $vp->write(); + $vpID = $vp->ID; $this->assertTrue($vp->publishRecursive()); // All is fine, the virtual page doesn't have a broken link $this->assertFalse($vp->HasBrokenLink); - // Delete the source page from draft, confirm that this creates a broken link + // Delete the source page from draft, cascades to virtual page $pID = $p->ID; $p->delete(); $vp->flushCache(); - $vp = DataObject::get_by_id(SiteTree::class, $vp->ID); - $this->assertEquals(1, $vp->HasBrokenLink); + $vpDraft = Versioned::get_by_stage(SiteTree::class, Versioned::DRAFT) + ->byID($pID); + $this->assertNull($vpDraft); // Delete the source page form live, confirm that the virtual page has also been unpublished - $pLive = Versioned::get_one_by_stage(SiteTree::class, Versioned::LIVE, '"SiteTree"."ID" = ' . $pID); + $pLive = Versioned::get_by_stage(SiteTree::class, Versioned::LIVE) + ->byID($pID); $this->assertTrue($pLive->doUnpublish()); - $vpLive = Versioned::get_one_by_stage(SiteTree::class, Versioned::LIVE, '"SiteTree"."ID" = ' . $vp->ID); + $vpLive = Versioned::get_by_stage(SiteTree::class, Versioned::LIVE) + ->byID($vpID); $this->assertNull($vpLive); - - // Delete from draft, confirm that the virtual page has a broken link on the draft site - $pLive->delete(); - $vp->flushCache(); - $vp = DataObject::get_by_id(SiteTree::class, $vp->ID); - $this->assertEquals(1, $vp->HasBrokenLink); } /**