V14/fix/element switch validation

[Migaroez]

Prerequisites

• I have added steps to test this contribution in the description below

Related issue: #15604

Description

Based on the linked issue, we need to ensure that we:

• Don't allow element type to change to document type if used in a block structure
• Don't allow document type to change to element type if there is already some content created with it
• Don't allow new content types to establish a composition trough inheritance on a parent that has a different IsElement flag than the new type.

Because some changes to the element flag might need to be done on multiple doctypes, we can not block updating a single doctype. Therefore we will issues warnings on these scenario's instead

• Warn if any of the ancestors have a different flag than the saving doctype (if the flag has changed)
• Warn if any of the children have a different flag than the saving doctype (if the flag has changed)

Current known block structures: block grid, block list, rich text editor

Testing notes

• All error states in the description should show a human readable error.
• Warnings are shown in the client, by use of the Umb-Notifications headers.
• The UI does not currently allow inheritance doctype composition, the management api does.

Possible Breaking changes

Content Editing service constructor => Mitigated trough obsoleting old constructor.

[bjarnef]

I think it will also cause issues if changing a parent document type to element type, which I in the linked issue did for "Component" to make ModelsBuilder generate correct models (implement IPublishedElement on child nodes / components).

"Component" itself wasn't created as content, but the child "Emergency" content type (document type) was created as content node, which threw the YSOD, because the it inherited IPublishedElement instead of IPublishedContent.

[Migaroez]

Hey Bjarne, thx for looking along. You insights made me think about how strict we should be with this.

It looks to me that if we enforce more and more rules on this, we might end up in a situation where restructuring a certain (nested/partial) data model is not possible because of the limitations. And I for sure would be upset if I had to recreate it from scratch.

So maybe introducing a setting that allows these changes, but warns you instead would be nice. Not sure yet, but I will bring it up with the team. If you have any other concerns around this feel free to note them here in the mean time.

[kjac]

Looks like we have a failing unit test (Can_Create_As_Child).

I would love to see an endpoint that the client could use to preemptively prevent these destructive/offensive actions. We have one for V13, which tests for content type usage and prevents turning an in-use document type into an element. Something similar (with the added rules) would make sense in V14:

[kjac]

Upon further reflection, all the inheritance related restrictions (warnings) are only relevant with ModelsBuilder enabled. Mixing elements and non-elements works fine if ModelsBuilder does not have to generate models. That got me thinking... Should we disable these validations/warnings if ModelsBuilder is not enabled? Anyone building content types purely for Delivery API couldn't care less about models being generated 🤔

[Migaroez]

Looks like we have a failing unit test (Can_Create_As_Child).

I would love to see an endpoint that the client could use to preemptively prevent these destructive/offensive actions. We have one for V13, which tests for content type usage and prevents turning an in-use document type into an element. Something similar (with the added rules) would make sense in V14:

Agreed, I will move the logic into a service so it can be called by both the save validation, event handler warnings, and (new) prevalidation endpoints.

[Migaroez]

Upon further reflection, all the inheritance related restrictions (warnings) are only relevant with ModelsBuilder enabled. Mixing elements and non-elements works fine if ModelsBuilder does not have to generate models. That got me thinking... Should we disable these validations/warnings if ModelsBuilder is not enabled? Anyone building content types purely for Delivery API couldn't care less about models being generated 🤔

True it "works" and only models builder breaks (right now), but a state where these "rules" are broken smells like bad data architecture. Warning them seems the right thing to do anyway.

[bjarnef]

Upon further reflection, all the inheritance related restrictions (warnings) are only relevant with ModelsBuilder enabled. Mixing elements and non-elements works fine if ModelsBuilder does not have to generate models.

Note that some generates models themselves using custom code or own packages like Limbo Models Builder
https://marketplace.umbraco.com/package/limbo.umbraco.modelsbuilder

I wonder if that would cause any issues as well with inheritance @abjerner ?

[bjarnef]

I have also though about if element types should live under a separate tree in future #15082

Ideally these can be shared between document, media and member types and act as compositions or blocks shared for the content types. I would also make the element type picker cleaner.

But it a bit off-topic.

[kjac]

Tests out good 💪