FieldLoader: fix field handling issues

fixes #13241
fixes #13259

library/Director/Web/Form/DirectorObjectForm.php

@@ -11,6 +11,7 @@ use Icinga\Module\Director\Exception\NestingError;
 use Icinga\Module\Director\IcingaConfig\StateFilterSet;
 use Icinga\Module\Director\IcingaConfig\TypeFilterSet;
 use Icinga\Module\Director\Objects\IcingaObject;
+use Icinga\Module\Director\Util;
 use Zend_Form_Element as ZfElement;
 use Zend_Form_Element_Select as ZfSelect;
 
@@ -38,6 +39,7 @@ abstract class DirectorObjectForm extends QuickForm
 
     protected $preferredObjectType;
 
+    /** @var IcingaObjectFieldLoader */
     protected $fieldLoader;
 
     private $allowsExperimental;
@@ -47,6 +49,16 @@ abstract class DirectorObjectForm extends QuickForm
 
     private $presetImports;
 
+    private $earlyProperties = array(
+        'imports',
+        'check_command',
+        'check_command_id',
+        'command',
+        'command_id',
+        'event_command',
+        'event_command_id',
+    );
+
     public function setPreferredObjectType($type)
     {
         $this->preferredObjectType = $type;
@@ -112,9 +124,23 @@ abstract class DirectorObjectForm extends QuickForm
         }
 
         if ($this->hasBeenSent()) {
-            if ($el = $this->getElement('imports')) {
+            // prefill special properties, required to resolve fields and similar
-                $this->populate($this->getRequest()->getPost());
+            $post = $this->getRequest()->getPost();
-                $object->set('imports', $el->getValue());
+            foreach ($this->earlyProperties as $key) {
+                if ($el = $this->getElement($key)) {
+                    if (array_key_exists($key, $post)) {
+                        $this->populate(array($key => $post[$key]));
+
+                        try {
+                            $old = $object->get($key);
+                            $object->set($key, $el->getValue());
+                            $object->resolveUnresolvedRelatedProperties();
+                        } catch (Exception $e) {
+                            $object->set($key, $old);
+                            $this->addException($e, $key);
+                        }
+                    }
+                }
             }
         }
 
@@ -235,7 +261,7 @@ abstract class DirectorObjectForm extends QuickForm
         $resolve = $this->assertResolvedImports();
         if ($this->hasBeenSent()) {
             foreach ($values as $key => $value) {
-                if ($key === 'imports' || substr($key, 0, 4) === 'var_') {
+                if (in_array($key, $this->earlyProperties) || substr($key, 0, 4) === 'var_') {
                     continue;
                 }
 
@@ -266,7 +292,11 @@ abstract class DirectorObjectForm extends QuickForm
         $this->setDefaults($this->removeEmptyProperties($props));
 
         if ($resolve) {
-            $this->showInheritedProperties($object);
+            try {
+                $this->showInheritedProperties($object);
+            } catch (Exception $e) {
+                $this->addException($e);
+            }
         }
     }
 
@@ -297,11 +327,11 @@ abstract class DirectorObjectForm extends QuickForm
         return $result;
     }
 
-    protected function loadFields($object)
+    protected function prepareFields($object)
     {
         if ($this->assertResolvedImports()) {
-            $loader = $this->fieldLoader($object);
+            $this->fieldLoader = new IcingaObjectFieldLoader($object);
-            $loader->addFieldsToForm($this);
+            $this->fieldLoader->prepareElements($this);
         }
 
         return $this;
@@ -309,14 +339,21 @@ abstract class DirectorObjectForm extends QuickForm
 
     protected function setCustomVarValues($object, & $values)
     {
-        if ($this->assertResolvedImports()) {
+        if ($this->fieldLoader) {
-            $loader = $this->fieldLoader($object);
+            $this->fieldLoader->setValues($values, 'var_');
-            $loader->setValues($values, 'var_');
         }
 
         return $this;
     }
 
+    protected function addFields()
+    {
+        if ($this->fieldLoader) {
+            $this->fieldLoader->addFieldsToForm($this);
+        }
+    }
+
+    // TODO: remove, used in sets I guess
     protected function fieldLoader($object)
     {
         if ($this->fieldLoader === null) {
@@ -602,7 +639,7 @@ abstract class DirectorObjectForm extends QuickForm
         $values = array();
 
         $object = $this->object();
-        $this->loadFields($object);
+        $this->prepareFields($object);
         if ($this->hasBeenSent()) {
 
             if ($this->shouldBeDeleted()) {
@@ -614,9 +651,10 @@ abstract class DirectorObjectForm extends QuickForm
             $values = $this->getValues();
 
             if ($object instanceof IcingaObject) {
-                $this->setCustomVarValues($object, $values);
+                $this->setCustomVarValues($object, $post);
             }
         }
+        $this->addFields();
 
         if ($object instanceof IcingaObject) {
             $this->handleRanges($object, $values);
@@ -913,8 +951,21 @@ abstract class DirectorObjectForm extends QuickForm
 
     protected function addImportsElement($required = null)
     {
+        $required = $required !== null ? $required : !$this->isTemplate();
         $enum = $this->enumAllowedTemplates();
         if (empty($enum)) {
+            if ($required) {
+                if ($this->hasBeenSent()) {
+                    $this->addError($this->translate('No template has been chosen'));
+                } else {
+                    if ($this->hasPermission('director/admin')) {
+                        $html = $this->translate('Please define a related template first');
+                    } else {
+                        $html = $this->translate('No related template has been provided yet');
+                    }
+                    $this->addHtml('<p class="warning">' . $html . '</p>');
+                }
+            }
             return $this;
         }
 
@@ -925,7 +976,7 @@ abstract class DirectorObjectForm extends QuickForm
                 . ' matters when importing properties from multiple templates: last one'
                 . ' wins'
             ),
-            'required'     => ($required !== null ? $required : !$this->isTemplate()),
+            'required'     => $required,
             'multiOptions' => $this->optionallyAddFromEnum($enum),
             'sorted'       => true,
             'value'        => $this->presetImports,
@@ -1293,6 +1344,15 @@ abstract class DirectorObjectForm extends QuickForm
         return $this;
     }
 
+    /**
+     * @param  string $permission
+     * @return bool
+     */
+    public function hasPermission($permission)
+    {
+        return Util::hasPermission($permission);
+    }
+
     protected function allowsExperimental()
     {
         // NO, it is NOT a good idea to use this. You'll break your monitoring

library/Director/Web/Form/IcingaObjectFieldLoader.php

@@ -2,10 +2,11 @@
 
 namespace Icinga\Module\Director\Web\Form;
 
+use Exception;
 use Icinga\Exception\IcingaException;
-use stdClass;
 use Icinga\Module\Director\Objects\IcingaObject;
 use Icinga\Module\Director\Objects\DirectorDatafield;
+use stdClass;
 use Zend_Form_Element as ZfElement;
 
 class IcingaObjectFieldLoader
@@ -102,10 +103,12 @@ class IcingaObjectFieldLoader
                 throw new IcingaException('No such element %s', $key);
             }
 
+            $el->setValue($value);
             $value = $el->getValue();
-            if ($value === '') {
+            if ($value === '' || $value === array()) {
                 $value = null;
             }
+
             $vars->set($varName, $value);
         }
 
@@ -143,6 +146,22 @@ class IcingaObjectFieldLoader
         return $this->elements;
     }
 
+    /**
+     * Prepare the form elements for our fields
+     *
+     * @param QuickForm $form Optional
+     *
+     * @return self
+     */
+    public function prepareElements(QuickForm $form = null)
+    {
+        if ($this->object->supportsFields()) {
+            $this->getElements($form);
+        }
+
+        return $this;
+    }
+
     /**
      * Attach our form fields to the given form
      *
@@ -233,14 +252,20 @@ class IcingaObjectFieldLoader
     /**
      * Create the fields for our object
      *
-     *
+     * @param IcingaObject $object
      * @return DirectorDatafield[]
      */
     protected function prepareObjectFields($object)
     {
         $fields = $this->loadResolvedFieldsForObject($object);
         if ($object->hasRelation('check_command')) {
-            $command = $object->getResolvedRelated('check_command');
+            try {
+                $command = $object->getResolvedRelated('check_command');
+            } catch (Exception $e) {
+                // Ignore failures
+                $command = null;
+            }
+
             if ($command) {
                 $cmdFields = $this->loadResolvedFieldsForObject($command);
                 foreach ($cmdFields as $varname => $field) {
@@ -336,6 +361,7 @@ class IcingaObjectFieldLoader
         foreach ($res as $r) {
             $id = $r->object_id;
             unset($r->object_id);
+
             $r->object = $objects[$id];
             if (! array_key_exists($id, $result)) {
                 $result[$id] = new stdClass;