Fix removal of a system home doesn't work properly

Icinga · Jun 1, 2021 · a58e449 · a58e449
1 parent 20a97c8
commit a58e449
Show file tree

Hide file tree

Showing 4 changed files with 36 additions and 24 deletions.
diff --git a/application/controllers/DashboardsController.php b/application/controllers/DashboardsController.php
@@ -39,7 +39,7 @@ public function indexAction()
 
         $activeHome = $this->dashboard->getActiveHome();
 
-        if (! $this->dashboard || ! $activeHome || ! $activeHome->hasPanes()) {
+        if (! $activeHome || ! $activeHome->hasPanes()) {
             $this->getTabs()->add('dashboard', [
                 'active'    => true,
                 'title'     => $this->translate('Dashboard'),

diff --git a/application/forms/Dashboard/DashletForm.php b/application/forms/Dashboard/DashletForm.php
@@ -409,9 +409,6 @@ public function updateDashlet()
             return;
         }
 
-        // Begin DB transaction
-        $db->beginTransaction();
-
         $createNewHome = true;
         if ($this->dashboard->hasHome($homeName)) {
             $home = $this->dashboard->getHome($homeName);
@@ -420,8 +417,7 @@ public function updateDashlet()
             $createNewHome = false;
 
             if ($orgHome->getName() !== $home->getName()) {
-                $this->dashboard->loadDashboards($homeName);
-                $homeId = $this->dashboard->getActiveHome()->getIdentifier();
+                $this->dashboard->loadDashboards($home->getName());
             }
         }
 
@@ -448,6 +444,9 @@ public function updateDashlet()
             $dashletDisabled = false;
         }
 
+        // Begin DB transaction
+        $db->beginTransaction();
+
         if (! $orgDashlet->isUserWidget()) { // System dashlets
             // Since system dashlets can be edited by multiple users, we need to change
             // the original id here so we don't encounter a duplicate key error

diff --git a/application/forms/Dashboard/HomeAndPaneForm.php b/application/forms/Dashboard/HomeAndPaneForm.php
@@ -217,7 +217,7 @@ public function onSuccess()
 
             if ($this->getPopulatedValue('btn_update')) {
                 $newHome = $this->getPopulatedValue('home', $orgHome->getName());
-                $orgHomeId = $orgHome->getIdentifier();
+                $homeId = $orgHome->getIdentifier();
 
                 if ($pane->getOwner() === DashboardHome::DEFAULT_IW2_USER && $orgHome->getName() !== $newHome) {
                     Notification::error(sprintf(
@@ -228,26 +228,21 @@ public function onSuccess()
                     return;
                 }
 
-                // Begin DB transaction
-                $db->beginTransaction();
+                $createNewHome = true;
+                if ($this->dashboard->hasHome($newHome)) {
+                    $home = $this->dashboard->getHome($newHome);
+                    $homeId = $home->getIdentifier();
 
-                $homeId = $orgHomeId;
-                if (! $this->dashboard->hasHome($newHome) || /** prevents from failing foreign key constraint */
-                    ($this->dashboard->getHome($newHome)->getOwner() === DashboardHome::DEFAULT_IW2_USER &&
-                    $this->dashboard->getHome($newHome)->getName() !== DashboardHome::DEFAULT_HOME)) {
-                    $label = $newHome;
+                    $createNewHome = false;
 
-                    if ($this->dashboard->hasHome($newHome)) {
-                        $label = $this->dashboard->getHome($newHome)->getLabel();
+                    if ($home->getName() !== $orgHome->getName()) {
+                        $this->dashboard->loadDashboards($home->getName());
                     }
-
-                    $db->insert('dashboard_home', ['name' => $newHome, 'label' => $label, 'owner' => $username]);
-
-                    $homeId = $db->lastInsertId();
-                } elseif ($orgHome->getName() !== $newHome) {
-                    $homeId = $this->dashboard->getHome($newHome)->getIdentifier();
                 }
 
+                // Begin the DB transaction
+                $db->beginTransaction();
+
                 $paneUpdated = false;
                 if ($this->getPopulatedValue('enable_pane') === 'y') {
                     $paneUpdated = true;
@@ -268,7 +263,7 @@ public function onSuccess()
                             'dashboard_id = ?'  => $pane->getPaneId(),
                         ]);
 
-                    } elseif ($pane->getOwner() === DashboardHome::DEFAULT_IW2_USER) { // System panes
+                    } elseif (! $pane->isUserWidget()) { // System panes
                         $paneId = DashboardHome::getSHA1($username . $newHome . $pane->getName());
 
                         $db->insert('dashboard_override', [
@@ -278,6 +273,24 @@ public function onSuccess()
                             'label'         => $this->getValue('title')
                         ]);
                     } else { // Custom panes
+                        if ($createNewHome || ($newHome !== DashboardHome::DEFAULT_HOME &&
+                            $this->dashboard->getActiveHome()->getOwner() === DashboardHome::DEFAULT_IW2_USER)) {
+                            $label = $newHome;
+
+                            if (! $createNewHome) {
+                                $activeHome = $this->dashboard->getActiveHome();
+                                $label = $activeHome->getLabel();
+                            }
+
+                            $db->insert('dashboard_home', [
+                                'name'  => $newHome,
+                                'label' => $label,
+                                'owner' => $username
+                            ]);
+
+                            $homeId = $db->lastInsertId();
+                        }
+
                         $db->update('dashboard', [
                             'home_id'   => $homeId,
                             'name'      => $this->getValue('name'),

diff --git a/test/php/library/Icinga/Web/Widget/DashboardTest.php b/test/php/library/Icinga/Web/Widget/DashboardTest.php
@@ -28,7 +28,7 @@ class DashboardWithPredefinableActiveName extends Dashboard
 {
     public $activeName = '';
 
-    public function getTabs($defaultPane = false)
+    public function getTabs()
     {
         return Mockery::mock('Icinga\Web\Widget\Tabs')
             ->shouldReceive('getActiveName')->andReturn($this->activeName)