New feature: allow setting goal revenue to event value for event matching goals. (#13483)

* New feature: allow setting goal revenue to event value for event matching goals.

* Forgot to add update file.

* Add check that event_value_as_revenue is used only if this is an event matching goal.

* Remove console.log.

* Move new goal option to goal revenue section.

* Add another event value as revenue note.

* Move goal conversion to after deprecated event + only override event value if event value is not empty + add tests for this.

* fixing tests

5 files changed, 52 insertions, 17 deletions

diff --git a/plugins/Goals/API.php b/plugins/Goals/API.php
index 5fbe256698..62ebde2c9a 100644
--- a/plugins/Goals/API.php
+++ b/plugins/Goals/API.php
@@ -136,7 +136,8 @@ class API extends \Piwik\Plugin\API
      * @param string $description
      * @return int ID of the new goal
      */
-    public function addGoal($idSite, $name, $matchAttribute, $pattern, $patternType, $caseSensitive = false, $revenue = false, $allowMultipleConversionsPerVisit = false, $description = '')
+    public function addGoal($idSite, $name, $matchAttribute, $pattern, $patternType, $caseSensitive = false, $revenue = false, $allowMultipleConversionsPerVisit = false, $description = '',
+                            $useEventValueAsRevenue = false)
     {
         Piwik::checkUserHasWriteAccess($idSite);
 
@@ -158,6 +159,7 @@ class API extends \Piwik\Plugin\API
             'allow_multiple'  => (int)$allowMultipleConversionsPerVisit,
             'revenue'         => $revenue,
             'deleted'         => 0,
+            'event_value_as_revenue' => (int) $useEventValueAsRevenue,
         );
 
         $idGoal = $this->getModel()->createGoalForSite($idSite, $goal);
@@ -190,7 +192,8 @@ class API extends \Piwik\Plugin\API
      * @param string $description
      * @return void
      */
-    public function updateGoal($idSite, $idGoal, $name, $matchAttribute, $pattern, $patternType, $caseSensitive = false, $revenue = false, $allowMultipleConversionsPerVisit = false, $description = '')
+    public function updateGoal($idSite, $idGoal, $name, $matchAttribute, $pattern, $patternType, $caseSensitive = false, $revenue = false, $allowMultipleConversionsPerVisit = false, $description = '',
+                               $useEventValueAsRevenue = false)
     {
         Piwik::checkUserHasWriteAccess($idSite);
 
@@ -202,7 +205,7 @@ class API extends \Piwik\Plugin\API
 
         $revenue = Common::forceDotAsSeparatorForDecimalPoint((float)$revenue);
 
-        $this->getModel()->updateGoal($idSite, $idGoal, array(
+        $goal = array(
             'name'            => $name,
             'description'     => $description,
             'match_attribute' => $matchAttribute,
@@ -211,13 +214,25 @@ class API extends \Piwik\Plugin\API
             'case_sensitive'  => (int) $caseSensitive,
             'allow_multiple'  => (int) $allowMultipleConversionsPerVisit,
             'revenue'         => $revenue,
-        ));
+            'event_value_as_revenue' => (int) $useEventValueAsRevenue,
+        );
+
+        $this->checkEventValueAsRevenue($goal);
+
+        $this->getModel()->updateGoal($idSite, $idGoal, $goal);
 
         $this->getGoalsInfoStaticCache()->delete(self::getCacheId($idSite));
 
         Cache::regenerateCacheWebsiteAttributes($idSite);
     }
 
+    private function checkEventValueAsRevenue($goal)
+    {
+        if ($goal['event_value_as_revenue'] && !GoalManager::isEventMatchingGoal($goal)) {
+            throw new \Exception("'useEventValueAsRevenue' can only be 1 if the goal matches an event attribute.");
+        }
+    }
+
     private function checkPatternIsValid($patternType, $pattern, $matchAttribute)
     {
         if ($patternType == 'exact'
diff --git a/plugins/Goals/angularjs/manage-goals/manage-goals.controller.js b/plugins/Goals/angularjs/manage-goals/manage-goals.controller.js
index 8ef619c310..f24dd16b5e 100644
--- a/plugins/Goals/angularjs/manage-goals/manage-goals.controller.js
+++ b/plugins/Goals/angularjs/manage-goals/manage-goals.controller.js
@@ -27,7 +27,7 @@
             });
         }
 
