fix3: resolve review comments

Icinga · Oct 1, 2024 · 03f0627 · 03f0627
1 parent d824e9a
commit 03f0627
Show file tree

Hide file tree

Showing 9 changed files with 38 additions and 33 deletions.
diff --git a/library/Icingadb/Common/IcingaRedis.php b/library/Icingadb/Common/IcingaRedis.php
@@ -8,7 +8,6 @@
 use Generator;
 use Icinga\Application\Config;
 use Icinga\Application\Logger;
-use ipl\Sql\Expression;
 use Predis\Client as Redis;
 
 class IcingaRedis
@@ -164,16 +163,7 @@ protected static function fetchState(string $key, array $ids, array $columns): G
         foreach ($results as $i => $json) {
             if ($json !== null) {
                 $data = json_decode($json, true);
-                $keyMap = [];
-
-                foreach ($columns as $alias => $column) {
-                    if ($column instanceof Expression) {
-                        $column = $alias;
-                    }
-
-                    $keyMap[] = $column;
-                }
-
+                $keyMap = array_fill_keys($columns, null);
                 unset($keyMap['is_overdue']); // Is calculated by Icinga DB, not Icinga 2, hence it's never in redis
 
                 // TODO: Remove once https://github.com/Icinga/icinga2/issues/9427 is fixed

diff --git a/library/Icingadb/Common/StateBadges.php b/library/Icingadb/Common/StateBadges.php
@@ -7,6 +7,7 @@
 use InvalidArgumentException;
 use ipl\Html\BaseHtmlElement;
 use ipl\Html\Html;
+use ipl\Html\HtmlElement;
 use ipl\Stdlib\BaseFilter;
 use ipl\Stdlib\Filter;
 use ipl\Web\Url;
@@ -157,7 +158,7 @@ protected function createBadge(string $state): ?BaseHtmlElement
                 );
             }
 
-            return Html::tag('li', $stateBadge);
+            return new HtmlElement('li', null, $stateBadge);
         }
 
         return null;
@@ -190,7 +191,7 @@ protected function createGroup(string $state): ?BaseHtmlElement
                 );
             }
 
-            $content[] = Html::tag('li', $unhandledStateBadge);
+            $content[] = new HtmlElement('li', null, $unhandledStateBadge);
         }
 
         if (isset($this->item->$handledKey) && $this->item->$handledKey) {
@@ -209,7 +210,7 @@ protected function createGroup(string $state): ?BaseHtmlElement
                 );
             }
 
