Issue #3262587 by phenaproxima: Show warnings when starting an update

src/Form/UpdaterForm.php

@@ -198,17 +198,21 @@ class UpdaterForm extends FormBase {
       ],
     ];
 
-    $results = $this->getReadinessErrors($recommended_release->getVersion());
-    if (empty($results)) {
-      $form['actions'] = $this->actions($form_state);
+    if ($form_state->isSubmitted()) {
+      $results = [];
     }
     else {
-      $this->messenger()->addError($this->getFailureMessageForSeverity(SystemManager::REQUIREMENT_ERROR));
-      foreach ($results as $result) {
-        $messages = $result->getMessages();
-        $message = count($messages) === 1 ? $messages[0] : $result->getSummary();
-        $this->messenger()->addError($message);
-      }
+      $event = new ReadinessCheckEvent($this->updater, [
+        'drupal' => $recommended_release->getVersion(),
+      ]);
+      $this->eventDispatcher->dispatch($event);
+      $results = $event->getResults();
+    }
+    $this->displayResults($results, $this->messenger());
+
+    // If there were no errors, allow the user to proceed with the update.
+    if ($this->getOverallSeverity($results) !== SystemManager::REQUIREMENT_ERROR) {
+      $form['actions'] = $this->actions($form_state);
     }
     return $form;
   }
@@ -273,19 +277,4 @@ class UpdaterForm extends FormBase {
     batch_set($batch);
   }
 
-  /**
-   * Gets validation errors before an update begins.
-   *
-   * @param string $update_version
-   *   The version of Drupal to which we will update.
-   *
-   * @return \Drupal\package_manager\ValidationResult[]
-   *   The error validation results.
-   */
-  private function getReadinessErrors(string $update_version): array {
-    $event = new ReadinessCheckEvent($this->updater, ['drupal' => $update_version]);
-    $this->eventDispatcher->dispatch($event);
-    return $event->getResults(SystemManager::REQUIREMENT_ERROR);
-  }
-
 }

src/Validation/AdminReadinessMessages.php

@@ -164,24 +164,7 @@ final class AdminReadinessMessages implements ContainerInjectionInterface {
     if (empty($results)) {
       return FALSE;
     }
-    $failure_message = $this->getFailureMessageForSeverity($severity);
-    if ($severity === SystemManager::REQUIREMENT_ERROR) {
-      $this->messenger()->addError($failure_message);
-    }
-    else {
-      $this->messenger()->addWarning($failure_message);
-    }
-
-    foreach ($results as $result) {
-      $messages = $result->getMessages();
-      $message = count($messages) === 1 ? $messages[0] : $result->getSummary();
-      if ($severity === SystemManager::REQUIREMENT_ERROR) {
-        $this->messenger()->addError($message);
-      }
-      else {
-        $this->messenger()->addWarning($message);
-      }
-    }
+    $this->displayResults($results, $this->messenger());
     return TRUE;
   }
 

src/Validation/ReadinessTrait.php

@@ -2,6 +2,7 @@
 
 namespace Drupal\automatic_updates\Validation;
 
+use Drupal\Core\Messenger\MessengerInterface;
 use Drupal\Core\StringTranslation\TranslatableMarkup;
 use Drupal\system\SystemManager;
 
@@ -31,4 +32,60 @@ trait ReadinessTrait {
       $this->t('Your site does not pass some readiness checks for automatic updates. It cannot be automatically updated until further action is performed.');
   }
 
+  /**
+   * Returns the overall severity for a set of validation results.
+   *
+   * @param \Drupal\package_manager\ValidationResult[] $results
+   *   The validation results.
+   *
+   * @return int
+   *   The overall severity of the results. Will be be one of the
+   *   SystemManager::REQUIREMENT_* constants.
+   */
+  protected function getOverallSeverity(array $results): int {
+    foreach ($results as $result) {
+      if ($result->getSeverity() === SystemManager::REQUIREMENT_ERROR) {
+        return SystemManager::REQUIREMENT_ERROR;
+      }
+    }
+    // If there were no errors, then any remaining results must be warnings.
+    return $results ? SystemManager::REQUIREMENT_WARNING : SystemManager::REQUIREMENT_OK;
+  }
+
+  /**
+   * Adds a set of validation results to the messages.
+   *
+   * @param \Drupal\package_manager\ValidationResult[] $results
+   *   The validation results.
+   * @param \Drupal\Core\Messenger\MessengerInterface $messenger
+   *   The messenger service.
+   */
+  protected function displayResults(array $results, MessengerInterface $messenger): void {
+    $severity = $this->getOverallSeverity($results);
+
+    if ($severity === SystemManager::REQUIREMENT_OK) {
+      return;
+    }
+
+    $message = $this->getFailureMessageForSeverity($severity);
+    if ($severity === SystemManager::REQUIREMENT_ERROR) {
+      $messenger->addError($message);
+    }
+    else {
+      $messenger->addWarning($message);
+    }
+
+    foreach ($results as $result) {
+      $messages = $result->getMessages();
+      $message = count($messages) === 1 ? $messages[0] : $result->getSummary();
+
+      if ($result->getSeverity() === SystemManager::REQUIREMENT_ERROR) {
+        $messenger->addError($message);
+      }
+      else {
+        $messenger->addWarning($message);
+      }
+    }
+  }
+
 }

tests/src/Functional/UpdaterFormTest.php

