Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added feature Custom Dimensions #9217

Merged
merged 7 commits into from
Nov 25, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
[submodule "tests/UI/expected-ui-screenshots"]
path = tests/UI/expected-ui-screenshots
url = https://github.com/piwik/piwik-ui-tests.git
branch = master
[submodule "tests/travis"]
path = tests/travis
url = https://github.com/piwik/travis-scripts
Expand All @@ -40,9 +41,14 @@
[submodule "plugins/AnonymousPiwikUsageMeasurement"]
path = plugins/AnonymousPiwikUsageMeasurement
url = https://github.com/piwik/plugin-AnonymousPiwikUsageMeasurement.git
branch = master
[submodule "plugins/CustomDimensions"]
path = plugins/CustomDimensions
url = https://github.com/piwik/plugin-CustomDimensions.git
branch = master


# Add new Plugin submodule above this line ^
# Add new Plugin submodule above this line ^^
#
# Note: when you add a submodule that SHOULD be left in the packaged release such as the few submodules below,
# then you MUST add these submodules names in the SUBMODULES_PACKAGED_WITH_CORE variable in:
Expand All @@ -55,3 +61,6 @@
path = misc/log-analytics
url = https://github.com/piwik/piwik-log-analytics.git
branch = master

# Note: do not add new plugin submodules here, but a few lines above

