Fix limited visits query when sorting ascending (#17060)

* Fix limited visits query when sorting ascending

* reactivate test for sorting visits ascending

2 files changed, 110 insertions, 31 deletions

diff --git a/plugins/Live/Model.php b/plugins/Live/Model.php
index 023d716983..b3ab64c524 100644
--- a/plugins/Live/Model.php
+++ b/plugins/Live/Model.php
@@ -58,9 +58,9 @@ class Model
             $date = 'yesterdaySameTime';
         }
 
-        list($dateStart, $dateEnd) = $this->getStartAndEndDate($idSite, $period, $date);
+        [$dateStart, $dateEnd] = $this->getStartAndEndDate($idSite, $period, $date);
 
-        $queries = $this->splitDatesIntoMultipleQueries($dateStart, $dateEnd, $limit, $offset);
+        $queries = $this->splitDatesIntoMultipleQueries($dateStart, $dateEnd, $limit, $offset, $filterSortOrder);
 
         $foundVisits = array();
 
@@ -75,14 +75,14 @@ class Model
                 $updatedOffset = 0; // we've already skipped enough rows
             }
 
-            list($sql, $bind) = $this->makeLogVisitsQueryString($idSite, $queryRange[0], $queryRange[1], $segment, $updatedOffset, $updatedLimit, $visitorId, $minTimestamp, $filterSortOrder);
+            [$sql, $bind] = $this->makeLogVisitsQueryString($idSite, $queryRange[0], $queryRange[1], $segment, $updatedOffset, $updatedLimit, $visitorId, $minTimestamp, $filterSortOrder);
 
             $visits = $this->executeLogVisitsQuery($sql, $bind, $segment, $dateStart, $dateEnd, $minTimestamp, $limit);
 
             if (!empty($offset) && empty($visits)) {
                 // find out if there are any matches
                 $updatedOffset = 0;
-                list($sql, $bind) = $this->makeLogVisitsQueryString($idSite, $queryRange[0], $queryRange[1], $segment, $updatedOffset, $updatedLimit, $visitorId, $minTimestamp, $filterSortOrder);
+                [$sql, $bind] = $this->makeLogVisitsQueryString($idSite, $queryRange[0], $queryRange[1], $segment, $updatedOffset, $updatedLimit, $visitorId, $minTimestamp, $filterSortOrder);
 
                 $visits = $this->executeLogVisitsQuery($sql, $bind, $segment, $dateStart, $dateEnd, $minTimestamp, $limit);
                 if (!empty($visits)) {
@@ -213,7 +213,7 @@ class Model
         return true;
     }
 
-    public function splitDatesIntoMultipleQueries($dateStart, $dateEnd, $limit, $offset)
+    public function splitDatesIntoMultipleQueries($dateStart, $dateEnd, $limit, $offset, $filterSortOrder)
     {
         $virtualDateEnd = $dateEnd;
         if (empty($dateEnd)) {
@@ -230,33 +230,64 @@ class Model
         if ($limit
             && $hasStartEndDateMoreThanOneDayInBetween
         ) {
-            $virtualDateEnd = $virtualDateEnd->subDay(1);
-            $queries[] = array($virtualDateEnd, $dateEnd); // need to use ",endDate" in case endDate is not set
+            if (strtolower($filterSortOrder) !== 'asc') {
+                $virtualDateEnd = $virtualDateEnd->subDay(1);
+                $queries[]      = [$virtualDateEnd, $dateEnd]; // need to use ",endDate" in case endDate is not set
 
-            if ($virtualDateStart->addDay(7)->isEarlier($virtualDateEnd)) {
-                $queries[] = array($virtualDateEnd->subDay(7), $virtualDateEnd->subSeconds(1));
-                $virtualDateEnd = $virtualDateEnd->subDay(7);
-            }
+                if ($virtualDateStart->addDay(7)->isEarlier($virtualDateEnd)) {
+                    $queries[]      = [$virtualDateEnd->subDay(7), $virtualDateEnd->subSeconds(1)];
+                    $virtualDateEnd = $virtualDateEnd->subDay(7);
+                }
 
-            if (!$offset) {
-                // only when no offset
-                // we would in worst case - if not enough visits are found to bypass the offset - execute below queries too often.
-                // like we would need to execute each of the queries twice just to find out if there are some visits that
-                // need to be skipped...
+                if (!$offset) {
+                    // only when no offset
+                    // we would in worst case - if not enough visits are found to bypass the offset - execute below queries too often.
+                    // like we would need to execute each of the queries twice just to find out if there are some visits that
+                    // need to be skipped...
+
+                    if ($virtualDateStart->addDay(30)->isEarlier($virtualDateEnd)) {
+                        $queries[]      = [$virtualDateEnd->subDay(30), $virtualDateEnd->subSeconds(1)];
+                        $virtualDateEnd = $virtualDateEnd->subDay(30);
+                    }
+                    if ($virtualDateStart->addPeriod(1, 'year')->isEarlier($virtualDateEnd)) {
+                        $queries[]      = [$virtualDateEnd->subYear(1), $virtualDateEnd->subSeconds(1)];
+                        $virtualDateEnd = $virtualDateEnd->subYear(1);
+                    }
+                }
 
-                if ($virtualDateStart->addDay(30)->isEarlier($virtualDateEnd)) {
-                    $queries[] = array($virtualDateEnd->subDay(30), $virtualDateEnd->subSeconds(1));
-                    $virtualDateEnd = $virtualDateEnd->subDay(30);
+                if ($virtualDateStart->isEarlier($virtualDateEnd)) {
+                    // need to use ",endDate" in case startDate is not set in which case we do not want to have any limit
+                    $queries[] = [$dateStart, $virtualDateEnd->subSeconds(1)];
                 }
-                if ($virtualDateStart->addPeriod(1, 'year')->isEarlier($virtualDateEnd)) {
-                    $queries[] = array($virtualDateEnd->subYear(1), $virtualDateEnd->subSeconds(1));
-                    $virtualDateEnd = $virtualDateEnd->subYear(1);
+            } else {
+                $queries[]      = [$virtualDateStart, $virtualDateStart->addDay(1)->subSeconds(1)];
+                $virtualDateStart = $virtualDateStart->addDay(1);
+
+                if ($virtualDateStart->addDay(7)->isEarlier($virtualDateEnd)) {
+                    $queries[]      = [$virtualDateStart, $virtualDateStart->addDay(7)->subSeconds(1)];
+                    $virtualDateStart = $virtualDateStart->addDay(7);
+                }
+
+                if (!$offset) {
+                    // only when no offset
+                    // we would in worst case - if not enough visits are found to bypass the offset - execute below queries too often.
+                    // like we would need to execute each of the queries twice just to find out if there are some visits that
+                    // need to be skipped...
+
+                    if ($virtualDateStart->addDay(30)->isEarlier($virtualDateEnd)) {
+                        $queries[]      = [$virtualDateStart, $virtualDateStart->addDay(30)->subSeconds(1)];
+                        $virtualDateStart = $virtualDateStart->addDay(30);
+                    }
+                    if ($virtualDateStart->addPeriod(1, 'year')->isEarlier($virtualDateEnd)) {
+                        $queries[]      = [$virtualDateStart, $virtualDateStart->addPeriod(1, 'year')->subSeconds(1)];
+                        $virtualDateStart = $virtualDateStart->addPeriod(1, 'year');
+                    }
                 }
-            }
 
-            if ($virtualDateStart->isEarlier($virtualDateEnd)) {
-                // need to use ",endDate" in case startDate is not set in which case we do not want to have any limit
-                $queries[] = array($dateStart, $virtualDateEnd->subSeconds(1));
+                if ($virtualDateStart->isEarlier($virtualDateEnd)) {
+                    // need to use ",endDate" in case startDate is not set in which case we do not want to have any limit
+                    $queries[] = [$virtualDateStart, $dateEnd];
+                }
             }
         } else {
             $queries[] = array($dateStart, $dateEnd);
@@ -348,7 +379,7 @@ class Model
             return 0;
         }
 
-        list($whereIdSites, $idSites) = $this->getIdSitesWhereClause($idSite, $from);
+        [$whereIdSites, $idSites] = $this->getIdSitesWhereClause($idSite, $from);
 
         $now = null;
         try {
@@ -459,9 +490,9 @@ class Model
      */
     public function makeLogVisitsQueryString($idSite, $startDate, $endDate, $segment, $offset, $limit, $visitorId, $minTimestamp, $filterSortOrder)
     {
-        list($whereClause, $bindIdSites) = $this->getIdSitesWhereClause($idSite);
+        [$whereClause, $bindIdSites] = $this->getIdSitesWhereClause($idSite);
 
-        list($whereBind, $where) = $this->getWhereClauseAndBind($whereClause, $bindIdSites, $startDate, $endDate, $visitorId, $minTimestamp);
+        [$whereBind, $where] = $this->getWhereClauseAndBind($whereClause, $bindIdSites, $startDate, $endDate, $visitorId, $minTimestamp);
 
         if (strtolower($filterSortOrder) !== 'asc') {
             $filterSortOrder = 'DESC';
diff --git a/plugins/Live/tests/Integration/ModelTest.php b/plugins/Live/tests/Integration/ModelTest.php
index cdb32d2d5e..6f883e1937 100644
--- a/plugins/Live/tests/Integration/ModelTest.php
+++ b/plugins/Live/tests/Integration/ModelTest.php
@@ -379,6 +379,12 @@ class ModelTest extends IntegrationTestCase
         $this->assertEquals(array('2010-01-01 00:00:00 2010-01-02 00:00:00'), $dates);
     }
 
+    public function test_splitDatesIntoMultipleQueries_notMoreThanADayUsesOnlyOneQueryDesc()
+    {
+        $dates = $this->splitDatesIntoMultipleQueries('2010-01-01 00:00:00', '2010-01-02 00:00:00', $limit = 5, $offset = 0, 'asc');
+
+        $this->assertEquals(array('2010-01-01 00:00:00 2010-01-02 00:00:00'), $dates);
+    }
 
     public function test_splitDatesIntoMultipleQueries_moreThanADayLessThanAWeek()
     {
@@ -387,6 +393,13 @@ class ModelTest extends IntegrationTestCase
         $this->assertEquals(array('2010-01-01 00:01:00 2010-01-02 00:01:00', '2010-01-01 00:00:00 2010-01-01 00:00:59'), $dates);
     }
 
+    public function test_splitDatesIntoMultipleQueries_moreThanADayLessThanAWeekAsc()
+    {
+        $dates = $this->splitDatesIntoMultipleQueries('2010-01-01 00:00:00', '2010-01-02 00:01:00', $limit = 5, $offset = 0, 'asc');
+
+        $this->assertEquals(array('2010-01-01 00:00:00 2010-01-01 23:59:59', '2010-01-02 00:00:00 2010-01-02 00:01:00'), $dates);
+    }
+
     public function test_splitDatesIntoMultipleQueries_moreThanAWeekLessThanMonth()
     {
         $dates = $this->splitDatesIntoMultipleQueries('2010-01-01 00:00:00', '2010-01-20 04:01:00', $limit = 5, $offset = 0);
@@ -394,6 +407,13 @@ class ModelTest extends IntegrationTestCase
         $this->assertEquals(array('2010-01-19 04:01:00 2010-01-20 04:01:00', '2010-01-12 04:01:00 2010-01-19 04:00:59', '2010-01-01 00:00:00 2010-01-12 04:00:59'), $dates);
     }
 
+    public function test_splitDatesIntoMultipleQueries_moreThanAWeekLessThanMonthAsc()
+    {
+        $dates = $this->splitDatesIntoMultipleQueries('2010-01-01 00:00:00', '2010-01-20 04:01:00', $limit = 5, $offset = 0, 'asc');
+
+        $this->assertEquals(array('2010-01-01 00:00:00 2010-01-01 23:59:59', '2010-01-02 00:00:00 2010-01-08 23:59:59', '2010-01-09 00:00:00 2010-01-20 04:01:00'), $dates);
+    }
+
     public function test_splitDatesIntoMultipleQueries_moreThanMonthLessThanYear()
     {
         $dates = $this->splitDatesIntoMultipleQueries('2010-01-01 00:00:00', '2010-02-20 04:01:00', $limit = 5, $offset = 0);
@@ -401,6 +421,13 @@ class ModelTest extends IntegrationTestCase
         $this->assertEquals(array('2010-02-19 04:01:00 2010-02-20 04:01:00', '2010-02-12 04:01:00 2010-02-19 04:00:59', '2010-01-13 04:01:00 2010-02-12 04:00:59', '2010-01-01 00:00:00 2010-01-13 04:00:59'), $dates);
     }
 
+    public function test_splitDatesIntoMultipleQueries_moreThanMonthLessThanYearAsc()
+    {
+        $dates = $this->splitDatesIntoMultipleQueries('2010-01-01 00:00:00', '2010-02-20 04:01:00', $limit = 5, $offset = 0, 'asc');
+
+        $this->assertEquals(array('2010-01-01 00:00:00 2010-01-01 23:59:59', '2010-01-02 00:00:00 2010-01-08 23:59:59', '2010-01-09 00:00:00 2010-02-07 23:59:59', '2010-02-08 00:00:00 2010-02-20 04:01:00'), $dates);
+    }
+
     public function test_splitDatesIntoMultipleQueries_moreThanYear()
     {
         $dates = $this->splitDatesIntoMultipleQueries('2010-01-01 00:00:00', '2012-02-20 04:01:00', $limit = 5, $offset = 0);
@@ -408,6 +435,13 @@ class ModelTest extends IntegrationTestCase
         $this->assertEquals(array('2012-02-19 04:01:00 2012-02-20 04:01:00', '2012-02-12 04:01:00 2012-02-19 04:00:59', '2012-01-13 04:01:00 2012-02-12 04:00:59', '2011-01-01 04:01:00 2012-01-13 04:00:59', '2010-01-01 00:00:00 2011-01-01 04:00:59'), $dates);
     }
 
+    public function test_splitDatesIntoMultipleQueries_moreThanYearAsc()
+    {
+        $dates = $this->splitDatesIntoMultipleQueries('2010-01-01 00:00:00', '2012-02-20 04:01:00', $limit = 5, $offset = 0, 'asc');
+
+        $this->assertEquals(array('2010-01-01 00:00:00 2010-01-01 23:59:59', '2010-01-02 00:00:00 2010-01-08 23:59:59', '2010-01-09 00:00:00 2010-02-07 23:59:59', '2010-02-08 00:00:00 2011-02-07 23:59:59', '2011-02-08 00:00:00 2012-02-20 04:01:00'), $dates);
+    }
+
     public function test_splitDatesIntoMultipleQueries_moreThanYear_withOffsetUsesLessQueries()
     {
         $dates = $this->splitDatesIntoMultipleQueries('2010-01-01 00:00:00', '2012-02-20 04:01:00', $limit = 5, $offset = 5);
@@ -415,6 +449,13 @@ class ModelTest extends IntegrationTestCase
         $this->assertEquals(array('2012-02-19 04:01:00 2012-02-20 04:01:00', '2012-02-12 04:01:00 2012-02-19 04:00:59', '2010-01-01 00:00:00 2012-02-12 04:00:59'), $dates);
     }
 
+    public function test_splitDatesIntoMultipleQueries_moreThanYear_withOffsetUsesLessQueriesAsc()
+    {
+        $dates = $this->splitDatesIntoMultipleQueries('2010-01-01 04:01:00', '2012-02-20 00:00:00', $limit = 5, $offset = 5, 'asc');
+
+        $this->assertEquals(array('2010-01-01 04:01:00 2010-01-02 04:00:59', '2010-01-02 04:01:00 2010-01-09 04:00:59', '2010-01-09 04:01:00 2012-02-20 00:00:00'), $dates);
+    }
+
     public function test_splitDatesIntoMultipleQueries_moreThanYear_noLimitDoesntUseMultipleQueries()
     {
         $dates = $this->splitDatesIntoMultipleQueries('2010-01-01 00:00:00', '2012-02-20 04:01:00', $limit = 0, $offset = 0);
@@ -422,6 +463,13 @@ class ModelTest extends IntegrationTestCase
         $this->assertEquals(array('2010-01-01 00:00:00 2012-02-20 04:01:00'), $dates);
     }
 
+    public function test_splitDatesIntoMultipleQueries_moreThanYear_noLimitDoesntUseMultipleQueriesAsc()
+    {
+        $dates = $this->splitDatesIntoMultipleQueries('2010-01-01 04:01:00', '2012-02-20 00:00:00', $limit = 0, $offset = 0, 'asc');
+
+        $this->assertEquals(array('2010-01-01 04:01:00 2012-02-20 00:00:00'), $dates);
+    }
+
     public function test_splitDatesIntoMultipleQueries_noStartDate()
     {
         $dates = $this->splitDatesIntoMultipleQueries(false, '2012-02-20 04:01:00', $limit = 5, $offset = 0);
@@ -429,7 +477,7 @@ class ModelTest extends IntegrationTestCase
         $this->assertEquals(array('2012-02-19 04:01:00 2012-02-20 04:01:00', '2012-02-12 04:01:00 2012-02-19 04:00:59', '2012-01-13 04:01:00 2012-02-12 04:00:59', '2011-01-01 04:01:00 2012-01-13 04:00:59', ' 2011-01-01 04:00:59'), $dates);
     }
 
-    private function splitDatesIntoMultipleQueries($startDate, $endDate, $limit, $offset)
+    private function splitDatesIntoMultipleQueries($startDate, $endDate, $limit, $offset, $order='desc')
     {
         if ($startDate) {
             $startDate = Date::factory($startDate);
@@ -438,7 +486,7 @@ class ModelTest extends IntegrationTestCase
             $endDate = Date::factory($endDate);
         }
         $model = new Model();
-        $queries = $model->splitDatesIntoMultipleQueries($startDate, $endDate, $limit, $offset);
+        $queries = $model->splitDatesIntoMultipleQueries($startDate, $endDate, $limit, $offset, $order);
 
         return array_map(function ($query) { return ($query[0] ? $query[0]->getDatetime() : '') . ' ' . ($query[1] ? $query[1]->getDatetime() : ''); }, $queries);
     }