From 637c05ebc9c8e2e287b21c97fe6dd680a77f5bfc Mon Sep 17 00:00:00 2001
From: Wim Leers <wim.leers@acquia.com>
Date: Tue, 25 Feb 2025 18:25:33 +0100
Subject: [PATCH 1/5] Add missing test coverage for the `status` syncing that
DOES work.
---
.../Functional/XbConfigEntityHttpApiTest.php | 61 +++++++++++++++++++
1 file changed, 61 insertions(+)
diff --git a/tests/src/Functional/XbConfigEntityHttpApiTest.php b/tests/src/Functional/XbConfigEntityHttpApiTest.php
index 003613ecb3..91160b35fb 100644
--- a/tests/src/Functional/XbConfigEntityHttpApiTest.php
+++ b/tests/src/Functional/XbConfigEntityHttpApiTest.php
@@ -31,6 +31,7 @@ class XbConfigEntityHttpApiTest extends HttpApiTestBase {
'block',
'experience_builder',
'xb_test_sdc',
+ 'node',
];
/**
@@ -602,6 +603,10 @@ class XbConfigEntityHttpApiTest extends HttpApiTestBase {
'js_footer' => '',
];
$this->assertSame($expected_component, $body);
+ // Confirm that the code component IS NOT exposed.
+ // @see docs/config-management.md#3.2.1
+ $this->assertExposedCodeComponents([], 'MISS', $request_options);
+ $this->assertExposedCodeComponents([], 'HIT', $request_options);
// Confirm no auto-save entity has been created.
$this->assertExpectedResponse('GET', $auto_save_url, $request_options, 204, ['user.permissions'], ['experience_builder__autosave', 'http_response'], 'UNCACHEABLE (request policy)', 'MISS');
$this->assertExpectedResponse('GET', $auto_save_url, $request_options, 204, ['user.permissions'], ['experience_builder__autosave', 'http_response'], 'UNCACHEABLE (request policy)', 'HIT');
@@ -678,6 +683,10 @@ class XbConfigEntityHttpApiTest extends HttpApiTestBase {
'http_response',
], 'UNCACHEABLE (request policy)', 'MISS');
$this->assertSame(['test' => $expected_component], $body);
+ // Confirm that the code component IS STILL NOT exposed, because `status` is
+ // still `FALSE`.
+ // @see docs/config-management.md#3.2.1
+ $this->assertExposedCodeComponents([], 'HIT', $request_options);
// Modify a Code Component incorrectly (consistency-wise): 422.
// @todo This currently returns a 200 response! https://www.drupal.org/i/3500043 will disallow PATCHing this if > 0 uses of this component exist.
@@ -686,6 +695,11 @@ class XbConfigEntityHttpApiTest extends HttpApiTestBase {
$request_options[RequestOptions::BODY] = self::encodeXBData($code_component_to_send);
$body = $this->assertExpectedResponse('PATCH', Url::fromUri('base:/xb/api/config/js_component/test'), $request_options, 200, NULL, NULL, NULL, NULL);
$this->assertSame($expected_component, $body);
+ // Confirm that the code component IS exposed, because `status` was just
+ // changed to `TRUE`.
+ // @see docs/config-management.md#3.2.1
+ $this->assertExposedCodeComponents(['js.test'], 'MISS', $request_options);
+ $this->assertExposedCodeComponents(['js.test'], 'HIT', $request_options);
// Create an auto-save entry for this config entity, to verify that neither
// the "list" nor the "individual" API responses tested here are affected by
@@ -709,6 +723,11 @@ class XbConfigEntityHttpApiTest extends HttpApiTestBase {
// Delete the 'test' Code Component via the XB HTTP API: 204.
$body = $this->assertExpectedResponse('DELETE', Url::fromUri('base:/xb/api/config/js_component/test'), [], 204, NULL, NULL, NULL, NULL);
$this->assertNull($body);
+ // Confirm that the code component IS NOT exposed, because it no longer
+ // exists.
+ // @see docs/config-management.md#3.2.1
+ $this->assertExposedCodeComponents([], 'MISS', $request_options);
+ $this->assertExposedCodeComponents([], 'HIT', $request_options);
// Re-retrieve list: 200, empty list. Dynamic Page Cache miss.
$body = $this->assertExpectedResponse('GET', $list_url, [], 200, ['user.permissions'], ['config:js_component_list', 'http_response'], 'UNCACHEABLE (request policy)', 'MISS');
@@ -839,4 +858,46 @@ class XbConfigEntityHttpApiTest extends HttpApiTestBase {
], json_decode((string) $response->getBody(), TRUE));
}
+ private function assertExposedCodeComponents(array $expected, string $expected_dynamic_page_cache, array $request_options): void {
+ assert(in_array($expected_dynamic_page_cache, ['HIT', 'MISS'], TRUE));
+ $expected_contexts = [
+ 'languages:language_content',
+ 'languages:language_interface',
+ 'route',
+ 'theme',
+ 'url.path',
+ 'url.query_args',
+ 'user.node_grants:view',
+ 'user.permissions',
+ 'user.roles:authenticated',
+ // The user_login_block is rendered as the anonymous user because for the
+ // authenticated user it is empty.
+ // @see \Drupal\experience_builder\Controller\ApiComponentsController::getCacheableClientSideInfo()
+ 'user.roles:anonymous',
+ ];
+ $body = $this->assertExpectedResponse('GET', Url::fromUri('base:/xb/api/config/component'), $request_options, 200, $expected_contexts, [
+ 'CACHE_MISS_IF_UNCACHEABLE_HTTP_METHOD:form',
+ 'config:component_list',
+ 'config:core.extension',
+ 'config:node_type_list',
+ 'config:system.menu.account',
+ 'config:system.site',
+ 'config:system.theme',
+ 'config:views.view.content_recent',
+ 'config:views.view.who_s_new',
+ 'http_response',
+ 'local_task',
+ 'node_list',
+ 'user:1',
+ 'user:2',
+ 'user_list',
+ ], 'UNCACHEABLE (request policy)', $expected_dynamic_page_cache);
+ self:self::assertNotNull($body);
+ $component_config_entity_ids = array_keys($body);
+ self::assertSame(
+ $expected,
+ array_values(array_filter($component_config_entity_ids, fn (string $id) => str_starts_with($id, 'js.'))),
+ );
+ }
+
}
--
GitLab
From 04bcd51cf284434cd38afa254d7efc8c9afa6976 Mon Sep 17 00:00:00 2001
From: Wim Leers <wim.leers@acquia.com>
Date: Tue, 25 Feb 2025 18:45:15 +0100
Subject: [PATCH 2/5] Both kernel and integration test coverage for how
`status` changes SHOUDL work.
---
.../Functional/XbConfigEntityHttpApiTest.php | 28 ++++++++++++-
.../Config/JavascriptComponentStorageTest.php | 39 ++++++++++++++++---
2 files changed, 60 insertions(+), 7 deletions(-)
diff --git a/tests/src/Functional/XbConfigEntityHttpApiTest.php b/tests/src/Functional/XbConfigEntityHttpApiTest.php
index 91160b35fb..ec725f017b 100644
--- a/tests/src/Functional/XbConfigEntityHttpApiTest.php
+++ b/tests/src/Functional/XbConfigEntityHttpApiTest.php
@@ -6,6 +6,7 @@ namespace Drupal\Tests\experience_builder\Functional;
use Drupal\Core\Url;
use Drupal\experience_builder\Entity\AssetLibrary;
+use Drupal\experience_builder\Entity\Component;
use Drupal\experience_builder\Entity\Pattern;
use Drupal\system\Entity\Menu;
use Drupal\Tests\experience_builder\Traits\ContribStrictConfigSchemaTestTrait;
@@ -688,8 +689,11 @@ class XbConfigEntityHttpApiTest extends HttpApiTestBase {
// @see docs/config-management.md#3.2.1
$this->assertExposedCodeComponents([], 'HIT', $request_options);
- // Modify a Code Component incorrectly (consistency-wise): 422.
- // @todo This currently returns a 200 response! https://www.drupal.org/i/3500043 will disallow PATCHing this if > 0 uses of this component exist.
+ // Modify a Code Component correctly: 200.
+ // ⚠️This is changing it from `internal` → `exposed`, for the first time,
+ // this must trigger the creation a corresponding `Component` config entity.
+ $this->assertNull(Component::load('js.test'));
+ // @todo https://www.drupal.org/i/3500043 will disallow PATCHing this if > 0 uses of this component exist.
$code_component_to_send['status'] = TRUE;
$expected_component['status'] = TRUE;
$request_options[RequestOptions::BODY] = self::encodeXBData($code_component_to_send);
@@ -698,9 +702,29 @@ class XbConfigEntityHttpApiTest extends HttpApiTestBase {
// Confirm that the code component IS exposed, because `status` was just
// changed to `TRUE`.
// @see docs/config-management.md#3.2.1
+ $this->assertNotNull(Component::load('js.test'));
$this->assertExposedCodeComponents(['js.test'], 'MISS', $request_options);
$this->assertExposedCodeComponents(['js.test'], 'HIT', $request_options);
+ // Modify a Code Component correctly: 200.
+ // ⚠️This is changing it from `exposed` → `internal`. This must cause the
+ // `Component` config entity to continue to exist, but get its `status` to
+ // change to `FALSE`, and cause it to be omitted from the list of available
+ // components for the Content Creator.
+ // @todo https://www.drupal.org/i/3500043 will disallow PATCHing this if > 0 uses of this component exist.
+ $code_component_to_send['status'] = FALSE;
+ $expected_component['status'] = FALSE;
+ $request_options[RequestOptions::BODY] = self::encodeXBData($code_component_to_send);
+ $body = $this->assertExpectedResponse('PATCH', Url::fromUri('base:/xb/api/config/js_component/test'), $request_options, 200, NULL, NULL, NULL, NULL);
+ $this->assertSame($expected_component, $body);
+ // Confirm that the code component IS exposed, because `status` was just
+ // changed to `TRUE`.
+ // @see docs/config-management.md#3.2.1
+ $this->assertNotNull(Component::load('js.test'));
+ $this->assertFalse(Component::load('js.test')->status());
+ $this->assertExposedCodeComponents([], 'MISS', $request_options);
+ $this->assertExposedCodeComponents([], 'HIT', $request_options);
+
// Create an auto-save entry for this config entity, to verify that neither
// the "list" nor the "individual" API responses tested here are affected by
// it.
diff --git a/tests/src/Kernel/Config/JavascriptComponentStorageTest.php b/tests/src/Kernel/Config/JavascriptComponentStorageTest.php
index f6e5714860..911cc4462d 100644
--- a/tests/src/Kernel/Config/JavascriptComponentStorageTest.php
+++ b/tests/src/Kernel/Config/JavascriptComponentStorageTest.php
@@ -16,7 +16,8 @@ use Drupal\Tests\user\Traits\UserCreationTrait;
* Tests JavascriptComponentStorage.
*
* @covers \Drupal\experience_builder\EntityHandlers\JavascriptComponentStorage
- * @covers \Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\JsComponent
+ * @covers \Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\JsComponent::createConfigEntity
+ * @covers \Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\JsComponent::updateConfigEntity
* @group JavaScriptComponents
* @group experience_builder
*/
@@ -74,9 +75,9 @@ final class JavascriptComponentStorageTest extends AssetLibraryStorageTest {
}
/**
- * Covers component creation.
+ * @covers \Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\JsComponent::createConfigEntity()
*/
- public function testComponentEntityCreation(): void {
+ public function testComponentEntityCreation(): JavaScriptComponent {
$js_component_id = $this->randomMachineName();
$component_id = JsComponent::componentIdFromJavascriptComponentId($js_component_id);
$reason_repository = $this->container->get(ComponentIncompatibilityReasonRepository::class);
@@ -174,10 +175,38 @@ final class JavascriptComponentStorageTest extends AssetLibraryStorageTest {
$js_component->set('name', $new_name);
$js_component->setProps($props)->save();
- $component = \Drupal::entityTypeManager()->getStorage(Component::ENTITY_TYPE_ID)->loadUnchanged($component_id);
- \assert($component instanceof ComponentInterface);
+ $component = $this->loadComponent($component_id);
self::assertEquals($new_name, $component->label());
self::assertEquals(['noodles', 'title'], \array_keys($component->getSettings()['prop_field_definitions']));
+
+ return $js_component;
+ }
+
+ /**
+ * @covers \Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\JsComponent::updateConfigEntity()
+ * @depends testComponentEntityCreation
+ */
+ public function testComponentEntityUpdate(JavaScriptComponent $js_component): void {
+ $component_id = JsComponent::componentIdFromJavascriptComponentId($js_component->id());
+
+ // Name should carry over.
+ $new_name = $js_component->label() . ' — updated';
+ $js_component->set('name', $new_name)->save();
+ $this->assertSame($new_name, $this->loadComponent($component_id)->label());
+
+ // Status should carry over.
+ $this->assertTrue($js_component->status());
+ $this->assertTrue($this->loadComponent($component_id)->status());
+ $js_component->disable()->save();
+ $this->assertFalse($js_component->status());
+ $this->assertFalse($this->loadComponent($component_id)->status());
+ $js_component->enable()->save();
+ $this->assertTrue($js_component->status());
+ $this->assertTrue($this->loadComponent($component_id)->status());
+ }
+
+ private function loadComponent(string $id): Component {
+ return \Drupal::entityTypeManager()->getStorage(Component::ENTITY_TYPE_ID)->loadUnchanged($id);
}
}
--
GitLab
From c1d2b19198bdacaf64a179123cf4f20afb974c76 Mon Sep 17 00:00:00 2001
From: Wim Leers <wim.leers@acquia.com>
Date: Tue, 25 Feb 2025 18:53:19 +0100
Subject: [PATCH 3/5] `phpstan`
---
tests/src/Kernel/Config/JavascriptComponentStorageTest.php | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tests/src/Kernel/Config/JavascriptComponentStorageTest.php b/tests/src/Kernel/Config/JavascriptComponentStorageTest.php
index 911cc4462d..856507576c 100644
--- a/tests/src/Kernel/Config/JavascriptComponentStorageTest.php
+++ b/tests/src/Kernel/Config/JavascriptComponentStorageTest.php
@@ -4,6 +4,7 @@ declare(strict_types=1);
namespace Drupal\Tests\experience_builder\Kernel\Config;
+use Drupal\Core\Entity\EntityTypeManagerInterface;
use Drupal\experience_builder\ComponentIncompatibilityReasonRepository;
use Drupal\experience_builder\Entity\Component;
use Drupal\experience_builder\Entity\ComponentInterface;
@@ -187,6 +188,7 @@ final class JavascriptComponentStorageTest extends AssetLibraryStorageTest {
* @depends testComponentEntityCreation
*/
public function testComponentEntityUpdate(JavaScriptComponent $js_component): void {
+ assert(is_string($js_component->id()));
$component_id = JsComponent::componentIdFromJavascriptComponentId($js_component->id());
// Name should carry over.
@@ -206,7 +208,10 @@ final class JavascriptComponentStorageTest extends AssetLibraryStorageTest {
}
private function loadComponent(string $id): Component {
- return \Drupal::entityTypeManager()->getStorage(Component::ENTITY_TYPE_ID)->loadUnchanged($id);
+ // @phpstan-ignore-next-line
+ return $this->container->get(EntityTypeManagerInterface::class)
+ ->getStorage(Component::ENTITY_TYPE_ID)
+ ->loadUnchanged($id);
}
}
--
GitLab
From 408738c93bd28d19c378b7a4d626b87432c464ef Mon Sep 17 00:00:00 2001
From: Wim Leers <wim.leers@acquia.com>
Date: Tue, 25 Feb 2025 19:31:45 +0100
Subject: [PATCH 4/5] =?UTF-8?q?Apparently=20PHPUnit=20on=20my=20local=20ma?=
=?UTF-8?q?chine=20can=20do=20more=20than=20on=20GitLab=20CI=20?=
=?UTF-8?q?=F0=9F=A4=B7=E2=80=8D=E2=99=82=EF=B8=8F?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
---
.../src/Kernel/Config/JavascriptComponentStorageTest.php | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/tests/src/Kernel/Config/JavascriptComponentStorageTest.php b/tests/src/Kernel/Config/JavascriptComponentStorageTest.php
index 856507576c..da04409b6f 100644
--- a/tests/src/Kernel/Config/JavascriptComponentStorageTest.php
+++ b/tests/src/Kernel/Config/JavascriptComponentStorageTest.php
@@ -78,7 +78,7 @@ final class JavascriptComponentStorageTest extends AssetLibraryStorageTest {
/**
* @covers \Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\JsComponent::createConfigEntity()
*/
- public function testComponentEntityCreation(): JavaScriptComponent {
+ public function testComponentEntityCreation(): array {
$js_component_id = $this->randomMachineName();
$component_id = JsComponent::componentIdFromJavascriptComponentId($js_component_id);
$reason_repository = $this->container->get(ComponentIncompatibilityReasonRepository::class);
@@ -180,14 +180,16 @@ final class JavascriptComponentStorageTest extends AssetLibraryStorageTest {
self::assertEquals($new_name, $component->label());
self::assertEquals(['noodles', 'title'], \array_keys($component->getSettings()['prop_field_definitions']));
- return $js_component;
+ return $js_component->toArray();
}
/**
* @covers \Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\JsComponent::updateConfigEntity()
* @depends testComponentEntityCreation
*/
- public function testComponentEntityUpdate(JavaScriptComponent $js_component): void {
+ public function testComponentEntityUpdate(array $js_component_values): void {
+ $js_component = JavaScriptComponent::create($js_component_values);
+ $js_component->save();
assert(is_string($js_component->id()));
$component_id = JsComponent::componentIdFromJavascriptComponentId($js_component->id());
--
GitLab
From 6a06dfcbdfb318c1f62e199322c85f7a444017fc Mon Sep 17 00:00:00 2001
From: Wim Leers <wim.leers@acquia.com>
Date: Tue, 25 Feb 2025 18:46:33 +0100
Subject: [PATCH 5/5] Fix.
---
src/Plugin/ExperienceBuilder/ComponentSource/JsComponent.php | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/Plugin/ExperienceBuilder/ComponentSource/JsComponent.php b/src/Plugin/ExperienceBuilder/ComponentSource/JsComponent.php
index 0f4c596109..281265d44e 100644
--- a/src/Plugin/ExperienceBuilder/ComponentSource/JsComponent.php
+++ b/src/Plugin/ExperienceBuilder/ComponentSource/JsComponent.php
@@ -254,6 +254,7 @@ final class JsComponent extends GeneratedFieldExplicitInputUxComponentSourceBase
$label_key = $component->getEntityType()->getKey('label');
assert(is_string($label_key));
$component->set($label_key, $js_component->label());
+ $component->setStatus($js_component->status());
try {
$ephemeral_sdc_component = self::buildEphemeralSdcPluginInstance($js_component);
}
--
GitLab