If you create several posts without "publishing" them yet, they still go into the queue. However, when cron runs, they are just deleted from the queue without being sent (presumably because they were not "published" yet.) Then, if you go and set them to be "published," they do not appear in the queue again, and never get sent to digest subscribers on the next cron run. Maybe the module should only add items to the queue after they are set into a "published" state, or there should be another table column to keep a "published state" entry?

I can work around this for the most part, but some subscriptions will be lost when the posts are not moderator-approved to be published in a timely manner.

UPDATE: If someone other than the superuser creates the post, the above is still true, except the super-user will get a correct digest of the posts just before they are deleted from the queue for everyone. If the superuser created the blog posts, he/she does not receive a digest and the cron run deletes them from the queue because they were not published.

Comments

salvis’s picture

salvis
he/his
commented
Status: Active » Needs work

Thank you for opening a new issue!

Yes, this is definitely a bug. I believe it happens only if users have notify On Updates turned off. If it's on, it seems to work, but of course it should work either way.

Users with the administer nodes permission should get the notifications whether the node is published or not (and a second time when it's being published, along with all the normal users — this is hard to avoid).

I'm confused that you see a difference depending on who writes the post. Are you really sure about that? Or have you maybe turned off getting notified about your own posts? Do you get them, when the node is created in published state?

gregarios’s picture

gregarios commented

I'm confused that you see a difference depending on who writes the post. Are you really sure about that? Or have you maybe turned off getting notified about your own posts? Do you get them, when the node is created in published state?

Oh... I thought this may be why I am getting it as the superuser... but I have "notify poster of own posts" off.

salvis’s picture

salvis
he/his
commented
Status: Needs work » Fixed

Ok, so the second part of the issue is working as intended, right?

I've just committed a fix for the first part to the -dev version. Give it up to 12h to be repackaged. Or grab subscriptions_content.module from CVS, if you're eager to try it out.

Thank you for your diligent testing, gregarios, this is very helpful!

gregarios’s picture

gregarios commented

I will get it tomorrow or later tonight and do further testing.

I tell you something... the Reroute Email module has been awesome at helping me test: http://drupal.org/project/reroute_email

salvis’s picture

salvis
he/his
commented

Thank you for the pointer. That could definitely come in handy, especially now that I know that it works with Subscriptions.

I run my own mail server and my test domain has a catch-all mail account. That way I can create any number of test users with mail addresses in that domain, and then receive mail with normal To addresses. This works very nicely for a test site, but cloning a live site and running it in a sandbox is an interesting idea.

gregarios’s picture

gregarios commented

That's exactly what I have. I made a script that clones the "dotcom" production site and makes an exact duplicate "dotnet" site out of it, which is only accessible from work or my home. Then I just log in and turn on the Reroute Email module and I'm free to try anything I want to the sandbox.

gregarios’s picture

gregarios commented

Ok, I downloaded and installed the late Aug 27th version of 6.x-1.x-dev, and performed the following tests:

  • Set maximum emails per cron run to 10
  • Set a taxonomy story term of 'newsletter' which are subscribed to by 1400 users in digest mode
  • Created 2 newsletter stories, but did not set them to publish. I sent them to moderation (Modr8 Module - http://drupal.org/project/modr8)
  • Examined subscriptions_queue table: 2800 entries
  • Ran cron once and examined subscriptions_queue table: 255 entries
  • Ran cron again and examined subscriptions_queue table: Empty
  • No emails received.
  • Approved the 2 stories, publishing them
  • Examined subscriptions_queue table: Empty
  • Created 2 more stories, unpublished, but not sent to moderation
  • Examined subscriptions_queue table: 2800 entries
  • Ran cron once and examined subscriptions_queue table: Empty
  • Published the 2 stories.
  • Examined subscriptions_queue table: 2800 entries
  • Ran cron again and examined subscriptions_queue table: 2780 entries
  • 10 emails received, each containing 2 stories! Yes!

(Note: Need a way to properly erase subscription queue and reset tables)

I'm not sure what to make of the first run where it left 255 entries in the table... these tests were run on a cloned site, with real users, so its possible these 255 had changed their newsletter subscriptions in some way. That doesn't explain why it worked perfectly on the second run though. Hmmm. Well... it doesn't affect this issue that I can tell.

There seems to be an incompatibility here between Subscriptions and the Modr8 Module. When Modr8 sets the stories to 'published' then Subscriptions Module doesn't recognize it. Subscriptions does recognize an unmoderated change in a story from 'unpublished' to 'published,' however.

Other than the Modr8 Module incompatibility, this version of Subscriptions seems to be working correctly! :-) Do you want to examine the Modr8 Module and see if you can fix the incompatibility, since Modr8 seems to be able to set nodes to 'published' satisfactorily?

gregarios’s picture

gregarios commented
Status: Fixed » Needs review
salvis’s picture

salvis
he/his
commented

Modr8 is yet another separate issue. Clearing the queue and resetting tables (?) likewise. I cannot deal with more than one issue per issue.

subscriptions_cron() is designed to not take more than 50% of your cron time. That probably explains why part of your queue remained after the first run. Check the watchdog log — it should tell you when it runs out of time.

It seems like this issue is fixed, no?

gregarios’s picture

gregarios commented

Well, the 255 left issue is not due to running out of cron time, because when the initial non-published nodes were deleted front he queue, it only took 3 seconds for the first 2500 to get deleted. :-) However... I do think this issue is completed, and fixed. I'll open a new Modr8 Issue separately.

