From 3a6b701d547a0cde121ff71569556be587462ca2 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Mon, 14 Oct 2024 15:46:23 +0200 Subject: [PATCH 1/4] TASK: Improve error for `nearestContentCollection` Fatal error: Uncaught TypeError: Neos\Neos\Fusion\Helper\NodeHelper_Original::nearestContentCollection(): Argument #2 ($nodePath) must be of type string, null given --- Neos.Neos/Classes/Fusion/Helper/NodeHelper.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Neos.Neos/Classes/Fusion/Helper/NodeHelper.php b/Neos.Neos/Classes/Fusion/Helper/NodeHelper.php index 643eb38e2a2..1137d57c780 100644 --- a/Neos.Neos/Classes/Fusion/Helper/NodeHelper.php +++ b/Neos.Neos/Classes/Fusion/Helper/NodeHelper.php @@ -53,13 +53,13 @@ class NodeHelper implements ProtectedContextAwareInterface * * @throws Exception */ - public function nearestContentCollection(Node $node, string $nodePath): Node + public function nearestContentCollection(Node $node, ?string $nodePath): Node { $contentCollectionType = NodeTypeNameFactory::NAME_CONTENT_COLLECTION; if ($this->isOfType($node, $contentCollectionType)) { return $node; } else { - if ($nodePath === '') { + if ($nodePath === null || $nodePath === '') { throw new Exception(sprintf( 'No content collection of type %s could be found in the current node and no node path was provided.' . ' You might want to configure the nodePath property' From 688ba821b617a99c102d12f9e0460875c7a7ee1a Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Mon, 14 Oct 2024 15:47:30 +0200 Subject: [PATCH 2/4] TASK: Declare `nearestContentCollection` internal implementation detail of Neos.Neos:ContentCollection --- .../Classes/Fusion/Helper/NodeHelper.php | 99 ++++++++++--------- 1 file changed, 50 insertions(+), 49 deletions(-) diff --git a/Neos.Neos/Classes/Fusion/Helper/NodeHelper.php b/Neos.Neos/Classes/Fusion/Helper/NodeHelper.php index 1137d57c780..a48c80385e6 100644 --- a/Neos.Neos/Classes/Fusion/Helper/NodeHelper.php +++ b/Neos.Neos/Classes/Fusion/Helper/NodeHelper.php @@ -47,55 +47,6 @@ class NodeHelper implements ProtectedContextAwareInterface #[Flow\Inject] protected NodeLabelGeneratorInterface $nodeLabelGenerator; - /** - * Check if the given node is already a collection, find collection by nodePath otherwise, throw exception - * if no content collection could be found - * - * @throws Exception - */ - public function nearestContentCollection(Node $node, ?string $nodePath): Node - { - $contentCollectionType = NodeTypeNameFactory::NAME_CONTENT_COLLECTION; - if ($this->isOfType($node, $contentCollectionType)) { - return $node; - } else { - if ($nodePath === null || $nodePath === '') { - throw new Exception(sprintf( - 'No content collection of type %s could be found in the current node and no node path was provided.' - . ' You might want to configure the nodePath property' - . ' with a relative path to the content collection.', - $contentCollectionType - ), 1409300545); - } - $nodePath = NodePath::fromString($nodePath); - $subgraph = $this->contentRepositoryRegistry->subgraphForNode($node); - - $subNode = $subgraph->findNodeByPath($nodePath, $node->aggregateId); - - if ($subNode !== null && $this->isOfType($subNode, $contentCollectionType)) { - return $subNode; - } else { - $nodePathOfNode = VisualNodePath::fromAncestors( - $node, - $this->contentRepositoryRegistry->subgraphForNode($node) - ->findAncestorNodes( - $node->aggregateId, - FindAncestorNodesFilter::create() - ) - ); - throw new Exception(sprintf( - 'No content collection of type %s could be found in the current node (%s) or at the path "%s".' - . ' You might want to adjust your node type configuration and create the missing child node' - . ' through the "flow structureadjustments:fix --node-type %s" command.', - $contentCollectionType, - $nodePathOfNode->value, - $nodePath->serializeToString(), - $node->nodeTypeName->value - ), 1389352984); - } - } - } - /** * Renders the actual node label based on the NodeType definition in Fusion. */ @@ -193,6 +144,56 @@ public function subgraphForNode(Node $node): ContentSubgraphInterface return $this->contentRepositoryRegistry->subgraphForNode($node); } + /** + * Check if the given node is already a collection, find collection by nodePath otherwise, throw exception + * if no content collection could be found + * + * @throws Exception + * @internal implementation detail of Neos.Neos:ContentCollection + */ + public function nearestContentCollection(Node $node, ?string $nodePath): Node + { + $contentCollectionType = NodeTypeNameFactory::NAME_CONTENT_COLLECTION; + if ($this->isOfType($node, $contentCollectionType)) { + return $node; + } else { + if ($nodePath === null || $nodePath === '') { + throw new Exception(sprintf( + 'No content collection of type %s could be found in the current node and no node path was provided.' + . ' You might want to configure the nodePath property' + . ' with a relative path to the content collection.', + $contentCollectionType + ), 1409300545); + } + $nodePath = NodePath::fromString($nodePath); + $subgraph = $this->contentRepositoryRegistry->subgraphForNode($node); + + $subNode = $subgraph->findNodeByPath($nodePath, $node->aggregateId); + + if ($subNode !== null && $this->isOfType($subNode, $contentCollectionType)) { + return $subNode; + } else { + $nodePathOfNode = VisualNodePath::fromAncestors( + $node, + $this->contentRepositoryRegistry->subgraphForNode($node) + ->findAncestorNodes( + $node->aggregateId, + FindAncestorNodesFilter::create() + ) + ); + throw new Exception(sprintf( + 'No content collection of type %s could be found in the current node (%s) or at the path "%s".' + . ' You might want to adjust your node type configuration and create the missing child node' + . ' through the "flow structureadjustments:fix --node-type %s" command.', + $contentCollectionType, + $nodePathOfNode->value, + $nodePath->serializeToString(), + $node->nodeTypeName->value + ), 1389352984); + } + } + } + /** * Return a builder to generate a label for a node with a chaining mechanism. To be used in NodeType definition: * From 5901adced3c1171f390bee4b9c69a33acc610468 Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Mon, 14 Oct 2024 16:36:36 +0200 Subject: [PATCH 3/4] TASK: Improve error of exception message by using node labels instead of their name and id No content collection of type Neos.Neos:ContentCollection could be found in the current node (/Neos.Neos:Sites/Home) or at the path "teaser" --- .../NodeLabel/DelegatingNodeLabelRenderer.php | 4 +-- .../Classes/Fusion/Helper/NodeHelper.php | 19 ++++++++------ .../Classes/Presentation/VisualNodePath.php | 25 +++++++++++++++---- 3 files changed, 33 insertions(+), 15 deletions(-) diff --git a/Neos.Neos/Classes/Domain/NodeLabel/DelegatingNodeLabelRenderer.php b/Neos.Neos/Classes/Domain/NodeLabel/DelegatingNodeLabelRenderer.php index 57f774c4003..fa09e3b0b21 100644 --- a/Neos.Neos/Classes/Domain/NodeLabel/DelegatingNodeLabelRenderer.php +++ b/Neos.Neos/Classes/Domain/NodeLabel/DelegatingNodeLabelRenderer.php @@ -59,10 +59,10 @@ private function getDelegatedGenerator(?NodeType $nodeType): NodeLabelGeneratorI public function getLabel(Node $node): string { return sprintf( - '%s %s', + '%s%s', $node->nodeTypeName->value, $node->name - ? sprintf('(%s)', $node->name->value) + ? sprintf(' (%s)', $node->name->value) : '' ); } diff --git a/Neos.Neos/Classes/Fusion/Helper/NodeHelper.php b/Neos.Neos/Classes/Fusion/Helper/NodeHelper.php index a48c80385e6..033b5d58863 100644 --- a/Neos.Neos/Classes/Fusion/Helper/NodeHelper.php +++ b/Neos.Neos/Classes/Fusion/Helper/NodeHelper.php @@ -158,11 +158,17 @@ public function nearestContentCollection(Node $node, ?string $nodePath): Node return $node; } else { if ($nodePath === null || $nodePath === '') { + $nodePathOfNode = VisualNodePath::buildFromAncestors( + $node, + $this->contentRepositoryRegistry->get($node->contentRepositoryId), + $this->nodeLabelGenerator + ); throw new Exception(sprintf( - 'No content collection of type %s could be found in the current node and no node path was provided.' + 'No content collection of type %s could be found in the current node (%s) and no node path was provided.' . ' You might want to configure the nodePath property' . ' with a relative path to the content collection.', - $contentCollectionType + $contentCollectionType, + $nodePathOfNode->value ), 1409300545); } $nodePath = NodePath::fromString($nodePath); @@ -173,13 +179,10 @@ public function nearestContentCollection(Node $node, ?string $nodePath): Node if ($subNode !== null && $this->isOfType($subNode, $contentCollectionType)) { return $subNode; } else { - $nodePathOfNode = VisualNodePath::fromAncestors( + $nodePathOfNode = VisualNodePath::buildFromAncestors( $node, - $this->contentRepositoryRegistry->subgraphForNode($node) - ->findAncestorNodes( - $node->aggregateId, - FindAncestorNodesFilter::create() - ) + $this->contentRepositoryRegistry->get($node->contentRepositoryId), + $this->nodeLabelGenerator ); throw new Exception(sprintf( 'No content collection of type %s could be found in the current node (%s) or at the path "%s".' diff --git a/Neos.Neos/Classes/Presentation/VisualNodePath.php b/Neos.Neos/Classes/Presentation/VisualNodePath.php index aaeff1da893..e28180144bb 100644 --- a/Neos.Neos/Classes/Presentation/VisualNodePath.php +++ b/Neos.Neos/Classes/Presentation/VisualNodePath.php @@ -14,11 +14,16 @@ namespace Neos\Neos\Presentation; +use Neos\ContentRepository\Core\ContentRepository; +use Neos\ContentRepository\Core\Projection\ContentGraph\Filter\FindAncestorNodesFilter; use Neos\ContentRepository\Core\Projection\ContentGraph\Node; use Neos\ContentRepository\Core\Projection\ContentGraph\Nodes; +use Neos\Neos\Domain\NodeLabel\NodeLabelGeneratorInterface; /** - * The string-based visual node path, composed of node names and node aggregate IDs as fallback + * The string-based visual node path + * + * @internal helper for enriched debug information */ final readonly class VisualNodePath { @@ -27,13 +32,23 @@ private function __construct( ) { } - public static function fromAncestors(Node $leafNode, Nodes $ancestors): self + public static function buildFromNodes(Nodes $nodes, NodeLabelGeneratorInterface $nodeLabelGenerator): self { $pathSegments = []; - foreach ($ancestors->reverse() as $ancestor) { - $pathSegments[] = $ancestor->name?->value ?: '[' . $ancestor->aggregateId->value . ']'; + foreach ($nodes as $node) { + $pathSegments[] = $nodeLabelGenerator->getLabel($node); } - return new self('/' . implode('/', $pathSegments)); } + + public static function buildFromAncestors(Node $startingNode, ContentRepository $contentRepository, NodeLabelGeneratorInterface $nodeLabelGenerator): self + { + $nodes = $contentRepository->getContentGraph($startingNode->workspaceName) + ->getSubgraph($startingNode->dimensionSpacePoint, $startingNode->visibilityConstraints) + ->findAncestorNodes( + $startingNode->aggregateId, + FindAncestorNodesFilter::create() + )->reverse()->append($startingNode); + return self::buildFromNodes($nodes, $nodeLabelGenerator); + } } From c0572bc71aaaf8ca306894c90c464b4d883abdcc Mon Sep 17 00:00:00 2001 From: mhsdesign <85400359+mhsdesign@users.noreply.github.com> Date: Mon, 14 Oct 2024 21:40:43 +0200 Subject: [PATCH 4/4] TASK: Adjust test assertions --- .../Tests/Behavior/Features/Fusion/ContentCollection.feature | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Neos.Neos/Tests/Behavior/Features/Fusion/ContentCollection.feature b/Neos.Neos/Tests/Behavior/Features/Fusion/ContentCollection.feature index f2cf6d5d7ed..e372eb7ab2c 100644 --- a/Neos.Neos/Tests/Behavior/Features/Fusion/ContentCollection.feature +++ b/Neos.Neos/Tests/Behavior/Features/Fusion/ContentCollection.feature @@ -75,7 +75,7 @@ Feature: Tests for the "Neos.Neos:ContentCollection" Fusion prototype """ Then I expect the following Fusion rendering error: """ - No content collection of type Neos.Neos:ContentCollection could be found in the current node (/[root]) or at the path "to-be-set-by-user". You might want to adjust your node type configuration and create the missing child node through the "flow structureadjustments:fix --node-type Neos.Neos:Site" command. + No content collection of type Neos.Neos:ContentCollection could be found in the current node (/Neos.Neos:Sites/Neos.Neos:Site) or at the path "to-be-set-by-user". You might want to adjust your node type configuration and create the missing child node through the "flow structureadjustments:fix --node-type Neos.Neos:Site" command. """ Scenario: invalid nodePath @@ -90,7 +90,7 @@ Feature: Tests for the "Neos.Neos:ContentCollection" Fusion prototype """ Then I expect the following Fusion rendering error: """ - No content collection of type Neos.Neos:ContentCollection could be found in the current node (/[root]) or at the path "invalid". You might want to adjust your node type configuration and create the missing child node through the "flow structureadjustments:fix --node-type Neos.Neos:Site" command. + No content collection of type Neos.Neos:ContentCollection could be found in the current node (/Neos.Neos:Sites/Neos.Neos:Site) or at the path "invalid". You might want to adjust your node type configuration and create the missing child node through the "flow structureadjustments:fix --node-type Neos.Neos:Site" command. """ Scenario: empty ContentCollection