When spam filters are disabled in the Drupal module listing, they remain in the spam_filters table and continue to show up in the filter list.

Also, filter installation/uninstallation should be properly encapsulated within their individual .install files.

CommentFileSizeAuthor
#2 spam-6.x-remove_system_disabled_modules.patch1.38 KBAlexisWilke

Comments

AlexisWilke’s picture

AlexisWilke commented
Assigned: Unassigned » AlexisWilke

Indeed, I'll get a patch ready soon to palliate to that problem.

Thank you for the report.
Alexis

AlexisWilke’s picture

AlexisWilke commented
StatusFileSize
new spam-6.x-remove_system_disabled_modules.patch1.38 KB

Okay, there is a patch for that problem. However...

The spam_filters table will only be updated whenever the system calls spam_admin_filter_overview() and that's probably not good. At this time I did not look at the way the spam_filters table is being used, but I am under the impression that it could cause problems in other places.

This being said, I just noticed that the Enable flag available in that table is actually very much the same as the Enable flag in the Modules installation page. Just saying. 8-)

Thank you.
Alexis Wilke

gnassar’s picture

gnassar commented

That's why there was a second part to the issue request; I think that addresses your concern.

"Proper encapsulation of install/uninstallation within the .install file" -- what I mean by that is, the install and uninstall process and all its necessary actions should be happening within the _install and _uninstall calls in the .install file, not in _admin_filter_overview, I think.

AlexisWilke’s picture

AlexisWilke commented

My concern with having code in the hook_[un]install() functions is that many people miss it when they implement their own sub-module. Yet, if it is in the API definition, I guess we'll be okay.

However, I definitively think that it would be a lot better there than the way it is now.

gnassar’s picture

gnassar commented

I agree that we wouldn't want people to leave that out when implementing submodules. I think that's helped by the spam_install_filter() function in spam.module -- right now, it's called by spam_init_filters() when it checks for new filters (as you've seen), but there's no reason that we can't have the .install file's _install hook call spam_install_filter() to do all the stuff that's standard for a submodule, and just make that one function (and the definition of the parameters it needs) a requirement in submodules defined in the API. We could do the same for _uninstall too, I imagine.