Update module should send notifications on configured day

Brought this up in #yes-no-queue and @catch mentioned this could still be valid.

Sounds like the request is to be able to select a day when selecting weekly under "Check for Updates"

Good point. 😅 Needs subsystem maintainer review. Here's a real summary.

Giving this a shot.

Added config and changed update_cron hook to take day of week in to account.

With the schema updates image we will need an upgrade path for that.

Also will need test coverage

Can someone, please, provide instructions on how can we write a test for this?

Brought could use a mock right? To pretend it's Thursday.

Added test coverage. Up for review.

Just realized Issue summary has some parts marked TBD that will need to be filled in.

But the test, least from what I can tell, is testing sending the notification always on "today" so not testing the Thursday feature I imagine.

The remaining tasks seem to be review related. So, keeping them pending.

But the test, is testing sending the notification always on "today" so not testing the Thursday feature I imagine.

The default day is Thursday, but we can set it to anything we want. In test, I am setting the update day to today and checking if I get the update notification email today. I couldn't find a way to pretend the other day to run cron. Other than that, I have added one more test case to confirm this by checking the notification is not sent on other days than the configured day.

Ah thanks for explaining makes more sense.

This is a great idea!

Have we accounted for the fact that the week starts on different days depending on your locale? "Thursday" can be counted as either day 4 or day 5, depending on whether the location's week starts on a Sunday. Furthermore, while scheduling updates for the day after the default security window is good -- but! It doesn't help Australia and New Zealand, because Thursday for them starts before the scheduled hours of the security window. So they need it the most but they'll always be one full week out of date.

I think we're on the right track here, but we need to think a little more about the datetime aspects, so that we have better defaults and configuration options that get localized properly. I'd suggest reaching out to DateTime maintainers as well as people who are actually from Australia and New Zealand, to get a sense of an internationalized version of this that's less US/EU-centric in its conception.

@yash.rode, FWIW, your persepctive on how this affects Indian sites is probably worthwhile as well, since midnight actually happens smack in the middle of the security release window for India (if you are still in Pune that is).

Tagging for subsystem maintainer review, but I actually mean the DateTime maintainers who understand the dark corners of timezone compatibility. There is no tag for "Needs APAC sense check." 😉

> Since SAs go out on Wednesdays this means it could be a full week between the SA and the notification.

I can't actually find that information online. Googling "drupal SA window" gets me only past SAs. https://www.drupal.org/security doesn't say.

Changed the description to take into account timezone and changed the day format.

Still needs submaintainer sign off but to keep the issue from stalling moving to RTBC.

Seems all threads have been resolved
Verified post_update still runs without issue.

Thanks for keeping this moving forward! I opened some new threads on the MR. The most significant being this:

Nice touch on this help text to localize it for the person configuring the site. However, the information about 6pm UTC is potentially frustrating since people have no control over what time of day the check is happening. 😞 That's nice to know it'll be "Thursday at 2am" for me, but does that mean I have to set it for Friday to be "safe", since maybe this will run at 12:01am on Thursday?

I'm not sure what to do with this. I think it's getting too complicated to add a time of day config knob, too, but maybe we "need" that. Or we need to invent something arbitrary like we always do these checks as close to "noon local time" as possible.

But if we're going to go that far, why make this a new config knob at all? Why not just automatically have the check logic know that if you're only checking weekly, it should know this:

> Security releases happen between 16:00 UTC and 22:00 UTC.

If the knob is configured for weekly checks, it should always aim to check as soon after Wednesday at 22:00 UTC as possible.

In theory, that policy could change, and we'll regret hard-coding this, but we release core often enough that if we ever did need to change this, we could...

I have some regret I didn't consider this when I first reviewed the issue, but at least I thought of it now! 😉 Thankfully, it should result in a much smaller change to core overall, and we don't have to further complicate the UI at all.

If there are no objections to my counter-proposal, this needs a summary update and an almost entirely different change. Perhaps it'd be cleaner in a new MR.

Thanks/sorry!
-Derek

p.s. Saving credit for everyone who's participated so far, since everything has been a meaningful contribution towards fixing this.

Also adding some extremely related issues...

@dww #34 Sounds like a good idea to me.

This probably not an issue but just thought I would raise it just in case....
Is the Update XML served by a CDN? If not would there be a problem with every site's cron requesting at around the same time? I figure it will not be a problem because cron is probably not going to run exactly on the hour for sites so it will vary.

Again i am in favor of @dww's idea just thought I would raise that issue in case it raise any flags with anyone more knowledgeable about such things than myself

Is the Update XML served by a CDN? If not would there be a problem with every site's cron requesting at around the same time? I figure it will not be a problem because cron is probably not going to run exactly on the hour for sites so it will vary.

As long as this is true, cron doesn’t run exactly on the hour, it’s all good. Our CDN does handle spikes at the top of the hour already, since it is common configuration. Its better to move away from that for any default configuration.

Updates and alert emails are sent once every 24 hours, which can result in an update alert getting sent 23 hours after a security update was released.

Since Drupal security updates are usually sent out around 18:00 GMT, it would be nice if there was a setting to define exactly when to check for updates and send email updates, for example at 20:00 GMT.

Maybe it's even worth considering a new option, which is "Check for updates > Daily > Every Hour" if you as an administrator want to make sure you always get email alerts ASAP?

I still think #34 makes more sense and would be vastly easier to implement than #39 or #40

P.s. if you want “immediate” email notifications of security updates, subscribe to the security announcements newsletter and/or consume the feed via RSS.

Checking frequently isn’t an issue for traffic server-side, the CDN absorbs that.

In highly critical security releases, even checking at the end of the security release window is not ideal. A bad enough vulnerability would be exploited before the window is over. And the window is ideal, but not a rule. If there’s a 0-day, any day is good for fixing the issue. If there’s some issue in the release process, late is better than waiting another week. Even under ideal conditions, we might reschedule, these sorts of things should not be hard-coded into Drupal core.

The one thing I would ask for is encouraging skewing when sites ask for updates, so we don’t have a spike of traffic at the top of an hour, its spread out throughout an hour.