Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
comment_notify is sending two copies of every notification
Comment | File | Size | Author |
---|---|---|---|
#4 | comment_notify-1278556.patch | 1.36 KB | moonray |
Comments
Comment #1
Joenet-dupe CreditAttribution: Joenet-dupe commentedSame Problem. It has been reported in #24 by the patch #14 here
Comment #2
moonray CreditAttribution: moonray commentedSame problem.
Tracked it down to the following 2 hooks sending emails:
comment_notify_comment_insert()
andcomment_notify_comment_publish()
One of the 2 hooks should be enough to get the email sent out.
Comment #3
moonray CreditAttribution: moonray commentedDid some more research...
Looks like the problem is the following: When
_comment_notify_mailalert()
does its thing it marks the comment as notified usingcomment_notify_mark_comment_as_notified()
. However, betweencomment_notify_comment_insert()
andcomment_notify_comment_publish()
the comment object is not reloaded from db, thus not triggeringcomment_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.
Comment #4
moonray CreditAttribution: moonray commentedAnd here's a patch that fixes this.
Comment #5
gregglesThanks for the research, moonray.
@joelko, @dusov can you test this?
Comment #6
gregglesI should have said that this looks sane to me :)
Comment #7
Wim LeersSubscribing. 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
.Comment #8
Vikom CreditAttribution: Vikom commentedSubscribing. Have the same problem, patch in #4 seems to have solved it.
Comment #9
gregglesI 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!
Comment #10
Wim LeersHurray! :)
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 :)
Comment #11
gregglesThe | 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.
Comment #12
Wim LeersThe latter I know (and use), the former I've never heard of. Where is this documented? :)
Comment #13
gregglesIt's not super documented. Dries and Angie did it for core for a while and then stopped doing it, I think.