fix: resolve review comments

Icinga · Oct 23, 2024 · 2450ef3 · 2450ef3
1 parent 2cd6c4a
commit 2450ef3
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 40 deletions.
diff --git 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,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);
         });
 

diff --git 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;
@@ -608,8 +609,8 @@ protected function fetchCustomVars()
     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)
@@ -618,6 +619,7 @@ protected function createRootProblems(): ?array
         }
 
         $rootProblems = UnreachableParent::on($this->getDb(), $this->object)
+            ->setResultSetClass(VolatileStateResults::class)
             ->with([
                 'redundancy_group',
                 'redundancy_group.state',
@@ -636,8 +638,6 @@ protected function createRootProblems(): ?array
                 '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
             ]);
 

diff --git 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
@@ -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
@@ -1,3 +1,9 @@
 .dependency-node-state-badges {
   .state-badges();
+
+  .state-badge  {
+    &.state-problem {
+      background-color: @color-critical;
+    }
+  }
 }