refs #5416 we need to execute 3 queries in order to get the correct number of actions and conversions in last X minutes

2 files changed, 181 insertions, 21 deletions

diff --git a/plugins/Live/API.php b/plugins/Live/API.php
index 66d2d68861..357114ebf4 100644
--- a/plugins/Live/API.php
+++ b/plugins/Live/API.php
@@ -68,34 +68,49 @@ class API extends \Piwik\Plugin\API
     public function getCounters($idSite, $lastMinutes, $segment = false)
     {
         Piwik::checkUserHasViewAccess($idSite);
-        $lastMinutes = (int)$lastMinutes;
+        $lastMinutes = (int) $lastMinutes;
 
-        $select = "count(*) as visits,
-				SUM(log_visit.visit_total_actions) as actions,
-				SUM(log_visit.visit_goal_converted) as visitsConverted,
-				COUNT(DISTINCT log_visit.idvisitor) as visitors";
+        $counters = array(
+            'visits'   => 0,
+            'actions'  => 0,
+            'visitors' => 0,
+            'visitsConverted' => 0,
+        );
 
-        $from = "log_visit";
+        if (empty($lastMinutes)) {
+            return array($counters);
+        }
 
         list($whereIdSites, $idSites) = $this->getIdSitesWhereClause($idSite);
 
-        $where = $whereIdSites . "AND log_visit.visit_last_action_time >= ?";
-        $bind = $idSites;
-        $bind[] = Date::factory(time() - $lastMinutes * 60)->toString('Y-m-d H:i:s');
+        $select  = "count(*) as visits, COUNT(DISTINCT log_visit.idvisitor) as visitors";
+        $where   = $whereIdSites . "AND log_visit.visit_last_action_time >= ?";
+        $bind    = $idSites;
+        $bind[]  = Date::factory(time() - $lastMinutes * 60)->toString('Y-m-d H:i:s');
 
         $segment = new Segment($segment, $idSite);
-        $query = $segment->getSelectQuery($select, $from, $where, $bind);
+        $query   = $segment->getSelectQuery($select, 'log_visit', $where, $bind);
 
-        $data = Db::fetchAll($query['sql'], $query['bind']);
+        $data    = Db::fetchAll($query['sql'], $query['bind']);
 
-        // These could be unset for some reasons, ensure they are set to 0
-        if (empty($data[0]['actions'])) {
-            $data[0]['actions'] = 0;
-        }
-        if (empty($data[0]['visitsConverted'])) {
-            $data[0]['visitsConverted'] = 0;
-        }
-        return $data;
+        $counters['visits']   = $data[0]['visits'];
+        $counters['visitors'] = $data[0]['visitors'];
+
+        $select = "count(*)";
+        $from   = 'log_link_visit_action';
+        list($whereIdSites) = $this->getIdSitesWhereClause($idSite, $from);
+        $where  = $whereIdSites . "AND log_link_visit_action.server_time >= ?";
+        $query  = $segment->getSelectQuery($select, $from, $where, $bind);
+        $counters['actions'] = Db::fetchOne($query['sql'], $query['bind']);
+
+        $select = "count(*)";
+        $from   = 'log_conversion';
+        list($whereIdSites) = $this->getIdSitesWhereClause($idSite, $from);
+        $where  = $whereIdSites . "AND log_conversion.server_time >= ?";
+        $query  = $segment->getSelectQuery($select, $from, $where, $bind);
+        $counters['visitsConverted'] = Db::fetchOne($query['sql'], $query['bind']);
+
+        return array($counters);
     }
 
     /**
@@ -710,15 +725,16 @@ class API extends \Piwik\Plugin\API
 
     /**
      * @param $idSite
+     * @param string $table
      * @return array
      */
-    private function getIdSitesWhereClause($idSite)
+    private function getIdSitesWhereClause($idSite, $table = 'log_visit')
     {
         $idSites = array($idSite);
         Piwik::postEvent('Live.API.getIdSitesString', array(&$idSites));
 
         $idSitesBind = Common::getSqlStringFieldsArray($idSites);
-        $whereClause = "log_visit.idsite in ($idSitesBind) ";
+        $whereClause = $table . ".idsite in ($idSitesBind) ";
         return array($whereClause, $idSites);
     }
 }
