If you delete this, it causes all manner of problems, including the inability to post a forum, and boat-loads of these errors:

    * notice: Undefined index: in /Users/webchick/Sites/head/modules/taxonomy/taxonomy.module on line 859.
    * notice: Trying to get property of non-object in /Users/webchick/Sites/head/modules/forum/forum.module on line 167.
    * warning: in_array() [function.in-array]: Wrong datatype for second argument in /Users/webchick/Sites/head/modules/forum/forum.module on line 167.
CommentFileSizeAuthor
#6 forum_27.patch1.75 KBfgm
#2 forum-remove-delete-from-vocabulary-172633.patch432 byteschx
#1 forum-remove-delete-from-vocabulary-172633-1.patch23.71 KBwebchick

Comments

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Active » Needs review
StatusFileSize
new forum-remove-delete-from-vocabulary-172633-1.patch23.71 KB

Here's a patch that just unsets the delete button so the form can't be deleted.

chx’s picture

chx commented
StatusFileSize
new forum-remove-delete-from-vocabulary-172633.patch432 bytes

You posted a bit too much. Do not credit me for this, I just unrolled other modules from the patch.

fgm’s picture

fgm
Primary language French
Location Paris, France
commented
Status: Needs review » Needs work

This works, but I think one should go further. Consider this scenario:

  1. enable forum: a vocabulary is created and defined as the one to be used by forum
  2. at this point, the vocabulary can't be removed thanks to the patch
  3. disable forum: forum_nav_vocabulary remains in the variables table
  4. remove the vocabulary: this is no longer forgotten
  5. reenable forum: since it still believes it has a vocabulary, the errors are here again

I think a better patch would also care for hook_enable/hook_disable.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Needs work » Needs review

Thanks chx! :D

fgm, the only time we should be destructive with data is on uninstall. forum_uninstall() contains code to delete the vocabulary, and once it is run, the next time Forum is enabled, its forum_install() will run, which re-creates it.

It sounds like you're saying that we should account for people going into their databases and manually deleting the record from the variables table. If someone does that, they're on their own. :P

If we deleted the variable on hook_disable, how would we know which vocabulary was Forum's in hook_enable? The vocabulary could've been renamed to 'Spaghetti', and we can't just pick whatever vocabulary has forum topics in it, since a) in 6.x, any arbitrary node type can be added to a forum, and b) multiple vocabularies (free-tagging, "Category", etc.) might be added to those same posts.

Marking this back for review, unless I misunderstood your concern.

fgm’s picture

fgm
Primary language French
Location Paris, France
commented
Status: Needs review » Needs work

Webchick, maybe I wasn't clear enough, sorry.

The case I mentioned was entirely within the Drupal UI. I do not advocate deleting the vocabulary in the module, but described what seems a plausible situation:

  1. admin activates forum.module, which creates a vocabulary
  2. deciding against it, he disables it. the auto-created vocabulary remains set, as it probably should, for the reason you describe
  3. some time later, he finds the unused vocabulary, and removes it from admin/content/taxonomy/edit/vocabulary/2 because it *is* unused, to "clean up" his site. Since forum.module is not running, this is not prevented, because the patch is implemented within forum.module
  4. yet later, he wants to use forum.module again. But this time the module thinks it has a defined vocabulary, and does not handle the "missing vocabulary" case.
  5. user weeps, despairs, and abandons drupal

    6. My suggestion is therefore to care for this case of a well-meaning webmaster who inadvertently shoots himself in the foot all within the Drupal UI, and I think this could be done by implementing hook_enable, to recreate the module designated by the forum configuration variable.

fgm’s picture

fgm
Primary language French
Location Paris, France
commented
Status: Needs work » Needs review
StatusFileSize
new forum_27.patch1.75 KB

OK, here is a tentative patch for my suggestion

chx’s picture

chx commented
Status: Needs review » Closed (duplicate)

Enrolled your fixes into http://drupal.org/node/172643 .