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.
Comment | File | Size | Author |
---|---|---|---|
#7 | uninstall_0.patch | 10.82 KB | webchick |
#5 | uninstall.patch | 10.96 KB | webchick |
#2 | uninstall_hook.patch.txt | 13.65 KB | neclimdul |
uninstall_hook.patch | 11.97 KB | webchick | |
Comments
Comment #1
Dries CreditAttribution: Dries commentedFirst review:
$retval
to something more descriptive.drupal_uninstall_module()
. From the looks, this value isn't needed or not used? If not used, remove for now.Comment #2
neclimdulStraight 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.
Comment #3
moshe weitzman CreditAttribution: moshe weitzman commentedpersonally, i think the confirm looks a little odd in this patch. i vote for the latter.
looking good.
Comment #4
Dries CreditAttribution: Dries commenteddrupal_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'. :)
Comment #5
webchickOk let's try this.
Comment #6
Dries CreditAttribution: Dries commentedThe 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?)
Comment #7
webchickYeah, good point. This removes the return value, and instead just calls the module's uninstall hook and updates the schema in the database.
Comment #8
Dries CreditAttribution: Dries commentedWorks for me.
Comment #9
drummWhere is the UI?
Comment #10
Dries CreditAttribution: Dries commentedThe 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.
Comment #11
drummThen I guess this is okay. The UI will need a big fat confirm screen.
Comment #12
drummCommitted to HEAD.
Comment #13
webchickThis 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).
Comment #14
(not verified) CreditAttribution: commented