diff --git a/plugins/Live/tests/APITest.php b/plugins/Live/tests/APITest.php
new file mode 100644
index 0000000000..588e467ad2
--- /dev/null
+++ b/plugins/Live/tests/APITest.php
@@ -0,0 +1,144 @@
+<?php
+/**
+ * Piwik - free/libre analytics platform
+ *
+ * @link http://piwik.org
+ * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later
+ */
+
+namespace Piwik\Plugins\Live\tests;
+
+use Piwik\Date;
+use Piwik\Plugins\Goals\API as GoalsApi;
+use Piwik\Plugins\Live\API;
+use FakeAccess;
+use Piwik\Access;
+use Piwik\Tests\Fixture;
+
+/**
+ * @group Live
+ * @group APITest
+ * @group Database
+ * @group Plugins
+ */
+class APITest extends \DatabaseTestCase
+{
+    /**
+     * @var API
+     */
+    private $api;
+    private $idSite = 1;
+
+    public function setUp()
+    {
+        parent::setUp();
+
+        $this->api = API::getInstance();
+        $this->setSuperUser();
+        $this->createSite();
+    }
+
+    /**
+     * @expectedException \Exception
+     * @expectedExceptionMessage checkUserHasViewAccess Fake exception
+     */
+    public function test_GetCounters_ShouldFail_IfUserHasNoPermission()
+    {
+        $this->setAnonymous();
+        $this->api->getCounters($this->idSite, 5);
+    }
+
+    public function test_GetCounters_ShouldReturnZeroForAllCounters_IfThereAreNoVisitsEtc()
+    {
+        $counters = $this->api->getCounters($this->idSite, 5);
+
+        $this->assertEquals($this->buildCounter(0, 0, 0, 0), $counters);
+    }
+
+    public function test_GetCounters_ShouldOnlyReturnResultsOfLastMinutes()
+    {
+        $this->trackSomeVisits();
+
+        $counters = $this->api->getCounters($this->idSite, 5);
+        $this->assertEquals($this->buildCounter(16, 32, 16, 16), $counters);
+
+        $counters = $this->api->getCounters($this->idSite, 20);
+        $this->assertEquals($this->buildCounter(20, 60, 20, 40), $counters);
+
+        $counters = $this->api->getCounters($this->idSite, 0);
+        $this->assertEquals($this->buildCounter(0, 0, 0, 0), $counters);
+    }
+
+    private function trackSomeVisits()
+    {
+        $nowTimestamp = time();
+
+        // use local tracker so mock location provider can be used
+        $t = Fixture::getTracker($this->idSite, $nowTimestamp, $defaultInit = true, $useLocal = true);
+
+        for ($i = 0; $i != 20; ++$i) {
+            $t->setForceNewVisit();
+            $t->setVisitorId( substr(md5($i * 1000), 0, $t::LENGTH_VISITOR_ID));
+
+            $factor = 10;
+            if ($i > 15) {
+                $factor = 30; // make sure first 15 visits are always within 5 minutes to prevent any random fails
+            }
+            $time = $nowTimestamp - ($i * $factor);
+
+            // first visit -> this one is > 5 minutes and should be ignored in one test
+            $date = Date::factory($time - 600);
+            $t->setForceVisitDateTime($date->getDatetime());
+            $t->setUrl("http://piwik.net/space/quest/iv");
+            $t->doTrackPageView("Space Quest XV");
+
+            $t->doTrackGoal(1); // this one is > 5 minutes and should be ignored in one test
+
+            // second visit
+            $date = Date::factory($time - 1);
+            $t->setForceVisitDateTime($date->getDatetime());
+            $t->setUrl("http://piwik.net/space/quest/iv");
+            $t->doTrackPageView("Space Quest XII");
+
+            // third visit
+            $date = Date::factory($time);
+            $t->setForceVisitDateTime($date->getDatetime());
+            $t->setUrl("http://piwik.net/grue/$i");
+            $t->doTrackPageView('It is pitch black...');
+
+            $t->doTrackGoal(2);
+        }
+    }
+
+    private function buildCounter($visits, $actions, $visitors, $visitsConverted)
+    {
+        return array(array(
+            'visits'   => $visits,
+            'actions'  => $actions,
+            'visitors' => $visitors,
+            'visitsConverted' => $visitsConverted,
+        ));
+    }
+
+    private function createSite()
+    {
+        Fixture::createWebsite('2013-01-23 01:23:45');
+        GoalsApi::getInstance()->addGoal(1, 'MyName', 'manually', '', 'contains');
+        GoalsApi::getInstance()->addGoal(1, 'MyGoal', 'manually', '', 'contains');
+    }
+
+    private function setSuperUser()
+    {
+        $pseudoMockAccess = new FakeAccess();
+        FakeAccess::$superUser = true;
+        Access::setSingletonInstance($pseudoMockAccess);
+    }
+
+    private function setAnonymous()
+    {
+        $pseudoMockAccess = new FakeAccess();
+        FakeAccess::$superUser = false;
+        Access::setSingletonInstance($pseudoMockAccess);
+    }
+
+}