From 2eb04ffa7894e145a29c3cfea34d9d850515f014 Mon Sep 17 00:00:00 2001 From: Guy Marriott Date: Fri, 31 May 2019 16:18:52 +1200 Subject: [PATCH] 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()