diff --git a/src/Form/UpdaterForm.php b/src/Form/UpdaterForm.php
index 36d4c8c6e02d369f8690aa417220aa6694e1bf43..ea080a22a8261debf10b967909aa4870730b8d9b 100644
--- a/src/Form/UpdaterForm.php
+++ b/src/Form/UpdaterForm.php
@@ -4,6 +4,7 @@ namespace Drupal\automatic_updates\Form;
use Drupal\automatic_updates\BatchProcessor;
use Drupal\automatic_updates\Updater;
+use Drupal\automatic_updates\Validation\ReadinessValidationManager;
use Drupal\automatic_updates_9_3_shim\ProjectRelease;
use Drupal\Core\Batch\BatchBuilder;
use Drupal\Core\Extension\ModuleHandlerInterface;
@@ -12,6 +13,7 @@ use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Link;
use Drupal\Core\State\StateInterface;
use Drupal\Core\Url;
+use Drupal\system\SystemManager;
use Drupal\update\UpdateManagerInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;
@@ -44,6 +46,13 @@ class UpdaterForm extends FormBase {
*/
protected $state;
+ /**
+ * The readiness validation manager service.
+ *
+ * @var \Drupal\automatic_updates\Validation\ReadinessValidationManager
+ */
+ protected $readinessValidationManager;
+
/**
* Constructs a new UpdaterForm object.
*
@@ -53,11 +62,14 @@ class UpdaterForm extends FormBase {
* The state service.
* @param \Drupal\automatic_updates\Updater $updater
* The updater service.
+ * @param \Drupal\automatic_updates\Validation\ReadinessValidationManager $readiness_validation_manager
+ * The readiness validation manager service.
*/
- public function __construct(ModuleHandlerInterface $module_handler, StateInterface $state, Updater $updater) {
+ public function __construct(ModuleHandlerInterface $module_handler, StateInterface $state, Updater $updater, ReadinessValidationManager $readiness_validation_manager) {
$this->updater = $updater;
$this->moduleHandler = $module_handler;
$this->state = $state;
+ $this->readinessValidationManager = $readiness_validation_manager;
}
/**
@@ -74,7 +86,8 @@ class UpdaterForm extends FormBase {
return new static(
$container->get('module_handler'),
$container->get('state'),
- $container->get('automatic_updates.updater')
+ $container->get('automatic_updates.updater'),
+ $container->get('automatic_updates.readiness_validation_manager')
);
}
@@ -190,7 +203,17 @@ class UpdaterForm extends FormBase {
],
];
- $form['actions'] = $this->actions();
+ // @todo Add a hasErrors() or getErrors() method to
+ // ReadinessValidationManager to make validation more introspectable.
+ // Re-running the readiness checks now should mean that when we display
+ // cached errors in automatic_updates_page_top(), we'll see errors that
+ // were raised during this run, instead of any previously cached results.
+ $errors = $this->readinessValidationManager->run()
+ ->getResults(SystemManager::REQUIREMENT_ERROR);
+
+ if (empty($errors)) {
+ $form['actions'] = $this->actions();
+ }
return $form;
}
diff --git a/tests/modules/automatic_updates_test/src/ReadinessChecker/TestChecker1.php b/tests/modules/automatic_updates_test/src/ReadinessChecker/TestChecker1.php
index 1294649269ea4c727009cf9feff90db10758482b..89f2bb964f791b57993dbb19910ac46890aeb3ab 100644
--- a/tests/modules/automatic_updates_test/src/ReadinessChecker/TestChecker1.php
+++ b/tests/modules/automatic_updates_test/src/ReadinessChecker/TestChecker1.php
@@ -40,14 +40,22 @@ class TestChecker1 implements EventSubscriberInterface {
* This method is static to enable setting the expected messages before the
* test module is enabled.
*
- * @param \Drupal\automatic_updates\Validation\ValidationResult[]|\Throwable $checker_results
- * The test validation results, or an exception to throw.
+ * @param \Drupal\automatic_updates\Validation\ValidationResult[]|\Throwable|null $checker_results
+ * The test validation results, or an exception to throw, or NULL to delete
+ * stored results.
* @param string $event_name
* (optional )The event name. Defaults to
* AutomaticUpdatesEvents::READINESS_CHECK.
*/
public static function setTestResult($checker_results, string $event_name = AutomaticUpdatesEvents::READINESS_CHECK): void {
- \Drupal::state()->set(static::STATE_KEY . ".$event_name", $checker_results);
+ $key = static::STATE_KEY . ".$event_name";
+
+ if (isset($checker_results)) {
+ \Drupal::state()->set($key, $checker_results);
+ }
+ else {
+ \Drupal::state()->delete($key);
+ }
}
/**
diff --git a/tests/src/Functional/ReadinessValidationTest.php b/tests/src/Functional/ReadinessValidationTest.php
index feb01de72df1f9a69efdbb4caef48e9ce8fb1082..181f731a3d84d16378f4a7da9ee4b8f75779eaed 100644
--- a/tests/src/Functional/ReadinessValidationTest.php
+++ b/tests/src/Functional/ReadinessValidationTest.php
@@ -22,16 +22,6 @@ class ReadinessValidationTest extends BrowserTestBase {
use CronRunTrait;
use ValidationTestTrait;
- /**
- * Expected explanation text when readiness checkers return error messages.
- */
- const ERRORS_EXPLANATION = 'Your site does not pass some readiness checks for automatic updates. It cannot be automatically updated until further action is performed.';
-
- /**
- * Expected explanation text when readiness checkers return warning messages.
- */
- const WARNINGS_EXPLANATION = 'Your site does not pass some readiness checks for automatic updates. Depending on the nature of the failures, it might affect the eligibility for automatic updates.';
-
/**
* {@inheritdoc}
*/
@@ -131,15 +121,15 @@ class ReadinessValidationTest extends BrowserTestBase {
// Assert that when the runners are run manually the message that updates
// will not be performed because of errors is displayed on the top of the
// page in message.
- $assert->pageTextMatchesCount(2, '/' . preg_quote(static::ERRORS_EXPLANATION) . '/');
- $this->assertReadinessReportMatches($expected_results[0]->getMessages()[0] . 'Run readiness checks now.', 'error', static::ERRORS_EXPLANATION);
+ $assert->pageTextMatchesCount(2, '/' . preg_quote(static::$errorsExplanation) . '/');
+ $this->assertReadinessReportMatches($expected_results[0]->getMessages()[0] . 'Run readiness checks now.', 'error', static::$errorsExplanation);
// @todo Should we always show when the checks were last run and a link to
// run when there is an error?
// Confirm a user without permission to run the checks sees the same error.
$this->drupalLogin($this->reportViewerUser);
$this->drupalGet('admin/reports/status');
- $this->assertReadinessReportMatches($expected_results[0]->getMessages()[0], 'error', static::ERRORS_EXPLANATION);
+ $this->assertReadinessReportMatches($expected_results[0]->getMessages()[0], 'error', static::$errorsExplanation);
$expected_results = $this->testResults['checker_1']['1 error 1 warning'];
TestChecker1::setTestResult($expected_results);
@@ -148,8 +138,8 @@ class ReadinessValidationTest extends BrowserTestBase {
$this->drupalGet('admin/reports/status');
// Confirm that on the status page if there is only 1 warning or error the
// the summaries will not be displayed.
- $this->assertReadinessReportMatches($expected_results['1:error']->getMessages()[0], 'error', static::ERRORS_EXPLANATION);
- $this->assertReadinessReportMatches($expected_results['1:warning']->getMessages()[0], 'warning', static::WARNINGS_EXPLANATION);
+ $this->assertReadinessReportMatches($expected_results['1:error']->getMessages()[0], 'error', static::$errorsExplanation);
+ $this->assertReadinessReportMatches($expected_results['1:warning']->getMessages()[0], 'warning', static::$warningsExplanation);
$assert->pageTextNotContains($expected_results['1:error']->getSummary());
$assert->pageTextNotContains($expected_results['1:warning']->getSummary());
@@ -159,8 +149,8 @@ class ReadinessValidationTest extends BrowserTestBase {
$this->drupalGet('admin/reports/status');
// Confirm that both messages and summaries will be displayed on status
// report when there multiple messages.
- $this->assertReadinessReportMatches($expected_results['1:errors']->getSummary() . ' ' . implode('', $expected_results['1:errors']->getMessages()), 'error', static::ERRORS_EXPLANATION);
- $this->assertReadinessReportMatches($expected_results['1:warnings']->getSummary() . ' ' . implode('', $expected_results['1:warnings']->getMessages()), 'warning', static::WARNINGS_EXPLANATION);
+ $this->assertReadinessReportMatches($expected_results['1:errors']->getSummary() . ' ' . implode('', $expected_results['1:errors']->getMessages()), 'error', static::$errorsExplanation);
+ $this->assertReadinessReportMatches($expected_results['1:warnings']->getSummary() . ' ' . implode('', $expected_results['1:warnings']->getMessages()), 'warning', static::$warningsExplanation);
$key_value->delete('readiness_validation_last_run');
$expected_results = $this->testResults['checker_1']['2 warnings'];
@@ -169,14 +159,14 @@ class ReadinessValidationTest extends BrowserTestBase {
$assert->pageTextContainsOnce('Update readiness checks');
// Confirm that warnings will display on the status report if there are no
// errors.
- $this->assertReadinessReportMatches($expected_results[0]->getSummary() . ' ' . implode('', $expected_results[0]->getMessages()), 'warning', static::WARNINGS_EXPLANATION);
+ $this->assertReadinessReportMatches($expected_results[0]->getSummary() . ' ' . implode('', $expected_results[0]->getMessages()), 'warning', static::$warningsExplanation);
$key_value->delete('readiness_validation_last_run');
$expected_results = $this->testResults['checker_1']['1 warning'];
TestChecker1::setTestResult($expected_results);
$this->drupalGet('admin/reports/status');
$assert->pageTextContainsOnce('Update readiness checks');
- $this->assertReadinessReportMatches($expected_results[0]->getMessages()[0], 'warning', static::WARNINGS_EXPLANATION);
+ $this->assertReadinessReportMatches($expected_results[0]->getMessages()[0], 'warning', static::$warningsExplanation);
}
/**
@@ -230,7 +220,7 @@ class ReadinessValidationTest extends BrowserTestBase {
$this->delayRequestTime();
$this->cronRun();
$this->drupalGet('admin/structure');
- $assert->pageTextContainsOnce(static::ERRORS_EXPLANATION);
+ $assert->pageTextContainsOnce(static::$errorsExplanation);
// Confirm on admin pages that a single error will be displayed instead of a
// summary.
$this->assertSame(SystemManager::REQUIREMENT_ERROR, $expected_results['1:error']->getSeverity());
@@ -265,7 +255,7 @@ class ReadinessValidationTest extends BrowserTestBase {
$assert->pageTextNotContains($expected_results['1:errors']->getMessages()[0]);
$assert->pageTextNotContains($expected_results['1:errors']->getMessages()[1]);
$assert->pageTextContainsOnce($expected_results['1:errors']->getSummary());
- $assert->pageTextContainsOnce(static::ERRORS_EXPLANATION);
+ $assert->pageTextContainsOnce(static::$errorsExplanation);
// Warnings are not displayed on admin pages if there are any errors.
$this->assertSame(SystemManager::REQUIREMENT_WARNING, $expected_results['1:warnings']->getSeverity());
$assert->pageTextNotContains($expected_results['1:warnings']->getMessages()[0]);
@@ -279,11 +269,11 @@ class ReadinessValidationTest extends BrowserTestBase {
$this->drupalGet('admin/structure');
// Confirm that the warnings summary is displayed on admin pages if there
// are no errors.
- $assert->pageTextNotContains(static::ERRORS_EXPLANATION);
+ $assert->pageTextNotContains(static::$errorsExplanation);
$this->assertSame(SystemManager::REQUIREMENT_WARNING, $expected_results[0]->getSeverity());
$assert->pageTextNotContains($expected_results[0]->getMessages()[0]);
$assert->pageTextNotContains($expected_results[0]->getMessages()[1]);
- $assert->pageTextContainsOnce(static::WARNINGS_EXPLANATION);
+ $assert->pageTextContainsOnce(static::$warningsExplanation);
$assert->pageTextContainsOnce($expected_results[0]->getSummary());
$expected_results = $this->testResults['checker_1']['1 warning'];
@@ -291,11 +281,11 @@ class ReadinessValidationTest extends BrowserTestBase {
$this->delayRequestTime();
$this->cronRun();
$this->drupalGet('admin/structure');
- $assert->pageTextNotContains(static::ERRORS_EXPLANATION);
+ $assert->pageTextNotContains(static::$errorsExplanation);
// Confirm that a single warning is displayed and not the summary on admin
// pages if there is only 1 warning and there are no errors.
$this->assertSame(SystemManager::REQUIREMENT_WARNING, $expected_results[0]->getSeverity());
- $assert->pageTextContainsOnce(static::WARNINGS_EXPLANATION);
+ $assert->pageTextContainsOnce(static::$warningsExplanation);
$assert->pageTextContainsOnce($expected_results[0]->getMessages()[0]);
$assert->pageTextNotContains($expected_results[0]->getSummary());
}
diff --git a/tests/src/Functional/UpdaterFormTest.php b/tests/src/Functional/UpdaterFormTest.php
index 287d258961aff4389af28857de97a6947b98c06a..bd9fb7248da1157f80312cffc8b3a7c578ea3307 100644
--- a/tests/src/Functional/UpdaterFormTest.php
+++ b/tests/src/Functional/UpdaterFormTest.php
@@ -4,6 +4,7 @@ namespace Drupal\Tests\automatic_updates\Functional;
use Drupal\automatic_updates\AutomaticUpdatesEvents;
use Drupal\automatic_updates\Exception\UpdateException;
+use Drupal\automatic_updates\Validation\ValidationResult;
use Drupal\automatic_updates_test\ReadinessChecker\TestChecker1;
use Drupal\Tests\automatic_updates\Traits\ValidationTestTrait;
use Drupal\Tests\BrowserTestBase;
@@ -109,27 +110,57 @@ class UpdaterFormTest extends BrowserTestBase {
$assert_session = $this->assertSession();
$page = $session->getPage();
+ // Store a fake readiness error, which will be cached.
+ $message = t("You've not experienced Shakespeare until you have read him in the original Klingon.");
+ $error = ValidationResult::createError([$message]);
+ TestChecker1::setTestResult([$error]);
+
+ $this->drupalLogin($this->rootUser);
+ $this->drupalGet('/admin/reports/status');
+ $page->clickLink('Run readiness checks');
+ $assert_session->pageTextContainsOnce((string) $message);
+ // Ensure that the fake error is cached.
+ $session->reload();
+ $assert_session->pageTextContainsOnce((string) $message);
+
$this->setCoreVersion('9.8.0');
- $this->createTestValidationResults();
+ $this->checkForUpdates();
+ // Set up a new fake error.
+ $this->createTestValidationResults();
$expected_results = $this->testResults['checker_1']['1 error'];
+ TestChecker1::setTestResult($expected_results);
+
+ // If a validator raises an error during readiness checking, the form should
+ // not have a submit button.
+ $this->drupalGet('/admin/automatic-update');
+ $assert_session->buttonNotExists('Download these updates');
+ // Since this is an administrative page, the error message should be visible
+ // thanks to automatic_updates_page_top(). The readiness checks were re-run
+ // during the form build, which means the new error should be cached and
+ // displayed instead of the previously cached error.
+ $assert_session->pageTextContainsOnce((string) $expected_results[0]->getMessages()[0]);
+ $assert_session->pageTextContainsOnce(static::$errorsExplanation);
+ $assert_session->pageTextNotContains(static::$warningsExplanation);
+ $assert_session->pageTextNotContains((string) $message);
+ TestChecker1::setTestResult(NULL);
+
// Repackage the validation error as an exception, so we can test what
- // happens if a validator throws.
+ // happens if a validator throws once the update has started.
$error = new UpdateException($expected_results, 'The update exploded.');
TestChecker1::setTestResult($error, AutomaticUpdatesEvents::PRE_START);
-
- $this->drupalLogin($this->rootUser);
- $this->checkForUpdates();
- $this->drupalGet('/admin/automatic-update');
+ $session->reload();
+ $assert_session->pageTextNotContains(static::$errorsExplanation);
+ $assert_session->pageTextNotContains(static::$warningsExplanation);
$page->pressButton('Download these updates');
$this->checkForMetaRefresh();
- $assert_session->pageTextContains('An error has occurred.');
+ $assert_session->pageTextContainsOnce('An error has occurred.');
$page->clickLink('the error page');
- $assert_session->pageTextContains((string) $expected_results[0]->getMessages()[0]);
+ $assert_session->pageTextContainsOnce((string) $expected_results[0]->getMessages()[0]);
// Since there's only one error message, we shouldn't see the summary...
$assert_session->pageTextNotContains($expected_results[0]->getSummary());
// ...but we should see the exception message.
- $assert_session->pageTextContains('The update exploded.');
+ $assert_session->pageTextContainsOnce('The update exploded.');
// If a validator flags an error, but doesn't throw, the update should still
// be halted.
@@ -137,11 +168,11 @@ class UpdaterFormTest extends BrowserTestBase {
$this->deleteStagedUpdate();
$page->pressButton('Download these updates');
$this->checkForMetaRefresh();
- $assert_session->pageTextContains('An error has occurred.');
+ $assert_session->pageTextContainsOnce('An error has occurred.');
$page->clickLink('the error page');
// Since there's only one message, we shouldn't see the summary.
$assert_session->pageTextNotContains($expected_results[0]->getSummary());
- $assert_session->pageTextContains((string) $expected_results[0]->getMessages()[0]);
+ $assert_session->pageTextContainsOnce((string) $expected_results[0]->getMessages()[0]);
// If a validator flags a warning, but doesn't throw, the update should
// continue.
diff --git a/tests/src/Traits/ValidationTestTrait.php b/tests/src/Traits/ValidationTestTrait.php
index 382f6352d77ea75623de8b3c4087379121a60997..7f313f23c9710ff60ab8f3d2994d0628287dc35c 100644
--- a/tests/src/Traits/ValidationTestTrait.php
+++ b/tests/src/Traits/ValidationTestTrait.php
@@ -9,6 +9,20 @@ use Drupal\automatic_updates\Validation\ValidationResult;
*/
trait ValidationTestTrait {
+ /**
+ * Expected explanation text when readiness checkers return error messages.
+ *
+ * @var string
+ */
+ protected static $errorsExplanation = 'Your site does not pass some readiness checks for automatic updates. It cannot be automatically updated until further action is performed.';
+
+ /**
+ * Expected explanation text when readiness checkers return warning messages.
+ *
+ * @var string
+ */
+ protected static $warningsExplanation = 'Your site does not pass some readiness checks for automatic updates. Depending on the nature of the failures, it might affect the eligibility for automatic updates.';
+
/**
* Test validation results.
*