Issue #2663796 by xjm: Use a confirmation form when enabling experimental modules

core/modules/system/src/Form/ModulesListConfirmForm.php

@@ -7,6 +7,7 @@
 
 namespace Drupal\system\Form;
 
+use Drupal\Component\Render\MarkupInterface;
 use Drupal\Core\Config\PreExistingConfigException;
 use Drupal\Core\Config\UnmetDependenciesException;
 use Drupal\Core\Extension\ModuleHandlerInterface;
@@ -124,16 +125,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
       return $this->redirect('system.modules_list');
     }
 
-    $items = array();
-    // Display a list of required modules that have to be installed as well but
-    // were not manually selected.
-    foreach ($this->modules['dependencies'] as $module => $dependencies) {
-      $items[] = $this->formatPlural(count($dependencies), 'You must enable the @required module to install @module.', 'You must enable the @required modules to install @module.', array(
-        '@module' => $this->modules['install'][$module],
-        '@required' => implode(', ', $dependencies),
-      ));
-    }
-
+    $items = $this->buildMessageList();
     $form['message'] = array(
       '#theme' => 'item_list',
       '#items' => $items,
@@ -142,6 +134,31 @@ public function buildForm(array $form, FormStateInterface $form_state) {
     return parent::buildForm($form, $form_state);
   }
 
+  /**
+   * Builds the message list for the confirmation form.
+   *
+   * @return MarkupInterface[]
+   *   Array of markup for the list of messages on the form.
+   *
+   * @see \Drupal\system\Form\ModulesListForm::buildModuleList()
+   */
+  protected function buildMessageList() {
+    $items = [];
+    if (!empty($this->modules['dependencies'])) {
+      // Display a list of required modules that have to be installed as well
+      // but were not manually selected.
+      foreach ($this->modules['dependencies'] as $module => $dependencies) {
+        $items[] = $this->formatPlural(count($dependencies), 'You must enable the @required module to install @module.', 'You must enable the @required modules to install @module.', [
+          '@module' => $this->modules['install'][$module],
+          // It is safe to implode this because module names are not translated
+          // markup and so will not be double-escaped.
+          '@required' => implode(', ', $dependencies),
+        ]);
+      }
+    }
+    return $items;
+  }
+
   /**
    * {@inheritdoc}
    */

core/modules/system/src/Form/ModulesListExperimentalConfirmForm.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\system\Form\ModulesListExperimentalConfirmForm.
+ */
+
+namespace Drupal\system\Form;
+
+use Drupal\Core\Form\ConfirmFormBase;
+use Drupal\Core\Form\FormStateInterface;
+
+/**
+ * Builds a confirmation form for enabling experimental modules.
+ */
+class ModulesListExperimentalConfirmForm extends ModulesListConfirmForm {
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getQuestion() {
+    return $this->t('Are you sure you wish to enable experimental modules?');
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getFormId() {
+    return 'system_modules_experimental_confirm_form';
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function buildMessageList() {
+    drupal_set_message($this->t('<a href=":url">Experimental modules</a> are provided for testing purposes only. Use at your own risk.', [':url' => 'https://www.drupal.org/core/experimental']), 'warning');
+
+    $items = parent::buildMessageList();
+    // Add the list of experimental modules after any other messages.
+    $items[] = $this->t('The following modules are experimental: @modules', ['@modules' => implode(', ', array_values($this->modules['experimental']))]);
+
+    return $items;
+  }
+
+}

core/modules/system/src/Form/ModulesListForm.php

@@ -368,6 +368,7 @@ protected function buildModuleList(FormStateInterface $form_state) {
     $modules = array(
       'install' => array(),
       'dependencies' => array(),
+      'experimental' => [],
     );
 
     // Required modules have to be installed.
@@ -380,10 +381,14 @@ protected function buildModuleList(FormStateInterface $form_state) {
     }
 
     // First, build a list of all modules that were selected.
-    foreach ($packages as $items) {
+    foreach ($packages as $package => $items) {
       foreach ($items as $name => $checkbox) {
         if ($checkbox['enable'] && !$this->moduleHandler->moduleExists($name)) {
           $modules['install'][$name] = $data[$name]->info['name'];
+          // Identify experimental modules.
+          if ($package == 'Core (Experimental)') {
+            $modules['experimental'][$name] = $data[$name]->info['name'];
+          }
         }
       }
     }
@@ -394,6 +399,11 @@ protected function buildModuleList(FormStateInterface $form_state) {
         if (!isset($modules['install'][$dependency]) && !$this->moduleHandler->moduleExists($dependency)) {
           $modules['dependencies'][$module][$dependency] = $data[$dependency]->info['name'];
           $modules['install'][$dependency] = $data[$dependency]->info['name'];
+
+          // Identify experimental modules.
+          if ($data[$dependency]->info['package'] == 'Core (Experimental)') {
+            $modules['experimental'][$dependency] = $data[$dependency]->info['name'];
+          }
         }
       }
     }
@@ -423,16 +433,16 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
     // Retrieve a list of modules to install and their dependencies.
     $modules = $this->buildModuleList($form_state);
 
-    // Check if we have to install any dependencies. If there is one or more
-    // dependencies that are not installed yet, redirect to the confirmation
-    // form.
-    if (!empty($modules['dependencies']) || !empty($modules['missing'])) {
+    // Redirect to a confirmation form if needed.
+    if (!empty($modules['experimental']) || !empty($modules['dependencies'])) {
+
+      $route_name = !empty($modules['experimental']) ? 'system.modules_list_experimental_confirm' : 'system.modules_list_confirm';
       // Write the list of changed module states into a key value store.
       $account = $this->currentUser()->id();
       $this->keyValueExpirable->setWithExpire($account, $modules, 60);
 
       // Redirect to the confirmation form.
-      $form_state->setRedirect('system.modules_list_confirm');
+      $form_state->setRedirect($route_name);
 
       // We can exit here because at least one modules has dependencies
       // which we have to prompt the user for in a confirmation form.

core/modules/system/src/Tests/Module/ExperimentalModuleTest.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\system\Tests\Module\ExperimentalModuleTest.
+ */
+
+namespace Drupal\system\Tests\Module;
+
+use Drupal\simpletest\WebTestBase;
+
+/**
+ * Tests the installation of modules.
+ *
+ * @group Module
+ */
+class ExperimentalModuleTest extends WebTestBase {
+
+
+  /**
+   * The admin user.
+   *
+   * @var \Drupal\user\UserInterface
+   */
+  protected $adminUser;
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setUp() {
+    parent::setUp();
+    $this->adminUser = $this->drupalCreateUser(['access administration pages', 'administer modules']);
+    $this->drupalLogin($this->adminUser);
+  }
+
+  /**
+   * Tests installing experimental modules and dependencies in the UI.
+   */
+  public function testExperimentalConfirmForm() {
+
+    // First, test installing a non-experimental module with no dependencies.
+    // There should be no confirmation form and no experimental module warning.
+    $edit = [];
+    $edit["modules[Testing][test_page_test][enable]"] = TRUE;
+    $this->drupalPostForm('admin/modules', $edit, t('Install'));
+    $this->assertText('Module Test page has been enabled.');
+    $this->assertNoText('Experimental modules are provided for testing purposes only.');
+
+    // Uninstall the module.
+    \Drupal::service('module_installer')->uninstall(['test_page_test']);
+
+    // Next, test installing an experimental module with no dependencies.
+    // There should be a confirmation form with an experimental warning, but no
+    // list of dependencies.
+    $edit = [];
+    $edit["modules[Core (Experimental)][experimental_module_test][enable]"] = TRUE;
+    $this->drupalPostForm('admin/modules', $edit, 'Install');
+
+    // 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('Experimental Test has been enabled.');
+    $this->assertText('Experimental modules are provided for testing purposes only.');
+    $this->assertText('The following modules are experimental: Experimental Test');
+
+    // There should be no message about enabling dependencies.
+    $this->assertNoText('You must enable');
+
+    // Enable the module and confirm that it worked.
+    $this->drupalPostForm(NULL, [], 'Continue');
+    $this->assertText('Experimental Test has been enabled.');
+
+    // Uninstall the module.
+    \Drupal::service('module_installer')->uninstall(['experimental_module_test']);
+
+    // Test enabling a module that is not itself experimental, but that depends
+    // on an experimental module.
+    $edit = [];
+    $edit["modules[Testing][experimental_module_dependency_test][enable]"] = TRUE;
+    $this->drupalPostForm('admin/modules', $edit, 'Install');
+
+    // 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('2 modules have been enabled: Experimental Dependency Test, Experimental Test');
+    $this->assertText('Experimental modules are provided for testing purposes only.');
+
+    $this->assertText('The following modules are experimental: Experimental Test');
+
+    // Ensure the non-experimental module is not listed as experimental.
+    $this->assertNoText('The following modules are experimental: Experimental Test, Experimental Dependency Test');
+    $this->assertNoText('The following modules are experimental: Experimental Dependency Test');
+
+    // There should be a message about enabling dependencies.
+    $this->assertText('You must enable the Experimental Test module to install Experimental Dependency Test');
+
+    // Enable the module and confirm that it worked.
+    $this->drupalPostForm(NULL, [], 'Continue');
+    $this->assertText('2 modules have been enabled: Experimental Dependency Test, Experimental Test');
+
+    // Uninstall the modules.
+    \Drupal::service('module_installer')->uninstall(['experimental_module_test', 'experimental_module_dependency_test']);
+
+    // Finally, check both the module and its experimental dependency. There is
+    // still a warning about experimental modules, but no message about
+    // dependencies, since the user specifically enabled the dependency.
+    $edit = [];
+    $edit["modules[Core (Experimental)][experimental_module_test][enable]"] = TRUE;
+    $edit["modules[Testing][experimental_module_dependency_test][enable]"] = TRUE;
+    $this->drupalPostForm('admin/modules', $edit, 'Install');
+
+    // 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('2 modules have been enabled: Experimental Dependency Test, Experimental Test');
+    $this->assertText('Experimental modules are provided for testing purposes only.');
+
+    $this->assertText('The following modules are experimental: Experimental Test');
+
+    // Ensure the non-experimental module is not listed as experimental.
+    $this->assertNoText('The following modules are experimental: Experimental Test, Experimental Dependency Test');
+    $this->assertNoText('The following modules are experimental: Experimental Dependency Test');
+
+    // There should be no message about enabling dependencies.
+    $this->assertNoText('You must enable');
+
+    // Enable the module and confirm that it worked.
+    $this->drupalPostForm(NULL, [], 'Continue');
+    $this->assertText('2 modules have been enabled: Experimental Test, Experimental Dependency Test');
+
+  }
+
+}

core/modules/system/src/Tests/Module/InstallUninstallTest.php

@@ -100,9 +100,24 @@ public function testInstallUninstall() {
       $edit["modules[$package][$name][enable]"] = TRUE;
       $this->drupalPostForm('admin/modules', $edit, t('Install'));
 
+      // Handle experimental modules, which require a confirmation screen.
+      if ($package == 'Core (Experimental)') {
+        $this->assertText('Are you sure you wish to enable experimental modules?');
+        if (count($modules_to_install) > 1) {
+          // When there are experimental modules, needed dependencies do not
+          // result in the same page title, but there will be expected text
+          // indicating they need to be enabled.
+          $this->assertText('You must enable');
+        }
+        $this->drupalPostForm(NULL, array(), t('Continue'));
+      }
       // Handle the case where modules were installed along with this one and
       // where we therefore hit a confirmation screen.
-      if (count($modules_to_install) > 1) {
+      elseif (count($modules_to_install) > 1) {
+        // Verify that we are on the correct form and that the expected text
+        // about enabling dependencies appears.
+        $this->assertText('Some required modules must be enabled');
+        $this->assertText('You must enable');
         $this->drupalPostForm(NULL, array(), t('Continue'));
       }
 
@@ -178,10 +193,21 @@ public function testInstallUninstall() {
     // - That enabling more than one module at the same time does not lead to
     //   any errors.
     $edit = array();
+    $experimental = FALSE;
     foreach ($all_modules as $name => $module) {
       $edit['modules[' . $module->info['package'] . '][' . $name . '][enable]'] = TRUE;
+      // Track whether there is at least one experimental module.
+      if ($module->info['package'] == 'Core (Experimental)') {
+        $experimental = TRUE;
+      }
     }
     $this->drupalPostForm('admin/modules', $edit, t('Install'));
+
+    // If there are experimental modules, click the confirm form.
+    if ($experimental) {
+      $this->assertText('Are you sure you wish to enable experimental modules?');
+      $this->drupalPostForm(NULL, array(), t('Continue'));
+    }
     $this->assertText(t('@count modules have been enabled: ', array('@count' => count($all_modules))), 'Modules status has been updated.');
   }
 

core/modules/system/system.routing.yml

@@ -273,6 +273,14 @@ system.modules_list_confirm:
   requirements:
     _permission: 'administer modules'
 
+system.modules_list_experimental_confirm:
+  path: '/admin/modules/list/confirm-experimental'
+  defaults:
+    _form: 'Drupal\system\Form\ModulesListExperimentalConfirmForm'
+    _title: 'Experimental modules'
+  requirements:
+    _permission: 'administer modules'
+
 system.theme_uninstall:
   path: '/admin/appearance/uninstall'
   defaults:

core/modules/system/tests/modules/experimental_module_dependency_test/experimental_module_dependency_test.info.yml

+name: 'Experimental Dependency Test'
+type: module
+description: 'Module with a dependency in the experimental package.'
+package: Testing
+dependencies:
+  - experimental_module_test
+version: VERSION
+core: 8.x

core/modules/system/tests/modules/experimental_module_dependency_test/experimental_module_dependency_test.module

+<?php
+
+/**
+ * @file
+ * Tests dependencies on an experimental module.
+ */

core/modules/system/tests/modules/experimental_module_test/experimental_module_test.info.yml

@@ -4,4 +4,3 @@ description: 'Module in the experimental package to test experimental functional
 package: Core (Experimental)
 version: 8.y.x-unstable
 core: 8.x
-hidden: true