Contents that get published based on schedule (using the scheduler module) set by the author do not get mailed out to the subsribers. The content was published before the schedule to notify is due, in other words, publishing scheduler executes before notification scheduler.

Remark: I noticed that there are quite a number of issues left untouched for weeks/months, I wonder if this project/module is still being maintained.

CommentFileSizeAuthor
#13 notify-scheduler.patch.txt1.44 KBkthagen
#8 notify.patch.txt421 byteskthagen

Comments

RobRoy’s picture

I recently took this project over and have just returned from vacation. I'll take a look at the scheduler issue.

Eagle-i’s picture

Is there any progress? My users who used it, do not receive any notifications anymore and are asking me about it.

RobRoy’s picture

So for this bug you have content that is scheduled to be published tomorrow, for example, but the content was created before Notify was installed/run so it is not catching it. I'd like to clear this up and I apologize for the delay. If anyone has some time, please feel free to submit a patch. If not, I will try to get to this soon.

Eagle-i’s picture

Update:

i discovered that my notify.module files were at zero (0) bytes on the server. No good upload. THat's fixed and should be working now. Sorry about that.

RobRoy’s picture

Hehe, cool. Can you confirm that the bug with scheduler still exists? I have a feeling it does.

Eagle-i’s picture

I am not sure what you mean with the scheduler, but i use notify module with poormanscron module and they seem to be working fine.

kthagen’s picture

I have had this situation occur under the following conditions:

1. Content is added with a scheduled publish date in the future.
2. The publish date is edited so that it is (significantly) earlier than the current time.
3. The next time cron triggers, the content is published, but there's no notification.

Looking at the code of notify.inc and scheduler.module, I think the problem is with scheduler, not notify.

When scheduler publishes a node, it sets the node's created and changed fields to publish_on. That means that if the difference between now and publish_on is greater than notify's period, notify will ignore the content, assuming that notifications have already been sent.

The fix should be to change the relevant update query in scheduler.module. I'll post a patch (not here--wrong project) once I've tested it.

kthagen’s picture

Status: Active » Needs review
StatusFileSize
new421 bytes

OK, more testing indicates that there is a fix necessary for notify. The modules in the cron chain are sorted first in terms of module weight, and then by name. Since "notify" comes before "schedule" and both have the default weight of 0, the notification routine will always run before schedule publishes the new nodes. Then, since the publication date is set to something before the time of the cron run, it will be outside of our window on the next invocation of notify.

The following patch to notify.install modifies the system table so that notify has a weight of 9. This cause notify to run after almost all the other modules have done their work. Schedule uses the default weight of 0, so a lower value would work too, but this will put it after other modules, like project, too. Run update.php after patching.

You still won't get a notification in the case I outlined in my last message, or in some cases if notify's frequency settings are less than the cron interval for schedule, but it should fix the ordinary case. (I've tested it, and this works for me.)

RobRoy’s picture

Status: Needs review » Needs work

Need a re-roll against HEAD.

- Should initialize $ret = array();
- Should put it in a $GLOBALS['db_type'] switch, even if we fall through all three db types mysql/mysqli/pgsql.
- Need Doxygen above update to explain what the reasoning is for the update.
- Should dynamically add +1 to whatever the current weight for publisher is since a user may have manually changed that to 10. If it's 0 then 0+1=1 is fine.

Thanks for all your hard work kthagen!

kthagen’s picture

Two points:

First, I'm not quite sure I follow what you mean by the second point. The update is standard SQL, so there's no need for a switch ($GLOBALS['db_type']) here. The general practice from the modules I've looked at, including the core system updates in database/updates.inc, seems to be to use that switch only if there's some mysql vs. postgresql difference.

Of course the same weight setting does need to be done in the _install, although again, I don't see why it should be in the switch.

Second, I think looking at scheduler's weight and picking the value from that has potential problems. A minor one is that if a user is going to reset the module weight manually after installing notify, the problem will recur. More importantly, if we give ourselves a weight of 1 (which is what will happen in the vast majority of installations), that's fine for scheduler, but what about some other module that publishes on the cron hook? It's easy to envision other modules, not yet in existence, that could assign themselves a weight >0.

I don't think the exact weight for notify matters much, but it should be fairly high. (I note that devel uses a weight of 88--perhaps less than that, but not by too much.)

I can have a new patch ready very quickly once I know your thinking on this, RobRoy.

RobRoy’s picture

The first point really is just my preference, mostly for the sake of being explicit and educational purposes for others looking at our install code. But, it does look a bit silly and core updates don't even do this so I'm alright with it either way.

  $ret = array();
  switch ($GLOBALS['db_type']) {
      case 'mysql':
      case 'mysqli':
      case 'pgsql':
        db_query('whatevs');
        break;
  }
 return $ret;

Yes, we should weight the module in hook_install() as well. Regarding the weighting, since we want to go AFTER scheduler, we really should be ensuring that. True, it's an edge case since I'm sure zero people have adjusted their module weights for scheduler. If there are other modules we want to go after, we should get those weights too and do a max(...) + 10 (this is a good point you brought up, to be on the safe side for any overlooked cron publishing modules).

So basically, neither of these things are super important. You can ignore the switch anad just hardcode the module weight to 10 or something and that'll be fine. But I slightly prefer the other way. My points #1 and #3 are the only definites.

Thanks.

RobRoy’s picture

On second thought, you're right on the switch stuff. It's just cruft to leave that in there. So leave the weighting in _install and _update_1 out of the switch like you said.

kthagen’s picture

Status: Needs work » Needs review
StatusFileSize
new1.44 KB

Here you go. The comments are a little chatty and might bear simplification.

RobRoy’s picture

Status: Needs review » Fixed

I committed this to 4.7 and up a while back, forgot to mark fixed. Thanks kthagen!

Anonymous’s picture

Status: Fixed » Closed (fixed)