search

neos / neos-development-collection

The unified repository containing the Neos core packages, used for Neos development.
https://www.neos.io/
GNU General Public License v3.0
260 stars 221 forks source link

!!! TASK: Node name consistency adjustments #5018

Closed nezaniel closed 2 months ago

nezaniel commented 2 months ago

~draft until #4993 is merged~

builds on https://github.com/neos/neos-development-collection/pull/4993 resolves #4150

This improves node name handling consistency by adjusting behavior in certain locations.

Unique node names for children on aggregate level

Child node aggregates must no longer share a name, even if they don't share covered dimension space points. This also means that ContentGraphInterface::findChildNodeAggregateByName now returns a single NodeAggregate, if any

Enforcement and respective tests have been adjusted

Node names are now stored in the contentgraph's node table

Changing the node name now trigger a copy on write

Manual structure adjustments regarding tethered nodes are now prohibited

Previously, it was possible to manually "fix" missing tethered nodes by renaming a node of the proper type or change the type of a node with the proper name. This is now no longer possible and instead to be performed globally via structure adjustments

Upgrade instructions

The method ContentGraphInterface::findChildNodeAggregateByName now returns a single NodeAggregate, if any and has been renamed:

- $contentGraph->findChildNodeAggregatesByName(...);
+ $contentGraph->findChildNodeAggregateByName(...);

./flow cr:setup and ./flow cr:replay contentgraph is required due to node names now being stored in the node instead of hierarchy relation table

Review instructions

coming soon

Checklist

mhsdesign commented 2 months ago

Is this ready for review?

nezaniel commented 2 months ago

I'd like #5025 merged first, afterwards, yes, I think so

nezaniel commented 2 months ago

now open for review

mhsdesign commented 2 months ago

Oh no. This turns out to be in MAJOR conflict with @kitsunet month old https://github.com/neos/neos-development-collection/pull/5028 pr

As there are also refactorings made regarding query building and stuff im not sure how to continue. Maybe a compromise would be to remove these NodeQueryBuilder optimisations from your pr @kitsunet? This pr fundamentally changes that the name of a node is now stored in the node table and not the hierarchy relation table.

Maybe even merge this one first as this contains real sql changes while the other one cosmetic ones and THESE changes must not get lost....

nezaniel commented 2 months ago

Wasn't that bad after all. I'd like #5040 merged first, though, to resolve the expected additional conflicts here

nezaniel commented 2 months ago

(as we want to release a new beta soon today tomorrow? i would not want to include it into that because of the replay, but it can be merged the second the beta was released)

Is there any particular reason why the replay should be in the next beta instead of this one?

mhsdesign commented 2 months ago

idk but it seems no replay for this beta is not possible see: https://github.com/neos/neos-development-collection/pull/5025#issuecomment-2106256549

was just thinking about keeping things simple when beta hopping and putting all migrate related stuff in one batch but youre rm as well so was just a slight thought

nezaniel commented 2 months ago

As this has to be merged (rather sooner than later) anyway, I'd say we can do so already