diff -u b/core/lib/Drupal/Core/Extension/RequiredModuleUninstallValidator.php b/core/lib/Drupal/Core/Extension/RequiredModuleUninstallValidator.php --- b/core/lib/Drupal/Core/Extension/RequiredModuleUninstallValidator.php +++ b/core/lib/Drupal/Core/Extension/RequiredModuleUninstallValidator.php @@ -34,7 +34,7 @@ $reasons = []; $module_info = $this->getModuleInfoByModule($module); if (!empty($module_info['required'])) { - $reasons[] = $this->t('The @module module is required.', ['@module' => $module_info['name']]); + $reasons[] = $this->t('The @module module is required', ['@module' => $module_info['name']]); } return $reasons; } diff -u b/core/modules/book/book.module b/core/modules/book/book.module --- b/core/modules/book/book.module +++ b/core/modules/book/book.module @@ -19,7 +19,6 @@ use Drupal\Core\Language\LanguageInterface; use Drupal\Core\Entity\Display\EntityViewDisplayInterface; use Drupal\Core\Template\Attribute; -use Drupal\Core\Extension\Extension; /** * Implements hook_help(). diff -u b/core/modules/book/src/BookUninstallValidator.php b/core/modules/book/src/BookUninstallValidator.php --- b/core/modules/book/src/BookUninstallValidator.php +++ b/core/modules/book/src/BookUninstallValidator.php @@ -57,13 +57,13 @@ $reasons = []; if ($module == 'book') { if ($this->hasBookOutlines()) { - $reasons[] = $this->t('To uninstall Book, delete all content that is part of a book.'); + $reasons[] = $this->t('To uninstall Book, delete all content that is part of a book'); } else { // The book node type is provided by the Book module. Prevent uninstall // if there are any nodes of that type. if ($this->hasBookNodes()) { - $reasons[] = $this->t('To uninstall Book, delete all content that has the Book content type.'); + $reasons[] = $this->t('To uninstall Book, delete all content that has the Book content type'); } } } diff -u /dev/null b/core/modules/book/src/Tests/BookUninstallTest.php --- /dev/null +++ b/core/modules/book/src/Tests/BookUninstallTest.php @@ -0,0 +1,99 @@ +installEntitySchema('user'); + $this->installEntitySchema('node'); + $this->installSchema('book', array('book')); + $this->installSchema('node', array('node_access')); + $this->installConfig(array('node', 'book', 'field')); + // For uninstall to work. + $this->installSchema('user', array('users_data')); + } + + /** + * Tests the book_system_info_alter() method. + */ + public function testBookUninstall() { + // No nodes exist. + $validation_reasons = \Drupal::service('module_installer')->validateUninstall(['book']); + $this->assertEqual([], $validation_reasons, 'The book module is not required.'); + + $content_type = NodeType::create(array( + 'type' => $this->randomMachineName(), + 'name' => $this->randomString(), + )); + $content_type->save(); + $book_config = $this->config('book.settings'); + $allowed_types = $book_config->get('allowed_types'); + $allowed_types[] = $content_type->id(); + $book_config->set('allowed_types', $allowed_types)->save(); + + $node = Node::create(array('type' => $content_type->id())); + $node->book['bid'] = 'new'; + $node->save(); + + // One node in a book but not of type book. + $validation_reasons = \Drupal::service('module_installer')->validateUninstall(['book']); + $this->assertEqual(['To uninstall Book, delete all content that is part of a book'], $validation_reasons['book']); + + $book_node = Node::create(array('type' => 'book')); + $book_node->book['bid'] = FALSE; + $book_node->save(); + + // Two nodes, one in a book but not of type book and one book node (which is + // not in a book). + $validation_reasons = \Drupal::service('module_installer')->validateUninstall(['book']); + $this->assertEqual(['To uninstall Book, delete all content that is part of a book'], $validation_reasons['book']); + + $node->delete(); + // One node of type book but not actually part of a book. + $validation_reasons = \Drupal::service('module_installer')->validateUninstall(['book']); + $this->assertEqual(['To uninstall Book, delete all content that has the Book content type'], $validation_reasons['book']); + + $book_node->delete(); + // No nodes exist therefore the book module is not required. + $module_data = _system_rebuild_module_data(); + $this->assertFalse(isset($module_data['book']->info['required']), 'The book module is not required.'); + + $node = Node::create(array('type' => $content_type->id())); + $node->save(); + // One node exists but is not part of a book therefore the book module is + // not required. + $validation_reasons = \Drupal::service('module_installer')->validateUninstall(['book']); + $this->assertEqual([], $validation_reasons, 'The book module is not required.'); + + // Uninstall the Book module and check the node type is deleted. + \Drupal::service('module_installer')->uninstall(array('book')); + $this->assertNull(NodeType::load('book'), "The book node type does not exist."); + } + +} diff -u b/core/modules/book/tests/src/Unit/BookUninstallValidatorTest.php b/core/modules/book/tests/src/Unit/BookUninstallValidatorTest.php --- b/core/modules/book/tests/src/Unit/BookUninstallValidatorTest.php +++ b/core/modules/book/tests/src/Unit/BookUninstallValidatorTest.php @@ -76,7 +76,7 @@ ->willReturn(TRUE); $module = 'book'; - $expected = ['To uninstall Book, delete all content that has the Book content type.']; + $expected = ['To uninstall Book, delete all content that has the Book content type']; $reasons = $this->bookUninstallValidator->validate($module); $this->assertSame($expected, $reasons); } @@ -92,7 +92,7 @@ ->method('hasBookNodes'); $module = 'book'; - $expected = ['To uninstall Book, delete all content that is part of a book.']; + $expected = ['To uninstall Book, delete all content that is part of a book']; $reasons = $this->bookUninstallValidator->validate($module); $this->assertSame($expected, $reasons); } diff -u b/core/modules/config/src/Tests/ConfigImportAllTest.php b/core/modules/config/src/Tests/ConfigImportAllTest.php --- b/core/modules/config/src/Tests/ConfigImportAllTest.php +++ b/core/modules/config/src/Tests/ConfigImportAllTest.php @@ -8,8 +8,10 @@ namespace Drupal\config\Tests; use Drupal\Core\Config\StorageComparer; +use Drupal\filter\Entity\FilterFormat; use Drupal\system\Tests\Module\ModuleTestBase; use Drupal\shortcut\Entity\Shortcut; +use Drupal\taxonomy\Entity\Term; /** * Tests the largest configuration import possible with all available modules. @@ -83,12 +85,13 @@ // Purge the data. field_purge_batch(1000); - // Delete any forum terms so it can be uninstalled. - $vid = $this->config('forum.settings')->get('vocabulary'); - $terms = entity_load_multiple_by_properties('taxonomy_term', ['vid' => $vid]); - foreach ($terms as $term) { - $term->delete(); - } + // Delete all terms. + $terms = Term::loadMultiple(); + entity_delete_multiple('taxonomy_term', array_keys($terms)); + + // Delete all filter formats. + $filters = FilterFormat::loadMultiple(); + entity_delete_multiple('filter_format', array_keys($filters)); // Delete any shortcuts so the shortcut module can be uninstalled. $shortcuts = Shortcut::loadMultiple(); @@ -97,7 +100,11 @@ system_list_reset(); $all_modules = system_rebuild_module_data(); - $modules_to_uninstall = array_filter($all_modules, function ($module) { + // Ensure that only core required modules and the install profile can not be uninstalled. + $validation_reasons = \Drupal::service('module_installer')->validateUninstall(array_keys($all_modules)); + $this->assertEqual(['standard', 'system', 'user'], array_keys($validation_reasons)); + + $modules_to_uninstall = array_filter($all_modules, function ($module) use ($validation_reasons) { // Filter required and not enabled modules. if (!empty($module->info['required']) || $module->status == FALSE) { return FALSE; @@ -108,14 +115,9 @@ // Can not uninstall config and use admin/config/development/configuration! unset($modules_to_uninstall['config']); - // Can not uninstall Editor and Filter and their dependencies as they - // provide filter plugins that can not be removed. - unset($modules_to_uninstall['editor']); - unset($modules_to_uninstall['filter']); - unset($modules_to_uninstall['file']); - unset($modules_to_uninstall['field']); - $this->assertTrue(isset($modules_to_uninstall['comment']), 'The comment module will be disabled'); + $this->assertTrue(isset($modules_to_uninstall['file']), 'The File module will be disabled'); + $this->assertTrue(isset($modules_to_uninstall['editor']), 'The Editor module will be disabled'); // Uninstall all modules that can be uninstalled. \Drupal::service('module_installer')->uninstall(array_keys($modules_to_uninstall)); diff -u b/core/modules/field/field.module b/core/modules/field/field.module --- b/core/modules/field/field.module +++ b/core/modules/field/field.module @@ -7,7 +7,6 @@ use Drupal\Core\Config\ConfigImporter; use Drupal\Core\Entity\EntityTypeInterface; use Drupal\Core\Entity\DynamicallyFieldableEntityStorageInterface; -use Drupal\Core\Extension\Extension; use Drupal\field\Entity\FieldConfig; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Routing\RouteMatchInterface; diff -u b/core/modules/field/src/FieldUninstallValidator.php b/core/modules/field/src/FieldUninstallValidator.php --- b/core/modules/field/src/FieldUninstallValidator.php +++ b/core/modules/field/src/FieldUninstallValidator.php @@ -55,10 +55,10 @@ } } if ($non_deleted) { - $reasons[] = $this->t('Fields type(s) in use.'); + $reasons[] = $this->t('Fields type(s) in use'); } else { - $reasons[] = $this->t('Fields pending deletion.'); + $reasons[] = $this->t('Fields pending deletion'); } } return $reasons; diff -u b/core/modules/field/tests/src/Unit/FieldUninstallValidatorTest.php b/core/modules/field/tests/src/Unit/FieldUninstallValidatorTest.php --- b/core/modules/field/tests/src/Unit/FieldUninstallValidatorTest.php +++ b/core/modules/field/tests/src/Unit/FieldUninstallValidatorTest.php @@ -61,7 +61,7 @@ ->willReturn([$field_storage]); $module = $this->randomMachineName(); - $expected = ['Fields pending deletion.']; + $expected = ['Fields pending deletion']; $reasons = $this->fieldUninstallValidator->validate($module); $this->assertSame($expected, $reasons); } @@ -81,7 +81,7 @@ ->willReturn([$field_storage]); $module = $this->randomMachineName(); - $expected = ['Fields type(s) in use.']; + $expected = ['Fields type(s) in use']; $reasons = $this->fieldUninstallValidator->validate($module); $this->assertSame($expected, $reasons); } diff -u b/core/modules/filter/filter.module b/core/modules/filter/filter.module --- b/core/modules/filter/filter.module +++ b/core/modules/filter/filter.module @@ -10,7 +10,6 @@ use Drupal\Component\Utility\SafeMarkup; use Drupal\Component\Utility\Xss; use Drupal\Core\Cache\Cache; -use Drupal\Core\Extension\Extension; use Drupal\Core\Render\Element; use Drupal\Core\Routing\RouteMatchInterface; use Drupal\Core\Session\AccountInterface; diff -u b/core/modules/filter/src/FilterUninstallValidator.php b/core/modules/filter/src/FilterUninstallValidator.php --- b/core/modules/filter/src/FilterUninstallValidator.php +++ b/core/modules/filter/src/FilterUninstallValidator.php @@ -69,7 +69,7 @@ } } if (!empty($used_in)) { - $reasons[] = $this->t('Provides a filter plugin that is in use in the following filter formats: %formats.', ['%formats' => implode(', ', $used_in)]); + $reasons[] = $this->t('Provides a filter plugin that is in use in the following filter formats: %formats', ['%formats' => implode(', ', $used_in)]); } } return $reasons; diff -u b/core/modules/filter/tests/src/Unit/FilterUninstallValidatorTest.php b/core/modules/filter/tests/src/Unit/FilterUninstallValidatorTest.php --- b/core/modules/filter/tests/src/Unit/FilterUninstallValidatorTest.php +++ b/core/modules/filter/tests/src/Unit/FilterUninstallValidatorTest.php @@ -160,7 +160,7 @@ ]); $expected = [ - SafeMarkup::format('Provides a filter plugin that is in use in the following filter formats: %formats.', ['%formats' => implode(', ', [ + SafeMarkup::format('Provides a filter plugin that is in use in the following filter formats: %formats', ['%formats' => implode(', ', [ 'Filter Format 1 Label', 'Filter Format 2 Label', ])]), diff -u b/core/modules/forum/forum.module b/core/modules/forum/forum.module --- b/core/modules/forum/forum.module +++ b/core/modules/forum/forum.module @@ -11,11 +11,8 @@ use Drupal\Core\Entity\EntityInterface; use Drupal\Core\Url; use Drupal\Component\Utility\SafeMarkup; -use Drupal\Core\Extension\Extension; use Drupal\Core\Form\FormStateInterface; use Drupal\Core\Routing\RouteMatchInterface; -use Drupal\taxonomy\Entity\Vocabulary; -use Symfony\Component\Routing\Exception\RouteNotFoundException; use Drupal\user\Entity\User; /** diff -u b/core/modules/forum/src/ForumUninstallValidator.php b/core/modules/forum/src/ForumUninstallValidator.php --- b/core/modules/forum/src/ForumUninstallValidator.php +++ b/core/modules/forum/src/ForumUninstallValidator.php @@ -70,19 +70,19 @@ $reasons = []; if ($module == 'forum') { if ($this->hasForumNodes()) { - $reasons[] = $this->t('To uninstall Forum, first delete all Forum content.'); + $reasons[] = $this->t('To uninstall Forum, first delete all Forum content'); } $vocabulary = $this->getForumVocabulary(); if ($this->hasTermsForVocabulary($vocabulary)) { if ($vocabulary->access('view')) { - $reasons[] = $this->t('To uninstall Forum, first delete all %vocabulary terms.', [ + $reasons[] = $this->t('To uninstall Forum, first delete all %vocabulary terms', [ '%vocabulary' => $vocabulary->label(), '!url' => $vocabulary->url('overview-form'), ]); } else { - $reasons[] = $this->t('To uninstall Forum, first delete all %vocabulary terms.', [ + $reasons[] = $this->t('To uninstall Forum, first delete all %vocabulary terms', [ '%vocabulary' => $vocabulary->label() ]); } diff -u b/core/modules/forum/src/Tests/ForumUninstallTest.php b/core/modules/forum/src/Tests/ForumUninstallTest.php --- b/core/modules/forum/src/Tests/ForumUninstallTest.php +++ b/core/modules/forum/src/Tests/ForumUninstallTest.php @@ -74,7 +74,7 @@ $this->drupalGet('admin/modules/uninstall'); // Assert forum is required. $this->assertNoFieldByName('uninstall[forum]'); - $this->assertText('To uninstall Forum, first delete all'); + $this->assertText('To uninstall Forum, first delete all Forum content'); // Delete the node. $this->drupalPostForm('node/' . $node->id() . '/delete', array(), t('Delete')); @@ -83,7 +83,7 @@ $this->drupalGet('admin/modules/uninstall'); // Assert forum is still required. $this->assertNoFieldByName('uninstall[forum]'); - $this->assertText('To uninstall Forum, first delete all'); + $this->assertText('To uninstall Forum, first delete all Forums terms'); // Delete any forum terms. $vid = $this->config('forum.settings')->get('vocabulary'); @@ -100,7 +100,6 @@ $this->drupalGet('admin/modules/uninstall'); // Assert forum is no longer required. $this->assertFieldByName('uninstall[forum]'); - $this->assertNoText('To uninstall Forum first delete all Forum content'); $this->drupalPostForm('admin/modules/uninstall', array( 'uninstall[forum]' => 1, ), t('Uninstall')); diff -u b/core/modules/forum/tests/src/Unit/ForumUninstallValidatorTest.php b/core/modules/forum/tests/src/Unit/ForumUninstallValidatorTest.php --- b/core/modules/forum/tests/src/Unit/ForumUninstallValidatorTest.php +++ b/core/modules/forum/tests/src/Unit/ForumUninstallValidatorTest.php @@ -92,7 +92,7 @@ $module = 'forum'; $expected = [ - 'To uninstall Forum, first delete all Forum content.', + 'To uninstall Forum, first delete all Forum content', ]; $reasons = $this->forumUninstallValidator->validate($module); $this->assertSame($expected, $reasons); @@ -126,8 +126,8 @@ $module = 'forum'; $expected = [ - 'To uninstall Forum, first delete all Forum content.', - SafeMarkup::format('To uninstall Forum, first delete all %vocabulary terms.', [ + 'To uninstall Forum, first delete all Forum content', + SafeMarkup::format('To uninstall Forum, first delete all %vocabulary terms', [ '!url' => '/path/to/vocabulary/overview', '%vocabulary' => 'Vocabulary label', ]), @@ -163,8 +163,8 @@ $module = 'forum'; $expected = [ - 'To uninstall Forum, first delete all Forum content.', - SafeMarkup::format('To uninstall Forum, first delete all %vocabulary terms.', [ + 'To uninstall Forum, first delete all Forum content', + SafeMarkup::format('To uninstall Forum, first delete all %vocabulary terms', [ '%vocabulary' => 'Vocabulary label', ]), ]; @@ -200,7 +200,7 @@ $module = 'forum'; $expected = [ - SafeMarkup::format('To uninstall Forum, first delete all %vocabulary terms.', [ + SafeMarkup::format('To uninstall Forum, first delete all %vocabulary terms', [ '!url' => '/path/to/vocabulary/overview', '%vocabulary' => 'Vocabulary label', ]), @@ -236,7 +236,7 @@ $module = 'forum'; $expected = [ - SafeMarkup::format('To uninstall Forum, first delete all %vocabulary terms.', [ + SafeMarkup::format('To uninstall Forum, first delete all %vocabulary terms', [ '%vocabulary' => 'Vocabulary label', ]), ]; diff -u b/core/modules/system/src/Tests/Module/DependencyTest.php b/core/modules/system/src/Tests/Module/DependencyTest.php --- b/core/modules/system/src/Tests/Module/DependencyTest.php +++ b/core/modules/system/src/Tests/Module/DependencyTest.php @@ -168,7 +168,8 @@ // Check that the comment module cannot be uninstalled. $this->drupalGet('admin/modules/uninstall'); - $this->assertNoFieldByName('uninstall[checkbox]'); + $checkbox = $this->xpath('//input[@type="checkbox" and @name="uninstall[comment]" and @disabled="disabled"]'); + $this->assert(count($checkbox) == 1, 'Checkbox for uninstalling the comment module is disabled.'); // Delete any forum terms. $vid = $this->config('forum.settings')->get('vocabulary'); diff -u b/core/modules/system/system.admin.inc b/core/modules/system/system.admin.inc --- b/core/modules/system/system.admin.inc +++ b/core/modules/system/system.admin.inc @@ -310,9 +310,9 @@ } if (!empty($form['modules'][$module]['#validation_reasons'])) { $disabled_message = \Drupal::translation()->formatPlural(count($form['modules'][$module]['#validation_reasons']), - 'The following reason prevents @module from being uninstalled: !reasons', - 'The following reasons prevents @module from being uninstalled: !reasons', - array('@module' => $form['modules'][$module]['#module_name'], '!reasons' => implode('; ', $form['modules'][$module]['#validation_reasons']))); + 'The following reason prevents @module from being uninstalled: !reasons.', + 'The following reasons prevents @module from being uninstalled: !reasons.', + array('@module' => $form['modules'][$module]['#module_name'], '!reasons' => SafeMarkup::checkAdminXss(implode('; ', $form['modules'][$module]['#validation_reasons'])))); } $rows[] = array( array('data' => drupal_render($form['uninstall'][$module]), 'align' => 'center'), diff -u b/core/modules/system/tests/modules/module_test/module_test.info.yml b/core/modules/system/tests/modules/module_required_test/module_required_test.info.yml --- b/core/modules/system/tests/modules/module_test/module_test.info.yml +++ b/core/modules/system/tests/modules/module_required_test/module_required_test.info.yml @@ -1,6 +1,11 @@ -name: 'Module test' +name: 'Module required test' type: module description: 'Support module for module system testing.' package: Testing version: VERSION core: 8.x +# Depends on the Node module to test making a module required using +# hook_system_info_alter() and ensuring that its dependencies also become +# required. +dependencies: + - drupal:node (>=8.x) diff -u b/core/profiles/standard/src/Tests/StandardTest.php b/core/profiles/standard/src/Tests/StandardTest.php --- b/core/profiles/standard/src/Tests/StandardTest.php +++ b/core/profiles/standard/src/Tests/StandardTest.php @@ -9,6 +9,7 @@ use Drupal\config\Tests\SchemaCheckTestTrait; use Drupal\contact\Entity\ContactForm; +use Drupal\filter\Entity\FilterFormat; use Drupal\simpletest\WebTestBase; use Drupal\user\Entity\Role; @@ -117,6 +118,25 @@ $this->assertConfigSchema($typed_config, $name, $config->get()); } + // Ensure that configuration from the Standard profile is not reused when + // enabling a module again since it contains configuration that can not be + // installed. For example, editor.editor.basic_html is editor configuration + // that depends on the ckeditor module. The ckeditor module can not be + // installed before the editor module since it depends on the editor module. + // The installer does not have this limitation since it ensures that all of + // the install profiles dependencies are installed before creating the + // editor configuration. + foreach (FilterFormat::loadMultiple() as $filter) { + // Ensure that editor can be uninstalled by removing use in filter + // formats. It is necessary to prime the filter collection before removing + // the filter. + $filter->filters(); + $filter->removeFilter('editor_file_reference'); + $filter->save(); + } + \Drupal::service('module_installer')->uninstall(array('editor', 'ckeditor')); + $this->rebuildContainer(); + \Drupal::service('module_installer')->install(array('editor')); /** @var \Drupal\contact\ContactFormInterface $contact_form */ $contact_form = ContactForm::load('feedback'); $recipients = $contact_form->getRecipients(); diff -u b/core/tests/Drupal/Tests/Core/Extension/RequiredModuleUninstallValidatorTest.php b/core/tests/Drupal/Tests/Core/Extension/RequiredModuleUninstallValidatorTest.php --- b/core/tests/Drupal/Tests/Core/Extension/RequiredModuleUninstallValidatorTest.php +++ b/core/tests/Drupal/Tests/Core/Extension/RequiredModuleUninstallValidatorTest.php @@ -72,7 +72,7 @@ ->method('getModuleInfoByModule') ->willReturn(['required' => TRUE, 'name' => $module]); - $expected = [SafeMarkup::format('The @module module is required.', ['@module' => $module])]; + $expected = [SafeMarkup::format('The @module module is required', ['@module' => $module])]; $reasons = $this->uninstallValidator->validate($module); $this->assertSame($expected, $reasons); } only in patch2: unchanged: --- a/core/lib/Drupal/Core/Extension/ModuleUninstallValidatorInterface.php +++ b/core/lib/Drupal/Core/Extension/ModuleUninstallValidatorInterface.php @@ -20,6 +20,10 @@ * * @return string[] * An array of reasons the module can not be uninstalled, empty if it can. + * Each reason should not end with any punctuation since multiple reasons + * can be displayed together. + * + * @see theme_system_modules_uninstall() */ public function validate($module); } only in patch2: unchanged: --- a/core/lib/Drupal/Core/Field/FieldModuleUninstallValidator.php +++ b/core/lib/Drupal/Core/Field/FieldModuleUninstallValidator.php @@ -53,7 +53,7 @@ public function validate($module_name) { if ($storage_definition->getProvider() == $module_name) { $storage = $this->entityManager->getStorage($entity_type_id); if ($storage instanceof FieldableEntityStorageInterface && $storage->countFieldData($storage_definition, TRUE)) { - $reasons[] = $this->t('There is data for the field @field-name on entity type @entity_type.', array( + $reasons[] = $this->t('There is data for the field @field-name on entity type @entity_type', array( '@field-name' => $storage_definition->getName(), '@entity_type' => $entity_type->getLabel(), ));