From 10e941cea6043f3e9d15be3775d2b007b9b9c388 Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandrocelaya@gmail.com>
Date: Wed, 27 Mar 2024 09:15:21 +0100
Subject: [PATCH] Add missing COALESCE when summing visits counts

---
 module/Core/src/Model/Ordering.php                    | 10 +++++-----
 .../ShortUrl/Repository/ShortUrlListRepository.php    |  4 ++--
 .../src/Visit/Listener/ShortUrlVisitsCountTracker.php |  4 ++--
 .../Repository/ShortUrlListRepositoryTest.php         | 11 +++++------
 4 files changed, 14 insertions(+), 15 deletions(-)

diff --git a/module/Core/src/Model/Ordering.php b/module/Core/src/Model/Ordering.php
index b56c0ea8..e1b91510 100644
--- a/module/Core/src/Model/Ordering.php
+++ b/module/Core/src/Model/Ordering.php
@@ -14,6 +14,11 @@ final readonly class Ordering
     {
     }
 
+    public static function none(): self
+    {
+        return new self();
+    }
+
     /**
      * @param array{string|null, string|null} $props
      */
@@ -23,11 +28,6 @@ final readonly class Ordering
         return new self($field, $dir ?? self::DEFAULT_DIR);
     }
 
-    public static function none(): self
-    {
-        return new self(null, self::DEFAULT_DIR);
-    }
-
     public static function fromFieldAsc(string $field): self
     {
         return new self($field, self::ASC_DIR);
diff --git a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php
index 0c0c3df3..790a3dbb 100644
--- a/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php
+++ b/module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php
@@ -30,7 +30,7 @@ class ShortUrlListRepository extends EntitySpecificationRepository implements Sh
     {
         $buildVisitsSubQuery = function (string $alias, bool $excludingBots): string {
             $vqb = $this->getEntityManager()->createQueryBuilder();
-            $vqb->select('SUM(' . $alias . '.count)')
+            $vqb->select('COALESCE(SUM(' . $alias . '.count), 0)')
                 ->from(ShortUrlVisitsCount::class, $alias)
                 ->where($vqb->expr()->eq($alias . '.shortUrl', 's'));
 
@@ -50,7 +50,7 @@ class ShortUrlListRepository extends EntitySpecificationRepository implements Sh
            ->setMaxResults($filtering->limit)
            ->setFirstResult($filtering->offset)
            // This param is used in one of the sub-queries, but needs to set in the parent query
-           ->setParameter('potentialBot', 0);
+           ->setParameter('potentialBot', false);
 
         $this->processOrderByForList($qb, $filtering);
 
diff --git a/module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php b/module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php
index 8d79c330..f64974dd 100644
--- a/module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php
+++ b/module/Core/src/Visit/Listener/ShortUrlVisitsCountTracker.php
@@ -128,7 +128,7 @@ final class ShortUrlVisitsCountTracker
                $qb->expr()->eq('slot_id', ':slot_id'),
            ))
            ->setParameter('short_url_id', $shortUrlId)
-           ->setParameter('potential_bot', $potentialBot)
+           ->setParameter('potential_bot', $potentialBot ? '1' : '0')
            ->setParameter('slot_id', $slotId)
            ->setMaxResults(1);
 
@@ -155,7 +155,7 @@ final class ShortUrlVisitsCountTracker
                    ));
 
         $writeQb->setParameter('short_url_id', $shortUrlId)
-                ->setParameter('potential_bot', $potentialBot)
+                ->setParameter('potential_bot', $potentialBot ? '1' : '0')
                 ->setParameter('slot_id', $slotId)
                 ->executeStatement();
     }
diff --git a/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php b/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php
index cad0569d..95924956 100644
--- a/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php
+++ b/module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php
@@ -115,12 +115,11 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase
         self::assertCount(3, $result);
         self::assertSame($bar, $result[0]->shortUrl);
 
-        // FIXME Check why this assertion fails
-//        $result = $this->repo->findList(new ShortUrlsListFiltering(
-//            orderBy: Ordering::fromFieldDesc(OrderableField::NON_BOT_VISITS->value),
-//        ));
-//        self::assertCount(3, $result);
-//        self::assertSame($foo2, $result[0]->shortUrl);
+        $result = $this->repo->findList(new ShortUrlsListFiltering(
+            orderBy: Ordering::fromFieldDesc(OrderableField::NON_BOT_VISITS->value),
+        ));
+        self::assertCount(3, $result);
+        self::assertSame($foo2, $result[0]->shortUrl);
 
         $result = $this->repo->findList(new ShortUrlsListFiltering(
             dateRange: DateRange::until(Chronos::now()->subDays(2)),