From 7f28c32427ca5364ebfbe6a0cf347bb33f7debed Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Fri, 24 Aug 2018 10:12:05 +1200 Subject: [PATCH 1/2] FIX Pages now correctly duplicate children across subsites --- src/Extensions/SiteTreeSubsites.php | 86 +++++++++++++++++------------ tests/php/SiteTreeSubsitesTest.php | 43 ++++++++++----- 2 files changed, 78 insertions(+), 51 deletions(-) diff --git a/src/Extensions/SiteTreeSubsites.php b/src/Extensions/SiteTreeSubsites.php index 488fef6..6c7e55f 100644 --- a/src/Extensions/SiteTreeSubsites.php +++ b/src/Extensions/SiteTreeSubsites.php @@ -2,7 +2,6 @@ namespace SilverStripe\Subsites\Extensions; -use Page; use SilverStripe\CMS\Model\SiteTree; use SilverStripe\Control\Controller; use SilverStripe\Control\Director; @@ -170,57 +169,71 @@ class SiteTreeSubsites extends DataExtension } /** - * Does the basic duplication, but doesn't write anything - * this means we can subclass this easier and do more complex - * relation duplication. + * Does the basic duplication, but doesn't write anything this means we can subclass this easier and do more + * complex relation duplication. + * + * Note that when duplicating including children, everything is written. + * + * @param Subsite|int $subsiteID + * @param bool $includeChildren + * @return SiteTree */ - public function duplicateToSubsitePrep($subsiteID) + public function duplicateToSubsitePrep($subsiteID, $includeChildren) { if (is_object($subsiteID)) { $subsiteID = $subsiteID->ID; } - $oldSubsite = SubsiteState::singleton()->getSubsiteId(); - if ($subsiteID) { - Subsite::changeSubsite($subsiteID); - } else { - $subsiteID = $oldSubsite; + return SubsiteState::singleton() + ->withState(function (SubsiteState $newState) use ($subsiteID, $includeChildren) { + $newState->setSubsiteId($subsiteID); + + /** @var SiteTree $page */ + $page = $this->owner; + + try { + $originalFilter = Subsite::$disable_subsite_filter; + Subsite::disable_subsite_filter(true); + + return $includeChildren ? $page->duplicateWithChildren() : $page->duplicate(false); + } finally { + Subsite::disable_subsite_filter($originalFilter); + } + }); + } + + /** + * When duplicating a page, assign the current subsite ID from the state + */ + public function onBeforeDuplicate() + { + $subsiteId = SubsiteState::singleton()->getSubsiteId(); + if ($subsiteId !== null) { + $this->owner->SubsiteID = $subsiteId; } - // doesn't write as we need to reset the SubsiteID, ParentID etc - $clone = $this->owner->duplicate(false); - $clone->CheckedPublicationDifferences = $clone->AddedToStage = true; - $subsiteID = ($subsiteID ? $subsiteID : $oldSubsite); - $clone->SubsiteID = $subsiteID; - // We have no idea what the parentID should be, so as a workaround use the url-segment and subsite ID - if ($this->owner->Parent()) { - $parentSeg = $this->owner->Parent()->URLSegment; - $newParentPage = Page::get()->filter('URLSegment', $parentSeg)->first(); - if ($newParentPage) { - $clone->ParentID = $newParentPage->ID; - } else { - // reset it to the top level, so the user can decide where to put it - $clone->ParentID = 0; - } - } - // MasterPageID is here for legacy purposes, to satisfy the subsites_relatedpages module - $clone->MasterPageID = $this->owner->ID; - return $clone; } /** * Create a duplicate of this page and save it to another subsite - * @param $subsiteID int|Subsite The Subsite to copy to, or its ID + * + * @param Subsite|int $subsiteID The Subsite to copy to, or its ID + * @param boolean $includeChildren Whether to duplicate child pages too + * @return SiteTree The duplicated page */ - public function duplicateToSubsite($subsiteID = null) + public function duplicateToSubsite($subsiteID = null, $includeChildren = false) { - $clone = $this->owner->duplicateToSubsitePrep($subsiteID); + $clone = $this->owner->duplicateToSubsitePrep($subsiteID, $includeChildren); $clone->invokeWithExtensions('onBeforeDuplicateToSubsite', $this->owner); - $clone->write(); + + if (!$includeChildren) { + // Write the new page if "include children" is false, because it is written by default when it's true. + $clone->write(); + } + // Deprecated: manually duplicate any configured relationships $clone->duplicateSubsiteRelations($this->owner); - // new extension hooks which happens after write, - // onAfterDuplicate isn't reliable due to - // https://github.com/silverstripe/silverstripe-cms/issues/1253 + $clone->invokeWithExtensions('onAfterDuplicateToSubsite', $this->owner); + return $clone; } @@ -231,6 +244,7 @@ class SiteTreeSubsites extends DataExtension * It may be that some relations are not diostinct to sub site so can stay * whereas others may need to be duplicated * + * @param SiteTree $originalPage */ public function duplicateSubsiteRelations($originalPage) { diff --git a/tests/php/SiteTreeSubsitesTest.php b/tests/php/SiteTreeSubsitesTest.php index 17131ae..efb16f8 100644 --- a/tests/php/SiteTreeSubsitesTest.php +++ b/tests/php/SiteTreeSubsitesTest.php @@ -359,28 +359,41 @@ class SiteTreeSubsitesTest extends BaseSubsiteTest $this->assertEquals('important-page', $mainSubsiteImportantPage->URLSegment); } - public function testCopySubsiteWithChildren() + /** + * @param bool $withChildren + * @param int $expectedChildren + * @dataProvider duplicateToSubsiteProvider + */ + public function testDuplicateToSubsite($withChildren, $expectedChildren) { - $page = $this->objFromFixture('Page', 'about'); + /** @var SiteTree $page */ + $page = $this->objFromFixture(Page::class, 'about'); + /** @var Subsite $newSubsite */ $newSubsite = $this->objFromFixture(Subsite::class, 'subsite1'); - $moved = $page->duplicateToSubsite($newSubsite->ID, true); - $this->assertEquals($moved->SubsiteID, $newSubsite->ID, 'Ensure returned records are on new subsite'); - $this->assertEquals( - $moved->AllChildren()->count(), - $page->AllChildren()->count(), - 'All pages are copied across' + /** @var SiteTree $duplicatedPage */ + $duplicatedPage = $page->duplicateToSubsite($newSubsite->ID, $withChildren); + $this->assertInstanceOf(SiteTree::class, $duplicatedPage, 'A new page is returned'); + + $this->assertEquals($newSubsite->ID, $duplicatedPage->SubsiteID, 'Ensure returned records are on new subsite'); + + $this->assertCount(1, $page->AllChildren()); + $this->assertCount( + $expectedChildren, + $duplicatedPage->AllChildren(), + 'Duplicated page also duplicates children' ); } - public function testCopySubsiteWithoutChildren() + /** + * @return array[] + */ + public function duplicateToSubsiteProvider() { - $page = $this->objFromFixture('Page', 'about'); - $newSubsite = $this->objFromFixture(Subsite::class, 'subsite2'); - - $moved = $page->duplicateToSubsite($newSubsite->ID, false); - $this->assertEquals($moved->SubsiteID, $newSubsite->ID, 'Ensure returned records are on new subsite'); - $this->assertEquals($moved->AllChildren()->count(), 0, 'All pages are copied across'); + return [ + [true, 1], + [false, 0], + ]; } public function testIfSubsiteThemeIsSetToThemeList() From e8a72e1c3341a90b9a3b440a47ef672611970aa4 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Fri, 24 Aug 2018 10:30:27 +1200 Subject: [PATCH 2/2] FIX Duplicate page's parent IDs are now assumed or zeroed after duplication --- src/Extensions/SiteTreeSubsites.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/Extensions/SiteTreeSubsites.php b/src/Extensions/SiteTreeSubsites.php index 6c7e55f..ecef3a5 100644 --- a/src/Extensions/SiteTreeSubsites.php +++ b/src/Extensions/SiteTreeSubsites.php @@ -2,6 +2,7 @@ namespace SilverStripe\Subsites\Extensions; +use Page; use SilverStripe\CMS\Model\SiteTree; use SilverStripe\Control\Controller; use SilverStripe\Control\Director; @@ -192,12 +193,30 @@ class SiteTreeSubsites extends DataExtension $page = $this->owner; try { + // We have no idea what the ParentID should be, but it shouldn't be the same as it was since + // we're now in a different subsite. As a workaround use the url-segment and subsite ID. + if ($page->Parent()) { + $parentSeg = $page->Parent()->URLSegment; + $newParentPage = Page::get()->filter('URLSegment', $parentSeg)->first(); + $originalParentID = $page->ParentID; + if ($newParentPage) { + $page->ParentID = (int) $newParentPage->ID; + } else { + // reset it to the top level, so the user can decide where to put it + $page->ParentID = 0; + } + } + + // Disable query filtering by subsite during actual duplication $originalFilter = Subsite::$disable_subsite_filter; Subsite::disable_subsite_filter(true); return $includeChildren ? $page->duplicateWithChildren() : $page->duplicate(false); } finally { Subsite::disable_subsite_filter($originalFilter); + + // Re-set the original parent ID for the current page + $page->ParentID = $originalParentID; } }); }