Introduce and utilize class NoDuplicateChildrenValidator

application/forms/AddNodeForm.php

@@ -5,9 +5,9 @@ namespace Icinga\Module\Businessprocess\Forms;
 use Icinga\Module\Businessprocess\BpNode;
 use Icinga\Module\Businessprocess\BpConfig;
 use Icinga\Module\Businessprocess\Modification\ProcessChanges;
-use Icinga\Module\Businessprocess\Web\Form\Element\Multiselect;
 use Icinga\Module\Businessprocess\Storage\Storage;
 use Icinga\Module\Businessprocess\Web\Form\QuickForm;
+use Icinga\Module\Businessprocess\Web\Form\Validator\NoDuplicateChildrenValidator;
 use Icinga\Module\Monitoring\Backend\MonitoringBackend;
 use Icinga\Web\Session\SessionNamespace;
 
@@ -75,6 +75,20 @@ class AddNodeForm extends QuickForm
             'description' => $this->translate(
                 'This is the unique identifier of this process'
             ),
+            'validators'    => [
+                ['Callback', true, [
+                    'callback'  => function ($value) {
+                        if ($this->hasParentNode()) {
+                            return ! $this->parent->hasChild($value);
+                        }
+
+                        return ! $this->bp->hasRootNode($value);
+                    },
+                    'messages'  => [
+                        'callbackValue' => $this->translate('%value% is already defined in this process')
+                    ]
+                ]]
+            ]
         ));
 
         $this->addElement('text', 'alias', array(
@@ -166,7 +180,7 @@ class AddNodeForm extends QuickForm
 
     protected function selectHost()
     {
-        $this->addElement(new Multiselect('children', [
+        $this->addElement('multiselect','children', [
             'label'        => $this->translate('Hosts'),
             'required'     => true,
             'size'         => 8,
@@ -175,22 +189,8 @@ class AddNodeForm extends QuickForm
             'description'  => $this->translate(
                 'Hosts that should be part of this business process node'
             ),
-            'validators'    => [
+            'validators'    => [[new NoDuplicateChildrenValidator($this, $this->bp, $this->parent), true]]
-                ['Callback', true, [
+        ]);
-                    'callback'  => function ($value) {
-                        if ($this->hasParentNode() && $this->parent->hasChild($value)) {
-                            $el = $this->getElement('children');
-                            $el->addError(sprintf(
-                                $this->translate('%s is already defined in this process'),
-                                $el->getMultiOptions()[$value]
-                            ));
-                        }
-
-                        return true;
-                    }
-                ]]
-            ]
-        ]));
     }
 
     protected function selectService()
@@ -216,7 +216,7 @@ class AddNodeForm extends QuickForm
 
     protected function addServicesElement($host)
     {
-        $this->addElement(new Multiselect('children', [
+        $this->addElement('multiselect','children', [
             'label'        => $this->translate('Services'),
             'required'     => true,
             'size'         => 8,
@@ -225,22 +225,8 @@ class AddNodeForm extends QuickForm
             'description'  => $this->translate(
                 'Services that should be part of this business process node'
             ),
-            'validators'    => [
+            'validators'    => [[new NoDuplicateChildrenValidator($this, $this->bp, $this->parent), true]]
-                ['Callback', true, [
+        ]);
-                    'callback'  => function ($value) {
-                        if ($this->hasParentNode() && $this->parent->hasChild($value)) {
-                            $el = $this->getElement('children');
-                            $el->addError(sprintf(
-                                $this->translate('%s is already defined in this process'),
-                                $el->getMultiOptions()[$value]
-                            ));
-                        }
-
-                        return true;
-                    }
-                ]]
-            ]
-        ]));
     }
 
     protected function addFileElement()
@@ -265,7 +251,7 @@ class AddNodeForm extends QuickForm
         }
 
         if (($file = $this->getSentValue('file')) || !$this->hasParentNode()) {
-            $this->addElement(new Multiselect('children', [
+            $this->addElement('multiselect','children', [
                 'label'        => $this->translate('Process nodes'),
                 'required'     => true,
                 'size'         => 8,
@@ -274,22 +260,8 @@ class AddNodeForm extends QuickForm
                 'description'  => $this->translate(
                     'Other processes that should be part of this business process node'
                 ),
-                'validators'    => [
+                'validators'    => [[new NoDuplicateChildrenValidator($this, $this->bp, $this->parent), true]]
-                    ['Callback', true, [
+            ]);
-                        'callback'  => function ($value) {
-                            if ($this->hasParentNode() && $this->parent->hasChild($value)) {
-                                $el = $this->getElement('children');
-                                $el->addError(sprintf(
-                                    $this->translate('%s is already defined in this process'),
-                                    $el->getMultiOptions()[$value]
-                                ));
-                            }
-
-                            return true;
-                        }
-                    ]]
-                ]
-            ]));
         } else {
             $this->setSubmitLabel($this->translate('Next'));
         }

application/forms/BpConfigForm.php

@@ -143,6 +143,7 @@ class BpConfigForm extends BpConfigBaseForm
                     $this->translate('A process named "%s" already exists'),
                     $name
                 ));
