🇪🇸Spain @isholgueras

isholgueras → created an issue.

Conflicts with 0.x fixed

Now is ready for a review. I've left some questions.

Tests are failing with the new expression. I'll check

I'll take a look

I'm happy with the code now. Ready to review.

Thanks @catch!

Maybe we're deleting errors that we don't want to remove from the messenger and wont be shown in following visits to the backend.

We also want to suppress all messages to XB interface.

After chatting with @f.mazeikis and @tedbow, @wim proposed to replace the messenger service right before we're starting to do things, and revert right after the rendering, so all the messages in the bag in Drupal are there but the messages produced by XB are automatically removed and do not interfere between both.

I've added a second branch (3520923-json-schema-if-core-issue-landed) to be ready when 🐛 Cannot delete a field which uses JSON type Active lands

Also the current MR in #3487533: Cannot delete a field which uses JSON type is a one-liner - it looks like it either needs test coverage or possibly just a comment, so why go to all the trouble here instead of trying to get that one committed?

I think that would be ideal.

I'll check what it needs to be done or which tests needs to be updated/created in core.

Remove duplicated code and tree in description for clarity because it doesn't exist, only inputs.

isholgueras → made their first commit to this issue’s fork.

Oh, gotcha! Thanks @laurii

Is this still an issue? I can't reproduce it.

What I've found is another issue (recorded in the gif). Here are the steps to reproduce:
1. Create and publish a page with alias /new-page
2. Create a second page, without alias, and publish
3. Now, add the same alias /new-page, expect to have the error message
4. Now try to publish and expect to have the error message in the publish modal.
5. And here the ... issue??... modify the alias to 123 and now you have 2 different errors.

The original issue, allowing to publish different nodes with the same alias, is not reproducible, but here is another (imho minor) issue.

What do you think, @laurii?

I've pushed a proof of concept of removing the hash by the original id.

I've adapted the tests and expected more things to break but they don't.

1. I've created a code component named nachojs and added to components. XB created default/files/astro-island/nachojs.js well.
2. I've modified the nachojs.js file title "nachojs changed title" and it changes well.

I've run all the tests that I've modified and all pass (except for those hash/filename that I expect), but I'll let the CI to run the whole battery.

isholgueras → made their first commit to this issue’s fork.

Yep, I've updated again 0.x, reinstalled, rebuilt ui (I thought I did it because I have it in the same command to reinstall the site) and now looks good. I can add any block component, change any setting and the UI is automatically refreshed.

It can be closed.

After investigating a bit, that's what I've found:

The new code added in ComponentInputsForm.php

    $tree = $request->get('form_xb_tree');
    [$component_id, $version] = \explode('@', \json_decode($tree, TRUE)['type']);
    if (empty($version)) {
      throw new \UnexpectedValueException('No component version specified.');
    }

Extracts the version, and if it's empty throw an exception.

In my fresh installed Drupal with XB and xb_test_block, If I add the who_s_new I get the error.

What is in tree is:

{"slots":[],"nodeType":"component","type":"block.views_block.who_s_new-block_1","uuid":"4adc7962-290e-4bcf-81f7-798f275518ba"}

and there is no such version

Maybe it's because of my installation/branch, but in the branch 📌 [SPIKE] Prove that it's possible to apply block settings update paths to stored XB component trees Active , right after rebasing 0.x, I get the following error every time I place a block:

	UnexpectedValueException: No component version specified. in Drupal\experience_builder\Form\ComponentInputsForm->buildForm() (line 72 of /var/www/html/web/modules/contrib/experience_builder/src/Form/ComponentInputsForm.php).

I'll investigate a bit

This is the error that is thrown:

