diff --git a/modules/field/field.api.php b/modules/field/field.api.php index 9c52d24..0790f0b 100644 --- a/modules/field/field.api.php +++ b/modules/field/field.api.php @@ -124,6 +124,10 @@ function hook_field_extra_fields_alter(&$info) { /** * Define Field API field types. * + * Note that fields and their instances should never be created in the same + * module that defines their field type, because disabling the module will mark + * the fields as inactive, preventing them from being removed on uninstall. + * * @return * An array whose keys are field type names and whose values are arrays * describing the field type, with the following key/value pairs: diff --git a/modules/field/field.crud.inc b/modules/field/field.crud.inc index 339e9c4..605564a 100644 --- a/modules/field/field.crud.inc +++ b/modules/field/field.crud.inc @@ -23,6 +23,10 @@ * This function does not bind the field to any bundle; use * field_create_instance() for that. * + * Note that fields and their instances should never be created in the same + * module that defines their field type, because disabling the module will mark + * the fields as inactive, preventing them from being removed on uninstall. + * * @param $field * A field definition array. The field_name and type properties are required. * Other properties, if omitted, will be given the following default values: @@ -415,6 +419,10 @@ function field_delete_field($field_name) { /** * Creates an instance of a field, binding it to a bundle. * + * Note that fields and their instances should never be created in the same + * module that defines their field type, because disabling the module will mark + * the fields as inactive, preventing them from being removed on uninstall. + * * @param $instance * A field instance definition array. The field_name, entity_type and * bundle properties are required. Other properties, if omitted, diff --git a/modules/field/field.module b/modules/field/field.module index 9e03c8d..290caca 100644 --- a/modules/field/field.module +++ b/modules/field/field.module @@ -306,13 +306,6 @@ define('FIELD_LOAD_REVISION', 'FIELD_LOAD_REVISION'); class FieldUpdateForbiddenException extends FieldException {} /** - * Implements hook_flush_caches(). - */ -function field_flush_caches() { - return array('cache_field'); -} - -/** * Implements hook_help(). */ function field_help($path, $arg) { @@ -386,45 +379,74 @@ function field_modules_uninstalled($modules) { } /** - * Implements hook_modules_enabled(). + * Implements hook_system_info_alter(). + * + * Goes through a list of all modules that provide a field type, and makes them + * required if there are any active fields of that type. */ -function field_modules_enabled($modules) { - foreach ($modules as $module) { - field_associate_fields($module); +function field_system_info_alter(&$info, $file, $type) { + if ($type == 'module' && module_hook($file->name, 'field_info')) { + $fields = field_read_fields(array('module' => $file->name), array('include_deleted' => TRUE)); + if ($fields) { + $info['required'] = TRUE; + + // Provide an explanation message (only mention pending deletions if there + // remains no actual, non-deleted fields) + $non_deleted = FALSE; + foreach ($fields as $field) { + if (empty($field['deleted'])) { + $non_deleted = TRUE; + break; + } + } + if ($non_deleted) { + $explanation = t('Field type(s) in use - see !link', array('!link' => l(t('Field list'), 'admin/reports/fields'))); + } + else { + $explanation = t('Fields pending deletion'); + } + $info['explanation'] = $explanation; + } } - field_cache_clear(); } + /** - * Implements hook_modules_disabled(). + * Implements hook_flush_caches(). */ -function field_modules_disabled($modules) { - // Track fields whose field type is being disabled. +function field_flush_caches() { + // Refresh the 'active' and 'storage_active' columns according to the current + // set of enabled modules. + $all_modules = system_rebuild_module_data(); + $modules = array(); + foreach ($all_modules as $module_name => $module) { + if ($module->status) { + $modules[] = $module_name; + field_associate_fields($module_name); + } + } db_update('field_config') ->fields(array('active' => 0)) - ->condition('module', $modules, 'IN') + ->condition('module', $modules, 'NOT IN') ->execute(); - - // Track fields whose storage backend is being disabled. db_update('field_config') ->fields(array('storage_active' => 0)) - ->condition('storage_module', $modules, 'IN') + ->condition('storage_module', $modules, 'NOT IN') ->execute(); - field_cache_clear(); + return array('cache_field'); } /** * Allows a module to update the database for fields and columns it controls. * - * @param string $module + * @param $module * The name of the module to update on. */ function field_associate_fields($module) { // Associate field types. - $field_types =(array) module_invoke($module, 'field_info'); + $field_types = (array) module_invoke($module, 'field_info'); foreach ($field_types as $name => $field_info) { - watchdog('field', 'Updating field type %type with module %module.', array('%type' => $name, '%module' => $module)); db_update('field_config') ->fields(array('module' => $module, 'active' => 1)) ->condition('type', $name) @@ -433,7 +455,6 @@ function field_associate_fields($module) { // Associate storage backends. $storage_types = (array) module_invoke($module, 'field_storage_info'); foreach ($storage_types as $name => $storage_info) { - watchdog('field', 'Updating field storage %type with module %module.', array('%type' => $name, '%module' => $module)); db_update('field_config') ->fields(array('storage_module' => $module, 'storage_active' => 1)) ->condition('storage_type', $name) diff --git a/modules/field/tests/field.test b/modules/field/tests/field.test index 9281273..8a24fc1 100644 --- a/modules/field/tests/field.test +++ b/modules/field/tests/field.test @@ -2356,6 +2356,7 @@ class FieldCrudTestCase extends FieldTestCase { $this->assertTrue($field_definition <= $field, t('The field was properly read.')); module_disable($modules, FALSE); + drupal_flush_all_caches(); $fields = field_read_fields(array('field_name' => $field_name), array('include_inactive' => TRUE)); $this->assertTrue(isset($fields[$field_name]) && $field_definition < $field, t('The field is properly read when explicitly fetching inactive fields.')); @@ -2368,6 +2369,7 @@ class FieldCrudTestCase extends FieldTestCase { $module = array_shift($modules); module_enable(array($module), FALSE); + drupal_flush_all_caches(); } // Check that the field is active again after all modules have been diff --git a/modules/field_ui/field_ui.admin.inc b/modules/field_ui/field_ui.admin.inc index 96beb13..db31004 100644 --- a/modules/field_ui/field_ui.admin.inc +++ b/modules/field_ui/field_ui.admin.inc @@ -12,17 +12,27 @@ function field_ui_fields_list() { $instances = field_info_instances(); $field_types = field_info_field_types(); $bundles = field_info_bundles(); + + $modules = system_rebuild_module_data(); + $header = array(t('Field name'), t('Field type'), t('Used in')); $rows = array(); foreach ($instances as $entity_type => $type_bundles) { foreach ($type_bundles as $bundle => $bundle_instances) { foreach ($bundle_instances as $field_name => $instance) { $field = field_info_field($field_name); + + // Initialize the row if we encounter the field for the first time. + if (!isset($rows[$field_name])) { + $rows[$field_name]['class'] = $field['locked'] ? array('menu-disabled') : array(''); + $rows[$field_name]['data'][0] = $field['locked'] ? t('@field_name (Locked)', array('@field_name' => $field_name)) : $field_name; + $module_name = $field_types[$field['type']]['module']; + $rows[$field_name]['data'][1] = $field_types[$field['type']]['label'] . ' ' . t('(module: !module)', array('!module' => $modules[$module_name]->info['name'])); + } + + // Add the current instance. $admin_path = _field_ui_bundle_admin_path($entity_type, $bundle); - $rows[$field_name]['data'][0] = $field['locked'] ? t('@field_name (Locked)', array('@field_name' => $field_name)) : $field_name; - $rows[$field_name]['data'][1] = $field_types[$field['type']]['label']; $rows[$field_name]['data'][2][] = $admin_path ? l($bundles[$entity_type][$bundle]['label'], $admin_path . '/fields') : $bundles[$entity_type][$bundle]['label']; - $rows[$field_name]['class'] = $field['locked'] ? array('menu-disabled') : array(''); } } } diff --git a/modules/forum/forum.test b/modules/forum/forum.test index 1dc45c6..d59ed44 100644 --- a/modules/forum/forum.test +++ b/modules/forum/forum.test @@ -31,6 +31,7 @@ class ForumTestCase extends DrupalWebTestCase { // Create users. $this->admin_user = $this->drupalCreateUser(array( 'access administration pages', + 'administer modules', 'administer blocks', 'administer forums', 'administer menu', @@ -52,6 +53,30 @@ class ForumTestCase extends DrupalWebTestCase { } /** + * Tests disabling and re-enabling forum. + */ + function testEnableForumField() { + $this->drupalLogin($this->admin_user); + + // Disable the forum module. + $edit = array(); + $edit['modules[Core][forum][enable]'] = FALSE; + $this->drupalPost('admin/modules', $edit, t('Save configuration')); + $this->assertText(t('The configuration options have been saved.'), t('Modules status has been updated.')); + module_list(TRUE); + $this->assertFalse(module_exists('forum'), t('Forum module is not enabled.')); + + // Attempt to re-enable the forum module and ensure it does not try to + // recreate the taxonomy_forums field. + $edit = array(); + $edit['modules[Core][forum][enable]'] = 'forum'; + $this->drupalPost('admin/modules', $edit, t('Save configuration')); + $this->assertText(t('The configuration options have been saved.'), t('Modules status has been updated.')); + module_list(TRUE); + $this->assertTrue(module_exists('forum'), t('Forum module is enabled.')); + } + + /** * Login users, create forum nodes, and test forum functionality through the admin and user interfaces. */ function testForum() { diff --git a/modules/system/system.admin.inc b/modules/system/system.admin.inc index 9e7d69d..abb796c 100644 --- a/modules/system/system.admin.inc +++ b/modules/system/system.admin.inc @@ -799,7 +799,7 @@ function system_modules($form, $form_state = array()) { $extra['enabled'] = (bool) $module->status; if (!empty($module->info['required'] )) { $extra['disabled'] = TRUE; - $extra['required_by'][] = $distribution_name; + $extra['required_by'][] = $distribution_name . (!empty($module->info['explanation']) ? ' ('. $module->info['explanation'] .')' : ''); } // If this module requires other modules, add them to the array. diff --git a/modules/system/system.test b/modules/system/system.test index be4e366..d5bbcff 100644 --- a/modules/system/system.test +++ b/modules/system/system.test @@ -294,39 +294,6 @@ class ModuleDependencyTestCase extends ModuleTestCase { } /** - * Tests re-enabling forum with taxonomy disabled. - */ - function testEnableForumTaxonomyFieldDependency() { - // Enable the forum module. - $edit = array(); - $edit['modules[Core][forum][enable]'] = 'forum'; - $this->drupalPost('admin/modules', $edit, t('Save configuration')); - $this->assertModules(array('forum'), TRUE); - - // Disable the forum module. - $edit = array(); - $edit['modules[Core][forum][enable]'] = FALSE; - $this->drupalPost('admin/modules', $edit, t('Save configuration')); - $this->assertModules(array('forum'), FALSE); - - // Disable the taxonomy module. - $edit = array(); - $edit['modules[Core][taxonomy][enable]'] = FALSE; - $this->drupalPost('admin/modules', $edit, t('Save configuration')); - $this->assertModules(array('taxonomy'), FALSE); - - // Attempt to re-enable the forum module with taxonomy disabled and ensure - // forum does not try to recreate the taxonomy_forums field. - $edit = array(); - $edit['modules[Core][forum][enable]'] = 'forum'; - $this->drupalPost('admin/modules', $edit, t('Save configuration')); - $this->assertText(t('Some required modules must be enabled'), t('Dependency required.')); - $this->drupalPost(NULL, NULL, t('Continue')); - $this->assertText(t('The configuration options have been saved.'), t('Modules status has been updated.')); - $this->assertModules(array('taxonomy', 'forum'), TRUE); - } - - /** * Tests that module dependencies are enabled in the correct order via the * UI. Dependencies should be enabled before their dependents. */ @@ -364,18 +331,18 @@ class ModuleDependencyTestCase extends ModuleTestCase { $this->drupalPost('admin/modules', $edit, t('Save configuration')); $this->assertModules(array('forum'), TRUE); - // Disable forum and taxonomy. Both should now be installed but disabled. + // Disable forum and comment. Both should now be installed but disabled. $edit = array('modules[Core][forum][enable]' => FALSE); $this->drupalPost('admin/modules', $edit, t('Save configuration')); $this->assertModules(array('forum'), FALSE); - $edit = array('modules[Core][taxonomy][enable]' => FALSE); + $edit = array('modules[Core][comment][enable]' => FALSE); $this->drupalPost('admin/modules', $edit, t('Save configuration')); - $this->assertModules(array('taxonomy'), FALSE); + $this->assertModules(array('comment'), FALSE); // Check that the taxonomy module cannot be uninstalled. $this->drupalGet('admin/modules/uninstall'); - $checkbox = $this->xpath('//input[@type="checkbox" and @disabled="disabled" and @name="uninstall[taxonomy]"]'); - $this->assert(count($checkbox) == 1, t('Checkbox for uninstalling the taxonomy module is disabled.')); + $checkbox = $this->xpath('//input[@type="checkbox" and @disabled="disabled" and @name="uninstall[comment]"]'); + $this->assert(count($checkbox) == 1, t('Checkbox for uninstalling the comment module is disabled.')); // Uninstall the forum module, and check that taxonomy now can also be // uninstalled. @@ -383,7 +350,7 @@ class ModuleDependencyTestCase extends ModuleTestCase { $this->drupalPost('admin/modules/uninstall', $edit, t('Uninstall')); $this->drupalPost(NULL, NULL, t('Uninstall')); $this->assertText(t('The selected modules have been uninstalled.'), t('Modules status has been updated.')); - $edit = array('uninstall[taxonomy]' => 'taxonomy'); + $edit = array('uninstall[comment]' => 'comment'); $this->drupalPost('admin/modules/uninstall', $edit, t('Uninstall')); $this->drupalPost(NULL, NULL, t('Uninstall')); $this->assertText(t('The selected modules have been uninstalled.'), t('Modules status has been updated.'));