Contrib.social

Tighten validation of `experience_builder.generated_field_explicit_input_ux: prop_field_definitions`

Open on Drupal.org →
Created on 27 May 2025, 7 days ago

Overview

In HEAD:

          default_value:
            # TRICKY: this MUST be a sequence, to allow for a multi-value default for `type: array` prop shapes.
            type: sequence
            label: 'Default values'
            sequence:
              type: field.value.[%parent.%parent.field_type]
            # @todo Add validation constraint because this is only *conditionally* nullable: if the SDC prop is optional
            nullable: true

This issue is about adding that missing validation.

Proposed resolution

Take inspiration from:

  • XB's KeyForEverySdcProp validation constraint
  • core's NotNull validation constraint

⇒

  1. Add a new NotNullValueForEveryRequiredSdcProp validation constraint. That should be able to follow the lead of the logic for KeyForEverySdcProp's validator. However, that uses \Drupal\experience_builder\PropShape\PropShape::getComponentProps(), which only looks at shapes, not required vs optional. To do that, use
    assert($component_plugin instanceof Drupal\Core\Plugin\Component);
$component_schema = $component_plugin->metadata->schema ?? [];
$required_props = $component_schema['required'] ?? [];

    For each of $required_props, a key must exist in the sequence, and it must have a not-null value. That's the validation logic we need 😊

  2. Add new ComponentValidationTest::testInvalidWidgetSettings() test that does
        $settings = $this->entity->getSettings();
    assert($settings['prop_field_definitions']['text']['default_value'] !== NULL);
    $settings['prop_field_definitions']['text']['default_value'] = NULL;
    $this->entity->setSettings($settings);
    $this->assertValidationErrors(['settings.prop_field_definitions.text.default_value' => 'a meaningfulerror message']);

User interface changes

None.

📌 Task
Status

Active

Version

0.0

Component

Config management

Created by

🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

  • Novice

    It would make a good project for someone who is new to the Drupal contribution process. It's preferred over Newbie.

Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @wim leers
  • Comment 7 days ago →
    🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
  • Comment 7 days ago →
    🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺
  • Comment 6 days ago →
    🇮🇳India libbna New Delhi, India

    I will give it a try!

  • Merge request !1078created new validation constraint for NotNullValueForEveryRequiredSdcProp. → (Open) created by libbna
  • Comment 6 days ago →
    🇮🇳India libbna New Delhi, India

    I've implemented a new validation constraint NotNullValueForEveryRequiredSdcProp to ensure that all required props defined in a component's schema have non-null entries in the default_value mapping. This is based on the required array from the component plugin's metadata schema. The logic closely follows the pattern used in KeyForEverySdcProp.

    I've tried to achieve the requirement as described—please let me know if anything is missed or needs adjustment.

  • Pipeline finished with Failed
    6 days ago
    #508399
  • Comment 5 days ago →
    🇮🇳India libbna New Delhi, India

    Hi @wim leers to test my code I followed the below steps:

    1. edit the existing component and remove the default text from the required prop
    2. saved the component

    Let me know if this is correct or have I missed anything?Thanks.

  • Comment 5 days ago →
    🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    This is not yet passing tests. 😇 Left a review with pointers.

  • Pipeline finished with Failed
    about 15 hours ago
    Total: 983s
    #511986
  • Comment about 15 hours ago →
    🇮🇳India libbna New Delhi, India

    I resolved the phpstan issue by adding " // @phpstan-ignore argument.type" comment by taking reference from

    KeyForEverySdcProp

    validator file. Please review and let me know if anymore changes are required.

    Unassinging myself so that someone else can work for test cases and keeping the issue to needs work only.

  • Comment about 15 hours ago →
    🇮🇳India libbna New Delhi, India
  • Pipeline finished with Failed
    about 15 hours ago
    Total: 765s
    #512013
contrib.social Blog FAQ Discussions
Production build 0.71.5 2024