ComponentSourceInterface::submitConfigurationForm and validateConfigurationForm are never called

Created on 9 May 2025, 4 months ago

Overview

E.g. in the case of the block plugin, there might be code in the respective block plugins that need to be called to massage the submitted form values.

Proposed resolution

Call both these methods and have them do something

User interface changes

📌 Task
Status

Active

Version

0.0

Component

Component sources

Created by

🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @larowlan
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    This AFAIK ties into follow-ups opened for 📌 Add support for Blocks as Components Active that were never followed up on. A quick scan suggests:

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    This should be fixed before 1.0, but not necessarily before 🌱 Milestone 1.0.0-beta1: Start creating non-throwaway sites Active  — because A) ComponentSource plugins won't be a stable API yet, B) I don't see how this could impact the data storage.

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    If we're lucky, it's this simple:

    diff --git a/src/Plugin/ExperienceBuilder/ComponentSource/BlockComponent.php b/src/Plugin/ExperienceBuilder/ComponentSource/BlockComponent.php
    index f0a7b094c..8e7fa4ba4 100644
    --- a/src/Plugin/ExperienceBuilder/ComponentSource/BlockComponent.php
    +++ b/src/Plugin/ExperienceBuilder/ComponentSource/BlockComponent.php
    @@ -325,14 +325,14 @@ final class BlockComponent extends ComponentSourceBase implements ContainerFacto
        * {@inheritdoc}
        */
       public function validateConfigurationForm(array &$form, FormStateInterface $form_state): void {
    -    // @todo Implementation.
    +    $this->getBlockPlugin()->validateConfigurationForm($form, $form_state);
       }
     
       /**
        * {@inheritdoc}
        */
       public function submitConfigurationForm(array &$form, FormStateInterface $form_state): void {
    -    // @todo Implementation.
    +    $this->getBlockPlugin()->submitConfigurationForm($form, $form_state);
       }
     
       /**
    
  • First commit to issue fork.
  • 🇭🇺Hungary Gábor Hojtsy Hungary

    I'm running into this in 🐛 Autocomplete machine name handling does not match to machine name Active with the webform block.

    Webform uses Drupal\Core\Entity\Element\EntityAutocomplete which has a validateEntityAutocomplete() which is where the mapping happens. That is defined as an #element_validate on the element. I tried the proposed fix in #4, that did not seem to help (change anything at all with what it did).

  • 🇺🇸United States bnjmnm Ann Arbor, MI

    The validateConfigurationForm / submitConfigurationForm methods in ExperienceBuilder/ComponentSource/BlockComponent aren't called at all, so the additions in #4 unfortunately won't work. This isn't a huge shock as (like the Page Data form) there isn't a traditional submit that would invoke the validate/submit methods in the way we're accustomed to.

    I attempted to submit the block form similar to how the entity form is programmatically validated/submitted in \Drupal\experience_builder\ClientDataToEntityConverter::setEntityFields and didn't have any luck - if elements in the form had #element_validate callbacks they weren't called. I suspect this is because of how complex it is to do this (a delicate balance of form state and form and input values) and not because it's an unworkable approach.

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    @bnjmnm Ah, right, this also needs ComponentInputsForm::validateForm() and ComponentInputsForm::submitForm() to call ComponentSourceInterface::(validate|submit)Form 😅

  • 🇬🇧United Kingdom f.mazeikis Brighton

    Currently, as per @bnjmnm comment #7, we do not call methods proposed by @wim-leers in #4. I've tried adding suggested code as per #7, but that isn't called either.
    In fact, ComponentInputsForm::buildForm() has the following comments on L102-L106:

    // Prevent form submission while specifying values for component inputs,
    // because changes are saved via Redux instead of a traditional submit.
    // @see ui/src/components/form/inputBehaviors.tsx

    I do not think we should classify this as a bug, as we've never built form submission/validation handling mechanism. We build form to allow user to input prop values, but the only validation performed is on FE side. At least until an attempt to publish/save is made, at which point some form inputs turn out to be invalid, as they were never massaged by "submission" mechanisms.
    I suspect that this will be much more involving than just adding code proposed in #4 and #7. Specifically, adapting FE application to do submissions and validation via Drupal methods.
    Considering that current UI allows user to enter some text into autocomplete, select a value and click away - there might be some interesting challenges to figure out what constitutes "form submission" or "form requiring validation" if there are no additional UI/UX changes introduced along with this.

    And even when such changes are in, I suspect we will need thorough manual and automated testing to find potential edge cases.

  • 🇺🇸United States bnjmnm Ann Arbor, MI
  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    I have some thoughts, I'll push this along a bit

  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10
  • Merge request !1386Issue #3523496: Block form submit/validate → (Merged) created by larowlan
  • Pipeline finished with Failed
    about 1 month ago
    #560204
  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    Pushed up a POC but we've got a couple of open threads/questions

    * In \Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\BlockComponent::buildConfigurationForm we're only calling ::blockForm and we probably should be calling ::buildConfigurationForm - current code is not calling the code for blocks that override that method - e.g. \Drupal\layout_builder\Plugin\Block\InlineBlock::buildConfigurationForm
    * We have issues with #tree - BlockForm in core uses sub-form state and sets #tree => TRUE on the form. If we do don't do that we end up with a mismatch in how the form works between here and normal block forms. But if we do that, we end up with extra values in the input names - e.g. see \Drupal\system\Plugin\Block\SystemBrandingBlock::blockSubmit which expects to see block_branding in the values. We don't currently have that because we're working with the schema for the config (as seen in \Drupal\system\Plugin\Block\SystemBrandingBlock::defaultConfiguration where there is no block_branding. We also have \Drupal\experience_builder\Form\ComponentInputsForm::applyElementParents to mess with #parents.

    I suspect the way forward will be to get \Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\BlockComponent::buildConfigurationForm into alignment with BlockForm and use sub-form state and properly pass along settings and #tree => TRUE to the FE, then in ::clientModelToInput we can undo that in a repeatable fashion, so that we can call ::blockSubmit for blocks like SystemBrandingBlock

    The failing e2e catches these issues 🙌 so that block was a good choice for test cases!

  • Pipeline finished with Failed
    about 1 month ago
    #560254
  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    larowlan changed the visibility of the branch 3523496-componentsourceinterfacesubmitconfigurationform-and-validateconfigurationform to hidden.

  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    larowlan changed the visibility of the branch 1.x to hidden.

  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    @gábor hojtsy any chance you could test this branch with webform to confirm it resolves the issue?
    The test module I've added has a block form with an entity autocomplete widget so 🤞 it does.

    If we're lucky, it's this simple:

    Famous last words 😂, this one turned out to be quite the battle

  • 🇭🇺Hungary Gábor Hojtsy Hungary

    Tried the latest MR in xb-demo. I unapplied the change to the block to make it a select box, so it is back to needing this fix, then applied this fix. Unfortunately when I try to enter something in the webform autocomplete that is not quite correct (eg. I start typing to wait for autocomplete to return), I get a red error in place of the form saying it did not render. I tried looking in the console logs but no useful data there. I tried reloading but it got into an "undo last action" loop.

    The logs have these:

    Referrer https://xb-demo.ddev.site/xb/xb_page/1/editor/component/ded7bed0-4370-41...
    Message AssertionError occurred during rendering of component ded7bed0-4370-4186-bb97-2ec6490cf548 in Page Awesome Demo Experience Builder (1), field components: Cannot load the "webform" entity with NULL ID.

    followed by

    AssertionError: Cannot load the "webform" entity with NULL ID. in assert() (line 262 of /var/www/html/web/core/lib/Drupal/Core/Entity/EntityStorageBase.php).

    #0 /var/www/html/web/core/lib/Drupal/Core/Entity/EntityStorageBase.php(262): assert()
    #1 /var/www/html/web/modules/contrib/webform/src/Plugin/Block/WebformBlock.php(259): Drupal\Core\Entity\EntityStorageBase->load()
    #2 /var/www/html/web/modules/contrib/webform/src/Plugin/Block/WebformBlock.php(112): Drupal\webform\Plugin\Block\WebformBlock->getWebform()
    #3 /var/www/html/web/core/lib/Drupal/Core/Block/BlockPluginTrait.php(188): Drupal\webform\Plugin\Block\WebformBlock->blockForm()
    #4 /var/www/html/web/core/lib/Drupal/Core/Block/BlockBase.php(36): Drupal\Core\Block\BlockBase->traitBuildConfigurationForm()
    #5 /var/www/html/web/modules/contrib/experience_builder/src/Plugin/ExperienceBuilder/ComponentSource/BlockComponent.php(377): Drupal\Core\Block\BlockBase->buildConfigurationForm()
    #6 /var/www/html/web/modules/contrib/experience_builder/src/Form/ComponentInputsForm.php(114): Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\BlockComponent->buildConfigurationForm()
    #7 [internal function]: Drupal\experience_builder\Form\ComponentInputsForm->buildForm()

    Line 259 in WebformBlock is the middle of this:

      protected function getWebform() {
        return $this->entityTypeManager->getStorage('webform')->load($this->configuration['webform_id']);
      }
    

    so if this gets NULL, that does not fulfill what the code expects assuming the default config for the block which is:

      public function defaultConfiguration() {
        return [
          'webform_id' => '',
          'default_data' => '',
          'redirect' => FALSE,
          'lazy' => FALSE,
        ];
      }
    

    Is this considered in the code/XB?

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Is this considered in the code/XB?

    Yes, it is:

          $settings = [
            // We are using strict config schema validation, so we need to provide
            // valid default settings for each block.
            'default_settings' => [
                // The generic block plugin settings: all block plugins have at least
                // this.
                // @see `type: block_settings`
                // @see `type: block.settings.*`
                // @todo Simplify when core simplifies `type: block_settings` in
                //   https://www.drupal.org/i/3426278
              'id' => $id,
              'label' => (string) $definition['admin_label'],
              // @todo Change this to FALSE once https://drupal.org/i/2544708 is
              //   fixed.
              'label_display' => '0',
              'provider' => $definition['provider'],
            ] + $block->defaultConfiguration(),
          ];
    

    \Drupal\experience_builder\Plugin\BlockManager::setCachedDefinitions()

    Will review & investigate #19.

  • 🇫🇮Finland lauriii Finland

    We need to test 📌 Machine name JS does not work in Page Data/Component Instance form Postponed after this because per @bnjmnm, this issue should address that too 👏

  • 🇺🇸United States bnjmnm Ann Arbor, MI

    We need to test #3513742: Machine name JS does not work in Page Data/Component Instance form after this because per @bnjmnm, this issue should address that too

    To clarify: the issue as reported (which has to do with the JS logic in machine-name.js) is not something that would be fixed here, but it likely already works due to other recent changes.

    There was an unrelated bug mentioned in #3513742 that is what #21 is referencing. That bug is related to problems with webform block autocomplete values that expect a machine name. It's also is what @gábor hojtsy tested in #19 - so it's already part of this evaluation - not something additional.

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    @bnjmnm's issue summary update in #11 is superb — thanks, Ben! 🤩

    📌 Redux-aware form input components should be aware of direct element manipulation Active landed yesterday. @Gábor Hojtsy, could you please re-test #19 — I've merged upstream changes into this MR 👍

    @larowlan: AFAICT this would make 🐛 Auto-saving of blocks needs to handle string-to-bool fixing Active obsolete — do you concur?

    #15:

    • In \Drupal\experience_builder\Plugin\ExperienceBuilder\ComponentSource\BlockComponent::buildConfigurationForm we're only calling ::blockForm and we probably should be calling ::buildConfigurationForm - current code is not calling the code for blocks that override that method - e.g. \Drupal\layout_builder\Plugin\Block\InlineBlock::buildConfigurationForm

    I disagree.

    📌 Define `props` in the context of Block components Active added the call to ::blockForm(). It did that because it method actually specified on the block plugin: \Drupal\Core\Block\BlockPluginInterface::blockForm().

    ::buildConfigurationForm() is not guaranteed to be implemented, and is specifically tailored towards Block config entities, because:

    1. \Drupal\Core\Block\BlockPluginTrait::buildConfigurationForm() is what implements it — an optional (but encouraged) trait, also used by \Drupal\Core\Block\BlockBase
    2. it adds a whole lot of noise to the component instance form for block-sourced components, 100% of which is irrelevant when used in XB; it's only relevant for Block config entities:
        public function buildConfigurationForm(array $form, FormStateInterface $form_state) {
          $definition = $this->getPluginDefinition();
          $form['provider'] = [
            '#type' => 'value',
            '#value' => $definition['provider'],
          ];
      
          $form['admin_label'] = [
            '#type' => 'item',
            '#title' => $this->t('Block description'),
            '#plain_text' => $definition['admin_label'],
          ];
          $form['label'] = [
            '#type' => 'textfield',
            '#title' => $this->t('Title'),
            '#maxlength' => 255,
            '#default_value' => $this->label(),
            '#required' => TRUE,
          ];
          $form['label_display'] = [
            '#type' => 'checkbox',
            '#title' => $this->t('Display title'),
            '#default_value' => ($this->configuration['label_display'] === BlockPluginInterface::BLOCK_LABEL_VISIBLE),
            '#return_value' => BlockPluginInterface::BLOCK_LABEL_VISIBLE,
          ];
      
          // Add plugin-specific settings for this block type.
          $form += $this->blockForm($form, $form_state);
          return $form;
        }
      

      👆 Of those:

      1. provider is pure noise — nope I mixed that up with 📌 [PP-1] Deprecate unused `provider` exported property from Block config entities Postponed
      2. admin_label is used only at /admin/structure/block, to allow site builders to relabel placed block [config entities] to make sense to them, in that particular UI
      3. label (and its sibling label_display) is used by block.html.twig, which XB doesn't use, because that's associated with Block config entities

      😬🫠 … except that apparently type: block_settings dictates that label, label_display etc. must all be present always 🫠

      … which means this is the right thing to do, and this should pave the path for fixing 🐛 block__content div is omitted from preview when "Use Experience Builder for page templates in this theme." is enabled Active , too.

      Plus, you've sidestepped the UX nightmare I identified by working your magic in BlockComponent::buildAnonymousFormForBlockPlugin() to remove those 4 form fields 🧙🏽‍♂️

  • 🇺🇸United States bnjmnm Ann Arbor, MI

    Might sound like splitting hairs, but IMO an important distinction to figuring this out:

    What @gábor hojtsy is encountering in #19 is something introduced by the MR. In other words it's not part of the underlying problem that this MR failed to account for.

    We're now invoking the submit/validate callbacks on the block form. That's overall a good thing, it's needed to massage values to the expected end result.

    BUT... The problem is due to logic in \Drupal\webform\Plugin\Block\WebformBlock::blockSubmit, which is now invoked as early as adding the Webform Block to the layout.

    public function blockSubmit($form, FormStateInterface $form_state) {
        $values = $form_state->getValues();
        $this->configuration['webform_id'] = $values['webform_id']; // 👈HERE IS THE PROBLEM!
        $this->configuration['default_data'] = $values['settings']['default_data'];
        $this->configuration['redirect'] = $values['settings']['redirect'];
        $this->configuration['lazy'] = $values['settings']['lazy'];
      }
    

    It's looking for $values['webform_id'], which is NULL, instead of the default value which is an empty string.

    NULL will fail the !is_null() assertion in \Drupal\Core\Entity\EntityStorageBase::load, but an empty string is handled fine.

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Whoops, I read over this bit in @Gábor Hojtsy's comment:

    Unfortunately when I try to enter something in the webform autocomplete that is not quite correct (eg. I start typing to wait for autocomplete to return), I get a red error in place of the form saying it did not render. I tried looking in the console logs but no useful data there. I tried reloading but it got into an "undo last action" loop.

    @bnjmnm is right in #24: that is expected: invalid component inputs SHOULD result in a failure to render, but not in a way that it's a hard crash — see the screenshots in 📌 Improve server-side error handling Active . That must be what Gábor is seeing.

    @Gábor: why do you expect that an invalid/nonsensical value would result in automatically all user input getting ignored and instead the block falling back to the default?

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    This MR looks equals parts impressive and scary — in #23 I articulated my main concern, but then was forced to withdraw it :D (aka concluding I was wrong!)

    I did spot a number of other concerning things though — concerning because I'm struggling to grok them 😅

    The most important ones:

    1. the use of resolved but that then not matching the exact inputs expected by block-sourced components: https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...
    2. +1 for your suggestion to lift some things into a new service, which then both BlockComponent and ClientDataToEntityConverter can use — suggestion at https://git.drupalcode.org/project/experience_builder/-/merge_requests/1... — without this, I worry about maintainability, so IMHO it should happen in this MR.
    3. concerns about ::fixBooleansUsingConfigSchema() still being around despite your remark at https://git.drupalcode.org/project/experience_builder/-/merge_requests/1... — see
    4. I don't understand something at all that's at the heart of this MR: https://git.drupalcode.org/project/experience_builder/-/merge_requests/1..., and while I could step through it to grok it completely, I worry about the complexity
    5. It seems that simply merging in upstream has caused Playwright tests to fail — because 📌 Port Cypress tests to Playwright Active added a Playwright test after this MR was last updated:
        3) [webkit] › tests/src/Playwright/blockForm.spec.ts:47:7 › Block form › Block settings form values are stored and the preview is updated 
          Error: Timed out 10000ms waiting for expect(locator).toBeChecked()
          Locator: locator('[data-testid="xb-contextual-panel"] [data-drupal-selector="component-inputs-form"] input[data-drupal-selector="edit-xb-component-props-78c73c1d-4988-4f9b-ad17-f7e337d40c29-use-site-logo"]')
          Expected: checked
          Received: <element(s) not found>
          Call log:
            - Expect "toBeChecked" with timeout 10000ms
            - waiting for locator('[data-testid="xb-contextual-panel"] [data-drupal-selector="component-inputs-form"] input[data-drupal-selector="edit-xb-component-props-78c73c1d-4988-4f9b-ad17-f7e337d40c29-use-site-logo"]')
            61 |       `[data-testid="xb-contextual-panel"] [data-drupal-selector="component-inputs-form"] input[data-drupal-selector="edit-xb-component-props-${componentUuid}-use-site-logo"]`,
            62 |     );
          > 63 |     await expect(siteLogoCheckbox).toBeChecked();
      

      https://git.drupalcode.org/project/experience_builder/-/jobs/6186837

      (It fails consistently.)

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
  • 🇺🇸United States bnjmnm Ann Arbor, MI

    I made a small change that (at least in my manual testing) addresses the Webform Block problem mentioned in #19. 🤞 this doesn't result in a bunch of exploding tests, but easy enough to revert if it does.

  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    I reverted #28 because it caused a regression in the changes here (the test for the new behaviour failed).

    I will see if I can work out a different way to do it.

  • 🇦🇺Australia larowlan 🇦🇺🏝.au GMT+10

    So the net cause of #19 is that in a traditional form the #required = TRUE would prevent us getting to the submission handler, but we don't want to prevent that in XB

    Made some changes and it seems to be working now

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Yay, 📌 Define how block settings should be presented in the UI Active is now indeed closed, but I was wrong about 📌 [PP-1] Implement client-side validation of block settings Active (see #16 and onwards there): while this issue's MR adds validation, it doesn't add client-side validation, and that'd still be a boon for UX. See the discussion there for concrete proposals for how to achieve that.

    The adoption of #tree broke BlockComponent::fixBooleansUsingConfigSchema() — see https://git.drupalcode.org/project/experience_builder/-/merge_requests/1...

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Placing the admin menu block and configuring it to start at level 1 and show 3 levels works and renders and is valid and saves, but … I see this getting stored for my content-defined component tree:

    {"level":"1","depth":"3","expand_all_items":true,"label":"Administration","label_display":"0"}
    

    🐛 Strings, not integers!

    And if I create a Pattern out of it (to create a config-defined component tree):

    uuid: d5b012be-b463-46b9-9558-eb88ac87ce7b
    langcode: en
    status: true
    dependencies:
      config:
        - experience_builder.component.block.system_menu_block.admin
    id: sadfsadf
    label: sadfsadf
    component_tree:
      -
        uuid: 0ad04f78-0cb9-4634-9419-6091d1fb4376
        component_id: block.system_menu_block.admin
        component_version: 02b80c9f6cbacff5
        inputs:
          label: Administration
          label_display: '0'
          level: '1'
          depth: '3'
          expand_all_items: true
    

    🐛 Again strings, not integers!

    🤯 I'd expected config schema validation to catch this, but while BlockComponent::validateComponentInput() does call config schema validation, it considers these valid! (Because it does the necessary casting based on the schema.)

    Yet if I place the exact same block plugin as a block and use the exact same form to set the same settings, I get the following Block config entity:

    uuid: eb9d7af5-d296-4481-a1f7-ddfc106f699d
    langcode: en
    status: true
    dependencies:
      config:
        - system.menu.admin
      module:
        - system
      theme:
        - olivero
    id: olivero_administration
    theme: olivero
    region: header
    weight: 0
    provider: null
    plugin: 'system_menu_block:admin'
    settings:
      id: 'system_menu_block:admin'
      label: Administration
      label_display: visible
      provider: system
      level: 1
      depth: 3
      expand_all_items: true
    visibility: {  }
    

    👆 No strings, but integers.

    Will investigate…

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Apparently #32 already is the case in HEAD. Created new issue for it: 🐛 Integer BlockComponent inputs (i.e. block settings) are incorrectly saved as strings Active .

  • Pipeline finished with Success
    16 days ago
    Total: 798s
    #572010
  • Pipeline finished with Canceled
    16 days ago
    #572028
  • Pipeline finished with Failed
    16 days ago
    Total: 781s
    #572046
  • Pipeline finished with Failed
    16 days ago
    Total: 416s
    #572065
  • Pipeline finished with Canceled
    16 days ago
    Total: 127s
    #572075
  • Pipeline finished with Canceled
    16 days ago
    Total: 66s
    #572076
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    So:

    1. added docs to clarify the (pre-existing!) messy situation with $client_model = ['resolved' => …] to hopefully save the next person time, and retitled (already existing issue) 📌 Evolve the organically grown `::getClientSideInfo()` into something cohesive Active to convey this is a system (DX) problem in the XB codebase that will continue to slow us down
    2. was able to actually remove ::fixBooleansUsingConfigSchema() because it was clearly dead code — which makes my #31 obsolete
    3. to do my due diligence in having done that, re-tested 🐛 Auto-saving of blocks needs to handle string-to-bool fixing Active and I can indeed no longer reproduce it — I stepped through the entire thing with both an admin menu block and the site branding block — works perfectly 🥳
    4. the fact that BlockComponent::(validate|submit)ConfigurationForm are still untouched/unused even after making all this work (they were introduced as no-ops in 📌 Define `props` in the context of Block components Active ) just proves that we won't need these, because the component instance form will NEVER be a traditional server-side form, so: removed
    5. follow-up: 🐛 Integer BlockComponent inputs (i.e. block settings) are incorrectly saved as strings Active — pre-existing problem in HEAD, this doesn't make it worse
    6. follow-up: 📌 Rename `ComponentInputsForm` to `ComponentInstanceForm`, to match the route name Active — pre-existing problem in HEAD, this doesn't make it worse

    Given this is so clearly such an important leap forward, even if there may be imperfections, I am trying to meet with @bnjmnm in the next 30 mins, otherwise I'm going ahead and merging 😇

    Just see the lovely results:

    @bnjmnm, feel free to reopen this issue if you have lingering concerns, we can do follow-up MRs 👍

  • Pipeline finished with Success
    16 days ago
    Total: 2058s
    #572078
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Walked @bnjmnm through my findings and changes, and he then double-checked the comments I added. He's +1 so … yay, let's get this in! 🥳

  • Pipeline finished with Skipped
    16 days ago
    #572147
  • Pipeline finished with Skipped
    16 days ago
    #572148
  • Pipeline finished with Skipped
    16 days ago
    #572168
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Thanks, all, and especially @larowlan & @bnjmnm!

  • I enabled the Test Block Components module to verify this issue. While testing, I encountered an error—please see the attached video for details.

    Console error:

    {
        "status": 500,
        "data": {
            "message": "Drupal\\experience_builder\\Plugin\\ExperienceBuilder\\ComponentSource\\BlockComponent::clientModelToInput(): Argument #3 ($client_model) must be of type array, null given, called in /var/www/html/web/modules/contrib/experience_builder/src/Form/ComponentInputsForm.php on line 102",
            "file": "/var/www/html/web/modules/contrib/experience_builder/src/Plugin/ExperienceBuilder/ComponentSource/BlockComponent.php",
            "line": 403,
            "trace": [
                {
                    "file": "/var/www/html/web/modules/contrib/experience_builder/src/Form/ComponentInputsForm.php",
                    "line": 102,
                    "function": "clientModelToInput",
                    "class": "Drupal\\experience_builder\\Plugin\\ExperienceBuilder\\ComponentSource\\BlockComponent",
                    "type": "->"
                },
            ]
        }
    }
    

    @wim-leers, could you please provide the testing steps required to verify this issue?

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    @mayur-sose Test using the https://drupal.org/project/webform contrib module — before this MR that should fail, with this MR it should work — see the screenshot in #30 📌 ComponentSourceInterface::submitConfigurationForm and validateConfigurationForm are never called Active .

  • Verified the changes :

    @wim-leers, please check attached video for TC3, failed test case.

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    Nice find, @mayur-sose!

    AFAIK, "undo last action" should've in principle worked: it should've undone the change from Contact (contact) (i.e. successfully autocompleted and executed #element_validate => EntityAutoComplete::validateEntityAutocomplete) to test (which was not a valid autocomplete result).

    But it didn't.

    This gets too far into FE territory for me to determine appropriate next steps, but I'm pretty sure @bnjmn knows 🤓😊 So handing this over to him! See also his excellent overview comment at #3513742-12: Machine name JS does not work in Page Data/Component Instance form . I suspect he'll want to repurpose #3513742 to tackle this remaining bit?

    P.S.: closed #3535986 at #3535986-4: Autocomplete machine name handling does not match to machine name, webform picking does not work .

  • 🇺🇸United States bnjmnm Ann Arbor, MI

    #40 TC doesn't seem particularly Front End to me - it looks like we're allowing invalid values to be published, which can result in errors when attempting to render.

    Based on the description / video:

    1. A label is manually entered into an entity autocomplete widget that doesn't have a corresponding entity
    2. An error then occurs when attempting to publish the content with the no-corresponing-entity chosen
    AssertionError: Cannot load the "webform" entity with NULL    
                                                           ID. in assert() (line 262 of                                  
                                                           /Users/ben.mullins/Sites/drupal/core/lib/Drupal/Core/Entity/  
                                                           EntityStorageBase.php). 

    From the looks of it, the block is being published despite not meeting validation criteria. When we attempt to do this in OG Drupal, it's not possible to submit:

    I suspect we need to do something like 📌 Publish checks validation constraints, but not form validation Active but targeted to blocks, so content with invalid field values aren't publishable.
    Created 📌 Prevent publishing when block values do not #validate Active for what was spotted in #40
    Created 📌 Provide better info than "an unexpected error has occurred" Active because it's unfortunate how much helpful info is literally just sitting there to make our lives easier, but we only get "an unexpected error has occurred"

Production build 0.71.5 2024