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

@@ -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)) {

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);
         }
 

src/Service/ThemeResolver.php

@@ -0,0 +1,95 @@
+<?php
+
+namespace SilverStripe\Subsites\Service;
+
+use SilverStripe\Core\Config\Configurable;
+use SilverStripe\Core\Injector\Injectable;
+use SilverStripe\Subsites\Model\Subsite;
+use SilverStripe\View\SSViewer;
+
+class ThemeResolver
+{
+    use Injectable;
+    use Configurable;
+
+    /**
+     * Cascading definitions for themes, keyed by the name they should appear under in the CMS. For example:
+     *
+     * [
+     *   'theme-1' => [
+     *     '$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);
+    }
+}

tests/php/Service/ThemeResolverTest.php

@@ -0,0 +1,143 @@
+<?php
+
+namespace SilverStripe\Subsites\Tests\Service;
+
+use SilverStripe\Core\Config\Config;
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\Subsites\Model\Subsite;
+use SilverStripe\Subsites\Service\ThemeResolver;
+use SilverStripe\View\SSViewer;
+
+class ThemeResolverTest extends SapphireTest
+{
+    protected $themeList = [
+        SSViewer::PUBLIC_THEME,
+        'custom',
+        'main',
+        'backup',
+        SSViewer::DEFAULT_THEME,
+    ];
+
+    protected function setUp()
+    {
+        parent::setUp();
+
+        // Setup known theme config
+        Config::modify()->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)
+            ],
+        ];
+    }
+}

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'];
+        $firstResolver = $this->createMock(ThemeResolver::class);
-        SSViewer::set_themes($defaultThemes);
+        $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(
+        $secondResolver = $this->createMock(ThemeResolver::class);
-            SSViewer::get_themes(),
+        $secondResolver->expects($this->once())->method('getThemeList');
-            $defaultThemes,
+        Injector::inst()->registerService($secondResolver, ThemeResolver::class);
-            'Themes should not be modified when Subsite has no theme defined'
-        );
 
-        $pageWithTheme = $this->objFromFixture(Page::class, 'subsite1_home');
+        $subsitePage = $this->objFromFixture(Page::class, 'subsite1_home');
-        Subsite::changeSubsite($pageWithTheme->SubsiteID);
+        Subsite::changeSubsite($subsitePage->SubsiteID);
-        $controller = ModelAsController::controller_for($pageWithTheme);
+        $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()