{
    "message": "Body does not match schema for content-type \u0022application\/json\u0022 for Request [patch \/xb\/api\/v0\/layout\/{entityTypeId}\/{entityId}]. [Keyword validation failed: Data has additional properties (name) which are not allowed in model]",
    "file": "\/var\/www\/html\/vendor\/league\/openapi-psr7-validator\/src\/PSR7\/Exception\/Validation\/AddressValidationFailed.php",
    "line": 31,
    "trace": [
        {
            "file": "\/var\/www\/html\/vendor\/league\/openapi-psr7-validator\/src\/PSR7\/Exception\/Validation\/InvalidBody.php",
            "line": 19,
            "function": "fromAddrAndPrev",
            "class": "League\\OpenAPIValidation\\PSR7\\Exception\\Validation\\AddressValidationFailed",
            "type": "::"
        },
        {
            "file": "\/var\/www\/html\/vendor\/league\/openapi-psr7-validator\/src\/PSR7\/Validators\/BodyValidator\/UnipartValidator.php",
            "line": 60,
            "function": "becauseBodyDoesNotMatchSchema",
            "class": "League\\OpenAPIValidation\\PSR7\\Exception\\Validation\\InvalidBody",
            "type": "::"
        },
        {
            "file": "\/var\/www\/html\/vendor\/league\/openapi-psr7-validator\/src\/PSR7\/Validators\/BodyValidator\/BodyValidator.php",
            "line": 73,
            "function": "validate",
            "class": "League\\OpenAPIValidation\\PSR7\\Validators\\BodyValidator\\UnipartValidator",
            "type": "-\u003E"
        },
        {
            "file": "\/var\/www\/html\/vendor\/league\/openapi-psr7-validator\/src\/PSR7\/Validators\/ValidatorChain.php",
            "line": 25,
            "function": "validate",
            "class": "League\\OpenAPIValidation\\PSR7\\Validators\\BodyValidator\\BodyValidator",
            "type": "-\u003E"
        },
        {
            "file": "\/var\/www\/html\/vendor\/league\/openapi-psr7-validator\/src\/PSR7\/RequestValidator.php",
            "line": 79,
            "function": "validate",
            "class": "League\\OpenAPIValidation\\PSR7\\Validators\\ValidatorChain",
            "type": "-\u003E"
        },
        {
            "file": "\/var\/www\/html\/web\/modules\/contrib\/experience_builder\/src\/EventSubscriber\/ApiRequestValidator.php",
            "line": 44,
            "function": "validate",
            "class": "League\\OpenAPIValidation\\PSR7\\RequestValidator",
            "type": "-\u003E"
        },
        {
            "file": "\/var\/www\/html\/web\/modules\/contrib\/experience_builder\/src\/EventSubscriber\/ApiMessageValidatorBase.php",
            "line": 75,
            "function": "validate",
            "class": "Drupal\\experience_builder\\EventSubscriber\\ApiRequestValidator",
            "type": "-\u003E"
        },
        {
            "file": "\/var\/www\/html\/vendor\/symfony\/event-dispatcher\/EventDispatcher.php",
            "line": 246,
            "function": "onMessage",
            "class": "Drupal\\experience_builder\\EventSubscriber\\ApiMessageValidatorBase",
            "type": "-\u003E"
        },
        {
            "file": "\/var\/www\/html\/vendor\/symfony\/event-dispatcher\/EventDispatcher.php",
            "line": 206,
            "function": "Symfony\\Component\\EventDispatcher\\{closure}",
            "class": "Symfony\\Component\\EventDispatcher\\EventDispatcher",
            "type": "::"
        },
        {
            "file": "\/var\/www\/html\/vendor\/symfony\/event-dispatcher\/EventDispatcher.php",
            "line": 56,
            "function": "callListeners",
            "class": "Symfony\\Component\\EventDispatcher\\EventDispatcher",
            "type": "-\u003E"
        },
        {
            "file": "\/var\/www\/html\/vendor\/symfony\/http-kernel\/HttpKernel.php",
            "line": 159,
            "function": "dispatch",
            "class": "Symfony\\Component\\EventDispatcher\\EventDispatcher",
            "type": "-\u003E"
        },
        {
            "file": "\/var\/www\/html\/vendor\/symfony\/http-kernel\/HttpKernel.php",
            "line": 76,
            "function": "handleRaw",
            "class": "Symfony\\Component\\HttpKernel\\HttpKernel",
            "type": "-\u003E"
        },
        {
            "file": "\/var\/www\/html\/web\/core\/lib\/Drupal\/Core\/StackMiddleware\/Session.php",
            "line": 53,
            "function": "handle",
            "class": "Symfony\\Component\\HttpKernel\\HttpKernel",
            "type": "-\u003E"
        },
        {
            "file": "\/var\/www\/html\/web\/core\/lib\/Drupal\/Core\/StackMiddleware\/KernelPreHandle.php",
            "line": 48,
            "function": "handle",
            "class": "Drupal\\Core\\StackMiddleware\\Session",
            "type": "-\u003E"
        },
        {
            "file": "\/var\/www\/html\/web\/core\/lib\/Drupal\/Core\/StackMiddleware\/ContentLength.php",
            "line": 28,
            "function": "handle",
            "class": "Drupal\\Core\\StackMiddleware\\KernelPreHandle",
            "type": "-\u003E"
        },
        {
            "file": "\/var\/www\/html\/web\/core\/modules\/big_pipe\/src\/StackMiddleware\/ContentLength.php",
            "line": 32,
            "function": "handle",
            "class": "Drupal\\Core\\StackMiddleware\\ContentLength",
            "type": "-\u003E"
        },
        {
            "file": "\/var\/www\/html\/web\/core\/modules\/page_cache\/src\/StackMiddleware\/PageCache.php",
            "line": 116,
            "function": "handle",
            "class": "Drupal\\big_pipe\\StackMiddleware\\ContentLength",
            "type": "-\u003E"
        },
        {
            "file": "\/var\/www\/html\/web\/core\/modules\/page_cache\/src\/StackMiddleware\/PageCache.php",
            "line": 90,
            "function": "pass",
            "class": "Drupal\\page_cache\\StackMiddleware\\PageCache",
            "type": "-\u003E"
        },
        {
            "file": "\/var\/www\/html\/web\/core\/lib\/Drupal\/Core\/StackMiddleware\/ReverseProxyMiddleware.php",
            "line": 48,
            "function": "handle",
            "class": "Drupal\\page_cache\\StackMiddleware\\PageCache",
            "type": "-\u003E"
        },
        {
            "file": "\/var\/www\/html\/web\/core\/lib\/Drupal\/Core\/StackMiddleware\/NegotiationMiddleware.php",
            "line": 51,
            "function": "handle",
            "class": "Drupal\\Core\\StackMiddleware\\ReverseProxyMiddleware",
            "type": "-\u003E"
        },
        {
            "file": "\/var\/www\/html\/web\/core\/lib\/Drupal\/Core\/StackMiddleware\/AjaxPageState.php",
            "line": 36,
            "function": "handle",
            "class": "Drupal\\Core\\StackMiddleware\\NegotiationMiddleware",
            "type": "-\u003E"
        },
        {
            "file": "\/var\/www\/html\/web\/core\/lib\/Drupal\/Core\/StackMiddleware\/StackedHttpKernel.php",
            "line": 51,
            "function": "handle",
            "class": "Drupal\\Core\\StackMiddleware\\AjaxPageState",
            "type": "-\u003E"
        },
        {
            "file": "\/var\/www\/html\/web\/core\/lib\/Drupal\/Core\/DrupalKernel.php",
            "line": 709,
            "function": "handle",
            "class": "Drupal\\Core\\StackMiddleware\\StackedHttpKernel",
            "type": "-\u003E"
        },
        {
            "file": "\/var\/www\/html\/web\/index.php",
            "line": 19,
            "function": "handle",
            "class": "Drupal\\Core\\DrupalKernel",
            "type": "-\u003E"
        }
    ]
}

