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.
When you subscribe using comment_notify, it would be better for the subscription attached to a node, not a comment. Otherwise, if you participate in a conversation, it may be nearly impossible to unsubscribe, since you may be subscribed every time you comment.
Comment | File | Size | Author |
---|---|---|---|
#14 | 539214_disable_notifications_more_broadly.patch | 2.58 KB | greggles |
#12 | comment_notify_node.patch | 1001 bytes | burgs |
#7 | comment_notify_539214_1.patch | 886 bytes | premanup |
#5 | comment_notify_unsubscribe_should_be_by_node.patch | 916 bytes | premanup |
Comments
Comment #1
gregglesThe underlying bug is:
1. Post 2 comments on a node where you subscribe to notifications on both
2. A new comment is posted, you get an e-mail
3. Click the unsubscribe link
Expected results:
The user never gets e-mails again from this thread.
Actual results:
One of the comments is unsubscribed, but the other remains subscribed.
The proposed solution from OP would also help solve #288726: Allow users to subscribe without posting a comment.
An alternate solution would be to replace the cid in the has with the nid.
Comment #2
nico24 CreditAttribution: nico24 commentedHere's is a solution. Make your own module OWN_MODULE:
For the people who do like hacking in thirdparty modules you can just change comment_notify.module line 202:
db_query("UPDATE {comment_notify} SET notify = 0 WHERE notify_hash = '%s'", $hash);
to:
I don't know about the performance of this fix, but it should be fine. Easier would be if the nid and uid also get saved in the comment_notify table. But for now this works.
edit: I didn't check all issues to see if there was already a solution. So sorry if this is double.
Comment #3
rfay@nico24, thanks for your study of this problem. Please provide a patch that can improve this module and provide it for review here.
Comment #4
gregglesI think we should first disable the subscriptions for the specific hash.
Then, we can disable all subscriptions on that node that are "node level" subscriptions.
That should stick with what I consider to be expected behavior around the two subscription modes.
Comment #5
premanup CreditAttribution: premanup commented@nico24, thanks for your approach, performance is good, this is a patch for #2
worked quite well for me at first glance. Will report if I come accross something strange.
Comment #7
premanup CreditAttribution: premanup commentedsecond try
Comment #8
premanup CreditAttribution: premanup commented#5: comment_notify_unsubscribe_should_be_by_node.patch queued for re-testing.
Comment #10
gregglesLet's fix in 7.x first and then backport.
Comment #11
gregglesI just wanted to share that there is another issue - #1404990: More informative confirmation when unsubscribing which came about for a different reason but has a potential solution to this issue.
Comment #12
burgs CreditAttribution: burgs commentedA patch is attached that gets all the comment ids (cid) for the node in question, and disables notifications on all of those. It's an extra query, but at least it's SQL99 compliant and now D7. I'm sure it could use a config flag at some point for people who have nested comments and complex requirements, but I presume that most people will want it to work this way. It should apply nicely to 7.1 and/or 7.x-dev.
Comment #13
Henke001 CreditAttribution: Henke001 commentedWould it be possible to get this for Drupal 6 and node comments? Some of my forum users are getting a bit upset by not being able to unsubscribe. Thanks in advance!
Comment #14
gregglesThanks, burgs! This seems like decent progress, but this seems like it would give unexpected results for someone who used threading and subscribed to different sub-threads.
I'm changing the title to reflect that goal. New patch attached - anyone up for a review?
@Henke001 I appreciate your enthusiasm, but the best thing to move it forward is a patch review.
Comment #15
gregglesOk, now fixed http://drupalcode.org/project/comment_notify.git/commit/37be47c to get some testing of this code in the 7.x-1.x-dev branch.
Thanks, burgs!
Comment #16
burgs CreditAttribution: burgs commentedThanks greggles! That's working really well now :)
Comment #17
gregglesCool, thanks burgs. Note that this should stay in a fixed state and gets closed automatically after 2 weeks. That delay helps people know what has changed recently.