From d0d33d990c9f174ef995e1eff0e292dbeb7b61df Mon Sep 17 00:00:00 2001 From: Yonas Habteab Date: Mon, 13 Jun 2022 09:08:47 +0200 Subject: [PATCH] Ensure to load dashlets of the current pane before creating/updating a dashlet --- .../forms/Dashboard/BaseDashboardForm.php | 2 +- application/forms/Dashboard/DashletForm.php | 57 ++++++++++--------- 2 files changed, 32 insertions(+), 27 deletions(-) diff --git a/application/forms/Dashboard/BaseDashboardForm.php b/application/forms/Dashboard/BaseDashboardForm.php index 47ade1c9c..e914ed1fe 100644 --- a/application/forms/Dashboard/BaseDashboardForm.php +++ b/application/forms/Dashboard/BaseDashboardForm.php @@ -82,7 +82,7 @@ abstract class BaseDashboardForm extends CompatForm */ protected function isUpdating(): bool { - return substr($this->requestUrl->getPath(), 0, 16) === Dashboard::BASE_ROUTE . DIRECTORY_SEPARATOR . 'edit-'; + return substr($this->requestUrl->getPath(), 0, 16) === Dashboard::BASE_ROUTE . '/edit-'; } /** diff --git a/application/forms/Dashboard/DashletForm.php b/application/forms/Dashboard/DashletForm.php index e5943d783..f8a290155 100644 --- a/application/forms/Dashboard/DashletForm.php +++ b/application/forms/Dashboard/DashletForm.php @@ -133,19 +133,19 @@ class DashletForm extends SetupNewDashboardForm $removeButton = $this->createRemoveButton($targetUrl, t('Remove Dashlet')); $formControls = $this->createFormControls(); - $formControls->add([ + $formControls->addHtml( $this->registerSubmitButton(t('Add to Dashboard')), $removeButton, $this->createCancelButton() - ]); + ); $this->addHtml($formControls); } else { $formControls = $this->createFormControls(); - $formControls->add([ + $formControls->addHtml( $this->registerSubmitButton(t('Add to Dashboard')), $this->createCancelButton() - ]); + ); $this->addHtml($formControls); } @@ -172,16 +172,22 @@ class DashletForm extends SetupNewDashboardForm } $currentHome = new DashboardHome($selectedHome); + $currentPane = new Pane($selectedPane); + if ($dashboard->hasEntry($currentHome->getName())) { $currentHome = clone $dashboard->getEntry($currentHome->getName()); - if ($currentHome->getName() !== $dashboard->getActiveHome()->getName()) { - $currentHome->loadDashboardEntries(); - } - } + $activatePane = $currentHome->hasEntry($selectedPane) + && $currentHome->getActivePane()->getName() !== $selectedPane + ? $selectedPane + : null; - $currentPane = new Pane($selectedPane); - if ($currentHome->hasEntry($currentPane->getName())) { - $currentPane = clone $currentHome->getEntry($currentPane->getName()); + if ($currentHome->getName() !== $dashboard->getActiveHome()->getName() || $activatePane) { + $currentHome->loadDashboardEntries($activatePane); + } + + if ($currentHome->hasEntry($currentPane->getName())) { + $currentPane = clone $currentHome->getActivePane(); + } } if (! $this->isUpdating()) { @@ -257,16 +263,22 @@ class DashletForm extends SetupNewDashboardForm } else { $orgHome = $dashboard->getEntry($this->getValue('org_home')); $orgPane = $orgHome->getEntry($this->getValue('org_pane')); - $orgDashlet = $orgPane->getEntry($this->getValue('org_dashlet')); + if ($orgHome->getActivePane()->getName() !== $orgPane->getName()) { + $orgHome->loadDashboardEntries($orgPane->getName()); + $orgPane = $orgHome->getActivePane(); + } + + $orgDashlet = $orgPane->getEntry($this->getValue('org_dashlet')); $currentPane->setHome($currentHome); - /** - * When the user wishes to create a new dashboard pane, we have to explicitly reset the dashboard panes - * of the original home, so that it isn't considered as we want to move the pane even though it isn't - * supposed to when the original home contains a dashboard with the same name - * {@see DashboardHome::manageEntry()} for details - */ - if (! $currentHome->hasEntry($currentPane->getName()) || $currentHome->getName() === $orgHome->getName()) { + + if (! $currentHome->hasEntry($currentPane->getName())) { + /** + * When the user is going to move the Dashlet into a new pane in a different home, it might be possible + * that the original Home contains a Pane with the same name and in {@see DashboardHome::manageEntry()} + * this would be interpreted as if we wanted to move the Pane from the original Home. Therefore, we need + * to explicitly reset all dashboard entries of the org Home here. + */ $orgHome->setEntries([]); } @@ -278,7 +290,6 @@ class DashletForm extends SetupNewDashboardForm ->setTitle($this->getValue('dashlet')) ->setDescription($this->getValue('description')); - if ($orgPane->getName() !== $currentPane->getName() && $currentPane->hasEntry($currentDashlet->getName())) { Notification::error(sprintf( @@ -304,12 +315,6 @@ class DashletForm extends SetupNewDashboardForm return; } - if (empty($paneDiff)) { - // No dashboard diff means the dashlet is still in the same pane, so just - // reset the dashlets of the original pane - $orgPane->setEntries([]); - } - $conn->beginTransaction(); try {