Email Notifications sent regardless of email checkboxes, revisited. bug in message_subscribe_email

Shouldn't this be placed in the Message subscribe module queue?

Wrong module

And here's the fix.

Shouldn't the flags IDs passed to fid already filter the non-user flags?

Not sure. This is my issue, and the patch fixes it.

What happens is that when a node is created in a group with say 2 members and 100 nodes, not only the 2 members get an e-mail, but also all users with the same UID as there are NID's in this group. So suppose there is a node with NID 5 in this group, the user with UID 5 gets an e-mail, while he is NOT part of this group.

Commons 3.4 (and before) is sending thousands of e-mails to users that are not part of the group, just because these users happen to have the same UID as the NID of the group where the node it is posted in.

I think a better solution would be to change in message_subscribe_email_message_subscribe_get_subscribers_alter()

$flags = message_subscribe_email_flag_get_flags();

to

$flags = message_subscribe_email_flag_get_flags('user');

Can you please check that?

Yes, that is indeed better. I've tested your proposal and that works as well.
Patch attached.

Hi WebSinPat,

I didn't think of that, and I assume you are correct, but it's up to Amitai to make sure.
Anyhow, I tried your code and my problem is still solved, so maybe your solution would be better.

I couldn't apply the patch though. This is how do it:

- Make a git clone of the latest code (https://drupal.org/project/message_subscribe/git-instructions)
- Paste in your code
- Do "git diff > name-of-my-patch.patch"

If you want to make a patch from a codebase that's already in git (eg; as part of a distro), you could use "git diff --relative > name-of-my-patch.patch"

Hi WebSinPat,

I got patch from #17 to apply, but it still did not solve the problem for me. I think there is one minor problem with your patch. Since the get_subscribers_alter hook passes in the uids by reference, your patch leaves all of the prior uids unchecked, which is not what we want, since we know that they will always send emails. I have re-rolled your patch with a minor modification to unset the notifiers of all uids at the beginning of the alter function, so that went we set them at the end of the function, they will only apply to the uids we want to send email to.

I have done some minimal testing with this, and it seems to produce the behavior that we're looking for.

Let's try this again. Instead of unsetting notifiers I just make them = array(). This passed testing locally.

Setting to correct status for test bot.
Haven't reviewed the code yet, but noticed there are some tabs (@dalinian you can use Dreditor to see those tabs)

Okay, I guess I should re-roll without tabs.

@dalinian, thanks -- can you add a simpleTest please to prevent regression.

I made a minor re-roll. @amitaibu - where do you want to write the tests? i'm a little confuse. Does message subscribe email module is the proper place to add the test?

> Does message subscribe email module is the proper place to add the test?

Yes.

Any updates here?

Yeah, i'm trying to get to this one.

Any updates? :)
Thanks for your work.

This was a published a long time ago so I need some more info. I tried to set this one up but the only i was able to send messages when creating a group content is to alter message_notify_example:

function message_notify_example_node_insert($node) {
  if ($node->type != 'group_content') {
    return;
  }

  $node_wrapper = entity_metadata_wrapper('node', $node);

  $message = message_create('example_og_post_in_group', array('uid' => $node->uid));
  $wrapper = entity_metadata_wrapper('message', $message);
  $wrapper->field_node_ref->set($node);
  $wrapper->field_node_groups_ref->set($node_wrapper->{OG_AUDIENCE_FIELD}->value(array('identifier' => TRUE)));
  message_subscribe_send_message('node', $node, $message);
}

If i'm wrong so far let me know but but except that the patch isn't worked for me. After disabling the emails for the group for demo(user2) he is the only one which receive emails. I'll try to see what going on here.

Link to the PR: https://github.com/Gizra/message_subscribe/pull/9

Sadly this patch expects two flags to exist and does not work otherwise. If a site is using Message Subscribe Email Frequency then only one flag is actually necessary.