Wrong message send for comments, when node belongs not to a group [#2088421]

If a node is commented and the node does not belong to group, the wrong message-type is used "commons_notify_node_created_no_groups".

This should be changed to "commons_notify_comment_created_no_groups" and this message type needs to be defined.

commons_notify.module:

/**
 * Implements hook_comment_insert().
 */
function commons_notify_comment_insert($comment) {
  $account = user_load($comment->uid);
  $node = node_load($comment->nid);
  $message_type = 'commons_notify_comment_created';
  $hook = 'comment_insert';
  if (!isset($node->og_group_ref[LANGUAGE_NONE][0]['target_id'])) {
    $message_type = 'commons_notify_node_created_no_groups';
  }

Comment	File	Size	Author
#8	2088421-commons_notify-comment-2.patch	9.57 KB	japerry
#2	commons_notify.patch	11.72 KB	slowflyer

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

ezra-g CreditAttribution: ezra-g commented 16 September 2013 at 20:45

Issue tags:

+Commons 7.x-3.4 radar

Thanks for this report! Are you able to help fix the problem by filing a patch?.

Adding to the Commons 3.4 radar.

Comment #3

ezra-g CreditAttribution: ezra-g commented 24 September 2013 at 18:47

Status:

Active

» Needs work

Thanks for the patch, slowflyer!

It appears that your patch contains a range of changes that aren't related to this issue. I think this hunk is the relevant one:

+++ b/profiles/commons/modules/contrib/commons_notify/commons_notify.module
@@ -76,7 +76,7 @@ function commons_notify_comment_insert($comment) {
   }

Marking as "needs work" though I can likely do some functional testing by making that change manually, since it appears like a single-line change.

Comment #4

slowflyer CreditAttribution: slowflyer commented 24 September 2013 at 19:29

Hi Ezra,

yes it's a single line of code in commons_notify.module + creating the new message type, which let the patch after recreating the feature appear really complex.

Comment #5

ezra-g CreditAttribution: ezra-g commented 26 September 2013 at 20:51

Status:

Needs work

» Needs review

Thanks for that explanation! I misunderstood the nature of the fix - Moving back to "needs review".

Comment #7

japerry

KVUO

CreditAttribution: japerry commented 14 November 2013 at 17:58

Assigned:

Unassigned

» japerry

Comment #8

japerry

KVUO

CreditAttribution: japerry commented 14 November 2013 at 18:45

File	Size
2088421-commons_notify-comment-2.patch	9.57 KB

1 file was hidden/shown/deleted

File	Size
commons_notify.patch	11.72 KB

Pushing a new patch thats compatible with 3.4. Still needs review.

Comment #9

japerry

KVUO

CreditAttribution: japerry commented 14 November 2013 at 19:38

Status:

Needs review

» Fixed

Needed a few more things like target nodes and comments. After I copied from the comments message notify, I tested comments on a non-group node and it tested out good.

Committed! http://drupalcode.org/project/commons.git/commit/93a4924

Comment #10

japerry

KVUO

CreditAttribution: japerry commented 27 November 2013 at 16:34

Status:	Fixed	» Needs work
Issue tags:	-commons 7.x-3.5 radar	+commons 7.x-3.6 radar

For some reason on Commons 3.5, this is not working again. Code is in there, but I think an .info line is missing. Also, node notifications has an attached comment field, which shouldn't exist.

Comment #11

ezra-g CreditAttribution: ezra-g commented 12 December 2013 at 17:24

I marked #2155449: New comment notifications use wrong template and #2118135: Comment on non-group sends wrong email (which also has a patch) as duplicates.

Comment #12

ezra-g CreditAttribution: ezra-g commented 17 December 2013 at 17:55

Priority:

Normal

» Major

Bumping to major given the number of reports we've had of this issue.

Comment #13

japerry

KVUO

CreditAttribution: japerry commented 19 December 2013 at 07:29

Status:

Needs work

» Fixed

Marking as fixed again. I committed the code to the info file here (sorry for the double issue post!)...

There appears to be other issues with email notifications being sent out, but at least this specific issue appears to be fixed in my testing. Feel free to re-open if you're seeing issues again.

http://drupalcode.org/project/commons.git/commit/151d778

Comment #14

ezra-g CreditAttribution: ezra-g commented 19 December 2013 at 15:53

Status:

Fixed

» Needs review

Looks like there's a lot of code that went into this commit that appears unrelated to this feature and as far as I can tell, didn't appear as a patch. @japerry, was that intentional? If so, can you share your thinking behind those changes?