Fixes #2667

* Stop aggregating visits for Custom variables of scope "page" * still OK for scope "visit" since there is supposed to be one custom var value per custom variable name for a given visit * Now we always display the Actions columns so that these rows with no visit still show the number of Actions * cleaned up from custom var output report, removed some "price_viewed" column left out UI Change (future FAQ maybe...) * When there is a "Visits" column for the Custom Variable report with a value of "-" (hyphen) then it means this custom variable was used with scope=page. Fixes #2662 * Added integration tests testing getPageUrl with multiple periods and websites * fixed a bug when idSite=all or 1,2,3 git-svn-id: http://dev.piwik.org/svn/trunk@5235 59fd770c-687e-43c8-a1e3-f5a4ff64c105
matomo-org · Sep 27, 2011 · c91c513 · c91c513
1 parent 88b0320
commit c91c513
Show file tree

Hide file tree

Showing 97 changed files with 2,389 additions and 334 deletions.
diff --git a/core/ArchiveProcessing/Day.php b/core/ArchiveProcessing/Day.php
@@ -536,23 +536,27 @@ public function getDataTableWithSubtablesFromArraysIndexedByLabel( $arrayLevel0,
 	 *
 	 * @return array
 	 */
-	public function getNewInterestRow($onlyMetricsAvailableInActionsTable = false)
+	public function getNewInterestRow($onlyMetricsAvailableInActionsTable = false, $doNotSumVisits = false)
 	{
 		if($onlyMetricsAvailableInActionsTable)
 		{
-			return array(	Piwik_Archive::INDEX_NB_UNIQ_VISITORS 	=> 0,
+			if($doNotSumVisits)
+			{
+					return array(Piwik_Archive::INDEX_NB_ACTIONS => 0 );
+			}
+			return array(	
+					Piwik_Archive::INDEX_NB_UNIQ_VISITORS 	=> 0,
 					Piwik_Archive::INDEX_NB_VISITS 			=> 0,
 					Piwik_Archive::INDEX_NB_ACTIONS 		=> 0 );
 		}
-
 		return array(	Piwik_Archive::INDEX_NB_UNIQ_VISITORS 	=> 0,
 						Piwik_Archive::INDEX_NB_VISITS 			=> 0,
 						Piwik_Archive::INDEX_NB_ACTIONS 		=> 0,
 						Piwik_Archive::INDEX_MAX_ACTIONS 		=> 0,
 						Piwik_Archive::INDEX_SUM_VISIT_LENGTH 	=> 0,
 						Piwik_Archive::INDEX_BOUNCE_COUNT 		=> 0,
 						Piwik_Archive::INDEX_NB_VISITS_CONVERTED=> 0,
-						);
+		);
 	}
 
 
@@ -581,15 +585,18 @@ public function getNewInterestRowLabeled( $label )
 	 * @param array $newRowToAdd
 	 * @param array $oldRowToUpdate
 	 */
-	public function updateInterestStats( $newRowToAdd, &$oldRowToUpdate, $onlyMetricsAvailableInActionsTable = false)
+	public function updateInterestStats( $newRowToAdd, &$oldRowToUpdate, $onlyMetricsAvailableInActionsTable = false, $doNotSumVisits = false)
 	{
 		// Pre 1.2 format: string indexed rows are returned from the DB
 		// Left here for Backward compatibility with plugins doing custom SQL queries using these metrics as string
 		if(!isset($newRowToAdd[Piwik_Archive::INDEX_NB_VISITS]))
 		{
-    		$oldRowToUpdate[Piwik_Archive::INDEX_NB_UNIQ_VISITORS]		+= $newRowToAdd['nb_uniq_visitors'];
-    		$oldRowToUpdate[Piwik_Archive::INDEX_NB_VISITS] 			+= $newRowToAdd['nb_visits'];
-    		$oldRowToUpdate[Piwik_Archive::INDEX_NB_ACTIONS] 			+= $newRowToAdd['nb_actions'];
+			if(!$doNotSumVisits)
+			{
+	    		$oldRowToUpdate[Piwik_Archive::INDEX_NB_UNIQ_VISITORS]		+= $newRowToAdd['nb_uniq_visitors'];
+	    		$oldRowToUpdate[Piwik_Archive::INDEX_NB_VISITS] 			+= $newRowToAdd['nb_visits'];
+			}
+			$oldRowToUpdate[Piwik_Archive::INDEX_NB_ACTIONS] 			+= $newRowToAdd['nb_actions'];
     		if($onlyMetricsAvailableInActionsTable)
     		{
     			return;
@@ -600,16 +607,17 @@ public function updateInterestStats( $newRowToAdd, &$oldRowToUpdate, $onlyMetric
     		$oldRowToUpdate[Piwik_Archive::INDEX_NB_VISITS_CONVERTED] 	+= $newRowToAdd['nb_visits_converted'];
     		return;
 		}
-
-		$oldRowToUpdate[Piwik_Archive::INDEX_NB_UNIQ_VISITORS]		+= $newRowToAdd[Piwik_Archive::INDEX_NB_UNIQ_VISITORS];
-		$oldRowToUpdate[Piwik_Archive::INDEX_NB_VISITS] 			+= $newRowToAdd[Piwik_Archive::INDEX_NB_VISITS];
+		if(!$doNotSumVisits)
+		{
+			$oldRowToUpdate[Piwik_Archive::INDEX_NB_UNIQ_VISITORS]		+= $newRowToAdd[Piwik_Archive::INDEX_NB_UNIQ_VISITORS];
+			$oldRowToUpdate[Piwik_Archive::INDEX_NB_VISITS] 			+= $newRowToAdd[Piwik_Archive::INDEX_NB_VISITS];
+		}
 		$oldRowToUpdate[Piwik_Archive::INDEX_NB_ACTIONS] 			+= $newRowToAdd[Piwik_Archive::INDEX_NB_ACTIONS];
 
 		// Hack for Price tracking on Ecommerce product/category pages
 		// The price is not summed, but AVG is taken in the SQL query 
 		$index = Piwik_Archive::INDEX_ECOMMERCE_ITEM_PRICE_VIEWED;
-		if(isset($newRowToAdd[$index])
-			&& !empty($newRowToAdd[$index]))
+		if(!empty($newRowToAdd[$index]))
 		{
 			$oldRowToUpdate[$index] = (float)$newRowToAdd[$index];
 		}

diff --git a/core/DataTable/Filter/AddColumnsProcessedMetrics.php b/core/DataTable/Filter/AddColumnsProcessedMetrics.php
@@ -38,7 +38,9 @@ public function filter($table)
 		foreach($table->getRows() as $key => $row)
 		{
 			$nbVisits = $this->getColumn($row, Piwik_Archive::INDEX_NB_VISITS);
+			$nbActions = $this->getColumn($row, Piwik_Archive::INDEX_NB_ACTIONS);
 			if($nbVisits == 0
+				&& $nbActions == 0
 				&& $this->deleteRowsWithNoVisit)
 			{
 				// case of keyword/website/campaign with a conversion for this day, 
@@ -48,7 +50,7 @@ public function filter($table)
 			}
 
 			$nbVisitsConverted = (int)$this->getColumn($row, Piwik_Archive::INDEX_NB_VISITS_CONVERTED);
-			if($nbVisitsConverted != 0)
+			if($nbVisitsConverted > 0)
 			{
 				$conversionRate = round(100 * $nbVisitsConverted / $nbVisits, $this->roundPrecision);
 				$row->addColumn('conversion_rate', $conversionRate."%");
@@ -63,7 +65,7 @@ public function filter($table)
 				// nb_actions / nb_visits => Actions/visit
 				// sum_visit_length / nb_visits => Avg. Time on Site 
 				// bounce_count / nb_visits => Bounce Rate
-				$actionsPerVisit = round($this->getColumn($row, Piwik_Archive::INDEX_NB_ACTIONS) / $nbVisits, $this->roundPrecision);
+				$actionsPerVisit = round($nbActions / $nbVisits, $this->roundPrecision);
 				$averageTimeOnSite = round($this->getColumn($row, Piwik_Archive::INDEX_SUM_VISIT_LENGTH) / $nbVisits, $rounding = 0);
 				$bounceRate = round(100 * $this->getColumn($row, Piwik_Archive::INDEX_BOUNCE_COUNT) / $nbVisits, $this->roundPrecision);
 			}

diff --git a/core/ViewDataTable/HtmlTable/AllColumns.php b/core/ViewDataTable/HtmlTable/AllColumns.php
@@ -53,6 +53,7 @@ protected function postDataTableLoadedFromAPI()
 		$this->setColumnsToDisplay(array('label', 
 										'nb_visits', 
 										$columnUniqueVisitors, 
+										'nb_actions',
 										'nb_actions_per_visit', 
 										'avg_time_on_site', 
 										'bounce_rate',

diff --git a/plugins/Actions/API.php b/plugins/Actions/API.php
@@ -129,7 +129,11 @@ protected function getFilterPageDatatableSearch($callBackParameters, $search, $a
     		else
     		{
 				$idSite = $callBackParameters[1];
-    			$searchedString = Piwik_Tracker_Action::excludeQueryParametersFromUrl($search, $idSite);
+				try {
+					$searchedString = Piwik_Tracker_Action::excludeQueryParametersFromUrl($search, $idSite);
+				} catch(Exception $e) {
+					$searchedString = $search;
+				}
     		}
 			$searchTree = Piwik_Actions::getActionExplodedNames($searchedString, $actionType);
 		}
@@ -146,6 +150,7 @@ protected function getFilterPageDatatableSearch($callBackParameters, $search, $a
 				// if an array occurs inside the nested table, we only look for the first match (see below)
 				$newTableArray = new Piwik_DataTable_Array;
 				$newTableArray->metadata = $table->metadata;
+				$newTableArray->setKeyName($table->getKeyName());
 
 				foreach ($table->getArray() as $label => $subTable)
 				{

diff --git a/plugins/CustomVariables/API.php b/plugins/CustomVariables/API.php
@@ -77,15 +77,24 @@ public function getCustomVariables($idSite, $period, $date, $segment = false, $e
 	/**
 	 * @return Piwik_DataTable
 	 */
-	public function getCustomVariablesValuesFromNameId($idSite, $period, $date, $idSubtable, $segment = false)
+	public function getCustomVariablesValuesFromNameId($idSite, $period, $date, $idSubtable, $segment = false, $_leavePriceViewedColumn = false)
 	{
 	    $dataTable = $this->getDataTable($idSite, $period, $date, $segment, $expanded = false, $idSubtable);
 
-	    // Hack Ecommerce product price tracking to display correctly
-	    $dataTable->renameColumn('price_viewed', 'price');
-    	$dataTable->queueFilter('ColumnCallbackReplace', 
-    		array('label', create_function('$label', 'return $label == Piwik_CustomVariables::LABEL_CUSTOM_VALUE_NOT_DEFINED ? "'. Piwik_Translate( 'General_NotDefined', Piwik_Translate('CustomVariables_ColumnCustomVariableValue')) .'" : $label;')));
 
+    	if(!$_leavePriceViewedColumn)
+    	{
+    		$dataTable->deleteColumn('price_viewed');
+    	}
+    	else
+    	{
+		    // Hack Ecommerce product price tracking to display correctly
+		    $dataTable->renameColumn('price_viewed', 'price');
+    	}
+    	$dataTable->queueFilter('ColumnCallbackReplace', array('label', create_function('$label', '
+    					return $label == Piwik_CustomVariables::LABEL_CUSTOM_VALUE_NOT_DEFINED 
+    						? "'. Piwik_Translate( 'General_NotDefined', Piwik_Translate('CustomVariables_ColumnCustomVariableValue')) .'" 
+    						: $label;')));
 		return $dataTable;
 	}
 }

diff --git a/plugins/CustomVariables/Controller.php b/plugins/CustomVariables/Controller.php
@@ -32,7 +32,7 @@ function getCustomVariables($fetch = false)
 		$this->setPeriodVariablesView($view);
 		$view->enableShowGoals();
 
-		$view->setColumnsToDisplay( array('label','nb_visits') );
+		$view->setColumnsToDisplay( array('label','nb_visits', 'nb_actions') );
 		$view->setColumnTranslation('label', Piwik_Translate('CustomVariables_ColumnCustomVariableName'));
 		$view->setSortedColumn( 'nb_visits'	 );
 		$view->setLimit( 10 );
@@ -46,8 +46,9 @@ function getCustomVariablesValuesFromNameId( $fetch = false)
 		$view->init( $this->pluginName, __FUNCTION__, 'CustomVariables.getCustomVariablesValuesFromNameId' );
 
 		$view->disableSearchBox();
+		$view->enableShowGoals();
 		$view->disableExcludeLowPopulation();
-		$view->setColumnsToDisplay( array('label','nb_visits') );
+		$view->setColumnsToDisplay( array('label','nb_visits', 'nb_actions') );
 		$view->setColumnTranslation('label', Piwik_Translate('CustomVariables_ColumnCustomVariableValue'));
 
 		return $this->renderView($view, $fetch);

diff --git a/plugins/CustomVariables/CustomVariables.php b/plugins/CustomVariables/CustomVariables.php
@@ -198,6 +198,12 @@ protected function archiveDayAggregate(Piwik_ArchiveProcessing_Day $archiveProce
 
 				$label = $row[$valueField];
 
+				// Remove price tracked if it's zero or we if we are not currently tracking an ecommerce var
+				if(!in_array($row[$keyField], array('_pks', '_pkn', '_pkc')))
+				{
+					unset($row[Piwik_Archive::INDEX_ECOMMERCE_ITEM_PRICE_VIEWED]);
+				}
+
 				// when custom variable value is a JSON array of categories
 				// possibly JSON value
 				$mustInsertCustomVariableValue = true;
@@ -234,12 +240,16 @@ protected function archiveDayAggregate(Piwik_ArchiveProcessing_Day $archiveProce
 					$archiveProcessing->updateInterestStats( $row, $this->interestByCustomVariablesAndValue[$row[$keyField]][$row[$valueField]], $onlyMetricsAvailableInActionsTable);
 				}
 
-				// Hack: when tracking Ecommerce product page view, we do not wish 
-				// to track the "price" in the Custom Variable name report, only in the values report
+				// Do not report on Price viewed for the Custom Variable names 
 				unset($row[Piwik_Archive::INDEX_ECOMMERCE_ITEM_PRICE_VIEWED]);
 
-				if(!isset($this->interestByCustomVariables[$row[$keyField]])) $this->interestByCustomVariables[$row[$keyField]]= $archiveProcessing->getNewInterestRow($onlyMetricsAvailableInActionsTable);
-				$archiveProcessing->updateInterestStats( $row, $this->interestByCustomVariables[$row[$keyField]], $onlyMetricsAvailableInActionsTable);
+				// When tracking Custom Variables with scope=page we do not add up visits numbers
+				// as it is incorrect to sum visits this way
+				// for scope=visit this is allowed, since there is supposed to be one custom var value per custom variable name for a given visit
+				$doNotSumVisits = true;
+
+				if(!isset($this->interestByCustomVariables[$row[$keyField]])) $this->interestByCustomVariables[$row[$keyField]]= $archiveProcessing->getNewInterestRow($onlyMetricsAvailableInActionsTable, $doNotSumVisits);
+				$archiveProcessing->updateInterestStats( $row, $this->interestByCustomVariables[$row[$keyField]], $onlyMetricsAvailableInActionsTable, $doNotSumVisits);
 			}
 
 			// Custom Vars names and values metrics for Goals

diff --git a/plugins/Goals/API.php b/plugins/Goals/API.php
@@ -295,7 +295,7 @@ protected function renameNotDefinedRow($dataTable, $notDefinedStringPretty)
 
 	protected function enrichItemsDataTableWithItemsViewMetrics($dataTable, $idSite, $period, $date, $idSubtable)
 	{
-		$ecommerceViews = Piwik_CustomVariables_API::getInstance()->getCustomVariablesValuesFromNameId($idSite, $period, $date, $idSubtable);
+		$ecommerceViews = Piwik_CustomVariables_API::getInstance()->getCustomVariablesValuesFromNameId($idSite, $period, $date, $idSubtable, $segment = false, $_leavePriceViewedColumn = true);
 
 		// For Product names and SKU reports, and for Category report 
 		// Use the Price (tracked on page views) 

diff --git a/tests/integration/Integration.php b/tests/integration/Integration.php
@@ -364,10 +364,10 @@ function callGetApiCompareOutput($testName, $formats = 'xml', $idSite = false, $
 			// Used in Actions.getPageUrl, .getDownload, etc.
 			// tied to Main.test.php doTest_oneVisitorTwoVisits
 			// will need refactoring when these same API functions are tested in a new function
-			'downloadUrl' 	=> 'http://piwik.org/path/again/latest.zip?phpsessid=this is ignored when searching',
-			'outlinkUrl' 	=> 'http://dev.piwik.org/svn',
-			'pageUrl' 		=> 'http://example.org/index.htm?sessionid=this is also ignored by default',
-			'pageName' 		=> ' Checkout / Purchasing... ',
+			'downloadUrl' 	=> urlencode('http://piwik.org/path/again/latest.zip?phpsessid=this is ignored when searching'),
+			'outlinkUrl' 	=> urlencode('http://dev.piwik.org/svn'),
+			'pageUrl' 		=> urlencode('http://example.org/index.htm?sessionid=this is also ignored by default'),
+			'pageName' 		=> urlencode(' Checkout / Purchasing... '),
 
 			// do not show the millisec timer in response or tests would always fail as value is changing
 			'showTimer'     => 0,
@@ -396,7 +396,7 @@ function callGetApiCompareOutput($testName, $formats = 'xml', $idSite = false, $
 		{
 			$parametersToSet['idGoal'] = $idGoal;
 		}
-		// Give it enough time for the current API test to finish (call all get* APIs)
+
 		Zend_Registry::get('config')->General->time_before_today_archive_considered_outdated = 10;
 
 		$requestUrls = $this->generateUrlsApi($parametersToSet, $formats, $periods, $setDateLastN, $language, $segment);