comment_notify is sending two copies of every notification

Files: 
CommentFileSizeAuthor
#4 comment_notify-1278556.patch1.36 KBmoonray

Comments

Same Problem. It has been reported in #24 by the patch #14 here

Same problem.

Tracked it down to the following 2 hooks sending emails: comment_notify_comment_insert() and comment_notify_comment_publish()

One of the 2 hooks should be enough to get the email sent out.

Did some more research...

Looks like the problem is the following: When _comment_notify_mailalert() does its thing it marks the comment as notified using comment_notify_mark_comment_as_notified(). However, between comment_notify_comment_insert() and comment_notify_comment_publish() the comment object is not reloaded from db, thus not triggering comment_notify_comment_load() which populated the $comment->notified flag.

We need to update the comment object itself, in addition to writing the flag to db.

Status:Active» Needs review
StatusFileSize
new1.36 KB

And here's a patch that fixes this.

Thanks for the research, moonray.

@joelko, @dusov can you test this?

I should have said that this looks sane to me :)

Status:Needs review» Reviewed & tested by the community

Subscribing. Same problem. I thought I was doing something wrong :)

Tested the patch in #4. Works perfectly. Doesn't apply cleanly though, due to bad paths in the patch. That's easy enough for @greggles to deal with though: just use patch -p6 < patchfile.

Subscribing. Have the same problem, patch in #4 seems to have solved it.

Title:Two copies of notificationTwo copies of notification, mark as notified only updates DB, not passed object
Status:Reviewed & tested by the community» Fixed

I added some comments to explain why the change is necessary.

Now committed - http://drupalcode.org/project/comment_notify.git/commit/82bf62b

Thanks moonray and Wim Leers for the work and review!

Hurray! :)

P.S.: is that the new d.o standard notation for attributing commits? A pipe symbol instead of just a comma and/or "and"? Just asking out of curiosity :)

The | is to separate the reviewer from the author. There's discussion on more specific formats for that, but this works now so I use it :)

I also use the --author flag on the commit to give credit to the author.

The latter I know (and use), the former I've never heard of. Where is this documented? :)

It's not super documented. Dries and Angie did it for core for a while and then stopped doing it, I think.

Status:Fixed» Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.