Overview

In ComponentTreeStructureConstraintValidator we have the todo

// @todo Validate that the string is a valid UUID. *And* that it is unique in the tree.

Proposed resolution

Add this validation.

See if we can do this all in \Drupal\experience_builder\Plugin\Validation\Constraint\ComponentTreeStructureConstraintValidator::validateTree

User interface changes

None

Issue fork experience_builder-3471026

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

tedbow created an issue. See original summary.

abhisekmazumdar’s picture

abhisekmazumdar
he/him
Primary language English
Location India
commented
Assigned: Unassigned » abhisekmazumdar

Picking this up and giving my best try.

abhisekmazumdar opened merge request !288

abhisekmazumdar’s picture

abhisekmazumdar
he/him
Primary language English
Location India
commented
Assigned: abhisekmazumdar » Unassigned
Status: Active » Needs review

The MR is ready for review.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: Unassigned » tedbow

Thanks @abhisekmazumdar!

@tedbow Thoughts on all the test-only UUIDs that now cause failures, such as

+    'tree[uuid-in-root][the_body][0][uuid]' => 'The UUID "<em class="placeholder">uuid-level-1</em>" is not a valid UUID.'
+    'tree[uuid-level-2][the_body][0][uuid]' => 'The UUID "<em class="placeholder">uuid-level-3</em>" is not a valid UUID.'
+    'tree[uuid-level-2][the_body][1][uuid]' => 'The UUID "<em class="placeholder">uuid-last-in-tree</em>" is not a valid UUID.'
+    'tree[uuid-level-1][the_body][0][uuid]' => 'The UUID "<em class="placeholder">uuid-level-2</em>" is not a valid UUID.'

?

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Harden UUID validation in ComponentTreeStructureConstraintValidator » [PP-1] Harden UUID validation in ComponentTreeStructureConstraintValidator
Assigned: tedbow » Unassigned
Status: Needs review » Postponed
Related issues: +#3472299: Update default config to make a fresh install result in an XB UI with an empty canvas

LGTM! Well done, @abhisekmazumdar 😊 That looks as simple/elegant as I'd hoped 😄

#3472299: Update default config to make a fresh install result in an XB UI with an empty canvas will change/reduce the amount of test expectations that need to be updated, so postponing this on that.

abhisekmazumdar’s picture

abhisekmazumdar
he/him
Primary language English
Location India
commented
Version: » 0.x-dev

Can we resume work on this ?

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: [PP-1] Harden UUID validation in ComponentTreeStructureConstraintValidator » Harden UUID validation in ComponentTreeStructureConstraintValidator
Status: Postponed » Needs work
Issue tags: +Novice, +Barcelona2024

Yes! Great catch! Thanks, @abhisekmazumdar :)

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: -Barcelona2024
Parent issue: » #3450586: [META] Back-end Kanban issue tracker

Didn't happen at Barcelona, but let's get it on https://contribkanban.com/board/experience_builder now 😄

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: Unassigned » wim leers
Priority: Minor » Normal
Status: Needs work » Postponed
Issue tags: -Novice +Configuration schema, +validation, +stable blocker

Given the huge range of tests that would need updating by now, I don't think this is Novice anymore.

It's something that might be a stable blocker. Doing it now doesn't buy us much though, so postponing.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: Harden UUID validation in ComponentTreeStructureConstraintValidator » [PP-1] Harden UUID validation in ComponentTreeStructureConstraintValidator
Assigned: wim leers » Unassigned
Parent issue: #3450586: [META] Back-end Kanban issue tracker » #3520449: [META] Production-ready data storage
Related issues: +#3468272: Store the ComponentTreeStructure field property one row per component instance

It looks like #3468272: Store the ComponentTreeStructure field property one row per component instance will largely do this!

wim leers credited larowlan.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: [PP-1] Harden UUID validation in ComponentTreeStructureConstraintValidator » Harden UUID validation in ComponentTreeStructureConstraintValidator
Status: Postponed » Closed (duplicate)

Reviewing #3468272: Store the ComponentTreeStructure field property one row per component instance right now, it 100% certainly addresses this! 🥳

wim leers closed merge request !288