+
                 return;
             }
 

application/forms/EditNodeForm.php

@@ -7,6 +7,7 @@ use Icinga\Module\Businessprocess\BpConfig;
 use Icinga\Module\Businessprocess\Modification\ProcessChanges;
 use Icinga\Module\Businessprocess\Node;
 use Icinga\Module\Businessprocess\Web\Form\QuickForm;
+use Icinga\Module\Businessprocess\Web\Form\Validator\NoDuplicateChildrenValidator;
 use Icinga\Module\Monitoring\Backend\MonitoringBackend;
 use Icinga\Web\Session\SessionNamespace;
 
@@ -175,21 +176,7 @@ class EditNodeForm extends QuickForm
             'multiOptions'  => $this->enumHostList(),
             'label'         => $this->translate('Host'),
             'description'   => $this->translate('The host for this business process node'),
-            'validators'    => [
+            'validators'    => [[new NoDuplicateChildrenValidator($this, $this->bp, $this->parent), true]]
-                ['Callback', true, [
-                    'callback'  => function ($value) {
-                        if ($this->hasParentNode() && $this->parent->hasChild($value)) {
-                            $el = $this->getElement('children');
-                            $el->addError(sprintf(
-                                $this->translate('%s is already defined in this process'),
-                                $el->getMultiOptions()[$value]
-                            ));
-                        }
-
-                        return true;
-                    }
-                ]]
-            ]
         ));
     }
 
@@ -227,21 +214,7 @@ class EditNodeForm extends QuickForm
             'multiOptions'  => $this->enumServiceList($host),
             'label'         => $this->translate('Service'),
             'description'   => $this->translate('The service for this business process node'),
-            'validators'    => [
+            'validators'    => [[new NoDuplicateChildrenValidator($this, $this->bp, $this->parent), true]]
-                ['Callback', true, [
-                    'callback'  => function ($value) {
-                        if ($this->hasParentNode() && $this->parent->hasChild($value)) {
-                            $el = $this->getElement('children');
-                            $el->addError(sprintf(
-                                $this->translate('%s is already defined in this process'),
-                                $el->getMultiOptions()[$value]
-                            ));
-                        }
-
-                        return true;
-                    }
-                ]]
-            ]
         ));
     }
 

library/Businessprocess/Web/Form/Element/Multiselect.php

@@ -1,15 +0,0 @@
-<?php
-
-namespace Icinga\Module\Businessprocess\Web\Form\Element;
-
-use Zend_Form_Element_Multiselect;
-
-class Multiselect extends Zend_Form_Element_Multiselect
-{
-    protected function _getErrorMessages()
-    {
-        // The base implementation is prone to message duplication in case of custom error messages.
-        // Since its actual behavior is not required it's entirely bypassed.
-        return $this->getErrorMessages();
-    }
-}

library/Businessprocess/Web/Form/Validator/NoDuplicateChildrenValidator.php

@@ -0,0 +1,54 @@
+<?php
+
+namespace Icinga\Module\Businessprocess\Web\Form\Validator;
+
+use Icinga\Module\Businessprocess\BpConfig;
+use Icinga\Module\Businessprocess\BpNode;
+use Icinga\Module\Businessprocess\Web\Form\QuickForm;
+use Zend_Validate_Abstract;
+
+class NoDuplicateChildrenValidator extends Zend_Validate_Abstract
+{
+    const CHILD_FOUND = 'childFound';
+
+    /** @var QuickForm */
+    protected $form;
+
+    /** @var BpConfig */
+    protected $bp;
+
+    /** @var BpNode */
+    protected $parent;
+
+    /** @var string */
+    protected $label;
+
+    public function __construct(QuickForm $form, BpConfig $bp, BpNode $parent = null)
+    {
+        $this->form = $form;
+        $this->bp = $bp;
+        $this->parent = $parent;
+
+        $this->_messageVariables['label'] = 'label';
+        $this->_messageTemplates = [
+            self::CHILD_FOUND => mt('businessprocess', '%label% is already defined in this process')
+        ];
+    }
+
+    public function isValid($value)
+    {
+        if ($this->parent === null) {
+            $found = $this->bp->hasRootNode($value);
+        } else {
+            $found = $this->parent->hasChild($value);
+        }
+
+        if (! $found) {
+            return true;
+        }
+
+        $this->label = $this->form->getElement('children')->getMultiOptions()[$value];
+        $this->_error(self::CHILD_FOUND);
+        return false;
+    }
+}