Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API Virtual pages now respect cascade_deletes on source page #1919

Merged
merged 1 commit into from
Aug 9, 2017
Merged

API Virtual pages now respect cascade_deletes on source page #1919

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion code/Model/SiteTree.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion code/Model/VirtualPage.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
100 changes: 40 additions & 60 deletions tests/model/SiteTreeBrokenLinksTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public function tearDown()

public function testBrokenLinksBetweenPages()
{
/** @var Page $obj */
$obj = $this->objFromFixture('Page', 'content');

$obj->Content = '<a href="[sitetree_link,id=3423423]">this is a broken link</a>';
Expand All @@ -50,6 +51,7 @@ public function testBrokenLinksBetweenPages()

public function testBrokenAnchorBetweenPages()
{
/** @var Page $obj */
$obj = $this->objFromFixture('Page', 'content');
$target = $this->objFromFixture('Page', 'about');

Expand Down Expand Up @@ -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
Expand All @@ -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 = "<p><a href=\"[sitetree_link,id=$linkDest->ID]\">about us</a></p>";
$linkSrc->write();
Expand Down Expand Up @@ -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";
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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 = "<a href=\"[sitetree_link,id=$p->ID]\">test</a>";
$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 = "<a href=\"[sitetree_link,id={$pageID}]\">test</a>";
$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;

Expand Down
97 changes: 28 additions & 69 deletions tests/model/VirtualPageTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,16 @@
<?php

use SilverStripe\ORM\DataObject;
use SilverStripe\Versioned\Versioned;
use SilverStripe\ORM\DB;
use SilverStripe\ORM\ValidationException;
use SilverStripe\ORM\FieldType\DBVarchar;
use SilverStripe\ORM\DataExtension;
use SilverStripe\CMS\Model\VirtualPage;
use SilverStripe\CMS\Model\SiteTree;
use SilverStripe\CMS\Model\RedirectorPage;
use SilverStripe\CMS\Controllers\ModelAsController;
use SilverStripe\CMS\Model\RedirectorPage;
use SilverStripe\CMS\Model\SiteTree;
use SilverStripe\CMS\Model\VirtualPage;
use SilverStripe\Core\Config\Config;
use SilverStripe\Dev\FunctionalTest;
use SilverStripe\Dev\TestOnly;
use SilverStripe\ORM\DataObject;
use SilverStripe\ORM\DB;
use SilverStripe\ORM\ValidationException;
use SilverStripe\Security\Member;
use SilverStripe\Versioned\Versioned;

class VirtualPageTest extends FunctionalTest
{
Expand Down Expand Up @@ -200,51 +198,12 @@ public function testCantPublishVirtualPagesBeforeTheirSource()
$this->assertTrue($vp->canPublish());
}

public function testCanDeleteOrphanedVirtualPagesFromLive()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooh, no more orphaned virtual pages, no wonder it's no longer needed in the "broken links" report

{
// 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);
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -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()
Expand All @@ -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);
}

/**
Expand Down