Move processing of comments to hook_exit

Seems reasonably. I personally won't work on it but I guess it could help.

Theoretically, this should be doable by adding something like this to function _comment_notify_mailalert:

function _comment_notify_mailalert($comment) {
  $comment = (object) $comment;
  
  //check if the comment is really published
  $result = db_query("SELECT status FROM {comments} WHERE cid = %d", $comment->cid);
  if (db_result($result) == 1) {
    //comment is not published, so don't give notifications
    return;
  }
  
  global $locale;
  global $base_url;
  // ...

I am trying this in a live website to see what happens.

Also, fixing this issue will also fix #229843: Getting emails about spam comments.

Nope, that one won't do. I recalled the method used for pingback.module and trackback.module. It turns out that you need to postpone the sending of emails to hook_exit(), because then every module work, including spam, akismet etc. would have been finished. I now use this patch, but I don't really know about the reliability.

I tested this on http://leon.info.tm and it works wonders. Spams do not trigger notifications when this patch is applied. See if somebody else can confirm this.

I upgrade comment_notify to 1.6, it still works. Do you consider to merge this patch into core?

The patch has a ton of debugging code left in it which should be cleaned up prior to it being committed.

Also, I believe that this module already uses the unofficial "publish" hook from a modified core to make sure it is not sending out "spam" messages. Can someone explain why this use of globals (which is not great, IMO, though often necessary) will fix that in a way that the current functioning can't?

Yep, this is the confusing part of the module. Actually I don't really understand why this module needs the "publish" hook, because trackback, pingback, spam modules etc. can work around the limitation using the above method.

I also don't understand why the publish hook doesn't work. My theory is that the akismet hooks get executed *after* the publish hook, so the notifications are sent *before* it is flagged as spam (and unpublished).

Well, #229843: Getting emails about spam comments is now marked as closed because Spam module has changed to better support things like this.

Given that it's been many weeks without an update to the code, and that there are alternatives (spam module, captcha, mollom) which work reasonably well, and that I don't like this solution much I'm marking this won't fix.

Um, I meant that #265205: Spam ruining comment notify is now marked as closed because...

Considering that the AntiSpam module is growing in usage and is the best way to integrate Akismet into Drupal, I would like to propose reopening this issue (or potentially discuss how to fix the issue with AntiSpam).

A user suggested some success using the hook_exit approach mentioned in #3, but I know you didn't like that solution 2 years ago.

I'd be willing to work on this issue, but I don't want to invest any time unless the maintainer is willing.

related issue: #598772: Give Antispam a lower weight in the system table (for better integration with other modules)

As far as I know, the Spam and Mollom modules prevent spam going out via comment notify. Couldn't the AntiSpam module do something similar?

To be more clear, I would be willing to change Comment Notify to make it easier to integrate the AntiSpam module as long as the changes are not drastic and are not specific to AntiSpam module.

i.e. if we need to provide more hooks in comment_notify that's fine by me.

i 2nd (or 3rd or 4th?) getting this fixed/resolved - having the same issue with using Comment Notify and the AntiSpam module..

Marking postponed until someone has a clear plan for how to fix this.

The answer may be in the AntiSpam module.

Re #4 (Leonth's patch). Tried it on 6.x-1.5. I had to manually patch things, and I've attached my alterations. But it didn't seem to work — my test anonymous subscriber still got an email notification of my test spam (despite it being marked as such).

I switched to Mollom, which works at least as well as AntiSpam, and doesn't have this problem (even if spam gets through).

I appreciate the effort here, but 3 years after the issue started we don't have a working patch and there are other solutions.

I believe comment_notify now has more support for comments being unpublished and then published which should make it possible for antispam to be effective with zero changes to comment_notify, so I'm marking this won't fix.

I'm testing the attached patch on a live site and so far everything appears to be working fine. There are no notifications being sent for spam/unpublished comments.

The issue could be related to this: http://drupal.org/node/1388082 as comment_notify uses the the comment hook. The patch uses hook_exit, similar to #15. With this patch, comment_notify works with the antispam module.

OK, on further review this seems like it makes sense for perceived performance and for the potential for better integration with other modules.

I would like this to be done in 7.x-1.x first and then backported, so marking needs work and assigning to that branch for now.

on the Mollom side of this issue I am putting a few notes on a bug thread together here: #1708536: Control over mollom notify triggering upon Mollom comment approval