Improve theme initializer (#4158)

Co-authored-by: Ere Maijala <[email protected]>

module/VuFindTheme/src/VuFindTheme/Initializer.php

@@ -34,6 +34,7 @@
 use Laminas\View\Resolver\TemplatePathStack;
 use Psr\Container\ContainerInterface;
 use VuFind\Config\Config;
+use VuFind\Cookie\CookieManager;
 
 /**
  * VuFind Theme Initializer
@@ -70,28 +71,28 @@ class Initializer
     /**
      * Top-level service container
      *
-     * @var \Psr\Container\ContainerInterface
+     * @var ContainerInterface
      */
     protected $serviceManager;
 
     /**
      * Theme tools object
      *
-     * @var \VuFindTheme\ThemeInfo
+     * @var ThemeInfo
      */
     protected $tools;
 
     /**
      * Mobile interface detector
      *
-     * @var \VuFindTheme\Mobile
+     * @var Mobile
      */
     protected $mobile;
 
     /**
      * Cookie manager
      *
-     * @var \VuFind\Cookie\CookieManager
+     * @var CookieManager
      */
     protected $cookieManager;
 
@@ -143,15 +144,13 @@ public function __construct(Config $config, $eventOrContainer)
         }
 
         // Get the cookie manager from the service manager:
-        $this->cookieManager = $this->serviceManager
-            ->get(\VuFind\Cookie\CookieManager::class);
+        $this->cookieManager = $this->serviceManager->get(CookieManager::class);
 
         // Get base directory from tools object:
-        $this->tools = $this->serviceManager->get(\VuFindTheme\ThemeInfo::class);
+        $this->tools = $this->serviceManager->get(ThemeInfo::class);
 
         // Set up mobile device detector:
-        $this->mobile = $this->serviceManager->get(\VuFindTheme\Mobile::class);
-        $this->mobile->enable(isset($this->config->mobile_theme));
+        $this->mobile = $this->serviceManager->get(Mobile::class);
     }
 
     /**
@@ -169,13 +168,16 @@ public function init()
         }
         self::$themeInitialized = true;
 
-        // Determine the current theme:
-        $currentTheme = $this->pickTheme(
-            isset($this->event) ? $this->event->getRequest() : null
+        // Determine the user-selected or default UI option, and the theme associated with it:
+        $themes = $this->getThemeAliasMap();
+        $selectedUI = $this->getSelectedUI(
+            $themes,
+            $this->event?->getRequest()
         );
+        $currentTheme = $themes[$selectedUI];
 
         // Determine theme options:
-        $this->sendThemeOptionsToView($currentTheme);
+        $this->sendThemeOptionsToView($selectedUI, $currentTheme);
 
         // Make sure the current theme is set correctly in the tools object:
         $error = null;
@@ -208,7 +210,7 @@ protected function getThemeAliasMap(): array
         if ($this->themeMap === null) {
             // Set up special-case 'standard' and 'mobile' aliases:
             $this->themeMap = ['standard' => $this->config->theme];
-            if ($this->mobile->enabled()) {
+            if (isset($this->config->mobile_theme)) {
                 $this->themeMap['mobile'] = $this->config->mobile_theme;
             }
 
@@ -227,12 +229,14 @@ protected function getThemeAliasMap(): array
     /**
      * Support method for init() -- figure out which theme option is active.
      *
+     * @param array   $themes  Data on all available themes (must include at
+     * minimum a 'standard' key)
      * @param Request $request Request object (for obtaining user parameters);
      * set to null if no request context is available.
      *
      * @return string
      */
-    protected function pickTheme(?Request $request)
+    protected function getSelectedUI(array $themes, ?Request $request): string
     {
         // The admin theme should always be picked if
         // - the Admin module is enabled AND
@@ -250,7 +254,6 @@ protected function pickTheme(?Request $request)
         }
 
         // Load standard configuration options:
-        $themes = $this->getThemeAliasMap();
         if (PHP_SAPI == 'cli') {
             return $themes['standard'];
         }
@@ -272,50 +275,51 @@ protected function pickTheme(?Request $request)
         $this->cookieManager->set('ui', $selectedUI);
 
         // Pick the selected theme (fall back to standard if unrecognized):
-        return $themes[$selectedUI] ?? $themes['standard'];
+        return isset($themes[$selectedUI]) ? $selectedUI : 'standard';
     }
 
     /**
      * Make the theme options available to the view.
      *
+     * @param string $selectedUI   Current UI setting
      * @param string $currentTheme Active theme
      *
      * @return void
      */
