Unify error messages & add some class docblocks

This commit is contained in:
Yonas Habteab 2022-06-14 09:11:02 +02:00
parent 73853c327c
commit ee829fd4dc
14 changed files with 115 additions and 99 deletions

View File

@ -34,21 +34,17 @@ class DashboardsController extends CompatController
/** @var Dashboard */
protected $dashboard;
/**
* Whether the currently loaded home/pane is also loaded on the previous page
*
* @var string
*/
protected $prevActive = null;
/** @var string Whether the currently loaded home/pane is also loaded on the previous page */
protected $highlighted = null;
public function init()
{
parent::init();
// The "prevActive" param indicates whether this home/pane is currently being loaded in the
// The "highlighted" param indicates whether this home/pane is currently being loaded in the
// DM view meanwhile rendering a modal view (update and remove actions). If it's indeed the case,
// we have to construct a proper http redirect after successfully removing this home
$this->prevActive = $this->params->shift('prevActive');
// we have to construct a proper http redirect after successfully removing this home/pane.
$this->highlighted = $this->params->shift('highlighted');
$this->dashboard = new Dashboard();
$this->dashboard->setUser($this->Auth()->getUser());
@ -117,7 +113,7 @@ class DashboardsController extends CompatController
$this->redirectNow('__CLOSE__');
})->handleRequest($this->getServerRequest());
$this->setTitle(t('Add new Dashboard Home'));
$this->addTitleTab(t('Add new Dashboard Home'));
$this->addContent($homeForm);
}
@ -139,7 +135,7 @@ class DashboardsController extends CompatController
$homeForm->load($this->dashboard->getActiveHome());
$this->setTitle(t('Update Home'));
$this->addTitleTab(t('Update Home'));
$this->addContent($homeForm);
}
@ -149,22 +145,22 @@ class DashboardsController extends CompatController
$this->dashboard->load($home);
$homeForm = (new RemoveHomeForm($this->dashboard))
->on(RemoveHomeForm::ON_SUCCESS, function () {
$response = $this->getResponse();
$response->setHeader('X-Icinga-Extra-Updates', '#menu');
$homeForm = new RemoveHomeForm($this->dashboard);
$homeForm->on(RemoveHomeForm::ON_SUCCESS, function () use ($homeForm) {
$response = $this->getResponse();
$response->setHeader('X-Icinga-Extra-Updates', '#menu');
if ($this->prevActive) {
$this->redirectNow(Url::fromPath(Dashboard::BASE_ROUTE . '/settings'));
} else {
$response->setHeader('X-Icinga-Container', 'modal-content', true);
if ($this->highlighted && $homeForm->requestSucceeded()) {
$this->redirectNow(Url::fromPath(Dashboard::BASE_ROUTE . '/settings'));
} else {
$response->setHeader('X-Icinga-Container', 'modal-content', true);
$this->redirectNow('__CLOSE__');
}
})
$this->redirectNow('__CLOSE__');
}
})
->handleRequest($this->getServerRequest());
$this->setTitle(t('Remove Home'));
$this->addTitleTab(t('Remove Home'));
$this->addContent($homeForm);
}
@ -182,7 +178,7 @@ class DashboardsController extends CompatController
})
->handleRequest($this->getServerRequest());
$this->setTitle(t('Add new Dashboard'));
$this->addTitleTab(t('Add new Pane'));
$this->addContent($paneForm);
}
@ -195,7 +191,7 @@ class DashboardsController extends CompatController
$paneForm = new PaneForm($this->dashboard);
$paneForm->on(PaneForm::ON_SUCCESS, function () use ($paneForm, $home) {
if ($this->prevActive && $home !== $paneForm->getPopulatedValue('home')) {
if ($paneForm->requestSucceeded() && $this->highlighted && $home !== $paneForm->getValue('home')) {
$params = $this->params->without('pane');
$this->redirectNow(Url::fromPath(Dashboard::BASE_ROUTE . '/settings')->setParams($params));
} else {
@ -207,7 +203,7 @@ class DashboardsController extends CompatController
$paneForm->load($this->dashboard->getActiveHome()->getEntry($pane));
$this->setTitle(t('Update Pane'));
$this->addTitleTab(t('Update Pane'));
$this->addContent($paneForm);
}
@ -220,9 +216,10 @@ class DashboardsController extends CompatController
$paneForm = new RemovePaneForm($this->dashboard);
$paneForm->populate(['org_name' => $paneParam]);
$paneForm->on(RemovePaneForm::ON_SUCCESS, function () {
if ($this->prevActive) {
$this->redirectNow(Url::fromPath(Dashboard::BASE_ROUTE . '/settings'));
$paneForm->on(RemovePaneForm::ON_SUCCESS, function () use ($paneForm) {
if ($this->highlighted && $paneForm->requestSucceeded()) {
$params = $this->params->without('pane');
$this->redirectNow(Url::fromPath(Dashboard::BASE_ROUTE . '/settings')->setParams($params));
} else {
$this->getResponse()->setHeader('X-Icinga-Container', 'modal-content', true);
@ -230,7 +227,7 @@ class DashboardsController extends CompatController
}
})->handleRequest($this->getServerRequest());
$this->setTitle(t('Remove Pane'));
$this->addTitleTab(t('Remove Pane'));
$this->addContent($paneForm);
}
@ -250,9 +247,9 @@ class DashboardsController extends CompatController
})->handleRequest($this->getServerRequest());
if (isset($this->getRequest()->getPost()['btn_next'])) {
$this->setTitle(t('Add Dashlet To Dashboard'));
$this->addTitleTab(t('Add Dashlet To Dashboard'));
} else {
$this->setTitle(t('Select Dashlets'));
$this->addTitleTab(t('Select Dashlets'));
}
$this->addContent($dashletForm);
@ -273,8 +270,7 @@ class DashboardsController extends CompatController
$this->redirectNow(Url::fromPath(Dashboard::BASE_ROUTE . '/settings')->setParams($params));
})->handleRequest($this->getServerRequest());
$this->setTitle(t('Add Dashlet To Dashboard'));
$this->addTitleTab(t('Add Dashlet To Dashboard'));
$this->addContent($dashletForm);
}
@ -303,7 +299,7 @@ class DashboardsController extends CompatController
$dashletForm->load($dashlet);
$this->setTitle(t('Edit Dashlet'));
$this->addTitleTab(t('Edit Dashlet'));
$this->addContent($dashletForm);
}
@ -328,7 +324,7 @@ class DashboardsController extends CompatController
})
->handleRequest($this->getServerRequest());
$this->setTitle(t('Remove Dashlet'));
$this->addTitleTab(t('Remove Dashlet'));
$this->addContent($removeForm);
}
@ -421,11 +417,9 @@ class DashboardsController extends CompatController
$redirect = $orgHome && $pane->getName() === $highlightPane;
Notification::success(sprintf(
t('%s dashboard pane "%s" successfully'),
$orgHome ? 'Moved' : 'Updated',
$pane->getTitle()
));
Notification::success(
sprintf(t('%s pane "%s" successfully'), $orgHome ? 'Moved' : 'Updated', $pane->getTitle())
);
break;
}
@ -437,9 +431,9 @@ class DashboardsController extends CompatController
if ($orgPane && $orgPane->hasEntry($dashlet) && $pane->hasEntry($dashlet)) {
Notification::error(sprintf(
t('Dashlet "%s" already exists within "%s" dashboard pane'),
$dashlet,
$pane->getTitle()
t('Pane "%s" has already a Dashlet called "%s"'),
$pane->getTitle(),
$dashlet
));
$duplicatedError = true;
@ -494,9 +488,9 @@ class DashboardsController extends CompatController
// rendered in the modal view when redirecting
$this->view->compact = true;
$this->setTitle(t('Configure Dashlets'));
$this->addTitleTab(t('Configure Dashlets'));
} else {
$this->setTitle(t('Add Dashlet'));
$this->addTitleTab(t('Add Dashlet'));
}
$this->addContent($setupForm);