-        function initGoalForm(goalMethodAPI, submitText, goalName, description, matchAttribute, pattern, patternType, caseSensitive, revenue, allowMultiple, goalId) {
+        function initGoalForm(goalMethodAPI, submitText, goalName, description, matchAttribute, pattern, patternType, caseSensitive, revenue, allowMultiple, useEventValueAsRevenue, goalId) {
 
             $rootScope.$emit('Goals.beforeInitGoalForm', goalMethodAPI, goalId);
 
@@ -56,6 +56,7 @@
             self.goal.caseSensitive = caseSensitive;
             self.goal.revenue = revenue;
             self.goal.apiMethod = goalMethodAPI;
+            self.goal.useEventValueAsRevenue = useEventValueAsRevenue;
 
             self.goal.submitText = submitText;
             self.goal.goalId = goalId;
@@ -96,8 +97,9 @@
                 parameters.pattern = encodeURIComponent(this.goal.pattern);
                 parameters.caseSensitive = this.goal.caseSensitive == true ? 1 : 0;
             }
-            parameters.revenue = this.goal.revenue;
+            parameters.revenue = this.goal.revenue || 0;
             parameters.allowMultipleConversionsPerVisit = this.goal.allowMultiple == true ? 1 : 0;
+            parameters.useEventValueAsRevenue = this.goal.useEventValueAsRevenue == true ? 1 : 0;
 
             parameters.idGoal = this.goal.goalId;
             parameters.method = this.goal.apiMethod;
@@ -165,14 +167,14 @@
             }
 
             this.showAddEditForm();
-            initGoalForm('Goals.addGoal', _pk_translate('Goals_AddGoal'), '', '', 'url', '', 'contains', /*caseSensitive = */false, /*allowMultiple = */'0', '0');
+            initGoalForm('Goals.addGoal', _pk_translate('Goals_AddGoal'), '', '', 'url', '', 'contains', /*caseSensitive = */false, '', /*allowMultiple = */ false, /*useEventValueAsRevenue = */ false, '0');
             scrollToTop();
-        }
+        };
 
         this.editGoal = function (goalId) {
             this.showAddEditForm();
             var goal = piwik.goals[goalId];
-            initGoalForm("Goals.updateGoal", _pk_translate('Goals_UpdateGoal'), goal.name, goal.description, goal.match_attribute, goal.pattern, goal.pattern_type, (goal.case_sensitive != '0'), goal.revenue, goal.allow_multiple, goalId);
+            initGoalForm("Goals.updateGoal", _pk_translate('Goals_UpdateGoal'), goal.name, goal.description, goal.match_attribute, goal.pattern, goal.pattern_type, (goal.case_sensitive != '0'), goal.revenue, goal.allow_multiple, (goal.event_value_as_revenue != '0'), goalId);
             scrollToTop();
         };
 
diff --git a/plugins/Goals/lang/en.json b/plugins/Goals/lang/en.json
index 0738df1780..93882fa1ec 100644
--- a/plugins/Goals/lang/en.json
+++ b/plugins/Goals/lang/en.json
@@ -43,7 +43,7 @@
         "DaysToConv": "Days to Conversion",
         "Details": "Goal details",
         "DefaultGoalConvertedOncePerVisit": "(default) Goal can only be converted once per visit",
-        "DefaultRevenue": "Goal default revenue is",
+        "DefaultRevenueLabel": "Goal default revenue",
         "DefaultRevenueHelp": "For example, a Contact Form submitted by a visitor may be worth $10 on average. Matomo will help you understand how well your visitors segments are performing.",
         "DeleteGoalConfirm": "Are you sure you want to delete the Goal %s?",
         "DocumentationRevenueGeneratedByProductSales": "Product sales. Excludes tax, shipping and discount",
@@ -110,6 +110,10 @@
         "WhenVisitors": "when visitors",
         "WhereThe": "where the",
         "WhereVisitedPageManuallyCallsJavascriptTrackerLearnMore": "The visited page needs to contain a call to the JavaScript 'trackGoal' method (%1$slearn more%2$s)",
-        "YouCanEnableEcommerceReports": "You can enable %1$s for this website in the %2$s page."
+        "YouCanEnableEcommerceReports": "You can enable %1$s for this website in the %2$s page.",
+        "UseEventValueAsRevenue": "Use the event value (if it exists) as the goal conversion revenue.",
+        "GoalRevenue": "Goal Revenue",
+        "EventValueAsRevenueHelp": "If the event you are matching has a revenue, and that revenue is tracked as the event value, you can enable this option to record the event value as the goal conversion's revenue. If your goal revenue will not vary per conversion, you can ignore this option and just set a default revenue above.",
+        "EventValueAsRevenueHelp2": "Note: If both a default goal revenue and event value are defined, the event value will be used. If this option is enabled and no event value is sent in a request, the default revenue will be used (if defined)."
     }
 }
