From 930898fd2f7ac4aedbdd0fb5f2b0fe2fc1ee1aa3 Mon Sep 17 00:00:00 2001 From: Johannes Meyer Date: Wed, 20 Apr 2022 12:56:35 +0200 Subject: [PATCH] Decouple menu and dashboard code --- .../controllers/DashboardsController.php | 57 ++++++++++++++-- application/forms/Dashboard/DashletForm.php | 2 +- .../Web/Dashboard/Common/DashboardManager.php | 68 +++++++++++-------- .../Icinga/Web/Dashboard/DashboardHome.php | 20 +++--- library/Icinga/Web/Menu.php | 48 ++++--------- .../Web/Navigation/DashboardHomeItem.php | 42 ------------ 6 files changed, 116 insertions(+), 121 deletions(-) delete mode 100644 library/Icinga/Web/Navigation/DashboardHomeItem.php diff --git a/application/controllers/DashboardsController.php b/application/controllers/DashboardsController.php index 865677a52..e9fedc956 100644 --- a/application/controllers/DashboardsController.php +++ b/application/controllers/DashboardsController.php @@ -35,11 +35,12 @@ class DashboardsController extends CompatController $this->dashboard = new Dashboard(); $this->dashboard->setUser($this->Auth()->getUser()); $this->dashboard->setTabs($this->getTabs()); - $this->dashboard->load(); } public function indexAction() { + $this->dashboard->load(DashboardHome::DEFAULT_HOME); + $this->createTabs(); $activeHome = $this->dashboard->getActiveHome(); @@ -73,10 +74,7 @@ class DashboardsController extends CompatController */ public function homeAction() { - $home = $this->params->getRequired('home'); - if (! $this->dashboard->hasEntry($home)) { - $this->httpNotFound(sprintf(t('Home "%s" not found'), $home)); - } + $this->dashboard->load($this->params->getRequired('home')); $activeHome = $this->dashboard->getActiveHome(); if (! $activeHome->getEntries()) { @@ -101,6 +99,7 @@ class DashboardsController extends CompatController public function newHomeAction() { $this->setTitle(t('Add new Dashboard Home')); + $this->dashboard->load(); $paneForm = (new NewHomePaneForm($this->dashboard)) ->on(NewHomePaneForm::ON_SUCCESS, function () { @@ -114,12 +113,17 @@ class DashboardsController extends CompatController public function editHomeAction() { $this->setTitle(t('Update Home')); + $this->dashboard->load(); $home = $this->params->getRequired('home'); if (! $this->dashboard->hasEntry($home)) { $this->httpNotFound(sprintf(t('Home "%s" not found'), $home)); } + // TODO: Shouldn't be necessary. load() should get the name already and + // the form should otherwise ensure it has the required data + $this->dashboard->activateHome($this->dashboard->getEntry($home)); + $homeForm = (new HomePaneForm($this->dashboard)) ->on(HomePaneForm::ON_SUCCESS, function () { $this->redirectNow(Url::fromPath(Dashboard::BASE_ROUTE . '/settings')); @@ -133,12 +137,17 @@ class DashboardsController extends CompatController public function removeHomeAction() { $this->setTitle(t('Remove Home')); + $this->dashboard->load(); $home = $this->params->getRequired('home'); if (! $this->dashboard->hasEntry($home)) { $this->httpNotFound(sprintf(t('Home "%s" not found'), $home)); } + // TODO: Shouldn't be necessary. load() should get the name already and + // the form should otherwise ensure it has the required data + $this->dashboard->activateHome($this->dashboard->getEntry($home)); + $homeForm = (new RemoveHomePaneForm($this->dashboard)) ->on(RemoveHomePaneForm::ON_SUCCESS, function () { $this->redirectNow(Url::fromPath(Dashboard::BASE_ROUTE . '/settings')); @@ -151,12 +160,17 @@ class DashboardsController extends CompatController public function newPaneAction() { $this->setTitle(t('Add new Dashboard')); + $this->dashboard->load(); $home = $this->params->getRequired('home'); if (! $this->dashboard->hasEntry($home)) { $this->httpNotFound(sprintf(t('Home "%s" not found'), $home)); } + // TODO: Shouldn't be necessary. load() should get the name already and + // the form should otherwise ensure it has the required data + $this->dashboard->activateHome($this->dashboard->getEntry($home)); + $paneForm = (new NewHomePaneForm($this->dashboard)) ->on(NewHomePaneForm::ON_SUCCESS, function () { $this->redirectNow(Url::fromPath(Dashboard::BASE_ROUTE . '/settings')); @@ -169,6 +183,7 @@ class DashboardsController extends CompatController public function editPaneAction() { $this->setTitle(t('Update Pane')); + $this->dashboard->load(); $pane = $this->params->getRequired('pane'); $home = $this->params->getRequired('home'); @@ -177,6 +192,10 @@ class DashboardsController extends CompatController $this->httpNotFound(sprintf(t('Home "%s" not found'), $home)); } + // TODO: Shouldn't be necessary. load() should get the name already and + // the form should otherwise ensure it has the required data + $this->dashboard->activateHome($this->dashboard->getEntry($home)); + if (! $this->dashboard->getActiveHome()->hasEntry($pane)) { $this->httpNotFound(sprintf(t('Pane "%s" not found'), $pane)); } @@ -195,6 +214,7 @@ class DashboardsController extends CompatController public function removePaneAction() { $this->setTitle(t('Remove Pane')); + $this->dashboard->load(); $home = $this->params->getRequired('home'); $paneParam = $this->params->getRequired('pane'); @@ -203,6 +223,10 @@ class DashboardsController extends CompatController $this->httpNotFound(sprintf(t('Home "%s" not found'), $home)); } + // TODO: Shouldn't be necessary. load() should get the name already and + // the form should otherwise ensure it has the required data + $this->dashboard->activateHome($this->dashboard->getEntry($home)); + if (! $this->dashboard->getActiveHome()->hasEntry($paneParam)) { $this->httpNotFound(sprintf(t('Pane "%s" not found'), $paneParam)); } @@ -224,6 +248,13 @@ class DashboardsController extends CompatController $this->setTitle(t('Select Dashlets')); } + $this->dashboard->load(); + + $home = $this->params->getRequired('home'); + // TODO: Shouldn't be necessary. load() should get the name already and + // the form should otherwise ensure it has the required data + $this->dashboard->activateHome($this->dashboard->getEntry($home)); + $dashletForm = new DashletForm($this->dashboard); $dashletForm->populate($this->getRequest()->getPost()); $dashletForm->on(DashletForm::ON_SUCCESS, function () { @@ -236,6 +267,7 @@ class DashboardsController extends CompatController public function editDashletAction() { $this->setTitle(t('Edit Dashlet')); + $this->dashboard->load(); $pane = $this->validateDashletParams(); $dashlet = $pane->getEntry($this->getParam('dashlet')); @@ -255,6 +287,8 @@ class DashboardsController extends CompatController public function removeDashletAction() { $this->setTitle(t('Remove Dashlet')); + $this->dashboard->load(); + $this->validateDashletParams(); $removeForm = (new RemoveDashletForm($this->dashboard)) @@ -281,6 +315,8 @@ class DashboardsController extends CompatController $originals = $dashboards['originals']; unset($dashboards['originals']); + $this->dashboard->load(); + $orgHome = null; $orgPane = null; if ($originals && isset($originals['originalHome'])) { @@ -402,6 +438,8 @@ class DashboardsController extends CompatController $this->setTitle(t('Add Dashlet')); } + $this->dashboard->load(); + $setupForm = new SetupNewDashboardForm($this->dashboard); $setupForm->on(SetupNewDashboardForm::ON_SUCCESS, function () use ($setupForm) { $this->redirectNow($setupForm->getRedirectUrl()); @@ -412,7 +450,9 @@ class DashboardsController extends CompatController public function settingsAction() { + $this->dashboard->load(); $this->createTabs(); + $activeHome = $this->dashboard->getActiveHome(); // We can't grant access the user to the dashboard manager if there aren't any dashboards to manage if (! $activeHome || (! $activeHome->hasEntries() && count($this->dashboard->getEntries()) === 1)) { @@ -445,8 +485,7 @@ class DashboardsController extends CompatController { $tabs = $this->dashboard->getTabs(); $activeHome = $this->dashboard->getActiveHome(); - if (($activeHome && $activeHome->hasEntries()) || - ($activeHome && ! $activeHome->isDisabled() && count($this->dashboard->getEntries()) > 1)) { + if ($activeHome && ($activeHome->getName() !== DashboardHome::DEFAULT_HOME || $activeHome->hasEntries())) { $tabs->extend(new DashboardSettings()); } @@ -463,6 +502,10 @@ class DashboardsController extends CompatController $this->httpNotFound(sprintf(t('Home "%s" not found'), $home)); } + // TODO: Shouldn't be necessary. load() should get the name already and + // the form should otherwise ensure it has the required data + $this->dashboard->activateHome($this->dashboard->getEntry($home)); + if (! $this->dashboard->getActiveHome()->hasEntry($pane)) { $this->httpNotFound(sprintf(t('Pane "%s" not found'), $pane)); } diff --git a/application/forms/Dashboard/DashletForm.php b/application/forms/Dashboard/DashletForm.php index 6c4cc2f8e..a57310a2c 100644 --- a/application/forms/Dashboard/DashletForm.php +++ b/application/forms/Dashboard/DashletForm.php @@ -56,7 +56,7 @@ class DashletForm extends SetupNewDashboardForm $panes = $firstHome->getEntryKeyTitleArr(); } } else { - $panes = $activeHome->getEntryKeyTitleArr(); + $panes = $activeHome->loadDashboardEntries()->getEntryKeyTitleArr(); } } elseif ($this->dashboard->hasEntry($populatedHome)) { $this->dashboard->loadDashboardEntries($populatedHome); diff --git a/library/Icinga/Web/Dashboard/Common/DashboardManager.php b/library/Icinga/Web/Dashboard/Common/DashboardManager.php index 829eaf1b4..ad00ba416 100644 --- a/library/Icinga/Web/Dashboard/Common/DashboardManager.php +++ b/library/Icinga/Web/Dashboard/Common/DashboardManager.php @@ -5,9 +5,9 @@ namespace Icinga\Web\Dashboard\Common; use Icinga\Application\Icinga; -use Icinga\Application\Modules\DashletContainer; use Icinga\Authentication\Auth; use Icinga\Common\Database; +use Icinga\Exception\Http\HttpNotFoundException; use Icinga\Exception\ProgrammingError; use Icinga\Model; use Icinga\User; @@ -15,13 +15,9 @@ use Icinga\Web\Dashboard\Dashboard; use Icinga\Web\Dashboard\DashboardHome; use Icinga\Web\Dashboard\Dashlet; use Icinga\Web\Dashboard\Pane; -use Icinga\Web\Menu; -use Icinga\Web\Navigation\DashboardPane; -use ipl\Orm\Query; use ipl\Sql\Connection; use ipl\Sql\Expression; use ipl\Stdlib\Filter; -use ipl\Web\Url; trait DashboardManager { @@ -40,12 +36,46 @@ trait DashboardManager */ private static $defaultPanes = []; - public function load() + /** + * Load the given or all homes (null) + * + * @param ?string $name + * + * @return void + */ + public function load(string $name = null) { - $this->setEntries((new Menu())->loadHomes()); - $this->loadDashboardEntries(); + $query = Model\Home::on(self::getConn()); + $query->filter(Filter::equal('username', $this::getUser()->getUsername())); + + if ($name !== null) { + $query->filter(Filter::equal('name', $name)); + + /** @var Model\Home $row */ + if (($row = $query->first()) === null) { + if ($name === DashboardHome::DEFAULT_HOME) { + $home = $this->initGetDefaultHome(); + } else { + throw new HttpNotFoundException(t('Home "%s" not found'), $name); + } + } else { + $home = DashboardHome::create($row); + $this->addEntry($home); + } + + $this->activateHome($home); + $home->loadDashboardEntries(); + } else { + foreach ($query as $row) { + $this->addEntry(DashboardHome::create($row)); + } + + if (($firstHome = $this->rewindEntries())) { + $this->activateHome($firstHome); + $firstHome->loadDashboardEntries(); + } + } - $this->initGetDefaultHome(); self::deployModuleDashlets(); } @@ -81,25 +111,7 @@ trait DashboardManager public function loadDashboardEntries(string $name = '') { - if ($name && $this->hasEntry($name)) { - $home = $this->getEntry($name); - } else { - $requestRoute = Url::fromRequest(); - if ($requestRoute->getPath() === Dashboard::BASE_ROUTE) { - $home = $this->initGetDefaultHome(); - } else { - $homeParam = $requestRoute->getParam('home'); - if (empty($homeParam) || ! $this->hasEntry($homeParam)) { - if (! ($home = $this->rewindEntries())) { - // No dashboard homes - return $this; - } - } else { - $home = $this->getEntry($homeParam); - } - } - } - + $home = $this->getEntry($name); $this->activateHome($home); $home->loadDashboardEntries(); diff --git a/library/Icinga/Web/Dashboard/DashboardHome.php b/library/Icinga/Web/Dashboard/DashboardHome.php index bd9db59a2..2f62721fc 100644 --- a/library/Icinga/Web/Dashboard/DashboardHome.php +++ b/library/Icinga/Web/Dashboard/DashboardHome.php @@ -5,10 +5,10 @@ namespace Icinga\Web\Dashboard; use Icinga\Exception\ProgrammingError; +use Icinga\Model\Home; use Icinga\Web\Dashboard\Common\BaseDashboard; use Icinga\Web\Dashboard\Common\DashboardControls; use Icinga\Web\Dashboard\Common\Sortable; -use Icinga\Web\Navigation\DashboardHomeItem; use ipl\Stdlib\Filter; use function ipl\Stdlib\get_php_type; @@ -53,21 +53,21 @@ class DashboardHome extends BaseDashboard implements Sortable protected $disabled = false; /** - * Create a new dashboard home from the given home item + * Create a new dashboard home from the given model * - * @param DashboardHomeItem $homeItem + * @param Home $home * * @return DashboardHome */ - public static function create(DashboardHomeItem $homeItem): self + public static function create(Home $home): self { - $self = new self($homeItem->getName()); + $self = new self($home->name); $self - ->setTitle($homeItem->getLabel()) - ->setPriority($homeItem->getPriority()) - ->setType($homeItem->getAttribute('type')) - ->setUuid($homeItem->getAttribute('uuid')) - ->setDisabled($homeItem->getAttribute('disabled')); + ->setTitle($home->label) + ->setPriority($home->priority) + ->setType($home->type) + ->setUuid($home->id) + ->setDisabled((bool) $home->disabled); return $self; } diff --git a/library/Icinga/Web/Menu.php b/library/Icinga/Web/Menu.php index cc8fe47f1..49c868579 100644 --- a/library/Icinga/Web/Menu.php +++ b/library/Icinga/Web/Menu.php @@ -7,7 +7,6 @@ use Icinga\Application\Logger; use Icinga\Authentication\Auth; use Icinga\Model\Home; use Icinga\Web\Dashboard\DashboardHome; -use Icinga\Web\Navigation\DashboardHomeItem; use Icinga\Web\Navigation\Navigation; use Icinga\Web\Dashboard\Dashboard; use ipl\Stdlib\Filter; @@ -23,8 +22,6 @@ class Menu extends Navigation public function __construct() { $this->init(); - $this->initHome(); - $this->load('menu-item'); } @@ -37,7 +34,8 @@ class Menu extends Navigation 'label' => t('Dashboard'), 'url' => Dashboard::BASE_ROUTE, 'icon' => 'dashboard', - 'priority' => 10 + 'priority' => 10, + 'children' => $this->fetchDashboardHomes() ]); $this->addItem('system', [ 'cssClass' => 'system-nav-item', @@ -157,49 +155,33 @@ class Menu extends Navigation } } - public function initHome() + protected function fetchDashboardHomes() { - $user = Dashboard::getUser(); - $dashboardItem = $this->getItem('dashboard'); + $dashboardHomes = []; try { $homes = Home::on(Dashboard::getConn()); - $homes->filter(Filter::equal('username', $user->getUsername())); + $homes->filter(Filter::equal('username', Auth::getInstance()->getUser()->getUsername())); foreach ($homes as $home) { - $dashboardHome = new DashboardHomeItem($home->name, [ - 'uuid' => $home->id, + if ($home->name === DashboardHome::DEFAULT_HOME) { + continue; + } + + $dashboardHomes[$home->name] = [ 'label' => t($home->label), 'priority' => $home->priority, - 'type' => $home->type, - 'disabled' => (bool) $home->disabled - ]); - - $dashboardItem->addChild($dashboardHome); + 'url' => Url::fromPath(Dashboard::BASE_ROUTE . '/home', [ + 'home' => $home->name + ]) + ]; } } catch (\Exception $_) { // Nothing to do // Any database issue will be noticed soon enough, so prevent the Menu // from being ruined in any case. } - } - /** - * Load dashboard homes form the navigation menu - * - * @return DashboardHome[] - */ - public function loadHomes() - { - $homes = []; - foreach ($this->getItem('dashboard')->getChildren() as $child) { - if (! $child instanceof DashboardHomeItem) { - continue; - } - - $homes[$child->getName()] = DashboardHome::create($child); - } - - return $homes; + return $dashboardHomes; } } diff --git a/library/Icinga/Web/Navigation/DashboardHomeItem.php b/library/Icinga/Web/Navigation/DashboardHomeItem.php deleted file mode 100644 index 0c3155f47..000000000 --- a/library/Icinga/Web/Navigation/DashboardHomeItem.php +++ /dev/null @@ -1,42 +0,0 @@ -getName() !== DashboardHome::DEFAULT_HOME) { - $this->setUrl(Url::fromPath(Dashboard::BASE_ROUTE . '/home', [ - 'home' => $this->getName() - ])); - } - } - - /** - * Get this dashboard home's url - * - * Parent class would always report a default url if $this->url isn't - * set, which we do it on purpose. - * - * @return \Icinga\Web\Url - */ - public function getUrl() - { - return $this->url; - } -}