Issue #3310946 by phenaproxima, omkar.podey, tedbow: Improve the wording of...

Issue #3310946 by phenaproxima, omkar.podey, tedbow: Improve the wording of the email notifications about a failed unattended update

automatic_updates.module

@@ -47,24 +47,45 @@ function automatic_updates_mail(string $key, array &$message, array $params): vo
   $options = [
     'langcode' => $message['langcode'],
   ];
-  switch ($key) {
-    case 'cron_successful':
-      $message['subject'] = t("Drupal core was successfully updated", [], $options);
-      $message['body'][] = t('Congratulations!', [], $options);
-      $message['body'][] = t('Drupal core was automatically updated from @previous_version to @updated_version.', [
-        '@previous_version' => $params['previous_version'],
-        '@updated_version' => $params['updated_version'],
-      ], $options);
-      break;
+  if ($key === 'cron_successful') {
+    $message['subject'] = t("Drupal core was successfully updated", [], $options);
+    $message['body'][] = t('Congratulations!', [], $options);
+    $message['body'][] = t('Drupal core was automatically updated from @previous_version to @updated_version.', [
+      '@previous_version' => $params['previous_version'],
+      '@updated_version' => $params['updated_version'],
+    ], $options);
+  }
+  elseif (str_starts_with($key, 'cron_failed')) {
+    $message['subject'] = t("Drupal core update failed", [], $options);
 
-    case 'cron_failed':
-      $message['subject'] = t("Drupal core update failed", [], $options);
-      $message['body'][] = t('Drupal core failed to update automatically from @previous_version to @target_version. The following error was logged:', [
-        '@previous_version' => $params['previous_version'],
-        '@target_version' => $params['target_version'],
+    // If this is considered urgent, prefix the subject line with a call to
+    // action.
+    if ($params['urgent']) {
+      $message['subject'] = t('URGENT: @subject', [
+        '@subject' => $message['subject'],
       ], $options);
-      $message['body'][] = $params['error_message'];
-      break;
+    }
+
+    $message['body'][] = t('Drupal core failed to update automatically from @previous_version to @target_version. The following error was logged:', [
+      '@previous_version' => $params['previous_version'],
+      '@target_version' => $params['target_version'],
+    ], $options);
+    $message['body'][] = $params['error_message'];
+
+    // If the problem was not due to a failed apply, provide a link for the site
+    // owner to do the update.
+    if ($key !== 'cron_failed_apply') {
+      $url = Url::fromRoute('update.report_update')
+        ->setAbsolute()
+        ->toString();
+
+      if ($key === 'cron_failed_insecure') {
+        $message['body'][] = t('Your site is running an insecure version of Drupal and should be updated as soon as possible. Visit @url to perform the update.', ['@url' => $url], $options);
+      }
+      else {
+        $message['body'][] = t('No immediate action is needed, but it is recommended that you visit @url to perform the update, or at least check that everything still looks good.', ['@url' => $url], $options);
+      }
+    }
   }
 
   // If this email was related to an unattended update, explicitly state that

package_manager/src/ProjectInfo.php

@@ -185,4 +185,26 @@ final class ProjectInfo {
     return $available_projects;
   }
 
+  /**
+   * Checks if the installed version of this project is safe to use.
+   *
+   * @return bool
+   *   TRUE if the installed version of this project is secure, supported, and
+   *   published. Otherwise, or if the project information could not be
+   *   retrieved, returns FALSE.
+   */
+  public function isInstalledVersionSafe(): bool {
+    $project_data = $this->getProjectInfo();
+    if ($project_data) {
+      $unsafe = [
+        UpdateManagerInterface::NOT_SECURE,
+        UpdateManagerInterface::NOT_SUPPORTED,
+        UpdateManagerInterface::REVOKED,
+      ];
+      return !in_array($project_data['status'], $unsafe, TRUE);
+    }
+    // If we couldn't get project data, assume the installed version is unsafe.
+    return FALSE;
+  }
+
 }

package_manager/tests/fixtures/release-history/drupal.9.8.2-unsupported_unpublished.xml

@@ -3,7 +3,7 @@
 Contains metadata about the following (fake) releases of Drupal core, in order:
 * 9.8.2
 * 9.8.1, which is unsupported
-* 9.8.0
+* 9.8.0, which is unpublished
 * 9.8.0-alpha1
 * 9.7.1
 * 9.7.0

package_manager/tests/src/Kernel/ProjectInfoTest.php

@@ -176,4 +176,65 @@ class ProjectInfoTest extends PackageManagerKernelTestBase {
     $project_info->getInstallableReleases();
   }
 
+  /**
+   * Data provider for ::testInstalledVersionSafe().
+   *
+   * @return array[]
+   *   The test cases.
+   */
+  public function providerInstalledVersionSafe(): array {
+    $dir = __DIR__ . '/../../fixtures/release-history';
+
+    return [
+      'safe version' => [
+        '9.8.0',
+        $dir . '/drupal.9.8.2.xml',
+        TRUE,
+      ],
+      'unpublished version' => [
+        '9.8.0',
+        $dir . '/drupal.9.8.2-unsupported_unpublished.xml',
+        FALSE,
+      ],
+      'unsupported branch' => [
+        '9.6.1',
+        $dir . '/drupal.9.8.2-unsupported_unpublished.xml',
+        FALSE,
+      ],
+      'unsupported version' => [
+        '9.8.1',
+        $dir . '/drupal.9.8.2-unsupported_unpublished.xml',
+        FALSE,
+      ],
+      'insecure version' => [
+        '9.8.0',
+        $dir . '/drupal.9.8.1-security.xml',
+        FALSE,
+      ],
+    ];
+  }
+
+  /**
+   * Tests checking if the currently installed version of a project is safe.
+   *
+   * @param string $installed_version
+   *   The currently installed version of the project.
+   * @param string $release_xml
+   *   The path of the release metadata.
+   * @param bool $expected_to_be_safe
+   *   Whether or not the installed version of the project is expected to be
+   *   found safe.
+   *
+   * @covers ::isInstalledVersionSafe
+   *
+   * @dataProvider providerInstalledVersionSafe
+   */
+  public function testInstalledVersionSafe(string $installed_version, string $release_xml, bool $expected_to_be_safe): void {
+    $this->setCoreVersion($installed_version);
+    $this->setReleaseMetadata(['drupal' => $release_xml]);
+
+    $project_info = new ProjectInfo('drupal');
+    $this->assertSame($expected_to_be_safe, $project_info->isInstalledVersionSafe());
+  }
+
 }

src/CronUpdater.php

@@ -7,6 +7,7 @@ use Drupal\Core\Logger\LoggerChannelFactoryInterface;
 use Drupal\Core\Mail\MailManagerInterface;
 use Drupal\Core\State\StateInterface;
 use Drupal\Core\Url;
+use Drupal\package_manager\Exception\ApplyFailedException;
 use Drupal\package_manager\Exception\StageValidationException;
 use Drupal\package_manager\ProjectInfo;
 use Drupal\update\ProjectRelease;
@@ -160,13 +161,15 @@ class CronUpdater extends Updater {
    * Performs the update.
    *
    * @param string $target_version
-   *   The target version.
+   *   The target version of Drupal core.
    * @param int|null $timeout
    *   How long to allow the operation to run before timing out, in seconds, or
    *   NULL to never time out.
    */
   private function performUpdate(string $target_version, ?int $timeout): void {
-    $installed_version = (new ProjectInfo('drupal'))->getInstalledVersion();
+    $project_info = new ProjectInfo('drupal');
+
+    $installed_version = $project_info->getInstalledVersion();
     if (empty($installed_version)) {
       $this->logger->error('Unable to determine the current version of Drupal core.');
       return;
@@ -188,8 +191,21 @@ class CronUpdater extends Updater {
         'target_version' => $target_version,
         'error_message' => $e->getMessage(),
       ];
+      if ($e instanceof ApplyFailedException || $e->getPrevious() instanceof ApplyFailedException) {
+        $mail_params['urgent'] = TRUE;
+        $key = 'cron_failed_apply';
+      }
+      elseif (!$project_info->isInstalledVersionSafe()) {
+        $mail_params['urgent'] = TRUE;
+        $key = 'cron_failed_insecure';
+      }
+      else {
+        $mail_params['urgent'] = FALSE;
+        $key = 'cron_failed';
+      }
+
       foreach ($this->getEmailRecipients() as $email => $langcode) {
-        $this->mailManager->mail('automatic_updates', 'cron_failed', $email, $langcode, $mail_params);
+        $this->mailManager->mail('automatic_updates', $key, $email, $langcode, $mail_params);
       }
       $this->logger->error($e->getMessage());
 

tests/src/Kernel/CronUpdaterTest.php

@@ -8,6 +8,7 @@ use Drupal\Core\DependencyInjection\ContainerBuilder;
 use Drupal\Core\Form\FormState;
 use Drupal\Core\Logger\RfcLogLevel;
 use Drupal\Core\Test\AssertMailTrait;
+use Drupal\Core\Url;
 use Drupal\package_manager\Event\PostApplyEvent;
 use Drupal\package_manager\Event\PostCreateEvent;
 use Drupal\package_manager\Event\PostDestroyEvent;
@@ -18,6 +19,7 @@ use Drupal\package_manager\Event\PreRequireEvent;
 use Drupal\package_manager\Event\PreCreateEvent;
 use Drupal\package_manager\Exception\StageValidationException;
 use Drupal\package_manager\ValidationResult;
+use Drupal\package_manager_bypass\Committer;
 use Drupal\Tests\package_manager\Traits\PackageManagerBypassTestTrait;
 use Drupal\Tests\user\Traits\UserCreationTrait;
 use Drupal\update\UpdateSettingsForm;
@@ -59,6 +61,8 @@ class CronUpdaterTest extends AutomaticUpdatesKernelTestBase {
    * should be emailed in.
    *
    * @var string[]
+   *
+   * @see ::setUp()
    */
   private $emailRecipients = [];
 
@@ -389,17 +393,16 @@ class CronUpdaterTest extends AutomaticUpdatesKernelTestBase {
     $this->container->get('cron')->run();
 
     // Ensure we sent a success message to all recipients.
-    $sent_messages = $this->getMails([
-      'subject' => "Drupal core was successfully updated",
-    ]);
-    $this->assertNotEmpty($sent_messages);
-    $this->assertSame(count($this->emailRecipients), count($sent_messages));
+    $expected_body = <<<END
+Congratulations!
 
-    foreach ($sent_messages as $message) {
-      $email = $message['to'];
-      $this->assertSame($message['langcode'], $this->emailRecipients[$email]);
-      $this->assertCorrectMessageSent($email, $message, $message['langcode'], "Congratulations!\n\nDrupal core was automatically updated from 9.8.0 to 9.8.1.\n");
-    }
+Drupal core was automatically updated from 9.8.0 to 9.8.1.
+
+This e-mail was sent by the Automatic Updates module. Unattended updates are not yet fully supported.
+
+If you are using this feature in production, it is strongly recommended for you to visit your site and ensure that everything still looks good.
+END;
+    $this->assertMessagesSent("Drupal core was successfully updated", $expected_body);
   }
 
   /**
@@ -423,61 +426,140 @@ class CronUpdaterTest extends AutomaticUpdatesKernelTestBase {
   }
 
   /**
-   * Tests that email is sent when an unattended update fails.
+   * Tests the failure e-mail when an unattended non-security update fails.
    *
    * @param string $event_class
    *   The event class that should trigger the failure.
    *
    * @dataProvider providerEmailOnFailure
    */
-  public function testEmailOnFailure(string $event_class): void {
+  public function testNonUrgentFailureEmail(string $event_class): void {
+    $this->setReleaseMetadata([
+      'drupal' => __DIR__ . '/../../../package_manager/tests/fixtures/release-history/drupal.9.8.2.xml',
+    ]);
+    $this->config('automatic_updates.settings')
+      ->set('cron', CronUpdater::ALL)
+      ->save();
+
     $results = [
       ValidationResult::createError(['Error while updating!']),
     ];
     TestSubscriber1::setTestResult($results, $event_class);
     $exception = new StageValidationException($results);
+    $this->container->get('cron')->run();
+
+    $url = Url::fromRoute('update.report_update')
+      ->setAbsolute()
+      ->toString();
+
+    $expected_body = <<<END
+Drupal core failed to update automatically from 9.8.0 to 9.8.2. The following error was logged:
+
+{$exception->getMessage()}
+
+No immediate action is needed, but it is recommended that you visit $url to perform the update, or at least check that everything still looks good.
+
+This e-mail was sent by the Automatic Updates module. Unattended updates are not yet fully supported.
+
+If you are using this feature in production, it is strongly recommended for you to visit your site and ensure that everything still looks good.
+END;
+    $this->assertMessagesSent("Drupal core update failed", $expected_body);
+  }
+
+  /**
+   * Tests the failure e-mail when an unattended security update fails.
+   *
+   * @param string $event_class
+   *   The event class that should trigger the failure.
+   *
+   * @dataProvider providerEmailOnFailure
+   */
+  public function testSecurityUpdateFailureEmail(string $event_class): void {
+    $results = [
+      ValidationResult::createError(['Error while updating!']),
+    ];
+    TestSubscriber1::setTestResult($results, $event_class);
+    $exception = new StageValidationException($results);
+    $this->container->get('cron')->run();
+
+    $url = Url::fromRoute('update.report_update')
+      ->setAbsolute()
+      ->toString();
+
+    $expected_body = <<<END
+Drupal core failed to update automatically from 9.8.0 to 9.8.1. The following error was logged:
+
+{$exception->getMessage()}
+
+Your site is running an insecure version of Drupal and should be updated as soon as possible. Visit $url to perform the update.
+
+This e-mail was sent by the Automatic Updates module. Unattended updates are not yet fully supported.
+
+If you are using this feature in production, it is strongly recommended for you to visit your site and ensure that everything still looks good.
+END;
+    $this->assertMessagesSent("URGENT: Drupal core update failed", $expected_body);
+  }
+
+  /**
+   * Tests the failure e-mail when an unattended update fails to apply.
+   */
+  public function testApplyFailureEmail(): void {
+    $error = new \Exception('I drink your milkshake!');
+    Committer::setException($error);
 
     $this->container->get('cron')->run();
+    $expected_body = <<<END
+Drupal core failed to update automatically from 9.8.0 to 9.8.1. The following error was logged:
 
-    // Ensure we sent a failure message to all recipients.
+The update operation failed to apply. The update may have been partially applied. It is recommended that the site be restored from a code backup.
+
+This e-mail was sent by the Automatic Updates module. Unattended updates are not yet fully supported.
+
+If you are using this feature in production, it is strongly recommended for you to visit your site and ensure that everything still looks good.
+END;
+    $this->assertMessagesSent('URGENT: Drupal core update failed', $expected_body);
+  }
+
+  /**
+   * Asserts that all recipients recieved a given email.
+   *
+   * @param string $subject
+   *   The subject line of the email that should have been sent.
+   * @param string $body
+   *   The beginning of the body text of the email that should have been sent.
+   *
+   * @see ::$emailRecipients
+   */
+  private function assertMessagesSent(string $subject, string $body): void {
     $sent_messages = $this->getMails([
-      'subject' => "Drupal core update failed",
+      'subject' => $subject,
     ]);
     $this->assertNotEmpty($sent_messages);
     $this->assertSame(count($this->emailRecipients), count($sent_messages));
 
+    // Ensure the body is formatted the way the PHP mailer would do it.
+    $message = [
+      'body' => [$body],
+    ];
+    $message = $this->container->get('plugin.manager.mail')
+      ->createInstance('php_mail')
+      ->format($message);
+    $body = $message['body'];
+
     foreach ($sent_messages as $message) {
       $email = $message['to'];
-      $this->assertSame($message['langcode'], $this->emailRecipients[$email]);
-      $this->assertCorrectMessageSent($email, $message, $message['langcode'], "Drupal core failed to update automatically from 9.8.0 to 9.8.1. The following\nerror was logged:\n\n" . $exception->getMessage());
+      $expected_langcode = $this->emailRecipients[$email];
+
+      $this->assertSame($expected_langcode, $message['langcode']);
+      // The message, and every line in it, should have been sent in the
+      // expected language.
+      // @see automatic_updates_test_mail_alter()
+      $this->assertArrayHasKey('line_langcodes', $message);
+      $this->assertSame([$expected_langcode], $message['line_langcodes']);
+      $this->assertStringStartsWith($body, $message['body']);
     }
   }
 
-  /**
-   * Asserts correct message sent to correct recipient.
-   *
-   * @param string $expected_recipient
-   *   The email address that should have received the message.
-   * @param array $sent_message
-   *   The sent message, as processed by hook_mail().
-   * @param string $expected_language_code
-   *   The language code that the recipient should have been emailed in.
-   * @param string $expected_body_text
-   *   The expected message that the email body should contain.
-   */
-  private function assertCorrectMessageSent(string $expected_recipient, array $sent_message, string $expected_language_code, string $expected_body_text): void {
-    // Ensure the messages had the correct body text, and were sent to the right
-    // people.
-    $this->assertSame($sent_message['to'], $expected_recipient);
-    $this->assertStringStartsWith($expected_body_text, $sent_message['body']);
-    // The message, and every line in it, should have been sent in the
-    // expected language.
-    $this->assertSame($expected_language_code, $sent_message['langcode']);
-    // @see automatic_updates_test_mail_alter()
-    $this->assertArrayHasKey('line_langcodes', $sent_message);
-    $this->assertSame([$expected_language_code], $sent_message['line_langcodes']);
-  }
-
   /**
    * Tests that setLogger is called on the cron updater service.
    */