From 69267ba0b6dac7c8c099b3ef07afde7d9a526dcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kai=20M=C3=B6ller?= Date: Fri, 12 Jun 2020 20:04:25 +0200 Subject: [PATCH 1/4] FEATURE: set unique form element identifier after node copy When a form element with identifier propery is copied an exception is thrown. @see https://github.com/neos/form/blob/3a6122db36ad2c9e6f06316653a35c80ff47dee1/Classes/Core/Model/FormDefinition.php#L553 This changes adds a signal slot for \Neos\ContentRepository\Domain\Model\Node "afterNodeCopy" to make the form element identifier unique. --- Classes/Package.php | 57 ++++++++++++++++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 13 deletions(-) diff --git a/Classes/Package.php b/Classes/Package.php index 72fbcad..0209171 100644 --- a/Classes/Package.php +++ b/Classes/Package.php @@ -12,6 +12,8 @@ */ class Package extends BasePackage { + const NODE_TYPE_IDENTIFIER_MIXIN = 'Neos.Form.Builder:IdentifierMixin'; + /** * @param Bootstrap $bootstrap The current bootstrap * @return void @@ -21,23 +23,52 @@ public function boot(Bootstrap $bootstrap) $dispatcher = $bootstrap->getSignalSlotDispatcher(); $dispatcher->connect(Node::class, 'nodePropertyChanged', function (NodeInterface $node, $propertyName, $_, $newValue) use ($bootstrap) { - if ($propertyName !== 'identifier' || empty($newValue) || !$node->getNodeType()->isOfType('Neos.Form.Builder:IdentifierMixin')) { + if ($propertyName !== 'identifier' || empty($newValue) || !$node->getNodeType()->isOfType(self::NODE_TYPE_IDENTIFIER_MIXIN)) { return; } - /** @noinspection PhpUndefinedMethodInspection */ - $flowQuery = (new FlowQuery([$node]))->context(['invisibleContentShown' => true, 'removedContentShown' => true, 'inaccessibleContentShown' => true]); - $possibleIdentifier = $initialIdentifier = $newValue; - $i = 1; - /** @noinspection PhpUndefinedMethodInspection */ - while ($flowQuery - ->closest('[instanceof Neos.Form.Builder:NodeBasedForm]') - // [identifier=".."] matches the Form Element identifier, [_identiier!="..."] excludes the current node - ->find(sprintf('[instanceof Neos.Form.Builder:IdentifierMixin][identifier="%s"][_identifier!="%s"]', $possibleIdentifier, $node->getIdentifier())) - ->count() > 0) { - $possibleIdentifier = $initialIdentifier . '-' . $i++; + $this->setUniqueFormElementIdentifier($node, $newValue); + }); + + $dispatcher->connect(Node::class, 'afterNodeCopy', function (NodeInterface $copiedNode, NodeInterface $targetParentNode) use ($bootstrap) { + try { + $identifier = $copiedNode->getProperty('identifier'); + + if (empty($identifier) || !$copiedNode->getNodeType()->isOfType(self::NODE_TYPE_IDENTIFIER_MIXIN)) { + return; + } + } catch (\Neos\ContentRepository\Exception\NodeException $e) { + return; } - $node->setProperty('identifier', $possibleIdentifier); + + $this->setUniqueFormElementIdentifier($copiedNode, $identifier); }); } + + /** + * @param NodeInterface $node + * @param string $identifier + * @throws \Neos\Eel\Exception + */ + protected function setUniqueFormElementIdentifier(NodeInterface $node, string $identifier): void + { + /** @noinspection PhpUndefinedMethodInspection */ + $flowQuery = (new FlowQuery([$node]))->context([ + 'invisibleContentShown' => true, + 'removedContentShown' => true, + 'inaccessibleContentShown' => true + ]); + $possibleIdentifier = $identifier; + $i = 1; + /** @noinspection PhpUndefinedMethodInspection */ + while ($flowQuery + ->closest('[instanceof Neos.Form.Builder:NodeBasedForm]') + // [identifier=".."] matches the Form Element identifier, [_identiier!="..."] excludes the current node + ->find(sprintf('[instanceof %s][identifier="%s"][_identifier!="%s"]', + self::NODE_TYPE_IDENTIFIER_MIXIN ,$possibleIdentifier, $node->getIdentifier())) + ->count() > 0) { + $possibleIdentifier = $identifier . '-' . $i++; + } + $node->setProperty('identifier', $possibleIdentifier); + } } From db4bcc4eadc275b976c1831396fee28cc82fdd8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kai=20M=C3=B6ller?= Date: Fri, 12 Jun 2020 20:54:12 +0200 Subject: [PATCH 2/4] TASK: use nodeAdded signal instead of afterNodeCopy If you copy and insert a whole section which includes a form element with identifier the exception is still thrown. Therefore I use nodeAdded signal. --- Classes/Package.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Classes/Package.php b/Classes/Package.php index 0209171..5fe5d2d 100644 --- a/Classes/Package.php +++ b/Classes/Package.php @@ -30,18 +30,18 @@ public function boot(Bootstrap $bootstrap) $this->setUniqueFormElementIdentifier($node, $newValue); }); - $dispatcher->connect(Node::class, 'afterNodeCopy', function (NodeInterface $copiedNode, NodeInterface $targetParentNode) use ($bootstrap) { + $dispatcher->connect(Node::class, 'nodeAdded', function (NodeInterface $node) use ($bootstrap) { try { - $identifier = $copiedNode->getProperty('identifier'); + $identifier = $node->getProperty('identifier'); - if (empty($identifier) || !$copiedNode->getNodeType()->isOfType(self::NODE_TYPE_IDENTIFIER_MIXIN)) { + if (empty($identifier) || !$node->getNodeType()->isOfType(self::NODE_TYPE_IDENTIFIER_MIXIN)) { return; } } catch (\Neos\ContentRepository\Exception\NodeException $e) { return; } - $this->setUniqueFormElementIdentifier($copiedNode, $identifier); + $this->setUniqueFormElementIdentifier($node, $identifier); }); } From 81a2ad941cc47a718283ea9a000381d083afe4f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kai=20M=C3=B6ller?= Date: Mon, 15 Jun 2020 09:35:03 +0200 Subject: [PATCH 3/4] TASK: changes after review --- Classes/Package.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Classes/Package.php b/Classes/Package.php index 5fe5d2d..5547779 100644 --- a/Classes/Package.php +++ b/Classes/Package.php @@ -22,7 +22,7 @@ public function boot(Bootstrap $bootstrap) { $dispatcher = $bootstrap->getSignalSlotDispatcher(); - $dispatcher->connect(Node::class, 'nodePropertyChanged', function (NodeInterface $node, $propertyName, $_, $newValue) use ($bootstrap) { + $dispatcher->connect(Node::class, 'nodePropertyChanged', function (NodeInterface $node, $propertyName, $_, $newValue) { if ($propertyName !== 'identifier' || empty($newValue) || !$node->getNodeType()->isOfType(self::NODE_TYPE_IDENTIFIER_MIXIN)) { return; } @@ -30,7 +30,7 @@ public function boot(Bootstrap $bootstrap) $this->setUniqueFormElementIdentifier($node, $newValue); }); - $dispatcher->connect(Node::class, 'nodeAdded', function (NodeInterface $node) use ($bootstrap) { + $dispatcher->connect(Node::class, 'nodeAdded', function (NodeInterface $node) { try { $identifier = $node->getProperty('identifier'); @@ -50,7 +50,7 @@ public function boot(Bootstrap $bootstrap) * @param string $identifier * @throws \Neos\Eel\Exception */ - protected function setUniqueFormElementIdentifier(NodeInterface $node, string $identifier): void + private function setUniqueFormElementIdentifier(NodeInterface $node, string $identifier): void { /** @noinspection PhpUndefinedMethodInspection */ $flowQuery = (new FlowQuery([$node]))->context([ From cbed8725892a3350eb6e42206d9abf1d38f04dbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kai=20M=C3=B6ller?= Date: Mon, 15 Jun 2020 09:38:24 +0200 Subject: [PATCH 4/4] TASK: make constant private --- Classes/Package.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Classes/Package.php b/Classes/Package.php index 5547779..c50300d 100644 --- a/Classes/Package.php +++ b/Classes/Package.php @@ -12,7 +12,7 @@ */ class Package extends BasePackage { - const NODE_TYPE_IDENTIFIER_MIXIN = 'Neos.Form.Builder:IdentifierMixin'; + private const NODE_TYPE_IDENTIFIER_MIXIN = 'Neos.Form.Builder:IdentifierMixin'; /** * @param Bootstrap $bootstrap The current bootstrap