isholgueras → created an issue.

It needs to fix a complex conflict.

I've found the issue. The problem is that the Renderer in Core can throw an exception and we're not handling it in XB.
It breaks in the ClientSideRepresentation. Here is the diff to explain a bit:

diff --git a/components/my-hero/my-hero.twig b/components/my-hero/my-hero.twig
index 5b39e616..ce9c5c02 100644
--- a/components/my-hero/my-hero.twig
+++ b/components/my-hero/my-hero.twig
@@ -1,4 +1,4 @@
-<div {{ attributes }} class="my-hero__container">
+<div {{ attributes|invalidFilter }} class="my-hero__container">
   <h1 class="my-hero__heading">{{ heading }}</h1>
   <p class="my-hero__subheading">{{ subheading }}</p>
   <div class="my-hero__actions">
diff --git a/src/ClientSideRepresentation.php b/src/ClientSideRepresentation.php
index 4637364f..baa6368f 100644
--- a/src/ClientSideRepresentation.php
+++ b/src/ClientSideRepresentation.php
@@ -11,6 +11,7 @@ use Drupal\Core\Cache\RefinableCacheableDependencyTrait;
 use Drupal\Core\Render\BubbleableMetadata;
 use Drupal\Core\Render\RendererInterface;
 use Drupal\experience_builder\Render\ImportMapResponseAttachmentsProcessor;
