Posted by rfay on August 3, 2009 at 11:15pm
| Project: | Comment notify |
| Version: | 7.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
Issue Summary
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.
Comments
#1
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.
#2
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.
#3
@nico24, thanks for your study of this problem. Please provide a patch that can improve this module and provide it for review here.
#4
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.
#5
@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.
#6
The last submitted patch, comment_notify_unsubscribe_should_be_by_node.patch, failed testing.
#7
second try
#8
#5: comment_notify_unsubscribe_should_be_by_node.patch queued for re-testing.
#9
The last submitted patch, comment_notify_539214_1.patch, failed testing.
#10
Let's fix in 7.x first and then backport.
#11
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.