Forms: better error handling

fixes #12926

library/Director/Objects/IcingaTemplateResolver.php

@@ -2,6 +2,7 @@
 
 namespace Icinga\Module\Director\Objects;
 
+use Icinga\Exception\NotFoundError;
 use Icinga\Module\Director\Db;
 use Icinga\Module\Director\Exception\NestingError;
 
@@ -247,6 +248,10 @@ class IcingaTemplateResolver
 
     protected function getIdForName($name)
     {
+        if (! array_key_exists($name, self::$nameToId[$this->type])) {
+            throw new NotFoundError('There is no such import: "%s"', $name);
+        }
+
         return self::$nameToId[$this->type][$name];
     }
 

library/Director/Web/Form/DirectorObjectForm.php

@@ -76,11 +76,12 @@ abstract class DirectorObjectForm extends QuickForm
         $object = $this->object;
 
         if (! $object instanceof IcingaObject) {
-            return $this;
+            return $this->resolvedImports = false;
         }
         if (! $object->supportsImports()) {
-            return $this;
+            return $this->resolvedImports = false;
         }
+
         if ($this->hasBeenSent()) {
             if ($el = $this->getElement('imports')) {
                 $this->populate($this->getRequest()->getPost());
@@ -93,6 +94,9 @@ abstract class DirectorObjectForm extends QuickForm
         } catch (NestingError $e) {
             $this->addUniqueErrorMessage($e->getMessage());
             return $this->resolvedImports = false;
+        } catch (Exception $e) {
+            $this->addException($e, 'imports');
+            return $this->resolvedImports = false;
         }
 
         return $this->resolvedImports = true;
@@ -194,8 +198,13 @@ abstract class DirectorObjectForm extends QuickForm
 
     protected function handleProperties($object, & $values)
     {
+        $resolve = $this->assertResolvedImports();
         if ($this->hasBeenSent()) {
             foreach ($values as $key => $value) {
+                if ($key === 'imports') {
+                    continue;
+                }
+
                 try {
                     $object->set($key, $value);
                     if ($object instanceof IcingaObject) {
@@ -203,39 +212,18 @@ abstract class DirectorObjectForm extends QuickForm
                     }
 
                 } catch (Exception $e) {
-
+                    $this->addException($e, $key);
-                    $file = preg_split('/[\/\\\]/', $e->getFile(), -1, PREG_SPLIT_NO_EMPTY);
-                    $file = array_pop($file);
-                    $msg = sprintf(
-                        '%s (%s:%d)',
-                        $e->getMessage(),
-                        $file,
-                        $e->getLine()
-                    );
-
-                    if ($el = $this->getElement($key)) {
-                        // TODO: to be preferred $el->addError($e->getMessage());
-                        $this->addError($msg);
-                    } else {
-                        $this->addError($msg);
-                    }
                 }
             }
         }
 
         if ($object instanceof IcingaObject) {
-            try {
+            $props = (array) $object->toPlainObject(
-                $props = (array) $object->toPlainObject(
+                false,
-                    false,
+                false,
-                    false,
+                null,
-                    null,
+                false // Do not resolve IDs
-                    false // Do not resolve IDs
+            );
-                );
-            } catch (NestingError $e) {
-                $this->addUniqueErrorMessage($e->getMessage());
-                $props = $object->getProperties();
-            }
-
         } else {
             $props = $object->getProperties();
             unset($props['vars']);
@@ -253,23 +241,19 @@ abstract class DirectorObjectForm extends QuickForm
             return $this;
         }
 
-        $inherited = (object) array();
+        if ($resolve) {
-        $origins   = (object) array();
+            $inherited = $object->getInheritedProperties();
-
+            $origins   = $object->getOriginsProperties();
-        if ($object->supportsImports()) {
+        } else {
-            try {
+            $inherited = (object) array();
-                $inherited = $object->getInheritedProperties();
+            $origins   = (object) array();
-                $origins   = $object->getOriginsProperties();
-            } catch (NestingError $e) {
-                $this->addUniqueErrorMessage($e->getMessage());
-            }
         }
 
         foreach ($props as $k => $v) {
             $this->setElementValue($k, $v);
             if ($k !== 'object_name' && property_exists($inherited, $k)) {
                 $el = $this->getElement($k);
-                if ($el) {
+                if ($el && $resolve) {
                     $this->setInheritedValue($el, $inherited->$k, $origins->$k);
                 }
             }

library/Director/Web/Form/Element/ExtensibleSet.php

@@ -30,6 +30,17 @@ class ExtensibleSet extends FormElement
         return $value;
     }
 
+    /**
+     * We do not want one message per entry
+     *
+     * @codingStandardsIgnoreStart
+     */
+    protected function _getErrorMessages()
+    {
+        return $this->_errorMessages;
+        // @codingStandardsIgnoreEnd
+    }
+
     /**
      * @codingStandardsIgnoreStart
      */

library/Director/Web/Form/IcingaObjectFieldLoader.php

@@ -2,7 +2,6 @@
 
 namespace Icinga\Module\Director\Web\Form;
 
-use Icinga\Module\Director\Exception\NestingError;
 use Icinga\Module\Director\Objects\IcingaObject;
 use Icinga\Module\Director\Objects\IcingaServiceSet;
 use Icinga\Module\Director\Objects\DirectorDatafield;

library/Director/Web/Form/QuickForm.php

@@ -316,7 +316,7 @@ abstract class QuickForm extends QuickBaseForm
                     try {
                         $this->onSuccess();
                     } catch (Exception $e) {
-                        $this->addError($e->getMessage());
+                        $this->addException($e);
                         $this->onFailure();
                     }
                 } else {
@@ -332,6 +332,24 @@ abstract class QuickForm extends QuickBaseForm
         return $this;
     }
 
+    public function addException(Exception $e, $elementName = null, $withDetails = true)
+    {
+        $file = preg_split('/[\/\\\]/', $e->getFile(), -1, PREG_SPLIT_NO_EMPTY);
+        $file = array_pop($file);
+        $msg = sprintf(
+            '%s (%s:%d)',
+            $e->getMessage(),
+            $file,
+            $e->getLine()
+        );
+
+        if ($el = $this->getElement($elementName)) {
+            $el->addError($msg);
+        } else {
+            $this->addError($msg);
+        }
+    }
+
     public function onSuccess()
     {
         $this->redirectOnSuccess();

public/css/module.less

@@ -714,7 +714,6 @@ form textarea {
 form dd ul.errors {
     list-style-type: none;
     padding-left: 0.3em;
-    font-size: 0.857em;
 
     li {
         color: @colorCritical;