Hopefully another in a series of patches from New module administration page with many new features, this one focuses on adding an uninstall hook to Drupal.

Rationale:
Now that modules are all in their own directories, all non "core" core modules (statistics, etc.) have had their tables encapsulated in .install files which are only executed when they're first enabled. The result is a nice, clean default install with very few tables. However, if you're just playing around to see what features do and then disabling them, the tables currently stay in the DB and clutter it up, along with the variables each module tosses into the variable table.

The solution is to add an "uninstall" hook which removes this stuff from the system when modules are uninstalled. Because dropping tables and variables is a destructive action, we don't want to do this on merely disabling a module (which you might do as a troubleshooting measure, or just to see what effect it has), but rather on an new "uninstall module" action through the UI.

Here's a partial patch which adds the uninstall hooks and drupal_uninstall_module function from the other patch. Missing is a UI and actual "code" to make this happen; not sure if I'll have a chance to work on that before the code freeze, so I would really appreciate someone else picking up the ball on this one. :)

Probably, admin/settings/modules turns into two tabs:

1. administer modules (the current admin/settings/modules screen)
2. uninstall modules

(This is based on Dries's feedback that tabs should be "actions")

We move "uninstall modules" to its own tab, because it's destructive and it would be bad to accidentally click on it.

On the "uninstall modules" tab, it lists any disabled modules and provides an "uninstall" link, which leads to a menu callback which will show a confirmation form, along with what data will be deleted if they proceed (currently, this is just a message like "all books" -- maybe also show a subset of the data?). If the delete/uninstall button is pressed, the module's uninstall hook fires.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Dries’s picture

First review:

  • For now, remove the $confirm stuff and show a generic message. (Some _uninstall functions seem to ignore the $confirm.)
  • Please rename $retval to something more descriptive.
  • Document the return value of drupal_uninstall_module(). From the looks, this value isn't needed or not used? If not used, remove for now.
neclimdul’s picture

FileSize
13.65 KB

Straight leg work patch. Kept everything as it was and implemented the changes requested by Dries.

I had to dig around to figure out how the $retval was suppose to work because the use of drupal_uninstall_module isn't provided in this patch but the parent patch. I think things would make more sense if we either fixed confirm to be obeyed by all hooks or implemented the confirm process prior to drupal_uninstall_module. If we did the later we could return the success of the uninstall rather than the inverse since we wont run into trouble with the string anymore.

If someone +1's either of those ideas I'm willing to do the leg work to fix it to work that way.

moshe weitzman’s picture

personally, i think the confirm looks a little odd in this patch. i vote for the latter.

looking good.

Dries’s picture

drupal_uninstall_module should not have any status reporting in it. That should be left to the calling function, IMO.
Otherwise looks good! Just take no more than five minutes to get this 'ready'. :)

webchick’s picture

Status: Needs work » Needs review
FileSize
10.96 KB

Ok let's try this.

Dries’s picture

Status: Needs review » Needs work

The return values won't work. Either remove the return value stuff from the _uninstall function, or fix all the uninstall hooks to return a proper value. (Will they ever return false?)

webchick’s picture

Status: Needs work » Needs review
FileSize
10.82 KB

Yeah, good point. This removes the return value, and instead just calls the module's uninstall hook and updates the schema in the database.

Dries’s picture

Status: Needs review » Reviewed & tested by the community

Works for me.

drumm’s picture

Where is the UI?

Dries’s picture

The UI can be added later as we don't need API changes/addition to add the UI. I'm OK with changing the ?q=admin/modules UI in the early stage of the code freeze, as long this does not involve changing or adding APIs.

drumm’s picture

Then I guess this is okay. The UI will need a big fat confirm screen.

drumm’s picture

Status: Reviewed & tested by the community » Fixed

Committed to HEAD.

webchick’s picture

This has been documented at updating your modules and in the install hook documentation (needs more fleshing out here, but that will happen when the UI gets completed).

Anonymous’s picture

Status: Fixed » Closed (fixed)