From 20faabff8cb6477ca762d4b60f8d32d81ae3b8a0 Mon Sep 17 00:00:00 2001 From: effulgentsia <alex.bronstein@acquia.com> Date: Wed, 9 Oct 2019 16:29:39 -0700 Subject: [PATCH] Issue #3084069 by Wim Leers, kamalzairig, zrpnr, tinko, webchick, lauriii, tedbow, ckrina: Allow marking themes as experimental --- .../src/Controller/SystemController.php | 9 +- .../system/src/Controller/ThemeController.php | 57 +++++- .../src/Form/ThemeExperimentalConfirmForm.php | 189 ++++++++++++++++++ core/modules/system/system.install | 25 ++- .../Theme/ExperimentalThemeTest.php | 128 ++++++++++++ ...xperimental_theme_dependency_test.info.yml | 6 + .../experimental_theme_test.info.yml | 7 + 7 files changed, 409 insertions(+), 12 deletions(-) create mode 100644 core/modules/system/src/Form/ThemeExperimentalConfirmForm.php create mode 100644 core/modules/system/tests/src/Functional/Theme/ExperimentalThemeTest.php create mode 100644 core/modules/system/tests/themes/experimental_theme_dependency_test/experimental_theme_dependency_test.info.yml create mode 100644 core/modules/system/tests/themes/experimental_theme_test/experimental_theme_test.info.yml diff --git a/core/modules/system/src/Controller/SystemController.php b/core/modules/system/src/Controller/SystemController.php index b0c3ccc25642..f27cc01eb111 100644 --- a/core/modules/system/src/Controller/SystemController.php +++ b/core/modules/system/src/Controller/SystemController.php @@ -197,6 +197,7 @@ public function themesPage() { } $theme->is_default = ($theme->getName() == $theme_default); $theme->is_admin = ($theme->getName() == $admin_theme || ($theme->is_default && empty($admin_theme))); + $theme->is_experimental = isset($theme->info['experimental']) && $theme->info['experimental']; // Identify theme screenshot. $theme->screenshot = NULL; @@ -269,7 +270,7 @@ public function themesPage() { 'attributes' => ['title' => $this->t('Set @theme as default theme', ['@theme' => $theme->info['name']])], ]; } - $admin_theme_options[$theme->getName()] = $theme->info['name']; + $admin_theme_options[$theme->getName()] = $theme->info['name'] . ($theme->is_experimental ? ' (' . t('Experimental') . ')' : ''); } else { $theme->operations[] = [ @@ -287,7 +288,8 @@ public function themesPage() { } } - // Add notes to default and administration theme. + // Add notes to default theme, administration theme and experimental + // themes. $theme->notes = []; if ($theme->is_default) { $theme->notes[] = $this->t('default theme'); @@ -295,6 +297,9 @@ public function themesPage() { if ($theme->is_admin) { $theme->notes[] = $this->t('administration theme'); } + if ($theme->is_experimental) { + $theme->notes[] = $this->t('experimental theme'); + } // Sort installed and uninstalled themes into their own groups. $theme_groups[$theme->status ? 'installed' : 'uninstalled'][] = $theme; diff --git a/core/modules/system/src/Controller/ThemeController.php b/core/modules/system/src/Controller/ThemeController.php index 3a77a59512de..0a8ad31cf81b 100644 --- a/core/modules/system/src/Controller/ThemeController.php +++ b/core/modules/system/src/Controller/ThemeController.php @@ -6,8 +6,10 @@ use Drupal\Core\Config\PreExistingConfigException; use Drupal\Core\Config\UnmetDependenciesException; use Drupal\Core\Controller\ControllerBase; +use Drupal\Core\Extension\ThemeExtensionList; use Drupal\Core\Extension\ThemeHandlerInterface; use Drupal\Core\Extension\ThemeInstallerInterface; +use Drupal\system\Form\ThemeExperimentalConfirmForm; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException; @@ -24,6 +26,13 @@ class ThemeController extends ControllerBase { */ protected $themeHandler; + /** + * An extension discovery instance. + * + * @var \Drupal\Core\Extension\ThemeExtensionList + */ + protected $themeList; + /** * The theme installer service. * @@ -36,13 +45,16 @@ class ThemeController extends ControllerBase { * * @param \Drupal\Core\Extension\ThemeHandlerInterface $theme_handler * The theme handler. + * @param \Drupal\Core\Extension\ThemeExtensionList $theme_list + * The theme extension list. * @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory * The config factory. * @param \Drupal\Core\Extension\ThemeInstallerInterface $theme_installer * The theme installer. */ - public function __construct(ThemeHandlerInterface $theme_handler, ConfigFactoryInterface $config_factory, ThemeInstallerInterface $theme_installer) { + public function __construct(ThemeHandlerInterface $theme_handler, ThemeExtensionList $theme_list, ConfigFactoryInterface $config_factory, ThemeInstallerInterface $theme_installer) { $this->themeHandler = $theme_handler; + $this->themeList = $theme_list; $this->configFactory = $config_factory; $this->themeInstaller = $theme_installer; } @@ -53,6 +65,7 @@ public function __construct(ThemeHandlerInterface $theme_handler, ConfigFactoryI public static function create(ContainerInterface $container) { return new static( $container->get('theme_handler'), + $container->get('extension.list.theme'), $container->get('config.factory'), $container->get('theme_installer') ); @@ -106,8 +119,9 @@ public function uninstall(Request $request) { * @param \Symfony\Component\HttpFoundation\Request $request * A request object containing a theme name and a valid token. * - * @return \Symfony\Component\HttpFoundation\RedirectResponse - * Redirects back to the appearance admin page. + * @return \Symfony\Component\HttpFoundation\RedirectResponse|array + * Redirects back to the appearance admin page or the confirmation form + * if an experimental theme will be installed. * * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException * Throws access denied when no theme or token is set in the request or when @@ -117,6 +131,11 @@ public function install(Request $request) { $theme = $request->query->get('theme'); if (isset($theme)) { + // Display confirmation form in case of experimental theme. + if ($this->willInstallExperimentalTheme($theme)) { + return $this->formBuilder()->getForm(ThemeExperimentalConfirmForm::class, $theme); + } + try { if ($this->themeInstaller->install([$theme])) { $themes = $this->themeHandler->listInfo(); @@ -149,14 +168,38 @@ public function install(Request $request) { throw new AccessDeniedHttpException(); } + /** + * Checks if the given theme requires the installation of experimental themes. + * + * @param string $theme + * The name of the theme to check. + * + * @return bool + * Whether experimental themes will be installed. + */ + protected function willInstallExperimentalTheme($theme) { + $all_themes = $this->themeList->getList(); + $dependencies = array_keys($all_themes[$theme]->requires); + $themes_to_enable = array_merge([$theme], $dependencies); + + foreach ($themes_to_enable as $name) { + if (!empty($all_themes[$name]->info['experimental']) && $all_themes[$name]->status === 0) { + return TRUE; + } + } + + return FALSE; + } + /** * Set the default theme. * * @param \Symfony\Component\HttpFoundation\Request $request * A request object containing a theme name. * - * @return \Symfony\Component\HttpFoundation\RedirectResponse - * Redirects back to the appearance admin page. + * @return \Symfony\Component\HttpFoundation\RedirectResponse|array + * Redirects back to the appearance admin page or the confirmation form + * if an experimental theme will be installed. * * @throws \Symfony\Component\HttpKernel\Exception\AccessDeniedHttpException * Throws access denied when no theme is set in the request. @@ -168,6 +211,10 @@ public function setDefaultTheme(Request $request) { if (isset($theme)) { // Get current list of themes. $themes = $this->themeHandler->listInfo(); + // Display confirmation form if an experimental theme is being installed. + if ($this->willInstallExperimentalTheme($theme)) { + return $this->formBuilder()->getForm(ThemeExperimentalConfirmForm::class, $theme, TRUE); + } // Check if the specified theme is one recognized by the system. // Or try to install the theme. diff --git a/core/modules/system/src/Form/ThemeExperimentalConfirmForm.php b/core/modules/system/src/Form/ThemeExperimentalConfirmForm.php new file mode 100644 index 000000000000..d83cd422ef9b --- /dev/null +++ b/core/modules/system/src/Form/ThemeExperimentalConfirmForm.php @@ -0,0 +1,189 @@ +<?php + +namespace Drupal\system\Form; + +use Drupal\Core\Config\PreExistingConfigException; +use Drupal\Core\Config\UnmetDependenciesException; +use Drupal\Core\Extension\ThemeExtensionList; +use Drupal\Core\Extension\ThemeInstallerInterface; +use Drupal\Core\Form\ConfirmFormBase; +use Drupal\Core\Form\FormStateInterface; +use Drupal\Core\Url; +use Symfony\Component\DependencyInjection\ContainerInterface; + +/** + * Builds a confirmation form for enabling experimental themes. + * + * @internal + */ +class ThemeExperimentalConfirmForm extends ConfirmFormBase { + + /** + * An extension discovery instance. + * + * @var \Drupal\Core\Extension\ThemeExtensionList + */ + protected $themeList; + + /** + * The theme installer service. + * + * @var \Drupal\Core\Extension\ThemeInstallerInterface + */ + protected $themeInstaller; + + /** + * Constructs a ThemeExperimentalConfirmForm object. + * + * @param \Drupal\Core\Extension\ThemeExtensionList $theme_list + * The theme extension list. + * @param \Drupal\Core\Extension\ThemeInstallerInterface $theme_installer + * The theme installer. + */ + public function __construct(ThemeExtensionList $theme_list, ThemeInstallerInterface $theme_installer) { + $this->themeList = $theme_list; + $this->themeInstaller = $theme_installer; + } + + /** + * {@inheritdoc} + */ + public static function create(ContainerInterface $container) { + return new static( + $container->get('extension.list.theme'), + $container->get('theme_installer') + ); + } + + /** + * {@inheritdoc} + */ + public function getQuestion() { + return $this->t('Are you sure you wish to install an experimental theme?'); + } + + /** + * {@inheritdoc} + */ + public function getCancelUrl() { + return new Url('system.themes_page'); + } + + /** + * {@inheritdoc} + */ + public function getConfirmText() { + return $this->t('Continue'); + } + + /** + * {@inheritdoc} + */ + public function getDescription() { + return $this->t('Would you like to continue with the above?'); + } + + /** + * {@inheritdoc} + */ + public function getFormId() { + return 'system_themes_experimental_confirm_form'; + } + + /** + * {@inheritdoc} + */ + public function buildForm(array $form, FormStateInterface $form_state) { + $theme = $form_state->getBuildInfo()['args'][0] ? $form_state->getBuildInfo()['args'][0] : NULL; + $all_themes = $this->themeList->getList(); + if (!isset($all_themes[$theme])) { + return $this->redirect('system.themes_page'); + } + $this->messenger()->addWarning($this->t('Experimental themes are provided for testing purposes only. Use at your own risk.')); + + $dependencies = array_keys($all_themes[$theme]->requires); + $themes = array_merge([$theme], $dependencies); + $is_experimental = function ($theme) use ($all_themes) { + return isset($all_themes[$theme]) && isset($all_themes[$theme]->info['experimental']) && $all_themes[$theme]->info['experimental']; + }; + $get_label = function ($theme) use ($all_themes) { + return $all_themes[$theme]->info['name']; + }; + + $items = []; + if (!empty($dependencies)) { + // Display a list of required themes that have to be installed as well. + $items[] = $this->formatPlural(count($dependencies), 'You must enable the @required theme to install @theme.', 'You must enable the @required themes to install @theme.', [ + '@theme' => $get_label($theme), + // It is safe to implode this because theme names are not translated + // markup and so will not be double-escaped. + '@required' => implode(', ', array_map($get_label, $dependencies)), + ]); + } + // Add the list of experimental themes after any other messages. + $items[] = $this->t('The following themes are experimental: @themes', ['@themes' => implode(', ', array_map($get_label, array_filter($themes, $is_experimental)))]); + $form['message'] = [ + '#theme' => 'item_list', + '#items' => $items, + ]; + return parent::buildForm($form, $form_state); + } + + /** + * {@inheritdoc} + */ + public function submitForm(array &$form, FormStateInterface $form_state) { + $args = $form_state->getBuildInfo()['args']; + $theme = isset($args[0]) ? $args[0] : NULL; + $set_default = isset($args[1]) ? $args[1] : FALSE; + $themes = $this->themeList->getList(); + $config = $this->configFactory()->getEditable('system.theme'); + try { + if ($this->themeInstaller->install([$theme])) { + if ($set_default) { + // Set the default theme. + $config->set('default', $theme)->save(); + + // The status message depends on whether an admin theme is currently + // in use: an empty string means the admin theme is set to be the + // default theme. + $admin_theme = $config->get('admin'); + if (!empty($admin_theme) && $admin_theme !== $theme) { + $this->messenger() + ->addStatus($this->t('Please note that the administration theme is still set to the %admin_theme theme; consequently, the theme on this page remains unchanged. All non-administrative sections of the site, however, will show the selected %selected_theme theme by default.', [ + '%admin_theme' => $themes[$admin_theme]->info['name'], + '%selected_theme' => $themes[$theme]->info['name'], + ])); + } + else { + $this->messenger()->addStatus($this->t('%theme is now the default theme.', ['%theme' => $themes[$theme]->info['name']])); + } + } + else { + $this->messenger()->addStatus($this->t('The %theme theme has been installed.', ['%theme' => $themes[$theme]->info['name']])); + } + } + else { + $this->messenger()->addError($this->t('The %theme theme was not found.', ['%theme' => $theme])); + } + } + catch (PreExistingConfigException $e) { + $config_objects = $e->flattenConfigObjects($e->getConfigObjects()); + $this->messenger()->addError( + $this->formatPlural( + count($config_objects), + 'Unable to install @extension, %config_names already exists in active configuration.', + 'Unable to install @extension, %config_names already exist in active configuration.', + [ + '%config_names' => implode(', ', $config_objects), + '@extension' => $theme, + ]) + ); + } + catch (UnmetDependenciesException $e) { + $this->messenger()->addError($e->getTranslatedMessage($this->getStringTranslation(), $theme)); + } + $form_state->setRedirectUrl($this->getCancelUrl()); + } + +} diff --git a/core/modules/system/system.install b/core/modules/system/system.install index 75f95acc5c32..cafd176ffa7a 100644 --- a/core/modules/system/system.install +++ b/core/modules/system/system.install @@ -65,18 +65,33 @@ function system_requirements($phase) { } // Warn if any experimental modules are installed. - $experimental = []; + $experimental_modules = []; $enabled_modules = \Drupal::moduleHandler()->getModuleList(); foreach ($enabled_modules as $module => $data) { $info = \Drupal::service('extension.list.module')->getExtensionInfo($module); if (isset($info['package']) && $info['package'] === 'Core (Experimental)') { - $experimental[$module] = $info['name']; + $experimental_modules[$module] = $info['name']; } } - if (!empty($experimental)) { - $requirements['experimental'] = [ + if (!empty($experimental_modules)) { + $requirements['experimental_modules'] = [ 'title' => t('Experimental modules enabled'), - 'value' => t('Experimental modules found: %module_list. <a href=":url">Experimental modules</a> are provided for testing purposes only. Use at your own risk.', ['%module_list' => implode(', ', $experimental), ':url' => 'https://www.drupal.org/core/experimental']), + 'value' => t('Experimental modules found: %module_list. <a href=":url">Experimental modules</a> are provided for testing purposes only. Use at your own risk.', ['%module_list' => implode(', ', $experimental_modules), ':url' => 'https://www.drupal.org/core/experimental']), + 'severity' => REQUIREMENT_WARNING, + ]; + } + // Warn if any experimental themes are installed. + $experimental_themes = []; + $installed_themes = \Drupal::service('theme_handler')->listInfo(); + foreach ($installed_themes as $theme => $data) { + if (isset($data->info['experimental']) && $data->info['experimental']) { + $experimental_themes[$theme] = $data->info['name']; + } + } + if (!empty($experimental_themes)) { + $requirements['experimental_themes'] = [ + 'title' => t('Experimental themes enabled'), + 'value' => t('Experimental themes found: %theme_list. Experimental themes are provided for testing purposes only. Use at your own risk.', ['%theme_list' => implode(', ', $experimental_themes)]), 'severity' => REQUIREMENT_WARNING, ]; } diff --git a/core/modules/system/tests/src/Functional/Theme/ExperimentalThemeTest.php b/core/modules/system/tests/src/Functional/Theme/ExperimentalThemeTest.php new file mode 100644 index 000000000000..be37f4a23c32 --- /dev/null +++ b/core/modules/system/tests/src/Functional/Theme/ExperimentalThemeTest.php @@ -0,0 +1,128 @@ +<?php + +namespace Drupal\Tests\system\Functional\Theme; + +use Drupal\Tests\BrowserTestBase; + +/** + * Tests the installation of themes. + * + * @group Theme + */ +class ExperimentalThemeTest extends BrowserTestBase { + + /** + * The admin user. + * + * @var \Drupal\user\UserInterface + */ + protected $adminUser; + + /** + * {@inheritdoc} + */ + protected function setUp() { + parent::setUp(); + $this->adminUser = $this->drupalCreateUser(['access administration pages', 'administer themes']); + $this->drupalLogin($this->adminUser); + } + + /** + * Tests installing experimental themes and dependencies in the UI. + */ + public function testExperimentalConfirmForm() { + // Only experimental themes should be marked as such with a parenthetical. + $this->drupalGet('admin/appearance'); + $this->assertText(sprintf('Experimental test %s (experimental theme)', \Drupal::VERSION)); + $this->assertText(sprintf('Experimental dependency test %s', \Drupal::VERSION)); + + // First, test installing a non-experimental theme with no dependencies. + // There should be no confirmation form and no experimental theme warning. + $this->drupalGet('admin/appearance'); + $this->cssSelect('a[title="Install <strong>Test theme</strong> theme"]')[0]->click(); + $this->assertText('The <strong>Test theme</strong> theme has been installed.'); + $this->assertNoText('Experimental modules are provided for testing purposes only.'); + + // Next, test installing an experimental theme with no dependencies. + // There should be a confirmation form with an experimental warning, but no + // list of dependencies. + $this->drupalGet('admin/appearance'); + $this->cssSelect('a[title="Install Experimental test theme"]')[0]->click(); + $this->assertText('Experimental themes are provided for testing purposes only. Use at your own risk.'); + + // The module should not be enabled and there should be a warning and a + // list of the experimental modules with only this one. + $this->assertNoText('The Experimental Test theme has been installed.'); + $this->assertText('Experimental themes are provided for testing purposes only.'); + + // There should be no message about enabling dependencies. + $this->assertNoText('You must enable'); + + // Enable the theme and confirm that it worked. + $this->drupalPostForm(NULL, [], 'Continue'); + $this->assertText('The Experimental test theme has been installed.'); + + // Setting it as the default should not ask for another confirmation. + $this->cssSelect('a[title="Set Experimental test as default theme"]')[0]->click(); + $this->assertNoText('Experimental themes are provided for testing purposes only. Use at your own risk.'); + $this->assertText('Experimental test is now the default theme.'); + $this->assertNoText(sprintf('Experimental test %s (experimental theme)', \Drupal::VERSION)); + $this->assertText(sprintf('Experimental test %s (default theme, administration theme, experimental theme)', \Drupal::VERSION)); + + // Uninstall the theme. + $this->config('system.theme')->set('default', 'test_theme')->save(); + \Drupal::service('theme_handler')->refreshInfo(); + \Drupal::service('theme_installer')->uninstall(['experimental_theme_test']); + + // Reinstall the same experimental theme, but this time immediately set it + // as the default. This should again trigger a confirmation form with an + // experimental warning. + $this->drupalGet('admin/appearance'); + $this->cssSelect('a[title="Install Experimental test as default theme"]')[0]->click(); + $this->assertText('Experimental themes are provided for testing purposes only. Use at your own risk.'); + + // Test enabling a theme that is not itself experimental, but that depends + // on an experimental module. + $this->drupalGet('admin/appearance'); + $this->cssSelect('a[title="Install Experimental dependency test theme"]')[0]->click(); + + // The theme should not be enabled and there should be a warning and a + // list of the experimental modules with only this one. + $this->assertNoText('The Experimental dependency test theme has been installed.'); + $this->assertText('Experimental themes are provided for testing purposes only. Use at your own risk.'); + $this->assertText('The following themes are experimental: Experimental test'); + + // Ensure the non-experimental theme is not listed as experimental. + $this->assertNoText('The following themes are experimental: Experimental test, Experimental dependency test'); + $this->assertNoText('The following themes are experimental: Experimental dependency test'); + + // There should be a message about enabling dependencies. + $this->assertText('You must enable the Experimental test theme to install Experimental dependency test'); + + // Enable the theme and confirm that it worked. + $this->drupalPostForm(NULL, [], 'Continue'); + $this->assertText('The Experimental dependency test theme has been installed.'); + $this->assertText(sprintf('Experimental test %s (experimental theme)', \Drupal::VERSION)); + $this->assertText(sprintf('Experimental dependency test %s', \Drupal::VERSION)); + + // Setting it as the default should not ask for another confirmation. + $this->cssSelect('a[title="Set Experimental dependency test as default theme"]')[0]->click(); + $this->assertNoText('Experimental themes are provided for testing purposes only. Use at your own risk.'); + $this->assertText('Experimental dependency test is now the default theme.'); + $this->assertText(sprintf('Experimental test %s (experimental theme)', \Drupal::VERSION)); + $this->assertText(sprintf('Experimental dependency test %s (default theme, administration theme)', \Drupal::VERSION)); + + // Uninstall the theme. + $this->config('system.theme')->set('default', 'test_theme')->save(); + \Drupal::service('theme_handler')->refreshInfo(); + \Drupal::service('theme_installer')->uninstall(['experimental_theme_test', 'experimental_theme_dependency_test']); + + // Reinstall the same theme, but this time immediately set it as the + // default. This should again trigger a confirmation form with an + // experimental warning for its dependency. + $this->drupalGet('admin/appearance'); + $this->cssSelect('a[title="Install Experimental dependency test as default theme"]')[0]->click(); + $this->assertText('Experimental themes are provided for testing purposes only. Use at your own risk.'); + } + +} diff --git a/core/modules/system/tests/themes/experimental_theme_dependency_test/experimental_theme_dependency_test.info.yml b/core/modules/system/tests/themes/experimental_theme_dependency_test/experimental_theme_dependency_test.info.yml new file mode 100644 index 000000000000..af20957d6935 --- /dev/null +++ b/core/modules/system/tests/themes/experimental_theme_dependency_test/experimental_theme_dependency_test.info.yml @@ -0,0 +1,6 @@ +name: 'Experimental dependency test' +type: theme +description: 'Experimental dependency test theme.' +version: VERSION +core: 8.x +base theme: experimental_theme_test diff --git a/core/modules/system/tests/themes/experimental_theme_test/experimental_theme_test.info.yml b/core/modules/system/tests/themes/experimental_theme_test/experimental_theme_test.info.yml new file mode 100644 index 000000000000..e80490a4d845 --- /dev/null +++ b/core/modules/system/tests/themes/experimental_theme_test/experimental_theme_test.info.yml @@ -0,0 +1,7 @@ +name: 'Experimental test' +type: theme +description: 'Experimental test theme.' +version: VERSION +core: 8.x +base theme: false +experimental: true -- GitLab