Skip to content

Commit

Permalink
fix: resolve review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
raviks789 committed Oct 24, 2024
1 parent c6329b7 commit 13686a7
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 48 deletions.
24 changes: 2 additions & 22 deletions library/Icingadb/Model/RedundancyGroupSummary.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -143,35 +143,15 @@ 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();
$q->columns($m->getSummaryColumns());

$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);
});

Expand Down
30 changes: 18 additions & 12 deletions library/Icingadb/Widget/Detail/ObjectDetail.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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)
Expand All @@ -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);

Expand Down
6 changes: 0 additions & 6 deletions public/css/common.less
Original file line number Diff line number Diff line change
Expand Up @@ -422,9 +422,3 @@ form[name="form_confirm_removal"] {
background-color: @color-ok;
}
}

.state-badge {
&.state-problem {
background-color: @color-critical;
}
}
15 changes: 7 additions & 8 deletions public/css/list/redundancy-group-list-item.less
Original file line number Diff line number Diff line change
@@ -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;
}
}
}
6 changes: 6 additions & 0 deletions public/css/widget/dependency-node-state-badges.less
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
.dependency-node-state-badges {
.state-badges();

.state-badge {
&.state-problem {
background-color: @color-critical;
}
}
}

0 comments on commit 13686a7

Please sign in to comment.