Skip to content

Commit

Permalink
Fixes #6313 - When a user starts to get a User ID set, then we set th…
Browse files Browse the repository at this point in the history 
…is User ID to the existing visit rather than create a new visit for this user id specifically
  • Loading branch information
@mattab
mattab committed Oct 1, 2014
1 parent 2cd5f09 commit cd1b52d
Show file tree
Hide file tree
Showing 42 changed files with 301 additions and 240 deletions.
4 changes: 3 additions & 1 deletion core/Tracker/Action.php
View file
Expand Up @@ -357,7 +357,9 @@ public function record(Visitor $visitor, $idReferrerActionUrl, $idReferrerAction
$visitAction['idlink_va'] = $this->idLinkVisitAction;

Common::printDebug("Inserted new action:");
Common::printDebug($visitAction);
$visitActionDebug = $visitAction;
$visitActionDebug['idvisitor'] = bin2hex($visitActionDebug['idvisitor']);
Common::printDebug($visitActionDebug);

/**
* Triggered after successfully persisting a [visit action entity](/guides/persistence-and-the-mysql-backend#visit-actions).
Expand Down
7 changes: 3 additions & 4 deletions core/Tracker/Request.php
View file
Expand Up @@ -551,12 +551,11 @@ public function setForceDateTime($dateTime)
public function getForcedUserId()
{
$userId = $this->getParam('uid');

if (strlen($userId) > 0) {
return $userId;
}

return null;
return false;
}

public function getForcedVisitorId()
Expand Down Expand Up @@ -608,8 +607,8 @@ private function truncateIdAsVisitorId($idVisitor)
* @param $userId
* @return string
*/
private function getUserIdHashed($userId)
public function getUserIdHashed($userId)
{
return sha1($userId);
return substr( sha1( $userId ), 0, 16);
}
}
31 changes: 26 additions & 5 deletions core/Tracker/Visit.php
View file
Expand Up @@ -513,11 +513,7 @@ private function getExistingVisitFieldsToUpdate($visitor, $action, $visitIsConve
{
$valuesToUpdate = array();

// Might update the idvisitor when it was forced or overwritten for this visit
if (strlen($this->visitorInfo['idvisitor']) == Tracker::LENGTH_BINARY_ID) {
$valuesToUpdate['idvisitor'] = $this->visitorInfo['idvisitor'];
$visitor->setVisitorColumn('idvisitor', $this->visitorInfo['idvisitor']);
}
$valuesToUpdate = $this->setIdVisitorForExistingVisit($visitor, $valuesToUpdate);

$dimensions = $this->getAllVisitDimensions();
$valuesToUpdate = $this->triggerHookOnDimensions($dimensions, 'onExistingVisit', $visitor, $action, $valuesToUpdate);
Expand Down Expand Up @@ -613,4 +609,29 @@ private function getVisitStandardLength()
{
return Config::getInstance()->Tracker['visit_standard_length'];
}

/**
* @param $visitor
* @param $valuesToUpdate
* @return mixed
*/
private function setIdVisitorForExistingVisit($visitor, $valuesToUpdate)
{
// Might update the idvisitor when it was forced or overwritten for this visit
if (strlen($this->visitorInfo['idvisitor']) == Tracker::LENGTH_BINARY_ID) {
$binIdVisitor = $this->visitorInfo['idvisitor'];
$visitor->setVisitorColumn('idvisitor', $binIdVisitor);
$valuesToUpdate['idvisitor'] = $binIdVisitor;
}

// User ID takes precedence and overwrites idvisitor value
$userId = $this->request->getForcedUserId();
if ($userId) {
$userIdHash = $this->request->getUserIdHashed($userId);
$binIdVisitor = Common::hex2bin($userIdHash);
$visitor->setVisitorColumn('idvisitor', $binIdVisitor);
$valuesToUpdate['idvisitor'] = $binIdVisitor;
}
return $valuesToUpdate;
}
}
26 changes: 18 additions & 8 deletions core/Tracker/Visitor.php
View file
Expand Up @@ -118,7 +118,8 @@ public function recognize()
// We use a UNION here so that each sql query uses its own INDEX
else {
// will use INDEX index_idsite_config_datetime (idsite, config_id, visit_last_action_time)
$where = ' AND config_id = ?';
$where = ' AND config_id = ?
AND user_id IS NULL ';
$bindSql[] = $configId;
$sqlConfigId = "$select ,
0 as priority
Expand Down Expand Up @@ -226,22 +227,31 @@ protected function getWindowLookupThisVisit()

protected function shouldLookupOneVisitorFieldOnly($isVisitorIdToLookup)
{
$isForcedUserIdMustMatch = (false !== $this->request->getForcedUserId());
if($isForcedUserIdMustMatch) {
// if &iud was set, we must try and match both idvisitor and config_id
return false;
}

// This setting would be enabled for Intranet websites, to ensure that visitors using all the same computer config, same IP
// are not counted as 1 visitor. In this case, we want to enforce and trust the visitor ID from the cookie.
$trustCookiesOnly = Config::getInstance()->Tracker['trust_visitors_cookies'];
if($isVisitorIdToLookup && $trustCookiesOnly) {
return true;
}

// If a &cid= was set, we force to select this visitor (or create a new one)
$isForcedVisitorIdMustMatch = ($this->request->getForcedVisitorId() != null);

// if &iud was set, we force to select this visitor (or create new one)
$isForcedUserIdMustMatch = ($this->request->getForcedUserId() !== null);
if($isForcedVisitorIdMustMatch) {
return true;
}

$shouldMatchOneFieldOnly = (($isVisitorIdToLookup && $trustCookiesOnly)
|| $isForcedVisitorIdMustMatch
|| $isForcedUserIdMustMatch
|| !$isVisitorIdToLookup);
if( !$isVisitorIdToLookup ) {
return true;
}

return $shouldMatchOneFieldOnly;
return false;
}

/**
Expand Down
3 changes: 2 additions & 1 deletion libs/PiwikTracker/PiwikTracker.php
View file
Expand Up @@ -1063,6 +1063,8 @@ public function setUserId($userId)
/**
* Hash function used internally by Piwik to hash a User ID into the Visitor ID.
*
* Note: matches implementation of Tracker\Request->getUserIdHashed()
*
* @param $id
* @return string
*/
Expand All @@ -1071,7 +1073,6 @@ static public function getUserIdHashed($id)
return substr( sha1( $id ), 0, 16);
}


/**
* Forces the requests to be recorded for the specified Visitor ID.
* Note: it is recommended to use ->setUserId($userId); instead.
Expand Down
4 changes: 2 additions & 2 deletions plugins/Contents/tests/expected/test_Contents__Live.getLastVisitsDetails_day.xml
View file
Expand Up @@ -28,7 +28,7 @@

<searches>0</searches>
<actions>1</actions>
<userId>0</userId>
<userId />
<visitorType>new</visitorType>
<visitorTypeIcon />
<visitConverted>0</visitConverted>
Expand Down Expand Up @@ -128,7 +128,7 @@

<searches>0</searches>
<actions>1</actions>
<userId>0</userId>
<userId />
<visitorType>new</visitorType>
<visitorTypeIcon />
<visitConverted>0</visitConverted>
Expand Down
4 changes: 2 additions & 2 deletions plugins/Contents/tests/expected/test_Contents__Live.getLastVisitsDetails_month.xml
View file
Expand Up @@ -28,7 +28,7 @@

<searches>0</searches>
<actions>1</actions>
<userId>0</userId>
<userId />
<visitorType>new</visitorType>
<visitorTypeIcon />
<visitConverted>0</visitConverted>
Expand Down Expand Up @@ -128,7 +128,7 @@

<searches>0</searches>
<actions>1</actions>
<userId>0</userId>
<userId />
<visitorType>new</visitorType>
<visitorTypeIcon />
<visitConverted>0</visitConverted>
Expand Down
4 changes: 2 additions & 2 deletions ...s/tests/expected/test_Contents_contentInteractionMatch__Live.getLastVisitsDetails_day.xml
View file
Expand Up @@ -28,7 +28,7 @@

<searches>0</searches>
<actions>1</actions>
<userId>0</userId>
<userId />
<visitorType>new</visitorType>
<visitorTypeIcon />
<visitConverted>0</visitConverted>
Expand Down Expand Up @@ -128,7 +128,7 @@

<searches>0</searches>
<actions>1</actions>
<userId>0</userId>
<userId />
<visitorType>new</visitorType>
<visitorTypeIcon />
<visitConverted>0</visitConverted>
Expand Down
4 changes: 2 additions & 2 deletions ...ntents/tests/expected/test_Contents_contentTargetMatch__Live.getLastVisitsDetails_day.xml
View file
Expand Up @@ -28,7 +28,7 @@

<searches>0</searches>
<actions>1</actions>
<userId>0</userId>
<userId />
<visitorType>new</visitorType>
<visitorTypeIcon />
<visitConverted>0</visitConverted>
Expand Down Expand Up @@ -128,7 +128,7 @@

<searches>0</searches>
<actions>1</actions>
<userId>0</userId>
<userId />
<visitorType>new</visitorType>
<visitorTypeIcon />
<visitConverted>0</visitConverted>
Expand Down
4 changes: 2 additions & 2 deletions ...ts/tests/expected/test_ContentscontentNameOrPieceMatch__Live.getLastVisitsDetails_day.xml
View file
Expand Up @@ -28,7 +28,7 @@

<searches>0</searches>
<actions>1</actions>
<userId>0</userId>
<userId />
<visitorType>new</visitorType>
<visitorTypeIcon />
<visitConverted>0</visitConverted>
Expand Down Expand Up @@ -128,7 +128,7 @@

<searches>0</searches>
<actions>1</actions>
<userId>0</userId>
<userId />
<visitorType>new</visitorType>
<visitorTypeIcon />
<visitConverted>0</visitConverted>
Expand Down
7 changes: 1 addition & 6 deletions plugins/CoreHome/Columns/UserId.php
View file
Expand Up @@ -48,12 +48,7 @@ public function onNewVisit(Request $request, Visitor $visitor, $action)
*/
public function onExistingVisit(Request $request, Visitor $visitor, $action)
{
$forcedUserId = $request->getForcedUserId();
if ($forcedUserId) {
return $forcedUserId;
}

return false;
return $request->getForcedUserId();
}

}
4 changes: 2 additions & 2 deletions plugins/CoreHome/Visitor.php
View file
Expand Up @@ -102,10 +102,10 @@ function getVisitLengthPretty()
function getUserId()
{
if (isset($this->details['user_id'])
&& !is_null($this->details['user_id'])) {
&& strlen($this->details['user_id']) > 0) {
return $this->details['user_id'];
}
return false;
return null;
}

}
2 changes: 1 addition & 1 deletion ...les/tests/expected/test_CustomVariablesIntegrationTest__Live.getLastVisitsDetails_day.xml
View file
Expand Up @@ -69,7 +69,7 @@

<searches>0</searches>
<actions>1</actions>
<userId>0</userId>
<userId />
<visitorType>new</visitorType>
<visitorTypeIcon />
<visitConverted>1</visitConverted>
Expand Down
2 changes: 1 addition & 1 deletion ...es/tests/processed/test_CustomVariablesIntegrationTest__Live.getLastVisitsDetails_day.xml
View file
Expand Up @@ -69,7 +69,7 @@

<searches>0</searches>
<actions>1</actions>
<userId>0</userId>
<userId />
<visitorType>new</visitorType>
<visitorTypeIcon />
<visitConverted>1</visitConverted>
Expand Down

0 comments on commit cd1b52d

Please sign in to comment.