Form: Fix notifications

* Coding style issues
* Notifications were not grouped by type
* Notifications of sub-forms were overwriting existing ones
(cherry picked from commit 147f6be714)

library/Icinga/Web/Form.php

@@ -39,26 +39,19 @@ class Form extends Zend_Form
     const DEFAULT_SUFFIX = '_default';
 
     /**
-     * The type of the notification for the error
+     * Identifier for notifications of type error
      */
-    const NOTIFICATION_ERROR    = 0;
+    const NOTIFICATION_ERROR = 0;
 
     /**
-     * The type of the notification for the warning
+     * Identifier for notifications of type warning
      */
-    const NOTIFICATION_WARNING  = 2;
+    const NOTIFICATION_WARNING = 1;
 
     /**
-     * The type of the notification for the info
+     * Identifier for notifications of type info
      */
-    const NOTIFICATION_INFO     = 4;
+    const NOTIFICATION_INFO = 2;
-
-    /**
-     * The notifications of the form
-     *
-     * @var array
-     */
-    protected $notifications = array();
 
     /**
      * Whether this form has been created
@@ -160,6 +153,13 @@ class Form extends Zend_Form
      */
     protected $descriptions;
 
+    /**
+     * The notifications of this form
+     *
+     * @var array
+     */
+    protected $notifications;
+
     /**
      * Whether the Autosubmit decorator should be applied to this form
      *
@@ -522,6 +522,50 @@ class Form extends Zend_Form
         return $this->descriptions;
     }
 
+    /**
+     * Set the notifications for this form
+     *
+     * @param   array   $notifications
+     *
+     * @return  $this
+     */
+    public function setNotifications(array $notifications)
+    {
+        $this->notifications = $notifications;
+        return $this;
+    }
+
+    /**
+     * Add a notification for this form
+     *
+     * If $notification is an array the second value should be
+     * an array as well containing additional HTML properties.
+     *
+     * @param   string|array    $notification
+     * @param   int             $type
+     *
+     * @return  $this
+     */
+    public function addNotification($notification, $type)
+    {
+        $this->notifications[$type][] = $notification;
+        return $this;
+    }
+
+    /**
+     * Return the notifications of this form
+     *
+     * @return  array
+     */
+    public function getNotifications()
+    {
+        if ($this->notifications === null) {
+            return array();
+        }
+
+        return $this->notifications;
+    }
+
     /**
      * Set whether the Autosubmit decorator should be applied to this form
      *
@@ -1264,55 +1308,48 @@ class Form extends Zend_Form
     }
 
     /**
-     * Return all form notifications
+     * Add a error notification and prevent the form from being successfully validated
      *
-     * @return array
+     * @param   string|array    $message    The notfication's message
-     */
-    public function getNotifications()
-    {
-        return $this->notifications;
-    }
-
-    /**
-     * Add a typed message to the notifications
      *
-     * @param string    $message    The message which would be displayed to the user
+     * @return  $this
-     *
-     * @param int       $type       The type of the message notification
-     */
-    public function addNotification($message, $type = self::NOTIFICATION_ERROR)
-    {
-        $this->notifications[$message] = $type;
-        $this->markAsError();
-    }
-
-    /**
-     * Add a error message to notifications
-     *
-     * @param string $message
      */
     public function error($message)
     {
-        $this->addNotification($message, $type = self::NOTIFICATION_ERROR);
+        $this->addNotification($message, self::NOTIFICATION_ERROR);
+        $this->markAsError();
+        return $this;
     }
 
     /**
-     * Add a warning message to notifications
+     * Add a warning notification and prevent the form from being successfully validated
      *
-     * @param string $message
+     * @param   string|array    $message    The notfication's message
+     *
+     * @return  $this
      */
     public function warning($message)
     {
-        $this->addNotification($message, $type = self::NOTIFICATION_WARNING);
+        $this->addNotification($message, self::NOTIFICATION_WARNING);
+        $this->markAsError();
+        return $this;
     }
 
     /**
-     * Add a info message to notifications
+     * Add a info notification
      *
-     * @param string $message
+     * @param   string|array    $message        The notfication's message
+     * @param   bool            $markAsError    Whether to prevent the form from being successfully validated or not
+     *
+     * @return  $this
      */
-    public function info($message)
+    public function info($message, $markAsError = true)
     {
-        $this->addNotification($message, $type = self::NOTIFICATION_INFO);
+        $this->addNotification($message, self::NOTIFICATION_INFO);
+        if ($markAsError) {
+            $this->markAsError();
+        }
+
+        return $this;
     }
 }

library/Icinga/Web/Form/Decorator/FormNotifications.php

