diff --git a/message_subscribe.module b/message_subscribe.module index da35358..f5747af 100755 --- a/message_subscribe.module +++ b/message_subscribe.module @@ -40,9 +40,15 @@ * worker. see message_subscribe_queue_worker(). * - "entity access": Determine if access to view the entity should be applied * when getting the list of subscribed users. Defaults to TRUE. + * - "notify blocked users": Determine whether blocked users + * should be notified. Typically this should be used in conjunction with + * "entity access" to ensure that blocked users don't receive notifications + * about entities which they used to have access to + * before they were blocked. Defaults to FALSE. * - "notify message owner": Determines if the user that created the entity * gets notified of their own action. If TRUE the author will get notified. * Defaults to FALSE. + * * @param $context * Optional; Array keyed with the entity type and array of entity IDs as * the value. For example, if the event is related to a node @@ -60,7 +66,7 @@ * @endcode * * @return Message - * The message object. + * The message object. */ function message_subscribe_send_message($entity_type, $entity, Message $message, $notify_options = array(), $subscribe_options = array(), $context = array()) { $use_queue = isset($subscribe_options['use queue']) ? $subscribe_options['use queue'] : variable_get('message_subscribe_use_queue', FALSE); @@ -77,6 +83,7 @@ function message_subscribe_send_message($entity_type, $entity, Message $message, 'use queue' => $use_queue, 'queue' => FALSE, 'entity access' => TRUE, + 'notify blocked users' => FALSE, 'notify message owner' => $notify_message_owner, ); @@ -212,26 +219,42 @@ function message_subscribe_get_subscribers($entity_type, $entity, Message $messa $notify_message_owner = isset($subscribe_options['notify message owner']) ? $subscribe_options['notify message owner'] : variable_get('message_subscribe_notify_own_actions', FALSE); $uids = array(); + // We don't use module_invoke_all() is we want to retain the array keys, // which are the user IDs. foreach (module_implements('message_subscribe_get_subscribers') as $module) { $function = $module . '_message_subscribe_get_subscribers'; $result = $function($message, $subscribe_options, $context); - foreach ($result as $uid => $values) { - // See if the author of the entity gets notified. - if (!$notify_message_owner && $message->uid == $uid) { - continue; - } + $uids += $result; + } - if (!empty($subscribe_options['entity access'])) { - $account = user_load($uid); - if (!entity_access('view', $entity_type, $entity, $account)) { - // User doesn't have access to view the entity. - continue; - } - } + // If we're not notifying blocked users, exclude those users from the result + // set now so that we avoid unnecessarily loading those users later. + if (empty($subscribe_options['notify blocked users']) && !empty($uids)) { + $query = new EntityFieldQuery(); + $results = $query + ->entityCondition('entity_type', 'user') + ->propertyCondition('status', 1) + ->propertyCondition('uid', array_keys($uids), 'IN') + ->execute(); + + if (!empty($results['user'])) { + $uids = array_intersect_key($uids, $results['user']); + } + } - $uids[$uid] = $values; + foreach ($uids as $uid => $values) { + // See if the author of the entity gets notified. + if (!$notify_message_owner && $entity->uid == $uid) { + unset($uids[$uid]); + } + + if (!empty($subscribe_options['entity access'])) { + $account = user_load($uid); + if (!entity_access('view', $entity_type, $entity, $account)) { + // User doesn't have access to view the entity. + unset($uids[$uid]); + } } } @@ -250,7 +273,6 @@ function message_subscribe_get_subscribers($entity_type, $entity, Message $messa return $uids; } - /** * Get context from entity type. * @@ -401,11 +423,17 @@ function message_subscribe_message_subscribe_get_subscribers(Message $message, $ $query->fields('fc') ->condition('fid', array_keys($fids), 'IN') - ->condition('uid', $subscribe_options['last uid'], '>') - ->orderBy('uid', 'ASC'); + ->condition('fc.uid', $subscribe_options['last uid'], '>') + ->orderBy('fc.uid', 'ASC'); if ($range) { $query->range(0, $range); + // If we're mpt notifying blocked users, we need to take this into account + // in this query so that the range is accurate. + if (empty($subscribe_options['notify blocked users'])) { + $query->join('users', 'users', 'users.uid = fc.uid'); + $query->condition('users.status', '1', '='); + } } $result = $query->execute(); diff --git a/message_subscribe.test b/message_subscribe.test index dc04f6a..c31f7f0 100644 --- a/message_subscribe.test +++ b/message_subscribe.test @@ -191,13 +191,19 @@ class MessageSubscribeSubscribersTest extends DrupalWebTestCase { 'flag subscribe_node', 'unflag subscribe_node', 'flag subscribe_user', - 'unflag subscribe_user' + 'unflag subscribe_user', )); $user2 = $this->drupalCreateUser(array( 'flag subscribe_node', 'unflag subscribe_node', 'flag subscribe_user', - 'unflag subscribe_user' + 'unflag subscribe_user', + )); + $user_blocked = $this->drupalCreateUser(array( + 'flag subscribe_node', + 'unflag subscribe_node', + 'flag subscribe_user', + 'unflag subscribe_user', )); // Create node. @@ -206,9 +212,10 @@ class MessageSubscribeSubscribersTest extends DrupalWebTestCase { $settings['uid'] = $user1->uid; $node = $this->drupalCreateNode($settings); - // User1 and User2 flags node1. + // User1, User2 and user_blocked flag node1 flag('flag', 'subscribe_node', $node->nid, $user1); flag('flag', 'subscribe_node', $node->nid, $user2); + flag('flag', 'subscribe_node', $node->nid, $user_blocked); // User2 flags User1. flag('flag', 'subscribe_user', $user1->uid, $user2); @@ -220,6 +227,11 @@ class MessageSubscribeSubscribersTest extends DrupalWebTestCase { $this->user1 = $user1; $this->user2 = $user2; + // $user_blocked is blocked in order to test + // $subscribe_options['notify blocked users']. + $user_blocked->status = 0; + user_save($user_blocked); + $this->user_blocked = $user_blocked; // Override default notifiers. variable_set('message_subscribe_default_notifiers', array()); } @@ -233,6 +245,7 @@ class MessageSubscribeSubscribersTest extends DrupalWebTestCase { $node = $this->node; $user2 = $this->user2; + $user_blocked = $this->user_blocked; $uids = message_subscribe_get_subscribers('node', $node, $message); // Assert subscribers data. @@ -248,17 +261,38 @@ class MessageSubscribeSubscribersTest extends DrupalWebTestCase { $this->assertEqual($uids, $expected_uids, 'All expected subscribers were fetched.'); + // Test notifying all users, including those who are blocked. + $subscribe_options['notify blocked users'] = TRUE; + $uids = message_subscribe_get_subscribers('node', $node, $message, $subscribe_options); + + $expected_uids = array( + $user2->uid => array( + 'notifiers' => array(), + 'flags' => array( + 'subscribe_node', + 'subscribe_user', + ), + ), + $user_blocked->uid => array( + 'notifiers' => array(), + 'flags' => array( + 'subscribe_node', + ), + ), + ); + $this->assertEqual($uids, $expected_uids, 'All expected subscribers were fetched, including blocked users.'); + $user3 = $this->drupalCreateUser(array( 'flag subscribe_node', 'unflag subscribe_node', 'flag subscribe_user', - 'unflag subscribe_user' + 'unflag subscribe_user', )); $user4 = $this->drupalCreateUser(array( 'flag subscribe_node', 'unflag subscribe_node', 'flag subscribe_user', - 'unflag subscribe_user' + 'unflag subscribe_user', )); flag('flag', 'subscribe_node', $node->nid, $user3); flag('flag', 'subscribe_node', $node->nid, $user4);