Prevent multiple notifications when comments are edited

This would also be useful to me: subscribers' mailboxes are flooded when someone decides to edit their comment multiple times. I'll work on a patch!

Looking at the 6.x-2.0 version of Notifications, on line 714 of notifications_content/notifications_content.module states that, "For 'publish' operations we notify if the comment was not published before." However, this seems to be incorrect, as my site is sending notifications for comments that were edited after already being published.

Anyone have any thoughts on this issue? I think my next step is to test comment creation and see if the notifications_comment_status variable is being set properly.

In notifications_content/notifications_content.module:

  switch ($form_id) {
    case 'comment_form':
      // Load the node which is possibly cached to get the node type
      $node = node_load($form['nid']['#value']);
      //drupal_set_message(print_r($node));
      if (notifications_content_type_enabled($node->type)) {
        if (notifications_event_enabled('node', 'comment')) {
          _notifications_content_add_disable_field($form);
        }
        // If editing the comment, add values to remember
        if (!empty($form['cid']['#value']) && !empty($form['admin']['status'])) {
          $form['notifications_comment_status'] = array('#type' => 'value', '#value' => $form['admin']['status']['#default_value']);
        }
      }

The problem is that $form['admin']['status'] isn't being rendered on my site for non-admin users, so the notifications_comment_status variable is never set. Because it's never set, Notifications looks at every edit and thinks that it's a new comment. We don't want users to be able to see the publication option. Altering the function in the following manner fixed the problem on my site:

if (!empty($form['cid']['#value']) && !empty($node->status)) {
  $form['notifications_comment_status'] = array('#type' => 'value', '#value' => "");
}

I'm changing this back to a bug report. Patch is attached. I think this change could be applied to 6.x-4.0 versions, as well.

Glad this patch was useful. It's sort of a specific situation, but an annoying one for users!

If notifications are enabled for comments, when a user who does not have "Administer comments" permission edits a comment (which has already created a notification event), another notification event is created. This causes users to receive duplicate notifications for the same comment. If the original comment notification has not yet been sent to the user (i.e. it is queued), then both the original and the edit notification will contain exactly the same content. By design, the notifications module appears to agree that once a comment has been published, it shouldn't create another notification event if edited and saved.

But, it uses a value in the "Admin" fieldset to determine the published status of the comment (in the notifications_content_module), and this fieldset on comments is only displayed to users with the "Administer comments" permission. So, a user that does not have this permission will never see this form and $form['admin']['status'] does not appear in the $form array.

To fix this issue in the notifications module:

@mthomas - I've changed your solution slightly - I wasn't sure of the need to check the node status, so removed it. I also implicitly specify the value of 'notifications_comment_status' to be 0, instead of your empty value. This is how the default is set in comment.module (line 1271):

  if (!empty($edit['status'])) {
	$status = $edit['status'];
  }
  else {
	$status = 0;
  }

Not having the "Administer comments" permission means that the user will never be able to edit an unpublished node, so if they are editing it, I think it's safe to assume that it is already published, and set the default status to published, which is strangely 0 for comments (as opposed to 1).

So my solution is:

        // If editing the comment, set a flag to prevent duplicate comment notifications
        if (!empty($form['cid']['#value'])) {
          if (!empty($form['admin']['status'])) {
            // The user has 'Administer comments' permission and the comment admin fieldset 
            // will have been displayed so we can safely use the status value.
            $form['notifications_comment_status'] = array('#type' => 'value', '#value' => $form['admin']['status']['#default_value']);
          }
          else {
            // Set the default value to published because if a user without the 'Administer comments'
            // permission is editing, then it must be published.
            $form['notifications_comment_status'] = array('#type' => 'value', '#value' => 0);  
          }
        }
      }

This doesn't cater for the scenario of an admin user who unpublishes a comment, and then republishes - then the duplicate notification will be sent. But I think that's fine - probably won't happen very often, and hopefully they will have permission to check the 'don't send notifications for this post'...

Here's my patch for notifications 6.x-2.3. If this approach is acceptable, I'll create a patch for the 6.x-4.x-dev and 6.x-2.x-dev versions. I changed priority to Major because I think that duplicate notifications are a serious problem.

just carefully tested #8 with success on 6.x-2.3
thanks!

changing the status. not sure how many need to test before that happens.

Committed (with some changes).

Thanks.