Added support to order tags with stats by short URLs or visits count. In a non-performant way

module/Core/src/Repository/TagRepository.php

@@ -16,6 +16,7 @@ use Shlinkio\Shlink\Rest\ApiKey\Spec\WithApiKeySpecsEnsuringJoin;
 use Shlinkio\Shlink\Rest\ApiKey\Spec\WithInlinedApiKeySpecsEnsuringJoin;
 use Shlinkio\Shlink\Rest\Entity\ApiKey;
 
+use function Functional\contains;
 use function Functional\map;
 
 use const PHP_INT_MAX;
@@ -40,12 +41,19 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito
      */
     public function findTagsWithInfo(?TagsListFiltering $filtering = null): array
     {
+        $orderBy = $filtering?->orderBy();
+        $orderField = $orderBy?->orderField();
+        $orderMainQuery = contains(['shortUrlsCount', 'visitsCount'], $orderField);
+
         $conn = $this->getEntityManager()->getConnection();
         $subQb = $this->createQueryBuilder('t');
-        $subQb->select('t.id', 't.name')
+        $subQb->select('t.id', 't.name');
-              ->orderBy('t.name', $filtering?->orderBy()?->orderDirection() ?? 'ASC') // TODO Make filed dynamic
+
+        if (! $orderMainQuery) {
+            $subQb->orderBy('t.name', $orderBy?->orderDirection() ?? 'ASC')
                   ->setMaxResults($filtering?->limit() ?? PHP_INT_MAX)
                   ->setFirstResult($filtering?->offset() ?? 0);
+        }
 
         $searchTerm = $filtering?->searchTerm();
         if ($searchTerm !== null) {
@@ -74,7 +82,7 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito
             ->leftJoin('st', 'short_urls', 's', $nativeQb->expr()->eq('s.id', 'st.short_url_id'))
             ->leftJoin('st', 'visits', 'v', $nativeQb->expr()->eq('s.id', 'v.short_url_id'))
             ->groupBy('t.id_0', 't.name_1')
-            ->orderBy('t.name_1', $filtering?->orderBy()?->orderDirection() ?? 'ASC'); // TODO Make field dynamic
+            ->orderBy('t.name_1', $orderBy?->orderDirection() ?? 'ASC'); // TODO Make field dynamic
 
         // Apply API key role conditions to the native query too, as they will affect the amounts on the aggregates
         $apiKey?->mapRoles(fn (string $roleName, array $meta) => match ($roleName) {
@@ -87,6 +95,16 @@ class TagRepository extends EntitySpecificationRepository implements TagReposito
             default => $nativeQb,
         });
 
+        if ($orderMainQuery) {
+            $nativeQb
+                ->orderBy(
+                    $orderField === 'shortUrlsCount' ? 'short_urls_count' : 'visits_count',
+                    $orderBy?->orderDirection() ?? 'ASC',
+                )
+                ->setMaxResults($filtering?->limit() ?? PHP_INT_MAX)
+                ->setFirstResult($filtering?->offset() ?? 0);
+        }
+
         $rsm = new ResultSetMappingBuilder($this->getEntityManager());
         $rsm->addRootEntityFromClassMetadata(Tag::class, 't');
         $rsm->addScalarResult('short_urls_count', 'shortUrlsCount');

module/Core/test-db/Repository/TagRepositoryTest.php

@@ -86,6 +86,15 @@ class TagRepositoryTest extends DatabaseTestCase
         $shortUrl2 = ShortUrl::fromMeta($metaWithTags($secondUrlTags, null), $this->relationResolver);
         $this->getEntityManager()->persist($shortUrl2);
         $this->getEntityManager()->persist(Visit::forValidShortUrl($shortUrl2, Visitor::emptyInstance()));
+
+        // One of the tags has two extra short URLs, but with no visits
+        $this->getEntityManager()->persist(
+            ShortUrl::fromMeta($metaWithTags(['bar'], null), $this->relationResolver),
+        );
+        $this->getEntityManager()->persist(
+            ShortUrl::fromMeta($metaWithTags(['bar'], null), $this->relationResolver),
+        );
+
         $this->getEntityManager()->flush();
 
         $result = $this->repo->findTagsWithInfo($filtering);
@@ -102,7 +111,7 @@ class TagRepositoryTest extends DatabaseTestCase
             self::assertEquals(0, $result[0]->visitsCount());
             self::assertEquals($tagNames[3], $result[0]->tag()->__toString());
 
-            self::assertEquals(1, $result[1]->shortUrlsCount());
+            self::assertEquals(3, $result[1]->shortUrlsCount());
             self::assertEquals(3, $result[1]->visitsCount());
             self::assertEquals($tagNames[1], $result[1]->tag()->__toString());
 
@@ -124,7 +133,7 @@ class TagRepositoryTest extends DatabaseTestCase
             self::assertEquals(0, $result[0]->visitsCount());
             self::assertEquals($tagNames[3], $result[0]->tag()->__toString());
 
-            self::assertEquals(1, $result[1]->shortUrlsCount());
+            self::assertEquals(3, $result[1]->shortUrlsCount());
             self::assertEquals(3, $result[1]->visitsCount());
             self::assertEquals($tagNames[1], $result[1]->tag()->__toString());
         }];
