From 35eeaf4282697851ccd8bc4bc3ccc1748ab4f0eb Mon Sep 17 00:00:00 2001
From: Alejandro Celaya <alejandro@alejandrocelaya.com>
Date: Mon, 16 Dec 2019 22:13:11 +0100
Subject: [PATCH] Improved repository tests covering fetching and counting
 filtered short URL lists

---
 .../src/Repository/ShortUrlRepository.php     | 28 ++++++++-----------
 .../Repository/ShortUrlRepositoryTest.php     | 23 +++++++++++++--
 2 files changed, 32 insertions(+), 19 deletions(-)

diff --git a/module/Core/src/Repository/ShortUrlRepository.php b/module/Core/src/Repository/ShortUrlRepository.php
index 40635f83..1c26c122 100644
--- a/module/Core/src/Repository/ShortUrlRepository.php
+++ b/module/Core/src/Repository/ShortUrlRepository.php
@@ -81,7 +81,7 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI
 
     public function countList(?string $searchTerm = null, array $tags = [], ?DateRange $dateRange = null): int
     {
-        $qb = $this->createListQueryBuilder($searchTerm, $tags);
+        $qb = $this->createListQueryBuilder($searchTerm, $tags, $dateRange);
         $qb->select('COUNT(DISTINCT s)');
 
         return (int) $qb->getQuery()->getSingleScalarResult();
@@ -96,15 +96,13 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI
         $qb->from(ShortUrl::class, 's');
         $qb->where('1=1');
 
-        if ($dateRange !== null) {
-            if ($dateRange->getStartDate() !== null) {
-                $qb->andWhere($qb->expr()->gte('s.dateCreated', ':startDate'));
-                $qb->setParameter('startDate', $dateRange->getStartDate());
-            }
-            if ($dateRange->getEndDate() !== null) {
-                $qb->andWhere($qb->expr()->lte('s.dateCreated', ':endDate'));
-                $qb->setParameter('endDate', $dateRange->getEndDate());
-            }
+        if ($dateRange !== null && $dateRange->getStartDate() !== null) {
+            $qb->andWhere($qb->expr()->gte('s.dateCreated', ':startDate'));
+            $qb->setParameter('startDate', $dateRange->getStartDate());
+        }
+        if ($dateRange !== null && $dateRange->getEndDate() !== null) {
+            $qb->andWhere($qb->expr()->lte('s.dateCreated', ':endDate'));
+            $qb->setParameter('endDate', $dateRange->getEndDate());
         }
 
         // Apply search term to every searchable field if not empty
@@ -114,14 +112,12 @@ class ShortUrlRepository extends EntityRepository implements ShortUrlRepositoryI
                 $qb->leftJoin('s.tags', 't');
             }
 
-            $conditions = [
+            // Apply search conditions
+            $qb->andWhere($qb->expr()->orX(
                 $qb->expr()->like('s.longUrl', ':searchPattern'),
                 $qb->expr()->like('s.shortCode', ':searchPattern'),
-                $qb->expr()->like('t.name', ':searchPattern'),
-            ];
-
-            // Unpack and apply search conditions
-            $qb->andWhere($qb->expr()->orX(...$conditions));
+                $qb->expr()->like('t.name', ':searchPattern')
+            ));
             $qb->setParameter('searchPattern', '%' . $searchTerm . '%');
         }
 
diff --git a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php
index f3913ea2..2006623a 100644
--- a/module/Core/test-db/Repository/ShortUrlRepositoryTest.php
+++ b/module/Core/test-db/Repository/ShortUrlRepositoryTest.php
@@ -6,6 +6,8 @@ namespace ShlinkioTest\Shlink\Core\Repository;
 
 use Cake\Chronos\Chronos;
 use Doctrine\Common\Collections\ArrayCollection;
+use ReflectionObject;
+use Shlinkio\Shlink\Common\Util\DateRange;
 use Shlinkio\Shlink\Core\Entity\Domain;
 use Shlinkio\Shlink\Core\Entity\ShortUrl;
 use Shlinkio\Shlink\Core\Entity\Tag;
@@ -108,7 +110,7 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
     }
 
     /** @test */
-    public function findListProperlyFiltersByTagAndSearchTerm(): void
+    public function findListProperlyFiltersResult(): void
     {
         $tag = new Tag('bar');
         $this->getEntityManager()->persist($tag);
@@ -124,12 +126,17 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
         $this->getEntityManager()->persist($bar);
 
         $foo2 = new ShortUrl('foo_2');
+        $ref = new ReflectionObject($foo2);
+        $dateProp = $ref->getProperty('dateCreated');
+        $dateProp->setAccessible(true);
+        $dateProp->setValue($foo2, Chronos::now()->subDays(5));
         $this->getEntityManager()->persist($foo2);
 
         $this->getEntityManager()->flush();
 
         $result = $this->repo->findList(null, null, 'foo', ['bar']);
         $this->assertCount(1, $result);
+        $this->assertEquals(1, $this->repo->countList('foo', ['bar']));
         $this->assertSame($foo, $result[0]);
 
         $result = $this->repo->findList();
@@ -141,12 +148,22 @@ class ShortUrlRepositoryTest extends DatabaseTestCase
         $result = $this->repo->findList(2, 1);
         $this->assertCount(2, $result);
 
-        $result = $this->repo->findList(2, 2);
-        $this->assertCount(1, $result);
+        $this->assertCount(1, $this->repo->findList(2, 2));
 
         $result = $this->repo->findList(null, null, null, [], ['visits' => 'DESC']);
         $this->assertCount(3, $result);
         $this->assertSame($bar, $result[0]);
+
+        $result = $this->repo->findList(null, null, null, [], null, new DateRange(null, Chronos::now()->subDays(2)));
+        $this->assertCount(1, $result);
+        $this->assertEquals(1, $this->repo->countList(null, [], new DateRange(null, Chronos::now()->subDays(2))));
+        $this->assertSame($foo2, $result[0]);
+
+        $this->assertCount(
+            2,
+            $this->repo->findList(null, null, null, [], null, new DateRange(Chronos::now()->subDays(2)))
+        );
+        $this->assertEquals(2, $this->repo->countList(null, [], new DateRange(Chronos::now()->subDays(2))));
     }
 
     /** @test */