From 9c25560ff565891f365c45abeecf830debb8b102 Mon Sep 17 00:00:00 2001
From: phenaproxima <phenaproxima@205645.no-reply.drupal.org>
Date: Mon, 6 Jun 2022 20:48:26 +0000
Subject: [PATCH] Issue #3277007 by phenaproxima, chrisfromredfin, tedbow,
Theresa.Grannum: Validation messages displayed on admin pages should be
grouped together so it's clear they are all related to automatic updates
---
.../src/Form/UpdaterForm.php | 16 +++-
src/Form/UpdaterForm.php | 18 ++++-
src/Validation/AdminReadinessMessages.php | 18 ++++-
src/Validation/ReadinessTrait.php | 36 +++++----
.../{Unit => Kernel}/ReadinessTraitTest.php | 74 +++++--------------
5 files changed, 84 insertions(+), 78 deletions(-)
rename tests/src/{Unit => Kernel}/ReadinessTraitTest.php (52%)
diff --git a/automatic_updates_extensions/src/Form/UpdaterForm.php b/automatic_updates_extensions/src/Form/UpdaterForm.php
index b059c9df51..46915c1ae6 100644
--- a/automatic_updates_extensions/src/Form/UpdaterForm.php
+++ b/automatic_updates_extensions/src/Form/UpdaterForm.php
@@ -9,6 +9,7 @@ use Drupal\automatic_updates_extensions\ExtensionUpdater;
use Drupal\Core\Batch\BatchBuilder;
use Drupal\Core\Form\FormBase;
use Drupal\Core\Form\FormStateInterface;
+use Drupal\Core\Render\RendererInterface;
use Drupal\system\SystemManager;
use Drupal\update\UpdateManagerInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;
@@ -38,6 +39,13 @@ final class UpdaterForm extends FormBase {
*/
private $eventDispatcher;
+ /**
+ * The renderer service.
+ *
+ * @var \Drupal\Core\Render\RendererInterface
+ */
+ private $renderer;
+
/**
* {@inheritdoc}
*/
@@ -45,6 +53,7 @@ final class UpdaterForm extends FormBase {
return new static(
$container->get('automatic_updates_extensions.updater'),
$container->get('event_dispatcher'),
+ $container->get('renderer')
);
}
@@ -55,10 +64,13 @@ final class UpdaterForm extends FormBase {
* The extension updater service.
* @param \Symfony\Component\EventDispatcher\EventDispatcherInterface $event_dispatcher
* The extension event dispatcher service.
+ * @param \Drupal\Core\Render\RendererInterface $renderer
+ * The renderer service.
*/
- public function __construct(ExtensionUpdater $extension_updater, EventDispatcherInterface $event_dispatcher) {
+ public function __construct(ExtensionUpdater $extension_updater, EventDispatcherInterface $event_dispatcher, RendererInterface $renderer) {
$this->extensionUpdater = $extension_updater;
$this->eventDispatcher = $event_dispatcher;
+ $this->renderer = $renderer;
}
/**
@@ -120,7 +132,7 @@ final class UpdaterForm extends FormBase {
$this->eventDispatcher->dispatch($event);
$results = $event->getResults();
}
- $this->displayResults($results, $this->messenger());
+ $this->displayResults($results, $this->messenger(), $this->renderer);
$security_level = $this->getOverallSeverity($results);
if ($update_projects && $security_level !== SystemManager::REQUIREMENT_ERROR) {
diff --git a/src/Form/UpdaterForm.php b/src/Form/UpdaterForm.php
index 82977859cd..7a8e8250c8 100644
--- a/src/Form/UpdaterForm.php
+++ b/src/Form/UpdaterForm.php
@@ -13,6 +13,7 @@ use Drupal\Core\Form\FormBase;
use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Link;
use Drupal\Core\Messenger\MessengerInterface;
+use Drupal\Core\Render\RendererInterface;
use Drupal\Core\State\StateInterface;
use Drupal\Core\Url;
use Drupal\package_manager\Exception\StageException;
@@ -60,6 +61,13 @@ class UpdaterForm extends FormBase {
*/
protected $releaseChooser;
+ /**
+ * The renderer service.
+ *
+ * @var \Drupal\Core\Render\RendererInterface
+ */
+ protected $renderer;
+
/**
* Constructs a new UpdaterForm object.
*
@@ -71,12 +79,15 @@ class UpdaterForm extends FormBase {
* The event dispatcher service.
* @param \Drupal\automatic_updates\ReleaseChooser $release_chooser
* The release chooser service.
+ * @param \Drupal\Core\Render\RendererInterface $renderer
+ * The renderer service.
*/
- public function __construct(StateInterface $state, Updater $updater, EventDispatcherInterface $event_dispatcher, ReleaseChooser $release_chooser) {
+ public function __construct(StateInterface $state, Updater $updater, EventDispatcherInterface $event_dispatcher, ReleaseChooser $release_chooser, RendererInterface $renderer) {
$this->updater = $updater;
$this->state = $state;
$this->eventDispatcher = $event_dispatcher;
$this->releaseChooser = $release_chooser;
+ $this->renderer = $renderer;
}
/**
@@ -94,7 +105,8 @@ class UpdaterForm extends FormBase {
$container->get('state'),
$container->get('automatic_updates.updater'),
$container->get('event_dispatcher'),
- $container->get('automatic_updates.release_chooser')
+ $container->get('automatic_updates.release_chooser'),
+ $container->get('renderer')
);
}
@@ -252,7 +264,7 @@ class UpdaterForm extends FormBase {
$this->eventDispatcher->dispatch($event);
$results = $event->getResults();
}
- $this->displayResults($results, $this->messenger());
+ $this->displayResults($results, $this->messenger(), $this->renderer);
if ($stage_exists) {
// If the form has been submitted, do not display this error message
diff --git a/src/Validation/AdminReadinessMessages.php b/src/Validation/AdminReadinessMessages.php
index 82084f29ce..b5e2777d15 100644
--- a/src/Validation/AdminReadinessMessages.php
+++ b/src/Validation/AdminReadinessMessages.php
@@ -6,6 +6,7 @@ use Drupal\automatic_updates\CronUpdater;
use Drupal\Core\DependencyInjection\ContainerInjectionInterface;
use Drupal\Core\Messenger\MessengerInterface;
use Drupal\Core\Messenger\MessengerTrait;
+use Drupal\Core\Render\RendererInterface;
use Drupal\Core\Routing\AdminContext;
use Drupal\Core\Routing\CurrentRouteMatch;
use Drupal\Core\Routing\RedirectDestinationTrait;
@@ -65,6 +66,13 @@ final class AdminReadinessMessages implements ContainerInjectionInterface {
*/
protected $cronUpdater;
+ /**
+ * The renderer service.
+ *
+ * @var \Drupal\Core\Render\RendererInterface
+ */
+ protected $renderer;
+
/**
* Constructs a ReadinessRequirement object.
*
@@ -82,8 +90,10 @@ final class AdminReadinessMessages implements ContainerInjectionInterface {
* The current route match.
* @param \Drupal\automatic_updates\CronUpdater $cron_updater
* The cron updater service.
+ * @param \Drupal\Core\Render\RendererInterface $renderer
+ * The renderer service.
*/
- public function __construct(ReadinessValidationManager $readiness_checker_manager, MessengerInterface $messenger, AdminContext $admin_context, AccountProxyInterface $current_user, TranslationInterface $translation, CurrentRouteMatch $current_route_match, CronUpdater $cron_updater) {
+ public function __construct(ReadinessValidationManager $readiness_checker_manager, MessengerInterface $messenger, AdminContext $admin_context, AccountProxyInterface $current_user, TranslationInterface $translation, CurrentRouteMatch $current_route_match, CronUpdater $cron_updater, RendererInterface $renderer) {
$this->readinessCheckerManager = $readiness_checker_manager;
$this->setMessenger($messenger);
$this->adminContext = $admin_context;
@@ -91,6 +101,7 @@ final class AdminReadinessMessages implements ContainerInjectionInterface {
$this->setStringTranslation($translation);
$this->currentRouteMatch = $current_route_match;
$this->cronUpdater = $cron_updater;
+ $this->renderer = $renderer;
}
/**
@@ -104,7 +115,8 @@ final class AdminReadinessMessages implements ContainerInjectionInterface {
$container->get('current_user'),
$container->get('string_translation'),
$container->get('current_route_match'),
- $container->get('automatic_updates.cron_updater')
+ $container->get('automatic_updates.cron_updater'),
+ $container->get('renderer')
);
}
@@ -167,7 +179,7 @@ final class AdminReadinessMessages implements ContainerInjectionInterface {
if (empty($results)) {
return FALSE;
}
- $this->displayResults($results, $this->messenger());
+ $this->displayResults($results, $this->messenger(), $this->renderer);
return TRUE;
}
diff --git a/src/Validation/ReadinessTrait.php b/src/Validation/ReadinessTrait.php
index 9966f24b5c..12b0df150f 100644
--- a/src/Validation/ReadinessTrait.php
+++ b/src/Validation/ReadinessTrait.php
@@ -3,11 +3,17 @@
namespace Drupal\automatic_updates\Validation;
use Drupal\Core\Messenger\MessengerInterface;
+use Drupal\Core\Render\RendererInterface;
use Drupal\Core\StringTranslation\TranslatableMarkup;
use Drupal\system\SystemManager;
/**
* Common methods for working with readiness checkers.
+ *
+ * @internal
+ * This class implements logic to output the messages from readiness checkers
+ * on admin pages. It may be changed or removed at any time without warning
+ * and should not be used by external code.
*/
trait ReadinessTrait {
@@ -59,33 +65,35 @@ trait ReadinessTrait {
* The validation results.
* @param \Drupal\Core\Messenger\MessengerInterface $messenger
* The messenger service.
+ * @param \Drupal\Core\Render\RendererInterface $renderer
+ * The renderer service.
*/
- protected function displayResults(array $results, MessengerInterface $messenger): void {
+ protected function displayResults(array $results, MessengerInterface $messenger, RendererInterface $renderer): void {
$severity = $this->getOverallSeverity($results);
if ($severity === SystemManager::REQUIREMENT_OK) {
return;
}
- $message = $this->getFailureMessageForSeverity($severity);
+ // Format the results as a single item list prefixed by a preamble message.
+ $build = [
+ '#theme' => 'item_list__automatic_updates_validation_results',
+ '#prefix' => $this->getFailureMessageForSeverity($severity),
+ ];
+ 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) {
$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);
- }
- }
}
}
diff --git a/tests/src/Unit/ReadinessTraitTest.php b/tests/src/Kernel/ReadinessTraitTest.php
similarity index 52%
rename from tests/src/Unit/ReadinessTraitTest.php
rename to tests/src/Kernel/ReadinessTraitTest.php
index 808927a099..6fb6c3423d 100644
--- a/tests/src/Unit/ReadinessTraitTest.php
+++ b/tests/src/Kernel/ReadinessTraitTest.php
@@ -1,25 +1,19 @@
<?php
-namespace Drupal\Tests\automatic_updates\Unit;
+namespace Drupal\Tests\automatic_updates\Kernel;
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\Messenger\MessengerInterface;
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 {
+class ReadinessTraitTest extends AutomaticUpdatesKernelTestBase {
use ReadinessTrait;
use StringTranslationTrait;
@@ -49,47 +43,43 @@ class ReadinessTraitTest extends UnitTestCase {
* @covers ::displayResults
*/
public function testDisplayResults(): void {
- $messenger = new Messenger(new FlashBag(), new KillSwitch());
-
- $translation = new TestTranslationManager();
- $this->setStringTranslation($translation);
+ $messenger = $this->container->get('messenger');
+ $renderer = $this->container->get('renderer');
// 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', 'Thursday'], $this->t('The Addams Family')),
+ ValidationResult::createError(['Wednesday', 'Lurch'], $this->t('The Addams Family')),
ValidationResult::createWarning(['Moo!']),
ValidationResult::createWarning(['Shaggy', 'The dog'], $this->t('Mystery Mobile')),
];
- $this->displayResults($results, $messenger);
+ $this->displayResults($results, $messenger, $renderer);
+ $failure_message = (string) $this->getFailureMessageForSeverity(SystemManager::REQUIREMENT_ERROR);
$expected_errors = [
- (string) $this->getFailureMessageForSeverity(SystemManager::REQUIREMENT_ERROR),
- 'Boo!',
- 'The Addams Family',
+ "$failure_message<ul><li>Boo!</li><li>The Addams Family</li><li>Moo!</li><li>Mystery Mobile</li></ul>",
];
- $actual_errors = array_map('strval', $messenger->deleteByType(Messenger::TYPE_ERROR));
+ $actual_errors = array_map('strval', $messenger->deleteByType(MessengerInterface::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);
+ // Even though there were warnings, they should have been included with the
+ // errors.
+ $actual_warnings = array_map('strval', $messenger->deleteByType(MessengerInterface::TYPE_WARNING));
+ $this->assertEmpty($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);
+ $this->displayResults($results, $messenger, $renderer);
+ $failure_message = (string) $this->getFailureMessageForSeverity(SystemManager::REQUIREMENT_WARNING);
$expected_warnings = [
- (string) $this->getFailureMessageForSeverity(SystemManager::REQUIREMENT_WARNING),
- 'Moo!',
- 'Mystery Mobile',
+ "$failure_message<ul><li>Moo!</li><li>Mystery Mobile</li></ul>",
];
- $actual_warnings = array_map('strval', $messenger->deleteByType(Messenger::TYPE_WARNING));
+ $actual_warnings = array_map('strval', $messenger->deleteByType(MessengerInterface::TYPE_WARNING));
$this->assertSame($expected_warnings, $actual_warnings);
// There shouldn't be any more messages.
@@ -97,31 +87,3 @@ class ReadinessTraitTest extends UnitTestCase {
}
}
-
-/**
- * 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);
- }
-
-}
--
GitLab