I've installed the previous -dev version (in December), and I've had to apply a bunch of patches to make it more or less work... I am comparing now the last -dev version (February 14, 2009) with it, and there are only a few changes, as below:

1. spam.module:

--- spam/spam.module.OLD	2008-12-25 06:42:09.000000000 +0100
+++ spam/spam.module		2009-02-13 17:36:05.000000000 +0100
@@ -1,5 +1,5 @@
 <?php
-// $Id: spam.module,v 1.51.4.1.2.41.2.30.2.1 2008/12/25 05:42:09 jeremy Exp $
+// $Id: spam.module,v 1.51.4.1.2.41.2.30.2.3 2009/02/13 16:36:05 jeremy Exp $
 
 /**
  * @file
@@ -1712,12 +1712,12 @@ function spam_links($type, $id, $content
  */
 function spam_mark_as_spam($type, $id, $extra = array()) {
   // TODO: Fix this loop
-  static $loop = FALSE;
-  if ($loop) {
+  static $loop = array();
+  if (isset($loop[$id])) {
     spam_log(SPAM_DEBUG, 'spam_mark_as_spam', t('FIX ME: looping'), $type, $id);
     return;
   }
-  $loop = TRUE;
+  $loop[$id] = TRUE;
 
   spam_update_statistics(t('@type marked as spam', array('@type' => $type)));
   $extra['sid'] = db_result(db_query("SELECT sid FROM {spam_tracker} WHERE content_type = '%s' AND content_id = %d", $type, $id));
@@ -1754,12 +1754,12 @@ function spam_mark_as_spam($type, $id, $
  */
 function spam_mark_as_not_spam($type, $id, $extra = array()) {
   // TODO: Fix this loop
-  static $loop = FALSE;
-  if ($loop) {
+  static $loop = array();
+  if (isset($loop[$id])) {
     spam_log(SPAM_DEBUG, 'spam_mark_as_not_spam', t('FIX ME: looping'), $type, $id);
     return;
   }
-  $loop = TRUE;
+  $loop[$id] = TRUE;
 
   spam_update_statistics(t('@type marked as not spam', array('@type' => $type)));
   $extra['sid'] = db_result(db_query("SELECT sid FROM {spam_tracker} WHERE content_type = '%s' AND content_id = %d", $type, $id));
@@ -2084,6 +2084,7 @@ function spam_range($low, $high, $step =
 function spam_unpublish($type, $id, $extra = array()) {
   spam_log(SPAM_VERBOSE, 'spam_unpublish', t('unpublished'), $type, $id);
   spam_invoke_module($type, 'unpublish', $id, $extra);
+  cache_clear_all();
   spam_update_statistics(t('unpublish @type', array('@type' => $type)));
 }
 
@@ -2094,5 +2095,6 @@ function spam_unpublish($type, $id, $ext
 function spam_publish($type, $id, $extra = array()) {
   spam_log(SPAM_VERBOSE, 'spam_publish', t('published'), $type, $id);
   spam_invoke_module($type, 'publish', $id, $extra);
+  cache_clear_all();
   spam_update_statistics(t('publish @type', array('@type' => $type)));
 }

2. spam_comment.inc:

--- spam/modules/spam_comment.inc.OLD	2008-12-25 06:42:11.000000000 +0100
+++ spam/modules/spam_comment.inc	2009-02-13 17:36:05.000000000 +0100
@@ -1,5 +1,5 @@
 <?php
-// $Id: spam_comment.inc,v 1.1.2.1.2.11.2.1 2008/12/25 05:42:11 jeremy Exp $
+// $Id: spam_comment.inc,v 1.1.2.1.2.11.2.2 2009/02/13 16:36:05 jeremy Exp $
 
 /**
  * @file
@@ -150,8 +150,6 @@ function comment_spamapi($op, $arg1 = NU
         return SPAM_NOT_PUBLISHED;
       }
 
-
-
     case 'hostname':
       return db_result(db_query('SELECT hostname FROM {comments} WHERE cid = %d', $arg1));
 

I wonder where are all other changes of code, among which there are some really necessary spam module at least to look like it works... ;-)

I am attaching all other patches (adapted to the last -dev version) which seems to make my installation a bit more comprehensive. It is however not easy to test how efficient spam module actually is (with or without those changes).

Please review the changes, and post some comments. Maybe some changes are not necessary (or not for any installation at least), but everything looks much better by my opinion (and it also hasn't caused any noticeable troubles since December / January on my production site, as applied to the previous -dev vesrion of spam module...).

Details about each particular change are all in different previously posted issues (I am sorry, but I can not track back any single change now...).

Comments

As If’s picture

Thank you for listing these all in one place.

naught101’s picture

For reference, it'd be good to have a list of links to the actual issues, and a quick description of what each patch does.

luti’s picture

naught101,

it'd be good to get a free beer as well... :-)

You are free to search within the issues of last few months, and post details... ;-)

naught101’s picture

LUTi, ah, I didn't realise what you'd done - I thought you'd just re-uploaded patches from other threads.

unfortunately, none of this stuff fixed any of my problems. Hopefully it'll help me avoid future ones though :)

[[EDIT]]: YES IT DID! thanks! importantly, you need to run update.php after patching

mcload’s picture

@LUTi
It looks like module developer is busy on other modules. Maybe you can apply for being co-maintainer or maintainer of this module to keep the module up-to-date and improve it.

luti’s picture

mcload,

I am not capable of being a developer / maintainer, sorry. I simply don't know enough about Drupal / PHP coding, unfortunately... :-(

mcload’s picture

Ok. Anyway, I hope somebody becomes a co-maintainer to keep this module up-to-date.

naught101’s picture

Status: Active » Reviewed & tested by the community

Dunno if this should really be set to reviewed and tested, since it's more of a meta-bug. But I have tested it :)

luti’s picture

It is declared as a support request, not as a bug report. ;-)

But, it is a solution for many bugs reported elsewhere, collected on one single place, which can easily be overlooked (that's why I've done it), considering there was a new version out after those bug reports, but neglecting them 100%, meaning it still contains all of these bugs...

franz-m’s picture

Feedback: the patches run successfully for me, and a lot is better now (eg. spam admin). Thanks a lot.

naught101’s picture

LUTi: I already marked a few of the bugs whose patches are included as "reviewed and tested", but I may have missed something, so maybe you could check through all the patches you applied, and marked them as tested?

luti’s picture

naught101,

as much as I've tested everything proposed in my initial post it seems to work, so regarding me "reviewed and tested" is OK. I only hope that it will be included in the next version of the module, not lost (again)... ;-)

By the way, I think it is not update.php that needs to be run (altough it doesn't hurt to do it...), it is just the cache cleaning part of it (which is also accessible through Admin -> Performance -> Clear Cached Data (a button on /admin/settings/performance).

It is generally a good habit to clean the cache after every site change (files or database), in Drupal (as already described) as well as in your browser. Can save you quite a headache in some cases... ;-)

obrigado’s picture

StatusFileSize
new47.56 KB

Here's the full module with all of the patches in the OP applied to the Feb 14 2009 dev build, 6.x-1.x-dev.

gnassar’s picture

Hi, guys,

I just signed on as co-maintainer.

I think I've found some of the specific bug fix issues that were addressed here, but I don't know that I've found them all. I'm sure that everyone would understand that with a module as big and complex as this, evaluating one huge patch (like here) would take a very long time compared to handling the issues individually. So it would be a help if the issues that were addressed here could be identified, or if at least you could drop a comment on the individual issue to push it to the top of the heap, as I'd like to deal with the ones that have been tested by the most people first.

Thanks!

naught101’s picture

Gnassar: pretty sure it's just all the patches that are marked "reviewed and tested by the community" -
Content type/user selection does not save in Drupal 6.10 http://drupal.org/node/429378
bad "Not Spam" link http://drupal.org/node/352179
admin/content/comment/list/spam page broken http://drupal.org/node/351280

I think they're the main killer bugs right now, and I would suggest applying those patches, and then comparing that to obrigado's patched module and seeing what's left. My feeling is that once those three patches are applied, a new dev release should go out.

As If’s picture

Well, it's reviewed and tested by me ;-) FWIW, I applied all of the changes in each patch *manually*, without even bothering to read the initial problem reports. Cleared the cache (good habit to have.) Everything worked perfectly after that.

luti’s picture

StatusFileSize
new46.26 KB
new19.83 KB

I've installed the new 6.x-1.x-dev version (2009-Jun-17), and found out the following issues:

1. Filter settings pages are missing; on my previous patched version, I've had 3 sub-tabs (under Filters) - "Overview" (that's the only page displayed now), "Custom" and "Duplicate" (accedd to those 2 pages is denied, if URLs are entered manually); consequently, it is not possible to enter custom filters and set duplicate filter parameters...

2. Pages /admin/content/comment and /admin/content/comment/spam still don't work as expected -"Mark as spam" and "Mark as not spam" are not working here; this was also the issue of my previous version, with the difference that at least there was no WSOD crash (as now when I select already marked comment to be marked as "not spam" - when I click Update button, WSOD comes up...)

It is interesting that links at the comment itself (mark as spam / mark as not spam) work as expected (in my version as well as in this new version now).

Comparing this new version to my previous one (diff is attached), I've also noticed that access arguments on some admin pages are not set to 'administer spam' (as in my previous version are). Shall it be like that? I mean, why is such a setting (in "Permissions" page), if it is not used after? Or, am I missing something here?

I am attaching also my working version (which I will not upgrade to the last -dev version, until that one will work at least as good as the existing one I have...), so maintainers can look at the differences and simply compare the behavior of each one (I've switched a couple of times between the new -dev version and my version without a problem - just don't forget to clear the cache, or use the /update.php page before starting to use the new version...).

gnassar’s picture

Status: Reviewed & tested by the community » Closed (fixed)

Thank you for troubleshooting. Feel free to establish issues for each of those that isn't already covered in an issue. But the "issue" of this thread is that patches were being neglected. That is, effectively, a closed problem now.

The version you have, with a combination of a collection of patches that you didn't write, would seem to itself have some bugs from looking at the code (and "fixes" problems that aren't problems). Applying N number of bugs at once and then troubleshooting them all at the same time is not a practical way to go about things. Progress on this continues in the individual issue queues, where the bugs will be fixed and development will continue. I'm closing this issue.