Notifications shouldn't send mail to blocked users

subscribing
seems to apply to all blocked users, whether or not it is from the initial creation or not.
Would there be a way to set all of the user's notifications to "inactive" when a user is blocked?

Agreed - restating title to match problem.

Subscriptions should get blocked, that's done on notifications_user().

Anyway, I agree we may need an extra check when actually sending the notification.

Fixed for 4.x

http://drupalcode.org/viewvc/drupal/contributions/modules/notifications/...

Where in the code (6.x-4.x-beta6 and higher) are you checking for the user's status from the user table? I'd like to backport this code into my 2.3 version.

Also, I get setting any notifications that are in the queue to status = 0 and then I see in 4.x that their status will be turned back on should the user's status become active. Is there any time when Notifications will flush stale notifications that have status = 0? Seems like a lot of dead weight to be carrying around.

I was able to fix this by adding one line of code to notifications_process_rows in notifications.cron.inc.

Was:

// Build query and fetch rows from queue
$query = notifications_queue_query($conditions);
$sql = "SELECT * FROM {notifications_queue} ";
$sql .= " WHERE ". implode(' AND ', $query['where']);
$sql .= " ORDER BY module, uid, destination, send_method, send_interval";

Now:

// Build query and fetch rows from queue
$query = notifications_queue_query($conditions);
$sql = "SELECT * FROM {notifications_queue} ";
$sql .= " JOIN {users} ON notifications_queue.uid = users.uid AND users.status = 1";
$sql .= " WHERE ". implode(' AND ', $query['where']);
$sql .= " ORDER BY module, uid, destination, send_method, send_interval";

You'll noticed the only difference is that I added a join on the users table.

There was an error with the query that I suggested in #6 above. When the queue was being processed, the following error would occur:

user warning: Column 'uid' in where clause is ambiguous query: SELECT * FROM notifications_queue JOIN users ON notifications_queue.uid = users.uid AND users.status = 1 WHERE sqid <= 1813 AND uid = 26 AND module = 'notifications' AND send_method = 'mail' AND send_interval = 0 AND cron = 1 ORDER BY module, uid, destination, send_method, send_interval LIMIT 0, 1 in sites/all/modules/notifications/notifications.cron.inc on line 208.

The query has been corrected and is now:

// Build query and fetch rows from queue
$query = notifications_queue_query($conditions);
$sql = "SELECT * FROM {notifications_queue} ";
$sql .= " JOIN {users} ON notifications_queue.uid = users.uid AND users.status = 1";
$sql .= " WHERE ". implode(' AND ', $query['where']);
$sql .= " ORDER BY module, users.uid, destination, send_method, send_interval";

I also discovered a problem when trying to send a broadcast email to OG members. It required that I do a string replace on the "where" conditions to avoid ambiguity on uid. Just before the query code above, I have added the following line.

$query['where'] = str_replace('uid', 'users.uid', $query['where']);

Please add the fix to the D7 version as well.

Changing the last line to users.uid doesn't fix the error. I'm still getting

Column 'uid' in where clause is ambiguous query: SELECT * FROM notifications_queue JOIN users ON notifications_queue.uid = users.uid AND users.status = 1

Also just where exactly was the str_replace going?

Attached is a patch for D7. hook_user_update() was not being called, so when a user's status changed, their subscription status was not being updated. Patch fixes this. It does not check users' status when sending notifications, so after applying patch you will want to check your db for subscriptions that are still active for users who are blocked. DB query to fix existing subscriptions is UPDATE notifications_subscription SET STATUS = 0 WHERE uid IN( SELECT uid FROM users WHERE STATUS = 0 ).