Merge pull request #5289 from mhsdesign/bugfix/improve-error-of-neare…

…st-content-collection BUGFIX: Improve error of nearest content collection
neos · Oct 17, 2024 · 3bfc572 · 3bfc572
2 parents 950f805 + c0572bc
commit 3bfc572
Show file tree

Hide file tree

Showing 4 changed files with 77 additions and 58 deletions.
diff --git 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
@@ -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 === '') {
-                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,59 @@ 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 === '') {
+                $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 (%s) and no node path was provided.'
+                    . ' You might want to configure the nodePath property'
+                    . ' with a relative path to the content collection.',
+                    $contentCollectionType,
+                    $nodePathOfNode->value
+                ), 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::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 (%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:
      *

diff --git 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);
+    }
 }
diff --git 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