diff --git a/lang/en.yml b/lang/en.yml index 87f27e5..37709dc 100644 --- a/lang/en.yml +++ b/lang/en.yml @@ -70,6 +70,8 @@ en: SubsiteField: Subsite SilverStripe\Subsites\Reports\SubsiteReportWrapper: ReportDropdown: Sites + ReportDropdownSubsite: Subsite + ReportDropdownAll: All Subsite: COPYSTRUCTURE: 'Copy structure from:' NOTEMPLATE: 'No template' diff --git a/src/Extensions/SiteTreeSubsites.php b/src/Extensions/SiteTreeSubsites.php index 15f50ae..dd4dc07 100644 --- a/src/Extensions/SiteTreeSubsites.php +++ b/src/Extensions/SiteTreeSubsites.php @@ -2,7 +2,6 @@ namespace SilverStripe\Subsites\Extensions; -use SilverStripe\Dev\Deprecation; use Page; use SilverStripe\CMS\Forms\SiteTreeURLSegmentField; use SilverStripe\CMS\Model\SiteTree; @@ -10,7 +9,6 @@ use SilverStripe\Control\Controller; use SilverStripe\Control\Director; use SilverStripe\Control\HTTP; use SilverStripe\Core\Config\Config; -use SilverStripe\Core\Convert; use SilverStripe\Forms\CheckboxField; use SilverStripe\Forms\DropdownField; use SilverStripe\Forms\FieldList; @@ -76,13 +74,7 @@ class SiteTreeSubsites extends DataExtension return; } - $subsiteID = null; - if (Subsite::$force_subsite) { - $subsiteID = Subsite::$force_subsite; - } else { - $subsiteID = SubsiteState::singleton()->getSubsiteId(); - } - + $subsiteID = SubsiteState::singleton()->getSubsiteId(); if ($subsiteID === null) { return; } @@ -423,7 +415,7 @@ class SiteTreeSubsites extends DataExtension // This helps deal with Link() returning an absolute URL. $url = Director::absoluteURL($this->owner->Link($action)); if ($this->owner->SubsiteID) { - $url = preg_replace('/\/\/[^\/]+\//', '//' . $this->owner->Subsite()->domain() . '/', $url ?? ''); + $url = preg_replace('#//[^/]+#', '//' . $this->owner->Subsite()->domain(), $url); } return $url; } @@ -444,22 +436,6 @@ class SiteTreeSubsites extends DataExtension return $link; } - /** - * This function is marked as deprecated for removal in 5.0.0 in silverstripe/cms - * so now simply passes execution to where the functionality exists for backwards compatiblity. - * CMS 4.0.0 SiteTree already throws a SilverStripe deprecation error before calling this function. - * @deprecated 2.2.0 Use updatePreviewLink() instead - * - * @param string|null $action - * @return string - */ - public function alternatePreviewLink($action = null) - { - Deprecation::notice('2.2.0', 'Use updatePreviewLink() instead'); - $link = ''; - return $this->updatePreviewLink($link, $action); - } - /** * Inject the subsite ID into the content so it can be used by frontend scripts. * @param $tags @@ -525,22 +501,14 @@ class SiteTreeSubsites extends DataExtension public function augmentValidURLSegment() { // If this page is being filtered in the current subsite, then no custom validation query is required. - $subsite = Subsite::$force_subsite ?: SubsiteState::singleton()->getSubsiteId(); - if (empty($this->owner->SubsiteID) || $subsite == $this->owner->SubsiteID) { + $subsiteID = SubsiteState::singleton()->getSubsiteId(); + if (empty($this->owner->SubsiteID) || $subsiteID == $this->owner->SubsiteID) { return null; } - - // Backup forced subsite - $prevForceSubsite = Subsite::$force_subsite; - Subsite::$force_subsite = $this->owner->SubsiteID; - - // Repeat validation in the correct subsite - $isValid = $this->owner->validURLSegment(); - - // Restore - Subsite::$force_subsite = $prevForceSubsite; - - return (bool)$isValid; + return SubsiteState::singleton()->withState(function (SubsiteState $newState) { + $newState->setSubsiteId($this->owner->SubsiteID); + return (bool) $this->owner->validURLSegment(); + }); } /** diff --git a/src/Model/Subsite.php b/src/Model/Subsite.php index 4e0042d..d32b0b4 100644 --- a/src/Model/Subsite.php +++ b/src/Model/Subsite.php @@ -52,14 +52,6 @@ class Subsite extends DataObject */ public static $disable_subsite_filter = false; - /** - * Allows you to force a specific subsite ID, or comma separated list of IDs. - * Only works for reading. An object cannot be written to more than 1 subsite. - * - * @deprecated 2.0.0 Use SubsiteState::singleton()->withState() instead. - */ - public static $force_subsite = null; - /** * Whether to write a host-map.php file * @@ -191,24 +183,6 @@ class Subsite extends DataObject return Subsite::get()->byID(SubsiteState::singleton()->getSubsiteId()); } - /** - * This function gets the current subsite ID from the session. It used in the backend so Ajax requests - * use the correct subsite. The frontend handles subsites differently. It calls getSubsiteIDForDomain - * directly from ModelAsController::getNestedController. Only gets Subsite instances which have their - * {@link IsPublic} flag set to TRUE. - * - * You can simulate subsite access without creating virtual hosts by appending ?SubsiteID= to the request. - * - * @return int ID of the current subsite instance - * - * @deprecated 2.0.0 Use SubsiteState::singleton()->getSubsiteId() instead - */ - public static function currentSubsiteID() - { - Deprecation::notice('2.0.0', 'Use SubsiteState::singleton()->getSubsiteId() instead'); - return SubsiteState::singleton()->getSubsiteId(); - } - /** * Switch to another subsite through storing the subsite identifier in the current PHP session. * Only takes effect when {@link SubsiteState::singleton()->getUseSessions()} is set to TRUE. @@ -218,7 +192,7 @@ class Subsite extends DataObject public static function changeSubsite($subsite) { // Session subsite change only meaningful if the session is active. - // Otherwise we risk setting it to wrong value, e.g. if we rely on currentSubsiteID. + // Otherwise we risk setting it to wrong value if (!SubsiteState::singleton()->getUseSessions()) { return; } @@ -352,6 +326,15 @@ class Subsite extends DataObject self::$disable_subsite_filter = $disabled; } + public static function withDisabledSubsiteFilter(callable $callable, bool $disabled = true): mixed + { + $orig = self::$disable_subsite_filter; + self::disable_subsite_filter($disabled); + $ret = $callable(); + self::disable_subsite_filter($orig); + return $ret; + } + /** * Flush caches on database reset */ diff --git a/src/Reports/SubsiteReportWrapper.php b/src/Reports/SubsiteReportWrapper.php index fb09bea..dbfc1e6 100644 --- a/src/Reports/SubsiteReportWrapper.php +++ b/src/Reports/SubsiteReportWrapper.php @@ -2,10 +2,11 @@ namespace SilverStripe\Subsites\Reports; +use SilverStripe\Forms\DropdownField; use SilverStripe\Forms\FieldList; -use SilverStripe\Forms\TreeMultiselectField; use SilverStripe\Reports\ReportWrapper; use SilverStripe\Subsites\Model\Subsite; +use SilverStripe\Subsites\State\SubsiteState; /** * Creates a subsite-aware version of another report. @@ -13,6 +14,7 @@ use SilverStripe\Subsites\Model\Subsite; */ class SubsiteReportWrapper extends ReportWrapper { + private const SUBSITE_ID_ALL = -1; /** * @return FieldList @@ -20,23 +22,22 @@ class SubsiteReportWrapper extends ReportWrapper public function parameterFields() { $subsites = Subsite::accessible_sites('CMS_ACCESS_CMSMain', true); - $options = $subsites->toDropdownMap('ID', 'Title'); + $options = [self::SUBSITE_ID_ALL => _t(__CLASS__ . '.ReportDropdownAll', 'All')] + $subsites->map()->toArray(); - $subsiteField = TreeMultiselectField::create( - 'Subsites', - _t(__CLASS__ . '.ReportDropdown', 'Sites'), + $subsiteField = DropdownField::create( + 'Subsite', + _t(__CLASS__ . '.ReportDropdownSubsite', 'Subsite'), $options ); - $subsiteField->setValue(array_keys($options ?? [])); // We don't need to make the field editable if only one subsite is available - if (sizeof($options ?? []) <= 1) { + if (sizeof($options ?? []) <= 2) { $subsiteField = $subsiteField->performReadonlyTransformation(); } $fields = parent::parameterFields(); if ($fields) { - $fields->insertBefore($fields->First()->Name(), $subsiteField); + $fields->insertBefore($fields->First()->getName(), $subsiteField); } else { $fields = FieldList::create($subsiteField); } @@ -49,37 +50,40 @@ class SubsiteReportWrapper extends ReportWrapper public function columns() { $columns = parent::columns(); - $columns['Subsite.Title'] = Subsite::class; + $columns['Subsite.Title'] = _t(__CLASS__ . '.ReportDropdownSubsite', 'Subsite'); return $columns; } - /////////////////////////////////////////////////////////////////////////////////////////// - // Querying - - /** - * @param arary $params - * @return void - */ - public function beforeQuery($params) + public function sourceQuery($params) { - // The user has select a few specific sites - if (!empty($params['Subsites'])) { - Subsite::$force_subsite = $params['Subsites']; - - // Default: restrict to all accessible sites - } else { - $subsites = Subsite::accessible_sites('CMS_ACCESS_CMSMain'); - $options = $subsites->toDropdownMap('ID', 'Title'); - Subsite::$force_subsite = join(',', array_keys($options ?? [])); + $subsiteID = (int) ($params['Subsite'] ?? self::SUBSITE_ID_ALL); + if ($subsiteID === self::SUBSITE_ID_ALL) { + return Subsite::withDisabledSubsiteFilter(function () use ($params) { + return parent::sourceQuery($params); + }); } + return SubsiteState::singleton()->withState(function (SubsiteState $newState) use ($subsiteID, $params) { + $newState->setSubsiteId($subsiteID); + return parent::sourceQuery($params); + }); } - /** - * @return void - */ - public function afterQuery() + public function sourceRecords($params = [], $sort = null, $limit = null) { - // Manually manage the subsite filtering - Subsite::$force_subsite = null; + $subsiteID = (int) ($params['Subsite'] ?? self::SUBSITE_ID_ALL); + if ($subsiteID === self::SUBSITE_ID_ALL) { + return Subsite::withDisabledSubsiteFilter(function () use ($params, $sort, $limit) { + return parent::sourceRecords($params, $sort, $limit); + }); + } + return SubsiteState::singleton()->withState(function (SubsiteState $newState) use ( + $subsiteID, + $params, + $sort, + $limit + ) { + $newState->setSubsiteId($subsiteID); + return parent::sourceRecords($params, $sort, $limit); + }); } } diff --git a/src/State/SubsiteState.php b/src/State/SubsiteState.php index 8bd5b0c..fea496d 100644 --- a/src/State/SubsiteState.php +++ b/src/State/SubsiteState.php @@ -13,28 +13,16 @@ class SubsiteState implements Resettable { use Injectable; - /** - * @var int|null - */ - protected $subsiteId; + protected ?int $subsiteId = null; + protected ?int $originalSubsiteId = null; - /** - * @var int|null - */ - protected $originalSubsiteId; - - /** - * @var bool - */ - protected $useSessions; + protected ?bool $useSessions = null; /** * Get the current subsite ID - * - * @return int|null */ - public function getSubsiteId() + public function getSubsiteId(): ?int { return $this->subsiteId; } @@ -42,11 +30,8 @@ class SubsiteState implements Resettable /** * Set the current subsite ID, and track the first subsite ID set as the "original". This is used to check * whether the ID has been changed through a request. - * - * @param int $id - * @return $this */ - public function setSubsiteId($id) + public function setSubsiteId(?int $id): static { if (is_null($this->originalSubsiteId)) { $this->originalSubsiteId = (int) $id; @@ -59,21 +44,16 @@ class SubsiteState implements Resettable /** * Get whether to use sessions for storing the subsite ID - * - * @return bool */ - public function getUseSessions() + public function getUseSessions(): ?bool { return $this->useSessions; } /** * Set whether to use sessions for storing the subsite ID - * - * @param bool $useSessions - * @return $this */ - public function setUseSessions($useSessions) + public function setUseSessions(?bool $useSessions): static { $this->useSessions = $useSessions; @@ -82,10 +62,8 @@ class SubsiteState implements Resettable /** * Get whether the subsite ID has been changed during a request, based on the original and current IDs - * - * @return bool */ - public function getSubsiteIdWasChanged() + public function getSubsiteIdWasChanged(): bool { return $this->originalSubsiteId !== $this->getSubsiteId(); } @@ -97,7 +75,7 @@ class SubsiteState implements Resettable * @param callable $callback Callback to run. Will be passed the nested state as a parameter * @return mixed Result of callback */ - public function withState(callable $callback) + public function withState(callable $callback): mixed { $newState = clone $this; try { @@ -111,7 +89,7 @@ class SubsiteState implements Resettable /** * Reset the local cache of the singleton */ - public static function reset() + public static function reset(): void { SubsiteState::singleton()->resetState(); } @@ -119,7 +97,7 @@ class SubsiteState implements Resettable /** * Reset the local cache of this object */ - public function resetState() + public function resetState(): void { $this->originalSubsiteId = null; $this->subsiteId = null; diff --git a/tests/php/BaseSubsiteTest.php b/tests/php/BaseSubsiteTest.php index ca29c68..123a1a5 100644 --- a/tests/php/BaseSubsiteTest.php +++ b/tests/php/BaseSubsiteTest.php @@ -15,7 +15,6 @@ class BaseSubsiteTest extends SapphireTest SubsiteState::singleton()->setUseSessions(true); Config::modify()->set(Subsite::class, 'write_hostmap', false); - Subsite::$force_subsite = null; } /** diff --git a/tests/php/SiteTreeSubsitesTest.php b/tests/php/SiteTreeSubsitesTest.php index ef8f938..20f36a8 100644 --- a/tests/php/SiteTreeSubsitesTest.php +++ b/tests/php/SiteTreeSubsitesTest.php @@ -275,8 +275,8 @@ class SiteTreeSubsitesTest extends BaseSubsiteTest // Staff is shifted to top level and given a unique url segment $domain = $otherSubsite->domain(); - $this->assertEquals('http://' . $domain . '/staff-2/', $staffPage2->AbsoluteLink()); - $this->assertEquals('http://' . $domain . '/contact-us-2/', $contactPage2->AbsoluteLink()); + $this->assertEquals('http://' . $domain . '/staff-2', $staffPage2->AbsoluteLink()); + $this->assertEquals('http://' . $domain . '/contact-us-2', $contactPage2->AbsoluteLink()); } public function testPageTypesBlacklistInCMSMain() @@ -443,13 +443,13 @@ class SiteTreeSubsitesTest extends BaseSubsiteTest public function provideAlternateAbsoluteLink() { return [ - ['home', null, 'http://localhost/'], + ['home', null, 'http://localhost'], ['home', 'myaction', 'http://localhost/home/myaction'], - ['contact', null, 'http://localhost/contact-us/'], + ['contact', null, 'http://localhost/contact-us'], ['contact', 'myaction', 'http://localhost/contact-us/myaction'], - ['subsite1_home', null, 'http://subsite1.localhost/'], + ['subsite1_home', null, 'http://subsite1.localhost'], ['subsite1_home', 'myaction', 'http://subsite1.localhost/home/myaction'], - ['subsite1_contactus', null, 'http://subsite1.localhost/contact-us/'], + ['subsite1_contactus', null, 'http://subsite1.localhost/contact-us'], ['subsite1_contactus', 'myaction', 'http://subsite1.localhost/contact-us/myaction'] ]; } @@ -463,7 +463,7 @@ class SiteTreeSubsitesTest extends BaseSubsiteTest public function testAlternateAbsoluteLink($pageFixtureName, $action, $expectedAbsoluteLink) { // Setting a control value, in case base url is set for the installation under test - Config::modify()->set(Director::class, 'alternate_base_url', 'http://localhost/'); + Config::modify()->set(Director::class, 'alternate_base_url', 'http://localhost'); /** @var Page $page */ $page = $this->objFromFixture(Page::class, $pageFixtureName); diff --git a/tests/php/SubsiteTest.php b/tests/php/SubsiteTest.php index bcd5085..d0e4dd5 100644 --- a/tests/php/SubsiteTest.php +++ b/tests/php/SubsiteTest.php @@ -301,8 +301,10 @@ class SubsiteTest extends BaseSubsiteTest ); $this->assertEquals($_SERVER['HTTP_HOST'], singleton(Subsite::class)->PrimaryDomain); + + // If there is a Director::baseURL() value this will also be included $this->assertEquals( - 'http://' . $_SERVER['HTTP_HOST'] . Director::baseURL(), + 'http://' . $_SERVER['HTTP_HOST'], singleton(Subsite::class)->absoluteBaseURL() ); } @@ -328,22 +330,22 @@ class SubsiteTest extends BaseSubsiteTest public function domainProtocolProvider() { return [ - [Subsite::class, 'domaintest2', false, 'http://two.mysite.com/'], - [SubsiteDomain::class, 'dt2a', false, 'http://two.mysite.com/'], - [SubsiteDomain::class, 'dt2b', false, 'http://subsite.mysite.com/'], - [Subsite::class, 'domaintest4', false, 'https://www.primary.com/'], - [SubsiteDomain::class, 'dt4a', false, 'https://www.primary.com/'], - [SubsiteDomain::class, 'dt4b', false, 'http://www.secondary.com/'], - [Subsite::class, 'domaintest5', false, 'http://www.tertiary.com/'], - [SubsiteDomain::class, 'dt5', false, 'http://www.tertiary.com/'], - [Subsite::class, 'domaintest2', true, 'https://two.mysite.com/'], - [SubsiteDomain::class, 'dt2a', true, 'https://two.mysite.com/'], - [SubsiteDomain::class, 'dt2b', true, 'https://subsite.mysite.com/'], - [Subsite::class, 'domaintest4', true, 'https://www.primary.com/'], - [SubsiteDomain::class, 'dt4a', true, 'https://www.primary.com/'], - [SubsiteDomain::class, 'dt4b', true, 'http://www.secondary.com/'], - [Subsite::class, 'domaintest5', true, 'http://www.tertiary.com/'], - [SubsiteDomain::class, 'dt5', true, 'http://www.tertiary.com/'], + [Subsite::class, 'domaintest2', false, 'http://two.mysite.com'], + [SubsiteDomain::class, 'dt2a', false, 'http://two.mysite.com'], + [SubsiteDomain::class, 'dt2b', false, 'http://subsite.mysite.com'], + [Subsite::class, 'domaintest4', false, 'https://www.primary.com'], + [SubsiteDomain::class, 'dt4a', false, 'https://www.primary.com'], + [SubsiteDomain::class, 'dt4b', false, 'http://www.secondary.com'], + [Subsite::class, 'domaintest5', false, 'http://www.tertiary.com'], + [SubsiteDomain::class, 'dt5', false, 'http://www.tertiary.com'], + [Subsite::class, 'domaintest2', true, 'https://two.mysite.com'], + [SubsiteDomain::class, 'dt2a', true, 'https://two.mysite.com'], + [SubsiteDomain::class, 'dt2b', true, 'https://subsite.mysite.com'], + [Subsite::class, 'domaintest4', true, 'https://www.primary.com'], + [SubsiteDomain::class, 'dt4a', true, 'https://www.primary.com'], + [SubsiteDomain::class, 'dt4b', true, 'http://www.secondary.com'], + [Subsite::class, 'domaintest5', true, 'http://www.tertiary.com'], + [SubsiteDomain::class, 'dt5', true, 'http://www.tertiary.com'], ]; }