salvis’s picture

salvis
he/his
commented

That is odd. I can't see what could change between the first cron run and the second, except that the first might stop before it's done because its time has run off, as was my guess in #9. Are you sure that the 3 seconds is for deleting the first 2545 records, and not for the 255 remaining ones in the second cron run?

Can you repeat that? Maybe with 3 posts rather than 2, because 255 is pretty close to 0, and the results may vary a bit.

gregarios’s picture

gregarios commented

Just sent out this week's 'Business Weekly' and all went perfectly. I created 4 unpublished nodes, allowed a cron run to cycle just for good measure, then set the nodes to 'published.' On the next cron run, the first 200 digests went out properly, with all content as it should. Same with the second run. In the end, after all the cron runs had run, a total of 1500 digests had been sent out.

The only small thing I noticed, which in no way affected me, was that the log entries showed various numbers of emails going out each run... The maximum is set at 200, but on the first run 171 went, on the second cron run 197 went, and so on... Not really a problem since it all seems to be working, just interesting. Each time the process only used about half of the total allowable cron time. Do you think it was just adjusting because of other items that ran during the same cron run which weren't logged?

salvis’s picture

salvis
he/his
commented

That's great news!

Yes, the cron hook is designed to use only half of the available time. We don't know whether other tasks should also run during cron (e.g. search indexing) and whether they run ahead or after Subscriptions. This didn't really work in the past, because the would be digests were saved for the second half of the available time.

It's nice to see it working now. Whether 50% is the good value is another question. You can configure it by setting a variable called 'subscriptions_cron_percent'; 50 is the default.

gregarios’s picture

gregarios commented

50% sounds about right to me. I guess it depends on how many other modules you have running cron tasks, which is apparently not easy to calculate as well. This works great for me. :-)

salvis’s picture

salvis
he/his
commented
Status: Needs review » Fixed

Well, now that the percentage is actually working, I've exposed it on the settings page. Committed to the -dev version, give it up to 12h to be repackaged.

The ability to limit the number of mails remains, because that can be a outside limitation, but for you it probably makes more sense to limit the percentage of time. A side-effect is that you can temporarily set the percentage to 0 to stop Subscriptions from sending out notifications.

This issue here was just a parenthesis to #350077-30: Maximum number of notification to send per cron job breaks Digest mode — let's turn our attention back there...

gregarios’s picture

gregarios commented

Interesting. How does the module calculate how much time to use? IE: If I set it to 50%, how does it know that is how much time it has available? If I set the allowable maximum time for a PHP process in the php.ini file, does that change the amount of time it thinks it has?

Honestly, I'm a bit confused as to why there is a cron-run time issue at all? Seems to be only with PHP stuff I guess... I have many cron tasks for other sites that run Perl scripts and shell scripts that sometimes take an hour to complete with no problems. I'm not understanding some crucial piece of info.

salvis’s picture

salvis
he/his
commented

Yes, we can retrieve the time from the php.ini file. The code is

  // Strategy for cron:
  // Use a defined percentage of the total cron time (default: 50%), but leave at least 5s.
  $total_seconds = ini_get('max_execution_time');
  $lost_seconds = timer_read('page')/1000;
  $available_seconds = $total_seconds - $lost_seconds;
  $cron_percentage = variable_get('subscriptions_cron_percent', 50);
  $usable_seconds = min(array($available_seconds - 5, $total_seconds*$cron_percentage/100));

The php.ini max_execution_time variable is a global safeguard against any one request taking too long (after all, it might just hang because of a program bug, needlessly burning CPU cycles). Depending on the permissions, this can even be overridden in subscriptions_cron(). I do this when debugging, see:

  // set_time_limit(3600);  drupal_set_message('DON\'T FORGET TO REMOVE THE TIME LIMIT EXTENSION!!!');

Comment this in (i.e. remove the two slashes), and you should get one hour of available cron time.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.