diff --git a/mollom.admin.inc b/mollom.admin.inc index 091ba0b..cc106f2 100644 --- a/mollom.admin.inc +++ b/mollom.admin.inc @@ -26,27 +26,24 @@ function mollom_admin_form_list() { ); $result = db_query("SELECT form_id FROM {mollom_form}"); $forms = array(); - $modules = array(); + $module_info = array(); while ($form_id = db_result($result)) { $forms[$form_id] = mollom_form_load($form_id); - // Load module information. $module = $forms[$form_id]['module']; - if (!isset($modules[$module])) { + if (!isset($module_info[$module])) { // Default to the module's machine name in case the query below does not // lead to a result (which is possible, because D6 does not inform modules // about uninstalled modules). - $modules[$module] = $module; - $module_info = db_result(db_query("SELECT info FROM {system} WHERE type = 'module' AND name = '%s'", $module)); - if ($module_info) { - $module_info = unserialize($module_info); - $modules[$module] = t($module_info['name']); + $module_info[$module]['name'] = $module; + $system_info = db_result(db_query("SELECT info FROM {system} WHERE type = 'module' AND name = '%s'", $module)); + if ($system_info) { + $module_info[$module] = unserialize($system_info); } } - $forms[$form_id]['title'] = t('!module: !form-title', array( '!form-title' => $forms[$form_id]['title'], - '!module' => $modules[$module], + '!module' => t($module_info[$module]['name']), )); } @@ -55,21 +52,38 @@ function mollom_admin_form_list() { $rows = array(); foreach ($forms as $form_id => $mollom_form) { + $row_attributes = array(); $row = array(); $row[] = $mollom_form['title']; - if ($mollom_form['mode'] == MOLLOM_MODE_ANALYSIS) { - // @todo Output unsure mode in summary listing. - $row[] = t('!protection-mode (@discard)', array( - '!protection-mode' => $modes[$mollom_form['mode']], - '@discard' => $mollom_form['discard'] ? t('discard') : t('retain'), - )); + if (isset($modes[$mollom_form['mode']])) { + if ($mollom_form['mode'] == MOLLOM_MODE_ANALYSIS) { + // @todo Output unsure mode in summary listing. + $row[] = t('!protection-mode (@discard)', array( + '!protection-mode' => $modes[$mollom_form['mode']], + '@discard' => $mollom_form['discard'] ? t('discard') : t('retain'), + )); + } + else { + $row[] = $modes[$mollom_form['mode']]; + } + } + else { + $row[] = t('- orphan -'); + } + if (empty($mollom_form['orphan'])) { + $row[] = l(t('Configure'), 'admin/settings/mollom/manage/' . $form_id); } else { - $row[] = $modes[$mollom_form['mode']]; + $row[] = ''; + $row_attributes['class'] = 'error'; + drupal_set_message(t("%module module's %form_id form no longer exists.", array( + '%form_id' => $form_id, + '%module' => isset($module_info[$mollom_form['module']]['name']) ? t($module_info[$mollom_form['module']]['name']) : $mollom_form['module'], + )), 'warning'); } - $row[] = l(t('Configure'), 'admin/settings/mollom/manage/' . $form_id); $row[] = l(t('Unprotect'), 'admin/settings/mollom/unprotect/' . $form_id); - $rows[] = $row; + + $rows[] = $row_attributes + array('data' => $row); } // Add a row to add a form. diff --git a/mollom.module b/mollom.module index be1c7d7..23a1c2c 100644 --- a/mollom.module +++ b/mollom.module @@ -880,15 +880,13 @@ function mollom_form_list() { * The form id to return information for. * @param $module * The module name $form_id belongs to. + * @param array $form_list + * (optional) The return value of hook_mollom_form_list() of $module, if + * already kown. Primarily used by mollom_form_load(). */ -function mollom_form_info($form_id, $module) { - $form_info = module_invoke($module, 'mollom_form_info', $form_id); - if (empty($form_info)) { - $form_info = array(); - } - - // Ensure default properties. - $form_info += array( +function mollom_form_info($form_id, $module, $form_list = NULL) { + // Default properties. + $form_info = array( // Base properties. 'form_id' => $form_id, 'title' => $form_id, @@ -907,8 +905,31 @@ function mollom_form_info($form_id, $module) { 'elements' => array(), 'mapping' => array(), 'mail ids' => array(), + 'orphan' => TRUE, ); + // Fetch the basic form information from hook_mollom_form_list() first. + // This makes the integrating module (needlessly) rebuild all of its available + // forms, but the base properties are absolutely required here, so we can + // apply the default properties below. + if (!isset($form_list)) { + $form_list = module_invoke($module, 'mollom_form_list'); + } + // If it is not listed, then the form has vanished. + if (!isset($form_list[$form_id])) { + return $form_info; + } + $module_form_info = module_invoke($module, 'mollom_form_info', $form_id); + // If no form info exists, then the form has vanished. + if (!isset($module_form_info)) { + return $form_info; + } + unset($form_info['orphan']); + + // Any information in hook_mollom_form_info() overrides the list info. + $form_info = array_merge($form_info, $form_list[$form_id]); + $form_info = array_merge($form_info, $module_form_info); + // Allow modules to alter the default form information. drupal_alter('mollom_form_info', $form_info, $form_id); @@ -927,7 +948,7 @@ function mollom_form_new($form_id) { if (isset($form_list[$form_id])) { $mollom_form += $form_list[$form_id]; } - $mollom_form += mollom_form_info($form_id, $form_list[$form_id]['module']); + $mollom_form += mollom_form_info($form_id, $form_list[$form_id]['module'], $form_list); // Enable all fields for textual analysis by default. $mollom_form['checks'] = array('spam'); @@ -951,11 +972,9 @@ function mollom_form_load($form_id) { $mollom_form['enabled_fields'] = unserialize($mollom_form['enabled_fields']); // Attach form registry information. - $form_list = module_invoke($mollom_form['module'], 'mollom_form_list'); - if (isset($form_list[$form_id])) { - $mollom_form += $form_list[$form_id]; - } - $mollom_form += mollom_form_info($form_id, $mollom_form['module']); + $form_info = mollom_form_info($form_id, $mollom_form['module']); + $mollom_form += $form_info; + cache_set($cid, $mollom_form); } @@ -3380,7 +3399,9 @@ function node_mollom_form_info($form_id) { // Retrieve internal type from $form_id. $nodetype = drupal_substr($form_id, 0, -10); - $type = node_get_types('type', $nodetype); + if (!$type = node_get_types('type', $nodetype)) { + return; + } $form_info = array( // @todo This is incompatible with node access. 'bypass access' => array('administer nodes'), diff --git a/tests/mollom.test b/tests/mollom.test index deec455..4fc98af 100644 --- a/tests/mollom.test +++ b/tests/mollom.test @@ -2587,6 +2587,40 @@ class MollomFormConfigurationTestCase extends MollomWebTestCase { } /** + * Tests invalid (stale) form configurations. + */ + function testInvalidForms() { + $forms = array( + 'nonexisting' => 'nonexisting_form', + 'user' => 'user_nonexisting_form', + 'node' => 'nonexisting_node_form', + 'comment' => 'comment_node_nonexisting_form', + ); + $mode = 0; + foreach ($forms as $module => $form_id) { + $mollom_form = mollom_form_info($form_id, $module, array()); + $mollom_form['mode'] = $mode++; + mollom_form_save($mollom_form); + } + + // Just visiting the form administration page is sufficient; it will throw + // fatal errors, warnings, and notices. + $this->drupalLogin($this->admin_user); + $this->drupalGet('admin/settings/mollom'); + + // Ensure that unprotecting the forms does not throw any notices either. + foreach ($forms as $form_id) { + $this->assertNoLinkByHref('admin/settings/mollom/manage/' . $form_id); + $this->assertLinkByHref('admin/settings/mollom/unprotect/' . $form_id); + $this->drupalPost('admin/settings/mollom/unprotect/' . $form_id, array(), t('Confirm')); + $this->assertNoLinkByHref('admin/settings/mollom/unprotect/' . $form_id); + } + // Confirm deletion. + $count = db_result(db_query('SELECT 1 FROM {mollom_form}')); + $this->assertFalse($count, 'No forms found.'); + } + + /** * Tests programmatically, conditionally disabling Mollom. */ function testFormAlter() {