Added support to order short URLs list by amount of non-bot visits

docs/swagger/paths/v1_short-urls.json

@@ -73,10 +73,12 @@
                         "shortCode-DESC",
                         "dateCreated-ASC",
                         "dateCreated-DESC",
+                        "title-ASC",
+                        "title-DESC",
                         "visits-ASC",
                         "visits-DESC",
-                        "title-ASC",
+                        "nonBotVisits-ASC",
-                        "title-DESC"
+                        "nonBotVisits-DESC"
                     ]
                 }
             },

module/Core/src/ShortUrl/Model/OrderableField.php

@@ -0,0 +1,37 @@
+<?php
+
+namespace Shlinkio\Shlink\Core\ShortUrl\Model;
+
+use function Functional\contains;
+use function Functional\map;
+
+enum OrderableField: string
+{
+    case LONG_URL = 'longUrl';
+    case SHORT_CODE = 'shortCode';
+    case DATE_CREATED = 'dateCreated';
+    case TITLE = 'title';
+    case VISITS = 'visits';
+    case NON_BOT_VISITS = 'nonBotVisits';
+
+    /**
+     * @return string[]
+     */
+    public static function values(): array
+    {
+        return map(self::cases(), static fn (OrderableField $field) => $field->value);
+    }
+
+    public static function isBasicField(string $value): bool
+    {
+        return contains(
+            [self::LONG_URL->value, self::SHORT_CODE->value, self::DATE_CREATED->value, self::TITLE->value],
+            $value,
+        );
+    }
+
+    public static function isVisitsField(string $value): bool
+    {
+        return $value === self::VISITS->value || $value === self::NON_BOT_VISITS->value;
+    }
+}

module/Core/src/ShortUrl/Model/ShortUrlsParams.php

@@ -14,7 +14,6 @@ use function Shlinkio\Shlink\Core\normalizeOptionalDate;
 
 final class ShortUrlsParams
 {
-    public const ORDERABLE_FIELDS = ['longUrl', 'shortCode', 'dateCreated', 'title', 'visits'];
     public const DEFAULT_ITEMS_PER_PAGE = 10;
 
     private function __construct(

module/Core/src/ShortUrl/Model/Validation/ShortUrlsParamsInputFilter.php

@@ -8,7 +8,7 @@ use Laminas\InputFilter\InputFilter;
 use Laminas\Validator\InArray;
 use Shlinkio\Shlink\Common\Paginator\Paginator;
 use Shlinkio\Shlink\Common\Validation;
-use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlsParams;
+use Shlinkio\Shlink\Core\ShortUrl\Model\OrderableField;
 use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode;
 
 class ShortUrlsParamsInputFilter extends InputFilter
@@ -51,7 +51,7 @@ class ShortUrlsParamsInputFilter extends InputFilter
         ]));
         $this->add($tagsMode);
 
-        $this->add($this->createOrderByInput(self::ORDER_BY, ShortUrlsParams::ORDERABLE_FIELDS));
+        $this->add($this->createOrderByInput(self::ORDER_BY, OrderableField::values()));
 
         $this->add($this->createBooleanInput(self::EXCLUDE_MAX_VISITS_REACHED, false));
         $this->add($this->createBooleanInput(self::EXCLUDE_PAST_VALID_UNTIL, false));

module/Core/src/ShortUrl/Repository/ShortUrlListRepository.php

@@ -10,13 +10,13 @@ use Doctrine\ORM\QueryBuilder;
 use Happyr\DoctrineSpecification\Repository\EntitySpecificationRepository;
 use Shlinkio\Shlink\Common\Doctrine\Type\ChronosDateTimeType;
 use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
+use Shlinkio\Shlink\Core\ShortUrl\Model\OrderableField;
 use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode;
 use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsCountFiltering;
 use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsListFiltering;
 use Shlinkio\Shlink\Core\Visit\Entity\Visit;
 
 use function array_column;
-use function Functional\contains;
 use function sprintf;
 
 class ShortUrlListRepository extends EntitySpecificationRepository implements ShortUrlListRepositoryInterface
@@ -31,11 +31,10 @@ class ShortUrlListRepository extends EntitySpecificationRepository implements Sh
            ->setMaxResults($filtering->limit)
            ->setFirstResult($filtering->offset);
 
-        // In case the ordering has been specified, the query could be more complex. Process it
         $this->processOrderByForList($qb, $filtering);
 
         $result = $qb->getQuery()->getResult();
