From 1e44e1d4bad737f310af7e51a95ac0c91f69072c Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Fri, 31 May 2019 09:46:24 +1200 Subject: [PATCH 01/10] FIX Domains now default to "Automatic" protocol, and have the correct help description --- lang/en.yml | 3 ++- src/Model/SubsiteDomain.php | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lang/en.yml b/lang/en.yml index 6852671..87f27e5 100644 --- a/lang/en.yml +++ b/lang/en.yml @@ -47,13 +47,14 @@ en: SilverStripe\Subsites\Model\SubsiteDomain: DOMAIN: Domain DOMAIN_DESCRIPTION: 'Hostname of this subsite (exclude protocol). Allows wildcards (*).' + ISPRIMARY_DESCRIPTION: 'Mark this as the default domain for this subsite' IS_PRIMARY: 'Is Primary Domain?' PLURALNAME: 'Subsite Domains' PLURALS: one: 'A Subsite Domain' other: '{count} Subsite Domains' PROTOCOL_AUTOMATIC: Automatic - PROTOCOL_DESCRIPTION: 'Mark this as the default domain for this subsite' + PROTOCOL_DESCRIPTION: 'When generating links to this subsite, use the selected protocol.
Selecting ''Automatic'' means subsite links will default to the current protocol.' PROTOCOL_HTTP: 'http://' PROTOCOL_HTTPS: 'https://' Protocol: Protocol diff --git a/src/Model/SubsiteDomain.php b/src/Model/SubsiteDomain.php index 66432e9..83f33b5 100644 --- a/src/Model/SubsiteDomain.php +++ b/src/Model/SubsiteDomain.php @@ -111,13 +111,14 @@ class SubsiteDomain extends DataObject self::PROTOCOL_HTTPS => _t(__CLASS__ . '.PROTOCOL_HTTPS', 'https://'), self::PROTOCOL_AUTOMATIC => _t(__CLASS__ . '.PROTOCOL_AUTOMATIC', 'Automatic') ]; - $fields = new FieldList( + $fields = FieldList::create( WildcardDomainField::create('Domain', $this->fieldLabel('Domain'), null, 255) ->setDescription(_t( __CLASS__ . '.DOMAIN_DESCRIPTION', 'Hostname of this subsite (exclude protocol). Allows wildcards (*).' )), OptionsetField::create('Protocol', $this->fieldLabel('Protocol'), $protocols) + ->setValue($this->Protocol ?: self::PROTOCOL_AUTOMATIC) ->setDescription(_t( __CLASS__ . '.PROTOCOL_DESCRIPTION', 'When generating links to this subsite, use the selected protocol.
' . @@ -125,7 +126,7 @@ class SubsiteDomain extends DataObject )), CheckboxField::create('IsPrimary', $this->fieldLabel('IsPrimary')) ->setDescription(_t( - __CLASS__ . '.PROTOCOL_DESCRIPTION', + __CLASS__ . '.ISPRIMARY_DESCRIPTION', 'Mark this as the default domain for this subsite' )) ); From 68c763da3e664393b8d3bcb47b41860c6b289fe4 Mon Sep 17 00:00:00 2001 From: Garion Herman Date: Fri, 31 May 2019 11:10:23 +1200 Subject: [PATCH 02/10] Tidy output of IsPublic value in Subsites admin --- src/Model/Subsite.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Model/Subsite.php b/src/Model/Subsite.php index c12721b..9f88643 100644 --- a/src/Model/Subsite.php +++ b/src/Model/Subsite.php @@ -113,7 +113,7 @@ class Subsite extends DataObject private static $summary_fields = [ 'Title', 'PrimaryDomain', - 'IsPublic' + 'IsPublic.Nice' ]; /** @@ -772,7 +772,7 @@ class Subsite extends DataObject $labels['DefaultSite'] = _t('Subsites.DefaultSiteFieldLabel', 'Default site'); $labels['Theme'] = _t('Subsites.ThemeFieldLabel', 'Theme'); $labels['Language'] = _t('Subsites.LanguageFieldLabel', 'Language'); - $labels['IsPublic'] = _t('Subsites.IsPublicFieldLabel', 'Enable public access'); + $labels['IsPublic.Nice'] = _t('Subsites.IsPublicFieldLabel', 'Enable public access'); $labels['PageTypeBlacklist'] = _t('Subsites.PageTypeBlacklistFieldLabel', 'Page Type Blacklist'); $labels['Domains.Domain'] = _t('Subsites.DomainFieldLabel', 'Domain'); $labels['PrimaryDomain'] = _t('Subsites.PrimaryDomainFieldLabel', 'Primary Domain'); From 3b8207d70cb98c42f1fdf204e815a0a078fdaa5e Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Fri, 31 May 2019 10:28:36 +1200 Subject: [PATCH 03/10] Ensure URL segment field type before using its API, and add docs around subsite and fluent domain compatibility --- docs/en/introduction.md | 11 +++++++++++ src/Extensions/SiteTreeSubsites.php | 4 +++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/docs/en/introduction.md b/docs/en/introduction.md index 24eabff..e9dba82 100644 --- a/docs/en/introduction.md +++ b/docs/en/introduction.md @@ -48,3 +48,14 @@ to speak to your website administrator or hosting provider to facilitate this. You can simulate subsite access without setting up virtual hosts by appending ?SubsiteID= to the request. + +### How do Subsite domains work with Fluent domains? + +The Subsites module and Fluent translation module both provide the concept of defining "domains" and let you +configure the host name for it. This functionality is essentially performing the same duty in both modules. + +In the "URL segment" field for CMS pages both Subsites and Fluent will add their context to the value. If you +have a Subsite domain configured but no Fluent domain, Fluent will respect the existing domain and add its +locale context to the value. If you have a Subsite domain configured and a Fluent domain configure, Fluent will +use its own domain host name value, and the Subsite domain value will be lost. For this reason, you will need +to ensure that you use the same host name in both Subsite and Fluent domain entries. diff --git a/src/Extensions/SiteTreeSubsites.php b/src/Extensions/SiteTreeSubsites.php index 728feca..ed79f98 100644 --- a/src/Extensions/SiteTreeSubsites.php +++ b/src/Extensions/SiteTreeSubsites.php @@ -3,6 +3,7 @@ namespace SilverStripe\Subsites\Extensions; use Page; +use SilverStripe\CMS\Forms\SiteTreeURLSegmentField; use SilverStripe\CMS\Model\SiteTree; use SilverStripe\Control\Controller; use SilverStripe\Control\Director; @@ -150,6 +151,7 @@ class SiteTreeSubsites extends DataExtension // replace readonly link prefix $subsite = $this->owner->Subsite(); $nested_urls_enabled = Config::inst()->get(SiteTree::class, 'nested_urls'); + /** @var Subsite $subsite */ if ($subsite && $subsite->exists()) { // Use baseurl from domain $baseLink = $subsite->absoluteBaseURL(); @@ -163,7 +165,7 @@ class SiteTreeSubsites extends DataExtension } $urlsegment = $fields->dataFieldByName('URLSegment'); - if ($urlsegment) { + if ($urlsegment && $urlsegment instanceof SiteTreeURLSegmentField) { $urlsegment->setURLPrefix($baseLink); } } From 4d7641e16a50e1c4184710f79a08e70af426f0e5 Mon Sep 17 00:00:00 2001 From: Garion Herman Date: Fri, 31 May 2019 10:40:22 +1200 Subject: [PATCH 04/10] FIX allowed pagetypes displaying incorrectly when switching subsite This patch depends on an update to the CMS module that provides this extension point. The code is inert when matched with existing CMS versions. --- _config/extensions.yml | 1 + src/Extensions/HintsCacheKeyExtension.php | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 src/Extensions/HintsCacheKeyExtension.php diff --git a/_config/extensions.yml b/_config/extensions.yml index 1c90a55..32731f5 100644 --- a/_config/extensions.yml +++ b/_config/extensions.yml @@ -50,6 +50,7 @@ SilverStripe\Admin\SecurityAdmin: SilverStripe\CMS\Controllers\CMSMain: extensions: + - SilverStripe\Subsites\Extensions\HintsCacheKeyExtension - SilverStripe\Subsites\Extensions\SubsiteMenuExtension SilverStripe\CMS\Controllers\CMSPagesController: diff --git a/src/Extensions/HintsCacheKeyExtension.php b/src/Extensions/HintsCacheKeyExtension.php new file mode 100644 index 0000000..74fb594 --- /dev/null +++ b/src/Extensions/HintsCacheKeyExtension.php @@ -0,0 +1,22 @@ +getSubsiteId(); + } +} From c60acb31906af5efc8182ff55c98b82f59a184d9 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Fri, 31 May 2019 11:04:41 +1200 Subject: [PATCH 05/10] FIX Field labels for subsites virtual pages are no longer repeated --- src/Pages/SubsitesVirtualPage.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Pages/SubsitesVirtualPage.php b/src/Pages/SubsitesVirtualPage.php index b4c3d4f..56abb60 100644 --- a/src/Pages/SubsitesVirtualPage.php +++ b/src/Pages/SubsitesVirtualPage.php @@ -113,7 +113,7 @@ class SubsitesVirtualPage extends VirtualPage 'Root.Main', TextareaField::create( 'CustomMetaKeywords', - $this->fieldLabel('CustomMetaTitle') + $this->fieldLabel('CustomMetaKeywords') )->setDescription(_t(__CLASS__ . '.OverrideNote', 'Overrides inherited value from the source')), 'MetaKeywords' ); @@ -121,7 +121,7 @@ class SubsitesVirtualPage extends VirtualPage 'Root.Main', TextareaField::create( 'CustomMetaDescription', - $this->fieldLabel('CustomMetaTitle') + $this->fieldLabel('CustomMetaDescription') )->setDescription(_t(__CLASS__ . '.OverrideNote', 'Overrides inherited value from the source')), 'MetaDescription' ); @@ -129,7 +129,7 @@ class SubsitesVirtualPage extends VirtualPage 'Root.Main', TextField::create( 'CustomExtraMeta', - $this->fieldLabel('CustomMetaTitle') + $this->fieldLabel('CustomExtraMeta') )->setDescription(_t(__CLASS__ . '.OverrideNote', 'Overrides inherited value from the source')), 'ExtraMeta' ); From 1f51fcd90944ea1587c5c98acf00c71b70e47795 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Fri, 31 May 2019 11:24:12 +1200 Subject: [PATCH 06/10] 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 07/10] 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()); + } } From 2b268765965568f929b04ee98c857a2a06678a98 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Fri, 31 May 2019 16:41:36 +1200 Subject: [PATCH 08/10] Add test for URLSegment prefix set to primary subsite domain for page --- tests/php/SiteTreeSubsitesTest.php | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/tests/php/SiteTreeSubsitesTest.php b/tests/php/SiteTreeSubsitesTest.php index d165336..dc54afa 100644 --- a/tests/php/SiteTreeSubsitesTest.php +++ b/tests/php/SiteTreeSubsitesTest.php @@ -5,10 +5,10 @@ namespace SilverStripe\Subsites\Tests; use Page; use SilverStripe\CMS\Controllers\CMSMain; use SilverStripe\CMS\Controllers\ModelAsController; +use SilverStripe\CMS\Forms\SiteTreeURLSegmentField; use SilverStripe\CMS\Model\SiteTree; use SilverStripe\Control\Director; use SilverStripe\Core\Config\Config; -use SilverStripe\Core\Convert; use SilverStripe\ErrorPage\ErrorPage; use SilverStripe\Forms\FieldList; use SilverStripe\Security\Member; @@ -21,6 +21,7 @@ use SilverStripe\Subsites\Tests\SiteTreeSubsitesTest\TestClassB; use SilverStripe\Subsites\Tests\SiteTreeSubsitesTest\TestErrorPage; use SilverStripe\Versioned\Versioned; use SilverStripe\View\SSViewer; +use TractorCow\Fluent\Extension\FluentSiteTreeExtension; class SiteTreeSubsitesTest extends BaseSubsiteTest { @@ -33,7 +34,9 @@ class SiteTreeSubsitesTest extends BaseSubsiteTest ]; protected static $illegal_extensions = [ - SiteTree::class => ['Translatable'] // @todo implement Translatable namespace + SiteTree::class => [ + FluentSiteTreeExtension::class, + ], ]; protected function setUp() @@ -449,7 +452,7 @@ class SiteTreeSubsitesTest extends BaseSubsiteTest /** * @dataProvider provideAlternateAbsoluteLink - * @param name $pageFixtureName + * @param string $pageFixtureName * @param string|null $action * @param string $expectedAbsoluteLink */ @@ -465,4 +468,23 @@ class SiteTreeSubsitesTest extends BaseSubsiteTest $this->assertEquals($expectedAbsoluteLink, $result); } + + public function testURLSegmentBaseIsSetToSubsiteBaseURL() + { + // This subsite has a domain with 'one.example.org' as the primary domain + /** @var Subsite $subsite */ + $subsite = $this->objFromFixture(Subsite::class, 'domaintest1'); + Subsite::changeSubsite($subsite); + + $page = new SiteTree(); + $page->SubsiteID = $subsite->ID; + $page->write(); + $fields = $page->getCMSFields(); + + /** @var SiteTreeURLSegmentField $urlSegmentField */ + $urlSegmentField = $fields->dataFieldByName('URLSegment'); + $this->assertInstanceOf(SiteTreeURLSegmentField::class, $urlSegmentField); + + $this->assertSame('http://one.example.org/', $urlSegmentField->getURLPrefix()); + } } From f6503822e8bd55edeb444bf9e988308163f3ec7a Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Wed, 5 Jun 2019 15:09:57 +1200 Subject: [PATCH 09/10] DOCS Fix typos [ci skip] Co-Authored-By: Garion Herman --- docs/en/introduction.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/en/introduction.md b/docs/en/introduction.md index e9dba82..66bf7e6 100644 --- a/docs/en/introduction.md +++ b/docs/en/introduction.md @@ -54,8 +54,8 @@ You can simulate subsite access without setting up virtual hosts by appending ?S The Subsites module and Fluent translation module both provide the concept of defining "domains" and let you configure the host name for it. This functionality is essentially performing the same duty in both modules. -In the "URL segment" field for CMS pages both Subsites and Fluent will add their context to the value. If you +In the "URL segment" field for CMS pages, both Subsites and Fluent will add their context to the value. If you have a Subsite domain configured but no Fluent domain, Fluent will respect the existing domain and add its -locale context to the value. If you have a Subsite domain configured and a Fluent domain configure, Fluent will +locale context to the value. If you have a Subsite domain configured and a Fluent domain configured, Fluent will use its own domain host name value, and the Subsite domain value will be lost. For this reason, you will need to ensure that you use the same host name in both Subsite and Fluent domain entries. From 4249fffc0fe98384060f562e29884960e2a8e0cb Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Mon, 24 Jun 2019 10:19:58 +1200 Subject: [PATCH 10/10] FIX LeftAndMainSubsites::canAccess() now accepts a Member argument and falls back to the session member --- src/Extensions/LeftAndMainSubsites.php | 15 +++++++++++---- tests/php/LeftAndMainSubsitesTest.php | 11 +++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/Extensions/LeftAndMainSubsites.php b/src/Extensions/LeftAndMainSubsites.php index a3dee3d..877e30e 100644 --- a/src/Extensions/LeftAndMainSubsites.php +++ b/src/Extensions/LeftAndMainSubsites.php @@ -215,11 +215,16 @@ class LeftAndMainSubsites extends LeftAndMainExtension /** * Check if the current controller is accessible for this user on this subsite. + * + * @param Member $member */ - public function canAccess() + public function canAccess(Member $member = null) { + if (!$member) { + $member = Security::getCurrentUser(); + } + // Admin can access everything, no point in checking. - $member = Security::getCurrentUser(); if ($member && (Permission::checkMember($member, [ 'ADMIN', // Full administrative rights @@ -238,10 +243,12 @@ class LeftAndMainSubsites extends LeftAndMainExtension /** * Prevent accessing disallowed resources. This happens after onBeforeInit has executed, * so all redirections should've already taken place. + * + * @param Member $member */ - public function alternateAccessCheck() + public function alternateAccessCheck(Member $member = null) { - return $this->owner->canAccess(); + return $this->owner->canAccess($member); } /** diff --git a/tests/php/LeftAndMainSubsitesTest.php b/tests/php/LeftAndMainSubsitesTest.php index 09df18f..ea60db5 100644 --- a/tests/php/LeftAndMainSubsitesTest.php +++ b/tests/php/LeftAndMainSubsitesTest.php @@ -9,6 +9,7 @@ use SilverStripe\CMS\Controllers\CMSPageEditController; use SilverStripe\Core\Config\Config; use SilverStripe\Dev\FunctionalTest; use SilverStripe\Security\Member; +use SilverStripe\Subsites\Extensions\LeftAndMainSubsites; use SilverStripe\Subsites\Model\Subsite; use SilverStripe\Subsites\State\SubsiteState; @@ -100,4 +101,14 @@ class LeftAndMainSubsitesTest extends FunctionalTest $this->assertTrue($l->shouldChangeSubsite(CMSPageEditController::class, 1, 5)); $this->assertFalse($l->shouldChangeSubsite(CMSPageEditController::class, 1, 1)); } + + public function testCanAccessWithPassedMember() + { + $memberID = $this->logInWithPermission('ADMIN'); + $member = Member::get()->byID($memberID); + + /** @var LeftAndMain&LeftAndMainSubsites $leftAndMain */ + $leftAndMain = new LeftAndMain(); + $this->assertTrue($leftAndMain->canAccess($member)); + } }