Overview

    First there was the ability to store XB Component trees in content entities, in the XB field type (\Drupal\experience_builder\Plugin\Field\FieldType\ComponentTreeItem and its tree + props field props).
  1. Then there was the ability to store XB Component trees in config, for the XB field type's default value (type: experience_builder.component_tree).
  2. Then a validation constraint was added that can consistently validate both of the above using the same logic: ValidComponentTree, added in #3456024: Lift most logic out of ComponentTreeItem::preSave() and into a new validation constraint
  3. Then a second, more detailed validation constraint ComponentTreeStructure was added for the tree prop of a XB Component tree, in e #3460856: Create validation constraint for ComponentTreeStructure
  4. Yesterday, a third validation constraint was added: ComponentTreeMeetRequirements, used by the new PageTemplate config entity, because while both of the above validation constraints are also in use and are still relevant, some kinds of XB Component trees just do not make sense at the "page" level: they cannot use DynamicPropSources (because the host entity, could be different for each page aka route/controller — or even none!). Added in #3478537: Introduce an XB `PageTemplate` config entity

Proposed resolution

Use the new ComponentTreeMeetRequirements validation constraint on:

  1. ✅ the XB PageTemplate config entity to disallow DynamicPropSource — done in #3478537: Introduce an XB `PageTemplate` config entity 👍
  2. the XB field type (ComponentTreeItem), to disallow DynamicPropSource — because preliminary designs for 7.1 Tokens (see #3455753: Milestone 0.2.0: Early preview) indicate that only the site builder will be able to link XB Component props to DynamicPropSources on the host entity type, aka use base/bundle fields' values.
  3. 💁 #3479643: Introduce a `Pattern` config entity's issue summary with the sample config schema already uses this new constraint, just like the XB PageTemplate config entity uses it 👍
  4. 💁 if/when #3455629: [PP-1] [META] 7. Content Templates — aka "default layouts" — affects the tree+props data model is worked on, the XB ContentTypeTemplate config entity allow both StaticPropSource and DynamicPropSource

User interface changes

None.

Issue fork experience_builder-3481720

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

wim leers created an issue. See original summary.

wim leers opened merge request !369

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: wim leers » tedbow
Issue summary: View changes
Status: Active » Needs work

I expect this to fail like so:

1) Drupal\Tests\experience_builder\Kernel\Plugin\Field\FieldType\ComponentTreeItemTest::testInvalidField with data set "valid values using dynamic props" (['{"a548b48d-58a8-4077-aa04-da9...ts"}}}', '{"dynamic-static-card2df":{"h...ue"}}}'], [])
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
-Array &0 []
+Array &0 [
+    'field_xb_test.0' => 'The 'dynamic' prop source type must be absent.',
+]

This has zero impact on the UI today, only on tests that need to be refactored a bit. @tedbow is best suited for that, because he worked on related validation constraints a few months ago and introduced ComponentTreeTestTrait which will need to have its uses of DynamicPropSources split out because many of those won’t be allowed anymore — as the cited test output failure shows.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue summary: View changes
Related issues: +#3479643: Introduce a `Pattern` config entity

tedbow made their first commit to this issue’s fork.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Related issues: +#3455629: [PP-1] [META] 7. Content Templates — aka "default layouts" — affects the tree+props data model

Added a comment in #3455629-33: [PP-1] [META] 7. Content Templates — aka "default layouts" — affects the tree+props data model because that issue will need to add back some of what is being remove here related to translation.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: tedbow » Unassigned
Status: Needs work » Needs review
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: Unassigned » wim leers
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: wim leers » tedbow
Status: Needs review » Reviewed & tested by the community

Clearly I underestimated the complexity of landing this, because I'd completely forgotten about @tedbow's excellent TranslationTest that verifies the correct translation of the host entity's base/bundle field is being used! 🙈

Thanks, @tedbow, for so gracefully pushing this across the finish line 🙏

As far as I'm concerned, this is RTBC. This still needs @tedbow's approval on the MR, so @tedbow: this is all yours to merge 😊

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: tedbow » f.mazeikis

I think the MR needs @f.mazeikis' approval because of the changes to `/config/schema/`

f.mazeikis’s picture

f.mazeikis commented

Looks good, approved

  • tedbow committed ba1cd9bd on 0.x authored by wim leers 
    Issue #3481720 by tedbow, wim leers: Tighten validation: only allow...
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Status: Reviewed & tested by the community » Fixed
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: f.mazeikis » Unassigned
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

🚀 This implemented point 2 of the proposed resolution. Point 1 already happened previously. Points 3 and 4 have their own respective issues 👍

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.