The code to send an email to the node author checks 'node_notify_mailalert' which is never set. Additionally, this code checks agains the current user UID instead of the UID attached to the comment. The result is that this email is never actually sent.

The code to send an email to the commenters passes the current user instead of the user who will actually get the alert to the token module. As a result the email always has the wrong user information available as tokens.

The code that implements hook_tokens never actually returns the replacements. As a result, the unsubscribe URL never gets output properly.

Patch is ready, just posting this first to get the issue ID for the filename.

CommentFileSizeAuthor
#7 node-author-mailalert-bug-1184280-7.patch742 bytesrwohleb
#1 multiple-mailalert-bugs-1184280-1.patch1.47 KBrwohleb

Comments

rwohleb’s picture

rwohleb
he/him
Primary language English
commented
Priority: Normal » Major
Status: Active » Needs review
StatusFileSize
new multiple-mailalert-bugs-1184280-1.patch1.47 KB
rwohleb’s picture

rwohleb
he/him
Primary language English
commented

Oops, looks like my token fix in this patch is already being addressed in #952072: [comment:unsubscribe-url] is not working (and a few other token issues).

trillex’s picture

trillex commented

Fails when patching.


/sites/all/modules/comment_notify# git apply -v multiple-mailalert-bugs-1184280-1.patch
multiple-mailalert-bugs-1184280-1.patch:31: trailing whitespace.

Checking patch comment_notify.module...
error: while searching for:
      );

      foreach ($raw_values as $k => $v) {
        $message[$k] = token_replace(t($v), array('comment' => $comment, 'node' => $node, 'user' => $user));
      }

      drupal_mail('comment_notify', 'comment_notify_mail', $mail, $language, $message);

error: patch failed: comment_notify.module:483
error: comment_notify.module: patch does not apply
Checking patch comment_notify.tokens.inc...
rwohleb’s picture

rwohleb
he/him
Primary language English
commented

The patch was made from a git clone of the 7.x-1.x branch yesterday. I used the instructions found at http://drupal.org/node/23158/git-instructions for this module.

rwohleb’s picture

rwohleb
he/him
Primary language English
commented
Title: Multiple bugs in _comment_notify_mailalert() and token handling » Bug in node author handling in _comment_notify_mailalert()

I need to re-roll this patch to remove the token fixes as that is being handled in #952072: [comment:unsubscribe-url] is not working (and a few other token issues). I'm changing the title to reflect this just as an issue with the node author email handling.

rwohleb’s picture

rwohleb
he/him
Primary language English
commented
Status: Needs review » Needs work
rwohleb’s picture

rwohleb
he/him
Primary language English
commented
Status: Needs work » Needs review
StatusFileSize
new node-author-mailalert-bug-1184280-7.patch742 bytes

Here is the re-rolled patch.

David_Rothstein’s picture

David_Rothstein commented
Status: Needs review » Closed (duplicate)

Marking as a duplicate of #974338: Undefined property 'node_notify_mailalert', and node author never gets a notification since that issue is older and already has a patch that's ready to go and which solves the bug.

Regarding this patch, I don't think it necessarily make sense to switch $user->uid to $comment->uid... because if an administrator is the node author and is editing someone else's comment (e.g. to publish/approve it) they probably don't want to get an email about it, right?

rwohleb’s picture

rwohleb
he/him
Primary language English
commented

David_Rothstein, I understand what you mean about admins, but I think that leaving it as a check against $user is counter-intuitive. If I were to add/modify a comment programmatically, then I would expect it to operate around the comment author rather than the current user. Correct me if I'm wrong, but as it is now if a rule/trigger/action is fired off under the current user, it will look at THAT user as the current commenter even if they aren't. For consistency reasons, I'd say that changing it makes more sense.

David_Rothstein’s picture

David_Rothstein commented

Interesting point, though if a comment is being edited programmatically I would think either the node author should always be notified or never at all (depending on what kind of edit it is); i.e., even if they are the comment author, if they didn't type in the changes themselves they might want to know about it, unless it is some kind of unimportant bulk edit.

The problem there is that Drupal doesn't really have a great way to distinguish between programmatic updates and others.