From 3821a56d0677d3602fb77aba52b6fbe55a6de0c1 Mon Sep 17 00:00:00 2001
From: diosmosis <benaka@piwik.pro>
Date: Tue, 7 Jul 2015 22:24:15 -0700
Subject: [PATCH] Move more conversion/actions related logic from
Visit::handle() to plugin specific RequestProcessors in
manipulateVisitProperties method. Also temporarily made GoalManager a
singleton that is set as a static var in GoalsRequestProcessor. Should be in
DI, but until all of Visit::handle() is dealt w/, this can't be done.
---
core/Tracker/RequestProcessor.php | 4 +-
core/Tracker/Visit.php | 33 +++++------------
.../Tracker/ActionsRequestProcessor.php | 12 +++++-
.../Tracker/VisitRequestProcessor.php | 2 +-
.../Goals/Tracker/GoalsRequestProcessor.php | 37 ++++++++++++++++---
.../Tracker/PingRequestProcessor.php | 5 +++
6 files changed, 61 insertions(+), 32 deletions(-)
diff --git a/core/Tracker/RequestProcessor.php b/core/Tracker/RequestProcessor.php
index 253c8f69f4..9b3be1bb3d 100644
--- a/core/Tracker/RequestProcessor.php
+++ b/core/Tracker/RequestProcessor.php
@@ -12,6 +12,8 @@ use Piwik\Tracker\Visit\VisitProperties;
/**
* TODO
+ *
+ * TODO: maybe we should rename manipulateVisitProperties to afterRequestProcessed and rename processRequest to handleRequest or recordLogs
*/
abstract class RequestProcessor
{
@@ -26,7 +28,7 @@ abstract class RequestProcessor
/**
* TODO
*/
- public function manipulateVisitProperties(VisitProperties $visitProperties)
+ public function manipulateVisitProperties(VisitProperties $visitProperties, Request $request)
{
return false;
}
diff --git a/core/Tracker/Visit.php b/core/Tracker/Visit.php
index 123d3f931b..74e490726d 100644
--- a/core/Tracker/Visit.php
+++ b/core/Tracker/Visit.php
@@ -18,6 +18,7 @@ use Piwik\Exception\UnexpectedWebsiteFoundException;
use Piwik\Network\IPUtils;
use Piwik\Piwik;
use Piwik\Plugin\Dimension\VisitDimension;
+use Piwik\Plugins\Goals\Tracker\GoalsRequestProcessor;
use Piwik\SettingsPiwik;
use Piwik\Tracker;
use Piwik\Tracker\Visit\VisitProperties;
@@ -111,29 +112,6 @@ class Visit implements VisitInterface
}
}
- /**
- * Goals & Ecommerce conversions
- */
- /** @var Action $action */
- $action = $this->visitProperties->getRequestMetadata('Actions', 'action');
-
- $goalManager = new GoalManager($this->request);
- $isManualGoalConversion = $goalManager->isManualGoalConversion();
- $requestIsEcommerce = $goalManager->requestIsEcommerce;
-
- if (!empty($action)) {
-
- if (!$requestIsEcommerce && !$isManualGoalConversion) {
-
- $someGoalsConverted = $goalManager->detectGoalsMatchingUrl($this->request->getIdSite(), $action);
- $this->visitProperties->setRequestMetadata('Goals', 'someGoalsConverted', $someGoalsConverted);
- $this->visitProperties->setRequestMetadata('Goals', 'visitIsConverted', $someGoalsConverted);
-
- $action->loadIdsFromLogActionTable();
- }
-
- }
-
/***
* Visitor recognition
*/
@@ -143,8 +121,15 @@ class Visit implements VisitInterface
$this->visitProperties->visitorInfo = $visitor->getVisitorInfo();
+ /** @var Action $action */
+ $action = $this->visitProperties->getRequestMetadata('Actions', 'action');
+
$isNewVisit = $this->isVisitNew($visitor, $action);
+ $goalManager = GoalsRequestProcessor::$goalManager;
+ $isManualGoalConversion = $goalManager->isManualGoalConversion();
+ $requestIsEcommerce = $goalManager->requestIsEcommerce;
+
// TODO: re-add optimization where if custom variables exist in request, don't bother selecting them in Visitor
$this->visitorCustomVariables = $this->request->getCustomVariables($scope = 'visit');
if (!empty($this->visitorCustomVariables)) {
@@ -153,7 +138,7 @@ class Visit implements VisitInterface
}
foreach ($this->requestProcessors as $processor) {
- $abort = $processor->manipulateVisitProperties($this->visitProperties);
+ $abort = $processor->manipulateVisitProperties($this->visitProperties, $this->request);
if ($abort) {
return;
}
diff --git a/plugins/Actions/Tracker/ActionsRequestProcessor.php b/plugins/Actions/Tracker/ActionsRequestProcessor.php
index 5b19b0ac9f..aa7090b224 100644
--- a/plugins/Actions/Tracker/ActionsRequestProcessor.php
+++ b/plugins/Actions/Tracker/ActionsRequestProcessor.php
@@ -28,4 +28,14 @@ class ActionsRequestProcessor extends RequestProcessor
$visitProperties->setRequestMetadata('Actions', 'action', $action);
}
-}
\ No newline at end of file
+
+ public function manipulateVisitProperties(VisitProperties $visitProperties, Request $request)
+ {
+ /** @var Action $action */
+ $action = $visitProperties->getRequestMetadata('Actions', 'action');
+
+ if (!empty($action)) { // other plugins can unset the action if they want
+ $action->loadIdsFromLogActionTable();
+ }
+ }
+}
diff --git a/plugins/CoreHome/Tracker/VisitRequestProcessor.php b/plugins/CoreHome/Tracker/VisitRequestProcessor.php
index 513b78b54a..596d0b926f 100644
--- a/plugins/CoreHome/Tracker/VisitRequestProcessor.php
+++ b/plugins/CoreHome/Tracker/VisitRequestProcessor.php
@@ -43,7 +43,7 @@ class VisitRequestProcessor extends RequestProcessor
return false;
}
- public function manipulateVisitProperties(VisitProperties $visitProperties)
+ public function manipulateVisitProperties(VisitProperties $visitProperties, Request $request)
{
/**
* Triggered after visits are tested for exclusion so plugins can modify the IP address
diff --git a/plugins/Goals/Tracker/GoalsRequestProcessor.php b/plugins/Goals/Tracker/GoalsRequestProcessor.php
index 0cadff1a21..22315f7511 100644
--- a/plugins/Goals/Tracker/GoalsRequestProcessor.php
+++ b/plugins/Goals/Tracker/GoalsRequestProcessor.php
@@ -9,6 +9,7 @@
namespace Piwik\Plugins\Goals\Tracker;
use Piwik\Common;
+use Piwik\Tracker\Action;
use Piwik\Tracker\GoalManager;
use Piwik\Tracker\Request;
use Piwik\Tracker\RequestProcessor;
@@ -21,13 +22,20 @@ use Piwik\Tracker\Visit\VisitProperties;
*/
class GoalsRequestProcessor extends RequestProcessor
{
+ /**
+ * TODO: GoalManager should be stateless and stored in DI.
+ *
+ * @var GoalManager
+ */
+ public static $goalManager = null;
+
public function processRequestParams(VisitProperties $visitProperties, Request $request)
{
- $goalManager = new GoalManager($request); // TODO: GoalManager should be stateless and stored in DI.
+ self::$goalManager = new GoalManager($request);
- if ($goalManager->isManualGoalConversion()) {
+ if (self::$goalManager->isManualGoalConversion()) {
// this request is from the JS call to piwikTracker.trackGoal()
- $someGoalsConverted = $goalManager->detectGoalId($request->getIdSite());
+ $someGoalsConverted = self::$goalManager->detectGoalId($request->getIdSite());
$visitProperties->setRequestMetadata('Goals', 'someGoalsConverted', $someGoalsConverted);
$visitProperties->setRequestMetadata('Goals', 'visitIsConverted', $someGoalsConverted);
@@ -36,11 +44,30 @@ class GoalsRequestProcessor extends RequestProcessor
// if we find a idgoal in the URL, but then the goal is not valid, this is most likely a fake request
if (!$someGoalsConverted) {
- Common::printDebug('Invalid goal tracking request for goal id = ' . $goalManager->idGoal);
+ Common::printDebug('Invalid goal tracking request for goal id = ' . self::$goalManager->idGoal);
return true;
}
}
return false;
}
-}
\ No newline at end of file
+
+ public function manipulateVisitProperties(VisitProperties $visitProperties, Request $request)
+ {
+ $visitsConverted = $visitProperties->getRequestMetadata('Goals', 'visitIsConverted');
+
+ /** @var Action $action */
+ $action = $visitProperties->getRequestMetadata('Actions', 'action');
+
+ // if the visit hasn't already been converted another way (ie, manual goal conversion or ecommerce conversion,
+ // try to convert based on the action)
+ if (!$visitsConverted
+ && $action
+ ) {
+ $someGoalsConverted = self::$goalManager->detectGoalsMatchingUrl($request->getIdSite(), $action);
+
+ $visitProperties->setRequestMetadata('Goals', 'someGoalsConverted', $someGoalsConverted);
+ $visitProperties->setRequestMetadata('Goals', 'visitIsConverted', $someGoalsConverted);
+ }
+ }
+}
diff --git a/plugins/Heartbeat/Tracker/PingRequestProcessor.php b/plugins/Heartbeat/Tracker/PingRequestProcessor.php
index 44224ce0ab..d513e3f223 100644
--- a/plugins/Heartbeat/Tracker/PingRequestProcessor.php
+++ b/plugins/Heartbeat/Tracker/PingRequestProcessor.php
@@ -24,7 +24,12 @@ class PingRequestProcessor extends RequestProcessor
Common::printDebug("-> ping=1 request: we do not track a new action nor a new visit nor any goal.");
$visitProperties->setRequestMetadata('Actions', 'action', null);
+ }
+ }
+ public function manipulateVisitProperties(VisitProperties $visitProperties, Request $request)
+ {
+ if ($this->isPingRequest($request)) {
$visitProperties->setRequestMetadata('Goals', 'someGoalsConverted', false);
$visitProperties->setRequestMetadata('Goals', 'visitIsConverted', false);
}
--
GitLab