!!! 9.0: Serious root nodes by nezaniel · Pull Request #4339 · neos/neos-development-collection

nezaniel · 2023-06-17T15:13:44Z

This cleans up the handling of root nodes and absolute node paths.

Root nodes don't have names but are to be resolved by type (if not identifier).
Thus, the following adjustments have been made:

Uniqueness of node type name for root nodes is now enforced on the write side (threw an exception on the read side until now)
The ContentSubgraphInterface has a new method "findRootNodeByType" similar to the ContentGraphInterface
Node paths can now be absolute by having a root node type name defined. Absolute paths can be serialized as '/<Neos.ContentRepository.Root>/my/path' with the first path segment being the root node type name
Tests for node paths have been written
ContentSubgraphInterface::findNodeByPath has been adjusted to accept either the path's root node aggregate type or the starting node aggregate ID
ContentSubgraphInterface::retrieveNodePath has been adjusted to return absolute paths with root node aggregate type ContentSubgraph::retrieveNodePath() returns invalid path #4323
ContentSubgraphInterface::retrieveNodePath also has been adjusted to fill up "missing" node names with node aggregate IDs ContentSubgraph::retrieveNodePath() when segment has no node name #4331
Adjusted the Neos endpoint API to support serialized absolute node paths for the reference editor

Review instructions

Besides the tests, try out a reference editor starting point like '/<Neos.ContentRepository.Root>/neosdemo'

Checklist

Code follows the PSR-12 coding style
Tests have been created, run and adjusted as needed
The PR is created against the 9.0 branch

mhsdesign · 2023-06-17T15:25:42Z

Should we provide a migration for people using „sites“ and rewrite this to Neos.Neos:Sites?

nezaniel · 2023-06-17T16:29:15Z

Sure

Should we provide a migration for people using „sites“ and rewrite this to Neos.Neos:Sites?

Sure thing, feel free to do it :)

…regate type also fill missing segments with node aggregate IDs

nezaniel · 2023-06-18T15:38:36Z

Still missing: UI integration, will follow in a bit

mhsdesign

Just some early thoughts ;)

…dAncestors

nezaniel · 2023-06-23T11:03:43Z

I think I covered everything, if the tests pass and there are no more important concerns, I'd like to merge

nezaniel · 2023-06-23T11:10:30Z

I think I covered everything, if the tests pass and there are no more important concerns, I'd like to merge

mhsdesign

Lets do it ;)

mficzel · 2023-07-02T17:15:49Z

Just tested this and the last commit 61b4d48 introduces an error in the ui whenever a reference error is opened
Could not convert target type "string": Warning: Array to string conversion in /tmp/neos/Development/SubContextddev/Cache/Code/Flow_Object_Classes/Neos_Flow_Property_TypeConverter_StringConverter.php line 114 - Check the logs for details

With the commit before 5b7e06c i could verify:

referencing of nodes in the same site (+ rendering links)
linking of nodes nodes in other sites (+ rendering links)
linking to nodes in totally different root-nodes (taxonomies)

otherwise flow will get confused and throw exceptions that type conversion failed

mficzel · 2023-07-02T17:23:03Z

Node: I could fix the problem with adjusting bringing the parameter comments into the same order as the function parameters. Pushed this change to the branch.

mficzel

Fine by testing .... what i tested:

referencing documents with configured starting point in the same site
referencing documents with configured starting point in another site
referencing other nodes (taxonomies) with a startingpoint other than /<Neos.Neos:Sites>

All three cases above worked and the nodes that were chosen by references editor could be used for traversal and link generation.

What i could not test is linking nodes in another cr which is uncharted territory even with this pr !!!

I personally cannot say that much about coding principles in the new cr other than that the code makes sense to me. So maybe count this as half an approval.

mficzel

Noticed a linting error in ci that obviously should be addressed. Seems to originate from changes made elsewhere since it also appeared in the last commit after running the test again.

nezaniel · 2023-07-05T19:43:16Z

I don't quite understand the linting error and since I cannot reproduce it locally, I'd just ignore it.
As for the test failure, I don't understand it either, and it should be resolved where it was introduced.

Go?

bwaidelich · 2023-07-07T08:08:20Z

Thanks a lot for this.
~~Unfortunately this seems to break Neos and neos/neos-ui#3530 does not seem to cover everything~~
Nevermind, I think it does fix the issue :)

bwaidelich · 2023-07-07T08:08:36Z

e.g. I immediately get

Warning: Undefined property: Neos\ContentRepository\Core\Projection\ContentGraph\AbsoluteNodePath::$value in Cache/Code/Flow_Object_Classes/Neos_Neos_Ui_FlowQueryOperations_NeosUiDefaultNodesOperation.php line 88

