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.

4 files changed, 49 insertions, 7 deletions

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);
         }