@@ -140,7 +149,7 @@ class TagRepositoryTest extends DatabaseTestCase
             static function (array $result, array $tagNames): void {
                 /** @var TagInfo[] $result */
                 self::assertCount(2, $result);
-                self::assertEquals(1, $result[0]->shortUrlsCount());
+                self::assertEquals(3, $result[0]->shortUrlsCount());
                 self::assertEquals(3, $result[0]->visitsCount());
                 self::assertEquals($tagNames[1], $result[0]->tag()->__toString());
 
@@ -154,7 +163,7 @@ class TagRepositoryTest extends DatabaseTestCase
             static function (array $result, array $tagNames): void {
                 /** @var TagInfo[] $result */
                 self::assertCount(2, $result);
-                self::assertEquals(1, $result[0]->shortUrlsCount());
+                self::assertEquals(3, $result[0]->shortUrlsCount());
                 self::assertEquals(3, $result[0]->visitsCount());
                 self::assertEquals($tagNames[1], $result[0]->tag()->__toString());
 
@@ -176,7 +185,7 @@ class TagRepositoryTest extends DatabaseTestCase
                 self::assertEquals(0, $result[3]->visitsCount());
                 self::assertEquals($tagNames[3], $result[3]->tag()->__toString());
 
-                self::assertEquals(1, $result[2]->shortUrlsCount());
+                self::assertEquals(3, $result[2]->shortUrlsCount());
                 self::assertEquals(3, $result[2]->visitsCount());
                 self::assertEquals($tagNames[1], $result[2]->tag()->__toString());
 
@@ -189,6 +198,94 @@ class TagRepositoryTest extends DatabaseTestCase
                 self::assertEquals($tagNames[0], $result[0]->tag()->__toString());
             },
         ];
