From 5f61e426dc90b7b697a46da009af2fc88eed3ad0 Mon Sep 17 00:00:00 2001 From: Alexandre Alapetite Date: Tue, 15 Jul 2025 12:39:51 +0200 Subject: Sort by category title, feed title (#7702) * Sort by category name, feed name fix https://github.com/FreshRSS/FreshRSS/issues/7698 Note that sorting is done with the default SQL collation for now, meaning that lower-case vs. upper-case and diacritics are influencing the sorting order. Improvements left for future work. Watch out that those sorting criteria are slower due to additional joins, additional requests, and poorer indexes. * i18n:pl Co-authored-by: Inverle * i18n: nl Co-authored-by: Frans de Jonge * Fix preserve sort --------- Co-authored-by: Inverle Co-authored-by: Frans de Jonge --- app/Models/Context.php | 4 +- app/Models/EntryDAO.php | 99 ++++++++++++++++++++++++++++------------ app/Models/UserConfiguration.php | 2 +- 3 files changed, 74 insertions(+), 31 deletions(-) (limited to 'app/Models') diff --git a/app/Models/Context.php b/app/Models/Context.php index 749e7e6ff..9a4e0192e 100644 --- a/app/Models/Context.php +++ b/app/Models/Context.php @@ -42,7 +42,7 @@ final class FreshRSS_Context { public static int $state = 0; /** @var 'ASC'|'DESC' */ public static string $order = 'DESC'; - /** @var 'id'|'date'|'link'|'title'|'rand' */ + /** @var 'id'|'c.name'|'date'|'f.name'|'link'|'title'|'rand' */ public static string $sort = 'id'; public static int $number = 0; public static int $offset = 0; @@ -237,7 +237,7 @@ final class FreshRSS_Context { $order = Minz_Request::paramString('order', true) ?: FreshRSS_Context::userConf()->sort_order; self::$order = in_array($order, ['ASC', 'DESC'], true) ? $order : 'DESC'; $sort = Minz_Request::paramString('sort', true) ?: FreshRSS_Context::userConf()->sort; - self::$sort = in_array($sort, ['id', 'date', 'link', 'title', 'rand'], true) ? $sort : 'id'; + self::$sort = in_array($sort, ['id', 'c.name', 'date', 'f.name', 'link', 'title', 'rand'], true) ? $sort : 'id'; self::$number = Minz_Request::paramInt('nb') ?: FreshRSS_Context::userConf()->posts_per_page; if (self::$number > FreshRSS_Context::userConf()->max_posts_per_rss) { self::$number = max( diff --git a/app/Models/EntryDAO.php b/app/Models/EntryDAO.php index aea6e6f65..8713ba930 100644 --- a/app/Models/EntryDAO.php +++ b/app/Models/EntryDAO.php @@ -1184,13 +1184,15 @@ SQL; /** * @param numeric-string $id_min * @param numeric-string $id_max - * @param 'id'|'date'|'link'|'title'|'rand' $sort + * @param 'id'|'c.name'|'date'|'f.name'|'link'|'title'|'rand' $sort * @param 'ASC'|'DESC' $order + * @param numeric-string $continuation_id + * @param list $continuation_values * @return array{0:list,1:string} */ protected function sqlListEntriesWhere(string $alias = '', int $state = FreshRSS_Entry::STATE_ALL, ?FreshRSS_BooleanSearch $filters = null, string $id_min = '0', string $id_max = '0', string $sort = 'id', string $order = 'DESC', - string $continuation_id = '0', string|int $continuation_value = 0): array { + string $continuation_id = '0', array $continuation_values = []): array { $search = ' '; $values = []; if ($state & FreshRSS_Entry::STATE_ANDS) { @@ -1250,13 +1252,29 @@ SQL; $values[] = $id_min; } - if ($continuation_id !== '0' && in_array($sort, ['date', 'link', 'title'], true)) { + if ($continuation_id !== '0' && in_array($sort, ['c.name', 'date', 'f.name', 'link', 'title'], true)) { $sign = $order === 'ASC' ? '>' : '<'; + $orderBy = match ($sort) { + 'c.name' => 'c.name', + 'f.name' => 'f.name', + default => $alias . $sort, + }; // Keyset pagination (Compatibility syntax due to poor performance of tuple syntax in MySQL https://bugs.mysql.com/bug.php?id=104128) - $search .= "AND ({$alias}{$sort} {$sign} ? OR ({$alias}{$sort} = ? AND {$alias}id {$sign}= ?)) "; - $values[] = $continuation_value; - $values[] = $continuation_value; - $values[] = $continuation_id; + if ($sort === 'c.name') { + // Includes a secondary sort by feed name + $search .= "AND ((c.name {$sign} ?) OR (c.name = ? AND f.name {$sign} ?) OR (c.name = ? AND f.name = ? AND {$alias}id {$sign}= ?)) "; + $values[] = $continuation_values[0]; + $values[] = $continuation_values[0]; + $values[] = $continuation_values[1]; + $values[] = $continuation_values[0]; + $values[] = $continuation_values[1]; + $values[] = $continuation_id; + } else { + $search .= "AND ({$orderBy} {$sign} ? OR ({$orderBy} = ? AND {$alias}id {$sign}= ?)) "; + $values[] = $continuation_values[0]; + $values[] = $continuation_values[0]; + $values[] = $continuation_id; + } } if ($filters !== null && count($filters->searches()) > 0) { @@ -1276,15 +1294,16 @@ SQL; * @param int $id category/feed/tag ID * @param numeric-string $id_min * @param numeric-string $id_max - * @param 'id'|'date'|'link'|'title'|'rand' $sort + * @param 'id'|'c.name'|'date'|'f.name'|'link'|'title'|'rand' $sort * @param 'ASC'|'DESC' $order * @param numeric-string $continuation_id + * @param list $continuation_values * @return array{0:list,1:string} * @throws FreshRSS_EntriesGetter_Exception */ private function sqlListWhere(string $type = 'a', int $id = 0, int $state = FreshRSS_Entry::STATE_ALL, ?FreshRSS_BooleanSearch $filters = null, string $id_min = '0', string $id_max = '0', string $sort = 'id', string $order = 'DESC', - string $continuation_id = '0', string|int $continuation_value = 0, int $limit = 1, int $offset = 0): array { + string $continuation_id = '0', array $continuation_values = [], int $limit = 1, int $offset = 0): array { if (!$state) { $state = FreshRSS_Entry::STATE_ALL; } @@ -1334,21 +1353,28 @@ SQL; } $order = in_array($order, ['ASC', 'DESC'], true) ? $order : 'DESC'; - $sort = in_array($sort, ['id', 'date', 'link', 'title', 'rand'], true) ? $sort : 'id'; - $orderBy = ($sort === 'rand' ? static::sqlRandom() : 'e.' . $sort); + $sort = in_array($sort, ['id', 'c.name', 'date', 'f.name', 'link', 'title', 'rand'], true) ? $sort : 'id'; + $orderBy = match ($sort) { + 'c.name' => 'c.name', + 'f.name' => 'f.name', + 'rand' => static::sqlRandom(), + default => 'e.' . $sort, + }; [$searchValues, $search] = $this->sqlListEntriesWhere(alias: 'e.', state: $state, filters: $filters, id_min: $id_min, id_max: $id_max, - sort: $sort, order: $order, continuation_id: $continuation_id, continuation_value: $continuation_value); + sort: $sort, order: $order, continuation_id: $continuation_id, continuation_values: $continuation_values); return [array_merge($values, $searchValues), 'SELECT ' . ($type === 'T' ? 'DISTINCT ' : '') . 'e.id' - . ($type === 'T' && $orderBy !== 'e.id' ? ', ' . $orderBy : '') // SELECT DISTINCT, ORDER BY expressions must appear in SELECT + . ($type === 'T' && $sort !== 'id' ? ', ' . $orderBy : '') // SELECT DISTINCT, ORDER BY expressions must appear in SELECT . ' FROM `_entry` e ' - . 'INNER JOIN `_feed` f ON e.id_feed = f.id ' + . 'INNER JOIN `_feed` f ON f.id = e.id_feed ' + . ($sort === 'c.name' ? 'INNER JOIN `_category` c ON c.id = f.category ' : '') . ($type === 't' || $type === 'T' ? 'INNER JOIN `_entrytag` et ON et.id_entry = e.id ' : '') . 'WHERE ' . $where . $search . 'ORDER BY ' . $orderBy . ' ' . $order + . ($sort === 'c.name' ? ', f.name ' . $order : '') // Secondary sort . ($sort === 'id' ? '' : ', e.id ' . $order) // For keyset pagination . ($limit > 0 ? ' LIMIT ' . $limit : '') // http://explainextended.com/2009/10/23/mysql-order-by-limit-performance-late-row-lookups/ . ($offset > 0 ? ' OFFSET ' . $offset : '') @@ -1360,29 +1386,44 @@ SQL; * @param int $id category/feed/tag ID * @param numeric-string $id_min * @param numeric-string $id_max - * @param 'id'|'date'|'link'|'title'|'rand' $sort + * @param 'id'|'c.name'|'date'|'f.name'|'link'|'title'|'rand' $sort * @param 'ASC'|'DESC' $order * @param numeric-string $continuation_id + * @param list $continuation_values * @throws FreshRSS_EntriesGetter_Exception */ private function listWhereRaw(string $type = 'a', int $id = 0, int $state = FreshRSS_Entry::STATE_ALL, ?FreshRSS_BooleanSearch $filters = null, string $id_min = '0', string $id_max = '0', string $sort = 'id', string $order = 'DESC', - string $continuation_id = '0', string|int $continuation_value = 0, int $limit = 1, int $offset = 0): PDOStatement|false { + string $continuation_id = '0', array $continuation_values = [], int $limit = 1, int $offset = 0): PDOStatement|false { $order = in_array($order, ['ASC', 'DESC'], true) ? $order : 'DESC'; - $sort = in_array($sort, ['id', 'date', 'link', 'title', 'rand'], true) ? $sort : 'id'; + $sort = in_array($sort, ['id', 'c.name', 'date', 'f.name', 'link', 'title', 'rand'], true) ? $sort : 'id'; [$values, $sql] = $this->sqlListWhere($type, $id, $state, $filters, id_min: $id_min, id_max: $id_max, sort: $sort, order: $order, - continuation_id: $continuation_id, continuation_value: $continuation_value, limit: $limit, offset: $offset); - - $orderBy = ($sort === 'rand' ? static::sqlRandom() : 'e0.' . $sort); + continuation_id: $continuation_id, continuation_values: $continuation_values, limit: $limit, offset: $offset); + + $orderBy = match ($sort) { + 'c.name' => 'c0.name', + 'f.name' => 'f0.name', + 'rand' => static::sqlRandom(), + default => 'e0.' . $sort, + }; $content = static::isCompressed() ? 'UNCOMPRESS(e0.content_bin) AS content' : 'e0.content'; $hash = static::sqlHexEncode('e0.hash'); $sql = <<autoUpdateDb($info)) { return $this->listWhereRaw($type, $id, $state, $filters, id_min: $id_min, id_max: $id_max, sort: $sort, order: $order, - continuation_id: $continuation_id, continuation_value: $continuation_value, limit: $limit, offset: $offset); + continuation_id: $continuation_id, continuation_values: $continuation_values, limit: $limit, offset: $offset); } Minz_Log::error('SQL error ' . __METHOD__ . json_encode($info)); return false; @@ -1407,17 +1448,18 @@ SQL; * @param int $id category/feed/tag ID * @param numeric-string $id_min * @param numeric-string $id_max - * @param 'id'|'date'|'link'|'title'|'rand' $sort + * @param 'id'|'c.name'|'date'|'f.name'|'link'|'title'|'rand' $sort * @param 'ASC'|'DESC' $order * @param numeric-string $continuation_id + * @param list $continuation_values * @return Traversable * @throws FreshRSS_EntriesGetter_Exception */ public function listWhere(string $type = 'a', int $id = 0, int $state = FreshRSS_Entry::STATE_ALL, ?FreshRSS_BooleanSearch $filters = null, string $id_min = '0', string $id_max = '0', string $sort = 'id', string $order = 'DESC', - string $continuation_id = '0', string|int $continuation_value = 0, int $limit = 1, int $offset = 0): Traversable { + string $continuation_id = '0', array $continuation_values = [], int $limit = 1, int $offset = 0): Traversable { $stm = $this->listWhereRaw($type, $id, $state, $filters, id_min: $id_min, id_max: $id_max, sort: $sort, order: $order, - continuation_id: $continuation_id, continuation_value: $continuation_value, limit: $limit, offset: $offset); + continuation_id: $continuation_id, continuation_values: $continuation_values, limit: $limit, offset: $offset); if ($stm !== false) { while (is_array($row = $stm->fetch(PDO::FETCH_ASSOC))) { /** @var array{'id':string,'id_feed':int,'guid':string,'title':string,'author':string,'content':string,'link':string,'date':int, @@ -1473,16 +1515,17 @@ SQL; * @param int $id category/feed/tag ID * @param numeric-string $id_min * @param numeric-string $id_max - * @param numeric-string $continuation_id * @param 'ASC'|'DESC' $order + * @param numeric-string $continuation_id + * @param list $continuation_values * @return list|null * @throws FreshRSS_EntriesGetter_Exception */ public function listIdsWhere(string $type = 'a', int $id = 0, int $state = FreshRSS_Entry::STATE_ALL, ?FreshRSS_BooleanSearch $filters = null, string $id_min = '0', string $id_max = '0', string $order = 'DESC', - string $continuation_id = '0', string|int $continuation_value = 0, int $limit = 1, int $offset = 0): ?array { + string $continuation_id = '0', array $continuation_values = [], int $limit = 1, int $offset = 0): ?array { [$values, $sql] = $this->sqlListWhere($type, $id, $state, $filters, id_min: $id_min, id_max: $id_max, order: $order, - continuation_id: $continuation_id, continuation_value: $continuation_value, limit: $limit, offset: $offset); + continuation_id: $continuation_id, continuation_values: $continuation_values, limit: $limit, offset: $offset); $stm = $this->pdo->prepare($sql); if ($stm !== false && $stm->execute($values)) { /** @var list $res */ diff --git a/app/Models/UserConfiguration.php b/app/Models/UserConfiguration.php index 23016c9f2..e53de00d3 100644 --- a/app/Models/UserConfiguration.php +++ b/app/Models/UserConfiguration.php @@ -52,7 +52,7 @@ declare(strict_types=1); * @property bool $show_nav_buttons * @property 'big'|'small'|'none' $mark_read_button * @property 'ASC'|'DESC' $sort_order - * @property 'id'|'date'|'link'|'title'|'rand' $sort + * @property 'id'|'c.name'|'date'|'f.name'|'link'|'title'|'rand' $sort * @property array> $sharing * @property array $shortcuts * @property bool $sides_close_article -- cgit v1.2.3