From d659e81b0902e8c872337d508e9fc7b92eaa3330 Mon Sep 17 00:00:00 2001 From: raviks789 Date: Mon, 28 Oct 2024 16:13:15 +0100 Subject: [PATCH] fix: resolve SD's review --- library/Icingadb/Common/StateBadges.php | 23 ++++--- .../Model/Behavior/HasProblematicParent.php | 2 +- .../Icingadb/Model/RedundancyGroupSummary.php | 64 +++++++++---------- .../Widget/DependencyNodeStatistics.php | 5 +- .../ItemList/RedundancyGroupListItem.php | 14 ++-- 5 files changed, 50 insertions(+), 58 deletions(-) diff --git a/library/Icingadb/Common/StateBadges.php b/library/Icingadb/Common/StateBadges.php index 2836407bc..d9b5c45b9 100644 --- a/library/Icingadb/Common/StateBadges.php +++ b/library/Icingadb/Common/StateBadges.php @@ -82,7 +82,7 @@ protected function getBaseUrl(): ?Url */ protected function getStateInt(string $state): int { - throw new InvalidArgumentException('%s is not a valid state', $state); + throw new InvalidArgumentException(sprintf('%s is not a valid state', $state)); } /** @@ -147,21 +147,20 @@ public function createLink($content, Filter\Rule $filter = null): Link protected function createBadge(string $state): ?BaseHtmlElement { $key = $this->prefix . "_{$state}"; + if (empty($this->item->$key)) { + return null; + } - if (isset($this->item->$key) && $this->item->$key) { - $stateBadge = new StateBadge($this->item->$key, $state); + $stateBadge = new StateBadge($this->item->$key, $state); - if ($this->url !== null) { - $this->createLink( - $stateBadge, - Filter::equal($this->type . '.state.soft_state', $this->getStateInt($state)) - ); - } - - return new HtmlElement('li', null, $stateBadge); + if ($this->url !== null) { + $this->createLink( + $stateBadge, + Filter::equal($this->type . '.state.soft_state', $this->getStateInt($state)) + ); } - return null; + return new HtmlElement('li', null, $stateBadge); } /** diff --git a/library/Icingadb/Model/Behavior/HasProblematicParent.php b/library/Icingadb/Model/Behavior/HasProblematicParent.php index a70b2d5c9..9f2f691d9 100644 --- a/library/Icingadb/Model/Behavior/HasProblematicParent.php +++ b/library/Icingadb/Model/Behavior/HasProblematicParent.php @@ -92,7 +92,7 @@ public function rewriteColumnDefinition(ColumnDefinition $def, string $relation) public function rewriteCondition(Filter\Condition $condition, $relation = null): void { - $column = substr($condition->getColumn(), strlen($relation)); + $column = substr($condition->getColumn(), strlen($relation ?? '')); if ($this->isSelectableColumn($column)) { throw new InvalidColumnException($column, $this->query->getModel()); diff --git a/library/Icingadb/Model/RedundancyGroupSummary.php b/library/Icingadb/Model/RedundancyGroupSummary.php index 5aa4415f4..35ad03aaf 100644 --- a/library/Icingadb/Model/RedundancyGroupSummary.php +++ b/library/Icingadb/Model/RedundancyGroupSummary.php @@ -5,7 +5,6 @@ namespace Icinga\Module\Icingadb\Model; use ipl\Orm\Query; -use ipl\Sql\Adapter\Pgsql; use ipl\Sql\Connection; use ipl\Sql\Expression; use ipl\Sql\Select; @@ -28,39 +27,30 @@ class RedundancyGroupSummary extends RedundancyGroup public function getSummaryColumns(): array { return [ - 'nodes_total' => new Expression( - 'SUM(CASE' - . ' WHEN %s IS NOT NULL THEN 1' - . ' WHEN %s IS NOT NULL THEN 1' - . ' ELSE 0 END)', - [ - 'from.to.service_id', - 'from.to.host_id', - ] - ), + 'nodes_total' => new Expression('COUNT(*)'), 'nodes_ok' => new Expression( 'SUM(CASE' . ' WHEN %s IS NOT NULL THEN (CASE WHEN %s = 0 THEN 1 ELSE 0 END)' - . ' WHEN %s IS NOT NULL THEN (CASE WHEN %s = 0 THEN 1 ELSE 0 END)' - . ' ELSE 0 END)', + . ' WHEN %s = 0 THEN 1' + . ' ELSE 0' + . ' END)', [ - 'from.to.service.id', + 'from.to.service_id', 'from.to.service.state.soft_state', - 'from.to.host_id', 'from.to.host.state.soft_state', ] ), 'nodes_problem_handled' => new Expression( 'SUM(CASE' - . ' WHEN %s IS NOT NULL THEN (CASE WHEN %s = 2 AND (%s = \'y\' OR %s = \'n\') THEN 1 ELSE 0 END)' - . ' WHEN %s IS NOT NULL THEN (CASE WHEN %s = 1 AND (%s = \'y\' OR %s = \'n\') THEN 1 ELSE 0 END)' - . ' ELSE 0 END)', + . " WHEN %s IS NOT NULL THEN (CASE WHEN %s = 2 AND (%s = 'y' OR %s = 'n') THEN 1 ELSE 0 END)" + . " WHEN %s = 1 AND (%s = 'y' OR %s = 'n') THEN 1" + . ' ELSE 0' + . ' END)', [ 'from.to.service_id', 'from.to.service.state.soft_state', 'from.to.service.state.is_handled', 'from.to.service.state.is_reachable', - 'from.to.host_id', 'from.to.host.state.soft_state', 'from.to.host.state.is_handled', 'from.to.host.state.is_reachable', @@ -68,15 +58,15 @@ public function getSummaryColumns(): array ), 'nodes_problem_unhandled' => new Expression( 'SUM(CASE' - . ' WHEN %s IS NOT NULL THEN (CASE WHEN %s = 2 AND (%s = \'n\' AND %s = \'y\') THEN 1 ELSE 0 END)' - . ' WHEN %s IS NOT NULL THEN (CASE WHEN %s = 1 AND (%s = \'n\' AND %s = \'y\') THEN 1 ELSE 0 END)' - . ' ELSE 0 END)', + . " WHEN %s IS NOT NULL THEN (CASE WHEN %s = 2 AND (%s = 'n' AND %s = 'y') THEN 1 ELSE 0 END)" + . " WHEN %s = 1 AND (%s = 'n' AND %s = 'y') THEN 1" + . ' ELSE 0' + . ' END)', [ 'from.to.service_id', 'from.to.service.state.soft_state', 'from.to.service.state.is_handled', 'from.to.service.state.is_reachable', - 'from.to.host_id', 'from.to.host.state.soft_state', 'from.to.host.state.is_handled', 'from.to.host.state.is_reachable', @@ -85,19 +75,20 @@ public function getSummaryColumns(): array 'nodes_pending' => new Expression( 'SUM(CASE' . ' WHEN %s IS NOT NULL THEN (CASE WHEN %s = 99 THEN 1 ELSE 0 END)' - . ' WHEN %s IS NOT NULL THEN (CASE WHEN %s = 99 THEN 1 ELSE 0 END)' - . ' ELSE 0 END)', + . ' WHEN %s = 99 THEN 1' + . ' ELSE 0' + . ' END)', [ - 'from.to.service.id', + 'from.to.service_id', 'from.to.service.state.soft_state', - 'from.to.host_id', 'from.to.host.state.soft_state', ] ), 'nodes_unknown_handled' => new Expression( 'SUM(CASE' - . ' WHEN %s IS NOT NULL THEN (CASE WHEN %s = 3 AND (%s = \'y\' OR %s = \'n\') THEN 1 ELSE 0 END)' - . ' ELSE 0 END)', + . " WHEN %s IS NOT NULL THEN (CASE WHEN %s = 3 AND (%s = 'y' OR %s = 'n') THEN 1 ELSE 0 END)" + . ' ELSE 0' + . ' END)', [ 'from.to.service_id', 'from.to.service.state.soft_state', @@ -107,8 +98,9 @@ public function getSummaryColumns(): array ), 'nodes_unknown_unhandled' => new Expression( 'SUM(CASE' - . ' WHEN %s IS NOT NULL THEN (CASE WHEN %s = 3 AND (%s = \'n\' AND %s = \'y\') THEN 1 ELSE 0 END)' - . ' ELSE 0 END)', + . " WHEN %s IS NOT NULL THEN (CASE WHEN %s = 3 AND (%s = 'n' AND %s = 'y') THEN 1 ELSE 0 END)" + . ' ELSE 0' + . ' END)', [ 'from.to.service_id', 'from.to.service.state.soft_state', @@ -118,8 +110,9 @@ public function getSummaryColumns(): array ), 'nodes_warning_handled' => new Expression( 'SUM(CASE' - . ' WHEN %s IS NOT NULL THEN (CASE WHEN %s = 1 AND (%s = \'y\' OR %s = \'n\') THEN 1 ELSE 0 END)' - . ' ELSE 0 END)', + . " WHEN %s IS NOT NULL THEN (CASE WHEN %s = 1 AND (%s = 'y' OR %s = 'n') THEN 1 ELSE 0 END)" + . ' ELSE 0' + . ' END)', [ 'from.to.service_id', 'from.to.service.state.soft_state', @@ -129,8 +122,9 @@ public function getSummaryColumns(): array ), 'nodes_warning_unhandled' => new Expression( 'SUM(CASE' - . ' WHEN %s IS NOT NULL THEN (CASE WHEN %s = 1 AND (%s = \'n\' AND %s = \'y\') THEN 1 ELSE 0 END)' - . ' ELSE 0 END)', + . " WHEN %s IS NOT NULL THEN (CASE WHEN %s = 1 AND (%s = 'n' AND %s = 'y') THEN 1 ELSE 0 END)" + . ' ELSE 0' + . ' END)', [ 'from.to.service_id', 'from.to.service.state.soft_state', diff --git a/library/Icingadb/Widget/DependencyNodeStatistics.php b/library/Icingadb/Widget/DependencyNodeStatistics.php index 53e8d5e23..d8dca91ae 100644 --- a/library/Icingadb/Widget/DependencyNodeStatistics.php +++ b/library/Icingadb/Widget/DependencyNodeStatistics.php @@ -5,18 +5,19 @@ namespace Icinga\Module\Icingadb\Widget; use Icinga\Chart\Donut; +use Icinga\Module\Dependencies\Model\DependencyNodeSummary; use Icinga\Module\Icingadb\Model\RedundancyGroupSummary; use Icinga\Module\Icingadb\Widget\Detail\ObjectStatistics; use ipl\Html\Text; use ipl\Html\ValidHtml; use ipl\Html\HtmlString; -/** +/**e * Dependency node statistics */ class DependencyNodeStatistics extends ObjectStatistics { - /** @var RedundancyGroupSummary Dependency node summary */ + /** @var RedundancyGroupSummary|DependencyNodeSummary Dependency node summary*/ protected $summary; public function __construct($summary) diff --git a/library/Icingadb/Widget/ItemList/RedundancyGroupListItem.php b/library/Icingadb/Widget/ItemList/RedundancyGroupListItem.php index 6a3bdc110..2b87723d2 100644 --- a/library/Icingadb/Widget/ItemList/RedundancyGroupListItem.php +++ b/library/Icingadb/Widget/ItemList/RedundancyGroupListItem.php @@ -19,7 +19,7 @@ use ipl\Web\Widget\TimeSince; /** - * Redundancy group list item of a root problem list. Represents one database row. + * Redundancy group list item. Represents one database row. * * @property RedundancyGroup $item */ @@ -28,7 +28,7 @@ class RedundancyGroupListItem extends StateListItem use ListItemCommonLayout; use Database; - protected $defaultAttributes = ['class' => ['list-item', 'redundancy-group-list-item']]; + protected $defaultAttributes = ['class' => ['redundancy-group-list-item']]; /** @var RedundancyGroupState */ protected $state; @@ -70,14 +70,12 @@ protected function assembleTitle(BaseHtmlElement $title): void { $title->addHtml($this->createSubject()); if ($this->state->failed) { - $title->addHtml(HtmlElement::create( - 'span', - null, - Text::create($this->translate('has no working objects')) - )); + $text = $this->translate('has no working objects'); } else { - $title->addHtml(HtmlElement::create('span', null, Text::create($this->translate('has working objects')))); + $text = $this->translate('has working objects'); } + + $title->addHtml(HtmlElement::create('span', null, Text::create($text))); } protected function assemble(): void