View File

@ -15,7 +15,7 @@ use PDO;
class BaseDashboardTestCase extends BaseTestCase
{
const TEST_HOME = 'Test Home';
public const TEST_HOME = 'Test Home';
/** @var Dashboard */
protected $dashboard;

View File

@ -19,6 +19,10 @@ class DBUtils
/** @var Connection */
private static $conn;
private function __construct()
{
}
/**
* Get Database connection
*
@ -114,9 +118,7 @@ class DBUtils
*/
public static function isBinary(string $data): bool
{
// Stolen from php.net
$data = preg_replace('/\s/', '', $data ?? '');
$data = preg_replace('/\s/', '', $data);
return ! empty($data) && ! ctype_print($data);
}

View File

@ -109,7 +109,7 @@ trait DashboardEntries
public function unsetEntry(BaseDashboard $dashboard)
{
if (! $this->hasEntry($dashboard->getName())) {
throw new ProgrammingError('Trying to unset an invalid Dashboard entry: "%s"', $dashboard->getName());
throw new ProgrammingError('Trying to unset an invalid Dashboard entry: "%s"', $dashboard->getTitle());
}
unset($this->dashboards[strtolower($dashboard->getName())]);

View File

@ -100,7 +100,10 @@ trait DashboardManager
public function activateHome(DashboardHome $home): self
{
if (! $this->hasEntry($home->getName())) {
throw new ProgrammingError('Trying to activate Dashboard Home "%s" that does not exist.', $home->getName());
throw new ProgrammingError(
'Trying to activate Dashboard Home "%s" that does not exist.',
$home->getTitle()
);
}
$activeHome = $this->getActiveHome();

View File

@ -10,7 +10,6 @@ use Icinga\User;
use Icinga\Util\DBUtils;
use ipl\Stdlib\Filter;
// TODO: Remove this completely as soon as we have introduced a daemon in Icinga Web 2.
trait DashboardUserManager
{
/** @var User */

View File

@ -31,14 +31,14 @@ class Dashboard extends BaseHtmlElement implements DashboardEntry
*
* @var string
*/
const BASE_ROUTE = 'dashboards';
public const BASE_ROUTE = 'dashboards';
/**
* System dashboards are provided by the modules in PHP code and are available to all users
*
* @var string
*/
const SYSTEM = 'system';
public const SYSTEM = 'system';
/**
* Public dashboards are created by authorized users and are available
@ -46,14 +46,14 @@ class Dashboard extends BaseHtmlElement implements DashboardEntry
*
* @var string
*/
const PUBLIC_DS = 'public';
public const PUBLIC_DS = 'public';
/**
* Private dashboards are created by any user and are only available to this user
*
* @var string
*/
const PRIVATE_DS = 'private';
public const PRIVATE_DS = 'private';
/**
* Shared dashboards are available to users who have accepted a share or who
@ -61,7 +61,7 @@ class Dashboard extends BaseHtmlElement implements DashboardEntry
*
* @var string
*/
const SHARED = 'shared';
public const SHARED = 'shared';
protected $tag = 'div';
@ -130,7 +130,7 @@ class Dashboard extends BaseHtmlElement implements DashboardEntry
return $this->tabs;
}
/*** @var Pane $pane */
/** @var Pane $pane */
foreach ($activeHome->getEntries() as $pane) {
if (! $this->tabs->get($pane->getName())) {
$this->tabs->add(
@ -173,9 +173,7 @@ class Dashboard extends BaseHtmlElement implements DashboardEntry
. ' You will always be able to edit them afterwards.'
);
$this->addHtml(HtmlElement::create('p', null, $message));
} elseif (! $activeHome->hasEntries()) {
$this->addHtml(HtmlElement::create('h1', null, t('No dashboard added to this dashboard home.')));
} else {
} elseif ($activeHome->hasEntries()) {
$activePane = $activeHome->getActivePane();
if (! $activePane->hasEntries()) {
@ -185,6 +183,8 @@ class Dashboard extends BaseHtmlElement implements DashboardEntry
$this->addHtml($dashlet->getHtml());
}
}
} else {
$this->addHtml(HtmlElement::create('h1', null, t('No dashboard added to this dashboard home.')));
}
}
}

View File

@ -14,10 +14,16 @@ use Icinga\Web\Dashboard\Common\DashboardEntry;
use Icinga\Web\Dashboard\Common\Sortable;
use Icinga\Util\DBUtils;
use Icinga\Web\Dashboard\Common\WidgetState;
use Icinga\Web\Navigation\NavigationItem;
use ipl\Stdlib\Filter;
use function ipl\Stdlib\get_php_type;
/**
* A Dashboard Home groups various Dashboard Panes and provides the ability
* to load Panes from different entry point of view. Dashboard Homes are
* rendered as child of {@see NavigationItem}s of the main dashboard menu.
*/
class DashboardHome extends BaseDashboard implements DashboardEntry, Sortable
{
use DashboardEntries;
@ -108,7 +114,10 @@ class DashboardHome extends BaseDashboard implements DashboardEntry, Sortable
public function activatePane(Pane $pane): self
{
if (! $this->hasEntry($pane->getName())) {
throw new ProgrammingError('Trying to activate Dashboard Pane "%s" that does not exist.', $pane->getName());
throw new ProgrammingError(
'Trying to activate Dashboard Pane "%s" that does not exist.',
$pane->getTitle()
);
}
$active = $this->getActivePane();
@ -116,7 +125,7 @@ class DashboardHome extends BaseDashboard implements DashboardEntry, Sortable
$active->setActive(false);
}
$pane->setActive(true);
$pane->setActive();
return $this;
}
@ -142,7 +151,7 @@ class DashboardHome extends BaseDashboard implements DashboardEntry, Sortable
{
$name = $pane instanceof Pane ? $pane->getName() : $pane;
if (! $this->hasEntry($name)) {
throw new ProgrammingError('Trying to remove invalid dashboard pane "%s"', $name);
throw new ProgrammingError('Trying to remove invalid pane "%s"', $name);
}
$pane = $pane instanceof Pane ? $pane : $this->getEntry($pane);
@ -250,6 +259,8 @@ class DashboardHome extends BaseDashboard implements DashboardEntry, Sortable
'id = ?' => $origin->getEntry($pane->getName())->getUuid(),
'home_id = ?' => $origin->getUuid()
];
$this->addEntry($pane);
}
$conn->update(Pane::TABLE, [
@ -262,9 +273,10 @@ class DashboardHome extends BaseDashboard implements DashboardEntry, Sortable
// Failed to move the pane! Should have already been handled by the caller,
// though I think it's better that we raise an exception here!!
throw new AlreadyExistsException(
'Dashboard Pane "%s" could not be managed. Dashboard Home "%s" has Pane with the same name!',
$pane->getTitle(),
$this->getTitle()
'Failed to successfully manage the pane. Dashboard Home "%s" has already' .
' a Pane named "%s"!',
$this->getTitle(),
$pane->getTitle()
);
}

View File

@ -13,16 +13,15 @@ use ipl\Html\HtmlElement;
use ipl\Web\Widget\Link;
/**
* A dashboard pane dashlet
*
* This is the new element being used for the Dashlets view
* A Dashlet/View is basically an Url which is being visualized
* into a single View by Icinga Web 2
*/
class Dashlet extends BaseDashboard
{
use WidgetState;
/** @var string Database table name */
const TABLE = 'icingaweb_dashlet';
public const TABLE = 'icingaweb_dashlet';
/**
* The url of this Dashlet
@ -91,7 +90,7 @@ class Dashlet extends BaseDashboard
}
/**
* Set the dashlets URL
* Set the URL of this dashlet
*
* @param string|Url $url The url to use, either as an Url object or as a path
*
@ -191,7 +190,7 @@ class Dashlet extends BaseDashboard
}
/**
* Set whether this dashlet widget is provided by a module
* Set whether this dashlet is provided by a module
*
* @param bool $moduleDashlet
*
@ -213,22 +212,22 @@ class Dashlet extends BaseDashboard
{
$dashletHtml = HtmlElement::create('div', ['class' => 'container']);
if (! $this->getUrl()) {
$dashletHtml->addHtml(HtmlElement::create('h1', null, t($this->getTitle())));
$dashletHtml->addHtml(HtmlElement::create('h1', null, $this->getTitle()));
$dashletHtml->addHtml(HtmlElement::create(
'p',
['class' => 'error-message'],
sprintf(t('Cannot create dashboard dashlet "%s" without valid URL'), t($this->getTitle()))
sprintf(t('Cannot create dashboard dashlet "%s" without valid URL'), $this->getTitle())
));
} else {
$url = $this->getUrl();
$dashletHtml->setAttribute('data-icinga-url', $url->with('showCompact', true));
$dashletHtml->addHtml(new HtmlElement('h1', null, new Link(
t($this->getTitle()),
$this->getTitle(),
$url->without(['limit', 'view'])->getRelativeUrl(),
[
'aria-label' => t($this->getTitle()),
'title' => t($this->getTitle()),
'aria-label' => $this->getTitle(),
'title' => $this->getTitle(),
'data-base-target' => 'col1'
]
)));

View File

@ -83,7 +83,7 @@ class DashboardHomeList extends ItemListControl
]);
if ($this->home->isActive()) {
$url->addParams(['prevActive' => true]);
$url->addParams(['highlighted' => true]);
}
$this->assembleHeader($url, $this->home->getTitle());
@ -106,7 +106,7 @@ class DashboardHomeList extends ItemListControl
$url->setParams(['home' => $this->home->getName()]);
return new Link(
[new Icon('plus'), t('Add Dashboard')],
[new Icon('plus'), t('Add Pane')],
$url,
['class' => ['button-link', 'add-dashboard']]
);

View File

@ -52,7 +52,7 @@ class DashboardList extends ItemListControl
]);
if ($this->pane->isActive()) {
$url->addParams(['prevActive' => true]);
$url->addParams(['highlighted' => true]);
}
$this->assembleHeader($url, $pane->getTitle());

View File

@ -15,19 +15,22 @@ use Icinga\Util\DBUtils;
use Icinga\Web\Dashboard\Common\DashboardEntry;
use Icinga\Web\Dashboard\Common\Sortable;
use Icinga\Web\Dashboard\Common\WidgetState;
use InvalidArgumentException;
use ipl\Stdlib\Filter;
use LogicException;
use function ipl\Stdlib\get_php_type;
/**
* A pane, displaying different Dashboard dashlets
* A Pane or Dashboard Pane organizes various Dashlets/Views into a single panel
* and can be accessed via the tabs rendered on the upper navigation bar of Icinga Web 2.
*/
class Pane extends BaseDashboard implements DashboardEntry, Sortable
{
use DashboardEntries;
use WidgetState;
const TABLE = 'icingaweb_dashboard';
public const TABLE = 'icingaweb_dashboard';
/**
* A dashboard home this pane is a part of
@ -100,6 +103,7 @@ class Pane extends BaseDashboard implements DashboardEntry, Sortable
$dashlets = Model\Dashlet::on(DBUtils::getConn())
->utilize(self::TABLE)
->with('icingaweb_module_dashlet');
$dashlets->filter(Filter::equal('dashboard_id', $this->getUuid()));
// TODO(yh): Qualify those columns properly??
@ -157,7 +161,7 @@ class Pane extends BaseDashboard implements DashboardEntry, Sortable
public function manageEntry($entryOrEntries, BaseDashboard $origin = null, bool $manageRecursive = false)
{
if ($origin && ! $origin instanceof Pane) {
throw new \InvalidArgumentException(sprintf(
throw new InvalidArgumentException(sprintf(
__METHOD__ . ' expects parameter "$origin" to be an instance of "%s". Got "%s" instead.',
get_php_type($this),
get_php_type($origin)
@ -165,8 +169,9 @@ class Pane extends BaseDashboard implements DashboardEntry, Sortable
}
if (! $this->getHome()) {
throw new \LogicException(
'Dashlets cannot be managed. Please make sure to set the current dashboard home beforehand.'
throw new LogicException(
'Dashlet(s) cannot be managed without a valid Dashboard Home. Please make sure to set' .
' the current dashboard home beforehand.'
);
}
@ -231,6 +236,8 @@ class Pane extends BaseDashboard implements DashboardEntry, Sortable
'id = ?' => $origin->getEntry($dashlet->getName())->getUuid(),
'dashboard_id = ?' => $origin->getUuid()
];
$this->addEntry($dashlet);
}
$conn->update(Dashlet::TABLE, [
@ -246,9 +253,9 @@ class Pane extends BaseDashboard implements DashboardEntry, Sortable
// Failed to move the pane! Should have already been handled by the caller,
// though I think it's better that we raise an exception here!!
throw new AlreadyExistsException(
'Dashlet "%s" could not be managed. Dashboard Pane "%s" has a Dashlet with the same name!',
$dashlet->getTitle(),
$this->getTitle()
'Failed to successfully manage the Dashlet. Pane "%s" has already a Dashlet called "%s"!',
$this->getTitle(),
$dashlet->getTitle()
);
}

View File

@ -105,7 +105,7 @@ class DashboardsCommand extends Command
} elseif ($dashboardHome->hasEntry($pane)) {
do {
$pane = readline(sprintf(
'Dashboard Pane "%s" already exists within the "%s" Dashboard Home.' . "\n" .
'Pane "%s" already exists within the "%s" Dashboard Home.' . "\n" .
'Please enter another name for this pane or rerun the command with the "silent"' .
' param to suppress such errors!: ',
$pane,

View File

@ -37,7 +37,7 @@ class PaneTest extends BaseDashboardTestCase
$this->assertEquals(
self::TEST_PANE,
$home->getActivePane()->getName(),
'DashboardHome::activatePane() could not activate expected Dashboard Pane'
'DashboardHome::activatePane() could not activate expected Pane'
);
}
@ -58,7 +58,7 @@ class PaneTest extends BaseDashboardTestCase
$this->assertEquals(
self::TEST_PANE,
$home->getActivePane()->getName(),
'DashboardHome::loadDashboardEntries() could not activate expected Dashboard Pane'
'DashboardHome::loadDashboardEntries() could not activate expected Pane'
);
}
@ -78,7 +78,7 @@ class PaneTest extends BaseDashboardTestCase
$this->assertEquals(
self::TEST_PANE,
$home->getActivePane()->getName(),
'DashboardHome::loadDashboardEntries() could not load and activate expected Dashboard Pane'
'DashboardHome::loadDashboardEntries() could not load and activate expected Pane'
);
}
@ -116,7 +116,7 @@ class PaneTest extends BaseDashboardTestCase
$this->assertCount(
1,
$home->getEntries(),
'DashboardHome::manageEntry() could not manage a new Dashboard Pane'
'DashboardHome::manageEntry() could not manage a new Pane'
);
}
@ -143,7 +143,7 @@ class PaneTest extends BaseDashboardTestCase
$this->assertEquals(
'Hello',
$home->getActivePane()->getTitle(),
'DashboardHome::manageEntry() could not update existing Dashboard Pane'
'DashboardHome::manageEntry() could not update existing Pane'
);
}
@ -171,7 +171,7 @@ class PaneTest extends BaseDashboardTestCase
$this->assertCount(
1,
$default->getEntries(),
'DashboardHome::manageEntry() could not move a Dashboard Pane to another existing Dashboard Home'
'DashboardHome::manageEntry() could not move a Pane to another existing Dashboard Home'
);
}
@ -224,7 +224,7 @@ class PaneTest extends BaseDashboardTestCase
$this->assertFalse(
$home->hasEntry(self::TEST_PANE),
'DashboardHome::removeEntry() could not remove expected Dashboard Pane'
'DashboardHome::removeEntry() could not remove expected Pane'
);
}
@ -249,7 +249,7 @@ class PaneTest extends BaseDashboardTestCase
$this->assertFalse(
$home->hasEntries(),
'DashboardHome::removeEntries() could not remove all Dashboard Panes'
'DashboardHome::removeEntries() could not remove all Panes'
);
}
}