Posted by fab65 on July 21, 2008 at 10:46am
6 followers
Jump to:
| Project: | Notifications |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | task |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
Hello,
I have got an issue. It's when I add a comment on a node. There is two notif which are send. I explain with more details :
An user A is subscribe to content type Doc (I have create this content type).An user B create a node of this type content. One notif is send to user A (it's ok).
Now, if user B add a comment, there are 2 notifications send to user A. But if it's the user A who add a comment, only one notification is send to user A.
I don't understand why!!!
I hope that someone can help me.
Thank you
Comments
#1
Tell me if you don't understand me please, I re-explain
#2
We'd need to know a bit more about which notifications plug-ins you're using and which subscriptions you have enabled (preferably a dump of the notifications and notifications_fields table for these two users)
There are two many different ways for a user to be subscribed to a comment: through the node, group, user, taxonomy, etc...
#3
I think i found the answer to my issue.
My issue come because when an user add a comment on a node, the function 'notifications_content_comment($comment, $op)' (line 380 in notifications_content.module) is call 2 times : because of 'insert' $op and 'publish' $op.
The condition "if" is valide the two times :
$comment = (array)$comment;if (!isset($comment['nomail']) && $comment['status'] == 0 && ($op == 'insert' || $op == 'update' || ($op == 'publish' && !user_access('administer comments')))) {
$event = array(
'module' => 'node',
'uid' => $comment['uid'],
'type' => 'node',
'action' => 'comment', // $op,
'node' => node_load($comment['nid']),
'comment' => (object)$comment,
'params' => array('nid' => $comment['nid'], 'cid' => $comment['cid']),
);
notifications_event($event);
}
Thus the function notifications_event($event) is call 2 times and there are two notifications send.
For the moment, for resolve the issue, I don't do action for the operation "publish" :
$comment = (array)$comment;
if (!isset($comment['nomail']) && $comment['status'] == 0 && ($op == 'insert' || $op == 'update')){// || ($op == 'publish' && !user_access('administer comments')))) {
$event = array(
'module' => 'node',
'uid' => $comment['uid'],
'type' => 'node',
'action' => 'comment', // $op,
'node' => node_load($comment['nid']),
'comment' => (object)$comment,
'params' => array('nid' => $comment['nid'], 'cid' => $comment['cid']),
);
notifications_event($event);
}
I don't know if it's a good idea but it's run
#4
#5
Only users with permission to 'administer comments' have the issue. Patch works.
#6
I see the patch may work, the use case is not that clear to me though.
I'm thinking it may make more sense to post the notification when the comment is published. However, administrators may want also to be notified when the comment is post and pending on the queue.
#7
Some more related interesting notes from this other issue #333480: Notifications and comment approval workflow
So it seems we need to fully redefine how notifications/comment approval is working.
#8
It's rigth, i'm agree with you. It's better to post the notification when the comment is published. The condition to notified must be review. Maybe it's not :
if (!isset($comment['nomail']) && $comment['status'] == 0 && ($op == 'insert' || $op == 'update' || ($op == 'publish' && !user_access('administer comments'))))but
if (!isset($comment['nomail']) && $comment['status'] == 0 && (($op == 'insert' && user_access('administer comments')) || ($op == 'update' && user_access('administer comments')) || ($op == 'publish' && !user_access('administer comments'))))What do you say that?
#9
Please see:
http://drupal.org/node/340082#comment-1130297
In my opinion notifications should be sent only when a comment is published ($op == published). We should not take any action with $op == 'insert', and should not be checking user_access.
#10
Thanks you for this link. I will use it. Can you tell me if you know when this issue will be delete into official version of notification's module ?
I just see the new release of notification's module (6.x.1-0) and there is always this issue.
#11
Applied the patch in #340082
It makes sense till we find a better solution.
This 'workflow' issue is still open though: Get administrators notified when the comment is posted / other users when the comment is published.
#12
Jose,
Please see if this makes sense in regards to covering more use cases without breaking others:
around line 555 of notifications_content.module
function notifications_content_comment($comment, $op) {
...
// $op == 'insert' && $comment->status == COMMENT_PUBLISHED covers the use case for allowing comments posted programatically without moderation (like via mail2web)
if ((($op == 'update' || $op == 'publish') && empty($comment->notifications_content_disable) && empty($comment->status)) || ($op == 'insert' && $comment->status == COMMENT_PUBLISHED)) {
...
If so I can role a patch.
R,
C
#13
Updating title. We need to handle both node and comment approval.
#14
We need also to make the difference between comment post and comment update (from #390242: Different events for comment post / comment update)
#15
Reworked node / comment events. See comments in code in:
- notifications_content_comment()
- notifications_content_nodeapi()
And maybe the most important, I've added some unit tests for this.
#16
Automatically closed -- issue fixed for 2 weeks with no activity.