1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ This is a changelog for Piwik platform developers. All changes for our HTTP API'
* When generating a new plugin skeleton via `generate:plugin` command, plugin name must now contain only letters and numbers.
* JavaScript Tracker tests no longer require `SQLite`. The existing MySQL configuration for tests is used now. In order to run the tests make sure Piwik is installed and `[database_tests]` is configured in `config/config.ini.php`.
* The definitions for search engine and social network detection have been moved from bundled data files to a separate package (see [https://github.com/piwik/searchengine-and-social-list](https://github.com/piwik/searchengine-and-social-list)).
* In [UI screenshot tests](https://developer.piwik.org/guides/tests-ui), a test environment `configOverride` setting should be no longer overwritten. Instead new values should be added to the existing `configOverride` array in PHP or JavaScript. For example instead of `testEnvironment.configOverride = {group: {name: 1}}` use `testEnvironment.overrideConfig('group', 'name', '1')`.

### New APIs
* Add your own SMS/Text provider by creating a new class in the `SMSProvider` directory of your plugin. The class has to extend `Piwik\Plugins\MobileMessaging\SMSProvider` and implement the required methods.
Expand Down
10 changes: 9 additions & 1 deletion core/API/DataTableManipulator.php
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,15 @@ private function getApiMethodForSubtable($request)
$idSite = 'all';
}

$meta = API::getInstance()->getMetadata($idSite, $this->apiModule, $this->apiMethod);
$apiParameters = array();
if (!empty($request['idDimension'])) {
$apiParameters['idDimension'] = $request['idDimension'];
}
if (!empty($request['idGoal'])) {
$apiParameters['idGoal'] = $request['idGoal'];
}

$meta = API::getInstance()->getMetadata($idSite, $this->apiModule, $this->apiMethod, $apiParameters);

if (empty($meta)) {
throw new Exception(sprintf(
Expand Down
1 change: 1 addition & 0 deletions core/API/DocumentationGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ public function getExampleUrl($class, $methodName, $parametersToSet = array())
$aParameters['disable_queued_filters'] = false;
$aParameters['disable_generic_filters'] = false;
$aParameters['expanded'] = false;
$aParameters['idDimenson'] = false;

$moduleName = Proxy::getInstance()->getModuleNameFromClassName($class);
$aParameters = array_merge(array('module' => 'API', 'method' => $moduleName . '.' . $methodName), $aParameters);
Expand Down
74 changes: 54 additions & 20 deletions core/DataArray.php
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public function getDataArrayWithTwoLevels()
public function sumMetricsVisits($label, $row)
{
if (!isset($this->data[$label])) {
$this->data[$label] = self::makeEmptyRow();
$this->data[$label] = static::makeEmptyRow();
}
$this->doSumVisitsMetrics($row, $this->data[$label]);
}
Expand Down Expand Up @@ -80,17 +80,14 @@ public static function makeEmptyRow()
*
* @return void
*/
protected function doSumVisitsMetrics($newRowToAdd, &$oldRowToUpdate, $onlyMetricsAvailableInActionsTable = false)
protected function doSumVisitsMetrics($newRowToAdd, &$oldRowToUpdate)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of a boolean parameter splitted logic into doSumVisitsMetrics and doSumActionMetrics

{
// 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[Metrics::INDEX_NB_VISITS])) {
$oldRowToUpdate[Metrics::INDEX_NB_VISITS] += $newRowToAdd['nb_visits'];
$oldRowToUpdate[Metrics::INDEX_NB_ACTIONS] += $newRowToAdd['nb_actions'];
$oldRowToUpdate[Metrics::INDEX_NB_UNIQ_VISITORS] += $newRowToAdd['nb_uniq_visitors'];
if ($onlyMetricsAvailableInActionsTable) {
return;
}
$oldRowToUpdate[Metrics::INDEX_NB_USERS] += $newRowToAdd['nb_users'];
$oldRowToUpdate[Metrics::INDEX_MAX_ACTIONS] = (float)max($newRowToAdd['max_actions'], $oldRowToUpdate[Metrics::INDEX_MAX_ACTIONS]);
$oldRowToUpdate[Metrics::INDEX_SUM_VISIT_LENGTH] += $newRowToAdd['sum_visit_length'];
Expand All @@ -107,9 +104,6 @@ protected function doSumVisitsMetrics($newRowToAdd, &$oldRowToUpdate, $onlyMetri
$oldRowToUpdate[Metrics::INDEX_NB_VISITS] += $newRowToAdd[Metrics::INDEX_NB_VISITS];
$oldRowToUpdate[Metrics::INDEX_NB_ACTIONS] += $newRowToAdd[Metrics::INDEX_NB_ACTIONS];
$oldRowToUpdate[Metrics::INDEX_NB_UNIQ_VISITORS] += $newRowToAdd[Metrics::INDEX_NB_UNIQ_VISITORS];
if ($onlyMetricsAvailableInActionsTable) {
return;
}

// In case the existing Row had no action metrics (eg. Custom Variable XYZ with "visit" scope)
// but the new Row has action metrics (eg. same Custom Variable XYZ this time with a "page" scope)
Expand All @@ -133,11 +127,50 @@ protected function doSumVisitsMetrics($newRowToAdd, &$oldRowToUpdate, $onlyMetri
$oldRowToUpdate[Metrics::INDEX_NB_VISITS_CONVERTED] += $newRowToAdd[Metrics::INDEX_NB_VISITS_CONVERTED];
}

/**
* Adds the given row $newRowToAdd to the existing $oldRowToUpdate passed by reference
* The rows are php arrays Name => value
*
* @param array $newRowToAdd
* @param array $oldRowToUpdate
* @param bool $onlyMetricsAvailableInActionsTable
*
* @return void
*/
protected function doSumActionsMetrics($newRowToAdd, &$oldRowToUpdate)
{
// 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[Metrics::INDEX_NB_VISITS])) {
$oldRowToUpdate[Metrics::INDEX_NB_VISITS] += $newRowToAdd['nb_visits'];
$oldRowToUpdate[Metrics::INDEX_NB_ACTIONS] += $newRowToAdd['nb_actions'];
$oldRowToUpdate[Metrics::INDEX_NB_UNIQ_VISITORS] += $newRowToAdd['nb_uniq_visitors'];
return;
}

// Edge case fail safe
if (!isset($oldRowToUpdate[Metrics::INDEX_NB_VISITS])) {
return;
}

$oldRowToUpdate[Metrics::INDEX_NB_VISITS] += $newRowToAdd[Metrics::INDEX_NB_VISITS];
if (array_key_exists(Metrics::INDEX_NB_ACTIONS, $newRowToAdd)) {
$oldRowToUpdate[Metrics::INDEX_NB_ACTIONS] += $newRowToAdd[Metrics::INDEX_NB_ACTIONS];
}
if (array_key_exists(Metrics::INDEX_PAGE_NB_HITS, $newRowToAdd)) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We noticed sometimes there are hits and sometimes there are actions, needs to work with both

if (!array_key_exists(Metrics::INDEX_PAGE_NB_HITS, $oldRowToUpdate)) {
$oldRowToUpdate[Metrics::INDEX_PAGE_NB_HITS] = 0;
}
$oldRowToUpdate[Metrics::INDEX_PAGE_NB_HITS] += $newRowToAdd[Metrics::INDEX_PAGE_NB_HITS];
}
$oldRowToUpdate[Metrics::INDEX_NB_UNIQ_VISITORS] += $newRowToAdd[Metrics::INDEX_NB_UNIQ_VISITORS];
}

public function sumMetricsGoals($label, $row)
{
$idGoal = $row['idgoal'];
if (!isset($this->data[$label][Metrics::INDEX_GOALS][$idGoal])) {
$this->data[$label][Metrics::INDEX_GOALS][$idGoal] = self::makeEmptyGoalRow($idGoal);
$this->data[$label][Metrics::INDEX_GOALS][$idGoal] = static::makeEmptyGoalRow($idGoal);
}
$this->doSumGoalsMetrics($row, $this->data[$label][Metrics::INDEX_GOALS][$idGoal]);
}
Expand Down Expand Up @@ -201,9 +234,10 @@ protected function doSumGoalsMetrics($newRowToAdd, &$oldRowToUpdate)
public function sumMetricsActions($label, $row)
{
if (!isset($this->data[$label])) {
$this->data[$label] = self::makeEmptyActionRow();
$this->data[$label] = static::makeEmptyActionRow();
}
$this->doSumVisitsMetrics($row, $this->data[$label], $onlyMetricsAvailableInActionsTable = true);

$this->doSumActionsMetrics($row, $this->data[$label]);
}

protected static function makeEmptyActionRow()
Expand All @@ -218,7 +252,7 @@ protected static function makeEmptyActionRow()
public function sumMetricsEvents($label, $row)
{
if (!isset($this->data[$label])) {
$this->data[$label] = self::makeEmptyEventRow();
$this->data[$label] = static::makeEmptyEventRow();
}
$this->doSumEventsMetrics($row, $this->data[$label], $onlyMetricsAvailableInActionsTable = true);
}
Expand Down Expand Up @@ -250,16 +284,16 @@ protected function doSumEventsMetrics($newRowToAdd, &$oldRowToUpdate)
$oldRowToUpdate[Metrics::INDEX_EVENT_NB_HITS] += $newRowToAdd[Metrics::INDEX_EVENT_NB_HITS];
$oldRowToUpdate[Metrics::INDEX_EVENT_NB_HITS_WITH_VALUE] += $newRowToAdd[Metrics::INDEX_EVENT_NB_HITS_WITH_VALUE];

$newRowToAdd[Metrics::INDEX_EVENT_SUM_EVENT_VALUE] = round($newRowToAdd[Metrics::INDEX_EVENT_SUM_EVENT_VALUE], self::EVENT_VALUE_PRECISION);
$newRowToAdd[Metrics::INDEX_EVENT_SUM_EVENT_VALUE] = round($newRowToAdd[Metrics::INDEX_EVENT_SUM_EVENT_VALUE], static::EVENT_VALUE_PRECISION);
$oldRowToUpdate[Metrics::INDEX_EVENT_SUM_EVENT_VALUE] += $newRowToAdd[Metrics::INDEX_EVENT_SUM_EVENT_VALUE];
$oldRowToUpdate[Metrics::INDEX_EVENT_MAX_EVENT_VALUE] = round(max($newRowToAdd[Metrics::INDEX_EVENT_MAX_EVENT_VALUE], $oldRowToUpdate[Metrics::INDEX_EVENT_MAX_EVENT_VALUE]), self::EVENT_VALUE_PRECISION);
$oldRowToUpdate[Metrics::INDEX_EVENT_MAX_EVENT_VALUE] = round(max($newRowToAdd[Metrics::INDEX_EVENT_MAX_EVENT_VALUE], $oldRowToUpdate[Metrics::INDEX_EVENT_MAX_EVENT_VALUE]), static::EVENT_VALUE_PRECISION);

// Update minimum only if it is set
if ($newRowToAdd[Metrics::INDEX_EVENT_MIN_EVENT_VALUE] !== false) {
if ($oldRowToUpdate[Metrics::INDEX_EVENT_MIN_EVENT_VALUE] === false) {
$oldRowToUpdate[Metrics::INDEX_EVENT_MIN_EVENT_VALUE] = round($newRowToAdd[Metrics::INDEX_EVENT_MIN_EVENT_VALUE], self::EVENT_VALUE_PRECISION);
$oldRowToUpdate[Metrics::INDEX_EVENT_MIN_EVENT_VALUE] = round($newRowToAdd[Metrics::INDEX_EVENT_MIN_EVENT_VALUE], static::EVENT_VALUE_PRECISION);
} else {
$oldRowToUpdate[Metrics::INDEX_EVENT_MIN_EVENT_VALUE] = round(min($newRowToAdd[Metrics::INDEX_EVENT_MIN_EVENT_VALUE], $oldRowToUpdate[Metrics::INDEX_EVENT_MIN_EVENT_VALUE]), self::EVENT_VALUE_PRECISION);
$oldRowToUpdate[Metrics::INDEX_EVENT_MIN_EVENT_VALUE] = round(min($newRowToAdd[Metrics::INDEX_EVENT_MIN_EVENT_VALUE], $oldRowToUpdate[Metrics::INDEX_EVENT_MIN_EVENT_VALUE]), static::EVENT_VALUE_PRECISION);
}
}
}
Expand Down Expand Up @@ -290,7 +324,7 @@ public function sumMetrics($label, $row)
public function sumMetricsVisitsPivot($parentLabel, $label, $row)
{
if (!isset($this->dataTwoLevels[$parentLabel][$label])) {
$this->dataTwoLevels[$parentLabel][$label] = self::makeEmptyRow();
$this->dataTwoLevels[$parentLabel][$label] = static::makeEmptyRow();
}
$this->doSumVisitsMetrics($row, $this->dataTwoLevels[$parentLabel][$label]);
}
Expand All @@ -299,7 +333,7 @@ public function sumMetricsGoalsPivot($parentLabel, $label, $row)
{
$idGoal = $row['idgoal'];
if (!isset($this->dataTwoLevels[$parentLabel][$label][Metrics::INDEX_GOALS][$idGoal])) {
$this->dataTwoLevels[$parentLabel][$label][Metrics::INDEX_GOALS][$idGoal] = self::makeEmptyGoalRow($idGoal);
$this->dataTwoLevels[$parentLabel][$label][Metrics::INDEX_GOALS][$idGoal] = static::makeEmptyGoalRow($idGoal);
}
$this->doSumGoalsMetrics($row, $this->dataTwoLevels[$parentLabel][$label][Metrics::INDEX_GOALS][$idGoal]);
}
Expand All @@ -309,7 +343,7 @@ public function sumMetricsActionsPivot($parentLabel, $label, $row)
if (!isset($this->dataTwoLevels[$parentLabel][$label])) {
$this->dataTwoLevels[$parentLabel][$label] = $this->makeEmptyActionRow();
}
$this->doSumVisitsMetrics($row, $this->dataTwoLevels[$parentLabel][$label], $onlyMetricsAvailableInActionsTable = true);
$this->doSumActionsMetrics($row, $this->dataTwoLevels[$parentLabel][$label]);
}

public function sumMetricsEventsPivot($parentLabel, $label, $row)
Expand Down Expand Up @@ -382,7 +416,7 @@ protected function enrichWithConversions(&$data)
*/
public static function isRowActions($row)
{
return (count($row) == count(self::makeEmptyActionRow())) && isset($row[Metrics::INDEX_NB_ACTIONS]);
return (count($row) == count(static::makeEmptyActionRow())) && isset($row[Metrics::INDEX_NB_ACTIONS]);
}

/**
Expand Down
13 changes: 9 additions & 4 deletions core/Plugin/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -1070,11 +1070,20 @@ private function installPluginIfNecessary(Plugin $plugin)

if ($saveConfig) {
PiwikConfig::getInstance()->forceSave();
$this->clearCache($pluginName);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was actually a bug, when a plugin was installed we sometimes did not clear the caches. Eg when installing core plugins

}
}

public function isTrackerPlugin(Plugin $plugin)
{
if (!$this->isPluginInstalled($plugin->getPluginName())) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not load a plugin in tracker mode that is not installed. Otherwise the plugin might wants to read or write to columns that do not exist yet

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good find!

return false;
}

if ($plugin->isTrackerPlugin()) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved it further up as a performance tweak, this way we do not have to check for all dimensions, events, ... when a plugin directly marks itself as a tracker plugin

return true;
}

$dimensions = VisitDimension::getDimensions($plugin);
if (!empty($dimensions)) {
return true;
Expand All @@ -1101,10 +1110,6 @@ public function isTrackerPlugin(Plugin $plugin)
return true;
}

if ($plugin->isTrackerPlugin()) {
return true;
}

return false;
}

Expand Down
3 changes: 2 additions & 1 deletion core/Plugin/RequestProcessors.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ class RequestProcessors
{
public function getRequestProcessors()
{
$processors = Manager::getInstance()->findMultipleComponents('Tracker', 'Piwik\\Tracker\\RequestProcessor');
$manager = Manager::getInstance();
$processors = $manager->findMultipleComponents('Tracker', 'Piwik\\Tracker\\RequestProcessor');

$instances = array();
foreach ($processors as $processor) {
Expand Down
13 changes: 12 additions & 1 deletion core/Plugin/Visualization.php
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,18 @@ private function getReportMetadata()
$idSite = Common::getRequestVar('idSite', null, 'string', $request);
$module = $this->requestConfig->getApiModuleToRequest();
$action = $this->requestConfig->getApiMethodToRequest();
$metadata = ApiApi::getInstance()->getMetadata($idSite, $module, $action);

$apiParameters = array();
$idDimension = Common::getRequestVar('idDimension', 0, 'int');
$idGoal = Common::getRequestVar('idGoal', 0, 'int');
if ($idDimension > 0) {
$apiParameters['idDimension'] = $idDimension;
}
if ($idGoal > 0) {
$apiParameters['idGoal'] = $idGoal;
}

$metadata = ApiApi::getInstance()->getMetadata($idSite, $module, $action, $apiParameters);

if (!empty($metadata)) {
return array_shift($metadata);
Expand Down
19 changes: 12 additions & 7 deletions core/Tracker/Action.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ abstract class Action

private $idLinkVisitAction;
private $actionIdsCached = array();
private $customFields = array();
private $actionName;
private $actionType;

Expand Down Expand Up @@ -228,6 +229,16 @@ protected function getUrlAndType()
return false;
}

public function setCustomField($field, $value)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was needed in order to add any fields for the tracker from a RequestProcessor. We could not use a Dimension here since Dimension work only for one column, but CustomDimensions plugin adds data for many columns under circumstances. We could create a Dimension classes dynamically in the plugin but this would be a bit fragile, at least for now. We can use this for CustomVariables soon as well, this is something I will try next

{
$this->customFields[$field] = $value;
}

public function getCustomFields()
{
return $this->customFields;
}

public function getIdActionUrl()
{
$idUrl = $this->actionIdsCached['idaction_url'];
Expand Down Expand Up @@ -379,13 +390,7 @@ public function record(Visitor $visitor, $idReferrerActionUrl, $idReferrerAction
$visitAction[self::DB_COLUMN_CUSTOM_FLOAT] = Common::forceDotAsSeparatorForDecimalPoint($customValue);
}

$customVariables = $this->getCustomVariables();
if (!empty($customVariables)) {
Common::printDebug("Page level Custom Variables: ");
Common::printDebug($customVariables);
}

$visitAction = array_merge($visitAction, $customVariables);
$visitAction = array_merge($visitAction, $this->customFields);

$this->idLinkVisitAction = $this->getModel()->createAction($visitAction);

Expand Down
32 changes: 30 additions & 2 deletions core/Tracker/Model.php
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ public function createVisit($visit)

public function updateVisit($idSite, $idVisit, $valuesToUpdate)
{
list($updateParts, $sqlBind) = $this->visitFieldsToQuery($valuesToUpdate);
list($updateParts, $sqlBind) = $this->fieldsToQuery($valuesToUpdate);

$parts = implode($updateParts, ', ');
$table = Common::prefixTable('log_visit');
Expand All @@ -312,6 +312,34 @@ public function updateVisit($idSite, $idVisit, $valuesToUpdate)
return $wasInserted;
}

public function updateAction($idLinkVa, $valuesToUpdate)
{
if (empty($idLinkVa)) {
return;
}

list($updateParts, $sqlBind) = $this->fieldsToQuery($valuesToUpdate);

$parts = implode($updateParts, ', ');
$table = Common::prefixTable('log_link_visit_action');

$sqlQuery = "UPDATE $table SET $parts WHERE idlink_va = ?";

$sqlBind[] = $idLinkVa;

$db = $this->getDb();
$result = $db->query($sqlQuery, $sqlBind);
$wasInserted = $db->rowCount($result) != 0;

if (!$wasInserted) {
Common::printDebug("Action with this idLinkVa wasn't found in the DB.");
Common::printDebug("$sqlQuery --- ");
Common::printDebug($sqlBind);
}

return $wasInserted;
}

public function findVisitor($idSite, $configId, $idVisitor, $fieldsToRead, $shouldMatchOneFieldOnly, $isVisitorIdToLookup, $timeLookBack, $timeLookAhead)
{
$selectCustomVariables = '';
Expand Down Expand Up @@ -396,7 +424,7 @@ public function isSiteEmpty($siteId)
return $result == null;
}

private function visitFieldsToQuery($valuesToUpdate)
private function fieldsToQuery($valuesToUpdate)
{
$updateParts = array();
$sqlBind = array();
Expand Down
Loading