refactor: eliminate data array with object

Author: thorsten

phpmyfaq/admin/stat.adminlog.php

@@ -17,19 +17,24 @@
 
 use phpMyFAQ\Administration\AdminLog;
 use phpMyFAQ\Component\Alert;
+use phpMyFAQ\Configuration;
 use phpMyFAQ\Date;
 use phpMyFAQ\Enums\PermissionType;
 use phpMyFAQ\Filter;
 use phpMyFAQ\Pagination;
 use phpMyFAQ\Session\Token;
 use phpMyFAQ\Strings;
 use phpMyFAQ\Translation;
+use phpMyFAQ\User\CurrentUser;
 
 if (!defined('IS_VALID_PHPMYFAQ')) {
     http_response_code(400);
     exit();
 }
 
+$faqConfig = Configuration::getConfigurationInstance();
+$user = CurrentUser::getCurrentUser($faqConfig);
+
 $logging = new AdminLog($faqConfig);
 $csrfToken = Filter::filterInput(INPUT_GET, 'csrf', FILTER_SANITIZE_SPECIAL_CHARS);
 
@@ -44,16 +49,16 @@
     'adminlog' === $action
 ) {
     $date = new Date($faqConfig);
-    $perpage = 15;
+    $perPage = 15;
     $pages = Filter::filterInput(INPUT_GET, 'pages', FILTER_VALIDATE_INT);
     $page = Filter::filterInput(INPUT_GET, 'page', FILTER_VALIDATE_INT, 1);
 
     if (is_null($pages)) {
-        $pages = round(($logging->getNumberOfEntries() + ($perpage / 3)) / $perpage, 0);
+        $pages = round(($logging->getNumberOfEntries() + ($perPage / 3)) / $perPage, 0);
     }
 
-    $start = ($page - 1) * $perpage;
-    $lastPage = $start + $perpage;
+    $start = ($page - 1) * $perPage;
+    $lastPage = $start + $perPage;
 
     $baseUrl = sprintf(
         '%sadmin/?action=adminlog&page=%d',
@@ -65,15 +70,16 @@
     $options = [
         'baseUrl' => $baseUrl,
         'total' => $logging->getNumberOfEntries(),
-        'perPage' => $perpage,
+        'perPage' => $perPage,
         'pageParamName' => 'page',
     ];
     $pagination = new Pagination($options);
 
     $loggingData = $logging->getAll();
     ?>
 
-    <div class="d-flex justify-content-between flex-wrap flex-md-nowrap align-items-center pt-3 pb-2 mb-3 border-bottom">
+    <div
+        class="d-flex justify-content-between flex-wrap flex-md-nowrap align-items-center pt-3 pb-2 mb-3 border-bottom">
         <h1 class="h2">
             <i aria-hidden="true" class="bi bi-list-ol"></i> <?= Translation::get('ad_menu_adminlog') ?>
         </h1>
@@ -105,8 +111,8 @@
     <?php
     $counter = $displayedCounter = 0;
 
-    foreach ($loggingData as $loggingId => $loggingValue) {
-        if ($displayedCounter >= $perpage) {
+    foreach ($loggingData as $value) {
+        if ($displayedCounter >= $perPage) {
             ++$displayedCounter;
             continue;
         }
@@ -117,15 +123,15 @@
         }
         ++$displayedCounter;
 
-        $user->getUserById($loggingValue['usr'], true);
+        $user->getUserById($value->getUserId(), true);
         ?>
         <tr>
-            <td><?= $loggingId ?></td>
-            <td><?= $date->format(date('Y-m-d H:i', $loggingValue['time'])) ?></td>
+            <td><?= $value->getId() ?></td>
+            <td><?= $date->format(date('Y-m-d H:i', $value->getTime())) ?></td>
             <td><?= Strings::htmlentities($user->getLogin()) ?></td>
-            <td><?= $loggingValue['ip'] ?></td>
+            <td><?= $value->getIp() ?></td>
             <td><small><?php
-            $text = Strings::htmlentities($loggingValue['text']);
+            $text = Strings::htmlentities($value->getText());
             $text = str_replace('Loginerror', Translation::get('ad_log_lger'), $text);
             $text = str_replace('Session expired', Translation::get('ad_log_sess'), $text);
             $text = str_replace('Useredit', Translation::get('ad_log_edit'), $text);

phpmyfaq/src/phpMyFAQ/Administration/AdminLog.php

@@ -20,6 +20,7 @@
 use phpMyFAQ\Configuration;
 use phpMyFAQ\Database;
 use phpMyFAQ\User;
+use phpMyFAQ\Entity\AdminLog as AdminLogEntity;
 use Symfony\Component\HttpFoundation\Request;
 
 /**
@@ -50,6 +51,7 @@ public function getNumberOfEntries(): int
 
     /**
      * Returns all data from the admin log.
+     * @return AdminLogEntity[]
      */
     public function getAll(): array
     {
@@ -68,7 +70,14 @@ public function getAll(): array
         $result = $this->configuration->getDb()->query($query);
 
         while ($row = $this->configuration->getDb()->fetchObject($result)) {
-            $data[$row->id] = ['time' => $row->time, 'usr' => $row->usr, 'text' => $row->text, 'ip' => $row->ip];
+            $adminLog = new AdminLogEntity();
+            $adminLog
+                ->setId($row->id)
+                ->setTime($row->time)
+                ->setUserId($row->usr)
+                ->setText($row->text)
+                ->setIp($row->ip);
+            $data[$row->id] = $adminLog;
         }
 
         return $data;

phpmyfaq/src/phpMyFAQ/Entity/AdminLog.php

@@ -0,0 +1,70 @@
+<?php
+
+namespace phpMyFAQ\Entity;
+
+class AdminLog
+{
+    private int $id;
+
+    private int $time;
+
+    private int $userId;
+
+    private string $text;
+
+    private string $ip;
+    public function getId(): int
+    {
+        return $this->id;
+    }
+
+    public function setId(int $id): AdminLog
+    {
+        $this->id = $id;
+        return $this;
+    }
+
+    public function getTime(): int
+    {
+        return $this->time;
+    }
+
+    public function setTime(int $time): AdminLog
+    {
+        $this->time = $time;
+        return $this;
+    }
+
+    public function getUserId(): int
+    {
+        return $this->userId;
+    }
+
+    public function setUserId(int $userId): AdminLog
+    {
+        $this->userId = $userId;
+        return $this;
+    }
+
+    public function getText(): string
+    {
+        return $this->text;
+    }
+
+    public function setText(string $text): AdminLog
+    {
+        $this->text = $text;
+        return $this;
+    }
+
+    public function getIp(): string
+    {
+        return $this->ip;
+    }
+
+    public function setIp(string $ip): AdminLog
+    {
+        $this->ip = $ip;
+        return $this;
+    }
+}

phpmyfaq/src/phpMyFAQ/Entity/CategoryEntity.php

@@ -52,7 +52,7 @@ public function getActive(): bool
     /**
      * @param bool $active
      */
-    public function setActive($active): CategoryEntity
+    public function setActive(bool $active): CategoryEntity
     {
         $this->active = $active;
 
@@ -127,7 +127,7 @@ public function getParentId(): int
     /**
      * @param int $parentId
      */
-    public function setParentId($parentId): CategoryEntity
+    public function setParentId(int $parentId): CategoryEntity
     {
         $this->parentId = $parentId;
 
@@ -142,7 +142,7 @@ public function getUserId(): int
     /**
      * @param int $userId
      */
-    public function setUserId($userId): CategoryEntity
+    public function setUserId(int $userId): CategoryEntity
     {
         $this->userId = $userId;
 

tests/phpMyFAQ/Administration/AdminLogTest.php

@@ -3,7 +3,9 @@
 namespace phpMyFAQ\Administration;
 
 use phpMyFAQ\Configuration;
+use phpMyFAQ\Core\Exception;
 use phpMyFAQ\Database\Sqlite3;
+use phpMyFAQ\Entity\AdminLog as AdminLogEntity;
 use phpMyFAQ\System;
 use phpMyFAQ\User;
 use PHPUnit\Framework\TestCase;
@@ -62,27 +64,34 @@ public function testGetNumberOfEntries(): void
         $this->assertEquals(2, $this->adminLog->getNumberOfEntries());
     }
 
+    /**
+     * @throws Exception
+     */
     public function testGetAll(): void
     {
         $_SERVER['REQUEST_TIME'] = $this->now;
         $this->adminLog->log(new User($this->configuration), 'foo');
         $this->adminLog->log(new User($this->configuration), 'bar');
 
         $result = $this->adminLog->getAll();
+
+        $adminLogEntity = new AdminLogEntity();
+        $one = $adminLogEntity->setId(1)
+            ->setTime($this->now)
+            ->setUserId(-1)
+            ->setText('foo')
+            ->setIp('127.0.0.1');
+        $adminLogEntity = new AdminLogEntity();
+        $two = $adminLogEntity->setId(2)
+            ->setTime($this->now)
+            ->setUserId(-1)
+            ->setText('bar')
+            ->setIp('127.0.0.1');
+
         $this->assertEquals(
             [
-                2 => [
-                    'time' => $this->now,
-                    'usr' => -1,
-                    'text' => 'bar',
-                    'ip' => '127.0.0.1'
-                ],
-                1 => [
-                    'time' => $this->now,
-                    'usr' => -1,
-                    'text' => 'foo',
-                    'ip' => '127.0.0.1'
-                ]
+                2 => $two,
+                1 => $one
             ],
             $result
         );