The present version of the Notify module uses the following expression in hook_cron()
to compute whether to send out a notification:
if (
($send_start - $send_last > $send_interval)
&& (date('G', $send_start) >= $send_hour || $send_interval < 86400)
&& ($send_interval != -1)
)
{
_notify_send($send_start);
variable_set('notify_send_last', $send_start);
}
Transforming to pseudo-code, we get.
if (
(time_elapsed > interval)
&& (hour_of_last_send >= send_hour || interval < 24_hours)
&& (not_never)
) send_notification();
The problem is with this construct: hour_of_last_send >= send_hour
.
Let's say interval
is 24 hours and you want notifications at hour 23 (send_hour
). Let's say that for some reason hour_of_last_send
is 1 (i.e. the last notification was sent around hour 1).
Then hour_of_last_send >= send_hour
will never be true (and the or
clause will also be false if the interval is 24 hours or longer). Even a week later, nothing will have been be sent. (As a workaround until this is fixed set send_hour
to zero or your interval to less than 24 hours.)
I am proposing to replace this expression with a simple function to compute when the next notification should be sent. Below is a function that does exactly that:
/**
* Compute the next time a notification should be sent.
*
* @param int $send_last
* timestamp of last notification
*
* @return int
* -1 never, 0 send instantly, else next time to notify.
*/
function _notify_next_notificaton($send_last) {
$send_interval = variable_get('notify_send', 86400);
if ($send_interval < 0) {
return(-1);
}
$send_hour = variable_get('notify_send_hour', 0);
$next_last = $next_frst = $send_last + $send_interval;
if (REQUEST_TIME >= $next_frst) {
return(0);
}
if ($send_interval >= 86400) {
$next_last = strtotime(date('Y-m-d ', $next_frst) . $send_hour . ':00:00');
if ($next_last < REQUEST_TIME) $next_last += 86400;
}
return($next_last);
}
Then all we need to do in hook_cron()
is to compare the present with this timestamp, and notify when the present passes this time. Then we send the notification.
Comments
Comment #0.0
gisletypo
Comment #0.1
gisleWorkaround suggestion.
Comment #1
gisleThis bug is in the version for Drupal 6 as well.
Comment #1.0
gisleworkaround
Comment #1.1
gislemarkup
Comment #2
gisleI just commited a development snapshot for the Drupal 7 version that I think fixes this.
This bugfix need to be backported to Drupal 6, so I'm changing version.
Comment #3
gisleAnd reverting to unassigned (I am only working on the D7 port).
Comment #3.0
gislemarkup
Comment #4
AdamPS CreditAttribution: AdamPS commentedSo if I understand correctly, the bug is fixed in D7 and D6 is no longer supported. In which case can this issue safely be marked as "fixed"?
Comment #5
gisleYes. Or rather: "outdated".