…yType` we dont do constraints in the read side. We have command validation see neos#4339 and there are ProjectionIntegrityChecks but during reading we trust the graph

…yType` we dont do constraints in the read side. We have command validation see neos/neos-development-collection#4339 and there are ProjectionIntegrityChecks but during reading we trust the graph

Bernhard Schmitt added 4 commits June 17, 2023 15:41

Enforce type-uniqueness of root nodes

9d63932

Remove usage of dynamic properties in tests

9f5f2c0

Implement ContentSubgraphInterface::findRootNodeByType

dd4ab5a

Refactor node paths to being able to be absolute

8cdeac1

github-actions Bot added the 9.0 label Jun 17, 2023

mhsdesign mentioned this pull request Sep 19, 2023

Provide a migration for people using „sites“ and rewrite this to Neos.Neos:Sites? neos/rector#24

Closed

Bernhard Schmitt added 2 commits June 18, 2023 16:22

Add test cases for ContentSubgraphInterface::findNodeByPath

a7f9cd5

Adjust ContentSubgraph to retrieve node paths including root node agg…

f8b1b66

…regate type also fill missing segments with node aggregate IDs

nezaniel requested review from bwaidelich, grebaldi, mficzel, mhsdesign and skurfuerst and removed request for bwaidelich June 18, 2023 15:25

Bernhard Schmitt added 6 commits June 18, 2023 18:21

Adjust test cases to use distinct root node types

70e2dac

Properly serialize adjusted node paths

7f0e9b0

Pacify linter

0df3029

Adjust Neos nodes endpoint to paths

8088a4e

Pacify linter

5dcdb4b

Adjust documentation about paths

9e5add7

nezaniel changed the title ~~WIP: Serious root nodes~~ 9.0: Serious root nodes Jun 18, 2023

nezaniel requested a review from kitsunet June 18, 2023 19:46

mhsdesign reviewed Jun 19, 2023

View reviewed changes

Comment thread Neos.ContentRepository.Core/Classes/Projection/ContentGraph/NodePath.php Outdated

Comment thread Neos.ContentRepository.Core/Classes/Projection/ContentGraph/ContentSubgraphInterface.php Outdated

Bernhard Schmitt added 3 commits June 19, 2023 23:56

Separate absolute from relative paths

2b4e5b6

Adjust path tests

93561c1

Properly render node helper path exception

9924830

Bernhard Schmitt added 2 commits June 22, 2023 17:26

Merge branch '9.0' into seriousRootNodes

055ed82

refactor usage of ContentSubgraphInterface::retrieveNodePath to ::fin…

5b7e06c

…dAncestors

nezaniel requested review from bwaidelich, grebaldi and mhsdesign June 23, 2023 11:00

move added fallback node identifiers to back of argument list

61b4d48

mhsdesign approved these changes Jun 23, 2023

View reviewed changes

BUGFIX: Use same order for parameter comments as is used in function

96ef199

otherwise flow will get confused and throw exceptions that type conversion failed

mficzel approved these changes Jul 2, 2023

View reviewed changes

mficzel requested changes Jul 2, 2023

View reviewed changes

Bernhard Schmitt added 2 commits July 4, 2023 23:09

accept both single and multiple node IDs in NodesController

96f5e77

Add missing type annotations

df4f11d

nezaniel requested a review from mficzel July 5, 2023 20:00

mficzel approved these changes Jul 5, 2023

View reviewed changes

nezaniel merged commit e9f3c6d into 9.0 Jul 6, 2023

nezaniel deleted the seriousRootNodes branch July 6, 2023 08:25

mhsdesign mentioned this pull request Jun 22, 2024

!!! TASK: Streamline ContentGraph::findRootNodeAggregateByType #5158

Merged

6 tasks

grebaldi mentioned this pull request Jul 15, 2024

BUG: Node search in Link editor (RTE) leads to exception neos/neos-ui#3829

Closed

Uh oh!

Conversation

nezaniel commented Jun 17, 2023 • edited by mhsdesign Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mhsdesign commented Jun 17, 2023

Uh oh!

nezaniel commented Jun 17, 2023

Uh oh!

nezaniel commented Jun 18, 2023

Uh oh!

mhsdesign left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

nezaniel commented Jun 23, 2023

Uh oh!

nezaniel commented Jun 23, 2023

Uh oh!

mhsdesign left a comment

Choose a reason for hiding this comment

Uh oh!

mficzel commented Jul 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mficzel commented Jul 2, 2023

Uh oh!

mficzel left a comment

Choose a reason for hiding this comment

Uh oh!

mficzel left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nezaniel commented Jul 5, 2023

Uh oh!

bwaidelich commented Jul 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bwaidelich commented Jul 7, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

nezaniel commented Jun 17, 2023 •

edited by mhsdesign

Loading

mficzel commented Jul 2, 2023 •

edited

Loading

mficzel left a comment •

edited

Loading

bwaidelich commented Jul 7, 2023 •

edited

Loading