From 56f2e341a2911bc18f41a46541395c6cbb9b21a5 Mon Sep 17 00:00:00 2001
From: phenaproxima <phenaproxima@205645.no-reply.drupal.org>
Date: Tue, 7 Jun 2022 14:27:21 +0000
Subject: [PATCH] Issue #3284346 by phenaproxima: Show all validation messages,
not just the summary, on the updater form
---
src/Form/UpdaterForm.php | 21 ++++++++++++++++++++-
src/Validation/ReadinessTrait.php | 23 +++++++++++++++++------
tests/src/Functional/UpdaterFormTest.php | 11 +++++++----
3 files changed, 44 insertions(+), 11 deletions(-)
diff --git a/src/Form/UpdaterForm.php b/src/Form/UpdaterForm.php
index 7a8e8250c8..63e50ad978 100644
--- a/src/Form/UpdaterForm.php
+++ b/src/Form/UpdaterForm.php
@@ -18,6 +18,7 @@ use Drupal\Core\State\StateInterface;
use Drupal\Core\Url;
use Drupal\package_manager\Exception\StageException;
use Drupal\package_manager\Exception\StageOwnershipException;
+use Drupal\package_manager\ValidationResult;
use Drupal\system\SystemManager;
use Drupal\update\UpdateManagerInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;
@@ -31,7 +32,9 @@ use Symfony\Component\EventDispatcher\EventDispatcherInterface;
*/
class UpdaterForm extends FormBase {
- use ReadinessTrait;
+ use ReadinessTrait {
+ formatResult as traitFormatResult;
+ }
/**
* The updater service.
@@ -322,4 +325,20 @@ class UpdaterForm extends FormBase {
batch_set($batch);
}
+ /**
+ * {@inheritdoc}
+ */
+ protected function formatResult(ValidationResult $result) {
+ $messages = $result->getMessages();
+
+ if (count($messages) > 1) {
+ return [
+ '#theme' => 'item_list__automatic_updates_validation_results',
+ '#prefix' => $result->getSummary(),
+ '#items' => $messages,
+ ];
+ }
+ return $this->traitFormatResult($result);
+ }
+
}
diff --git a/src/Validation/ReadinessTrait.php b/src/Validation/ReadinessTrait.php
index 12b0df150f..a01906bd54 100644
--- a/src/Validation/ReadinessTrait.php
+++ b/src/Validation/ReadinessTrait.php
@@ -5,6 +5,7 @@ namespace Drupal\automatic_updates\Validation;
use Drupal\Core\Messenger\MessengerInterface;
use Drupal\Core\Render\RendererInterface;
use Drupal\Core\StringTranslation\TranslatableMarkup;
+use Drupal\package_manager\ValidationResult;
use Drupal\system\SystemManager;
/**
@@ -79,13 +80,8 @@ trait ReadinessTrait {
$build = [
'#theme' => 'item_list__automatic_updates_validation_results',
'#prefix' => $this->getFailureMessageForSeverity($severity),
+ '#items' => array_map([$this, 'formatResult'], $results),
];
- foreach ($results as $result) {
- $messages = $result->getMessages();
- // @todo Find a way to show all the messages, not just the summary, if
- // we're on the updater form in https://drupal.org/i/3284346.
- $build['#items'][] = count($messages) === 1 ? reset($messages) : $result->getSummary();
- }
$message = $renderer->renderRoot($build);
if ($severity === SystemManager::REQUIREMENT_ERROR) {
@@ -96,4 +92,19 @@ trait ReadinessTrait {
}
}
+ /**
+ * Formats a single validation result as an item in an item list.
+ *
+ * @param \Drupal\package_manager\ValidationResult $result
+ * A validation result.
+ *
+ * @return \Drupal\Core\StringTranslation\TranslatableMarkup|array
+ * The validation result, formatted for inclusion in a themed item list as
+ * either a translated string, or a renderable array.
+ */
+ protected function formatResult(ValidationResult $result) {
+ $messages = $result->getMessages();
+ return count($messages) === 1 ? reset($messages) : $result->getSummary();
+ }
+
}
diff --git a/tests/src/Functional/UpdaterFormTest.php b/tests/src/Functional/UpdaterFormTest.php
index cd1ec9b129..fd4a157606 100644
--- a/tests/src/Functional/UpdaterFormTest.php
+++ b/tests/src/Functional/UpdaterFormTest.php
@@ -168,9 +168,10 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase {
$this->setCoreVersion('9.8.0');
$this->checkForUpdates();
- // Set up a new fake error.
+ // Set up a new fake error. Use an error with multiple messages so we can
+ // ensure that they're all displayed, along with their summary.
$this->createTestValidationResults();
- $expected_results = $this->testResults['checker_1']['1 error'];
+ $expected_results = [$this->testResults['checker_1']['2 errors 2 warnings']['1:errors']];
TestSubscriber1::setTestResult($expected_results, ReadinessCheckEvent::class);
// If a validator raises an error during readiness checking, the form should
@@ -182,6 +183,8 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase {
// 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((string) $expected_results[0]->getMessages()[1]);
+ $assert_session->pageTextContainsOnce((string) $expected_results[0]->getSummary());
$assert_session->pageTextContainsOnce(static::$errorsExplanation);
$assert_session->pageTextNotContains(static::$warningsExplanation);
$assert_session->pageTextNotContains($cached_message);
@@ -218,9 +221,9 @@ class UpdaterFormTest extends AutomaticUpdatesFunctionalTestBase {
$this->assertUpdateStagedTimes(0);
$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->pageTextContainsOnce($expected_results[0]->getSummary());
$assert_session->pageTextContainsOnce((string) $expected_results[0]->getMessages()[0]);
+ $assert_session->pageTextContainsOnce((string) $expected_results[0]->getMessages()[1]);
$assert_session->pageTextNotContains($cached_message);
}
--
GitLab