-        if ($filtering->orderBy->field === 'visits') {
+        if (OrderableField::isVisitsField($filtering->orderBy->field ?? '')) {
             return array_column($result, 0);
         }
 
@@ -45,23 +44,28 @@ class ShortUrlListRepository extends EntitySpecificationRepository implements Sh
     private function processOrderByForList(QueryBuilder $qb, ShortUrlsListFiltering $filtering): void
     {
         // With no explicit order by, fallback to dateCreated-DESC
-        if (! $filtering->orderBy->hasOrderField()) {
+        $fieldName = $filtering->orderBy->field;
+        if ($fieldName === null) {
             $qb->orderBy('s.dateCreated', 'DESC');
             return;
         }
 
-        $fieldName = $filtering->orderBy->field;
         $order = $filtering->orderBy->direction;
 
-        if ($fieldName === 'visits') {
+        if (OrderableField::isBasicField($fieldName)) {
+            $qb->orderBy('s.' . $fieldName, $order);
+        } elseif (OrderableField::isVisitsField($fieldName)) {
             // FIXME This query is inefficient.
             //       Diagnostic: It might need to use a sub-query, as done with the tags list query.
             $qb->addSelect('COUNT(DISTINCT v)')
-                ->leftJoin('s.visits', 'v')
+               ->leftJoin('s.visits', 'v', Join::WITH, $qb->expr()->andX(
+                   $qb->expr()->eq('v.shortUrl', 's'),
+                   $fieldName === OrderableField::NON_BOT_VISITS->value
+                       ? $qb->expr()->eq('v.potentialBot', 'false')
+                       : null,
+               ))
                ->groupBy('s')
                ->orderBy('COUNT(DISTINCT v)', $order);
-        } elseif (contains(['longUrl', 'shortCode', 'dateCreated', 'title'], $fieldName)) {
-            $qb->orderBy('s.' . $fieldName, $order);
         }
     }
 

module/Core/test-db/ShortUrl/Repository/ShortUrlListRepositoryTest.php

@@ -10,6 +10,7 @@ use ReflectionObject;
 use Shlinkio\Shlink\Common\Util\DateRange;
 use Shlinkio\Shlink\Core\Model\Ordering;
 use Shlinkio\Shlink\Core\ShortUrl\Entity\ShortUrl;
+use Shlinkio\Shlink\Core\ShortUrl\Model\OrderableField;
 use Shlinkio\Shlink\Core\ShortUrl\Model\ShortUrlCreation;
 use Shlinkio\Shlink\Core\ShortUrl\Model\TagsMode;
 use Shlinkio\Shlink\Core\ShortUrl\Persistence\ShortUrlsCountFiltering;
@@ -21,6 +22,8 @@ use Shlinkio\Shlink\Core\Visit\Model\Visitor;
 use Shlinkio\Shlink\TestUtils\DbTest\DatabaseTestCase;
 
 use function count;
+use function Functional\map;
+use function range;
 
 class ShortUrlListRepositoryTest extends DatabaseTestCase
 {
@@ -56,12 +59,23 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase
         $this->getEntityManager()->persist($foo);
 
         $bar = ShortUrl::withLongUrl('bar');
-        $visit = Visit::forValidShortUrl($bar, Visitor::emptyInstance());
+        $visits = map(range(0, 5), function () use ($bar) {
+            $visit = Visit::forValidShortUrl($bar, Visitor::botInstance());
             $this->getEntityManager()->persist($visit);
-        $bar->setVisits(new ArrayCollection([$visit]));
+
+            return $visit;
+        });
+        $bar->setVisits(new ArrayCollection($visits));
         $this->getEntityManager()->persist($bar);
 
         $foo2 = ShortUrl::withLongUrl('foo_2');
+        $visits2 = map(range(0, 3), function () use ($foo2) {
+            $visit = Visit::forValidShortUrl($foo2, Visitor::emptyInstance());
+            $this->getEntityManager()->persist($visit);
+
+            return $visit;
+        });
+        $foo2->setVisits(new ArrayCollection($visits2));
         $ref = new ReflectionObject($foo2);
         $dateProp = $ref->getProperty('dateCreated');
         $dateProp->setAccessible(true);
@@ -95,11 +109,19 @@ class ShortUrlListRepositoryTest extends DatabaseTestCase
         self::assertCount(1, $this->repo->findList(new ShortUrlsListFiltering(2, 2, Ordering::emptyInstance())));
 
         $result = $this->repo->findList(
-            new ShortUrlsListFiltering(null, null, Ordering::fromTuple(['visits', 'DESC'])),
+            new ShortUrlsListFiltering(null, null, Ordering::fromTuple([OrderableField::VISITS->value, 'DESC'])),
         );
         self::assertCount(3, $result);
         self::assertSame($bar, $result[0]);
 
+        $result = $this->repo->findList(
+            new ShortUrlsListFiltering(null, null, Ordering::fromTuple(
+                [OrderableField::NON_BOT_VISITS->value, 'DESC'],
+            )),
+        );
+        self::assertCount(3, $result);
+        self::assertSame($foo2, $result[0]);
+
         $result = $this->repo->findList(
             new ShortUrlsListFiltering(null, null, Ordering::emptyInstance(), null, [], null, DateRange::until(
                 Chronos::now()->subDays(2),