@@ -4,15 +4,16 @@
 namespace Icinga\Web\Form\Decorator;
 
 use Zend_Form_Decorator_Abstract;
-use Icinga\Web\Form as Form;
+use Icinga\Exception\ProgrammingError;
+use Icinga\Web\Form;
 
 /**
- * Decorator to add a list of notifications at the top of a form
+ * Decorator to add a list of notifications at the top or bottom of a form
  */
 class FormNotifications extends Zend_Form_Decorator_Abstract
 {
     /**
-     * Render form descriptions
+     * Render form notifications
      *
      * @param   string      $content    The html rendered so far
      *
@@ -31,16 +32,27 @@ class FormNotifications extends Zend_Form_Decorator_Abstract
         }
 
         $notifications = $this->recurseForm($form);
-
         if (empty($notifications)) {
             return $content;
         }
 
         $html = '<ul class="form-notifications">';
+        foreach (array(Form::NOTIFICATION_ERROR, Form::NOTIFICATION_WARNING, Form::NOTIFICATION_INFO) as $type) {
+            if (isset($notifications[$type])) {
+                $html .= '<li><ul class="' . $this->getNotificationTypeName($type) . '">';
+                foreach ($notifications[$type] as $message) {
+                    if (is_array($message)) {
+                        list($message, $properties) = $message;
+                        $html .= '<li' . $view->propertiesToString($properties) . '>'
+                            . $view->escape($message)
+                            . '</li>';
+                    } else {
+                        $html .= '<li>' . $view->escape($message) . '</li>';
+                    }
+                }
 
-        asort($notifications);
+                $html .= '</ul></li>';
-        foreach ($notifications as $message => $type) {
+            }
-            $html .= '<li class="'.self::getNotificationTypeName($type).'">' . $view->escape($message) . '</li>';
         }
 
         switch ($this->getPlacement()) {
@@ -54,42 +66,44 @@ class FormNotifications extends Zend_Form_Decorator_Abstract
     /**
      * Recurse the given form and return the notifications for it and all of its subforms
      *
-     * @param   Form    $form       The form to recurse
+     * @param   Form    $form   The form to recurse
      *
-     * @return array
+     * @return  array
      */
     protected function recurseForm(Form $form)
     {
         $notifications = $form->getNotifications();
-
         foreach ($form->getSubForms() as $subForm) {
-            $notifications = $notifications + $this->recurseForm($subForm);
+            foreach ($this->recurseForm($subForm) as $type => $messages) {
+                foreach ($messages as $message) {
+                    $notifications[$type][] = $message;
+                }
+            }
         }
 
         return $notifications;
     }
 
     /**
-     * Get the readable type name of the notification
+     * Return the name for the given notification type
      *
-     * @param   $type       Type of the message
+     * @param   int     $type
      *
      * @return  string
+     *
+     * @throws  ProgrammingError    In case the given type is invalid
      */
-    public static function getNotificationTypeName($type)
+    protected function getNotificationTypeName($type)
     {
         switch ($type) {
             case Form::NOTIFICATION_ERROR:
                 return 'error';
-                break;
             case Form::NOTIFICATION_WARNING:
                 return 'warning';
-                break;
             case Form::NOTIFICATION_INFO:
                 return 'info';
-                break;
             default:
-                return 'unknown';
+                throw new ProgrammingError('Invalid notification type "%s" provided', $type);
         }
     }
 }

public/css/icinga/forms.less

@@ -139,7 +139,6 @@ form div.element ul.errors {
   li {
     color: @colorCritical;
     font-weight: bold;
-    line-height: 1.5em;
   }
 }
 
@@ -160,33 +159,33 @@ form ul.form-errors {
   li {
     color: white;
     font-weight: bold;
-    line-height: 1.5em;
   }
 }
 
 form ul.form-notifications {
   .non-list-like-list;
   margin-bottom: 1em;
-  padding: 0em;
+  padding: 0;
 
-  li.info {
+  ul {
-    background: @colorFormNotificationInfo;
+    .non-list-like-list;
-  }
 
-  li.warning {
+    &.info {
-    background: @colorFormNotificationWarning;
+      background-color: @colorFormNotificationInfo;
-  }
+    }
 
-  li.error {
+    &.warning {
-    background: @colorFormNotificationError;
+      background-color: @colorFormNotificationWarning;
+    }
+
+    &.error {
+      background-color: @colorFormNotificationError;
+    }
   }
 
   li {
     color: white;
     font-weight: bold;
-    line-height: 1.5em;
-    padding: 0.5em;
-    margin-bottom: 0.5em;
   }
 }