From 13686a74c35b45fe7f9acef40ff2abe5349a2c32 Mon Sep 17 00:00:00 2001 From: raviks789 Date: Wed, 23 Oct 2024 14:24:12 +0200 Subject: [PATCH] fix: resolve review comments --- .../Icingadb/Model/RedundancyGroupSummary.php | 24 ++------------- .../Icingadb/Widget/Detail/ObjectDetail.php | 30 +++++++++++-------- public/css/common.less | 6 ---- .../css/list/redundancy-group-list-item.less | 15 +++++----- .../widget/dependency-node-state-badges.less | 6 ++++ 5 files changed, 33 insertions(+), 48 deletions(-) diff --git a/library/Icingadb/Model/RedundancyGroupSummary.php b/library/Icingadb/Model/RedundancyGroupSummary.php index 6a1cc52f0..5aa4415f4 100644 --- a/library/Icingadb/Model/RedundancyGroupSummary.php +++ b/library/Icingadb/Model/RedundancyGroupSummary.php @@ -11,7 +11,7 @@ use ipl\Sql\Select; /** - * Redundancy group's summary (The nodes could only host and service) + * Redundancy group's summary * * @property int $nodes_total * @property int $nodes_ok @@ -143,13 +143,7 @@ public function getSummaryColumns(): array public static function on(Connection $db): Query { - $q = parent::on($db)->with([ - 'from', - 'from.to.host', - 'from.to.host.state', - 'from.to.service', - 'from.to.service.state' - ]); + $q = parent::on($db); /** @var static $m */ $m = $q->getModel(); @@ -157,21 +151,7 @@ public static function on(Connection $db): Query $q->on($q::ON_SELECT_ASSEMBLED, function (Select $select) use ($q) { $model = $q->getModel(); - $groupBy = $q->getResolver()->qualifyColumnsAndAliases((array) $model->getKeyName(), $model, false); - - // For PostgreSQL, ALL non-aggregate SELECT columns must appear in the GROUP BY clause: - if ($q->getDb()->getAdapter() instanceof Pgsql) { - /** - * Ignore Expressions, i.e. aggregate functions {@see getColumns()}, - * which do not need to be added to the GROUP BY. - */ - $candidates = array_filter($select->getColumns(), 'is_string'); - // Remove already considered columns for the GROUP BY, i.e. the primary key. - $candidates = array_diff_assoc($candidates, $groupBy); - $groupBy = array_merge($groupBy, $candidates); - } - $select->groupBy($groupBy); }); diff --git a/library/Icingadb/Widget/Detail/ObjectDetail.php b/library/Icingadb/Widget/Detail/ObjectDetail.php index 9a95f297d..7949814ab 100644 --- a/library/Icingadb/Widget/Detail/ObjectDetail.php +++ b/library/Icingadb/Widget/Detail/ObjectDetail.php @@ -23,6 +23,7 @@ use Icinga\Module\Icingadb\Model\CustomvarFlat; use Icinga\Module\Icingadb\Model\Service; use Icinga\Module\Icingadb\Model\UnreachableParent; +use Icinga\Module\Icingadb\Redis\VolatileStateResults; use Icinga\Module\Icingadb\Web\Navigation\Action; use Icinga\Module\Icingadb\Widget\ItemList\DependencyNodeList; use Icinga\Module\Icingadb\Widget\MarkdownText; @@ -375,7 +376,7 @@ protected function createNotes() protected function createNotifications(): array { - list($users, $usergroups) = $this->getUsersAndUsergroups(); + [$users, $usergroups] = $this->getUsersAndUsergroups(); $userList = new TagList(); $usergroupList = new TagList(); @@ -605,11 +606,16 @@ protected function fetchCustomVars() } } + /** + * Create a list of root problems of the object that is unreachable because of dependency failure + * + * @return ?BaseHtmlElement[] + */ protected function createRootProblems(): ?array { // If a dependency has failed, then the children are not reachable. Hence, the root problems should not be shown - // if the object is not reachable. And in case of a service, since, it may be also be unreachable because of its - // host being down, only show its root problems if they exist. + // if the object is reachable. And in case of a service, since, it may be also be unreachable because of its + // host being down, only show its root problems if it's really caused by a dependency failure. if ( $this->object->state->is_reachable || ($this->object instanceof Service && ! $this->object->has_problematic_parent) @@ -631,15 +637,15 @@ protected function createRootProblems(): ?array 'service.state.last_comment', 'service.host', 'service.host.state' - ])->orderBy([ - 'host.state.severity' => SORT_DESC, - 'host.state.last_state_change' => SORT_DESC, - 'service.state.severity' => SORT_DESC, - 'service.state.last_state_change' => SORT_DESC, - 'service.host.state.severity' => SORT_DESC, - 'service.host.state.last_state_change' => SORT_DESC, - 'redundancy_group.state.last_state_change' => SORT_DESC - ]); + ]) + ->setResultSetClass(VolatileStateResults::class) + ->orderBy([ + 'host.state.severity', + 'host.state.last_state_change', + 'service.state.severity', + 'service.state.last_state_change', + 'redundancy_group.state.last_state_change' + ], SORT_DESC); $this->applyRestrictions($rootProblems); diff --git a/public/css/common.less b/public/css/common.less index be596dd74..3a28d03b0 100644 --- a/public/css/common.less +++ b/public/css/common.less @@ -422,9 +422,3 @@ form[name="form_confirm_removal"] { background-color: @color-ok; } } - -.state-badge { - &.state-problem { - background-color: @color-critical; - } -} diff --git a/public/css/list/redundancy-group-list-item.less b/public/css/list/redundancy-group-list-item.less index 7826323da..1c8fff2d0 100644 --- a/public/css/list/redundancy-group-list-item.less +++ b/public/css/list/redundancy-group-list-item.less @@ -1,20 +1,19 @@ .redundancy-group-list-item { .caption { display: flex; - justify-content: space-between; + align-items: baseline; .plugin-output { + // Allows the plugin output width to grow or shrink based on the width of object statistics + flex: 1 1 auto; + // Since caption is not -webkit-box anymore, its line-clamp rule will not be inherited + // and hence applied here directly .line-clamp(2); } .object-statistics { - ul { - display: flex; - } - - .state-badges { - font-size: 0.75em; - } + // Prevents the wrapping effect caused by changing caption to a flexbox + flex: 0 0 auto; } } } diff --git a/public/css/widget/dependency-node-state-badges.less b/public/css/widget/dependency-node-state-badges.less index 0a2e71e64..dabeecf9a 100644 --- a/public/css/widget/dependency-node-state-badges.less +++ b/public/css/widget/dependency-node-state-badges.less @@ -1,3 +1,9 @@ .dependency-node-state-badges { .state-badges(); + + .state-badge { + &.state-problem { + background-color: @color-critical; + } + } }