From 1f51fcd90944ea1587c5c98acf00c71b70e47795 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Fri, 31 May 2019 11:24:12 +1200 Subject: [PATCH 1/2] FIX Subsites virtual pages now allow you to re-save them when used in conjunction with silverstripe-fluent --- src/Pages/SubsitesVirtualPage.php | 31 +++++++++++-------------------- 1 file changed, 11 insertions(+), 20 deletions(-) diff --git a/src/Pages/SubsitesVirtualPage.php b/src/Pages/SubsitesVirtualPage.php index b4c3d4f..d5ab24e 100644 --- a/src/Pages/SubsitesVirtualPage.php +++ b/src/Pages/SubsitesVirtualPage.php @@ -8,7 +8,6 @@ use SilverStripe\CMS\Model\VirtualPage; use SilverStripe\Control\Controller; use SilverStripe\Core\Config\Config; use SilverStripe\Forms\DropdownField; -use SilverStripe\Forms\LabelField; use SilverStripe\Forms\LiteralField; use SilverStripe\Forms\TextareaField; use SilverStripe\Forms\TextField; @@ -223,33 +222,25 @@ class SubsitesVirtualPage extends VirtualPage { $isValid = parent::validURLSegment(); + $filters = [ + 'URLSegment' => $this->URLSegment, + ]; + // Veto the validation rules if its false. In this case, some logic // needs to be duplicated from parent to find out the exact reason the validation failed. if (!$isValid) { - $IDFilter = $this->ID ? "AND \"SiteTree\".\"ID\" <> $this->ID" : null; - $parentFilter = null; + // Exclude the current page from the filter + $filters['ID:not'] = $this->ID; if (Config::inst()->get(SiteTree::class, 'nested_urls')) { - if ($this->ParentID) { - $parentFilter = " AND \"SiteTree\".\"ParentID\" = $this->ParentID"; - } else { - $parentFilter = ' AND "SiteTree"."ParentID" = 0'; - } + $filters['ParentID'] = $this->ParentID ?: 0; } $origDisableSubsiteFilter = Subsite::$disable_subsite_filter; - Subsite::$disable_subsite_filter = true; - $existingPage = DataObject::get_one( - SiteTree::class, - "\"URLSegment\" = '$this->URLSegment' $IDFilter $parentFilter", - false // disable cache, it doesn't include subsite status in the key - ); - Subsite::$disable_subsite_filter = $origDisableSubsiteFilter; - $existingPageInSubsite = DataObject::get_one( - SiteTree::class, - "\"URLSegment\" = '$this->URLSegment' $IDFilter $parentFilter", - false // disable cache, it doesn't include subsite status in the key - ); + Subsite::disable_subsite_filter(); + $existingPage = SiteTree::get()->filter($filters)->first(); + Subsite::disable_subsite_filter($origDisableSubsiteFilter); + $existingPageInSubsite = SiteTree::get()->filter($filters)->first(); // If URL has been vetoed because of an existing page, // be more specific and allow same URLSegments in different subsites From 900d04d94af650a4ee1ac65ebb40cd0f35fc0574 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Fri, 31 May 2019 16:32:28 +1200 Subject: [PATCH 2/2] Add tests and move logic into the if statement --- src/Pages/SubsitesVirtualPage.php | 10 +++----- tests/php/SubsitesVirtualPageTest.php | 37 +++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/src/Pages/SubsitesVirtualPage.php b/src/Pages/SubsitesVirtualPage.php index d5ab24e..d36cb4d 100644 --- a/src/Pages/SubsitesVirtualPage.php +++ b/src/Pages/SubsitesVirtualPage.php @@ -222,15 +222,13 @@ class SubsitesVirtualPage extends VirtualPage { $isValid = parent::validURLSegment(); - $filters = [ - 'URLSegment' => $this->URLSegment, - ]; - // Veto the validation rules if its false. In this case, some logic // needs to be duplicated from parent to find out the exact reason the validation failed. if (!$isValid) { - // Exclude the current page from the filter - $filters['ID:not'] = $this->ID; + $filters = [ + 'URLSegment' => $this->URLSegment, + 'ID:not' => $this->ID, + ]; if (Config::inst()->get(SiteTree::class, 'nested_urls')) { $filters['ParentID'] = $this->ParentID ?: 0; diff --git a/tests/php/SubsitesVirtualPageTest.php b/tests/php/SubsitesVirtualPageTest.php index 99e9646..527927f 100644 --- a/tests/php/SubsitesVirtualPageTest.php +++ b/tests/php/SubsitesVirtualPageTest.php @@ -311,4 +311,41 @@ class SubsitesVirtualPageTest extends BaseSubsiteTest Versioned::prepopulate_versionnumber_cache(SiteTree::class, 'Live', [$p->ID]); } } + + public function testValidURLSegmentWithUniquePageAndNestedURLs() + { + SiteTree::config()->set('nested_urls', true); + + $newPage = new SubsitesVirtualPage(); + $newPage->Title = 'My new page'; + $newPage->URLSegment = 'my-new-page'; + + $this->assertTrue($newPage->validURLSegment()); + } + + public function testValidURLSegmentWithExistingPageInSubsite() + { + $subsite1 = $this->objFromFixture(Subsite::class, 'subsite1'); + Subsite::changeSubsite($subsite1->ID); + + SiteTree::config()->set('nested_urls', false); + + $similarContactUsPage = new SubsitesVirtualPage(); + $similarContactUsPage->Title = 'Similar to Contact Us in Subsite 1'; + $similarContactUsPage->URLSegment = 'contact-us'; + + $this->assertFalse($similarContactUsPage->validURLSegment()); + } + + public function testValidURLSegmentWithExistingPageInAnotherSubsite() + { + $subsite1 = $this->objFromFixture(Subsite::class, 'subsite1'); + Subsite::changeSubsite($subsite1->ID); + + $similarStaffPage = new SubsitesVirtualPage(); + $similarStaffPage->Title = 'Similar to Staff page in main site'; + $similarStaffPage->URLSegment = 'staff'; + + $this->assertFalse($similarStaffPage->validURLSegment()); + } }