The module doesn't implement hook_uninstall()

markus_petrux - October 4, 2009 - 08:47
Project:Vertical Tabs
Version:6.x-1.x-dev
Component:Code
Category:bug report
Priority:normal
Assigned:Dave Reid
Status:closed
Description

This can be solved from implementation of hook_uninstall(). For example:

<?php
/**
* Implementation of hook_uninstall().
*/
function vertical_tabs_uninstall() {
 
// Delete all module variables and then clear the variables cache.
  // Note: backslash used because _ is wildcard character in SQL.
 
db_query("DELETE FROM {variable} WHERE name LIKE 'vertical\_tabs\_%%'");
 
cache_clear_all('variables', 'cache');
}
?>

#1

kiamlaluno - October 25, 2009 - 23:39
Title:Module variables remain when module is disabled» The module doesn't implement hook_uninstall()
Version:6.x-1.0-beta4» 6.x-1.x-dev

This is still true for the latest development snapshot.

#2

Dave Reid - November 6, 2009 - 20:26
Assigned to:Anonymous» Dave Reid

Please provide a patch that uses variable_del() or I'll get this done tonight.

#3

Dave Reid - November 6, 2009 - 23:03
Status:active» fixed

Fixed in CVS. http://drupal.org/cvs?commit=285286

#4

markus_petrux - November 7, 2009 - 07:52
Status:fixed» active

@todo: hook_node_type() needs to deal with the case where a node type name is renamed.

Out of curiosity, any reason for not using DELETE LIKE in hook_uninstall(). I'm using this pattern on my modules, and now I'm concerned about there's something odd I'm missing?

#5

kiamlaluno - November 7, 2009 - 09:46

I'm using this pattern on my modules, and now I'm concerned about there's something odd I'm missing?

Using variable_del(), the code would execute two SQL queries, where one query deletes the content of the cache used for the Drupal variables. When you delete three Drupal variables in row, that query would be executed without any result (for the fact the cache has been already cleared when deleting the first Drupal variable). IMO, considering that the code is in an uninstallation function, it is safe to use an SQL query.

#6

Dave Reid - November 7, 2009 - 14:22
Status:active» fixed

1. Using variable_del for each variable in uninstall is the standard set by core (and still is in Drupal 7). You never know if someone were to create a module named vertical_tabs_better, whose variables would be deleted by this blanket SQL.
2. http://api.drupal.org/api/function/node_type_form_submit/6 actually renames variables itself. I know, strange huh? :) This is why core's comment_node_type does not do this as well. Try it! I dare you!

#7

markus_petrux - November 7, 2009 - 16:24

Both good points! Thanks! :)

#8

kiamlaluno - November 7, 2009 - 19:22

A little side note: vertical_tabs_uninstall() is reported to be an implementation of hook_install().

#9

Dave Reid - November 7, 2009 - 19:34

@KiamLaLuno: Hah, thanks. It'll get picked up with the next commit.

#10

hass - November 8, 2009 - 00:32

@Kiam: nodewords modules... hook_uninstall()... CNW.

#11

kiamlaluno - November 8, 2009 - 00:38

CNW?

#12

hass - November 8, 2009 - 11:30

As i often said - use variable_del() in .install files. Nodewords don't use it. Therefore nodewords needs work.

#13

kiamlaluno - November 8, 2009 - 12:51

As i often said - use variable_del() in .install files. Nodewords don't use it.

That is not anymore true.

#14

System Message - November 22, 2009 - 13:00
Status:fixed» closed

Automatically closed -- issue fixed for 2 weeks with no activity.

 
 

Drupal is a registered trademark of Dries Buytaert.