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.

Files: 
CommentFileSizeAuthor
#14 539214_disable_notifications_more_broadly.patch2.58 KBgreggles
#12 comment_notify_node.patch1001 bytesburgs
#7 comment_notify_539214_1.patch886 bytespremanup
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch comment_notify_539214_1.patch.
[ View ]
#5 comment_notify_unsubscribe_should_be_by_node.patch916 bytespremanup
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch comment_notify_unsubscribe_should_be_by_node.patch.
[ View ]

Comments

The 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.

Here's is a solution. Make your own module OWN_MODULE:

/**
* Alters the existing filter path to this custom function
*
* @param unknown_type $callbacks
*/
function OWN_MODULE_menu_alter(&$callbacks) {
$callbacks['comment_notify/disable/%']['page callback'] = 'OWN_MODULE_disable_page';
}
/**
* Page callback to allow users to unsubscribe simply by visiting the page.
*/
function OWN_MODULE_disable_page($hash) {
db_query("UPDATE {comment_notify} AS cn, (
SELECT c.cid
FROM {comments} c, (
SELECT oc.nid, oc.uid
FROM {comments} AS oc, {comment_notify} AS ocn
WHERE oc.cid = ocn.cid
AND ocn.notify_hash = '%s'
) AS o
WHERE o.nid = c.nid
AND o.uid = c.uid
) AS sc
SET cn.notify = 0
WHERE cn.cid = sc.cid", $hash);
  drupal_set_message(t('Your comment follow-up notification for this post was disabled. Thanks.'));
  return ' ';
}

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:

db_query("UPDATE {comment_notify} AS cn, (
SELECT c.cid
FROM {comments} c, (
SELECT oc.nid, oc.uid
FROM {comments} AS oc, {comment_notify} AS ocn
WHERE oc.cid = ocn.cid
AND ocn.notify_hash = '%s'
) AS o
WHERE o.nid = c.nid
AND o.uid = c.uid
) AS sc
SET cn.notify = 0
WHERE cn.cid = sc.cid", $hash);

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.

@nico24, thanks for your study of this problem. Please provide a patch that can improve this module and provide it for review here.

I 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.

Status:Needs work» Needs review
StatusFileSize
new916 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch comment_notify_unsubscribe_should_be_by_node.patch.
[ View ]

@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.

Status:Active» Needs work

The last submitted patch, comment_notify_unsubscribe_should_be_by_node.patch, failed testing.

Status:Needs review» Needs work
StatusFileSize
new886 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch comment_notify_539214_1.patch.
[ View ]

second try

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, comment_notify_539214_1.patch, failed testing.

Version:6.x-1.2» 7.x-1.x-dev

Let's fix in 7.x first and then backport.

I 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.

Status:Needs work» Needs review
StatusFileSize
new1001 bytes

A 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.

Would 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!

Title:Subscribe/Unsubscribe should be by node, not by commentSubscribe/Unsubscribe should be by node, not by comment if the subscription is to all comments
StatusFileSize
new2.58 KB

Thanks, 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.

Status:Needs review» Fixed

Ok, 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!

Status:Fixed» Closed (fixed)

Thanks greggles! That's working really well now :)

Status:Closed (fixed)» Fixed

Cool, 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.

Status:Fixed» Closed (fixed)

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