Constraint slot names allowed by XB in its component tree

wim leers → credited phenaproxima → .

Thanks, @phenaproxima at https://git.drupalcode.org/project/experience_builder/-/merge_requests/8..., for making me realize this!

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

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

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.

Because this is (well, must be) a XB-wide restriction, this
does not belong in \Drupal\experience_builder\ComponentSource\ComponentSourceInterface::checkRequirements(), but at the config entity level.

The issue summary is already accurate.

I guess I'll see how far I can take this.

Before I write test coverage, would love some quick confirmation that this approach is sound, or if there's a better or different place this should go.

Well on track! 👏

I added some light test coverage here, but I have a question about where this validation belongs. The MR feedback seems to conflict with #9, and I'm not sure whether to believe now-Wim or past-Wim. :)

Been thinking about this more, and the sense I'm getting is that what we want here is validation on multiple fronts. In other words:

• It shouldn't be possible to save a Component entity if its source plugin defines any invalid slots. This is what I've implemented so far in the MR.
• Nor should it be possible for components laid out in a tree to be sitting in an invalid slot (regardless of where and how that tree is stored).

#16++

#15: good question, responded on the MR 😊

This looks very close! 🥳

Let's make sure to test that the UI handles this gracefully to not introduce more error handling debt.

This looks great!

I wanted to RTBC & merge, but:

• it's not yet passing CI
• I think the validation probably should be tightened slightly, do you agree?

Looking great! Reviewing, and adjusting slightly now that 📌 Version component prop definitions for SDC and Code components Active is in: that requires some changes to the newly added tests.

Adjusted for 📌 Version component prop definitions for SDC and Code components Active 👍

Follow-up: 📌 Create unit test coverage for `ValidSlotNameConstraintValidator` Active .

Performed manual testing.

• When creating a code component with -foo as the entered slot name: foo becomes the machine slot name automatically, the title remains -foo
• When creating a code component with "n😈t_valid as the entered slot name:

Actually, found a bug in exposed slot name validation… fixing.

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