Skip to content
Extraits de code Groupes Projets
Valider fe46664d rédigé par Matthieu Aubry's avatar Matthieu Aubry
Parcourir les fichiers

Merge pull request #9387 from piwik/9194_2 

Fix segment counts more conversion than All visits segment
parents bbe352fc 7e0e0823
Aucune branche associée trouvée
Aucune étiquette associée trouvée
Aucune requête de fusion associée trouvée
Masquer les modifications d'espaces
En ligne Côte à côte
Affichage de
avec 350 ajouts et 5 suppressions
core/DataAccess/LogQueryBuilder.php
+ 23
− 3
Voir le fichier @ fe46664d
......@@ -22,6 +22,8 @@ class LogQueryBuilder
$from = array($from);
}
$fromInitially = $from;
if (!$segmentExpression->isEmpty()) {
$segmentExpression->parseSubExpressionsIntoSqlExpressions($from);
$segmentSql = $segmentExpression->getSql();
......@@ -33,7 +35,16 @@ class LogQueryBuilder
$joinWithSubSelect = $joins['joinWithSubSelect'];
$from = $joins['sql'];
if ($joinWithSubSelect) {
// hack for https://github.com/piwik/piwik/issues/9194#issuecomment-164321612
$useSpecialConversionGroupBy = (!empty($segmentSql)
&& strpos($groupBy, 'log_conversion.idgoal') !== false
&& $fromInitially == array('log_conversion')
&& strpos($from, 'log_link_visit_action') !== false);
if ($useSpecialConversionGroupBy) {
$innerGroupBy = "CONCAT(log_conversion.idvisit, '_' , log_conversion.idgoal, '_', log_conversion.buster)";
$sql = $this->buildWrappedSelectQuery($select, $from, $where, $groupBy, $orderBy, $limit, $innerGroupBy);
} elseif ($joinWithSubSelect) {
$sql = $this->buildWrappedSelectQuery($select, $from, $where, $groupBy, $orderBy, $limit);
} else {
$sql = $this->buildSelectQuery($select, $from, $where, $groupBy, $orderBy, $limit);
......@@ -239,10 +250,11 @@ class LogQueryBuilder
* @param string $groupBy
* @param string $orderBy
* @param string $limit
* @param null|string $innerGroupBy If given, this inner group by will be used. If not, we try to detect one
* @throws Exception
* @return string
*/
private function buildWrappedSelectQuery($select, $from, $where, $groupBy, $orderBy, $limit)
private function buildWrappedSelectQuery($select, $from, $where, $groupBy, $orderBy, $limit, $innerGroupBy = null)
{
$matchTables = "(log_visit|log_conversion_item|log_conversion|log_action)";
preg_match_all("/". $matchTables ."\.[a-z0-9_\*]+/", $select, $matches);
......@@ -253,12 +265,20 @@ class LogQueryBuilder
. "Please use a table prefix.");
}
preg_match_all("/". $matchTables . "/", $from, $matchesFrom);
$innerSelect = implode(", \n", $neededFields);
$innerFrom = $from;
$innerWhere = $where;
$innerLimit = $limit;
$innerGroupBy = "log_visit.idvisit";
if (!isset($innerGroupBy) && in_array('log_visit', $matchesFrom[1])) {
$innerGroupBy = "log_visit.idvisit";
} elseif (!isset($innerGroupBy)) {
throw new Exception('Cannot use subselect for join as no group by rule is specified');
}
$innerOrderBy = "NULL";
if ($innerLimit && $orderBy) {
// only When LIMITing we can apply to the inner query the same ORDER BY as the parent query
......
tests/PHPUnit/Integration/SegmentTest.php
+ 142
− 0
Voir le fichier @ fe46664d
......@@ -1140,6 +1140,148 @@ class SegmentTest extends IntegrationTestCase
$this->assertCacheWasHit($hits = 2);
}
// se https://github.com/piwik/piwik/issues/9194
public function test_getSelectQuery_whenQueryingLogConversionWithSegmentThatUsesLogLinkVisitAction_shouldUseSubselect()
{
$select = 'log_conversion.idgoal AS `idgoal`,
count(*) AS `1`,
count(distinct log_conversion.idvisit) AS `3`,
ROUND(SUM(log_conversion.revenue),2) AS `2`,
SUM(log_conversion.items) AS `8`';
$from = 'log_conversion';
$where = 'log_conversion.server_time >= ? AND log_conversion.server_time <= ? AND log_conversion.idsite IN (?)';
$groupBy = 'log_conversion.idgoal';
$bind = array('2015-10-14 11:00:00', '2015-10-15 10:59:59', 1);
$segment = 'pageUrl=@/';
$segment = new Segment($segment, $idSites = array());
$query = $segment->getSelectQuery($select, $from, $where, $bind, $orderBy = false, $groupBy);
$logConversionTable = Common::prefixTable('log_conversion');
$logLinkVisitActionTable = Common::prefixTable('log_link_visit_action');
$expectedBind = $bind;
$expectedBind[] = '/';
$expected = array(
"sql" => "
SELECT log_inner.idgoal AS `idgoal`, count(*) AS `1`, count(distinct log_inner.idvisit) AS `3`, ROUND(SUM(log_inner.revenue),2) AS `2`, SUM(log_inner.items) AS `8`
FROM (
SELECT log_conversion.idgoal, log_conversion.idvisit, log_conversion.revenue, log_conversion.items
FROM $logConversionTable AS log_conversion
LEFT JOIN $logLinkVisitActionTable AS log_link_visit_action ON log_conversion.idvisit = log_link_visit_action.idvisit
WHERE ( log_conversion.server_time >= ?
AND log_conversion.server_time <= ?
AND log_conversion.idsite IN (?) )
AND ( ( log_link_visit_action.idaction_url IN (SELECT idaction FROM log_action WHERE ( name LIKE CONCAT('%', ?, '%') AND type = 1 )) ) )
GROUP BY CONCAT(log_conversion.idvisit, '_' , log_conversion.idgoal, '_', log_conversion.buster)
ORDER BY NULL )
AS log_inner GROUP BY log_inner.idgoal",
"bind" => $expectedBind);
$this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($query));
}
// se https://github.com/piwik/piwik/issues/9194
public function test_getSelectQuery_whenQueryingLogConversionWithSegmentThatUsesLogLinkVisitActionAndGroupsForUsageInConversionsByTypeOfVisit_shouldUseSubselect()
{
$select = 'log_conversion.idgoal AS `idgoal`,
log_conversion.referer_type AS `referer_type`,
log_conversion.referer_name AS `referer_name`,
log_conversion.referer_keyword AS `referer_keyword`,
count(*) AS `1`,
count(distinct log_conversion.idvisit) AS `3`,
ROUND(SUM(log_conversion.revenue),2) AS `2`';
$from = 'log_conversion';
$where = 'log_conversion.server_time >= ? AND log_conversion.server_time <= ? AND log_conversion.idsite IN (?)';
$groupBy = 'log_conversion.idgoal, log_conversion.referer_type, log_conversion.referer_name, log_conversion.referer_keyword';
$bind = array('2015-10-14 11:00:00', '2015-10-15 10:59:59', 1);
$segment = 'pageUrl=@/';
$segment = new Segment($segment, $idSites = array());
$query = $segment->getSelectQuery($select, $from, $where, $bind, $orderBy = false, $groupBy);
$logConversionTable = Common::prefixTable('log_conversion');
$logLinkVisitActionTable = Common::prefixTable('log_link_visit_action');
$expectedBind = $bind;
$expectedBind[] = '/';
$expected = array(
"sql" => "
SELECT log_inner.idgoal AS `idgoal`,
log_inner.referer_type AS `referer_type`,
log_inner.referer_name AS `referer_name`,
log_inner.referer_keyword AS `referer_keyword`,
count(*) AS `1`,
count(distinct log_inner.idvisit) AS `3`,
ROUND(SUM(log_inner.revenue),2) AS `2`
FROM (
SELECT log_conversion.idgoal, log_conversion.referer_type, log_conversion.referer_name, log_conversion.referer_keyword, log_conversion.idvisit, log_conversion.revenue
FROM $logConversionTable AS log_conversion
LEFT JOIN $logLinkVisitActionTable AS log_link_visit_action ON log_conversion.idvisit = log_link_visit_action.idvisit
WHERE ( log_conversion.server_time >= ?
AND log_conversion.server_time <= ?
AND log_conversion.idsite IN (?) )
AND ( ( log_link_visit_action.idaction_url IN (SELECT idaction FROM log_action WHERE ( name LIKE CONCAT('%', ?, '%') AND type = 1 )) ) )
GROUP BY CONCAT(log_conversion.idvisit, '_' , log_conversion.idgoal, '_', log_conversion.buster)
ORDER BY NULL )
AS log_inner GROUP BY log_inner.idgoal, log_inner.referer_type, log_inner.referer_name, log_inner.referer_keyword",
"bind" => $expectedBind);
$this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($query));
}
// se https://github.com/piwik/piwik/issues/9194
public function test_getSelectQuery_whenQueryingLogConversionWithSegmentThatUsesLogLinkVisitActionAndLogVisit_shouldUseSubselectGroupedByIdVisitAndBuster()
{
$select = 'log_conversion.idgoal AS `idgoal`,
count(*) AS `1`,
count(distinct log_conversion.idvisit) AS `3`,
ROUND(SUM(log_conversion.revenue),2) AS `2`';
$from = 'log_conversion';
$where = 'log_conversion.server_time >= ? AND log_conversion.server_time <= ? AND log_conversion.idsite IN (?)';
$groupBy = 'log_conversion.idgoal';
$bind = array('2015-10-14 11:00:00', '2015-10-15 10:59:59', 1);
$segment = 'visitorType==returning,visitorType==returningCustomer;pageUrl=@/';
$segment = new Segment($segment, $idSites = array());
$query = $segment->getSelectQuery($select, $from, $where, $bind, $orderBy = false, $groupBy);
$logConversionTable = Common::prefixTable('log_conversion');
$logLinkVisitActionTable = Common::prefixTable('log_link_visit_action');
$logVisitTable = Common::prefixTable('log_visit');
$expectedBind = $bind;
$expectedBind[] = 1;
$expectedBind[] = 2;
$expectedBind[] = '/';
$expected = array(
"sql" => "
SELECT log_inner.idgoal AS `idgoal`, count(*) AS `1`, count(distinct log_inner.idvisit) AS `3`, ROUND(SUM(log_inner.revenue),2) AS `2`
FROM (
SELECT log_conversion.idgoal, log_conversion.idvisit, log_conversion.revenue
FROM $logConversionTable AS log_conversion
LEFT JOIN $logLinkVisitActionTable AS log_link_visit_action ON log_conversion.idvisit = log_link_visit_action.idvisit
LEFT JOIN $logVisitTable AS log_visit ON log_visit.idvisit = log_link_visit_action.idvisit
WHERE ( log_conversion.server_time >= ?
AND log_conversion.server_time <= ?
AND log_conversion.idsite IN (?) )
AND ( (log_visit.visitor_returning = ?
OR log_visit.visitor_returning = ?)
AND ( log_link_visit_action.idaction_url IN (SELECT idaction FROM log_action WHERE ( name LIKE CONCAT('%', ?, '%') AND type = 1 )) ) )
GROUP BY CONCAT(log_conversion.idvisit, '_' , log_conversion.idgoal, '_', log_conversion.buster)
ORDER BY NULL )
AS log_inner
GROUP BY log_inner.idgoal",
"bind" => $expectedBind);
$this->assertEquals($this->removeExtraWhiteSpaces($expected), $this->removeExtraWhiteSpaces($query));
}
/**
* @param $pageUrlFoundInDb
* @return string
......
tests/PHPUnit/System/TrackGoalsAllowMultipleConversionsPerVisitTest.php
+ 10
− 2
Voir le fichier @ fe46664d
......@@ -50,11 +50,19 @@ class TrackGoalsAllowMultipleConversionsPerVisitTest extends SystemTestCase
{
$apiToCall = array(
'VisitTime.getVisitInformationPerServerTime',
'VisitsSummary.get'
'VisitsSummary.get',
'Goals.get'
);
return array(
array($apiToCall, array('idSite' => self::$fixture->idSite, 'date' => self::$fixture->dateTime))
array($apiToCall, array('idSite' => self::$fixture->idSite, 'date' => self::$fixture->dateTime)),
array(array('Goals.get'), array(
'idSite' => self::$fixture->idSite,
'date' => self::$fixture->dateTime,
'segment' => 'pageUrl=@/',
'testSuffix' => '_withLogLinkVisitActionSegment'
)),
);
}
......
tests/PHPUnit/System/TrackGoalsOneConversionPerVisitTest.php 0 → 100755
+ 64
− 0
Voir le fichier @ fe46664d
<?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\Tests\System;
use Piwik\Tests\Framework\TestCase\SystemTestCase;
use Piwik\Tests\Fixtures\TwoVisitsWithCustomVariables;
/**
* @group Plugins
* @group TrackGoalsOneConversionPerVisitTest
*/
class TrackGoalsOneConversionPerVisitTest extends SystemTestCase
{
/**
* @var TwoVisitsWithCustomVariables
*/
public static $fixture = null; // initialized below class definition
/**
* @dataProvider getApiForTesting
*/
public function testApi($api, $params)
{
$this->runApiTests($api, $params);
}
public function getApiForTesting()
{
$apiToCall = array('Goals.get');
return array(
array($apiToCall, array(
'idSite' => self::$fixture->idSite,
'date' => self::$fixture->dateTime,
'periods' => array('day'))),
// test for https://github.com/piwik/piwik/issues/9194 requesting log_conversion with log_link_visit_action segment
array($apiToCall, array(
'idSite' => self::$fixture->idSite,
'date' => self::$fixture->dateTime,
'segment' => 'pageUrl=@/',
'testSuffix' => '_withLogLinkVisitActionSegment'
)),
array($apiToCall, array(
'idSite' => self::$fixture->idSite,
'date' => self::$fixture->dateTime,
'segment' => 'visitCount>=1;pageUrl=@/',
'testSuffix' => '_withLogLinkVisitActionAndLogVisitSegment'
)),
);
}
public static function getOutputPrefix()
{
return 'trackGoals_oneConversionPerVisit';
}
}
TrackGoalsOneConversionPerVisitTest::$fixture = new TwoVisitsWithCustomVariables();
TrackGoalsOneConversionPerVisitTest::$fixture->doExtraQuoteTests = false;
\ No newline at end of file
tests/PHPUnit/System/expected/test_trackGoals_allowMultipleConversionsPerVisit__Goals.get_day.xml 0 → 100644
+ 15
− 0
Voir le fichier @ fe46664d
<?xml version="1.0" encoding="utf-8" ?>
<result>
<nb_conversions>8</nb_conversions>
<nb_visits_converted>2</nb_visits_converted>
<revenue>1332</revenue>
<conversion_rate>100%</conversion_rate>
<nb_conversions_new_visit>8</nb_conversions_new_visit>
<nb_visits_converted_new_visit>2</nb_visits_converted_new_visit>
<revenue_new_visit>1332</revenue_new_visit>
<conversion_rate_new_visit>100%</conversion_rate_new_visit>
<nb_conversions_returning_visit>0</nb_conversions_returning_visit>
<nb_visits_converted_returning_visit>0</nb_visits_converted_returning_visit>
<revenue_returning_visit>0</revenue_returning_visit>
<conversion_rate_returning_visit>0%</conversion_rate_returning_visit>
</result>
\ No newline at end of file
tests/PHPUnit/System/expected/test_trackGoals_allowMultipleConversionsPerVisit_withLogLinkVisitActionSegment__Goals.get_day.xml 0 → 100644
+ 15
− 0
Voir le fichier @ fe46664d
<?xml version="1.0" encoding="utf-8" ?>
<result>
<nb_conversions>8</nb_conversions>
<nb_visits_converted>2</nb_visits_converted>
<revenue>1332</revenue>
<conversion_rate>100%</conversion_rate>
<nb_conversions_new_visit>8</nb_conversions_new_visit>
<nb_visits_converted_new_visit>2</nb_visits_converted_new_visit>
<revenue_new_visit>1332</revenue_new_visit>
<conversion_rate_new_visit>100%</conversion_rate_new_visit>
<nb_conversions_returning_visit>0</nb_conversions_returning_visit>
<nb_visits_converted_returning_visit>0</nb_visits_converted_returning_visit>
<revenue_returning_visit>0</revenue_returning_visit>
<conversion_rate_returning_visit>0%</conversion_rate_returning_visit>
</result>
\ No newline at end of file
tests/PHPUnit/System/expected/test_trackGoals_oneConversionPerVisit__Goals.get_day.xml 0 → 100644
+ 15
− 0
Voir le fichier @ fe46664d
<?xml version="1.0" encoding="utf-8" ?>
<result>
<nb_conversions>3</nb_conversions>
<nb_visits_converted>2</nb_visits_converted>
<revenue>1000</revenue>
<conversion_rate>66.67%</conversion_rate>
<nb_conversions_new_visit>3</nb_conversions_new_visit>
<nb_visits_converted_new_visit>2</nb_visits_converted_new_visit>
<revenue_new_visit>1000</revenue_new_visit>
<conversion_rate_new_visit>66.67%</conversion_rate_new_visit>
<nb_conversions_returning_visit>0</nb_conversions_returning_visit>
<nb_visits_converted_returning_visit>0</nb_visits_converted_returning_visit>
<revenue_returning_visit>0</revenue_returning_visit>
<conversion_rate_returning_visit>0%</conversion_rate_returning_visit>
</result>
\ No newline at end of file
tests/PHPUnit/System/expected/test_trackGoals_oneConversionPerVisit_withLogLinkVisitActionAndLogVisitSegment__Goals.get_day.xml 0 → 100644
+ 15
− 0
Voir le fichier @ fe46664d
<?xml version="1.0" encoding="utf-8" ?>
<result>
<nb_conversions>3</nb_conversions>
<nb_visits_converted>2</nb_visits_converted>
<revenue>1000</revenue>
<conversion_rate>100%</conversion_rate>
<nb_conversions_new_visit>3</nb_conversions_new_visit>
<nb_visits_converted_new_visit>2</nb_visits_converted_new_visit>
<revenue_new_visit>1000</revenue_new_visit>
<conversion_rate_new_visit>100%</conversion_rate_new_visit>
<nb_conversions_returning_visit>0</nb_conversions_returning_visit>
<nb_visits_converted_returning_visit>0</nb_visits_converted_returning_visit>
<revenue_returning_visit>0</revenue_returning_visit>
<conversion_rate_returning_visit>0%</conversion_rate_returning_visit>
</result>
\ No newline at end of file
tests/PHPUnit/System/expected/test_trackGoals_oneConversionPerVisit_withLogLinkVisitActionSegment__Goals.get_day.xml 0 → 100644
+ 15
− 0
Voir le fichier @ fe46664d
<?xml version="1.0" encoding="utf-8" ?>
<result>
<nb_conversions>3</nb_conversions>
<nb_visits_converted>2</nb_visits_converted>
<revenue>1000</revenue>
<conversion_rate>100%</conversion_rate>
<nb_conversions_new_visit>3</nb_conversions_new_visit>
<nb_visits_converted_new_visit>2</nb_visits_converted_new_visit>
<revenue_new_visit>1000</revenue_new_visit>
<conversion_rate_new_visit>100%</conversion_rate_new_visit>
<nb_conversions_returning_visit>0</nb_conversions_returning_visit>
<nb_visits_converted_returning_visit>0</nb_visits_converted_returning_visit>
<revenue_returning_visit>0</revenue_returning_visit>
<conversion_rate_returning_visit>0%</conversion_rate_returning_visit>
</result>
\ No newline at end of file
tests/PHPUnit/System/expected/test_twoVisitsWithCustomVariables_segmentMatchALL_noGoalData__Goals.get_week.xml 0 → 100644
+ 21
− 0
Voir le fichier @ fe46664d
<?xml version="1.0" encoding="utf-8" ?>
<results>
<result idSite="1">
<result date="From 2009-12-28 to 2010-01-03">
<nb_conversions>3</nb_conversions>
<nb_visits_converted>2</nb_visits_converted>
<revenue>1000</revenue>
<conversion_rate>66.67%</conversion_rate>
<nb_conversions_new_visit>3</nb_conversions_new_visit>
<nb_visits_converted_new_visit>2</nb_visits_converted_new_visit>
<revenue_new_visit>1000</revenue_new_visit>
<conversion_rate_new_visit>66.67%</conversion_rate_new_visit>
</result>
<result date="From 2010-01-04 to 2010-01-10" />
<result date="From 2010-01-11 to 2010-01-17" />
<result date="From 2010-01-18 to 2010-01-24" />
<result date="From 2010-01-25 to 2010-01-31" />
<result date="From 2010-02-01 to 2010-02-07" />
<result date="From 2010-02-08 to 2010-02-14" />
</result>
</results>
\ No newline at end of file
tests/PHPUnit/System/expected/test_twoVisitsWithCustomVariables_segmentMatchALL_noGoalData_withLogLinkVisitActionSegment__Goals.get_day.xml 0 → 100644
+ 15
− 0
Voir le fichier @ fe46664d
<?xml version="1.0" encoding="utf-8" ?>
<result>
<nb_conversions>3</nb_conversions>
<nb_visits_converted>2</nb_visits_converted>
<revenue>1000</revenue>
<conversion_rate>100%</conversion_rate>
<nb_conversions_new_visit>3</nb_conversions_new_visit>
<nb_visits_converted_new_visit>2</nb_visits_converted_new_visit>
<revenue_new_visit>1000</revenue_new_visit>
<conversion_rate_new_visit>100%</conversion_rate_new_visit>
<nb_conversions_returning_visit>0</nb_conversions_returning_visit>
<nb_visits_converted_returning_visit>0</nb_visits_converted_returning_visit>
<revenue_returning_visit>0</revenue_returning_visit>
<conversion_rate_returning_visit>0%</conversion_rate_returning_visit>
</result>
\ No newline at end of file
0% ou .
You are about to add 0 people to the discussion. Proceed with caution.
Terminez d'abord l'édition de ce message.
Veuillez vous inscrire ou vous pour commenter