Download & Extend

Cannot uninstall comment when node is disabled.

Project:Drupal core
Version:8.x-dev
Component:comment.module
Category:bug report
Priority:normal
Assigned:Unassigned
Status:needs work

Issue Summary

When running aka testing #375397: Make Node module optional

drush site-install --yes
drush pm-disable node comment --yes
drush pm-uninstall comment --yes

the last command fails with
PHP Fatal error:  Call to undefined function node_type_get_types() in /Users/clemens/Sites/drupal/d8/www/core/modules/comment/comment.install on line 17

Comments

#1

In hook_uninstall() the module cleans up variables like this:

  $node_types = array_keys(node_type_get_types());
  foreach ($node_types as $node_type) {
    field_attach_delete_bundle('comment', 'comment_node_' . $node_type);
    variable_del('comment_' . $node_type);
    variable_del('comment_anonymous_' . $node_type);
    variable_del('comment_controls_' . $node_type);
    variable_del('comment_default_mode_' . $node_type);
    variable_del('comment_default_order_' . $node_type);
    variable_del('comment_default_per_page_' . $node_type);
    variable_del('comment_form_location_' . $node_type);
    variable_del('comment_preview_' . $node_type);
    variable_del('comment_subject_field_' . $node_type);
  }

A lot of comment modules variables are postfixed with node types, so what do we do? Let node module clean them up? Leave them? Delete based on the first part of the variable name?

#2

Come to think of it - if the variables are going in CMI-config files at some point, then this issue more or less solves it self.

#3

Status:active» needs review

Here's a test.

AttachmentSizeStatusTest resultOperations
comment-1426062-3.patch1.31 KBIdleFAILED: [[SimpleTest]]: [MySQL] 42,194 pass(es), 1 fail(s), and 2 exception(s).View details | Re-test

#4

Status:needs review» needs work

The last submitted patch, comment-1426062-3.patch, failed testing.

#5

As of the latest patch in #731724: Decouple comment.module from node by turning comment settings into a field, this would be obsolete. Still worth fixing in case that doesn't land.