Inform administrators of failed updates with a site-wide persistent message if insecure, on some pages if secure, and always on the status report

I am wondering if we should generalize this so that administrators receive the message if cron updates could not be completed for any reason.

In \Drupal\automatic_updates\CronUpdateStage::performUpdate() we have the try/catch

try {
      // @see ::begin()
      $stage_id = parent::begin(['drupal' => $target_version], $timeout);
      $this->stage();
      $this->apply();
    }
    catch (\Throwable $e) {
...
}

StagedDatabaseUpdateValidator could add an error which would result in exception but so could other validators

I think if that happens we should store that info in state so that we can display messages to the admin.

We would need to store

1. The exception message. In the case that exception is StageEventException, which will be the case if was from validation error like StagedDatabaseUpdateValidator, then we should check the event and if it is `PreOperationStageEvent` we should store the results so we can translate them for the admin
2. The target version of the update
3. The current version of site
4. Whether the current version is safe as return by \Drupal\package_manager\ProjectInfo::isInstalledVersionSafe()

In automatic_updates_page_top() where we would display the message we should first check if the current version of the site is the same version that we saved in the state. If not then the site has already been updated, either through the module or Composer directly. In that case we can just delete the saved state value because the admin no longer needs the message.

Otherwise we should display the message to the user. If the current version of the site is safe the message should be a warning if the current version of the site is not safe the message should an error.

I am little a conflicted about whether we should actually show the message on all pages if the site is on a safe version. Drupal doesn't have user dismissible messages (and we can't solve that here!) so the admin might see the message for a month if they are ok with the site being on the current patch version. They may check out the patch version release notes and determine there isn't a reason to update to it but also not want to turn off cron updates.

We could also show this message on the status report regardless of whether the site is on safe version or not. If we did that maybe would be ok to only show the message on all admin pages if the site is on an unsafe version.

I am wondering if we should generalize this so that administrators receive the message if cron updates could not be completed for any reason.

💯

I am little a conflicted about whether we should actually show the message on all pages if the site is on a safe version.

I am too.

But at the same time, it's better to currently over-inform than under-inform. At least as a first step.

Two thoughts:

1. What if there's three new settings besides the existing status_check_mail = disabled|errors_only|all? So:
   1. status_check_message = disabled|errors_only|all → to allow to not get e-mails and only messages, or both, or neither 👈 the scope expansion you proposed in #2
   2. cron_mail = disabled|errors_only|security_warnings_only|all 👈 the original scope of this issue
   3. cron_message = disabled|errors_only|security_warnings_only|all 👈 the original scope of this issue
2. We could implement this using the JavaScript Messages API → instead of the MessengerInterface service, and then theme it to be dismissable, where we'd carefully use the message ID to track (in window.sessionStorage) which messages have been dismissed to avoid showing them again.

   Downside: "dismissable messages" would be a new UI pattern for Drupal, which would come with usability approval etc.

re my previous comment

I am little a conflicted about whether we should actually show the message on all pages if the site is on a safe version.

I am in favor keeping it a little simpler than adding the new setting and dismissible messages(the same would still the same message on different devicies/browsers).

Here is my thought.

1. If the site is on an insecure version should the message on most pages
2. If the site on a secure version only show the message on certain pages: The Updates form, the available updates page, and an item on the status report.

   This way they will know not to expect cron to update, or if the message is something that is specific to the last time updates were attempted, for example of custom validator that stops update at busy times, they can won't be bothered everywhere.

3. If the site is not different version that it was when the updates were attempted don't show a message
4. If cron updates have since been turned off don't show a message

@Wim Leers in the summary you wrote

But not every site has outbound e-mails configured. Plus, they could end up in spam.

I there a way to detect this. will our attempts send emails throw an exception or will just fail silently.

@Wim Leers I don't quite get 2.1. in proposed resolution, can you explain that in detail?

Just talked to @kunal.sachdev. I didn't actually write 2.1, @tedbow did that in #5, but I think we figured it out together 👍

Based on:

• by @tedbow in #2
• in #2, and added to the issue summary in #5

@tedbow: please check the changes I made to the issue summary and confirm you agree with that more specific/concrete direction I provided for @kunal.sachdev.

Looking great! Fixed some language nits. One commit-blocking problem remains, plus two test clarity problems — all are easy to fix! 👍😊

Thanks to the better structure of the test, a new problem has become apparent that I missed before: the test is lacking clarity and is not fully taking advantage of defining expectations in the data provider. Provided guidance on the MR on how to improve that 😊

Assigning to myself since I am reviewing

This is getting more complicated but I think that is okay because we have to consider if site was secure at the time the cron update was attempted but is insecure by the time the user views the message. See the MR comment .

I have pushed up a start to test that I think can be simpler.