From 57260445dafa53e317790c732b8b9f06d33d7507 Mon Sep 17 00:00:00 2001
From: Adam G-H <32250-phenaproxima@users.noreply.drupalcode.org>
Date: Thu, 6 Feb 2025 21:57:27 +0000
Subject: [PATCH] Issue #3494512: Package Manager validation is only necessary
if the project to install is not already in the filesystem
---
src/Controller/InstallerController.php | 43 ++++++++++++
src/Element/ProjectBrowser.php | 22 +-----
src/InstallReadiness.php | 63 ------------------
src/ProjectBrowserServiceProvider.php | 3 -
sveltejs/public/build/bundle.js | Bin 299622 -> 298920 bytes
sveltejs/public/build/bundle.js.map | Bin 270987 -> 269655 bytes
sveltejs/src/DetailModal.svelte | 2 +-
sveltejs/src/Project/ActionButton.svelte | 2 +-
sveltejs/src/ProjectBrowser.svelte | 22 ------
sveltejs/src/ProjectGrid.svelte | 2 +-
.../src/ProjectBrowserTestServiceProvider.php | 6 +-
.../ProjectBrowserInstallerUiTest.php | 16 +++--
12 files changed, 60 insertions(+), 121 deletions(-)
delete mode 100644 src/InstallReadiness.php
diff --git a/src/Controller/InstallerController.php b/src/Controller/InstallerController.php
index 0be6732ac..f91671d3f 100644
--- a/src/Controller/InstallerController.php
+++ b/src/Controller/InstallerController.php
@@ -8,24 +8,29 @@ use Drupal\Core\Access\AccessResult;
use Drupal\Core\Controller\ControllerBase;
use Drupal\Core\Url;
use Drupal\package_manager\Exception\StageException;
+use Drupal\package_manager\StatusCheckTrait;
use Drupal\project_browser\ActivatorInterface;
use Drupal\project_browser\ComposerInstaller\Installer;
use Drupal\project_browser\EnabledSourceHandler;
use Drupal\project_browser\InstallState;
use Drupal\project_browser\ProjectBrowser\Project;
use Drupal\project_browser_test\Plugin\ProjectBrowserSource\ProjectBrowserTestMock;
+use Drupal\system\SystemManager;
use Psr\Log\LoggerInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\HttpFoundation\JsonResponse;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
+use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
/**
* Defines a controller to install projects via UI.
*/
final class InstallerController extends ControllerBase {
+ use StatusCheckTrait;
+
/**
* No require or install in progress for a given module.
*
@@ -61,6 +66,7 @@ final class InstallerController extends ControllerBase {
private readonly LoggerInterface $logger,
private readonly ActivatorInterface $activator,
private readonly InstallState $installState,
+ private readonly EventDispatcherInterface $eventDispatcher,
) {}
/**
@@ -78,6 +84,7 @@ final class InstallerController extends ControllerBase {
$container->get('logger.channel.project_browser'),
$container->get(ActivatorInterface::class),
$container->get(InstallState::class),
+ $container->get(EventDispatcherInterface::class),
);
}
@@ -323,6 +330,16 @@ final class InstallerController extends ControllerBase {
return $this->lockedResponse($message, $unlock_url);
}
+ // Ensure the environment is ready to use Package Manager.
+ ['errors' => $errors, 'warnings' => $warnings] = $this->validatePackageManager();
+ if ($warnings) {
+ $this->logger->warning(implode("\n", $warnings));
+ }
+ if ($errors) {
+ $error_message = '<ul><li>' . implode('</li><li>', $errors) . '</li></ul>';
+ return $this->errorResponse(new StageException($this->installer, $error_message));
+ }
+
try {
$stage_id = $this->installer->create();
}
@@ -464,4 +481,30 @@ final class InstallerController extends ControllerBase {
return $response ?? new JsonResponse(['status' => 0]);
}
+ /**
+ * Checks if the environment meets Package Manager install requirements.
+ *
+ * @return array[]
+ * An array with two sub-elements:
+ * - errors: The validation messages with an "error" severity.
+ * - warnings: All other validation messages, which are probably warnings.
+ */
+ private function validatePackageManager(): array {
+ $results = [
+ 'errors' => [],
+ 'warnings' => [],
+ ];
+ foreach ($this->runStatusCheck($this->installer, $this->eventDispatcher) as $result) {
+ $group = $result->severity === SystemManager::REQUIREMENT_ERROR
+ ? 'errors'
+ : 'warnings';
+
+ if ($result->summary) {
+ $results[$group][] = $result->summary;
+ }
+ $results[$group] = array_merge($results[$group], $result->messages);
+ }
+ return $results;
+ }
+
}
diff --git a/src/Element/ProjectBrowser.php b/src/Element/ProjectBrowser.php
index 00d0dee06..254d54761 100644
--- a/src/Element/ProjectBrowser.php
+++ b/src/Element/ProjectBrowser.php
@@ -13,7 +13,6 @@ use Drupal\Core\Render\Attribute\RenderElement;
use Drupal\Core\Render\Element;
use Drupal\Core\Render\Element\ElementInterface;
use Drupal\Core\Render\Element\RenderElementBase;
-use Drupal\project_browser\InstallReadiness;
use Drupal\project_browser\Plugin\ProjectBrowserSourceInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;
@@ -30,7 +29,6 @@ final class ProjectBrowser implements ElementInterface, ContainerFactoryPluginIn
public function __construct(
private readonly string $pluginId,
private readonly mixed $pluginDefinition,
- private readonly ?InstallReadiness $installReadiness,
private readonly ModuleHandlerInterface $moduleHandler,
private readonly ConfigFactoryInterface $configFactory,
private readonly UuidInterface $uuid,
@@ -55,13 +53,9 @@ final class ProjectBrowser implements ElementInterface, ContainerFactoryPluginIn
* {@inheritdoc}
*/
public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition): static {
- $install_readiness = $container->get(InstallReadiness::class, ContainerInterface::NULL_ON_INVALID_REFERENCE);
- assert(is_null($install_readiness) || $install_readiness instanceof InstallReadiness);
-
return new static(
$plugin_id,
$plugin_definition,
- $install_readiness,
$container->get(ModuleHandlerInterface::class),
$container->get(ConfigFactoryInterface::class),
$container->get(UuidInterface::class),
@@ -127,25 +121,11 @@ final class ProjectBrowser implements ElementInterface, ContainerFactoryPluginIn
if (is_int($max_selections) && $max_selections <= 0) {
throw new \InvalidArgumentException('$max_selections must be a positive integer or NULL.');
}
-
- $package_manager = [
- 'available' => $this->configFactory->get('project_browser.admin_settings')->get('allow_ui_install') && is_object($this->installReadiness),
- 'errors' => [],
- 'warnings' => [],
- 'status_checked' => FALSE,
- ];
- // @todo Fix https://www.drupal.org/node/3494512 to avoid adding
- // hard-coded values. #techdebt
- if ($source->getPluginId() !== 'recipes' && $package_manager['available'] && is_object($this->installReadiness)) {
- $package_manager = array_merge($package_manager, $this->installReadiness->validatePackageManager());
- $package_manager['status_checked'] = TRUE;
- }
-
$sort_options = $source->getSortOptions();
return [
'module_path' => $this->moduleHandler->getModule('project_browser')->getPath(),
'default_plugin_id' => $source->getPluginId(),
- 'package_manager' => $package_manager,
+ 'package_manager' => $this->configFactory->get('project_browser.admin_settings')->get('allow_ui_install') && $this->moduleHandler->moduleExists('package_manager'),
'max_selections' => $max_selections,
'current_path' => '/' . $this->currentPath->getPath(),
'instances' => [
diff --git a/src/InstallReadiness.php b/src/InstallReadiness.php
deleted file mode 100644
index c3e6ae040..000000000
--- a/src/InstallReadiness.php
+++ /dev/null
@@ -1,63 +0,0 @@
-<?php
-
-namespace Drupal\project_browser;
-
-use Drupal\package_manager\StatusCheckTrait;
-use Drupal\project_browser\ComposerInstaller\Installer;
-use Drupal\system\SystemManager;
-use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
-
-/**
- * Defines Installer service.
- */
-class InstallReadiness {
-
- use StatusCheckTrait;
-
- public function __construct(
- private readonly Installer $installer,
- private readonly EventDispatcherInterface $eventDispatcher,
- ) {}
-
- /**
- * Checks if the environment meets Package Manager install requirements.
- *
- * @return array[]
- * errors - an array of messages with severity 2
- * messages - all other messages below severity 2 (warnings)
- */
- public function validatePackageManager() {
- $errors = [];
- $warnings = [];
- foreach ($this->runStatusCheck($this->installer, $this->eventDispatcher) as $result) {
- $messages = $result->messages;
- $summary = $result->summary;
- if ($summary) {
- array_unshift($messages, $summary);
- }
- $text = implode("\n", $messages);
-
- if ($result->severity === SystemManager::REQUIREMENT_ERROR) {
- $errors[] = $text;
- }
- else {
- $warnings[] = $text;
- }
- }
- return [
- 'errors' => $errors,
- 'warnings' => $warnings,
- ];
- }
-
- /**
- * Checks if the installer is available.
- *
- * @return bool
- * If the installer is currently available.
- */
- public function installerAvailable() {
- return $this->installer->isAvailable();
- }
-
-}
diff --git a/src/ProjectBrowserServiceProvider.php b/src/ProjectBrowserServiceProvider.php
index 595a6a4d9..1dcd23e1d 100644
--- a/src/ProjectBrowserServiceProvider.php
+++ b/src/ProjectBrowserServiceProvider.php
@@ -41,9 +41,6 @@ class ProjectBrowserServiceProvider extends ServiceProviderBase {
->setArgument('$keyValueFactory', new Reference('keyvalue'))
->setAutowired(TRUE);
- $container->register(InstallReadiness::class, InstallReadiness::class)
- ->setAutowired(TRUE);
-
$container->register(CoreNotUpdatedValidator::class, CoreNotUpdatedValidator::class)
->addTag('event_subscriber')
->setAutowired(TRUE);
diff --git a/sveltejs/public/build/bundle.js b/sveltejs/public/build/bundle.js
index 8027f477d5716190e8c5bc1eaeb350d90a124a48..252132cb62651d7b2b0f2dfc5fdd969dafb7d84e 100644
GIT binary patch
delta 185
zcmaF1L}<l)p@uDtcXn@owwsZicl-Afj57{wKhVi^SaSNrT1NTpmlreLTmoiJ&(C4z
z*nZc8xkP#T#vVqI?Lhtwq3wb7%mNFU49vGDE3mv`VKOk=?yt%s#Li@5usvCag@u*L
z)NFgEA<IuDCR5YxDJCp8nVHPYw%c2<nDDZh7+RTVDokd4D!RSTl|_=5%@84QD2U}Z
e3zM<s_Wj{3jIz`BN;9!fwtgV9owuANQW^lRRzDB`
delta 835
zcmZ26U+CEqp@uDtcXo5>C6*;-<|HQNq)r!D%A~Sw594DVK7^oxnwrA&g-e*kxBolA
zSbM+_RVKjE+1t_GHQv|J4@d>+r4|+C7ZvN}q~@iUWGL9$+A0`KKe&WRV(Lv6!RZZ~
zm^inu?PQuLsZ4|^)8}4f*4-YvnCa#c4w&-k8~K?frhhmBG$+)9xki~CDluI#hgp34
z(`@D&LP-AEzPFB9U?G!{<@WRPEU#FYj4ZbAR$&ohXEHO|eqNh}g_X(NV*6DCmY+;a
z=H}Zk7_;1DX0ou@zS^9{gqO+8c>6;a79(CJW8>*ShU9j=AeP@OOlF4LwZmB$Wv9z>
zG4f1bSI5j>4|7~jDk!Wp@CQjvjRLxY^2DON%)E41sA*~{RC6f+0Vfb-rV%huQ=uNA
zniJh<Xmq6I7r7=TXK17<*xDf(&dHgZT3if_s?;LA#FP|`gwQ-tGEgYVS4hb(&&$bA
zOi{?qPbtkwEmkPYOjIbzNL2{+v{F#1POVMQQK(iZsVqpfnl4ttBErncSv!5BAd^@<
zHW#D&5ZMjrPKK)`*3lsusYR&@xrvnuNvR5%{>8<mK<AccWF}`Qq^6~%CYQiH0}N!C
vf6zUlpboWHU7=Rf8Xo*WPc59uC^}u?0VBtBo3Bj#lTY8{-R@t)5-AM;7P1P1
diff --git a/sveltejs/public/build/bundle.js.map b/sveltejs/public/build/bundle.js.map
index 12e6f84478a3785c493a3b22f5cc6b21035b4c04..da849d5c90149ad2b157ea1e813ac8bb8ebbf0e5 100644
GIT binary patch
delta 232
zcmeA^DsX+5K*JWsUk9eId&#J_UF#rYgZlQ{FBnfMZePd66l&7G+L3AdYDeZIaTZTU
z*MjK@|Cq%DQXL&rb<&+39X)m29UV)ZrU$w(i%fTlWL9CSa+)sK$1JkFCX!j)dio|C
zW)W6T$8zVe?f0iJCzuKsIynaBIO%{5@N{%^bkWJ3o><E)2{hb4ZTkAt%o@}GonjW>
ze&!UjDl1nwT=8`N3(V$>Y10$lF^g@FyTDwa$&?+q{lZUXUtXrH!0qzvEPuG!N}U|t
b^IWDYTw@m9t{}=1$jxNxx4l?_MS=+c;^9tQ
delta 1312
zcmaKqTSyd99LCu>Yp$AUu4ZA{F)zCd!*&rhTr|$k4$GvgrYQ)bj=N{wv31sV*WJ8j
zK1K8pVs(0oeCesc2XU$=2_%Aq5F#)vC8D60?4bt<YA$<m*Ki(YhI792`@Zi#^WkOw
zoz?uCOS;{aY%3q(1wTLN$I|YK?uqGKasgIX!*;|pBPZMMLap+n!-|8!Krm$WV{s_#
zgN==i@T~OsyY5Z-9uS!=pu*B-Hnm+d(<?&t61>nHe~0_1i43x3Qe{5ad}?YRC&~v!
zK^%(bv-IMay#-Dtxk{D;(JAWvX=5|+27)Z__L<Fie&Kmx-v|zc<f-7GmG^ke16`sr
zQy30FParD#1H1>01UzGY9D)&nhhZOvU0egKoW#=u^xkyZyu#z7xB;3}D-+!-&TqXa
znZLc1-+yq8^Z}%T(@Q7Rsa0lDt0Wz@XaiS#IEdj0KMn^m6hfgfxl`09xP1_Nz1SU2
zG?qU_ymY*2I&2cgn#Qu2;51#3Dzez*`ZG&qWcjP^#I5xvN?LCMt;IzFJ7P!eG)4&1
z3_=XuiBKoGdk>r>?{a~j^mT(uok*^^L6P*Z8x-4)Bvdqq5J$E*KoN=dfC>YLqD+m0
zW*@A9Lg~{SxKwN9&}F|};UYzh6gZ-t$}QDU-F1*z0hEHa2|Gbwz$q#E8n78v8bUpz
zBGL}BD)#;kFp{mepj0<O+JAsr^0OZ3rMflHYRQgv@QS6AX2`-Ta9UdY4JvY}NQd+u
z=>O!DaflmsXqo=QCm%OJNtO#Cm(~{jq6*2QZ{Sd#T1s)Z^&qz1q0QmxRmwox%pfE4
zjGb<lH?`$wo0Q=gtIbz2v2loz1u_<qxzCi5{N+Jr)mR*Pp3$TXVj>Q0dTLGOMi%q*
z<r%VPvzH{okQOsAWDYZ$rMi4$mC8!Dlm)4lbs>k0q*=sVNLpS`w^W<UBE+gc2hEbX
hEnqxsN3IFGw&x^Amw{2*$<w#wQ2uu5Vu}7Z^$)4dl6U|B
diff --git a/sveltejs/src/DetailModal.svelte b/sveltejs/src/DetailModal.svelte
index aa530ab20..3744187dc 100644
--- a/sveltejs/src/DetailModal.svelte
+++ b/sveltejs/src/DetailModal.svelte
@@ -58,7 +58,7 @@
</div>
<div class="pb-detail-modal__sidebar">
<!-- <Image sources={project.logo} class="pb-detail-modal__project-logo" /> -->
- {#if PACKAGE_MANAGER.available}
+ {#if PACKAGE_MANAGER}
<div
class="pb-detail-modal__view-commands pb-detail-modal__sidebar_element"
>
diff --git a/sveltejs/src/Project/ActionButton.svelte b/sveltejs/src/Project/ActionButton.svelte
index e00da38a3..28f59602c 100644
--- a/sveltejs/src/Project/ActionButton.svelte
+++ b/sveltejs/src/Project/ActionButton.svelte
@@ -67,7 +67,7 @@
</ProjectStatusIndicator>
{:else}
<span>
- {#if PACKAGE_MANAGER.available && PACKAGE_MANAGER.errors.length === 0}
+ {#if PACKAGE_MANAGER}
{#if isInInstallList && !processMultipleProjects}
<ProjectButtonBase>
<LoadingEllipsis />
diff --git a/sveltejs/src/ProjectBrowser.svelte b/sveltejs/src/ProjectBrowser.svelte
index b525b1079..ab0c555b5 100644
--- a/sveltejs/src/ProjectBrowser.svelte
+++ b/sveltejs/src/ProjectBrowser.svelte
@@ -118,28 +118,6 @@
if (data[source].error && data[source].error.length) {
messenger.add(data[source].error, { type: 'error' });
}
-
- if (
- PACKAGE_MANAGER.available &&
- (PACKAGE_MANAGER.errors.length || PACKAGE_MANAGER.warnings.length)
- ) {
- if (PACKAGE_MANAGER.errors.length) {
- PACKAGE_MANAGER.errors.forEach((e) => {
- messenger.add(`Unable to download modules via the UI: ${e}`, {
- type: 'error',
- });
- });
- }
-
- if (PACKAGE_MANAGER.warnings.length) {
- PACKAGE_MANAGER.warnings.forEach((e) => {
- messenger.add(
- `There may be issues which effect downloading modules: ${e}`,
- { type: 'warning' },
- );
- });
- }
- }
} else {
rows = [];
rowsCount = 0;
diff --git a/sveltejs/src/ProjectGrid.svelte b/sveltejs/src/ProjectGrid.svelte
index 351f46e0c..cd2898b2d 100644
--- a/sveltejs/src/ProjectGrid.svelte
+++ b/sveltejs/src/ProjectGrid.svelte
@@ -72,7 +72,7 @@
>
<slot rows={visibleRows} />
</ul>
- {#if PACKAGE_MANAGER.available && processMultipleProjects}
+ {#if PACKAGE_MANAGER && processMultipleProjects}
<ProcessInstallListButton />
{/if}
{/if}
diff --git a/tests/modules/project_browser_test/src/ProjectBrowserTestServiceProvider.php b/tests/modules/project_browser_test/src/ProjectBrowserTestServiceProvider.php
index d70305429..91b9b3038 100644
--- a/tests/modules/project_browser_test/src/ProjectBrowserTestServiceProvider.php
+++ b/tests/modules/project_browser_test/src/ProjectBrowserTestServiceProvider.php
@@ -6,7 +6,6 @@ namespace Drupal\project_browser_test;
use Drupal\Core\DependencyInjection\ContainerBuilder;
use Drupal\Core\DependencyInjection\ServiceProviderBase;
-use Drupal\project_browser\InstallReadiness;
/**
* Overrides the module installer service.
@@ -17,9 +16,8 @@ class ProjectBrowserTestServiceProvider extends ServiceProviderBase {
* {@inheritdoc}
*/
public function alter(ContainerBuilder $container): void {
- // The InstallReadiness service is defined by ProjectBrowserServiceProvider
- // if Package Manager is installed.
- if ($container->hasDefinition(InstallReadiness::class)) {
+ assert(is_array($container->getParameter('container.modules')));
+ if (array_key_exists('package_manager', $container->getParameter('container.modules'))) {
$container->register(TestInstallReadiness::class, TestInstallReadiness::class)
->setAutowired(TRUE)
->addTag('event_subscriber');
diff --git a/tests/src/FunctionalJavascript/ProjectBrowserInstallerUiTest.php b/tests/src/FunctionalJavascript/ProjectBrowserInstallerUiTest.php
index d7f459c28..b3dadd9ce 100644
--- a/tests/src/FunctionalJavascript/ProjectBrowserInstallerUiTest.php
+++ b/tests/src/FunctionalJavascript/ProjectBrowserInstallerUiTest.php
@@ -40,6 +40,7 @@ class ProjectBrowserInstallerUiTest extends WebDriverTestBase {
'package_manager_test_validation',
'project_browser',
'project_browser_test',
+ 'dblog',
];
/**
@@ -67,6 +68,7 @@ class ProjectBrowserInstallerUiTest extends WebDriverTestBase {
$this->drupalLogin($this->drupalCreateUser([
'administer modules',
'administer site configuration',
+ 'access site reports',
]));
}
@@ -294,14 +296,17 @@ class ProjectBrowserInstallerUiTest extends WebDriverTestBase {
$assert_session = $this->assertSession();
$this->drupalGet('admin/modules/browse/project_browser_test_mock');
- $settings = $this->getDrupalSettings();
- $this->assertTrue($settings['project_browser']['package_manager']['status_checked']);
$this->svelteInitHelper('text', 'Cream cheese on a bagel');
- $assert_session->statusMessageContains("Simulate an error message for the project browser.", 'error');
$cream_cheese_module_selector = '#project-browser .pb-layout__main ul > li:nth-child(1)';
+ $download_button = $assert_session->waitForElementVisible('css', "$cream_cheese_module_selector button.pb__action_button");
+ $this->assertNotEmpty($download_button);
+ $this->assertSame('Install Cream cheese on a bagel', $download_button->getText());
+ $download_button->click();
+ $this->assertSame('Installing', $download_button->getText());
+ $this->assertTrue($assert_session->waitForText('Simulate an error message for the project browser.'));
$download_button_text = $assert_session->waitForElementVisible('css', "$cream_cheese_module_selector button.pb__action_button")
?->getText();
- $this->assertSame('View Commands for Cream cheese on a bagel', $download_button_text);
+ $this->assertSame('Install Cream cheese on a bagel', $download_button_text);
}
/**
@@ -316,7 +321,6 @@ class ProjectBrowserInstallerUiTest extends WebDriverTestBase {
$page = $this->getSession()->getPage();
$this->drupalGet('admin/modules/browse/project_browser_test_mock');
$this->svelteInitHelper('text', 'Cream cheese on a bagel');
- $assert_session->statusMessageContains("Simulate a warning message for the project browser.", 'warning');
$cream_cheese_module_selector = '#project-browser .pb-layout__main ul > li:nth-child(1)';
$download_button = $assert_session->waitForElementVisible('css', "$cream_cheese_module_selector button.pb__action_button");
$this->assertNotEmpty($download_button);
@@ -328,6 +332,8 @@ class ProjectBrowserInstallerUiTest extends WebDriverTestBase {
return $button->getText() === 'Cream cheese on a bagel is Installed';
});
$this->assertTrue($installed_action);
+ $this->drupalGet('admin/reports/dblog');
+ $assert_session->pageTextContains('Simulate a warning message for the project browser.');
}
/**
--
GitLab