diff --git a/package_manager/package_manager.api.php b/package_manager/package_manager.api.php index 3b565fec8cdaefd9041cb3a2367dc27675897ea6..5adbc6711ea613deb031075fd25e7bcfe21d73fa 100644 --- a/package_manager/package_manager.api.php +++ b/package_manager/package_manager.api.php @@ -248,12 +248,15 @@ * cycle from proceeding any further. If a validator encounters an exception, * it can use ::addErrorFromThrowable() instead of ::addError(). During status * checks, validators can call ::addWarning() for less severe problems -- - * warnings will NOT stop the stage life cycle. + * warnings will NOT stop the stage life cycle. All three are convenience + * methods for equivalent \Drupal\package_manager\ValidationResult constructors, + * which can then be added to the event using ::addResult(). * * @see \Drupal\package_manager\ValidationResult * @see \Drupal\package_manager\Event\PreOperationStageEvent::addError() * @see \Drupal\package_manager\Event\PreOperationStageEvent::addErrorFromThrowable() * @see \Drupal\package_manager\Event\StatusCheckEvent::addWarning() + * @see \Drupal\package_manager\Event\PreOperationStageEvent::addResult() * * @section sec_excluded_paths Excluding files from stage operations * Certain files are never copied into the stage directory because they are diff --git a/package_manager/src/Event/PreOperationStageEvent.php b/package_manager/src/Event/PreOperationStageEvent.php index 63d69971162f6ba27ec07e40dd94b5b62783b587..d4510043538ff43fec89552e0ca83d0989e0c2ce 100644 --- a/package_manager/src/Event/PreOperationStageEvent.php +++ b/package_manager/src/Event/PreOperationStageEvent.php @@ -40,7 +40,7 @@ abstract class PreOperationStageEvent extends StageEvent { } /** - * Adds error information to the event. + * Convenience method, adds error validation result. * * @param \Drupal\Core\StringTranslation\TranslatableMarkup[] $messages * The error messages. @@ -49,11 +49,11 @@ abstract class PreOperationStageEvent extends StageEvent { * is more than one message. */ public function addError(array $messages, ?TranslatableMarkup $summary = NULL): void { - $this->results[] = ValidationResult::createError(array_values($messages), $summary); + $this->addResult(ValidationResult::createError(array_values($messages), $summary)); } /** - * Adds an error from a throwable. + * Convenience method, adds an error validation result from a throwable. * * @param \Throwable $throwable * The throwable. @@ -61,7 +61,24 @@ abstract class PreOperationStageEvent extends StageEvent { * (optional) The summary of error messages. */ public function addErrorFromThrowable(\Throwable $throwable, ?TranslatableMarkup $summary = NULL): void { - $this->results[] = ValidationResult::createErrorFromThrowable($throwable, $summary); + $this->addResult(ValidationResult::createErrorFromThrowable($throwable, $summary)); + } + + /** + * Adds a validation result to the event. + * + * @param \Drupal\package_manager\ValidationResult $result + * The validation result to add. + * + * @throws \InvalidArgumentException + * Thrown if the validation result is not an error. + */ + public function addResult(ValidationResult $result): void { + // Only errors are allowed for this event. + if ($result->severity !== SystemManager::REQUIREMENT_ERROR) { + throw new \InvalidArgumentException('Only errors are allowed.'); + } + $this->results[] = $result; } /** diff --git a/package_manager/src/Event/StatusCheckEvent.php b/package_manager/src/Event/StatusCheckEvent.php index 41dc77cde77a1c1deea038623a2b58faf0a31f3b..c50629af1c79e08d72a71a17fd7c941130c071e6 100644 --- a/package_manager/src/Event/StatusCheckEvent.php +++ b/package_manager/src/Event/StatusCheckEvent.php @@ -60,7 +60,15 @@ final class StatusCheckEvent extends PreOperationStageEvent { * message, optional otherwise. */ public function addWarning(array $messages, ?TranslatableMarkup $summary = NULL): void { - $this->results[] = ValidationResult::createWarning($messages, $summary); + $this->addResult(ValidationResult::createWarning($messages, $summary)); + } + + /** + * {@inheritdoc} + */ + public function addResult(ValidationResult $result): void { + // Override the parent to also allow warnings. + $this->results[] = $result; } } diff --git a/package_manager/tests/modules/package_manager_test_validation/src/EventSubscriber/TestSubscriber.php b/package_manager/tests/modules/package_manager_test_validation/src/EventSubscriber/TestSubscriber.php index 4a74187565e53a3d056d315c0552d1a9e8d5b443..260c0761ab483c025584d6db70009bd5c5ca5074 100644 --- a/package_manager/tests/modules/package_manager_test_validation/src/EventSubscriber/TestSubscriber.php +++ b/package_manager/tests/modules/package_manager_test_validation/src/EventSubscriber/TestSubscriber.php @@ -15,7 +15,6 @@ use Drupal\package_manager\Event\PreDestroyEvent; use Drupal\package_manager\Event\PreRequireEvent; use Drupal\package_manager\Event\StageEvent; use Drupal\package_manager\Event\StatusCheckEvent; -use Drupal\system\SystemManager; use Symfony\Component\EventDispatcher\EventSubscriberInterface; /** @@ -144,12 +143,7 @@ class TestSubscriber implements EventSubscriberInterface { } /** @var \Drupal\package_manager\ValidationResult $result */ foreach ($results as $result) { - if ($result->severity === SystemManager::REQUIREMENT_ERROR) { - $event->addError($result->messages, $result->summary); - } - else { - $event->addWarning($result->messages, $result->summary); - } + $event->addResult($result); } } diff --git a/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php b/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php index 909ab4017b15bd5531e4eff243e4599d76325fb4..e3fec5d5704a735c0834caaf081be1c8c247a492 100644 --- a/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php +++ b/package_manager/tests/src/Kernel/PackageManagerKernelTestBase.php @@ -20,7 +20,6 @@ use Drupal\package_manager\PathLocator; use Drupal\package_manager\StatusCheckTrait; use Drupal\package_manager\Validator\DiskSpaceValidator; use Drupal\package_manager\StageBase; -use Drupal\system\SystemManager; use Drupal\Tests\package_manager\Traits\AssertPreconditionsTrait; use Drupal\Tests\package_manager\Traits\FixtureManipulatorTrait; use Drupal\Tests\package_manager\Traits\FixtureUtilityTrait; @@ -456,12 +455,7 @@ abstract class PackageManagerKernelTestBase extends KernelTestBase { */ protected function createStageEventExceptionFromResults(array $expected_results, string $event_class = PreCreateEvent::class, StageBase $stage = NULL): StageEventException { $event = new $event_class($stage ?? $this->createStage(), []); - - foreach ($expected_results as $result) { - if ($result->severity === SystemManager::REQUIREMENT_ERROR) { - $event->addError($result->messages, $result->summary); - } - } + array_walk($expected_results, $event->addResult(...)); return new StageEventException($event); } diff --git a/package_manager/tests/src/Kernel/StageEventsTest.php b/package_manager/tests/src/Kernel/StageEventsTest.php index 2429dbb59e4bc3c1c55308bf95f7b6a2c2cb8a79..1ba4a48f3533dc810c1a57a98d56fcd482b95356 100644 --- a/package_manager/tests/src/Kernel/StageEventsTest.php +++ b/package_manager/tests/src/Kernel/StageEventsTest.php @@ -15,6 +15,7 @@ use Drupal\package_manager\Event\PreDestroyEvent; use Drupal\package_manager\Event\PreOperationStageEvent; use Drupal\package_manager\Event\PreRequireEvent; use Drupal\package_manager\Event\StageEvent; +use Drupal\package_manager\Event\StatusCheckEvent; use Drupal\package_manager\Exception\StageEventException; use Drupal\package_manager\ValidationResult; use Symfony\Component\EventDispatcher\EventSubscriberInterface; @@ -155,6 +156,35 @@ class StageEventsTest extends PackageManagerKernelTestBase implements EventSubsc $this->assertResults([$result], $event_class); } + /** + * Tests adding validation results to events. + */ + public function testAddResult(): void { + $stage = $this->createStage(); + + $error = ValidationResult::createError([ + t('Burn, baby, burn!'), + ]); + $warning = ValidationResult::createWarning([ + t('The path ahead is scary...'), + ]); + + // Status check events can accept both errors and warnings. + $event = new StatusCheckEvent($stage, NULL); + $event->addResult($error); + $event->addResult($warning); + $this->assertSame([$error, $warning], $event->getResults()); + + // Other stage events will accept errors, but throw an exception if you try + // to add a warning. + $event = new PreCreateEvent($stage, []); + $event->addResult($error); + $this->assertSame([$error], $event->getResults()); + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Only errors are allowed.'); + $event->addResult($warning); + } + /** * Tests that pre- and post-require events have access to the package lists. */ diff --git a/src/Validation/AdminStatusCheckMessages.php b/src/Validation/AdminStatusCheckMessages.php index 4e0d9df309485515525cae9e5835002b53c225ba..47afe48ccb94409c141ceac24901fa2448f3dae6 100644 --- a/src/Validation/AdminStatusCheckMessages.php +++ b/src/Validation/AdminStatusCheckMessages.php @@ -201,7 +201,7 @@ final class AdminStatusCheckMessages implements ContainerInjectionInterface { // multiple messages. This is because, on regular admin pages, we merely // want to alert users that problems exist, but not burden them with the // details. They can get those on the status report and updater form. - $format_result = function (ValidationResult $result): TranslatableMarkup { + $format_result = function (ValidationResult $result): TranslatableMarkup|string { $messages = $result->messages; return $result->summary ?: reset($messages); }; diff --git a/tests/src/Functional/UpdateErrorTest.php b/tests/src/Functional/UpdateErrorTest.php index 803ebea0e806695f48a7d9f00825aa6cb3a04e1e..2111614ce2689a35a1b8d3cc653a4c24cbdd1b56 100644 --- a/tests/src/Functional/UpdateErrorTest.php +++ b/tests/src/Functional/UpdateErrorTest.php @@ -76,6 +76,21 @@ class UpdateErrorTest extends UpdaterFormTestBase { $assert_session->buttonExists('Cancel update'); } + /** + * Tests that throwables will be displayed properly. + */ + public function testDisplayErrorCreatedFromThrowable(): void { + $throwable = new \Exception("I want to be the pirate king because he's the freest man alive."); + $result = ValidationResult::createErrorFromThrowable($throwable); + TestSubscriber1::setTestResult([$result], StatusCheckEvent::class); + $this->drupalGet('/admin/reports/status'); + $this->clickLink('Rerun readiness checks'); + $this->drupalGet('/admin'); + $assert_session = $this->assertSession(); + $assert_session->statusCodeEquals(200); + $assert_session->statusMessageContains($throwable->getMessage(), 'error'); + } + /** * Tests the display of errors and warnings during status check. */