@@ -328,13 +328,21 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase {
     $this->setCoreVersion('9.8.0');
     $this->checkForUpdates();
 
+    // Flag a warning, which will not block the update but should be displayed
+    // on the updater form.
+    $this->createTestValidationResults();
+    $expected_results = $this->testResults['checker_1']['1 warning'];
+    TestChecker1::setTestResult($expected_results, ReadinessCheckEvent::class);
+    $messages = reset($expected_results)->getMessages();
+
     $page = $this->getSession()->getPage();
     $this->drupalGet($update_form_url);
+    $assert_session = $this->assertSession();
+    $assert_session->pageTextContains(reset($messages));
     $page->pressButton('Update');
     $this->checkForMetaRefresh();
     $this->assertUpdateStagedTimes(1);
     $this->assertUpdateReady();
-    $assert_session = $this->assertSession();
     $page->pressButton('Continue');
     $this->checkForMetaRefresh();
     $assert_session->addressEquals('/admin/reports/updates');

tests/src/Unit/ReadinessTraitTest.php

+<?php
+
+namespace Drupal\Tests\automatic_updates\Unit;
+
+use Drupal\automatic_updates\Validation\ReadinessTrait;
+use Drupal\Core\Messenger\Messenger;
+use Drupal\Core\PageCache\ResponsePolicy\KillSwitch;
+use Drupal\Core\StringTranslation\PluralTranslatableMarkup;
+use Drupal\Core\StringTranslation\StringTranslationTrait;
+use Drupal\Core\StringTranslation\TranslatableMarkup;
+use Drupal\Core\StringTranslation\TranslationInterface;
+use Drupal\package_manager\ValidationResult;
+use Drupal\system\SystemManager;
+use Drupal\Tests\UnitTestCase;
+use Symfony\Component\HttpFoundation\Session\Flash\FlashBag;
+
+/**
+ * @coversDefaultClass \Drupal\automatic_updates\Validation\ReadinessTrait
+ *
+ * @group automatic_updates
+ */
+class ReadinessTraitTest extends UnitTestCase {
+
+  use ReadinessTrait;
+  use StringTranslationTrait;
+
+  /**
+   * @covers ::getOverallSeverity
+   */
+  public function testOverallSeverity(): void {
+    // An error and a warning should be counted as an error.
+    $results = [
+      ValidationResult::createError(['Boo!']),
+      ValidationResult::createWarning(['Moo!']),
+    ];
+    $this->assertSame(SystemManager::REQUIREMENT_ERROR, $this->getOverallSeverity($results));
+
+    // If there are no results, but no errors, the results should be counted as
+    // a warning.
+    array_shift($results);
+    $this->assertSame(SystemManager::REQUIREMENT_WARNING, $this->getOverallSeverity($results));
+
+    // If there are just plain no results, we should get REQUIREMENT_OK.
+    array_shift($results);
+    $this->assertSame(SystemManager::REQUIREMENT_OK, $this->getOverallSeverity($results));
+  }
+
+  /**
+   * @covers ::displayResults
+   */
+  public function testDisplayResults(): void {
+    $messenger = new Messenger(new FlashBag(), new KillSwitch());
+
+    $translation = new TestTranslationManager();
+    $this->setStringTranslation($translation);
+
+    // An error and a warning should display the error preamble, and the result
+    // messages as errors and warnings, respectively.
+    $results = [
+      ValidationResult::createError(['Boo!']),
+      ValidationResult::createError(['Wednesday', 'Pugsley'], $this->t('The Addams Family')),
+      ValidationResult::createWarning(['Moo!']),
+      ValidationResult::createWarning(['Shaggy', 'Scooby'], $this->t('Mystery Mobile')),
+    ];
+    $this->displayResults($results, $messenger);
+
+    $expected_errors = [
+      (string) $this->getFailureMessageForSeverity(SystemManager::REQUIREMENT_ERROR),
+      'Boo!',
+      'The Addams Family',
+    ];
+    $actual_errors = array_map('strval', $messenger->deleteByType(Messenger::TYPE_ERROR));
+    $this->assertSame($expected_errors, $actual_errors);
+
+    // Even though there were warnings, we shouldn't see the warning preamble.
+    $expected_warnings = ['Moo!', 'Mystery Mobile'];
+    $actual_warnings = array_map('strval', $messenger->deleteByType(Messenger::TYPE_WARNING));
+    $this->assertSame($expected_warnings, $actual_warnings);
+
+    // There shouldn't be any more messages.
+    $this->assertEmpty($messenger->all());
+
+    // If there are only warnings, we should see the warning preamble.
+    $results = array_slice($results, -2);
+    $this->displayResults($results, $messenger);
+
+    $expected_warnings = [
+      (string) $this->getFailureMessageForSeverity(SystemManager::REQUIREMENT_WARNING),
+      'Moo!',
+      'Mystery Mobile',
+    ];
+    $actual_warnings = array_map('strval', $messenger->deleteByType(Messenger::TYPE_WARNING));
+    $this->assertSame($expected_warnings, $actual_warnings);
+
+    // There shouldn't be any more messages.
+    $this->assertEmpty($messenger->all());
+  }
+
+}
+
+/**
+ * Implements a translation manager in tests.
+ */
+class TestTranslationManager implements TranslationInterface {
+
+  /**
+   * {@inheritdoc}
+   */
+  public function translate($string, array $args = [], array $options = []) {
+    return new TranslatableMarkup($string, $args, $options, $this);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function translateString(TranslatableMarkup $translated_string) {
+    return $translated_string->getUntranslatedString();
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function formatPlural($count, $singular, $plural, array $args = [], array $options = []) {
+    return new PluralTranslatableMarkup($count, $singular, $plural, $args, $options, $this);
+  }
+
+}