Subscribe/Unsubscribe should be by node, not by comment if the subscription is to all 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.

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.

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

second try

#5: comment_notify_unsubscribe_should_be_by_node.patch queued for re-testing.

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.

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!

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.

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!

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

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.