-            $content[] = Html::tag('li', $handledStateBadge);
+            $content[] = new HtmlElement('li', null, $handledStateBadge);
         }
 
         if (empty($content)) {

diff --git a/library/Icingadb/Model/RedundancyGroupParentStateSummary.php b/library/Icingadb/Model/RedundancyGroupParentStateSummary.php
@@ -4,10 +4,13 @@
 
 namespace Icinga\Module\Icingadb\Model;
 
+use ipl\Orm\Query;
 use ipl\Sql\Connection;
 use ipl\Sql\Expression;
 
 /**
+ * Redundancy group's parent nodes summary
+ *
  * @property int $objects_down_critical_handled
  * @property int $objects_down_critical_unhandled
  * @property int $objects_pending
@@ -21,7 +24,7 @@
  */
 class RedundancyGroupParentStateSummary extends RedundancyGroup
 {
-    public function getSummaryColumns()
+    public function getSummaryColumns(): array
     {
         return [
             'objects_problem_handled' => new Expression(
@@ -81,7 +84,7 @@ public function getSummaryColumns()
         ];
     }
 
-    public static function on(Connection $db)
+    public static function on(Connection $db): Query
     {
         $q = parent::on($db)->with([
             'from',
@@ -102,9 +105,4 @@ public function getColumns(): array
     {
         return array_merge(parent::getColumns(), $this->getSummaryColumns());
     }
-
-    public function getDefaultSort()
-    {
-        return null;
-    }
 }
diff --git a/library/Icingadb/Widget/Detail/ObjectDetail.php b/library/Icingadb/Widget/Detail/ObjectDetail.php
@@ -628,10 +628,19 @@ 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,
+                'redundancy_group.state.last_state_change'  => SORT_DESC
             ]);
 
         $this->applyRestrictions($rootProblems);
 
-        return [HtmlElement::create('h2', null, t('Root Problems')), new DependencyNodeList($rootProblems)];
+        return [
+            HtmlElement::create('h2', null, Text::create(t('Root Problems'))),
+            new DependencyNodeList($rootProblems)
+        ];
     }
 }
diff --git a/library/Icingadb/Widget/ItemList/DependencyNodeList.php b/library/Icingadb/Widget/ItemList/DependencyNodeList.php
@@ -6,7 +6,11 @@
 
 use Icinga\Module\Icingadb\Model\DependencyNode;
 use Icinga\Module\Icingadb\Model\UnreachableParent;
+use ipl\Web\Common\BaseListItem;
 
+/**
+ * Dependency node list
+ */
 class DependencyNodeList extends StateList
 {
     protected $defaultAttributes = ['class' => ['root-problem-list']];
@@ -21,7 +25,7 @@ protected function getItemClass(): string
         return '';
     }
 
-    protected function createListItem(object $data)
+    protected function createListItem(object $data): BaseListItem
     {
         /** @var UnreachableParent|DependencyNode $data */
         if ($data->redundancy_group_id !== null) {

diff --git a/library/Icingadb/Widget/ItemList/RedundancyGroupListItem.php b/library/Icingadb/Widget/ItemList/RedundancyGroupListItem.php
@@ -56,10 +56,7 @@ protected function createSubject(): BaseHtmlElement
 
     protected function assembleVisual(BaseHtmlElement $visual): void
     {
-        $stateText = $this->state->failed ? 'problem' : 'ok';
-        $stateBall = new StateBall($stateText, $this->getStateBallSize());
-
-        $visual->addHtml($stateBall);
+        $visual->addHtml(new StateBall($this->item->state->getStateText(), $this->getStateBallSize()));
     }
 
     protected function assembleCaption(BaseHtmlElement $caption): void
@@ -111,7 +108,7 @@ protected function assembleCaption(BaseHtmlElement $caption): void
             ->orderBy([
                 'objects_severity',
                 'objects_last_state_change',
-            ], 'DESC');
+            ], SORT_DESC);
 
         $this->applyRestrictions($members);
 
@@ -135,13 +132,13 @@ protected function assembleTitle(BaseHtmlElement $title): void
             $stateTextElement = Html::sprintf(
                 t('%s has %s', '<hostname> has <state-text>'),
                 $subject,
-                Html::tag('span', ['class' => 'state-text'], 'FAILED')
+                new HtmlElement('span', Attributes::create(['class' => 'state-text']), Text::create('FAILED'))
             );
         } else {
             $stateTextElement = Html::sprintf(
                 t('%s is %s', '<hostname> is <state-text>'),
                 $subject,
-                Html::tag('span', ['class' => 'state-text'], 'OK')
+                new HtmlElement('span', Attributes::create(['class' => 'state-text']), Text::create('OK'))
             );
         }
 

diff --git a/library/Icingadb/Widget/ObjectsStateBadges.php b/library/Icingadb/Widget/ObjectsStateBadges.php
@@ -6,19 +6,22 @@
 
 use Icinga\Module\Icingadb\Common\StateBadges;
 
+/**
+ * State badges for the objects
+ */
 class ObjectsStateBadges extends StateBadges
 {
     protected function getType(): string
     {
-        return 'redundancy_group_objects';
+        return 'objects';
     }
 
     protected function getPrefix(): string
     {
         return 'objects';
     }
 
-    protected function assemble()
+    protected function assemble(): void
     {
         $this->addAttributes(['class' => 'objects-state-badges']);
 

diff --git a/library/Icingadb/Widget/ObjectsStatistics.php b/library/Icingadb/Widget/ObjectsStatistics.php
@@ -11,6 +11,9 @@
 use ipl\Html\ValidHtml;
 use ipl\Html\HtmlString;
 
+/**
+ * Objects statistics
+ */
 class ObjectsStatistics extends ObjectStatistics
 {
     protected $summary;

diff --git a/public/css/list/redundancy-group-list-item.less b/public/css/list/redundancy-group-list-item.less
@@ -22,4 +22,4 @@
   .state-problem {
     background-color: @state-critical;
   }
-}
+}