+use Twig\Error\SyntaxError;
 
 /**
  * @see \Drupal\jsonapi\Normalizer\Value\CacheableNormalization
@@ -49,7 +50,11 @@ final class ClientSideRepresentation implements RefinableCacheableDependencyInte
     }
 
     $build = $this->preview;
-    $default_markup = $renderer->renderInIsolation($build);
+    try {
+      $default_markup = $renderer->renderInIsolation($build);
+    } catch (SyntaxError $e) {
+            $default_markup = '<div>Render error: ' . htmlspecialchars($e->getMessage()) . '</div>';
+    }
     $assets = AttachedAssets::createFromRenderArray($build);
     $import_map = ImportMapResponseAttachmentsProcessor::buildHtmlTagForAttachedImportMaps(BubbleableMetadata::createFromRenderArray($build)) ?? [];

That's why it's throwing an exception and we're not catching it, and with this, the result is... better.

The problem is that in the Renderer, when it detects the exceptions, it catch it,set it to renderingRoot to false and throw again the exception.

    try {
      return $this->doRender($elements, $is_root_call);
    }
    catch (\Exception $e) {
      // Mark the ::rootRender() call finished due to this exception & re-throw.
      $this->isRenderingRoot = FALSE;
      throw $e;
    }

isholgueras → made their first commit to this issue’s fork.

I wanted to add a new code component with 2 props, one with enums and another one with enums and meta:enums to have a yml file with this as an example.

We had tested it in XbConfigEntityHttpApiTest but I think having a code component in a yml is useful for testing and for new adopters to see how is done.

I've also fixed issues with HEAD.

Steps to test, in Summary, is also updated.

isholgueras → created an issue.

isholgueras → made their first commit to this issue’s fork.

I can't reproduce. It runs well for me in 0.x:

╭─    ~/apps/contrib/drupal11/web/modules/contrib/experience_builder    0.x ·························· ✔  17:23:44  ─╮
╰─ g pull --prune --rebase                                                                                                  ─╯
Already up to date.

╭─    ~/apps/contrib/drupal11/web/modules/contrib/experience_builder    0.x ·························· ✔  17:23:49  ─╮
╰─ ddev phpunit tests/src/Kernel/Config/JavaScriptComponentValidationTest.php                                               ─╯
PHPUnit 10.5.46 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.3.19
Configuration: /var/www/html/web/core/phpunit.xml.dist

................................D.D.....................          56 / 56 (100%)

Time: 01:35.943, Memory: 6.00 MB

2 tests triggered 1 PHP deprecation:

1) /var/www/html/web/core/modules/options/src/Plugin/Field/FieldType/ListItemBase.php:514
Implicit conversion from float 3.14 to int loses precision

Triggered by:

* Drupal\Tests\experience_builder\Kernel\Config\JavaScriptComponentValidationTest::testInvalidEnumsAndExamples#Invalid string
  /var/www/html/web/modules/contrib/experience_builder/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php:145

* Drupal\Tests\experience_builder\Kernel\Config\JavaScriptComponentValidationTest::testValidEnumsAndExamples#3
  /var/www/html/web/modules/contrib/experience_builder/tests/src/Kernel/Config/JavaScriptComponentValidationTest.php:121

OK, but there were issues!
Tests: 56, Assertions: 164, Deprecations: 1.

What I see is that I have this specific issue reproducible in 2 different unrelated builds that are not updated with 0.x at this time:

• https://git.drupalcode.org/project/experience_builder/-/jobs/5229666#L885
• https://git.drupalcode.org/project/experience_builder/-/jobs/5229606#L703

I vote for
- Montaña Franco (mon_franco)
- Will Huggins (zoocha-will)

I've identified difference between both behaviors.
The error is a 500 error because>

    "message": "assert(self::isUrlJsonSchema($this-\u003EjsonSchema))",
    "file": "\/var\/www\/html\/web\/modules\/contrib\/experience_builder\/src\/PropSource\/DefaultRelativeUrlPropSource.php",
    "line": 111,

The problem is because when you add the component and add the image, the "model" values that comes in the request are:

      'model' => [
        'source' => [
          'image' => [
            'expression' => 'ℹ︎entity_reference␟{src↝entity␜␜entity:media:image␝field_media_image␞␟entity␜␜entity:file␝uri␞␟url,alt↝entity␜␜entity:media:image␝field_media_image␞␟alt,width↝entity␜␜entity:media:image␝field_media_image␞␟width,height↝entity␜␜entity:media:image␝field_media_image␞␟height}',
            'sourceType' => 'static:field_item:entity_reference',
            'value' => '1',
...

(There is no jsonSchema but the expression of the media entity_reference)

But if you follow the steps to reproduce, the model values that comes in the request are:

      'model' => [
        'source' => [
          'image' => [
            'sourceType' => 'default-relative-url',
            'jsonSchema' => [
              'type' => 'object',
              'properties' => [
                'src' =>
                  [
                    'type' => 'string',
                    'format' => 'uri-reference',
                    'pattern' => '^(/|https?://)?.*\\.(png|gif|jpg|jpeg|webp)(\\?.*)?(#.*)?$',
                  ],
...

Here, there is a jsonSchema of type default-relative-url, and it fails because the value 1 (the image with media id 1) is not a valid UrlJsonSchema.

I'll keep digging in.

Merge with 0.x is now complete and now the expected #is_preview works well. If I set it as preview, it returns the '#is_preview' => TRUE, otherwise is FALSE.

After rebasing 0.x all tests successfully failed 🤣.

Now it's ready for review.

There is a conflict with 0.x that I need to work on.

I think it's ready for review.

The only difference that I've found in JavaScriptComponents is when is a draft in preview, but I've tested, in a separate tests, the 4 different options, JavascriptComponent regular, in draft, in preview and draft in preview.

I've also adapted the tests for the BlockOverride, that otherwise it fails.

That will cause "draft CSS+JS code component URLs" to appear in the resulting HTML

Perfect, I'll work on that. Thanks!

I've added the isPreview:true option to every single test to validate each.

When previewing, there are none of those wrapping HTML comments. So all the existing test expectations are no longer met.

I couldn't see any output difference (in terms of wrapping html comments). Everything is handled by each renderComponent

• For BlockComponent, is only being used for$build['#xb_preview'] = $isPreview;.
• For SingleDirectoryComponent, $isPreviewis not being used.
• For JsComponent, $isPreview; is being used to generate the component URL, the CssLibrary, scopedDependencies, libraries and for the autosave.

With this in mind, the component types that are being tested in the root are SDC and Block (with the OverrideTest), but no JsComponent. JsComponent are added in the slots.

Should we add test for them too in the uuid-in-root ?

Oh, I see. I've ran only the tests for ComponentTreeHydratedTest locally and it passed, but fails for omponentTreeHydratedWithBlockOverride

If there are no comments, but the ComponentTreeHydratedTest tests pass... maybe there is something wrong here. I'll check.

Ok, I'll add more options to test.

Thanks!

isholgueras → made their first commit to this issue’s fork.

I'm going to start working on this.

wim leers → credited isholgueras → .

I'm Ok with this. Chris has been helping me with the latest releases and he's taking care of the issue queue better than me.

Isn't this a duplicate of 🐛 Disable additionalProperties in slots, props in SDC json schema Active : Disable additionalProperties in slots, props in SDC json schema from core?

I think it is. The validation is done in the ComponentValidator.php in core. Slot names don't seem to be validate.

Confirmed you can create slot with emojis in the name,

and this breaks the site.

Note: breaks the site but you can remove the component and everything works

isholgueras → made their first commit to this issue’s fork.

In the ticket 📌 Provide granular permissions for pages Active there were these 3 permissions (create, edit and delete).

As I didn't have any option for overview or a general "administer", I could only use "edit xb_page" as the permission to use whenever a editor wants to edit it.

Maybe I was mistaken or maybe it's not clear.

I've:

• - Changed all string literal for permissions
• - Replaced them by their own constants
• - Add a reference in the file for a better developer experience
• - Add a new constant for the permission administer components

isholgueras → created an issue.

I've updated the README.md and the CONTRIBUTING.md to match the new requirements (Drupal >= 11.1.2) and to include better information about the ddev-drupal-xb-dev ddev addon.

What I don't know is if we should include sections like:

1. Prerequisites: to include it requires php8.3, Drupal 11.1.2, ...
2. Code structure overview: To explain how's the code structure (tests, tests modules, ui, ...)
3. Contribution workflow: I assume this is implicit in any Drupal contribution, but we can add a specific section about the usage of phpunit, phpstan, ... It's, more or less, added to both sections, but maybe something clearer.
4. Development best practices: Besides of the "use dependency injection,..." type of things, practices or contracts made in this specific project. Maybe like ADRs but not that strict.
5. Links to other projects related with XB?

Any other thing to add things to add?

isholgueras → created an issue.

Can we use 📌 Introduce unit test coverage for both ComponentSource plugins (Block + SDC) Active to use xb_test_block as blocks to update?

Tests refactored to @depends to speed them up and make it quicker.

I've tested it and it works well for me, test included.

The only issue I've found, that I guess only affects my environment and, even though, is not important as it's a test module is that I can't uninstall the module:

╰─ ddev drush pmu xb_test_article_fields                                                                            ─╯

In ExceptionHandler.php line 52:
                                                                                                                      
  SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual tha  
  t corresponds to your MariaDB server version for the right syntax to use near 'LIMIT 1 OFFSET 0' at line 5: SELECT  
   1 AS "expression"                                                                                                  
  FROM                                                                                                                
  "node_revision__field_xbt_path" "t"                                                                                 
  WHERE                                                                                                               
  LIMIT 1 OFFSET 0; Array                                                                                             
  (                                                                                                                   
  )                                                                                                                   
                                                                                                                      

In StatementWrapperIterator.php line 113:
                                                                                                                      
  SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual tha  
  t corresponds to your MariaDB server version for the right syntax to use near 'LIMIT 1 OFFSET 0' at line 5          
                                                                                                                      

I'll let others to test in case they also have the same issue with the uninstall.

Note: I haven't reinstalled the site.

I think I've finished the new permissions, replace administer xb_page by access content , create xb_page , edit xb_page and delete xb_page.

What I've done is adapt the permissions and review the tests that were already done, by maybe is something else needed to do.

Hi @wim-leers,

I've added the first draft of the BlockComponentTest. To do so, I've created a xb_test_block to populate a basic block and I'm testing the plugin id and the cache tags just to see if this is the correct approach for this test. If so, I'll continue adding new tests to it.

Thanks!