From 7d27abf2b1bab1251d26e30342c99f0153403f8e Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Tue, 25 Jun 2019 16:05:38 +1200 Subject: [PATCH 1/6] Update expected json content type in unit test --- tests/php/SubsiteXHRControllerTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/php/SubsiteXHRControllerTest.php b/tests/php/SubsiteXHRControllerTest.php index ba6549d..671bb57 100644 --- a/tests/php/SubsiteXHRControllerTest.php +++ b/tests/php/SubsiteXHRControllerTest.php @@ -35,7 +35,7 @@ class SubsiteXHRControllerTest extends FunctionalTest ]); $this->assertEquals(200, $result->getStatusCode()); - $this->assertEquals('text/json', $result->getHeader('Content-Type')); + $this->assertEquals('application/json', $result->getHeader('Content-Type')); $body = $result->getBody(); $this->assertContains('Main site', $body); From 2eb04ffa7894e145a29c3cfea34d9d850515f014 Mon Sep 17 00:00:00 2001 From: Guy Marriott Date: Fri, 31 May 2019 16:18:52 +1200 Subject: [PATCH 2/6] FIX Improving support for cascading themes - Fixes an issue where themes would cascade "up" the list of themes - Provides configuration for defining custom theme options with their own sets of cascading themes Fixes #392 --- src/Extensions/SiteTreeSubsites.php | 4 +- src/Model/Subsite.php | 4 +- src/Service/ThemeResolver.php | 95 ++++++++++++++++ tests/php/Service/ThemeResolverTest.php | 143 ++++++++++++++++++++++++ tests/php/SiteTreeSubsitesTest.php | 37 ++---- 5 files changed, 255 insertions(+), 28 deletions(-) create mode 100644 src/Service/ThemeResolver.php create mode 100644 tests/php/Service/ThemeResolverTest.php diff --git a/src/Extensions/SiteTreeSubsites.php b/src/Extensions/SiteTreeSubsites.php index 68c5ca6..4d9308f 100644 --- a/src/Extensions/SiteTreeSubsites.php +++ b/src/Extensions/SiteTreeSubsites.php @@ -25,6 +25,7 @@ use SilverStripe\Security\Member; use SilverStripe\Security\Security; use SilverStripe\SiteConfig\SiteConfig; use SilverStripe\Subsites\Model\Subsite; +use SilverStripe\Subsites\Service\ThemeResolver; use SilverStripe\Subsites\State\SubsiteState; use SilverStripe\View\SSViewer; @@ -387,10 +388,11 @@ class SiteTreeSubsites extends DataExtension */ public static function contentcontrollerInit($controller) { + /** @var Subsite $subsite */ $subsite = Subsite::currentSubsite(); if ($subsite && $subsite->Theme) { - SSViewer::add_themes([$subsite->Theme]); + SSViewer::set_themes(ThemeResolver::singleton()->getThemeList($subsite)); } if ($subsite && i18n::getData()->validate($subsite->Language)) { diff --git a/src/Model/Subsite.php b/src/Model/Subsite.php index c12721b..174d9ba 100644 --- a/src/Model/Subsite.php +++ b/src/Model/Subsite.php @@ -28,6 +28,7 @@ use SilverStripe\Security\Group; use SilverStripe\Security\Member; use SilverStripe\Security\Permission; use SilverStripe\Security\Security; +use SilverStripe\Subsites\Service\ThemeResolver; use SilverStripe\Subsites\State\SubsiteState; use SilverStripe\Versioned\Versioned; use UnexpectedValueException; @@ -182,7 +183,6 @@ class Subsite extends DataObject /** * Gets the subsite currently set in the session. * - * @uses ControllerSubsites->controllerAugmentInit() * @return DataObject The current Subsite */ public static function currentSubsite() @@ -787,7 +787,7 @@ class Subsite extends DataObject */ public function allowedThemes() { - if ($themes = self::$allowed_themes) { + if (($themes = self::$allowed_themes) || ($themes = ThemeResolver::singleton()->getCustomThemeOptions())) { return ArrayLib::valuekey($themes); } diff --git a/src/Service/ThemeResolver.php b/src/Service/ThemeResolver.php new file mode 100644 index 0000000..98dd846 --- /dev/null +++ b/src/Service/ThemeResolver.php @@ -0,0 +1,95 @@ + [ + * '$public', + * 'starter', + * '$default', + * ], + * 'theme-2' => [ + * 'custom', + * 'watea', + * 'starter', + * '$public', + * '$default', + * ] + * ] + * + * @config + * @var null|array[] + */ + private static $theme_options; + + /** + * Get the list of themes for the given sub site that can be given to SSViewer::set_themes + * + * @param Subsite $site + * @return array + */ + public function getThemeList(Subsite $site) + { + $themes = array_values(SSViewer::get_themes()); + $siteTheme = $site->Theme; + + if (!$siteTheme) { + return $themes; + } + + $customOptions = $this->config()->get('theme_options'); + if ($customOptions && isset($customOptions[$siteTheme])) { + return $customOptions[$siteTheme]; + } + + // Ensure themes don't cascade "up" the list + $index = array_search($siteTheme, $themes); + + if ($index > 0) { + // Check if the default is public themes + $publicDefault = $themes[0] === SSViewer::PUBLIC_THEME; + + // Take only those that appear after theme chosen (non-inclusive) + $themes = array_slice($themes, $index + 1); + + // Add back in public + if ($publicDefault) { + array_unshift($themes, SSViewer::PUBLIC_THEME); + } + } + + // Add our theme + array_unshift($themes, $siteTheme); + + return $themes; + } + + /** + * Get a list of custom cascading theme definitions if available + * + * @return null|array + */ + public function getCustomThemeOptions() + { + $config = $this->config()->get('theme_options'); + + if (!$config) { + return null; + } + + return array_keys($config); + } +} diff --git a/tests/php/Service/ThemeResolverTest.php b/tests/php/Service/ThemeResolverTest.php new file mode 100644 index 0000000..9050c4a --- /dev/null +++ b/tests/php/Service/ThemeResolverTest.php @@ -0,0 +1,143 @@ +set(SSViewer::class, 'themes', $this->themeList); + } + + public function testSubsiteWithoutThemeReturnsDefaultThemeList() + { + $subsite = new Subsite(); + $resolver = new ThemeResolver(); + + $this->assertSame($this->themeList, $resolver->getThemeList($subsite)); + } + + public function testSubsiteWithCustomThemePrependsToList() + { + $subsite = new Subsite(); + $subsite->Theme = 'subsite'; + + $resolver = new ThemeResolver(); + + $expected = array_merge(['subsite'], $this->themeList); + + $this->assertSame($expected, $resolver->getThemeList($subsite)); + } + + public function testSubsiteWithCustomThemeDoesNotCascadeUpTheList() + { + $subsite = new Subsite(); + $subsite->Theme = 'main'; + + $resolver = new ThemeResolver(); + + $expected = [ + 'main', // 'main' is moved to the top + SSViewer::PUBLIC_THEME, // $public is preserved + // Anything above 'main' is removed + 'backup', + SSViewer::DEFAULT_THEME, + ]; + + $this->assertSame($expected, $resolver->getThemeList($subsite)); + } + + /** + * @dataProvider customThemeDefinitionsAreRespectedProvider + */ + public function testCustomThemeDefinitionsAreRespected($themeOptions, $siteTheme, $expected) + { + Config::modify()->set(ThemeResolver::class, 'theme_options', $themeOptions); + + $subsite = new Subsite(); + $subsite->Theme = $siteTheme; + + $resolver = new ThemeResolver(); + + $this->assertSame($expected, $resolver->getThemeList($subsite)); + } + + public function customThemeDefinitionsAreRespectedProvider() + { + return [ + // Simple + [ + ['test' => $expected = [ + 'subsite', + 'backup', + SSViewer::PUBLIC_THEME, + SSViewer::DEFAULT_THEME, + ]], + 'test', + $expected + ], + // Many options + [ + [ + 'aye' => [ + 'aye', + 'thing', + SSViewer::DEFAULT_THEME, + ], + 'bee' => $expected = [ + 'subsite', + 'backup', + SSViewer::PUBLIC_THEME, + SSViewer::DEFAULT_THEME, + ], + 'sea' => [ + 'mer', + 'ocean', + SSViewer::DEFAULT_THEME, + ], + ], + 'bee', + $expected + ], + // Conflicting with root definitions + [ + ['main' => $expected = [ + 'subsite', + 'backup', + SSViewer::PUBLIC_THEME, + SSViewer::DEFAULT_THEME, + ]], + 'main', + $expected + ], + // Declaring a theme specifically should still work + [ + ['test' => [ + 'subsite', + 'backup', + SSViewer::PUBLIC_THEME, + SSViewer::DEFAULT_THEME, + ]], + 'other', + array_merge(['other'], $this->themeList) + ], + ]; + } +} diff --git a/tests/php/SiteTreeSubsitesTest.php b/tests/php/SiteTreeSubsitesTest.php index d165336..f6a58f7 100644 --- a/tests/php/SiteTreeSubsitesTest.php +++ b/tests/php/SiteTreeSubsitesTest.php @@ -9,6 +9,7 @@ use SilverStripe\CMS\Model\SiteTree; use SilverStripe\Control\Director; use SilverStripe\Core\Config\Config; use SilverStripe\Core\Convert; +use SilverStripe\Core\Injector\Injector; use SilverStripe\ErrorPage\ErrorPage; use SilverStripe\Forms\FieldList; use SilverStripe\Security\Member; @@ -16,6 +17,7 @@ use SilverStripe\SiteConfig\SiteConfig; use SilverStripe\Subsites\Extensions\SiteTreeSubsites; use SilverStripe\Subsites\Model\Subsite; use SilverStripe\Subsites\Pages\SubsitesVirtualPage; +use SilverStripe\Subsites\Service\ThemeResolver; use SilverStripe\Subsites\Tests\SiteTreeSubsitesTest\TestClassA; use SilverStripe\Subsites\Tests\SiteTreeSubsitesTest\TestClassB; use SilverStripe\Subsites\Tests\SiteTreeSubsitesTest\TestErrorPage; @@ -396,41 +398,26 @@ class SiteTreeSubsitesTest extends BaseSubsiteTest ]; } - public function testIfSubsiteThemeIsSetToThemeList() + public function testThemeResolverIsUsedForSettingThemeList() { - $defaultThemes = ['default']; - SSViewer::set_themes($defaultThemes); + $firstResolver = $this->createMock(ThemeResolver::class); + $firstResolver->expects($this->never())->method('getThemeList'); + Injector::inst()->registerService($firstResolver, ThemeResolver::class); $subsitePage = $this->objFromFixture(Page::class, 'home'); Subsite::changeSubsite($subsitePage->SubsiteID); $controller = ModelAsController::controller_for($subsitePage); SiteTree::singleton()->extend('contentcontrollerInit', $controller); - $this->assertEquals( - SSViewer::get_themes(), - $defaultThemes, - 'Themes should not be modified when Subsite has no theme defined' - ); + $secondResolver = $this->createMock(ThemeResolver::class); + $secondResolver->expects($this->once())->method('getThemeList'); + Injector::inst()->registerService($secondResolver, ThemeResolver::class); - $pageWithTheme = $this->objFromFixture(Page::class, 'subsite1_home'); - Subsite::changeSubsite($pageWithTheme->SubsiteID); - $controller = ModelAsController::controller_for($pageWithTheme); + $subsitePage = $this->objFromFixture(Page::class, 'subsite1_home'); + Subsite::changeSubsite($subsitePage->SubsiteID); + $controller = ModelAsController::controller_for($subsitePage); SiteTree::singleton()->extend('contentcontrollerInit', $controller); - $subsiteTheme = $pageWithTheme->Subsite()->Theme; - $allThemes = SSViewer::get_themes(); - - $this->assertContains( - $subsiteTheme, - $allThemes, - 'Themes should be modified when Subsite has theme defined' - ); - - $this->assertEquals( - $subsiteTheme, - array_shift($allThemes), - 'Subsite theme should be prepeded to theme list' - ); } public function provideAlternateAbsoluteLink() From 9feef185dce259ca4f7a90a150c7e6bb49e3184f Mon Sep 17 00:00:00 2001 From: Guy Marriott Date: Fri, 31 May 2019 16:31:08 +1200 Subject: [PATCH 3/6] Adding documentation about cascading themes --- README.md | 50 ++++++++++++++++++++++++- src/Service/ThemeResolver.php | 5 ++- tests/php/Service/ThemeResolverTest.php | 12 +++--- tests/php/SiteTreeSubsitesTest.php | 1 - 4 files changed, 59 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index 9a910e2..7618002 100644 --- a/README.md +++ b/README.md @@ -96,7 +96,55 @@ In some Browsers the SubsiteID is visible if you hover over the "Edit" link in t ### Subsite-specific themes -Download a second theme from http://www.silverstripe.com/themes/ and put it in your themes folder. Open admin/subsites?flush=1 and select one of your subsites from the menu on the bottom-left. You should see a Theme dropdown in the subsite details, and it should list both your original theme and the new theme. Select the new theme in the dropdown. Now, this subsite will use a different theme from the main site. +Download a second theme from http://www.silverstripe.com/themes/ and put it in your themes folder. Open +admin/subsites?flush=1 and select one of your subsites from the menu on the bottom-left. You should see a +Theme dropdown in the subsite details, and it should list both your original theme and the new theme. Select the new +theme in the dropdown. Now, this subsite will use a different theme from the main site. + +#### Cascading themes + +In SilverStripe 4 themes will resolve theme files by looking through a list of themes (see the documentation on +[creating your own theme](https://docs.silverstripe.org/en/4/developer_guides/templates/themes/#developing-your-own-theme)). +Subsites will inherit this configuration for the order of themes. Choosing a theme for a Subsite will set the list of +themes to that chosen theme, and all themes that are defined below the chosen theme in priority. For example, with a +theme configuration as follows: + +```yaml +SilverStripe\View\SSViewer: + themes: + - '$public' + - 'my-theme' + - 'watea' + - 'starter' + - '$default' +``` + +Choosing `watea` in your Subsite will create a cascading config as follows: + +```yaml +themes: + - 'watea' + - '$public' + - 'starter' + - '$default' +``` + +You may also completely define your own cascading theme lists for CMS users to choose as theme options for their +subsite: + +```yaml +SilverStripe\Subsites\Service\ThemeResolver: + theme_options: + normal: + - '$public' + - 'watea' + - 'starter' + - '$default' + special: + - 'my-theme' + - 'starter' + - '$default' +``` ### Limit available themes for a subsite diff --git a/src/Service/ThemeResolver.php b/src/Service/ThemeResolver.php index 98dd846..c61c6cf 100644 --- a/src/Service/ThemeResolver.php +++ b/src/Service/ThemeResolver.php @@ -59,8 +59,11 @@ class ThemeResolver $index = array_search($siteTheme, $themes); if ($index > 0) { + // 4.0 didn't have support for themes in the public webroot + $publicConstantDefined = defined('SSViewer::PUBLIC_THEME'); + // Check if the default is public themes - $publicDefault = $themes[0] === SSViewer::PUBLIC_THEME; + $publicDefault = $publicConstantDefined && $themes[0] === SSViewer::PUBLIC_THEME; // Take only those that appear after theme chosen (non-inclusive) $themes = array_slice($themes, $index + 1); diff --git a/tests/php/Service/ThemeResolverTest.php b/tests/php/Service/ThemeResolverTest.php index 9050c4a..0b98f3b 100644 --- a/tests/php/Service/ThemeResolverTest.php +++ b/tests/php/Service/ThemeResolverTest.php @@ -11,7 +11,7 @@ use SilverStripe\View\SSViewer; class ThemeResolverTest extends SapphireTest { protected $themeList = [ - SSViewer::PUBLIC_THEME, + '$public', 'custom', 'main', 'backup', @@ -55,7 +55,7 @@ class ThemeResolverTest extends SapphireTest $expected = [ 'main', // 'main' is moved to the top - SSViewer::PUBLIC_THEME, // $public is preserved + '$public', // $public is preserved // Anything above 'main' is removed 'backup', SSViewer::DEFAULT_THEME, @@ -87,7 +87,7 @@ class ThemeResolverTest extends SapphireTest ['test' => $expected = [ 'subsite', 'backup', - SSViewer::PUBLIC_THEME, + '$public', SSViewer::DEFAULT_THEME, ]], 'test', @@ -104,7 +104,7 @@ class ThemeResolverTest extends SapphireTest 'bee' => $expected = [ 'subsite', 'backup', - SSViewer::PUBLIC_THEME, + '$public', SSViewer::DEFAULT_THEME, ], 'sea' => [ @@ -121,7 +121,7 @@ class ThemeResolverTest extends SapphireTest ['main' => $expected = [ 'subsite', 'backup', - SSViewer::PUBLIC_THEME, + '$public', SSViewer::DEFAULT_THEME, ]], 'main', @@ -132,7 +132,7 @@ class ThemeResolverTest extends SapphireTest ['test' => [ 'subsite', 'backup', - SSViewer::PUBLIC_THEME, + '$public', SSViewer::DEFAULT_THEME, ]], 'other', diff --git a/tests/php/SiteTreeSubsitesTest.php b/tests/php/SiteTreeSubsitesTest.php index f6a58f7..640e02f 100644 --- a/tests/php/SiteTreeSubsitesTest.php +++ b/tests/php/SiteTreeSubsitesTest.php @@ -417,7 +417,6 @@ class SiteTreeSubsitesTest extends BaseSubsiteTest Subsite::changeSubsite($subsitePage->SubsiteID); $controller = ModelAsController::controller_for($subsitePage); SiteTree::singleton()->extend('contentcontrollerInit', $controller); - } public function provideAlternateAbsoluteLink() From 58f89801b09e6945dd93529843340948ccb10e03 Mon Sep 17 00:00:00 2001 From: Guy Marriott Date: Fri, 12 Jul 2019 15:24:10 +1200 Subject: [PATCH 4/6] FIX Ensure constant is accessed correctly --- src/Service/ThemeResolver.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Service/ThemeResolver.php b/src/Service/ThemeResolver.php index c61c6cf..297a2bc 100644 --- a/src/Service/ThemeResolver.php +++ b/src/Service/ThemeResolver.php @@ -60,7 +60,8 @@ class ThemeResolver if ($index > 0) { // 4.0 didn't have support for themes in the public webroot - $publicConstantDefined = defined('SSViewer::PUBLIC_THEME'); + $constant = SSViewer::class . '::PUBLIC_THEME'; + $publicConstantDefined = defined($constant); // Check if the default is public themes $publicDefault = $publicConstantDefined && $themes[0] === SSViewer::PUBLIC_THEME; From 9a7cdbbe2d59518d83f04da726c821ced35ea663 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Fri, 26 Jul 2019 09:53:54 +0200 Subject: [PATCH 5/6] FIX Prevent undefined index notice when trying to determine HTTP_HOST during dev/build --- src/Model/Subsite.php | 2 +- src/Model/SubsiteDomain.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Model/Subsite.php b/src/Model/Subsite.php index 229604d..d3190bc 100644 --- a/src/Model/Subsite.php +++ b/src/Model/Subsite.php @@ -870,7 +870,7 @@ class Subsite extends DataObject } // If there are no objects, default to the current hostname - return $_SERVER['HTTP_HOST']; + return isset($_SERVER['HTTP_HOST']) ? $_SERVER['HTTP_HOST'] : 'localhost'; } /** diff --git a/src/Model/SubsiteDomain.php b/src/Model/SubsiteDomain.php index 66432e9..a3aef03 100644 --- a/src/Model/SubsiteDomain.php +++ b/src/Model/SubsiteDomain.php @@ -185,7 +185,7 @@ class SubsiteDomain extends DataObject */ public function getSubstitutedDomain() { - $currentHost = $_SERVER['HTTP_HOST']; + $currentHost = isset($_SERVER['HTTP_HOST']) ? $_SERVER['HTTP_HOST'] : 'localhost'; // If there are wildcards in the primary domain (not recommended), make some // educated guesses about what to replace them with: From 09abe2b2f2dc405fca8e876fd372399980d2f146 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Mon, 29 Jul 2019 10:38:14 +0200 Subject: [PATCH 6/6] Use Director::host() over direct $_SERVER access --- src/Model/Subsite.php | 2 +- src/Model/SubsiteDomain.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Model/Subsite.php b/src/Model/Subsite.php index d3190bc..2bcdbff 100644 --- a/src/Model/Subsite.php +++ b/src/Model/Subsite.php @@ -870,7 +870,7 @@ class Subsite extends DataObject } // If there are no objects, default to the current hostname - return isset($_SERVER['HTTP_HOST']) ? $_SERVER['HTTP_HOST'] : 'localhost'; + return Director::host(); } /** diff --git a/src/Model/SubsiteDomain.php b/src/Model/SubsiteDomain.php index a3aef03..aed9750 100644 --- a/src/Model/SubsiteDomain.php +++ b/src/Model/SubsiteDomain.php @@ -185,7 +185,7 @@ class SubsiteDomain extends DataObject */ public function getSubstitutedDomain() { - $currentHost = isset($_SERVER['HTTP_HOST']) ? $_SERVER['HTTP_HOST'] : 'localhost'; + $currentHost = Director::host(); // If there are wildcards in the primary domain (not recommended), make some // educated guesses about what to replace them with: