Improve handling of ambiguous columns
authorMarcel Werk <burntime@woltlab.com>
Thu, 12 Dec 2024 16:09:40 +0000 (17:09 +0100)
committerMarcel Werk <burntime@woltlab.com>
Thu, 12 Dec 2024 16:09:40 +0000 (17:09 +0100)
16 files changed:
wcfsetup/install/files/lib/data/acp/session/log/ACPSessionLogList.class.php
wcfsetup/install/files/lib/system/gridView/ACPSessionLogGridView.class.php
wcfsetup/install/files/lib/system/gridView/AbstractGridView.class.php
wcfsetup/install/files/lib/system/gridView/DatabaseObjectListGridView.class.php
wcfsetup/install/files/lib/system/gridView/GridViewColumn.class.php
wcfsetup/install/files/lib/system/gridView/ModificationLogGridView.class.php
wcfsetup/install/files/lib/system/gridView/UserRankGridView.class.php
wcfsetup/install/files/lib/system/gridView/filter/AbstractFilter.class.php [new file with mode: 0644]
wcfsetup/install/files/lib/system/gridView/filter/BooleanFilter.class.php [new file with mode: 0644]
wcfsetup/install/files/lib/system/gridView/filter/I18nTextFilter.class.php
wcfsetup/install/files/lib/system/gridView/filter/IpAddressFilter.class.php
wcfsetup/install/files/lib/system/gridView/filter/ObjectIdFilter.class.php
wcfsetup/install/files/lib/system/gridView/filter/SelectFilter.class.php
wcfsetup/install/files/lib/system/gridView/filter/TextFilter.class.php
wcfsetup/install/files/lib/system/gridView/filter/TimeFilter.class.php
wcfsetup/install/files/lib/system/gridView/filter/UserFilter.class.php

index 32658ec85b36bc2a48328d491ad43ba0e41e6473..29b769a88c01dbe6ebe309817214eb8c9150caf1 100644 (file)
@@ -23,28 +23,4 @@ class ACPSessionLogList extends DatabaseObjectList
      * @inheritDoc
      */
     public $className = ACPSessionLog::class;
-
-    /**
-     * @inheritDoc
-     */
-    public function readObjects()
-    {
-        if (!empty($this->sqlSelects)) {
-            $this->sqlSelects .= ',';
-        }
-        $this->sqlSelects .= "
-            (
-                SELECT  username
-                FROM    wcf1_user
-                WHERE   userID = " . $this->getDatabaseTableAlias() . ".userID
-            ) AS username,
-            0 AS active,
-            (
-                SELECT  COUNT(*)
-                FROM    wcf1_acp_session_access_log
-                WHERE   sessionLogID = " . $this->getDatabaseTableAlias() . ".sessionLogID
-            ) AS accesses";
-
-        parent::readObjects();
-    }
 }
index b0cb7e55926a74704bfb25b61c8a038f2e8a0287..d59bf8bf9c8750dd489fe1a1fbe837ee243b6287 100644 (file)
@@ -2,6 +2,7 @@
 
 namespace wcf\system\gridView;
 
+use wcf\acp\form\UserEditForm;
 use wcf\acp\page\ACPSessionLogPage;
 use wcf\data\acp\session\log\ACPSessionLogList;
 use wcf\data\DatabaseObjectList;
@@ -15,7 +16,7 @@ use wcf\system\gridView\renderer\IpAddressColumnRenderer;
 use wcf\system\gridView\renderer\NumberColumnRenderer;
 use wcf\system\gridView\renderer\TimeColumnRenderer;
 use wcf\system\gridView\renderer\TruncatedTextColumnRenderer;
-use wcf\system\gridView\renderer\UserColumnRenderer;
+use wcf\system\gridView\renderer\UserLinkColumnRenderer;
 use wcf\system\WCF;
 
 /**
@@ -37,9 +38,8 @@ final class ACPSessionLogGridView extends DatabaseObjectListGridView
                 ->sortable(),
             GridViewColumn::for('userID')
                 ->label('wcf.user.username')
-                ->sortable()
-                ->sortById('username')
-                ->renderer(new UserColumnRenderer())
+                ->sortable(true, 'user_table.username')
+                ->renderer(new UserLinkColumnRenderer(UserEditForm::class))
                 ->filter(new UserFilter()),
             GridViewColumn::for('ipAddress')
                 ->label('wcf.user.ipAddress')
@@ -50,7 +50,7 @@ final class ACPSessionLogGridView extends DatabaseObjectListGridView
                 ->label('wcf.user.userAgent')
                 ->sortable()
                 ->valueEncoding(false)
-                ->renderer(new TruncatedTextColumnRenderer())
+                ->renderer(new TruncatedTextColumnRenderer(50))
                 ->filter(new TextFilter()),
             GridViewColumn::for('time')
                 ->label('wcf.acp.sessionLog.time')
@@ -64,7 +64,7 @@ final class ACPSessionLogGridView extends DatabaseObjectListGridView
                 ->filter(new TimeFilter()),
             GridViewColumn::for('accesses')
                 ->label('wcf.acp.sessionLog.actions')
-                ->sortable()
+                ->sortable(true, 'accesses')
                 ->renderer(new NumberColumnRenderer()),
         ]);
 
@@ -82,7 +82,18 @@ final class ACPSessionLogGridView extends DatabaseObjectListGridView
     #[\Override]
     protected function createObjectList(): DatabaseObjectList
     {
-        return new ACPSessionLogList();
+        $list = new ACPSessionLogList();
+        $list->sqlSelects .= "
+            user_table.username,
+            0 AS active,
+            (
+                SELECT  COUNT(*)
+                FROM    wcf1_acp_session_access_log
+                WHERE   sessionLogID = " . $list->getDatabaseTableAlias() . ".sessionLogID
+            ) AS accesses";
+        $list->sqlJoins = $list->sqlConditionJoins .= " JOIN wcf" . WCF_N . "_user user_table ON (user_table.userID = " . $list->getDatabaseTableAlias() . ".userID)";
+
+        return $list;
     }
 
     #[\Override]
index 8e4da21f5eb440e4ec43cab36e815fc29702de72..fcd41a4d21a4d0cc1f4f9e2fe4c9380cad498158 100644 (file)
@@ -458,7 +458,9 @@ abstract class AbstractGridView
             throw new LogicException("No value for filter '" . $id . "' found.");
         }
 
-        return $column->getLabel() . ': ' . $column->getFilter()->renderValue($this->activeFilters[$id]);
+        $value = $column->getFilter()->renderValue($this->activeFilters[$id]);
+
+        return $column->getLabel() . ($value !== '' ? ': ' . $value : '');
     }
 
     /**
index ff013025ea194a653dd7e317a1274c0bbe787349..448668ea3ef25139171566f58f4c1dc8d362c4aa 100644 (file)
@@ -18,12 +18,15 @@ abstract class DatabaseObjectListGridView extends AbstractGridView
 {
     protected DatabaseObjectList $objectList;
     private int $objectCount;
+    public int $counter = 0;
 
     #[\Override]
     public function getRows(): array
     {
         $this->getObjectList()->readObjects();
 
+        $this->counter++;
+
         return $this->getObjectList()->getObjects();
     }
 
@@ -55,11 +58,15 @@ abstract class DatabaseObjectListGridView extends AbstractGridView
         $this->objectList->sqlOffset = ($this->getPageNo() - 1) * $this->getRowsPerPage();
         if ($this->getSortField()) {
             $column = $this->getColumn($this->getSortField());
-            if ($column && $column->getSortById()) {
-                $this->objectList->sqlOrderBy = $column->getSortById() . ' ' . $this->getSortOrder();
+            if ($column && $column->getSortByDatabaseColumn()) {
+                $this->objectList->sqlOrderBy = $column->getSortByDatabaseColumn() . ' ' . $this->getSortOrder();
             } else {
-                $this->objectList->sqlOrderBy = $this->getSortField() . ' ' . $this->getSortOrder();
+                $this->objectList->sqlOrderBy = $this->objectList->getDatabaseTableAlias() .
+                    '.' . $this->getSortField() . ' ' . $this->getSortOrder();
             }
+
+            $this->objectList->sqlOrderBy .= ',' . $this->objectList->getDatabaseTableAlias() .
+                '.' . $this->objectList->getDatabaseTableIndexName() . ' ' . $this->getSortOrder();
         }
         $this->applyFilters();
         $this->fireInitializedEvent();
index 1d1d44331d7250424f67f5207c652bfdecbfd0b1..b80d13aa5aa54fa6da7cbdf50b32e74ac2874b1d 100644 (file)
@@ -27,7 +27,7 @@ final class GridViewColumn
     private string $label = '';
     private static DefaultColumnRenderer $defaultRenderer;
     private bool $sortable = false;
-    private string $sortById = '';
+    private string $sortByDatabaseColumn = '';
     private ?IGridViewFilter $filter = null;
     private bool $hidden = false;
     private bool $valueEncoding = true;
@@ -105,19 +105,10 @@ final class GridViewColumn
     /**
      * Sets the sortable state of this column.
      */
-    public function sortable(bool $sortable = true): static
+    public function sortable(bool $sortable = true, string $sortByDatabaseColumn = ''): static
     {
         $this->sortable = $sortable;
-
-        return $this;
-    }
-
-    /**
-     * Defines the ID by which this column is to be sorted.
-     */
-    public function sortById(string $id): static
-    {
-        $this->sortById = $id;
+        $this->sortByDatabaseColumn = $sortByDatabaseColumn;
 
         return $this;
     }
@@ -156,11 +147,11 @@ final class GridViewColumn
     }
 
     /**
-     * Returns the ID by which this column is to be sorted.
+     * Returns the name of the database column by which this column is to be sorted.
      */
-    public function getSortById(): string
+    public function getSortByDatabaseColumn(): string
     {
-        return $this->sortById;
+        return $this->sortByDatabaseColumn;
     }
 
     /**
index 82e1227c58f477bfb9157ac24f3c2697e8ab4afc..e4fc88089e9795ac72e89e63bb068be772f07943 100644 (file)
@@ -51,10 +51,9 @@ final class ModificationLogGridView extends DatabaseObjectListGridView
                 ->sortable(),
             GridViewColumn::for('userID')
                 ->label('wcf.user.username')
-                ->sortable()
-                ->sortById('username')
+                ->sortable(true, 'modification_log.username')
                 ->renderer(new UserLinkColumnRenderer())
-                ->filter(new TextFilter('username')),
+                ->filter(new TextFilter('modification_log.username')),
             GridViewColumn::for('action')
                 ->label('wcf.acp.modificationLog.action')
                 ->renderer([
index 11ea17e9eb7a4a543b565cc249d7b87e23fff02d..a3e62564394202fff389fba308954561f1c36df5 100644 (file)
@@ -38,8 +38,7 @@ final class UserRankGridView extends DatabaseObjectListGridView
                 ->sortable(),
             GridViewColumn::for('rankTitle')
                 ->label('wcf.acp.user.rank.title')
-                ->sortable()
-                ->sortById('rankTitleI18n')
+                ->sortable(true, 'rankTitleI18n')
                 ->filter(new I18nTextFilter())
                 ->renderer([
                     new class extends TitleColumnRenderer {
diff --git a/wcfsetup/install/files/lib/system/gridView/filter/AbstractFilter.class.php b/wcfsetup/install/files/lib/system/gridView/filter/AbstractFilter.class.php
new file mode 100644 (file)
index 0000000..417efd2
--- /dev/null
@@ -0,0 +1,29 @@
+<?php
+
+namespace wcf\system\gridView\filter;
+
+use wcf\data\DatabaseObjectList;
+
+/**
+ * Abstract implementation for grid view column filters.
+ *
+ * @author      Marcel Werk
+ * @copyright   2001-2024 WoltLab GmbH
+ * @license     GNU Lesser General Public License <http://opensource.org/licenses/lgpl-license.php>
+ * @since       6.2
+ */
+abstract class AbstractFilter implements IGridViewFilter
+{
+    public function __construct(protected readonly string $databaseColumn = '') {}
+
+    #[\Override]
+    public function renderValue(string $value): string
+    {
+        return $value;
+    }
+
+    protected function getDatabaseColumnName(DatabaseObjectList $list, string $id): string
+    {
+        return ($this->databaseColumn ?: $list->getDatabaseTableAlias() . '.' . $id);
+    }
+}
diff --git a/wcfsetup/install/files/lib/system/gridView/filter/BooleanFilter.class.php b/wcfsetup/install/files/lib/system/gridView/filter/BooleanFilter.class.php
new file mode 100644 (file)
index 0000000..4d039fd
--- /dev/null
@@ -0,0 +1,46 @@
+<?php
+
+namespace wcf\system\gridView\filter;
+
+use wcf\data\DatabaseObjectList;
+use wcf\system\form\builder\field\AbstractFormField;
+use wcf\system\form\builder\field\CheckboxFormField;
+
+/**
+ * Filter for boolean columns.
+ *
+ * @author      Marcel Werk
+ * @copyright   2001-2024 WoltLab GmbH
+ * @license     GNU Lesser General Public License <http://opensource.org/licenses/lgpl-license.php>
+ * @since       6.2
+ */
+class BooleanFilter extends AbstractFilter
+{
+    #[\Override]
+    public function getFormField(string $id, string $label): AbstractFormField
+    {
+        return CheckboxFormField::create($id)
+            ->label($label)
+            ->nullable();
+    }
+
+    #[\Override]
+    public function applyFilter(DatabaseObjectList $list, string $id, string $value): void
+    {
+        $columnName = $this->getDatabaseColumnName($list, $id);
+
+        $list->getConditionBuilder()->add("{$columnName} = ?", [1]);
+    }
+
+    #[\Override]
+    public function matches(string $filterValue, string $rowValue): bool
+    {
+        return $rowValue == 1;
+    }
+
+    #[\Override]
+    public function renderValue(string $value): string
+    {
+        return '';
+    }
+}
index 36d78e4f3caabb9c7a586c827d311c25c5f290eb..861d15fc0137701b77137f2c954f5612590a092b 100644 (file)
@@ -18,7 +18,9 @@ class I18nTextFilter extends TextFilter
     #[\Override]
     public function applyFilter(DatabaseObjectList $list, string $id, string $value): void
     {
-        $list->getConditionBuilder()->add("($id LIKE ? OR $id IN (SELECT languageItem FROM wcf1_language_item WHERE languageID = ? AND languageItemValue LIKE ?))", [
+        $columnName = $this->getDatabaseColumnName($list, $id);
+
+        $list->getConditionBuilder()->add("({$columnName} LIKE ? OR {$columnName} IN (SELECT languageItem FROM wcf1_language_item WHERE languageID = ? AND languageItemValue LIKE ?))", [
             '%' . WCF::getDB()->escapeLikeValue($value) . '%',
             WCF::getLanguage()->languageID,
             '%' . WCF::getDB()->escapeLikeValue($value) . '%'
index 8669f7a41c79782ae14b479e3e41fa508afdc7b3..c3ed8c06951c80aecc0299d47698efb8a9b90d8a 100644 (file)
@@ -16,7 +16,7 @@ use wcf\util\UserUtil;
  * @license     GNU Lesser General Public License <http://opensource.org/licenses/lgpl-license.php>
  * @since       6.2
  */
-class IpAddressFilter implements IGridViewFilter
+class IpAddressFilter extends AbstractFilter
 {
     #[\Override]
     public function getFormField(string $id, string $label): AbstractFormField
@@ -28,8 +28,10 @@ class IpAddressFilter implements IGridViewFilter
     #[\Override]
     public function applyFilter(DatabaseObjectList $list, string $id, string $value): void
     {
+        $columnName = $this->getDatabaseColumnName($list, $id);
+
         $list->getConditionBuilder()->add(
-            "{$id} LIKE ?",
+            "{$columnName} LIKE ?",
             ['%' . WCF::getDB()->escapeLikeValue($this->convertIPv4To6($value)) . '%']
         );
     }
@@ -40,12 +42,6 @@ class IpAddressFilter implements IGridViewFilter
         return \str_contains($rowValue, $this->convertIPv4To6($filterValue));
     }
 
-    #[\Override]
-    public function renderValue(string $value): string
-    {
-        return $value;
-    }
-
     private function convertIPv4To6(string $value): string
     {
         return UserUtil::convertIPv4To6($value);
index 501026b20b6ff11ae5702c88cce35f180ad761e4..9d0dfca77c0c6df0fd33f6ff59f750a3f002451d 100644 (file)
@@ -14,7 +14,7 @@ use wcf\system\form\builder\field\IntegerFormField;
  * @license     GNU Lesser General Public License <http://opensource.org/licenses/lgpl-license.php>
  * @since       6.2
  */
-class ObjectIdFilter implements IGridViewFilter
+class ObjectIdFilter extends AbstractFilter
 {
     #[\Override]
     public function getFormField(string $id, string $label): AbstractFormField
@@ -28,7 +28,9 @@ class ObjectIdFilter implements IGridViewFilter
     #[\Override]
     public function applyFilter(DatabaseObjectList $list, string $id, string $value): void
     {
-        $list->getConditionBuilder()->add("$id = ?", [$value]);
+        $columnName = $this->getDatabaseColumnName($list, $id);
+
+        $list->getConditionBuilder()->add("{$columnName} = ?", [$value]);
     }
 
     #[\Override]
@@ -36,10 +38,4 @@ class ObjectIdFilter implements IGridViewFilter
     {
         return $filterValue == $rowValue;
     }
-
-    #[\Override]
-    public function renderValue(string $value): string
-    {
-        return $value;
-    }
 }
index b6b6179bd623a21110aa0b87a6f1a25d96187e90..532c717646d400cb7f14ca26ca1b53439791ad99 100644 (file)
@@ -15,9 +15,12 @@ use wcf\system\WCF;
  * @license     GNU Lesser General Public License <http://opensource.org/licenses/lgpl-license.php>
  * @since       6.2
  */
-class SelectFilter implements IGridViewFilter
+class SelectFilter extends AbstractFilter
 {
-    public function __construct(private readonly array $options) {}
+    public function __construct(private readonly array $options, string $databaseColumn = '')
+    {
+        parent::__construct($databaseColumn);
+    }
 
     #[\Override]
     public function getFormField(string $id, string $label): AbstractFormField
@@ -30,7 +33,9 @@ class SelectFilter implements IGridViewFilter
     #[\Override]
     public function applyFilter(DatabaseObjectList $list, string $id, string $value): void
     {
-        $list->getConditionBuilder()->add("$id = ?", [$value]);
+        $columnName = $this->getDatabaseColumnName($list, $id);
+
+        $list->getConditionBuilder()->add("{$columnName} = ?", [$value]);
     }
 
     #[\Override]
index 16505597192dec57afb5c3ecdb448cf3cea64f29..35f1e40a8dc5bcce28ac22b2d307414977ca8899 100644 (file)
@@ -15,10 +15,8 @@ use wcf\system\WCF;
  * @license     GNU Lesser General Public License <http://opensource.org/licenses/lgpl-license.php>
  * @since       6.2
  */
-class TextFilter implements IGridViewFilter
+class TextFilter extends AbstractFilter
 {
-    public function __construct(private readonly string $columnName = '') {}
-
     #[\Override]
     public function getFormField(string $id, string $label): AbstractFormField
     {
@@ -29,8 +27,10 @@ class TextFilter implements IGridViewFilter
     #[\Override]
     public function applyFilter(DatabaseObjectList $list, string $id, string $value): void
     {
+        $columnName = $this->getDatabaseColumnName($list, $id);
+
         $list->getConditionBuilder()->add(
-            ($this->columnName ?: $id) . " LIKE ?",
+            "{$columnName} LIKE ?",
             ['%' . WCF::getDB()->escapeLikeValue($value) . '%']
         );
     }
@@ -40,10 +40,4 @@ class TextFilter implements IGridViewFilter
     {
         return \str_contains(\mb_strtolower($rowValue), \mb_strtolower($filterValue));
     }
-
-    #[\Override]
-    public function renderValue(string $value): string
-    {
-        return $value;
-    }
 }
index 0aa876635f7514e9829fa885a6c08449c34f249c..238d68727aa488ba9c6dbfb9fa73075505b69f74 100644 (file)
@@ -15,7 +15,7 @@ use wcf\system\WCF;
  * @license     GNU Lesser General Public License <http://opensource.org/licenses/lgpl-license.php>
  * @since       6.2
  */
-class TimeFilter implements IGridViewFilter
+class TimeFilter extends AbstractFilter
 {
     #[\Override]
     public function getFormField(string $id, string $label): AbstractFormField
@@ -29,6 +29,7 @@ class TimeFilter implements IGridViewFilter
     #[\Override]
     public function applyFilter(DatabaseObjectList $list, string $id, string $value): void
     {
+        $columnName = $this->getDatabaseColumnName($list, $id);
         $timestamps = $this->getTimestamps($value);
 
         if (!$timestamps['from'] && !$timestamps['to']) {
@@ -36,9 +37,9 @@ class TimeFilter implements IGridViewFilter
         }
 
         if (!$timestamps['to']) {
-            $list->getConditionBuilder()->add("$id >= ?", [$timestamps['from']]);
+            $list->getConditionBuilder()->add("{$columnName} >= ?", [$timestamps['from']]);
         } else {
-            $list->getConditionBuilder()->add("$id BETWEEN ? AND ?", [$timestamps['from'], $timestamps['to']]);
+            $list->getConditionBuilder()->add("{$columnName} BETWEEN ? AND ?", [$timestamps['from'], $timestamps['to']]);
         }
     }
 
index 48d972f729975819dc7b4ad81e896962f762dacf..98d66f1f9dfa95f245f323b7c907f0878f3dbdc9 100644 (file)
@@ -15,7 +15,7 @@ use wcf\system\form\builder\field\user\UserFormField;
  * @license     GNU Lesser General Public License <http://opensource.org/licenses/lgpl-license.php>
  * @since       6.2
  */
-class UserFilter implements IGridViewFilter
+class UserFilter extends AbstractFilter
 {
     #[\Override]
     public function getFormField(string $id, string $label): AbstractFormField
@@ -28,7 +28,9 @@ class UserFilter implements IGridViewFilter
     #[\Override]
     public function applyFilter(DatabaseObjectList $list, string $id, string $value): void
     {
-        $list->getConditionBuilder()->add("{$id} = ?", [$value]);
+        $columnName = $this->getDatabaseColumnName($list, $id);
+
+        $list->getConditionBuilder()->add("{$columnName} = ?", [$value]);
     }
 
     #[\Override]