+        yield 'short URLs count ASC ordering' => [
+            new TagsListFiltering(null, null, null, Ordering::fromTuple(['shortUrlsCount', 'ASC'])),
+            static function (array $result, array $tagNames): void {
+                /** @var TagInfo[] $result */
+                self::assertCount(4, $result);
+                self::assertEquals(0, $result[0]->shortUrlsCount());
+                self::assertEquals(0, $result[0]->visitsCount());
+                self::assertEquals($tagNames[3], $result[0]->tag()->__toString());
+
+                self::assertEquals(1, $result[1]->shortUrlsCount());
+                self::assertEquals(3, $result[1]->visitsCount());
+                self::assertEquals($tagNames[2], $result[1]->tag()->__toString());
+
+                self::assertEquals(2, $result[2]->shortUrlsCount());
+                self::assertEquals(4, $result[2]->visitsCount());
+                self::assertEquals($tagNames[0], $result[2]->tag()->__toString());
+
+                self::assertEquals(3, $result[3]->shortUrlsCount());
+                self::assertEquals(3, $result[3]->visitsCount());
+                self::assertEquals($tagNames[1], $result[3]->tag()->__toString());
+            },
+        ];
+        yield 'short URLs count DESC ordering' => [
+            new TagsListFiltering(null, null, null, Ordering::fromTuple(['shortUrlsCount', 'DESC'])),
+            static function (array $result, array $tagNames): void {
+                /** @var TagInfo[] $result */
+                self::assertCount(4, $result);
+                self::assertEquals(3, $result[0]->shortUrlsCount());
+                self::assertEquals(3, $result[0]->visitsCount());
+                self::assertEquals($tagNames[1], $result[0]->tag()->__toString());
+
+                self::assertEquals(2, $result[1]->shortUrlsCount());
+                self::assertEquals(4, $result[1]->visitsCount());
+                self::assertEquals($tagNames[0], $result[1]->tag()->__toString());
+
+                self::assertEquals(1, $result[2]->shortUrlsCount());
+                self::assertEquals(3, $result[2]->visitsCount());
+                self::assertEquals($tagNames[2], $result[2]->tag()->__toString());
+
+                self::assertEquals(0, $result[3]->shortUrlsCount());
+                self::assertEquals(0, $result[3]->visitsCount());
+                self::assertEquals($tagNames[3], $result[3]->tag()->__toString());
+            },
+        ];
+        yield 'visits count ASC ordering' => [
+            new TagsListFiltering(null, null, null, Ordering::fromTuple(['visitsCount', 'ASC'])),
+            static function (array $result, array $tagNames): void {
+                /** @var TagInfo[] $result */
+                self::assertCount(4, $result);
+                self::assertEquals(0, $result[0]->shortUrlsCount());
+                self::assertEquals(0, $result[0]->visitsCount());
+                self::assertEquals($tagNames[3], $result[0]->tag()->__toString());
+
+                self::assertEquals(3, $result[1]->shortUrlsCount());
+                self::assertEquals(3, $result[1]->visitsCount());
+                self::assertEquals($tagNames[1], $result[1]->tag()->__toString());
+
+                self::assertEquals(1, $result[2]->shortUrlsCount());
+                self::assertEquals(3, $result[2]->visitsCount());
+                self::assertEquals($tagNames[2], $result[2]->tag()->__toString());
+
+                self::assertEquals(2, $result[3]->shortUrlsCount());
+                self::assertEquals(4, $result[3]->visitsCount());
+                self::assertEquals($tagNames[0], $result[3]->tag()->__toString());
+            },
+        ];
+        yield 'visits count DESC ordering' => [
+            new TagsListFiltering(null, null, null, Ordering::fromTuple(['visitsCount', 'DESC'])),
+            static function (array $result, array $tagNames): void {
+                /** @var TagInfo[] $result */
+                self::assertCount(4, $result);
+                self::assertEquals(2, $result[0]->shortUrlsCount());
+                self::assertEquals(4, $result[0]->visitsCount());
+                self::assertEquals($tagNames[0], $result[0]->tag()->__toString());
+
+                self::assertEquals(3, $result[1]->shortUrlsCount());
+                self::assertEquals(3, $result[1]->visitsCount());
+                self::assertEquals($tagNames[1], $result[1]->tag()->__toString());
+
+                self::assertEquals(1, $result[2]->shortUrlsCount());
+                self::assertEquals(3, $result[2]->visitsCount());
+                self::assertEquals($tagNames[2], $result[2]->tag()->__toString());
+
+                self::assertEquals(0, $result[3]->shortUrlsCount());
+                self::assertEquals(0, $result[3]->visitsCount());
+                self::assertEquals($tagNames[3], $result[3]->tag()->__toString());
+            },
+        ];
         yield 'api key' => [new TagsListFiltering(null, null, null, null, ApiKey::fromMeta(
             ApiKeyMeta::withRoles(RoleDefinition::forAuthoredShortUrls()),
         )), static function (array $result, array $tagNames): void {