diff --git a/plugins/Goals/templates/_formAddGoal.twig b/plugins/Goals/templates/_formAddGoal.twig
index 84620ee1f5..0d6e2b93f3 100644
--- a/plugins/Goals/templates/_formAddGoal.twig
+++ b/plugins/Goals/templates/_formAddGoal.twig
@@ -120,6 +120,7 @@
             </div>
         </div>
 
+
         <div piwik-field uicontrol="checkbox" name="case_sensitive"
              ng-model="manageGoals.goal.caseSensitive"
              ng-show="manageGoals.goal.triggerType != 'manually'"
@@ -133,12 +134,22 @@
              inline-help="{{ 'Goals_HelpOneConversionPerVisit'|translate|e('html_attr') }}">
         </div>
 
-        <div piwik-field uicontrol="text" name="revenue"
+        <h3>{{ 'Goals_GoalRevenue'|translate }} {{ 'Goals_Optional'|translate }}</h3>
+
+        <div piwik-field uicontrol="number" name="revenue"
              ng-model="manageGoals.goal.revenue"
-             introduction="{{ 'Goals_DefaultRevenue'|translate }} {{ 'Goals_Optional'|translate }}"
+             placeholder="{{ 'Goals_DefaultRevenueLabel'|translate }}"
              inline-help="{{ 'Goals_DefaultRevenueHelp'|translate|e('html_attr') }}">
         </div>
 
+        <div piwik-field uicontrol="checkbox" name="use_event_value"
+             ng-model="manageGoals.goal.useEventValueAsRevenue"
+             title="{{ 'Goals_UseEventValueAsRevenue'|translate|e('html_attr') }}"
+             ng-show="manageGoals.goal.matchAttribute == 'event'"
+             inline-help="{{ 'Goals_EventValueAsRevenueHelp'|translate|e('html_attr') }} &lt;br/&gt;&lt;br/&gt; {{ 'Goals_EventValueAsRevenueHelp2'|translate|e('html_attr') }}"
+        >
+        </div>
+
         {{ postEvent("Template.endGoalEditTable") }}
 
         <input type="hidden" name="goalIdUpdate" value=""/>
diff --git a/plugins/Goals/tests/Integration/APITest.php b/plugins/Goals/tests/Integration/APITest.php
index 740dff436f..0d7b3cccc0 100644
--- a/plugins/Goals/tests/Integration/APITest.php
+++ b/plugins/Goals/tests/Integration/APITest.php
@@ -57,9 +57,9 @@ class APITest extends IntegrationTestCase
 
     public function test_addGoal_ShouldSucceed_IfAllFieldsGiven()
     {
-        $idGoal = $this->api->addGoal($this->idSite, 'MyName', 'url', 'http://www.test.de', 'exact', true, 50, true);
+        $idGoal = $this->api->addGoal($this->idSite, 'MyName', 'url', 'http://www.test.de', 'exact', true, 50, true, 'desc', true);
 
-        $this->assertGoal($idGoal, 'MyName', '', 'url', 'http://www.test.de', 'exact', 1, 50, 1);
+        $this->assertGoal($idGoal, 'MyName', 'desc', 'url', 'http://www.test.de', 'exact', 1, 50, 1, 1);
     }
 
     public function test_addGoal_ShouldSucceed_IfExactPageTitle()
@@ -246,6 +246,7 @@ class APITest extends IntegrationTestCase
             'allow_multiple' => '0',
             'revenue' => '0',
             'deleted' => '0',
+            'event_value_as_revenue' => '0',
         ), $goal);
     }
 
@@ -261,7 +262,8 @@ class APITest extends IntegrationTestCase
         $this->assertEmpty($goals);
     }
 
-    private function assertGoal($idGoal, $name, $description, $url, $pattern, $patternType, $caseSenstive = 0, $revenue = 0, $allowMultiple = 0)
+    private function assertGoal($idGoal, $name, $description, $url, $pattern, $patternType, $caseSenstive = 0, $revenue = 0, $allowMultiple = 0,
+                                $eventAsRevenue = 0)
     {
         $expected = array($idGoal => array(
             'idsite' => $this->idSite,
@@ -274,7 +276,8 @@ class APITest extends IntegrationTestCase
             'case_sensitive' => $caseSenstive,
             'allow_multiple' => $allowMultiple,
             'revenue' => $revenue,
-            'deleted' => 0
+            'deleted' => 0,
+            'event_value_as_revenue' => $eventAsRevenue,
         ));
 
         $goals = $this->getGoals();