Fix style and docstrings, exception when removing the last tab

refs #4192

application/controllers/DashboardController.php

@@ -1,7 +1,6 @@
 <?php
 
 use Icinga\Web\ActionController;
-use Icinga\Application\Config;
 use Icinga\Web\Url;
 use Icinga\Application\Icinga;
 use Icinga\Web\Widget\Dashboard;
@@ -40,17 +39,18 @@ class DashboardController extends ActionController
     {
         $pane =  $this->_getParam('pane');
         $dashboard = $this->getDashboard();
-        $dashboard->removeComponent(
+        try {
-            $pane,
+            $dashboard->removeComponent(
-            $this->_getParam('component')
+                $pane,
-        )->store();
+                $this->_getParam('component')
+            )->store();
+            $this->redirectNow(Url::fromPath('dashboard', array('pane' => $pane)));
+        } catch(ConfigurationError $exc ) {
 
-        // When the pane doesn't exist anymore, display the default pane
+            $this->_helper->viewRenderer('show_configuration');
-        if ($dashboard->isEmptyPane($pane)) {
+            $this->view->exceptionMessage = $exc->getMessage();
-            $this->redirectNow(Url::fromPath('dashboard'));
+            $this->view->iniConfigurationString = $dashboard->toIni();
-            return;
         }
-        $this->redirectNow(Url::fromPath('dashboard', array('pane' => $pane)));
     }
 
 

application/forms/Dashboard/AddUrlForm.php

@@ -5,25 +5,34 @@ namespace Icinga\Form\Dashboard;
 use Icinga\Application\Config as IcingaConfig;
 use Icinga\Web\Form;
 use Icinga\Web\Widget\Dashboard;
-use Icinga\Web\Widget\Dashboard\Component;
 use Zend_Form_Element_Text;
 use Zend_Form_Element_Submit;
 use Zend_Form_Element_Hidden;
+use Zend_Form_Element_Select;
 
+/**
+ * Form to add an url a dashboard pane
+ *
+ */
 class AddUrlForm extends Form
 {
 
+    /**
+     * Add a selection box for different panes to the form
+     *
+     * @param Dashboard $dashboard      The dashboard to retrieve the panes from
+     */
     private function addPaneSelectionBox(Dashboard $dashboard)
     {
 
-        $selectPane = new \Zend_Form_Element_Select('pane', array(
+        $selectPane = new Zend_Form_Element_Select('pane', array(
             'label'     => 'Dashboard',
             'required'  => true,
             'style'     => 'display:inline-block',
             'multiOptions' => $dashboard->getPaneKeyTitleArray()
         ));
 
-        $newDashboardBtn = new \Zend_Form_Element_Submit('create_new_pane', array(
+        $newDashboardBtn = new Zend_Form_Element_Submit('create_new_pane', array(
             'label'     => '+',
             'required'  => false,
             'style'     => 'display:inline-block'
@@ -39,6 +48,10 @@ class AddUrlForm extends Form
         $this->enableAutoSubmit(array('create_new_pane'));
     }
 
+    /**
+     *  Add a textfield for creating a new pane to this form
+     *
+     */
     private function addNewPaneTextField()
     {
         $txtCreatePane = new Zend_Form_Element_Text('pane', array(
@@ -47,9 +60,7 @@ class AddUrlForm extends Form
             'style'     => 'display:inline-block'
         ));
 
-        /**
+        // Marks this field as a new pane (and prevents the checkbox being displayed when validation errors occur)
-         * Marks this field as a new pane (and prevents the checkbox being displayed when validation errors occur)
-         */
         $markAsNewPane = new Zend_Form_Element_Hidden('create_new_pane', array(
             'required'  => true,
             'value'     => 1
@@ -72,6 +83,7 @@ class AddUrlForm extends Form
 
     /**
      * Add elements to this form (used by extending classes)
+     *
      */
     protected function create()
     {
@@ -83,9 +95,9 @@ class AddUrlForm extends Form
         ));
         $elems = $dashboard->getPaneKeyTitleArray();
 
-        if (empty($elems) ||                                                // show textfield instead of combobox when no pane is available
+        if (empty($elems) ||    // show textfield instead of combobox when no pane is available
-            ($this->getRequest()->getPost('create_new_pane', '0') &&        // or when a new pane should be created (+ button)
+            ($this->getRequest()->getPost('create_new_pane', '0') &&   // or when a new pane should be created (+ button)
-            ! $this->getRequest()->getPost('use_existing_dashboard', '0'))  // and the user didn't click the 'use existing' button
+            !$this->getRequest()->getPost('use_existing_dashboard', '0'))  // and the user didn't click the 'use existing' button
         ) {
             $this->addNewPaneTextField();
         } else {

config/dashboard/dashboard.ini

@@ -0,0 +1,4 @@
+[gdssdg]
+title = "gdssdg"
+[gdssdg.dsgdsg]
+url = "dsdgs"

doc/dashboard.md

@@ -9,7 +9,7 @@ the objects you're interested in and can add and remove elements.
 
 * The building blocks of dashboards are components - those represent a single URL and display it's content (often in
   a more condensed layout)
-* Different components can be added to a pane and will be shown their. All panes are shown as tabs on top of the dashboard,
+* Different components can be added to a pane and will be shown there. All panes are shown as tabs on top of the dashboard,
   whereas the title is used for the text in the tab
 * The dashboard itself is just the view containing the panes
 

library/Icinga/Util/Dimension.php

@@ -9,7 +9,6 @@
 
 namespace Icinga\Util;
 
-use InvalidArgumentException;
 
 class Dimension
 {

library/Icinga/Web/Widget/Dashboard.php

@@ -3,7 +3,7 @@
 namespace Icinga\Web\Widget;
 
 use Icinga\Application\Icinga;
-use Icinga\Config\Config as IcingaConfig;;
+use Icinga\Config\Config as IcingaConfig;
 use Icinga\Application\Logger;
 use Icinga\Exception\ConfigurationError;
 use Icinga\Web\Widget\Widget;
@@ -11,6 +11,7 @@ use Icinga\Web\Widget\Dashboard\Pane;
 use Icinga\Web\Widget\Dashboard\Component as DashboardComponent;
 
 use Icinga\Web\Url;
+use Zend_View_Abstract;
 
 /**
  * Dashboards display multiple views on a single page
@@ -72,8 +73,8 @@ class Dashboard implements Widget
         $url = Url::fromRequest()->getUrlWithout($this->tabParam);
         if ($this->tabs === null) {
             $this->tabs = new Tabs();
-            foreach ($this->panes as $key => $pane) {
 
+            foreach ($this->panes as $key => $pane) {
                 $this->tabs->add($key, array(
                     'title'     => $pane->getTitle(),
                     'url'       => clone($url),
@@ -105,8 +106,12 @@ class Dashboard implements Widget
             Logger::error('Tried to persist dashboard to %s, but path is not writeable', $file);
             throw new ConfigurationError('Can\'t persist dashboard');
         }
-
+        // make sure empty dashboards don't cause errors
-        if (! @file_put_contents($file, $this->toIni())) {
+        $iniString = trim($this->toIni());
+        if (!$iniString) {
+            $iniString = " ";
+        }
+        if (!@file_put_contents($file, $iniString)) {
             $error = error_get_last();
             if ($error == NULL) {
                 $error = 'Unknown error';
@@ -145,7 +150,6 @@ class Dashboard implements Widget
         $pane = new Pane($title);
         $pane->setTitle($title);
         $this->addPane($pane);
-
     }
 
     /**
@@ -251,15 +255,16 @@ class Dashboard implements Widget
      */
     public function getPane($name)
     {
-        if (!isset($this->panes[$name]))
+        if (!isset($this->panes[$name])) {
             return null;
+        }
         return $this->panes[$name];
     }
 
     /**
      * @see Icinga\Web\Widget::render
      */
-    public function render(\Zend_View_Abstract $view)
+    public function render(Zend_View_Abstract $view)
     {
         if (empty($this->panes)) {
             return '';
@@ -288,7 +293,7 @@ class Dashboard implements Widget
     public function determineActivePane()
     {
         $active = $this->getTabs()->getActiveName();
-        if (! $active) {
+        if (!$active) {
             if ($active = Url::fromRequest()->getParam($this->tabParam)) {
                 if ($this->isEmptyPane($active)) {
                     $active = $this->setDefaultPane();

library/Icinga/Web/Widget/Dashboard/Component.php

@@ -169,21 +169,19 @@ EOD;
     {
         $url = clone($this->url);
         $url->addParams(array('view' => 'compact'));
-        if (isset($_GET['layout'])) {
+
-            $url->addParams(array('layout' => $_GET['layout']));
-        }
         $removeUrl = Url::fromPath(
-            "/dashboard/removecomponent",
+            '/dashboard/removecomponent',
             array(
-                "pane" => $this->pane->getName(),
+                'pane' => $this->pane->getName(),
-                "component" => $this->getTitle()
+                'component' => $this->getTitle()
             )
         );
 
-        $html = str_replace("{URL}", $url->getAbsoluteUrl(), $this->template);
+        $html = str_replace('{URL}', $url->getAbsoluteUrl(), $this->template);
-        $html = str_replace("{REMOVE_URL}", $removeUrl, $html);
+        $html = str_replace('{REMOVE_URL}', $removeUrl, $html);
-        $html = str_replace("{DIMENSION}", $this->getBoxSizeAsCSS(), $html);
+        $html = str_replace('{DIMENSION}', $this->getBoxSizeAsCSS(), $html);
-        $html = str_replace("{TITLE}", $view->escape($this->getTitle()), $html);
+        $html = str_replace('{TITLE}', $view->escape($this->getTitle()), $html);
         return $html;
     }
 
@@ -194,7 +192,7 @@ EOD;
      */
     private function getBoxSizeAsCSS()
     {
-        $style = "";
+        $style = '';
         if ($this->height) {
             $style .= 'height:'.(string) $this->height.';';
         }
@@ -219,16 +217,16 @@ EOD;
         $width = null;
         $url = $config->get('url');
         $parameters = $config->toArray();
-        unset($parameters["url"]); // otherwise there's an url = parameter in the Url
+        unset($parameters['url']); // otherwise there's an url = parameter in the Url
 
-        if (isset($parameters["height"])) {
+        if (isset($parameters['height'])) {
-            $height = Dimension::fromString($parameters["height"]);
+            $height = Dimension::fromString($parameters['height']);
-            unset($parameters["height"]);
+            unset($parameters['height']);
         }
 
-        if (isset($parameters["width"])) {
+        if (isset($parameters['width'])) {
-            $width = Dimension::fromString($parameters["width"]);
+            $width = Dimension::fromString($parameters['width']);
-            unset($parameters["width"]);
+            unset($parameters['width']);
         }
 
         $cmp = new Component($title, Url::fromPath($url, $parameters), $pane);

library/Icinga/Web/Widget/Dashboard/Pane.php

@@ -2,10 +2,10 @@
 
 namespace Icinga\Web\Widget\Dashboard;
 
-use Icinga\Web\Url;
 use Icinga\Exception\ConfigurationError;
 use Icinga\Web\Widget\Widget;
 use Zend_Config;
+use Zend_View_Abstract;
 
 /**
  * A pane, displaying different Dashboard components
@@ -136,10 +136,10 @@ class Pane implements Widget
     /**
      * @see Widget::render
      */
-    public function render(\Zend_View_Abstract $view)
+    public function render(Zend_View_Abstract $view)
     {
         $html = PHP_EOL;
-        foreach ($this->getComponents() as $component) {
+        foreach ($this->components as $component) {
             $html .= PHP_EOL.$component->render($view);
         }
         return $html;

library/Icinga/Web/Widget/Tab.php

@@ -3,6 +3,7 @@
 namespace Icinga\Web\Widget;
 
 use Icinga\Exception\ProgrammingError;
+use Zend_View_Abstract;
 
 /**
  * A single tab, usually used through the tabs widget
@@ -37,15 +38,46 @@ class Tab implements Widget
      */
     private $name = null;
 
+    /**
+     * The title displayed for this tab
+     *
+     * @var string
+     */
     private $title = '';
+
+    /**
+     * The Url this tab points to
+     *
+     * @var string|null
+     */
     private $url = null;
+
+    /**
+     * The parameters for this tab's Url
+     *
+     * @var array
+     */
     private $urlParams = array();
+
+    /**
+     * The icon image to use for this tab or null if none
+     *
+     * @var string|null
+     */
     private $icon = null;
+
+    /**
+     * The icon class to use if $icon is null
+     *
+     * @var string|null
+     */
     private $iconCls = null;
 
 
     /**
-     * @param mixed $icon
+     * Sets an icon image for this tab
+     *
+     * @param string $icon      The url of the image to use
      */
     public function setIcon($icon)
     {
@@ -53,22 +85,15 @@ class Tab implements Widget
     }
 
     /**
-     * @return mixed
+     * Set's an icon class that will be used in an <i> tag if no icon image is set
+     *
+     * @param string $iconCls       The CSS class of the icon to use
      */
-    public function getIcon()
-    {
-        return $this->icon;
-    }
-
     public function setIconCls($iconCls)
     {
         $this->iconCls = $iconCls;
     }
 
-    public function getIconCls()
-    {
-        return $this->iconCls;
-    }
 
     /**
      * @param mixed $name
@@ -95,31 +120,20 @@ class Tab implements Widget
     }
 
     /**
-     * @return mixed
+     * Set the Url this tab points to
-     */
+     *
-    public function getTitle()
+     * @param string $url       The Url to use for this tab
-    {
-        return $this->title;
-    }
-
-    /**
-     * @param mixed $url
      */
     public function setUrl($url)
     {
         $this->url = $url;
     }
 
-    /**
-     * @return mixed
-     */
-    public function getUrl()
-    {
-        return $this->url;
-    }
 
     /**
-     * @param mixed $url
+     * Set the parameters to be set for this tabs Url
+     *
+     * @param array $url        The Url parameters to set
      */
     public function setUrlParams(array $urlParams)
     {
@@ -127,13 +141,12 @@ class Tab implements Widget
     }
 
     /**
-     * @return mixed
+     * Create a new Tab with the given properties
+     *
+     * Allowed properties are all properties for which a setter exists
+     *
+     * @param array $properties     An array of properties
      */
-    public function getUrlParams()
-    {
-        return $this->urlParams;
-    }
-
     public function __construct(array $properties = array())
     {
         foreach ($properties as $name=>$value) {
@@ -163,24 +176,13 @@ class Tab implements Widget
         return $this;
     }
 
-    /**
-     * Whether this tab is currently active
-     *
-     * @return bool
-     */
-    public function isActive()
-    {
-        return $this->active;
-    }
 
     /**
-     * This is where the list item HTML is created
+     * @see Widget::render()
-     *
-     * @return string
      */
-    public function render(\Zend_View_Abstract $view)
+    public function render(Zend_View_Abstract $view)
     {
-        $class = $this->isActive() ? ' class="active"' : '';
+        $class = $this->active ? ' class="active"' : '';
         $caption = $this->title;
         if ($this->icon !== null) {
             $caption = $view->img($this->icon, array(
@@ -201,7 +203,7 @@ class Tab implements Widget
             $tab = $caption;
         }
 
-        return "<li $class>$tab</li>\n";
+        return '<li '.$class.'>'.$tab.'</li>'.PHP_EOL;
     }
 
 }

library/Icinga/Web/Widget/Tabs.php

@@ -4,18 +4,12 @@ namespace Icinga\Web\Widget;
 
 use Icinga\Exception\ProgrammingError;
 use Icinga\Web\Url;
-use Zend_Controller_Action_HelperBroker as ZfActionHelper;
 
 use Countable;
 
 /**
  * Navigation tab widget
  *
- * Useful if you want to create navigation tabs
- *
- * @copyright  Copyright (c) 2013 Icinga-Web Team <info@icinga.org>
- * @author     Icinga-Web Team <info@icinga.org>
- * @license    http://www.gnu.org/copyleft/gpl.html GNU General Public License
  */
 class Tabs implements Countable, Widget
 {
@@ -40,6 +34,12 @@ class Tabs implements Countable, Widget
      */
     private $tab_class = 'nav-tabs';
 
+    /**
+     * Array when special actions (dropdown) are enabled
+     * @TODO: Remove special part from tabs (Bug #4512)
+     *
+     * @var bool|array
+     */
     private $specialActions = false;
 
     /**
@@ -75,6 +75,11 @@ class Tabs implements Countable, Widget
         );
     }
 
+    /**
+     * Return the name of the active tab
+     *
+     * @return string
+     */
     public function getActiveName()
     {
         return $this->active;
@@ -116,7 +121,7 @@ class Tabs implements Countable, Widget
      */
     public function get($name)
     {
-        if (! $this->has($name)) {
+        if (!$this->has($name)) {
             throw new ProgrammingError(
                 sprintf(
                     'There is no such tab: %s',
@@ -175,6 +180,13 @@ class Tabs implements Countable, Widget
         return $this;
     }
 
+    /**
+     * Enable special actions (dropdown with format, basket and dashboard)
+     *
+     * @TODO: Remove special part from tabs (Bug #4512)
+     *
+     * @return $this
+     */
     public function enableSpecialActions()
     {
         $this->specialActions = true;
@@ -182,16 +194,14 @@ class Tabs implements Countable, Widget
     }
 
     /**
-     * This is where the tabs are going to be rendered
+     * @see Widget::render
-     *
-     * @return string
      */
     public function render(\Zend_View_Abstract $view)
     {
         if (empty($this->tabs)) {
             return '';
         }
-        $html = '<ul class="nav ' . $this->tab_class . '">' . "\n";
+        $html = '<ul class="nav ' . $this->tab_class . '">' . PHP_EOL;
 
         foreach ($this->tabs as $tab) {
             $html .= $tab->render($view);
@@ -252,12 +262,23 @@ class Tabs implements Countable, Widget
         return $html;
     }
 
-
+    /**
+     * Return the number of tabs
+     *
+     * @see Countable
+     *
+     * @return int
+     */
     public function count()
     {
         return count($this->tabs);
     }
 
+    /**
+     * Return all tabs contained in this tab panel
+     *
+     * @return array
+     */
     public function getTabs()
     {
         return $this->tabs;

library/Icinga/Web/Widget/Widget.php

@@ -1,18 +1,23 @@
 <?php
-/**
- * Created by JetBrains PhpStorm.
- * User: moja
- * Date: 8/5/13
- * Time: 11:58 AM
- * To change this template use File | Settings | File Templates.
- */
 
 namespace Icinga\Web\Widget;
 
-
 use Icinga\Web\View;
+use Zend_View_Abstract;
 
-interface Widget {
+/**
-
+ * Abstract class for reusable view elements that can be
-    public function render(\Zend_View_Abstract $view);
+ * rendered to a view
+ *
+ */
+interface Widget
+{
+    /**
+     * Renders this widget via the given view and returns the
+     * HTML as a string
+     *
+     * @param \Zend_View_Abstract $view
+     * @return string
+     */
+    public function render(Zend_View_Abstract $view);
 }

modules/monitoring/application/views/scripts/list/hosts-compact.phtml

@@ -1,4 +1,3 @@
-
 <?php
 $hosts = $this->hosts->paginate();
 $viewHelper = $this->getHelper('MonitoringState');
@@ -59,7 +58,7 @@ $trimArea = $this->getHelper('Trim');
             <td class="indicator state" title="<?= $viewHelper->getStateTitle($host, 'host'); ?>">
                 <div class="statetext">
                 <?= $this->qlink(
-                        "<b>".ucfirst($viewHelper->monitoringState($host, 'host'))."</b>".
+                        "<b>".ucfirst($viewHelper->monitoringState($host, 'host')).'</b>'.
                              '<div class="nowrap"> since&nbsp;'.
                             $this->timeSince($host->host_last_state_change),
                         'monitoring/show/history', array(
@@ -82,8 +81,8 @@ $trimArea = $this->getHelper('Trim');
                     </a>
                 <?php endif; ?>
                 <?= $this->qlink(
-                    "<b>".$host->host_name."</b><br/>".
+                    '<b>'.$host->host_name.'</b><br/>'.
-                    "<i>".$host->host_address."</i>",
+                    '<i>'.$host->host_address.'</i>',
                     'monitoring/show/host', array(
                         'host'    => $host->host_name
                     ), array(
@@ -93,11 +92,11 @@ $trimArea = $this->getHelper('Trim');
                 ); ?>
 
 
-                <?php if ($host->host_action_url != ""): ?>
+                <?php if ($host->host_action_url != ''): ?>
                 <a href="<?= $host->host_action_url; ?>">Action</a>
                 <?php endif; ?>
 
-                <?php if ($host->host_notes_url != ""): ?>
+                <?php if ($host->host_notes_url != ''): ?>
                 <a href="<?= $host->host_notes_url; ?>">Notes</a>
                 <?php endif; ?>
             </td>