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

fab65’s picture

fab65 commented

Tell me if you don't understand me please, I re-explain

jose reyero’s picture

jose reyero commented
Category: bug » support
Status: Active » Postponed (maintainer needs more info)

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

fab65’s picture

fab65 commented

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

fab65’s picture

fab65 commented
Status: Postponed (maintainer needs more info) » Needs review
filiptc’s picture

filiptc commented
Status: Needs review » Reviewed & tested by the community

Only users with permission to 'administer comments' have the issue. Patch works.

jose reyero’s picture

jose reyero commented

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.

jose reyero’s picture

jose reyero commented
Title: 2 notifications send for the same action » Redefine notifications and comment approval workflow (2 notifications sent for the same action)
Category: support » task

Some more related interesting notes from this other issue #333480: Notifications and comment approval workflow

I'm having difficulty with notifications and comment approval. Comments posted on my site (version 5.12) require moderator approval before they are published. When someone subscribes to my forum thread, they receive notifications when the author makes updates and when an administrator posts a comment; however, when someone else leaves a comment that requires approval, no notifications are sent out when it's published. I checked the message queue and no messages are created for delivery before or after comment approval.

So it seems we need to fully redefine how notifications/comment approval is working.

fab65’s picture

fab65 commented

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?

ryan_courtnage’s picture

ryan_courtnage commented

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.

fab65’s picture

fab65 commented

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.

jose reyero’s picture

jose reyero commented
Status: Reviewed & tested by the community » Active

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.

cglusky’s picture

cglusky commented

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

jose reyero’s picture

jose reyero commented
Title: Redefine notifications and comment approval workflow (2 notifications sent for the same action) » Redefine notifications and node/comment approval workflow
Priority: Normal » Critical

Updating title. We need to handle both node and comment approval.

jose reyero’s picture

jose reyero commented

We need also to make the difference between comment post and comment update (from #390242: Different events for comment post / comment update)

jose reyero’s picture

jose reyero commented
Status: Active » Fixed

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.

Status: Fixed » Closed (fixed)

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