-    protected function sendThemeOptionsToView($currentTheme)
+    protected function sendThemeOptionsToView(string $selectedUI, string $currentTheme): void
     {
         // Get access to the view model:
         if (PHP_SAPI !== 'cli') {
             $viewModel = $this->serviceManager->get('ViewManager')->getViewModel();
 
             // Send down the view options:
-            $viewModel->setVariable('themeOptions', $this->getThemeOptions($currentTheme));
+            $viewModel->setVariable('themeOptions', $this->getThemeOptions($selectedUI, $currentTheme));
         }
     }
 
     /**
      * Return an array of information about user-selectable themes. Each entry in
      * the array is an associative array with 'name', 'desc' and 'selected' keys.
      *
+     * @param string $selectedUI   Current UI setting
      * @param string $currentTheme Active theme
      *
      * @return array
      */
-    protected function getThemeOptions($currentTheme)
+    protected function getThemeOptions(string $selectedUI, string $currentTheme): array
     {
         $options = [];
         if (isset($this->config->selectable_themes)) {
             $parts = explode(',', $this->config->selectable_themes);
             $foundSelected = false;
-            $uiCookie = $this->cookieManager->get('ui');
             foreach ($parts as $part) {
                 $subparts = explode(':', $part);
                 $name = trim($subparts[0]);
                 $desc = isset($subparts[1]) ? trim($subparts[1]) : '';
                 $desc = empty($desc) ? $name : $desc;
                 // Easiest and most accurate way to pick a selected theme is to check
                 // if the name matches the current value of the ui cookie:
-                $selected = $uiCookie === $name;
+                $selected = $selectedUI === $name;
                 $foundSelected = $foundSelected || $selected;
                 if (!empty($name)) {
                     $options[] = compact('name', 'desc', 'selected');
@@ -365,8 +369,7 @@ protected function setUpThemes($themes)
         $templatePathStack = [];
 
         // Grab the resource manager for tracking CSS, JS, etc.:
-        $resources = $this->serviceManager
-            ->get(\VuFindTheme\ResourceContainer::class);
+        $resources = $this->serviceManager->get(ResourceContainer::class);
 
         // Set generator if necessary:
         if (isset($this->config->generator)) {

module/VuFindTheme/src/VuFindTheme/Mobile.php

@@ -55,13 +55,6 @@ class Mobile
      */
     protected $detector;
 
-    /**
-     * Are mobile themes enabled?
-     *
-     * @var bool
-     */
-    protected $enabled = false;
-
     /**
      * Constructor
      *
@@ -84,26 +77,4 @@ public function detect()
         // class of devices.
         return $this->detector->DetectMobileLong();
     }
-
-    /**
-     * Function to set enabled status of mobile themes.
-     *
-     * @param bool $enabled Are mobile themes enabled?
-     *
-     * @return void
-     */
-    public function enable($enabled = true)
-    {
-        $this->enabled = $enabled;
-    }
-
-    /**
-     * Function to check whether mobile theme is configured.
-     *
-     * @return bool
-     */
-    public function enabled()
-    {
-        return $this->enabled;
-    }
 }

module/VuFindTheme/tests/unit-tests/src/VuFindTest/ThemeMobileTest.php

@@ -43,36 +43,32 @@
 class ThemeMobileTest extends \PHPUnit\Framework\TestCase
 {
     /**
-     * Test namespace stripping.
+     * Data provider for testDetection.
      *
-     * @return void
+     * @return array[]
      */
-    public function testEnable()
+    public static function detectionProvider(): array
     {
-        $mobile = new Mobile();
-        // default behavior
-        $this->assertFalse($mobile->enabled());
-        // turn on
-        $mobile->enable();
-        $this->assertTrue($mobile->enabled());
-        // turn off
-        $mobile->enable(false);
-        $this->assertFalse($mobile->enabled());
+        return [
+            'mobile detected' => [true],
+            'mobile not detected' => [false],
+        ];
     }
 
     /**
      * Test detection wrapping.
      *
+     * @param bool $active Result of mobile detection
+     *
      * @return void
+     *
+     * @dataProvider detectionProvider
      */
-    public function testDetection()
+    public function testDetection(bool $active): void
     {
-        $detector = $this->getMockBuilder(\uagent_info::class)
-            ->onlyMethods(['DetectMobileLong'])
-            ->getMock();
-        $detector->expects($this->once())
-            ->method('DetectMobileLong')->will($this->returnValue(true));
+        $detector = $this->createMock(\uagent_info::class);
+        $detector->expects($this->once())->method('DetectMobileLong')->willReturn($active);
         $mobile = new Mobile($detector);
-        $this->assertTrue($mobile->detect());
+        $this->assertEquals($active, $mobile->detect());
     }
 }