From c604bb6e4943f01b75015f09662c49bb51d39198 Mon Sep 17 00:00:00 2001 From: diosmosis Date: Wed, 2 Apr 2014 19:07:44 +0100 Subject: [PATCH 01/29] Fix bug in editing menu item: if subcategory is null, menu item is not changed. --- core/Menu/MenuAbstract.php | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/core/Menu/MenuAbstract.php b/core/Menu/MenuAbstract.php index 1d35509e368..781aed7b81d 100644 --- a/core/Menu/MenuAbstract.php +++ b/core/Menu/MenuAbstract.php @@ -163,10 +163,17 @@ private function applyEdits() $mainMenuToEdit = $edit[0]; $subMenuToEdit = $edit[1]; $newUrl = $edit[2]; - if (!isset($this->menu[$mainMenuToEdit][$subMenuToEdit])) { + + if ($subMenuToEdit === null) { + $menuDataToEdit = @$this->menu[$mainMenuToEdit]; + } else { + $menuDataToEdit = @$this->menu[$mainMenuToEdit][$subMenuToEdit]; + } + + if (empty($menuDataToEdit)) { $this->buildMenuItem($mainMenuToEdit, $subMenuToEdit, $newUrl); } else { - $this->menu[$mainMenuToEdit][$subMenuToEdit]['_url'] = $newUrl; + $menuDataToEdit['_url'] = $newUrl; } } } From 267783eb8a8904efd807d309203987d6dae25d2e Mon Sep 17 00:00:00 2001 From: diosmosis Date: Wed, 2 Apr 2014 19:08:57 +0100 Subject: [PATCH 02/29] Replace CloudAdmin reference to EnterpriseAdmin. --- core/Console.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/Console.php b/core/Console.php index e6d1f91397b..f2db14934f9 100644 --- a/core/Console.php +++ b/core/Console.php @@ -119,9 +119,9 @@ private function getDefaultPiwikCommands() $commands = array( 'Piwik\CliMulti\RequestCommand' ); - - if (class_exists('Piwik\Plugins\CloudAdmin\CloudAdmin')) { - $extra = new \Piwik\Plugins\CloudAdmin\CloudAdmin(); + + if (class_exists('Piwik\Plugins\EnterpriseAdmin\EnterpriseAdmin')) { + $extra = new \Piwik\Plugins\EnterpriseAdmin\EnterpriseAdmin(); $extra->addConsoleCommands($commands); } return $commands; From 23bf6195a3cd3bdb1c24d76690336914f384da0f Mon Sep 17 00:00:00 2001 From: diosmosis Date: Wed, 2 Apr 2014 19:09:18 +0100 Subject: [PATCH 03/29] Tweak Feedback help page text. --- lang/en.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lang/en.json b/lang/en.json index 64cc6df84ce..82dfe851176 100644 --- a/lang/en.json +++ b/lang/en.json @@ -928,7 +928,7 @@ "DoYouHaveBugReportOrFeatureRequest": "Do you have a bug to report or a feature request?", "ViewUserGuides": "Learn how to configure Piwik and how to effectively analyze your data with our %1$suser guides%2$s", "ViewAnswersToFAQ": "View answers to %sFrequently Asked Questions%s", - "VisitTheForums": "Visit the %s Forums%s", + "VisitTheForums": "Visit the %s Forums%s and get help from the community of Piwik users", "LearnWaysToParticipate": "Learn about all the ways you can %s participate%s", "HowToCreateIssue": "Please read the recommendations on writing a good %1$sbug report%2$s or %3$sfeature request%4$s. Then %5$sregister%6$s or %7$slogin%8$s on our issue tracker and create a %9$snew issue%10$s.", "SpecialRequest": "Do you have a special request for the Piwik team?", From 98bc277a08af76e7f498b37b13dada76be727e8f Mon Sep 17 00:00:00 2001 From: Thomas Steur Date: Thu, 3 Apr 2014 01:11:18 +0200 Subject: [PATCH 04/29] started to work on command to set number of available custom variables --- core/Db.php | 13 ++ core/Db/Schema/Mysql.php | 31 +-- .../Commands/SetNumberOfCustomVariables.php | 190 ++++++++++++++++++ plugins/CustomVariables/CustomVariables.php | 11 + plugins/CustomVariables/Model.php | 124 ++++++++++++ plugins/CustomVariables/tests/ModelTest.php | 113 +++++++++++ tests/PHPUnit/Integration/Core/DbTest.php | 32 +++ 7 files changed, 484 insertions(+), 30 deletions(-) create mode 100644 plugins/CustomVariables/Commands/SetNumberOfCustomVariables.php create mode 100644 plugins/CustomVariables/Model.php create mode 100644 plugins/CustomVariables/tests/ModelTest.php create mode 100644 tests/PHPUnit/Integration/Core/DbTest.php diff --git a/core/Db.php b/core/Db.php index f099d014095..3647a5878fc 100644 --- a/core/Db.php +++ b/core/Db.php @@ -330,6 +330,19 @@ static public function dropTables($tables) return self::query("DROP TABLE " . implode(',', $tables)); } + /** + * Get columns information from table + * + * @param string|array $table The name of the table you want to get the columns definition for. + * @return \Zend_Db_Statement + */ + static public function getColumnNamesFromTable($table) + { + $columns = self::fetchAssoc("SHOW COLUMNS FROM " . $table); + + return array_keys($columns); + } + /** * Locks the supplied table or tables. * diff --git a/core/Db/Schema/Mysql.php b/core/Db/Schema/Mysql.php index fa326a9dc40..34c3764f2fd 100644 --- a/core/Db/Schema/Mysql.php +++ b/core/Db/Schema/Mysql.php @@ -192,16 +192,6 @@ public function getTablesCreateSql() location_city varchar(255) DEFAULT NULL, location_latitude float(10, 6) DEFAULT NULL, location_longitude float(10, 6) DEFAULT NULL, - custom_var_k1 VARCHAR(200) DEFAULT NULL, - custom_var_v1 VARCHAR(200) DEFAULT NULL, - custom_var_k2 VARCHAR(200) DEFAULT NULL, - custom_var_v2 VARCHAR(200) DEFAULT NULL, - custom_var_k3 VARCHAR(200) DEFAULT NULL, - custom_var_v3 VARCHAR(200) DEFAULT NULL, - custom_var_k4 VARCHAR(200) DEFAULT NULL, - custom_var_v4 VARCHAR(200) DEFAULT NULL, - custom_var_k5 VARCHAR(200) DEFAULT NULL, - custom_var_v5 VARCHAR(200) DEFAULT NULL, PRIMARY KEY(idvisit), INDEX index_idsite_config_datetime (idsite, config_id, visit_last_action_time), INDEX index_idsite_datetime (idsite, visit_last_action_time), @@ -264,16 +254,6 @@ public function getTablesCreateSql() revenue_shipping float default NULL, revenue_discount float default NULL, - custom_var_k1 VARCHAR(200) DEFAULT NULL, - custom_var_v1 VARCHAR(200) DEFAULT NULL, - custom_var_k2 VARCHAR(200) DEFAULT NULL, - custom_var_v2 VARCHAR(200) DEFAULT NULL, - custom_var_k3 VARCHAR(200) DEFAULT NULL, - custom_var_v3 VARCHAR(200) DEFAULT NULL, - custom_var_k4 VARCHAR(200) DEFAULT NULL, - custom_var_v4 VARCHAR(200) DEFAULT NULL, - custom_var_k5 VARCHAR(200) DEFAULT NULL, - custom_var_v5 VARCHAR(200) DEFAULT NULL, PRIMARY KEY (idvisit, idgoal, buster), UNIQUE KEY unique_idsite_idorder (idsite, idorder), INDEX index_idsite_datetime ( idsite, server_time ) @@ -293,16 +273,7 @@ public function getTablesCreateSql() idaction_event_category INTEGER(10) UNSIGNED DEFAULT NULL, idaction_event_action INTEGER(10) UNSIGNED DEFAULT NULL, time_spent_ref_action INTEGER(10) UNSIGNED NOT NULL, - custom_var_k1 VARCHAR(200) DEFAULT NULL, - custom_var_v1 VARCHAR(200) DEFAULT NULL, - custom_var_k2 VARCHAR(200) DEFAULT NULL, - custom_var_v2 VARCHAR(200) DEFAULT NULL, - custom_var_k3 VARCHAR(200) DEFAULT NULL, - custom_var_v3 VARCHAR(200) DEFAULT NULL, - custom_var_k4 VARCHAR(200) DEFAULT NULL, - custom_var_v4 VARCHAR(200) DEFAULT NULL, - custom_var_k5 VARCHAR(200) DEFAULT NULL, - custom_var_v5 VARCHAR(200) DEFAULT NULL, + custom_float FLOAT NULL DEFAULT NULL, PRIMARY KEY(idlink_va), INDEX index_idvisit(idvisit), diff --git a/plugins/CustomVariables/Commands/SetNumberOfCustomVariables.php b/plugins/CustomVariables/Commands/SetNumberOfCustomVariables.php new file mode 100644 index 00000000000..57fa3f174d1 --- /dev/null +++ b/plugins/CustomVariables/Commands/SetNumberOfCustomVariables.php @@ -0,0 +1,190 @@ +setName('customvariables:set-number-available-custom-variables'); + $this->setDescription('Change the number of available custom variables'); + $this->setHelp("Example: +./console customvariables:set-number-available-custom-variables 10 +=> 10 custom variables will be available in total +"); + $this->addArgument('maxCustomVars', InputArgument::REQUIRED, 'The number of available custom variables'); + } + + protected function execute(InputInterface $input, OutputInterface $output) + { + $numVarsToSet = $this->getNumVariablesToSet($input); + $numChangesToPerform = $this->getNumberOfChangesToPerform($numVarsToSet); + + if (0 === $numChangesToPerform) { + $this->writeSuccessMessage($output, array( + 'Your Piwik is already configured for ' . $numVarsToSet . ' custom variables.' + )); + return; + } + + foreach (Model::getScopes() as $scope) { + $this->printChanges($scope, $numVarsToSet, $output); + } + + if (!$this->confirmChange($output)) { + return; + } + + $output->writeln(''); + $output->writeln('Starting to apply changes'); + $output->writeln(''); + + $this->progress = $this->initProgress($numChangesToPerform, $output); + + foreach (Model::getScopes() as $scope) { + $this->performChange($scope, $numVarsToSet, $output); + } + + $this->progress->finish(); + + $this->writeSuccessMessage($output, array( + 'Your Piwik is now configured for ' . $numVarsToSet . ' custom variables.' + )); + } + + private function initProgress($numChangesToPerform, OutputInterface $output) + { + /** @var \Symfony\Component\Console\Helper\ProgressHelper $progress */ + $progress = $this->getHelperSet()->get('progress'); + $progress->start($output, $numChangesToPerform); + + return $progress; + } + + private function performChange($scope, $numVarsToSet, OutputInterface $output) + { + $model = new Model($scope); + $numCurrentVars = $model->getCurrentNumCustomVars(); + $numDifference = $this->getAbsoluteDifference($numCurrentVars, $numVarsToSet); + + if ($numVarsToSet > $numCurrentVars) { + $this->addCustomVariables($model, $numDifference, $output); + return; + } + + $this->removeCustomVariables($model, $numDifference, $output); + } + + private function getNumVariablesToSet(InputInterface $input) + { + $maxCustomVars = $input->getArgument('maxCustomVars'); + + if (!is_numeric($maxCustomVars)) { + throw new \Exception('The number of available custom variables has to be number'); + } + + $maxCustomVars = (int) $maxCustomVars; + + if ($maxCustomVars <= 0) { + throw new \Exception('There has to be at least 1 custom variable'); + } + + return $maxCustomVars; + } + + private function confirmChange(OutputInterface $output) + { + $output->writeln(''); + + $dialog = $this->getHelperSet()->get('dialog'); + return $dialog->askConfirmation( + $output, + 'Are you sure you want to perform these actions? (y/N)', + false + ); + } + + private function printChanges($scope, $numVarsToSet, OutputInterface $output) + { + $model = new Model($scope); + $scopeName = $model->getScopeName(); + $highestIndex = $model->getHighestCustomVarIndex(); + $numCurrentCustomVars = $model->getCurrentNumCustomVars(); + $numVarsDifference = $this->getAbsoluteDifference($numCurrentCustomVars, $numVarsToSet); + + $output->writeln(''); + $output->writeln(sprintf('Scope "%s"', $scopeName)); + + if ($numVarsToSet > $numCurrentCustomVars) { + + $indexes = implode(',', range($highestIndex + 1, $highestIndex + $numVarsDifference)); + $output->writeln( + sprintf('%s new custom variables having the index(es) %s will be ADDED', $numVarsDifference, $indexes) + ); + + } elseif ($numVarsToSet < $numCurrentCustomVars) { + + $indexes = implode(',', range($highestIndex - $numVarsDifference + 1, $highestIndex)); + $output->writeln( + sprintf("%s existing custom variables having the index(es) %s will be REMOVED.", $numVarsDifference, $indexes) + ); + $output->writeln('This is an irreversible change'); + } + } + + private function getAbsoluteDifference($currentNumber, $numberToSet) + { + return abs($numberToSet - $currentNumber); + } + + private function removeCustomVariables(Model $model, $numberOfVarsToRemove, OutputInterface $output) + { + for ($index = 0; $index < $numberOfVarsToRemove; $index++) { + $indexRemoved = $model->removeCustomVariable(); + $this->progress->advance(); + $output->writeln(' Removed a variable in scope "' . $model->getScopeName() . '" having the index ' . $indexRemoved . ''); + } + } + + private function addCustomVariables(Model $model, $numberOfVarsToAdd, OutputInterface $output) + { + for ($index = 0; $index < $numberOfVarsToAdd; $index++) { + $indexAdded = $model->addCustomVariable(); + $this->progress->advance(); + $output->writeln(' Added a variable in scope "' . $model->getScopeName() . '" having the index ' . $indexAdded . ''); + } + } + + private function getNumberOfChangesToPerform($numVarsToSet) + { + $numChangesToPerform = 0; + + foreach (Model::getScopes() as $scope) { + $model = new Model($scope); + $numCurrentCustomVars = $model->getCurrentNumCustomVars(); + $numChangesToPerform += $this->getAbsoluteDifference($numCurrentCustomVars, $numVarsToSet); + } + + return $numChangesToPerform; + } +} diff --git a/plugins/CustomVariables/CustomVariables.php b/plugins/CustomVariables/CustomVariables.php index 12bc5b007bd..c7e47f08e12 100644 --- a/plugins/CustomVariables/CustomVariables.php +++ b/plugins/CustomVariables/CustomVariables.php @@ -38,6 +38,7 @@ public function getListHooksRegistered() 'API.getReportMetadata' => 'getReportMetadata', 'API.getSegmentDimensionMetadata' => 'getSegmentsMetadata', 'ViewDataTable.configure' => 'configureViewDataTable', + 'Console.addCommands' => 'addConsoleCommands' ); return $hooks; } @@ -52,6 +53,16 @@ public function addMenus() MenuMain::getInstance()->add('General_Visitors', 'CustomVariables_CustomVariables', array('module' => 'CustomVariables', 'action' => 'index'), $display = true, $order = 50); } + public function install() + { + Model::install(); + } + + public function addConsoleCommands(&$commands) + { + $commands[] = __NAMESPACE__ . '\\Commands\\SetNumberOfCustomVariables'; + } + /** * Returns metadata for available reports */ diff --git a/plugins/CustomVariables/Model.php b/plugins/CustomVariables/Model.php new file mode 100644 index 00000000000..992be895e5d --- /dev/null +++ b/plugins/CustomVariables/Model.php @@ -0,0 +1,124 @@ +getScopes())) { + throw new \Exception('Invalid custom variable scope'); + } + + $this->scope = $scope; + } + + public function getScopeName() + { + // actually we should have a class for each scope but don't want to overengineer it for now + switch ($this->scope) { + case self::SCOPE_PAGE: + return 'Page'; + case self::SCOPE_VISIT: + return 'Visit'; + case self::SCOPE_CONVERSION: + return 'Conversion'; + } + } + + public function getCurrentNumCustomVars() + { + $customVarColumns = $this->getCustomVarColumnNames(); + + $currentNumCustomVars = count($customVarColumns) / 2; + + return (int) $currentNumCustomVars; + } + + public function getHighestCustomVarIndex() + { + $columns = $this->getCustomVarColumnNames(); + + if (empty($columns)) { + return 0; + } + + $indexes = array_map(function ($column) { + $onlyNumber = str_replace(array('custom_var_k', 'custom_var_v'), '', $column); + return (int) $onlyNumber; + }, $columns); + + return max($indexes); + } + + private function getCustomVarColumnNames() + { + $dbTable = Common::prefixTable($this->scope); + $columns = Db::getColumnNamesFromTable($dbTable); + + $customVarColumns = array_filter($columns, function ($column) { + return false !== strpos($column, 'custom_var_'); + }); + + return $customVarColumns; + } + + public function removeCustomVariable() + { + $dbTable = Common::prefixTable($this->scope); + $index = $this->getHighestCustomVarIndex(); + + if ($index < 1) { + return null; + } + + Db::exec(sprintf('ALTER TABLE %s DROP COLUMN custom_var_k%d', $dbTable, $index)); + Db::exec(sprintf('ALTER TABLE %s DROP COLUMN custom_var_v%d', $dbTable, $index)); + + return $index; + } + + public function addCustomVariable() + { + $dbTable = Common::prefixTable($this->scope); + $index = $this->getHighestCustomVarIndex() + 1; + + Db::exec(sprintf('ALTER TABLE %s ADD COLUMN custom_var_k%d VARCHAR(200) DEFAULT NULL', $dbTable, $index)); + Db::exec(sprintf('ALTER TABLE %s ADD COLUMN custom_var_v%d VARCHAR(200) DEFAULT NULL', $dbTable, $index)); + + return $index; + } + + public static function getScopes() + { + return array(self::SCOPE_PAGE, self::SCOPE_VISIT, self::SCOPE_CONVERSION); + } + + public static function install() + { + foreach (self::getScopes() as $scope) { + $model = new Model($scope); + for ($index = 0; $index < 5; $index++) { + $model->addCustomVariable(); + } + } + } + +} + diff --git a/plugins/CustomVariables/tests/ModelTest.php b/plugins/CustomVariables/tests/ModelTest.php new file mode 100644 index 00000000000..b141cb4f5df --- /dev/null +++ b/plugins/CustomVariables/tests/ModelTest.php @@ -0,0 +1,113 @@ +assertEquals(array('log_link_visit_action', 'log_visit', 'log_conversion'), Model::getScopes()); + } + + public function testGetScopeName() + { + $this->assertEquals('Page', $this->getPageScope()->getScopeName()); + $this->assertEquals('Visit', $this->getVisitScope()->getScopeName()); + $this->assertEquals('Conversion', $this->getConversionScope()->getScopeName()); + } + + public function test_getCurrentNumCustomVars() + { + $this->assertEquals(5, $this->getPageScope()->getCurrentNumCustomVars()); + $this->assertEquals(5, $this->getVisitScope()->getCurrentNumCustomVars()); + $this->assertEquals(5, $this->getConversionScope()->getCurrentNumCustomVars()); + + $this->getPageScope()->addCustomVariable(); + $this->getConversionScope()->removeCustomVariable(); + + $this->assertEquals(6, $this->getPageScope()->getCurrentNumCustomVars()); + $this->assertEquals(5, $this->getVisitScope()->getCurrentNumCustomVars()); + $this->assertEquals(4, $this->getConversionScope()->getCurrentNumCustomVars()); + } + + public function test_getHighestCustomVarIndex_addCustomVariable_removeCustomVariable() + { + $this->assertEquals(5, $this->getPageScope()->getHighestCustomVarIndex()); + $this->assertEquals(5, $this->getVisitScope()->getHighestCustomVarIndex()); + $this->assertEquals(5, $this->getConversionScope()->getHighestCustomVarIndex()); + + $this->getPageScope()->addCustomVariable(); + $this->getConversionScope()->removeCustomVariable(); + + $this->assertEquals(6, $this->getPageScope()->getHighestCustomVarIndex()); + $this->assertEquals(5, $this->getVisitScope()->getHighestCustomVarIndex()); + $this->assertEquals(4, $this->getConversionScope()->getHighestCustomVarIndex()); + + $this->getConversionScope()->removeCustomVariable(); + $this->getPageScope()->addCustomVariable(); + $this->getVisitScope()->addCustomVariable(); + $this->getPageScope()->addCustomVariable(); + $this->getConversionScope()->removeCustomVariable(); + + $this->assertEquals(8, $this->getPageScope()->getHighestCustomVarIndex()); + $this->assertEquals(6, $this->getVisitScope()->getHighestCustomVarIndex()); + $this->assertEquals(2, $this->getConversionScope()->getHighestCustomVarIndex()); + } + + public function test_removeCustomVariable_shouldNotFailIfRemovesMoreThanExist() + { + $scope = $this->getPageScope(); + + $this->assertEquals(5, $scope->getHighestCustomVarIndex()); + + for ($index = 0; $index < 5; $index++) { + $scope->removeCustomVariable(); + $this->assertEquals(4 - $index, $scope->getHighestCustomVarIndex()); + } + + $this->assertNull($scope->removeCustomVariable()); + $this->assertEquals(0, $scope->getHighestCustomVarIndex()); + $this->assertEquals(0, $scope->getCurrentNumCustomVars()); + } + + public function test_removeCustomVariable_addCustomVariable_ReturnsIndex() + { + $scopeToAdd = $this->getPageScope(); + $scopeToRemove = $this->getVisitScope(); + + for ($index = 0; $index < 5; $index++) { + $this->assertEquals(5 - $index, $scopeToRemove->removeCustomVariable()); + $this->assertEquals(6 + $index, $scopeToAdd->addCustomVariable()); + } + } + + private function getPageScope() + { + return new Model(Model::SCOPE_PAGE); + } + + private function getVisitScope() + { + return new Model(Model::SCOPE_VISIT); + } + + private function getConversionScope() + { + return new Model(Model::SCOPE_CONVERSION); + } + + +} diff --git a/tests/PHPUnit/Integration/Core/DbTest.php b/tests/PHPUnit/Integration/Core/DbTest.php new file mode 100644 index 00000000000..3a8c31e7e8c --- /dev/null +++ b/tests/PHPUnit/Integration/Core/DbTest.php @@ -0,0 +1,32 @@ +assertColumnNames('access', array('login', 'idsite', 'access')); + $this->assertColumnNames('option', array('option_name', 'option_value', 'autoload')); + } + + private function assertColumnNames($tableName, $expectedColumnNames) + { + $colmuns = Db::getColumnNamesFromTable(Common::prefixTable($tableName)); + + $this->assertEquals($expectedColumnNames, $colmuns); + } + +} \ No newline at end of file From 9f687ad2eaea1c02b087175b12c3946ee4694c60 Mon Sep 17 00:00:00 2001 From: diosmosis Date: Thu, 3 Apr 2014 00:54:06 +0100 Subject: [PATCH 05/29] Remove stray newlines in javascript tracking code output, use server side tracking code generation for JS code & for image tracking code, add new SitesManager.getImageTrackingCode API method for server side image tracking code generation, and fix bug where goals for initially selected site in image tracking code generator were not loaded. --- core/Piwik.php | 18 +- .../javascripts/jsTrackingGenerator.js | 244 +++++------------- plugins/SitesManager/API.php | 53 +++- .../Zeitgeist/templates/javascriptCode.tpl | 1 - 4 files changed, 129 insertions(+), 187 deletions(-) diff --git a/core/Piwik.php b/core/Piwik.php index 54d7e543758..a0cff3cdc2d 100644 --- a/core/Piwik.php +++ b/core/Piwik.php @@ -186,16 +186,16 @@ static public function getJavascriptCode($idSite, $piwikUrl, $mergeSubdomains = * this event to customise the JavaScript tracking code that is displayed to the * user. * - * @param array $codeImpl An array containing snippets of code that the event handler - * can modify. Will contain the following elements: + * @param array &$codeImpl An array containing snippets of code that the event handler + * can modify. Will contain the following elements: * - * - **idSite**: The ID of the site being tracked. - * - **piwikUrl**: The tracker URL to use. - * - **options**: A string of JavaScript code that customises - * the JavaScript tracker. + * - **idSite**: The ID of the site being tracked. + * - **piwikUrl**: The tracker URL to use. + * - **options**: A string of JavaScript code that customises + * the JavaScript tracker. * - * The **httpsPiwikUrl** element can be set if the HTTPS - * domain is different from the normal domain. + * The **httpsPiwikUrl** element can be set if the HTTPS + * domain is different from the normal domain. * @param array $parameters The parameters supplied to the `Piwik::getJavascriptCode()`. */ self::postEvent('Piwik.getJavascriptCode', array(&$codeImpl, $parameters)); @@ -889,7 +889,7 @@ protected static function getJavascriptTagOptions($idSite, $mergeSubdomains, $me } $options = ''; if ($mergeSubdomains && !empty($websiteHosts)) { - $options .= PHP_EOL . ' _paq.push(["setCookieDomain", "*.' . $websiteHosts[0] . '"]);' . PHP_EOL; + $options .= ' _paq.push(["setCookieDomain", "*.' . $websiteHosts[0] . '"]);' . PHP_EOL; } if ($mergeAliasUrls && !empty($websiteHosts)) { $urls = '["*.' . implode('","*.', $websiteHosts) . '"]'; diff --git a/plugins/CoreAdminHome/javascripts/jsTrackingGenerator.js b/plugins/CoreAdminHome/javascripts/jsTrackingGenerator.js index d5a9f7363f1..707a3eef946 100644 --- a/plugins/CoreAdminHome/javascripts/jsTrackingGenerator.js +++ b/plugins/CoreAdminHome/javascripts/jsTrackingGenerator.js @@ -12,46 +12,9 @@ exports = require('piwik/Tracking'); /** - * Singleton class used to trigger events on. Plugins can bind to these events - * to customize the tracking code shown to users. + * This class is deprecated. Use server-side events instead. * - * The following events are triggered: - * - * - customizeJavaScriptParams: function (eventObject, params) - * - * Triggered before tracking JavaScript is generated. This event can be used to - * customize how the generated code looks, without having to parse JavaScript. - * - * params is an object containing the data used to create the JavaScript tracking - * code. - * - * - customizeJavaScript: function (eventObject, eventData) - * - * Triggered after tracking JavaScript is generated. This event can be used to - * customize how the generated code looks, if it cannot be done with the - * customizeJavaScriptParams event. - * - * eventData is an object with one element, 'code', which holds the JavaScript - * tracking code. - * - * - customizeTrackerLinkParams: function (eventObject, params) - * - * Triggered before the tracking link is generated. This event can be used to - * customize how the generated link looks, without having to parse the HTML - * link code. - * - * params is an object containing the data used to create the tracking link. - * - * - customizeTrackerLink: function (eventObject, eventData) - * - * Triggered after the tracking link is generated. This event can be used to - * customize how the generated link looks, if the customization cannot be done - * with the customizeTrackerLinkParams event. - * - * eventData is an object with one element, 'code', which holds the tracking - * link HTML. - * - * @constructor + * @deprecated */ var TrackingCodeGenerator = function () { // empty @@ -185,150 +148,81 @@ }; // function that generates JS code - var generateJsCode = function () { - // get params used to generate JS code - var params = { - idSite: $('#js-tracker-website').attr('siteid'), - groupPageTitlesByDomain: $('#javascript-tracking-group-by-domain').is(':checked'), - mergeSubdomains: $('#javascript-tracking-all-subdomains').is(':checked'), - mergeAliasUrls: $('#javascript-tracking-all-aliases').is(':checked'), - visitorCustomVariables: getCustomVariables('javascript-tracking-visitor-cv'), - pageCustomVariables: getCustomVariables('javascript-tracking-page-cv'), - customCampaignNameQueryParam: null, - customCampaignKeywordParam: null, - doNotTrack: $('#javascript-tracking-do-not-track').is(':checked'), - piwikHost: piwikHost, - piwikPath: piwikPath - }; - - if ($('#custom-campaign-query-params-check').is(':checked')) { - params.customCampaignNameQueryParam = $('#custom-campaign-name-query-param').val(); - params.customCampaignKeywordParam = $('#custom-campaign-keyword-query-param').val(); - } - - // allow plugins to modify data used to generate tracking code - $(TrackingCodeGeneratorSingleton).trigger('customizeJavaScriptParams', params); - - var idSite = params.idSite; - - // generate JS - // changes made to this code should be mirrored in core/Piwik.php function getJavascriptCode() - var result = '\n\ -\n\ -'+''; - - // allow plugins to modify generated JavaScript - var eventData = {code: result, piwikHost: piwikHost }; - $(TrackingCodeGeneratorSingleton).trigger('customizeJavaScript', eventData); - result = eventData.code; - - $('#javascript-text').find('textarea').val(result) - }; - - // function that generates image tracker link - var generateImageTrackerLink = function () { - // get data used to generate the link - var generateDataParams = { - idSite: $('#image-tracker-website').attr('siteid'), - actionName: $('#image-tracker-action-name').val(), - piwikHost: piwikHost, - piwikPath: piwikPath - }; - - if ($('#image-tracking-goal-check').is(':checked')) { - generateDataParams.idGoal = $('#image-tracker-goal').val(); - if (generateDataParams.idGoal) { - generateDataParams.revenue = $('#image-tracker-advanced-options').find('.revenue').val(); + if (generateJsCodeAjax) { + generateJsCodeAjax.abort(); } - } - - // allow plugins to modify data used to generate tracking code - $(TrackingCodeGeneratorSingleton).trigger('customizeTrackerLinkParams', generateDataParams); - // generate link HTML - var params = { - idsite: generateDataParams.idSite, - rec: 1 + generateJsCodeAjax = new ajaxHelper(); + generateJsCodeAjax.addParams({ + module: 'API', + format: 'json', + method: 'SitesManager.getJavascriptTag', + idSite: $('#js-tracker-website').attr('siteid') + }, 'GET'); + generateJsCodeAjax.addParams(params, 'POST'); + generateJsCodeAjax.setCallback(function (response) { + generateJsCodeAjax = null; + + $('#javascript-text').find('textarea').val(response.value); + }); + generateJsCodeAjax.send(); }; - if (generateDataParams.actionName) { - params.action_name = generateDataParams.actionName; - } - - if (generateDataParams.idGoal) { - params.idGoal = generateDataParams.idGoal; - if (generateDataParams.revenue) { - params.revenue = generateDataParams.revenue; + // function that generates image tracker link + var generateImageTrackingAjax = null, + generateImageTrackerLink = function () { + // get data used to generate the link + var generateDataParams = { + piwikUrl: piwikHost + piwikPath, + actionName: $('#image-tracker-action-name').val(), + }; + + if ($('#image-tracking-goal-check').is(':checked')) { + generateDataParams.idGoal = $('#image-tracker-goal').val(); + if (generateDataParams.idGoal) { + generateDataParams.revenue = $('#image-tracker-advanced-options').find('.revenue').val(); + } } - } - - var piwikURL = ("https:" == document.location.protocol ? "https://" : "http://") + generateDataParams.piwikHost - + generateDataParams.piwikPath + '/piwik.php'; - var result = '\n\ -\n\ -'; - - // allow plugins to modify tracking link code - var eventData = {code: result}; - $(TrackingCodeGeneratorSingleton).trigger('customizeTrackerLink', eventData); - result = eventData.code; + if (generateImageTrackingAjax) { + generateImageTrackingAjax.abort(); + } - result = result.replace(/[&]/g, "&"); - $('#image-tracking-text').find('textarea').val(result); - }; + generateImageTrackingAjax = new ajaxHelper(); + generateImageTrackingAjax.addParams({ + module: 'API', + format: 'json', + method: 'SitesManager.getImageTrackingCode', + idSite: $('#image-tracker-website').attr('siteid') + }, 'GET'); + generateImageTrackingAjax.addParams(generateDataParams, 'POST'); + generateImageTrackingAjax.setCallback(function (response) { + generateImageTrackingAjax = null; + + $('#image-tracking-text').find('textarea').val(response.value); + }); + generateImageTrackingAjax.send(); + }; // on image link tracker site change, change available goals $('#image-tracker-website').bind('change', function (e, site) { @@ -403,7 +297,7 @@ $('#js-tracker-website').attr('siteid'), '#js-code-options,#image-tracking-code-options', function () { - var imageTrackerSiteId = $('#image-tracker-website').find('.custom_select_main_link').attr('data-siteid'); + var imageTrackerSiteId = $('#image-tracker-website').attr('siteid'); resetGoalSelectItems(imageTrackerSiteId, 'image-tracker-goal'); generateJsCode(); diff --git a/plugins/SitesManager/API.php b/plugins/SitesManager/API.php index 6bbab8de7ec..3baf42679eb 100644 --- a/plugins/SitesManager/API.php +++ b/plugins/SitesManager/API.php @@ -24,6 +24,7 @@ use Piwik\Tracker\Cache; use Piwik\Url; use Piwik\UrlHelper; +use Piwik\ProxyHttp; /** * The SitesManager API gives you full control on Websites in Piwik (create, update and delete), and many methods to retrieve websites based on various attributes. @@ -71,7 +72,10 @@ class API extends \Piwik\Plugin\API * @internal param $ * @return string The Javascript tag ready to be included on the HTML pages */ - public function getJavascriptTag($idSite, $piwikUrl = '', $mergeSubdomains = false, $groupPageTitlesByDomain = false, $mergeAliasUrls = false, $visitorCustomVariables = false, $pageCustomVariables = false, $customCampaignNameQueryParam = false, $customCampaignKeywordParam = false, $doNotTrack = false) + public function getJavascriptTag($idSite, $piwikUrl = '', $mergeSubdomains = false, $groupPageTitlesByDomain = false, + $mergeAliasUrls = false, $visitorCustomVariables = false, $pageCustomVariables = false, + $customCampaignNameQueryParam = false, $customCampaignKeywordParam = false, + $doNotTrack = false) { Piwik::checkUserHasViewAccess($idSite); @@ -80,11 +84,56 @@ public function getJavascriptTag($idSite, $piwikUrl = '', $mergeSubdomains = fal } $piwikUrl = Common::sanitizeInputValues($piwikUrl); - $htmlEncoded = Piwik::getJavascriptCode($idSite, $piwikUrl, $mergeSubdomains, $groupPageTitlesByDomain, $mergeAliasUrls, $visitorCustomVariables, $pageCustomVariables, $customCampaignNameQueryParam, $customCampaignKeywordParam, $doNotTrack); + $htmlEncoded = Piwik::getJavascriptCode($idSite, $piwikUrl, $mergeSubdomains, $groupPageTitlesByDomain, + $mergeAliasUrls, $visitorCustomVariables, $pageCustomVariables, + $customCampaignNameQueryParam, $customCampaignKeywordParam, + $doNotTrack); $htmlEncoded = str_replace(array('
', '
', '
'), '', $htmlEncoded); return $htmlEncoded; } + /** + * Returns image link tracking code for a given site with specified options. + * + * @param int $idSite The ID to generate tracking code for. + * @param string $piwikUrl The domain and URL path to the Piwik installation. + * @param int $idGoal An ID for a goal to trigger a conversion for. + * @param int $revenue The revenue of the goal conversion. Only used if $idGoal is supplied. + * @return string The HTML tracking code. + */ + public function getImageTrackingCode($idSite, $piwikUrl = '', $actionName = false, $idGoal = false, $revenue = false) + { + $urlParams = array('idSite' => $idSite, 'rec' => 1); + + if ($actionName !== false) { + $urlParams['action_name'] = urlencode(Common::unsanitizeInputValue($actionName)); + } + + if ($idGoal !== false) { + $urlParams['idGoal'] = $idGoal; + if ($revenue !== false) { + $urlParams['revenue'] = $revenue; + } + } + + /** + * Triggered when generating image link tracking code server side. Plugins can use + * this event to customise the image tracking code that is displayed to the + * user. + * + * @param string &$piwikHost The domain and URL path to the Piwik installation, eg, + * `'examplepiwik.com/path/to/piwik'`. + * @param array &$urlParams The query parameters used in the element's src + * URL. See Piwik's image tracking docs for more info. + */ + Piwik::postEvent('SitesManager.getImageTrackingCode', array(&$piwikUrl, &$urlParams)); + + $piwikUrl = (ProxyHttp::isHttps() ? "https://" : "http://") . $piwikUrl . '/piwik.php'; + return " +\"\" +"; + } + /** * Returns all websites belonging to the specified group * @param string $group Group name diff --git a/plugins/Zeitgeist/templates/javascriptCode.tpl b/plugins/Zeitgeist/templates/javascriptCode.tpl index 104e39cbbf2..488f39d00c1 100644 --- a/plugins/Zeitgeist/templates/javascriptCode.tpl +++ b/plugins/Zeitgeist/templates/javascriptCode.tpl @@ -10,7 +10,6 @@ var d=document, g=d.createElement('script'), s=d.getElementsByTagName('script')[0]; g.type='text/javascript'; g.defer=true; g.async=true; g.src=u+'piwik.js'; s.parentNode.insertBefore(g,s); })(); - From f2e5c90681631373b43ce2414bd436a251384b67 Mon Sep 17 00:00:00 2001 From: diosmosis Date: Thu, 3 Apr 2014 01:16:50 +0100 Subject: [PATCH 06/29] Do not hide widget icons on widget drag end. --- plugins/Dashboard/javascripts/dashboardObject.js | 1 - 1 file changed, 1 deletion(-) diff --git a/plugins/Dashboard/javascripts/dashboardObject.js b/plugins/Dashboard/javascripts/dashboardObject.js index 86af47f54fa..ccdde5051c2 100644 --- a/plugins/Dashboard/javascripts/dashboardObject.js +++ b/plugins/Dashboard/javascripts/dashboardObject.js @@ -400,7 +400,6 @@ $('object', this).show(); $('.widgetHover', this).removeClass('widgetHover'); $('.widgetTopHover', this).removeClass('widgetTopHover'); - $('.button#close, .button#maximise', this).hide(); if ($('.widget:has(".piwik-graph")', ui.item).length) { reloadWidget($('.widget', ui.item).attr('id')); } From e48578145e3cc815cadc8f41c45569ed54079468 Mon Sep 17 00:00:00 2001 From: mattab Date: Thu, 3 Apr 2014 14:04:03 +1300 Subject: [PATCH 07/29] Remove the ResponseBuilder flag and instead use the existing PIWIK_PRINT_ERROR_BACKTRACE --- core/API/ResponseBuilder.php | 8 ++------ core/testMinimumPhpVersion.php | 3 +-- plugins/CustomAlerts | 2 +- tests/PHPUnit/Core/ReleaseCheckListTest.php | 3 +-- tests/PHPUnit/Integration/Core/LogTest.php | 2 +- ...est_OneVisitorTwoVisits__Referrers.getCleanKeyword.xml | 2 +- ...noData__Transitions.getTransitionsForPageTitle_day.xml | 2 +- ...Data__Transitions.getTransitionsForPageTitle_month.xml | 2 +- ...s_noData__Transitions.getTransitionsForPageUrl_day.xml | 2 +- ...noData__Transitions.getTransitionsForPageUrl_month.xml | 2 +- ...test_apiGetReportMetadata__API.getRowEvolution_day.xml | 2 +- ...ls.Get_NotExistingGoal__API.getProcessedReport_day.xml | 2 +- .../test_noVisit_PeriodIsLast__Referrers.getAll_day.xml | 2 +- .../test_noVisit_PeriodIsLast__Referrers.getAll_week.xml | 2 +- ...noVisit_PeriodIsLast__VisitTime.getByDayOfWeek_day.xml | 2 +- ...oVisit_PeriodIsLast__VisitTime.getByDayOfWeek_week.xml | 2 +- 16 files changed, 17 insertions(+), 23 deletions(-) diff --git a/core/API/ResponseBuilder.php b/core/API/ResponseBuilder.php index 55fce0d0d52..692a2ceb710 100644 --- a/core/API/ResponseBuilder.php +++ b/core/API/ResponseBuilder.php @@ -22,8 +22,6 @@ */ class ResponseBuilder { - const DISPLAY_BACKTRACE_DEBUG = false; - private $request = null; private $outputFormat = null; @@ -162,12 +160,10 @@ protected function decorateExceptionWithDebugTrace(Exception $e) { // If we are in tests, show full backtrace if (defined('PIWIK_PATH_TEST_TO_ROOT')) { - if (self::DISPLAY_BACKTRACE_DEBUG - || \Piwik_ShouldPrintBackTraceWithMessage() - ) { + if (\Piwik_ShouldPrintBackTraceWithMessage()) { $message = $e->getMessage() . " in \n " . $e->getFile() . ":" . $e->getLine() . " \n " . $e->getTraceAsString(); } else { - $message = $e->getMessage() . "\n \n --> To temporarily debug this error further, set const DISPLAY_BACKTRACE_DEBUG=true; in " . basename(__FILE__); + $message = $e->getMessage() . "\n \n --> To temporarily debug this error further, set const PIWIK_PRINT_ERROR_BACKTRACE=true; in index.php"; } return new Exception($message); } diff --git a/core/testMinimumPhpVersion.php b/core/testMinimumPhpVersion.php index a7dbac5a779..8a0e9a9470f 100644 --- a/core/testMinimumPhpVersion.php +++ b/core/testMinimumPhpVersion.php @@ -71,8 +71,7 @@ function Piwik_ShouldPrintBackTraceWithMessage() { $bool = (defined('PIWIK_PRINT_ERROR_BACKTRACE') && PIWIK_PRINT_ERROR_BACKTRACE) - || (defined('PIWIK_TRACKER_DEBUG') && PIWIK_TRACKER_DEBUG) - || \Piwik\API\ResponseBuilder::DISPLAY_BACKTRACE_DEBUG; + || (defined('PIWIK_TRACKER_DEBUG') && PIWIK_TRACKER_DEBUG); return $bool; } diff --git a/plugins/CustomAlerts b/plugins/CustomAlerts index c9edd879003..a1125e96827 160000 --- a/plugins/CustomAlerts +++ b/plugins/CustomAlerts @@ -1 +1 @@ -Subproject commit c9edd879003ca7a9f75d81fe07ec6ff0c2340f91 +Subproject commit a1125e96827e12c7bd5c3d6107b9c162aace32f8 diff --git a/tests/PHPUnit/Core/ReleaseCheckListTest.php b/tests/PHPUnit/Core/ReleaseCheckListTest.php index 139d528819c..02e55aac364 100644 --- a/tests/PHPUnit/Core/ReleaseCheckListTest.php +++ b/tests/PHPUnit/Core/ReleaseCheckListTest.php @@ -88,8 +88,7 @@ public function testCheckThatConfigurationValuesAreProductionValues() require_once PIWIK_INCLUDE_PATH . "/core/TaskScheduler.php"; $this->assertFalse(DEBUG_FORCE_SCHEDULED_TASKS); - require_once PIWIK_INCLUDE_PATH . "/core/API/ResponseBuilder.php"; - $this->assertFalse(\Piwik\API\ResponseBuilder::DISPLAY_BACKTRACE_DEBUG); + $this->assertFalse(PIWIK_PRINT_ERROR_BACKTRACE); } private function _checkEqual($key, $valueExpected) diff --git a/tests/PHPUnit/Integration/Core/LogTest.php b/tests/PHPUnit/Integration/Core/LogTest.php index 67e366404be..4e2fd72ecc0 100644 --- a/tests/PHPUnit/Integration/Core/LogTest.php +++ b/tests/PHPUnit/Integration/Core/LogTest.php @@ -29,7 +29,7 @@ class Core_LogTest extends DatabaseTestCase public static $expectedExceptionOutput = array( 'screen' => 'dummy error message

- --> To temporarily debug this error further, set const DISPLAY_BACKTRACE_DEBUG=true; in ResponseBuilder.php', + --> To temporarily debug this error further, set const PIWIK_PRINT_ERROR_BACKTRACE=true; in index.php', 'file' => '[Core_LogTest] LogTest.php(161): dummy error message dummy backtrace', 'database' => '[Core_LogTest] LogTest.php(161): dummy error message diff --git a/tests/PHPUnit/Integration/expected/test_OneVisitorTwoVisits__Referrers.getCleanKeyword.xml b/tests/PHPUnit/Integration/expected/test_OneVisitorTwoVisits__Referrers.getCleanKeyword.xml index 4f983c144f8..3ad40410575 100644 --- a/tests/PHPUnit/Integration/expected/test_OneVisitorTwoVisits__Referrers.getCleanKeyword.xml +++ b/tests/PHPUnit/Integration/expected/test_OneVisitorTwoVisits__Referrers.getCleanKeyword.xml @@ -2,5 +2,5 @@ + --> To temporarily debug this error further, set const PIWIK_PRINT_ERROR_BACKTRACE=true; in index.php" /> \ No newline at end of file diff --git a/tests/PHPUnit/Integration/expected/test_Transitions_noData__Transitions.getTransitionsForPageTitle_day.xml b/tests/PHPUnit/Integration/expected/test_Transitions_noData__Transitions.getTransitionsForPageTitle_day.xml index 2680f686f76..a0995fa6e93 100644 --- a/tests/PHPUnit/Integration/expected/test_Transitions_noData__Transitions.getTransitionsForPageTitle_day.xml +++ b/tests/PHPUnit/Integration/expected/test_Transitions_noData__Transitions.getTransitionsForPageTitle_day.xml @@ -2,5 +2,5 @@ + --> To temporarily debug this error further, set const PIWIK_PRINT_ERROR_BACKTRACE=true; in index.php" /> \ No newline at end of file diff --git a/tests/PHPUnit/Integration/expected/test_Transitions_noData__Transitions.getTransitionsForPageTitle_month.xml b/tests/PHPUnit/Integration/expected/test_Transitions_noData__Transitions.getTransitionsForPageTitle_month.xml index 2680f686f76..a0995fa6e93 100644 --- a/tests/PHPUnit/Integration/expected/test_Transitions_noData__Transitions.getTransitionsForPageTitle_month.xml +++ b/tests/PHPUnit/Integration/expected/test_Transitions_noData__Transitions.getTransitionsForPageTitle_month.xml @@ -2,5 +2,5 @@ + --> To temporarily debug this error further, set const PIWIK_PRINT_ERROR_BACKTRACE=true; in index.php" /> \ No newline at end of file diff --git a/tests/PHPUnit/Integration/expected/test_Transitions_noData__Transitions.getTransitionsForPageUrl_day.xml b/tests/PHPUnit/Integration/expected/test_Transitions_noData__Transitions.getTransitionsForPageUrl_day.xml index 2680f686f76..a0995fa6e93 100644 --- a/tests/PHPUnit/Integration/expected/test_Transitions_noData__Transitions.getTransitionsForPageUrl_day.xml +++ b/tests/PHPUnit/Integration/expected/test_Transitions_noData__Transitions.getTransitionsForPageUrl_day.xml @@ -2,5 +2,5 @@ + --> To temporarily debug this error further, set const PIWIK_PRINT_ERROR_BACKTRACE=true; in index.php" /> \ No newline at end of file diff --git a/tests/PHPUnit/Integration/expected/test_Transitions_noData__Transitions.getTransitionsForPageUrl_month.xml b/tests/PHPUnit/Integration/expected/test_Transitions_noData__Transitions.getTransitionsForPageUrl_month.xml index 2680f686f76..a0995fa6e93 100644 --- a/tests/PHPUnit/Integration/expected/test_Transitions_noData__Transitions.getTransitionsForPageUrl_month.xml +++ b/tests/PHPUnit/Integration/expected/test_Transitions_noData__Transitions.getTransitionsForPageUrl_month.xml @@ -2,5 +2,5 @@ + --> To temporarily debug this error further, set const PIWIK_PRINT_ERROR_BACKTRACE=true; in index.php" /> \ No newline at end of file diff --git a/tests/PHPUnit/Integration/expected/test_apiGetReportMetadata__API.getRowEvolution_day.xml b/tests/PHPUnit/Integration/expected/test_apiGetReportMetadata__API.getRowEvolution_day.xml index ecdcd30b5bb..29bf42cae82 100644 --- a/tests/PHPUnit/Integration/expected/test_apiGetReportMetadata__API.getRowEvolution_day.xml +++ b/tests/PHPUnit/Integration/expected/test_apiGetReportMetadata__API.getRowEvolution_day.xml @@ -2,5 +2,5 @@ + --> To temporarily debug this error further, set const PIWIK_PRINT_ERROR_BACKTRACE=true; in index.php" /> \ No newline at end of file diff --git a/tests/PHPUnit/Integration/expected/test_ecommerceOrderWithItems_Metadata_Goals.Get_NotExistingGoal__API.getProcessedReport_day.xml b/tests/PHPUnit/Integration/expected/test_ecommerceOrderWithItems_Metadata_Goals.Get_NotExistingGoal__API.getProcessedReport_day.xml index 69a86c5da69..1625ba026c9 100644 --- a/tests/PHPUnit/Integration/expected/test_ecommerceOrderWithItems_Metadata_Goals.Get_NotExistingGoal__API.getProcessedReport_day.xml +++ b/tests/PHPUnit/Integration/expected/test_ecommerceOrderWithItems_Metadata_Goals.Get_NotExistingGoal__API.getProcessedReport_day.xml @@ -3,5 +3,5 @@ + --> To temporarily debug this error further, set const PIWIK_PRINT_ERROR_BACKTRACE=true; in index.php" /> \ No newline at end of file diff --git a/tests/PHPUnit/Integration/expected/test_noVisit_PeriodIsLast__Referrers.getAll_day.xml b/tests/PHPUnit/Integration/expected/test_noVisit_PeriodIsLast__Referrers.getAll_day.xml index 06f6e4a7970..64d7aebcb2f 100644 --- a/tests/PHPUnit/Integration/expected/test_noVisit_PeriodIsLast__Referrers.getAll_day.xml +++ b/tests/PHPUnit/Integration/expected/test_noVisit_PeriodIsLast__Referrers.getAll_day.xml @@ -2,5 +2,5 @@ + --> To temporarily debug this error further, set const PIWIK_PRINT_ERROR_BACKTRACE=true; in index.php" /> \ No newline at end of file diff --git a/tests/PHPUnit/Integration/expected/test_noVisit_PeriodIsLast__Referrers.getAll_week.xml b/tests/PHPUnit/Integration/expected/test_noVisit_PeriodIsLast__Referrers.getAll_week.xml index 06f6e4a7970..64d7aebcb2f 100644 --- a/tests/PHPUnit/Integration/expected/test_noVisit_PeriodIsLast__Referrers.getAll_week.xml +++ b/tests/PHPUnit/Integration/expected/test_noVisit_PeriodIsLast__Referrers.getAll_week.xml @@ -2,5 +2,5 @@ + --> To temporarily debug this error further, set const PIWIK_PRINT_ERROR_BACKTRACE=true; in index.php" /> \ No newline at end of file diff --git a/tests/PHPUnit/Integration/expected/test_noVisit_PeriodIsLast__VisitTime.getByDayOfWeek_day.xml b/tests/PHPUnit/Integration/expected/test_noVisit_PeriodIsLast__VisitTime.getByDayOfWeek_day.xml index 9e698bf229d..547baf7416a 100644 --- a/tests/PHPUnit/Integration/expected/test_noVisit_PeriodIsLast__VisitTime.getByDayOfWeek_day.xml +++ b/tests/PHPUnit/Integration/expected/test_noVisit_PeriodIsLast__VisitTime.getByDayOfWeek_day.xml @@ -2,5 +2,5 @@ + --> To temporarily debug this error further, set const PIWIK_PRINT_ERROR_BACKTRACE=true; in index.php" /> \ No newline at end of file diff --git a/tests/PHPUnit/Integration/expected/test_noVisit_PeriodIsLast__VisitTime.getByDayOfWeek_week.xml b/tests/PHPUnit/Integration/expected/test_noVisit_PeriodIsLast__VisitTime.getByDayOfWeek_week.xml index 9e698bf229d..547baf7416a 100644 --- a/tests/PHPUnit/Integration/expected/test_noVisit_PeriodIsLast__VisitTime.getByDayOfWeek_week.xml +++ b/tests/PHPUnit/Integration/expected/test_noVisit_PeriodIsLast__VisitTime.getByDayOfWeek_week.xml @@ -2,5 +2,5 @@ + --> To temporarily debug this error further, set const PIWIK_PRINT_ERROR_BACKTRACE=true; in index.php" /> \ No newline at end of file From 0e1299365537880b1e5d6e2e2921dc3ab5d3731b Mon Sep 17 00:00:00 2001 From: mattab Date: Thu, 3 Apr 2014 14:22:39 +1300 Subject: [PATCH 08/29] Fix a test --- core/Tracker/Action.php | 3 --- core/testMinimumPhpVersion.php | 2 +- tests/PHPUnit/Core/ReleaseCheckListTest.php | 21 +++++++-------------- 3 files changed, 8 insertions(+), 18 deletions(-) diff --git a/core/Tracker/Action.php b/core/Tracker/Action.php index aa940151d7f..a1d879b52ee 100644 --- a/core/Tracker/Action.php +++ b/core/Tracker/Action.php @@ -190,9 +190,6 @@ public function getIdLinkVisitAction() public function writeDebugInfo() { - if (!isset($GLOBALS['PIWIK_TRACKER_DEBUG']) || !$GLOBALS['PIWIK_TRACKER_DEBUG']) { - return false; - } $type = self::getTypeAsString($this->getActionType()); Common::printDebug("Action is a $type, Action name = " . $this->getActionName() . ", diff --git a/core/testMinimumPhpVersion.php b/core/testMinimumPhpVersion.php index 8a0e9a9470f..2b4766aa36c 100644 --- a/core/testMinimumPhpVersion.php +++ b/core/testMinimumPhpVersion.php @@ -71,7 +71,7 @@ function Piwik_ShouldPrintBackTraceWithMessage() { $bool = (defined('PIWIK_PRINT_ERROR_BACKTRACE') && PIWIK_PRINT_ERROR_BACKTRACE) - || (defined('PIWIK_TRACKER_DEBUG') && PIWIK_TRACKER_DEBUG); + || !empty($GLOBALS['PIWIK_TRACKER_DEBUG']); return $bool; } diff --git a/tests/PHPUnit/Core/ReleaseCheckListTest.php b/tests/PHPUnit/Core/ReleaseCheckListTest.php index 02e55aac364..fdcd0cce0cf 100644 --- a/tests/PHPUnit/Core/ReleaseCheckListTest.php +++ b/tests/PHPUnit/Core/ReleaseCheckListTest.php @@ -14,6 +14,7 @@ class ReleaseCheckListTest extends PHPUnit_Framework_TestCase public function setUp() { $this->globalConfig = _parse_ini_file(PIWIK_PATH_TEST_TO_ROOT . '/config/global.ini.php', true); + parent::setUp(); } /** @@ -88,7 +89,11 @@ public function testCheckThatConfigurationValuesAreProductionValues() require_once PIWIK_INCLUDE_PATH . "/core/TaskScheduler.php"; $this->assertFalse(DEBUG_FORCE_SCHEDULED_TASKS); - $this->assertFalse(PIWIK_PRINT_ERROR_BACKTRACE); + + // Check the index.php has "backtrace disabled" + $content = file_get_contents(PIWIK_INCLUDE_PATH . "/index.php"); + $expected = "define('PIWIK_PRINT_ERROR_BACKTRACE', false);"; + $this->assertTrue( false !== strpos($content, $expected), 'index.php should contain: ' . $expected); } private function _checkEqual($key, $valueExpected) @@ -147,19 +152,7 @@ public function testProfilingDisabledInProduction() public function testPiwikTrackerDebugIsOff() { $this->assertTrue(!isset($GLOBALS['PIWIK_TRACKER_DEBUG'])); - - $oldGet = $_GET; - $_GET = array('idsite' => 1); - - // hiding echoed out message on empty request - ob_start(); - include PIWIK_PATH_TEST_TO_ROOT . "/piwik.php"; - ob_end_clean(); - - $_GET = $oldGet; - - $this->assertEquals(0, \Piwik\Config::getInstance()->Tracker['debug']); - $this->assertTrue($GLOBALS['PIWIK_TRACKER_DEBUG'] === false); + $this->assertEquals(0, $this->globalConfig['Tracker']['debug']); } /** From c2c5e16ef232049061e1353dad20337395a86a9e Mon Sep 17 00:00:00 2001 From: mattab Date: Thu, 3 Apr 2014 14:23:06 +1300 Subject: [PATCH 09/29] Adding comment to point users to "how to debug" documentation --- piwik.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/piwik.php b/piwik.php index 65fa14cc68c..42bf8cec80f 100644 --- a/piwik.php +++ b/piwik.php @@ -12,6 +12,9 @@ use Piwik\Timer; use Piwik\Tracker; +// Note: if you wish to debug the Tracking API please see this documentation: +// http://developer.piwik.org/api-reference/tracking-api#debugging-the-tracker + $GLOBALS['PIWIK_TRACKER_DEBUG_FORCE_SCHEDULED_TASKS'] = false; define('PIWIK_ENABLE_TRACKING', true); From 26d67856959d73946e54de76116c7169b1a3dd5f Mon Sep 17 00:00:00 2001 From: Thomas Steur Date: Thu, 3 Apr 2014 03:40:24 +0200 Subject: [PATCH 10/29] the tracker should not define any limitations of the custom variables --- core/Tracker.php | 4 --- core/Tracker/GoalManager.php | 4 +-- core/Tracker/Request.php | 5 +-- core/Tracker/Visit.php | 14 ++++---- plugins/CustomVariables/Archiver.php | 3 +- plugins/CustomVariables/CustomVariables.php | 2 +- plugins/CustomVariables/Model.php | 40 +++++++++++++++++---- plugins/CustomVariables/tests/ModelTest.php | 11 ++++++ plugins/Live/Visitor.php | 9 ++--- 9 files changed, 65 insertions(+), 27 deletions(-) diff --git a/core/Tracker.php b/core/Tracker.php index 05deb9a2aa9..17265e76ed4 100644 --- a/core/Tracker.php +++ b/core/Tracker.php @@ -43,10 +43,6 @@ class Tracker const LENGTH_HEX_ID_STRING = 16; const LENGTH_BINARY_ID = 8; - // These are also hardcoded in the Javascript - const MAX_CUSTOM_VARIABLES = 5; - const MAX_LENGTH_CUSTOM_VARIABLE = 200; - static protected $forcedDateTime = null; static protected $forcedIpString = null; static protected $forcedVisitorId = null; diff --git a/core/Tracker/GoalManager.php b/core/Tracker/GoalManager.php index 939565e7fe2..7771172e823 100644 --- a/core/Tracker/GoalManager.php +++ b/core/Tracker/GoalManager.php @@ -11,9 +11,9 @@ use Exception; use Piwik\Common; use Piwik\Config; -use Piwik\Log; use Piwik\Piwik; use Piwik\Tracker; +use Piwik\Plugins\CustomVariables\Model as CustomVariablesModel; /** */ @@ -227,7 +227,7 @@ public function recordGoals($idSite, $visitorInformation, $visitCustomVariables, } // Copy Custom Variables from Visit row to the Goal conversion - for ($i = 1; $i <= Tracker::MAX_CUSTOM_VARIABLES; $i++) { + for ($i = 1; $i <= CustomVariablesModel::getMaxCustomVariables(); $i++) { if (isset($visitorInformation['custom_var_k' . $i]) && strlen($visitorInformation['custom_var_k' . $i]) ) { diff --git a/core/Tracker/Request.php b/core/Tracker/Request.php index 3dbe3f98d89..80eb0627337 100644 --- a/core/Tracker/Request.php +++ b/core/Tracker/Request.php @@ -16,6 +16,7 @@ use Piwik\Piwik; use Piwik\Registry; use Piwik\Tracker; +use Piwik\Plugins\CustomVariables\Model as CustomVariablesModel; /** * The Request object holding the http parameters for this tracking request. Use getParam() to fetch a named parameter. @@ -355,7 +356,7 @@ public function getCustomVariables($scope) foreach ($customVar as $id => $keyValue) { $id = (int)$id; if ($id < 1 - || $id > Tracker::MAX_CUSTOM_VARIABLES + || $id > CustomVariablesModel::getMaxCustomVariables() || count($keyValue) != 2 || (!is_string($keyValue[0]) && !is_numeric($keyValue[0])) ) { @@ -379,7 +380,7 @@ public function getCustomVariables($scope) public static function truncateCustomVariable($input) { - return substr(trim($input), 0, Tracker::MAX_LENGTH_CUSTOM_VARIABLE); + return substr(trim($input), 0, CustomVariablesModel::getMaxLengthCustomVariables()); } protected function shouldUseThirdPartyCookie() diff --git a/core/Tracker/Visit.php b/core/Tracker/Visit.php index 470ec3eb4cd..639db2f9394 100644 --- a/core/Tracker/Visit.php +++ b/core/Tracker/Visit.php @@ -15,6 +15,7 @@ use Piwik\Piwik; use Piwik\Tracker; use UserAgentParser; +use Piwik\Plugins\CustomVariables\Model as CustomVariablesModel; /** * Class used to handle a Visit. @@ -396,12 +397,11 @@ protected function recognizeTheVisitor() $selectCustomVariables = ''; // No custom var were found in the request, so let's copy the previous one in a potential conversion later if (!$this->visitorCustomVariables) { - $selectCustomVariables = ', - custom_var_k1, custom_var_v1, - custom_var_k2, custom_var_v2, - custom_var_k3, custom_var_v3, - custom_var_k4, custom_var_v4, - custom_var_k5, custom_var_v5'; + $maxCustomVariables = CustomVariablesModel::getMaxCustomVariables(); + + for ($index = 1; $index <= $maxCustomVariables; $index++) { + $selectCustomVariables .= ', custom_var_k' . $index . ', custom_var_v' . $index; + } } $persistedVisitAttributes = $this->getVisitFieldsPersist(); @@ -513,7 +513,7 @@ protected function recognizeTheVisitor() // Custom Variables copied from Visit in potential later conversion if (!empty($selectCustomVariables)) { - for ($i = 1; $i <= Tracker::MAX_CUSTOM_VARIABLES; $i++) { + for ($i = 1; $i <= CustomVariablesModel::getMaxCustomVariables(); $i++) { if (isset($visitRow['custom_var_k' . $i]) && strlen($visitRow['custom_var_k' . $i]) ) { diff --git a/plugins/CustomVariables/Archiver.php b/plugins/CustomVariables/Archiver.php index bc9d9b3241d..d1f31f9c597 100644 --- a/plugins/CustomVariables/Archiver.php +++ b/plugins/CustomVariables/Archiver.php @@ -15,6 +15,7 @@ use Piwik\Metrics; use Piwik\Tracker; use Piwik\Tracker\GoalManager; +use Piwik\Plugins\CustomVariables\Model as CustomVariablesModel; require_once PIWIK_INCLUDE_PATH . '/libs/PiwikTracker/PiwikTracker.php'; @@ -59,7 +60,7 @@ public function aggregateDayReport() { $this->dataArray = new DataArray(); - for ($i = 1; $i <= Tracker::MAX_CUSTOM_VARIABLES; $i++) { + for ($i = 1; $i <= CustomVariablesModel::getMaxCustomVariables(); $i++) { $this->aggregateCustomVariable($i); } diff --git a/plugins/CustomVariables/CustomVariables.php b/plugins/CustomVariables/CustomVariables.php index c7e47f08e12..396d2ef3827 100644 --- a/plugins/CustomVariables/CustomVariables.php +++ b/plugins/CustomVariables/CustomVariables.php @@ -92,7 +92,7 @@ public function getReportMetadata(&$reports) public function getSegmentsMetadata(&$segments) { - for ($i = 1; $i <= Tracker::MAX_CUSTOM_VARIABLES; $i++) { + for ($i = 1; $i <= Model::getMaxCustomVariables(); $i++) { $segments[] = array( 'type' => 'dimension', 'category' => 'CustomVariables_CustomVariables', diff --git a/plugins/CustomVariables/Model.php b/plugins/CustomVariables/Model.php index 992be895e5d..a3cbf90d8ca 100644 --- a/plugins/CustomVariables/Model.php +++ b/plugins/CustomVariables/Model.php @@ -11,6 +11,7 @@ use Piwik\Common; use Piwik\DataTable; use Piwik\Db; +use Piwik\Log; class Model { @@ -60,8 +61,7 @@ public function getHighestCustomVarIndex() } $indexes = array_map(function ($column) { - $onlyNumber = str_replace(array('custom_var_k', 'custom_var_v'), '', $column); - return (int) $onlyNumber; + return Model::getCustomVariableIndexFromFieldName($column); }, $columns); return max($indexes); @@ -99,23 +99,51 @@ public function addCustomVariable() $dbTable = Common::prefixTable($this->scope); $index = $this->getHighestCustomVarIndex() + 1; - Db::exec(sprintf('ALTER TABLE %s ADD COLUMN custom_var_k%d VARCHAR(200) DEFAULT NULL', $dbTable, $index)); - Db::exec(sprintf('ALTER TABLE %s ADD COLUMN custom_var_v%d VARCHAR(200) DEFAULT NULL', $dbTable, $index)); + Db::exec(sprintf('ALTER TABLE %s ADD COLUMN custom_var_k%d VARCHAR(%d) DEFAULT NULL', $dbTable, $index, self::getMaxLengthCustomVariables())); + Db::exec(sprintf('ALTER TABLE %s ADD COLUMN custom_var_v%d VARCHAR(%d) DEFAULT NULL', $dbTable, $index, self::getMaxLengthCustomVariables())); return $index; } + public static function getCustomVariableIndexFromFieldName($fieldName) + { + $onlyNumber = str_replace(array('custom_var_k', 'custom_var_v'), '', $fieldName); + + if (is_numeric($onlyNumber)) { + return (int) $onlyNumber; + } + } + public static function getScopes() { return array(self::SCOPE_PAGE, self::SCOPE_VISIT, self::SCOPE_CONVERSION); } + public static function getMaxCustomVariables() + { + return 5; + } + + /** + * There are also some hardcoded places in JavaScript + * @return int + */ + public static function getMaxLengthCustomVariables() + { + return 200; + } + public static function install() { foreach (self::getScopes() as $scope) { $model = new Model($scope); - for ($index = 0; $index < 5; $index++) { - $model->addCustomVariable(); + + try { + for ($index = 0; $index < 5; $index++) { + $model->addCustomVariable(); + } + } catch (\Exception $e) { + Log::warning('Failed to add custom variable: ' . $e->getMessage()); } } } diff --git a/plugins/CustomVariables/tests/ModelTest.php b/plugins/CustomVariables/tests/ModelTest.php index b141cb4f5df..744dae2ce79 100644 --- a/plugins/CustomVariables/tests/ModelTest.php +++ b/plugins/CustomVariables/tests/ModelTest.php @@ -22,6 +22,17 @@ public function testGetAllScopes() $this->assertEquals(array('log_link_visit_action', 'log_visit', 'log_conversion'), Model::getScopes()); } + public function testGetCustomVariableIndexFromFieldName() + { + $this->assertSame(0, Model::getCustomVariableIndexFromFieldName('custom_var_k0')); + $this->assertSame(0, Model::getCustomVariableIndexFromFieldName('custom_var_v0')); + $this->assertSame(5, Model::getCustomVariableIndexFromFieldName('custom_var_k5')); + $this->assertSame(5, Model::getCustomVariableIndexFromFieldName('custom_var_v5')); + $this->assertSame(938, Model::getCustomVariableIndexFromFieldName('custom_var_k938')); + $this->assertSame(938, Model::getCustomVariableIndexFromFieldName('custom_var_v938')); + $this->assertSame(null, Model::getCustomVariableIndexFromFieldName('otherfield')); + } + public function testGetScopeName() { $this->assertEquals('Page', $this->getPageScope()->getScopeName()); diff --git a/plugins/Live/Visitor.php b/plugins/Live/Visitor.php index 75b54e383b9..0cc96a60f1e 100644 --- a/plugins/Live/Visitor.php +++ b/plugins/Live/Visitor.php @@ -23,6 +23,7 @@ use Piwik\Tracker; use Piwik\Tracker\Visit; use Piwik\UrlHelper; +use Piwik\Plugins\CustomVariables\Model as CustomVariablesModel; /** * @see plugins/Referrers/functions.php @@ -338,10 +339,10 @@ function getLongitude() function getCustomVariables() { $customVariables = array(); - for ($i = 1; $i <= Tracker::MAX_CUSTOM_VARIABLES; $i++) { + for ($i = 1; $i <= CustomVariablesModel::getMaxCustomVariables(); $i++) { if (!empty($this->details['custom_var_k' . $i])) { $customVariables[$i] = array( - 'customVariableName' . $i => $this->details['custom_var_k' . $i], + 'customVariableName' . $i => $this->details['custom_var_k' . $i], 'customVariableValue' . $i => $this->details['custom_var_v' . $i], ); } @@ -724,7 +725,7 @@ public static function enrichVisitorArrayWithActions($visitorDetailsArray, $acti $idVisit = $visitorDetailsArray['idVisit']; $sqlCustomVariables = ''; - for ($i = 1; $i <= Tracker::MAX_CUSTOM_VARIABLES; $i++) { + for ($i = 1; $i <= CustomVariablesModel::getMaxCustomVariables(); $i++) { $sqlCustomVariables .= ', custom_var_k' . $i . ', custom_var_v' . $i; } // The second join is a LEFT join to allow returning records that don't have a matching page title @@ -761,7 +762,7 @@ public static function enrichVisitorArrayWithActions($visitorDetailsArray, $acti foreach ($actionDetails as $actionIdx => &$actionDetail) { $actionDetail =& $actionDetails[$actionIdx]; $customVariablesPage = array(); - for ($i = 1; $i <= Tracker::MAX_CUSTOM_VARIABLES; $i++) { + for ($i = 1; $i <= CustomVariablesModel::getMaxCustomVariables(); $i++) { if (!empty($actionDetail['custom_var_k' . $i])) { $cvarKey = $actionDetail['custom_var_k' . $i]; $cvarKey = static::getCustomVariablePrettyKey($cvarKey); From c02b18e810eb1e2e1fc0e73204df1fd85a4857c5 Mon Sep 17 00:00:00 2001 From: mattab Date: Thu, 3 Apr 2014 14:53:41 +1300 Subject: [PATCH 11/29] Fix tests --- tests/PHPUnit/Integration/Core/PiwikTest.php | 1 - ..._apiGetReportMetadata_year__SitesManager.getJavascriptTag.xml | 1 - 2 files changed, 2 deletions(-) diff --git a/tests/PHPUnit/Integration/Core/PiwikTest.php b/tests/PHPUnit/Integration/Core/PiwikTest.php index 7dc414972a5..28e303f0f37 100644 --- a/tests/PHPUnit/Integration/Core/PiwikTest.php +++ b/tests/PHPUnit/Integration/Core/PiwikTest.php @@ -59,7 +59,6 @@ public function testJavascriptTrackingCode_withAllOptions() var d=document, g=d.createElement('script'), s=d.getElementsByTagName('script')[0]; g.type='text/javascript'; g.defer=true; g.async=true; g.src=u+'piwik.js'; s.parentNode.insertBefore(g,s); })(); - </script> <noscript><p><img src="http://localhost/piwik/piwik.php?idsite=1" style="border:0;" alt="" /></p></noscript> <!-- End Piwik Code --> diff --git a/tests/PHPUnit/Integration/expected/test_apiGetReportMetadata_year__SitesManager.getJavascriptTag.xml b/tests/PHPUnit/Integration/expected/test_apiGetReportMetadata_year__SitesManager.getJavascriptTag.xml index a9f39502fbc..3da4a2d812c 100644 --- a/tests/PHPUnit/Integration/expected/test_apiGetReportMetadata_year__SitesManager.getJavascriptTag.xml +++ b/tests/PHPUnit/Integration/expected/test_apiGetReportMetadata_year__SitesManager.getJavascriptTag.xml @@ -11,7 +11,6 @@ var d=document, g=d.createElement('script'), s=d.getElementsByTagName('script')[0]; g.type='text/javascript'; g.defer=true; g.async=true; g.src=u+'piwik.js'; s.parentNode.insertBefore(g,s); })(); - </script> <noscript><p><img src="http://example.org/piwik/piwik.php?idsite=1" style="border:0;" alt="" /></p></noscript> <!-- End Piwik Code --> From 625a29757d5d94194fb152508752123db8feb0de Mon Sep 17 00:00:00 2001 From: Thomas Steur Date: Thu, 3 Apr 2014 04:03:54 +0200 Subject: [PATCH 12/29] detect the number of max custom variables --- plugins/CustomVariables/Model.php | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/plugins/CustomVariables/Model.php b/plugins/CustomVariables/Model.php index a3cbf90d8ca..d33fe3353cd 100644 --- a/plugins/CustomVariables/Model.php +++ b/plugins/CustomVariables/Model.php @@ -12,6 +12,7 @@ use Piwik\DataTable; use Piwik\Db; use Piwik\Log; +use Piwik\Tracker\Cache; class Model { @@ -91,6 +92,8 @@ public function removeCustomVariable() Db::exec(sprintf('ALTER TABLE %s DROP COLUMN custom_var_k%d', $dbTable, $index)); Db::exec(sprintf('ALTER TABLE %s DROP COLUMN custom_var_v%d', $dbTable, $index)); + Cache::clearCacheGeneral(); + return $index; } @@ -102,6 +105,8 @@ public function addCustomVariable() Db::exec(sprintf('ALTER TABLE %s ADD COLUMN custom_var_k%d VARCHAR(%d) DEFAULT NULL', $dbTable, $index, self::getMaxLengthCustomVariables())); Db::exec(sprintf('ALTER TABLE %s ADD COLUMN custom_var_v%d VARCHAR(%d) DEFAULT NULL', $dbTable, $index, self::getMaxLengthCustomVariables())); + Cache::clearCacheGeneral(); + return $index; } @@ -121,7 +126,27 @@ public static function getScopes() public static function getMaxCustomVariables() { - return 5; + $cache = Cache::getCacheGeneral(); + $cacheKey = 'CustomVariables.MaxNumCustomVariables'; + + if (!array_key_exists($cacheKey, $cache)) { + + $maxCustomVar = 0; + + foreach (self::getScopes() as $scope) { + $model = new Model($scope); + $highestIndex = $model->getHighestCustomVarIndex(); + + if ($highestIndex > $maxCustomVar) { + $maxCustomVar = $highestIndex; + } + } + + $cache[$cacheKey] = $maxCustomVar; + Cache::setCacheGeneral($cache); + } + + return $cache[$cacheKey]; } /** From ffec1b81dd7694ec1129c1e37e7b27ebeb385230 Mon Sep 17 00:00:00 2001 From: mattab Date: Thu, 3 Apr 2014 15:19:41 +1300 Subject: [PATCH 13/29] Removed deprecated methods, renamed calls to the new methods. Refs #4942 --- core/Access.php | 35 ------------ core/Piwik.php | 60 -------------------- tests/PHPUnit/Core/DeprecatedMethodsTest.php | 3 +- tests/PHPUnit/FakeAccess.php | 25 -------- 4 files changed, 2 insertions(+), 121 deletions(-) diff --git a/core/Access.php b/core/Access.php index a7b4d7178e3..55335385d04 100644 --- a/core/Access.php +++ b/core/Access.php @@ -245,15 +245,6 @@ public function setSuperUserAccess($bool = true) } } - /** - * @see Access::setSuperUserAccess() - * @deprecated deprecated since version 2.0.4 - */ - public function setSuperUser($bool = true) - { - self::setSuperUserAccess($bool); - } - /** * Returns true if the current user is logged in as the Super User * @@ -264,15 +255,6 @@ public function hasSuperUserAccess() return $this->hasSuperUserAccess; } - /** - * @see Access::hasSuperUserAccess() - * @deprecated deprecated since version 2.0.4 - */ - public function isSuperUser() - { - return $this->hasSuperUserAccess(); - } - /** * Returns the current user login * @@ -314,14 +296,6 @@ protected function getAnySuperUserAccessLogin() return $firstSuperUser['login']; } - /** - * @deprecated deprecated since version 2.0.4 - */ - public function getSuperUserLogin() - { - return $this->getAnySuperUserAccessLogin(); - } - /** * Returns an array of ID sites for which the user has at least a VIEW access. * Which means VIEW or ADMIN or SUPERUSER. @@ -377,15 +351,6 @@ public function checkUserHasSuperUserAccess() } } - /** - * @see Access::checkUserHasSuperUserAccess() - * @deprecated deprecated since version 2.0.4 - */ - public function checkUserIsSuperUser() - { - self::checkUserHasSuperUserAccess(); - } - /** * If the user doesn't have an ADMIN access for at least one website, throws an exception * diff --git a/core/Piwik.php b/core/Piwik.php index a0cff3cdc2d..6c470707346 100644 --- a/core/Piwik.php +++ b/core/Piwik.php @@ -257,22 +257,6 @@ static public function getCurrentUserEmail() return $user['email']; } - /** - * @deprecated deprecated since version 2.0.4 - */ - static public function getSuperUserLogin() - { - return Access::getInstance()->getSuperUserLogin(); - } - - /** - * @deprecated deprecated since version 2.0.4 - */ - static public function getSuperUserEmail() - { - return ''; - } - /** * Get a list of all email addresses having Super User access. * @@ -335,24 +319,6 @@ static public function hasUserSuperUserAccessOrIsTheUser($theUser) } } - /** - * @see Piwik::hasUserSuperUserAccessOrIsTheUser() - * @deprecated deprecated since version 2.0.4 - */ - static public function isUserIsSuperUserOrTheUser($theUser) - { - return self::hasUserSuperUserAccessOrIsTheUser($theUser); - } - - /** - * @see Piwik::checkUserHasSuperUserAccessOrIsTheUser() - * @deprecated deprecated since version 2.0.4 - */ - static public function checkUserIsSuperUserOrTheUser($theUser) - { - self::checkUserHasSuperUserAccessOrIsTheUser($theUser); - } - /** * Check that the current user is either the specified user or the superuser. * @@ -404,14 +370,6 @@ static public function hasTheUserSuperUserAccess($theUser) return false; } - /** - * @see Piwik::hasUserSuperUserAccess() - * @deprecated deprecated since version 2.0.4 - */ - static public function isUserIsSuperUser() - { - return self::hasUserSuperUserAccess(); - } /** * Returns true if the current user has Super User access. @@ -464,24 +422,6 @@ static public function setUserHasSuperUserAccess($bool = true) Access::getInstance()->setSuperUserAccess($bool); } - /** - * @see Piwik::setUserHasSuperUserAccess() - * @deprecated deprecated since version 2.0.4 - */ - static public function setUserIsSuperUser($bool = true) - { - self::setUserHasSuperUserAccess($bool); - } - - /** - * @see Piwik::checkUserHasSuperUserAccess() - * @deprecated deprecated since version 2.0.4 - */ - static public function checkUserIsSuperUser() - { - self::checkUserHasSuperUserAccess(); - } - /** * Check that the current user has superuser access. * diff --git a/tests/PHPUnit/Core/DeprecatedMethodsTest.php b/tests/PHPUnit/Core/DeprecatedMethodsTest.php index bdcf46db8d9..786b968d0e9 100644 --- a/tests/PHPUnit/Core/DeprecatedMethodsTest.php +++ b/tests/PHPUnit/Core/DeprecatedMethodsTest.php @@ -22,7 +22,6 @@ class DeprecatedMethodsTest extends PHPUnit_Framework_TestCase public function test_version2_0_4() { $validTill = '2014-04-01'; - $this->assertDeprecatedMethodIsRemoved('\Piwik\Piwik', 'isUserIsSuperUserOrTheUser', $validTill); $this->assertDeprecatedMethodIsRemoved('\Piwik\Piwik', 'checkUserIsSuperUserOrTheUser', $validTill); $this->assertDeprecatedMethodIsRemoved('\Piwik\Piwik', 'isUserIsSuperUser', $validTill); @@ -37,6 +36,8 @@ public function test_version2_0_4() $this->assertDeprecatedMethodIsRemoved('\FakeAccess', 'checkUserIsSuperUser', $validTill); $this->assertDeprecatedMethodIsRemoved('\FakeAccess', 'setSuperUser', $validTill); $this->assertDeprecatedMethodIsRemoved('\FakeAccess', 'getSuperUserLogin', $validTill); + + $validTill = '2014-10-01'; $this->assertDeprecatedMethodIsRemoved('\Piwik\Config', 'getConfigSuperUserForBackwardCompatibility', $validTill); } diff --git a/tests/PHPUnit/FakeAccess.php b/tests/PHPUnit/FakeAccess.php index 0b550d17447..9711dbffbab 100644 --- a/tests/PHPUnit/FakeAccess.php +++ b/tests/PHPUnit/FakeAccess.php @@ -56,29 +56,11 @@ public static function checkUserHasSuperUserAccess() } } - /** - * @see FakeAccess::checkUserHasSuperUserAccess() - * @deprecated deprecated since version 2.0.4 - */ - public function checkUserIsSuperUser() - { - self::checkUserHasSuperUserAccess(); - } - public static function setSuperUserAccess($bool = true) { self::$superUser = $bool; } - /** - * @see FakeAccess::setSuperUserAccess() - * @deprecated deprecated since version 2.0.4 - */ - public static function setSuperUser($bool = true) - { - self::setSuperUserAccess($bool); - } - public static function reloadAccess() { } @@ -188,11 +170,4 @@ public function getRawSitesWithSomeViewAccess($login) return $result; } - /** - * @deprecated deprecated since version 2.0.4 - */ - public function getSuperUserLogin() - { - return self::$superUserLogin; - } } From 26043d38220da7cb0bebe1aed40ce472647f30d4 Mon Sep 17 00:00:00 2001 From: Thomas Steur Date: Thu, 3 Apr 2014 04:24:11 +0200 Subject: [PATCH 14/29] moving getMaxCustomVariables to CustomVariables class, a model should not really care about caching etc --- core/Tracker/GoalManager.php | 4 +- core/Tracker/Request.php | 4 +- core/Tracker/Visit.php | 6 +-- plugins/CustomVariables/Archiver.php | 3 +- .../Commands/SetNumberOfCustomVariables.php | 2 + plugins/CustomVariables/CustomVariables.php | 37 +++++++++++++++- plugins/CustomVariables/Model.php | 44 ++----------------- .../tests/CustomVariablesTest.php | 43 ++++++++++++++++++ plugins/Live/Visitor.php | 8 ++-- 9 files changed, 96 insertions(+), 55 deletions(-) create mode 100644 plugins/CustomVariables/tests/CustomVariablesTest.php diff --git a/core/Tracker/GoalManager.php b/core/Tracker/GoalManager.php index 7771172e823..c4e7fb67db7 100644 --- a/core/Tracker/GoalManager.php +++ b/core/Tracker/GoalManager.php @@ -12,8 +12,8 @@ use Piwik\Common; use Piwik\Config; use Piwik\Piwik; +use Piwik\Plugins\CustomVariables\CustomVariables; use Piwik\Tracker; -use Piwik\Plugins\CustomVariables\Model as CustomVariablesModel; /** */ @@ -227,7 +227,7 @@ public function recordGoals($idSite, $visitorInformation, $visitCustomVariables, } // Copy Custom Variables from Visit row to the Goal conversion - for ($i = 1; $i <= CustomVariablesModel::getMaxCustomVariables(); $i++) { + for ($i = 1; $i <= CustomVariables::getMaxCustomVariables(); $i++) { if (isset($visitorInformation['custom_var_k' . $i]) && strlen($visitorInformation['custom_var_k' . $i]) ) { diff --git a/core/Tracker/Request.php b/core/Tracker/Request.php index 80eb0627337..ab79d5f4706 100644 --- a/core/Tracker/Request.php +++ b/core/Tracker/Request.php @@ -14,9 +14,9 @@ use Piwik\Cookie; use Piwik\IP; use Piwik\Piwik; +use Piwik\Plugins\CustomVariables\CustomVariables; use Piwik\Registry; use Piwik\Tracker; -use Piwik\Plugins\CustomVariables\Model as CustomVariablesModel; /** * The Request object holding the http parameters for this tracking request. Use getParam() to fetch a named parameter. @@ -380,7 +380,7 @@ public function getCustomVariables($scope) public static function truncateCustomVariable($input) { - return substr(trim($input), 0, CustomVariablesModel::getMaxLengthCustomVariables()); + return substr(trim($input), 0, CustomVariables::getMaxLengthCustomVariables()); } protected function shouldUseThirdPartyCookie() diff --git a/core/Tracker/Visit.php b/core/Tracker/Visit.php index 639db2f9394..5926e8f1e2c 100644 --- a/core/Tracker/Visit.php +++ b/core/Tracker/Visit.php @@ -13,9 +13,9 @@ use Piwik\Config; use Piwik\IP; use Piwik\Piwik; +use Piwik\Plugins\CustomVariables\CustomVariables; use Piwik\Tracker; use UserAgentParser; -use Piwik\Plugins\CustomVariables\Model as CustomVariablesModel; /** * Class used to handle a Visit. @@ -397,7 +397,7 @@ protected function recognizeTheVisitor() $selectCustomVariables = ''; // No custom var were found in the request, so let's copy the previous one in a potential conversion later if (!$this->visitorCustomVariables) { - $maxCustomVariables = CustomVariablesModel::getMaxCustomVariables(); + $maxCustomVariables = CustomVariables::getMaxCustomVariables(); for ($index = 1; $index <= $maxCustomVariables; $index++) { $selectCustomVariables .= ', custom_var_k' . $index . ', custom_var_v' . $index; @@ -513,7 +513,7 @@ protected function recognizeTheVisitor() // Custom Variables copied from Visit in potential later conversion if (!empty($selectCustomVariables)) { - for ($i = 1; $i <= CustomVariablesModel::getMaxCustomVariables(); $i++) { + for ($i = 1; $i <= CustomVariables::getMaxCustomVariables(); $i++) { if (isset($visitRow['custom_var_k' . $i]) && strlen($visitRow['custom_var_k' . $i]) ) { diff --git a/plugins/CustomVariables/Archiver.php b/plugins/CustomVariables/Archiver.php index d1f31f9c597..f2bdd4d6da4 100644 --- a/plugins/CustomVariables/Archiver.php +++ b/plugins/CustomVariables/Archiver.php @@ -15,7 +15,6 @@ use Piwik\Metrics; use Piwik\Tracker; use Piwik\Tracker\GoalManager; -use Piwik\Plugins\CustomVariables\Model as CustomVariablesModel; require_once PIWIK_INCLUDE_PATH . '/libs/PiwikTracker/PiwikTracker.php'; @@ -60,7 +59,7 @@ public function aggregateDayReport() { $this->dataArray = new DataArray(); - for ($i = 1; $i <= CustomVariablesModel::getMaxCustomVariables(); $i++) { + for ($i = 1; $i <= CustomVariables::getMaxCustomVariables(); $i++) { $this->aggregateCustomVariable($i); } diff --git a/plugins/CustomVariables/Commands/SetNumberOfCustomVariables.php b/plugins/CustomVariables/Commands/SetNumberOfCustomVariables.php index 57fa3f174d1..acd7233a1e3 100644 --- a/plugins/CustomVariables/Commands/SetNumberOfCustomVariables.php +++ b/plugins/CustomVariables/Commands/SetNumberOfCustomVariables.php @@ -10,6 +10,7 @@ namespace Piwik\Plugins\CustomVariables\Commands; use Piwik\Plugin\ConsoleCommand; +use Piwik\Tracker\Cache; use Piwik\Plugins\CustomVariables\Model; use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; @@ -65,6 +66,7 @@ protected function execute(InputInterface $input, OutputInterface $output) $this->performChange($scope, $numVarsToSet, $output); } + Cache::clearCacheGeneral(); $this->progress->finish(); $this->writeSuccessMessage($output, array( diff --git a/plugins/CustomVariables/CustomVariables.php b/plugins/CustomVariables/CustomVariables.php index 396d2ef3827..6a7ae6618e5 100644 --- a/plugins/CustomVariables/CustomVariables.php +++ b/plugins/CustomVariables/CustomVariables.php @@ -14,6 +14,7 @@ use Piwik\Plugin\ViewDataTable; use Piwik\Tracker; use Piwik\WidgetsList; +use Piwik\Tracker\Cache; /** */ @@ -58,6 +59,40 @@ public function install() Model::install(); } + /** + * There are also some hardcoded places in JavaScript + * @return int + */ + public static function getMaxLengthCustomVariables() + { + return 200; + } + + public static function getMaxCustomVariables() + { + $cache = Cache::getCacheGeneral(); + $cacheKey = 'CustomVariables.MaxNumCustomVariables'; + + if (!array_key_exists($cacheKey, $cache)) { + + $maxCustomVar = 0; + + foreach (Model::getScopes() as $scope) { + $model = new Model($scope); + $highestIndex = $model->getHighestCustomVarIndex(); + + if ($highestIndex > $maxCustomVar) { + $maxCustomVar = $highestIndex; + } + } + + $cache[$cacheKey] = $maxCustomVar; + Cache::setCacheGeneral($cache); + } + + return $cache[$cacheKey]; + } + public function addConsoleCommands(&$commands) { $commands[] = __NAMESPACE__ . '\\Commands\\SetNumberOfCustomVariables'; @@ -92,7 +127,7 @@ public function getReportMetadata(&$reports) public function getSegmentsMetadata(&$segments) { - for ($i = 1; $i <= Model::getMaxCustomVariables(); $i++) { + for ($i = 1; $i <= self::getMaxCustomVariables(); $i++) { $segments[] = array( 'type' => 'dimension', 'category' => 'CustomVariables_CustomVariables', diff --git a/plugins/CustomVariables/Model.php b/plugins/CustomVariables/Model.php index d33fe3353cd..252718f87a2 100644 --- a/plugins/CustomVariables/Model.php +++ b/plugins/CustomVariables/Model.php @@ -12,7 +12,6 @@ use Piwik\DataTable; use Piwik\Db; use Piwik\Log; -use Piwik\Tracker\Cache; class Model { @@ -92,8 +91,6 @@ public function removeCustomVariable() Db::exec(sprintf('ALTER TABLE %s DROP COLUMN custom_var_k%d', $dbTable, $index)); Db::exec(sprintf('ALTER TABLE %s DROP COLUMN custom_var_v%d', $dbTable, $index)); - Cache::clearCacheGeneral(); - return $index; } @@ -101,11 +98,10 @@ public function addCustomVariable() { $dbTable = Common::prefixTable($this->scope); $index = $this->getHighestCustomVarIndex() + 1; + $maxLen = CustomVariables::getMaxLengthCustomVariables(); - Db::exec(sprintf('ALTER TABLE %s ADD COLUMN custom_var_k%d VARCHAR(%d) DEFAULT NULL', $dbTable, $index, self::getMaxLengthCustomVariables())); - Db::exec(sprintf('ALTER TABLE %s ADD COLUMN custom_var_v%d VARCHAR(%d) DEFAULT NULL', $dbTable, $index, self::getMaxLengthCustomVariables())); - - Cache::clearCacheGeneral(); + Db::exec(sprintf('ALTER TABLE %s ADD COLUMN custom_var_k%d VARCHAR(%d) DEFAULT NULL', $dbTable, $index, $maxLen)); + Db::exec(sprintf('ALTER TABLE %s ADD COLUMN custom_var_v%d VARCHAR(%d) DEFAULT NULL', $dbTable, $index, $maxLen)); return $index; } @@ -124,40 +120,6 @@ public static function getScopes() return array(self::SCOPE_PAGE, self::SCOPE_VISIT, self::SCOPE_CONVERSION); } - public static function getMaxCustomVariables() - { - $cache = Cache::getCacheGeneral(); - $cacheKey = 'CustomVariables.MaxNumCustomVariables'; - - if (!array_key_exists($cacheKey, $cache)) { - - $maxCustomVar = 0; - - foreach (self::getScopes() as $scope) { - $model = new Model($scope); - $highestIndex = $model->getHighestCustomVarIndex(); - - if ($highestIndex > $maxCustomVar) { - $maxCustomVar = $highestIndex; - } - } - - $cache[$cacheKey] = $maxCustomVar; - Cache::setCacheGeneral($cache); - } - - return $cache[$cacheKey]; - } - - /** - * There are also some hardcoded places in JavaScript - * @return int - */ - public static function getMaxLengthCustomVariables() - { - return 200; - } - public static function install() { foreach (self::getScopes() as $scope) { diff --git a/plugins/CustomVariables/tests/CustomVariablesTest.php b/plugins/CustomVariables/tests/CustomVariablesTest.php new file mode 100644 index 00000000000..980401d6080 --- /dev/null +++ b/plugins/CustomVariables/tests/CustomVariablesTest.php @@ -0,0 +1,43 @@ +assertSame(5, CustomVariables::getMaxCustomVariables()); + } + + public function testGetMaxCustomVariables_ShouldCacheTheResult() + { + CustomVariables::getMaxCustomVariables(); + $cache = Cache::getCacheGeneral(); + + $this->assertSame(5, $cache['CustomVariables.MaxNumCustomVariables']); + } + + public function testGetMaxCustomVariables_ShouldReadFromCacheIfPossible() + { + $cache = Cache::getCacheGeneral(); + $cache['CustomVariables.MaxNumCustomVariables'] = 10; + Cache::setCacheGeneral($cache); + + $this->assertSame(10, CustomVariables::getMaxCustomVariables()); + } + +} diff --git a/plugins/Live/Visitor.php b/plugins/Live/Visitor.php index 0cc96a60f1e..37a8c20f771 100644 --- a/plugins/Live/Visitor.php +++ b/plugins/Live/Visitor.php @@ -16,6 +16,7 @@ use Piwik\IP; use Piwik\Piwik; use Piwik\Plugins\API\API as APIMetadata; +use Piwik\Plugins\CustomVariables\CustomVariables; use Piwik\Plugins\Referrers\API as APIReferrers; use Piwik\Plugins\UserCountry\LocationProvider\GeoIp; use Piwik\Tracker\Action; @@ -23,7 +24,6 @@ use Piwik\Tracker; use Piwik\Tracker\Visit; use Piwik\UrlHelper; -use Piwik\Plugins\CustomVariables\Model as CustomVariablesModel; /** * @see plugins/Referrers/functions.php @@ -339,7 +339,7 @@ function getLongitude() function getCustomVariables() { $customVariables = array(); - for ($i = 1; $i <= CustomVariablesModel::getMaxCustomVariables(); $i++) { + for ($i = 1; $i <= CustomVariables::getMaxCustomVariables(); $i++) { if (!empty($this->details['custom_var_k' . $i])) { $customVariables[$i] = array( 'customVariableName' . $i => $this->details['custom_var_k' . $i], @@ -725,7 +725,7 @@ public static function enrichVisitorArrayWithActions($visitorDetailsArray, $acti $idVisit = $visitorDetailsArray['idVisit']; $sqlCustomVariables = ''; - for ($i = 1; $i <= CustomVariablesModel::getMaxCustomVariables(); $i++) { + for ($i = 1; $i <= CustomVariables::getMaxCustomVariables(); $i++) { $sqlCustomVariables .= ', custom_var_k' . $i . ', custom_var_v' . $i; } // The second join is a LEFT join to allow returning records that don't have a matching page title @@ -762,7 +762,7 @@ public static function enrichVisitorArrayWithActions($visitorDetailsArray, $acti foreach ($actionDetails as $actionIdx => &$actionDetail) { $actionDetail =& $actionDetails[$actionIdx]; $customVariablesPage = array(); - for ($i = 1; $i <= CustomVariablesModel::getMaxCustomVariables(); $i++) { + for ($i = 1; $i <= CustomVariables::getMaxCustomVariables(); $i++) { if (!empty($actionDetail['custom_var_k' . $i])) { $cvarKey = $actionDetail['custom_var_k' . $i]; $cvarKey = static::getCustomVariablePrettyKey($cvarKey); From 4cc825950ac7f3aa965de47b2196c23d99989548 Mon Sep 17 00:00:00 2001 From: mattab Date: Thu, 3 Apr 2014 15:35:51 +1300 Subject: [PATCH 15/29] Add new config flag for developers: enable_load_standalone_plugins_during_tests = 1 it can be set to 0 to disable the loading of third party plugins when running tests --- config/global.ini.php | 4 ++++ core/Plugin/Manager.php | 11 ++++++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/config/global.ini.php b/config/global.ini.php index 78204437b48..f98d57588c8 100644 --- a/config/global.ini.php +++ b/config/global.ini.php @@ -86,6 +86,10 @@ ; Allow automatic upgrades to Beta or RC releases allow_upgrades_to_beta = 0 +; Set to 1 by default. If you set to 0, the standalone plugins (with their own git repositories) +; will not be loaded when executing tests. +enable_load_standalone_plugins_during_tests = 1 + [General] ; the following settings control whether Unique Visitors will be processed for different period types. ; year and range periods are disabled by default, to ensure optimal performance for high traffic Piwik instances diff --git a/core/Plugin/Manager.php b/core/Plugin/Manager.php index 4bd1cc1d8c9..825c6fcaa45 100644 --- a/core/Plugin/Manager.php +++ b/core/Plugin/Manager.php @@ -11,6 +11,7 @@ use Piwik\Common; use Piwik\Config as PiwikConfig; +use Piwik\Config; use Piwik\EventDispatcher; use Piwik\Filesystem; use Piwik\Option; @@ -101,7 +102,15 @@ public function getPluginsToLoadDuringTests() // Also load plugins which are Git repositories (eg. being developed) $isPluginHasGitRepository = file_exists( PIWIK_INCLUDE_PATH . '/plugins/' . $plugin . '/.git/config'); - $loadPlugin = $isPluginBundledWithCore || $isPluginOfficiallySupported || $isPluginHasGitRepository; + $loadPlugin = $isPluginBundledWithCore || $isPluginOfficiallySupported; + + $loadStandalonePluginsDuringTests = Config::getInstance()->Debug['enable_load_standalone_plugins_during_tests']; + + if($loadStandalonePluginsDuringTests) { + $loadPlugin = $loadPlugin || $isPluginHasGitRepository; + } else { + $loadPlugin = $loadPlugin && !$isPluginHasGitRepository; + } // Do not enable other Themes $disabledThemes = $this->coreThemesDisabledByDefault; From e1387ab1087ae72173dda3803d369baea08fc8f6 Mon Sep 17 00:00:00 2001 From: mattab Date: Thu, 3 Apr 2014 15:40:32 +1300 Subject: [PATCH 16/29] Submodule --- tests/PHPUnit/UI | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/PHPUnit/UI b/tests/PHPUnit/UI index 618333d794b..6e2b0993fda 160000 --- a/tests/PHPUnit/UI +++ b/tests/PHPUnit/UI @@ -1 +1 @@ -Subproject commit 618333d794be0a8ad04c0106c0196fcc4f12d8dc +Subproject commit 6e2b0993fda17b6c4c2ac407c69fc09a1867dcd9 From d5e6e8f7b783f41324ec4aee4e13bd0eb159f87f Mon Sep 17 00:00:00 2001 From: Thomas Steur Date: Thu, 3 Apr 2014 04:48:35 +0200 Subject: [PATCH 17/29] some tweaks to the command ui output as well as some documentation and tests --- .../Commands/SetNumberOfCustomVariables.php | 27 +++++++++++++++---- plugins/CustomVariables/Model.php | 17 ++++++++++++ plugins/CustomVariables/tests/ModelTest.php | 16 +++++++++++ 3 files changed, 55 insertions(+), 5 deletions(-) diff --git a/plugins/CustomVariables/Commands/SetNumberOfCustomVariables.php b/plugins/CustomVariables/Commands/SetNumberOfCustomVariables.php index acd7233a1e3..b08946eab49 100644 --- a/plugins/CustomVariables/Commands/SetNumberOfCustomVariables.php +++ b/plugins/CustomVariables/Commands/SetNumberOfCustomVariables.php @@ -27,13 +27,13 @@ class SetNumberOfCustomVariables extends ConsoleCommand protected function configure() { - $this->setName('customvariables:set-number-available-custom-variables'); + $this->setName('customvariables:set-max-custom-variables'); $this->setDescription('Change the number of available custom variables'); $this->setHelp("Example: -./console customvariables:set-number-available-custom-variables 10 +./console customvariables:set-max-custom-variables 10 => 10 custom variables will be available in total "); - $this->addArgument('maxCustomVars', InputArgument::REQUIRED, 'The number of available custom variables'); + $this->addArgument('maxCustomVars', InputArgument::REQUIRED, 'Set the number of max available custom variables'); } protected function execute(InputInterface $input, OutputInterface $output) @@ -48,6 +48,11 @@ protected function execute(InputInterface $input, OutputInterface $output) return; } + + $output->writeln(''); + $output->writeln(sprintf('Configuring Piwik for %d custom variables', $numVarsToSet)); + + foreach (Model::getScopes() as $scope) { $this->printChanges($scope, $numVarsToSet, $output); } @@ -56,16 +61,19 @@ protected function execute(InputInterface $input, OutputInterface $output) return; } + $output->writeln(''); $output->writeln('Starting to apply changes'); $output->writeln(''); + $this->progress = $this->initProgress($numChangesToPerform, $output); foreach (Model::getScopes() as $scope) { $this->performChange($scope, $numVarsToSet, $output); } + Cache::clearCacheGeneral(); $this->progress->finish(); @@ -139,14 +147,23 @@ private function printChanges($scope, $numVarsToSet, OutputInterface $output) if ($numVarsToSet > $numCurrentCustomVars) { - $indexes = implode(',', range($highestIndex + 1, $highestIndex + $numVarsDifference)); + $indexes = $highestIndex + 1; + if (1 !== $numVarsDifference) { + $indexes .= ' - ' . ($highestIndex + $numVarsDifference); + } + $output->writeln( sprintf('%s new custom variables having the index(es) %s will be ADDED', $numVarsDifference, $indexes) ); } elseif ($numVarsToSet < $numCurrentCustomVars) { - $indexes = implode(',', range($highestIndex - $numVarsDifference + 1, $highestIndex)); + $indexes = $highestIndex - $numVarsDifference + 1; + + if (1 !== $numVarsDifference) { + $indexes .= ' - ' . $highestIndex; + } + $output->writeln( sprintf("%s existing custom variables having the index(es) %s will be REMOVED.", $numVarsDifference, $indexes) ); diff --git a/plugins/CustomVariables/Model.php b/plugins/CustomVariables/Model.php index 252718f87a2..60cbeac9470 100644 --- a/plugins/CustomVariables/Model.php +++ b/plugins/CustomVariables/Model.php @@ -43,6 +43,10 @@ public function getScopeName() } } + /** + * @see getHighestCustomVarIndex() + * @return int + */ public function getCurrentNumCustomVars() { $customVarColumns = $this->getCustomVarColumnNames(); @@ -52,6 +56,19 @@ public function getCurrentNumCustomVars() return (int) $currentNumCustomVars; } + /** + * result of getHighestCustomVarIndex() can be different to getCurrentNumCustomVars() in case there are some missing + * custom variable indexes. For instance in case of manual changes on the DB + * + * custom_var_v1 + * custom_var_v2 + * custom_var_v4 + * + * getHighestCustomVarIndex() -> returns 4 + * getCurrentNumCustomVars() -> returns 3 + * + * @return int + */ public function getHighestCustomVarIndex() { $columns = $this->getCustomVarColumnNames(); diff --git a/plugins/CustomVariables/tests/ModelTest.php b/plugins/CustomVariables/tests/ModelTest.php index 744dae2ce79..1cf77ac152f 100644 --- a/plugins/CustomVariables/tests/ModelTest.php +++ b/plugins/CustomVariables/tests/ModelTest.php @@ -17,6 +17,22 @@ */ class ModelTest extends \DatabaseTestCase { + /** + * @expectedException \Exception + */ + public function test_construct_shouldFailInCaseOfEmptyScope() + { + new Model(null); + } + + /** + * @expectedException \Exception + */ + public function test_construct_shouldFailInCaseOfInvalidScope() + { + new Model('inValId'); + } + public function testGetAllScopes() { $this->assertEquals(array('log_link_visit_action', 'log_visit', 'log_conversion'), Model::getScopes()); From f5bd856abd1499c3c087a145dcb455e4af8ec92c Mon Sep 17 00:00:00 2001 From: Thomas Steur Date: Thu, 3 Apr 2014 04:50:25 +0200 Subject: [PATCH 18/29] fix method call --- core/Tracker/Request.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/Tracker/Request.php b/core/Tracker/Request.php index ab79d5f4706..3c217f69211 100644 --- a/core/Tracker/Request.php +++ b/core/Tracker/Request.php @@ -356,7 +356,7 @@ public function getCustomVariables($scope) foreach ($customVar as $id => $keyValue) { $id = (int)$id; if ($id < 1 - || $id > CustomVariablesModel::getMaxCustomVariables() + || $id > CustomVariables::getMaxCustomVariables() || count($keyValue) != 2 || (!is_string($keyValue[0]) && !is_numeric($keyValue[0])) ) { From f693e297e32ec969bd8382d33b7a95f474ddbe80 Mon Sep 17 00:00:00 2001 From: Thomas Steur Date: Thu, 3 Apr 2014 04:56:16 +0200 Subject: [PATCH 19/29] fetchAll should exist even in tracker mode --- core/Db.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/core/Db.php b/core/Db.php index 3647a5878fc..5a65c8451bd 100644 --- a/core/Db.php +++ b/core/Db.php @@ -338,9 +338,14 @@ static public function dropTables($tables) */ static public function getColumnNamesFromTable($table) { - $columns = self::fetchAssoc("SHOW COLUMNS FROM " . $table); + $columns = self::fetchAll("SHOW COLUMNS FROM " . $table); - return array_keys($columns); + $columnNames = array(); + foreach ($columns as $column) { + $columnNames[] = $column['Field']; + } + + return $columnNames; } /** From baa840bf4a5d16bb6097625671c6e55d30becbac Mon Sep 17 00:00:00 2001 From: Thomas Steur Date: Thu, 3 Apr 2014 05:01:57 +0200 Subject: [PATCH 20/29] here we actually have a database test case --- tests/PHPUnit/{ => Integration}/Core/SegmentTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename tests/PHPUnit/{ => Integration}/Core/SegmentTest.php (99%) diff --git a/tests/PHPUnit/Core/SegmentTest.php b/tests/PHPUnit/Integration/Core/SegmentTest.php similarity index 99% rename from tests/PHPUnit/Core/SegmentTest.php rename to tests/PHPUnit/Integration/Core/SegmentTest.php index 1b272cc7a78..f0a313326a8 100644 --- a/tests/PHPUnit/Core/SegmentTest.php +++ b/tests/PHPUnit/Integration/Core/SegmentTest.php @@ -9,7 +9,7 @@ * @link http://piwik.org * @license http://www.gnu.org/licenses/gpl-3.0.html GPL v3 or later */ -class SegmentTest extends PHPUnit_Framework_TestCase +class SegmentTest extends DatabaseTestCase { public function setUp() { From 833fcf3506165e63fca8c82f9f91822e420dfe49 Mon Sep 17 00:00:00 2001 From: mattab Date: Thu, 3 Apr 2014 16:29:18 +1300 Subject: [PATCH 21/29] Fix notice --- core/CronArchive.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/CronArchive.php b/core/CronArchive.php index 0f3e877de58..784dd55b094 100644 --- a/core/CronArchive.php +++ b/core/CronArchive.php @@ -1122,6 +1122,9 @@ private function logFatalErrorUrlExpected() private function getVisitsLastPeriodFromApiResponse($stats) { + if(empty($stats)) { + return 0; + } $today = end($stats); return $today['nb_visits']; } From 2e125df447d76e8d618ca0b944fd2bad7adc145d Mon Sep 17 00:00:00 2001 From: mattab Date: Thu, 3 Apr 2014 16:30:13 +1300 Subject: [PATCH 22/29] Fix notice --- core/CronArchive.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core/CronArchive.php b/core/CronArchive.php index 784dd55b094..cbd5b36e4cf 100644 --- a/core/CronArchive.php +++ b/core/CronArchive.php @@ -1131,6 +1131,9 @@ private function getVisitsLastPeriodFromApiResponse($stats) private function getVisitsFromApiResponse($stats) { + if(empty($stats)) { + return 0; + } $visits = 0; foreach($stats as $metrics) { if(empty($metrics['nb_visits'])) { From 0dc5f51d9832dc34f0ccbe559635cd0e2a39f919 Mon Sep 17 00:00:00 2001 From: mattab Date: Thu, 3 Apr 2014 16:57:16 +1300 Subject: [PATCH 23/29] factor out this logic so it's reusable --- core/CronArchive.php | 2 +- plugins/CoreConsole/Commands/CoreArchiver.php | 42 +++++++++++-------- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/core/CronArchive.php b/core/CronArchive.php index cbd5b36e4cf..015298f37ca 100644 --- a/core/CronArchive.php +++ b/core/CronArchive.php @@ -1206,7 +1206,7 @@ public function __construct($message, $fullOutput = null) $this->fullOutput = $fullOutput; } - public function logAndExit($cronArchiver) + public function logAndExit(CronArchive $cronArchiver) { if ($cronArchiver->isCoreInited()) { $cronArchiver->logError($this->getMessage()); diff --git a/plugins/CoreConsole/Commands/CoreArchiver.php b/plugins/CoreConsole/Commands/CoreArchiver.php index 606f1d49411..fd7b566e233 100644 --- a/plugins/CoreConsole/Commands/CoreArchiver.php +++ b/plugins/CoreConsole/Commands/CoreArchiver.php @@ -18,24 +18,7 @@ class CoreArchiver extends ConsoleCommand { protected function configure() { - $this->setName('core:archive'); - $this->setDescription("Runs the CLI archiver. It usually runs as a cron and is a useful tool for general maintenance, and pre-process reports for a Fast dashboard rendering."); - $this->setHelp("* It is recommended to run the script with the option --piwik-domain=[piwik-server-url] only. Other options are not required. -* This script should be executed every hour via crontab, or as a daemon. -* You can also run it via http:// by specifying the Super User &token_auth=XYZ as a parameter ('Web Cron'), - but it is recommended to run it via command line/CLI instead. -* If you have any suggestion about this script, please let the team know at hello@piwik.org -* Enjoy!"); - $this->addOption('url', null, InputOption::VALUE_REQUIRED, "Mandatory option as an alternative to '--piwik-domain'. Must be set to the Piwik base URL.\nFor example: --url=http://analytics.example.org/ or --url=https://example.org/piwik/"); - $this->addOption('force-all-websites', null, InputOption::VALUE_NONE, "If specified, the script will trigger archiving on all websites and all past dates.\nYou may use --force-all-periods=[seconds] to trigger archiving on those websites\nthat had visits in the last [seconds] seconds."); - $this->addOption('force-all-periods', null, InputOption::VALUE_OPTIONAL, "Limits archiving to websites with some traffic in the last [seconds] seconds. \nFor example --force-all-periods=86400 will archive websites that had visits in the last 24 hours. \nIf [seconds] is not specified, all websites with visits in the last " . CronArchive::ARCHIVE_SITES_WITH_TRAFFIC_SINCE . "\n seconds (" . round( CronArchive::ARCHIVE_SITES_WITH_TRAFFIC_SINCE/86400 ) ." days) will be archived."); - $this->addOption('force-timeout-for-periods', null, InputOption::VALUE_OPTIONAL, "The current week/ current month/ current year will be processed at most every [seconds].\nIf not specified, defaults to ". CronArchive::SECONDS_DELAY_BETWEEN_PERIOD_ARCHIVES."."); - $this->addOption('force-date-last-n', null, InputOption::VALUE_REQUIRED, "This script calls the API with period=lastN. You can force the N in lastN by specifying this value."); - $this->addOption('force-idsites', null,InputOption::VALUE_REQUIRED, "Restricts archiving to the specified website IDs, comma separated list."); - $this->addOption('skip-idsites', null, InputOption::VALUE_REQUIRED, "If the specified websites IDs were to be archived, skip them instead."); - $this->addOption('disable-scheduled-tasks', null, InputOption::VALUE_NONE, "Skips executing Scheduled tasks (sending scheduled reports, db optimization, etc.)."); - $this->addOption('xhprof', null, InputOption::VALUE_NONE, "Enables XHProf profiler for this archive.php run. Requires XHPRof (see tests/README.xhprof.md)."); - $this->addOption('accept-invalid-ssl-certificate', null, InputOption::VALUE_NONE, "It is _NOT_ recommended to use this argument. Instead, you should use a valid SSL certificate!\nIt can be useful if you specified --url=https://... or if you are using Piwik with force_ssl=1"); + $this->configureArchiveCommand($this); } protected function execute(InputInterface $input, OutputInterface $output) @@ -46,4 +29,27 @@ protected function execute(InputInterface $input, OutputInterface $output) include PIWIK_INCLUDE_PATH . '/misc/cron/archive.php'; } + + // This is reused by another console command + static public function configureArchiveCommand(ConsoleCommand $command) + { + $command->setName('core:archive'); + $command->setDescription("Runs the CLI archiver. It usually runs as a cron and is a useful tool for general maintenance, and pre-process reports for a Fast dashboard rendering."); + $command->setHelp("* It is recommended to run the script with the option --piwik-domain=[piwik-server-url] only. Other options are not required. +* This script should be executed every hour via crontab, or as a daemon. +* You can also run it via http:// by specifying the Super User &token_auth=XYZ as a parameter ('Web Cron'), + but it is recommended to run it via command line/CLI instead. +* If you have any suggestion about this script, please let the team know at hello@piwik.org +* Enjoy!"); + $command->addOption('url', null, InputOption::VALUE_REQUIRED, "Mandatory option as an alternative to '--piwik-domain'. Must be set to the Piwik base URL.\nFor example: --url=http://analytics.example.org/ or --url=https://example.org/piwik/"); + $command->addOption('force-all-websites', null, InputOption::VALUE_NONE, "If specified, the script will trigger archiving on all websites and all past dates.\nYou may use --force-all-periods=[seconds] to trigger archiving on those websites\nthat had visits in the last [seconds] seconds."); + $command->addOption('force-all-periods', null, InputOption::VALUE_OPTIONAL, "Limits archiving to websites with some traffic in the last [seconds] seconds. \nFor example --force-all-periods=86400 will archive websites that had visits in the last 24 hours. \nIf [seconds] is not specified, all websites with visits in the last " . CronArchive::ARCHIVE_SITES_WITH_TRAFFIC_SINCE . "\n seconds (" . round(CronArchive::ARCHIVE_SITES_WITH_TRAFFIC_SINCE / 86400) . " days) will be archived."); + $command->addOption('force-timeout-for-periods', null, InputOption::VALUE_OPTIONAL, "The current week/ current month/ current year will be processed at most every [seconds].\nIf not specified, defaults to " . CronArchive::SECONDS_DELAY_BETWEEN_PERIOD_ARCHIVES . "."); + $command->addOption('force-date-last-n', null, InputOption::VALUE_REQUIRED, "This script calls the API with period=lastN. You can force the N in lastN by specifying this value."); + $command->addOption("force-idsites", null, InputOption::VALUE_OPTIONAL, 'If specified, archiving will be processed only for these Sites Ids (comma separated)'); + $command->addOption("skip-idsites", null, InputOption::VALUE_OPTIONAL, 'If specified, archiving will be skipped for these websites (in case these website ids would have been archived).'); + $command->addOption('disable-scheduled-tasks', null, InputOption::VALUE_NONE, "Skips executing Scheduled tasks (sending scheduled reports, db optimization, etc.)."); + $command->addOption('xhprof', null, InputOption::VALUE_NONE, "Enables XHProf profiler for this archive.php run. Requires XHPRof (see tests/README.xhprof.md)."); + $command->addOption('accept-invalid-ssl-certificate', null, InputOption::VALUE_NONE, "It is _NOT_ recommended to use this argument. Instead, you should use a valid SSL certificate!\nIt can be useful if you specified --url=https://... or if you are using Piwik with force_ssl=1"); + } } \ No newline at end of file From 5cbbb4f47b69e977aa43b4403fb3459187f0111b Mon Sep 17 00:00:00 2001 From: Thomas Steur Date: Thu, 3 Apr 2014 06:03:19 +0200 Subject: [PATCH 24/29] some more tweaks and added a comment to get info about currently configured custom vars --- core/Tracker/GoalManager.php | 8 ++- core/Tracker/Request.php | 3 +- core/Tracker/Visit.php | 3 +- plugins/CustomVariables/Archiver.php | 3 +- plugins/CustomVariables/Commands/Info.php | 69 +++++++++++++++++++++ plugins/CustomVariables/CustomVariables.php | 5 +- plugins/CustomVariables/Model.php | 32 +++++++--- plugins/CustomVariables/tests/ModelTest.php | 14 +++++ plugins/Live/Visitor.php | 14 ++++- 9 files changed, 132 insertions(+), 19 deletions(-) create mode 100644 plugins/CustomVariables/Commands/Info.php diff --git a/core/Tracker/GoalManager.php b/core/Tracker/GoalManager.php index c4e7fb67db7..daa88bd7b9a 100644 --- a/core/Tracker/GoalManager.php +++ b/core/Tracker/GoalManager.php @@ -227,7 +227,11 @@ public function recordGoals($idSite, $visitorInformation, $visitCustomVariables, } // Copy Custom Variables from Visit row to the Goal conversion - for ($i = 1; $i <= CustomVariables::getMaxCustomVariables(); $i++) { + // Otherwise, set the Custom Variables found in the cookie sent with this request + $goal += $visitCustomVariables; + $maxCustomVariables = CustomVariables::getMaxCustomVariables(); + + for ($i = 1; $i <= $maxCustomVariables; $i++) { if (isset($visitorInformation['custom_var_k' . $i]) && strlen($visitorInformation['custom_var_k' . $i]) ) { @@ -239,8 +243,6 @@ public function recordGoals($idSite, $visitorInformation, $visitCustomVariables, $goal['custom_var_v' . $i] = $visitorInformation['custom_var_v' . $i]; } } - // Otherwise, set the Custom Variables found in the cookie sent with this request - $goal += $visitCustomVariables; // Attributing the correct Referrer to this conversion. // Priority order is as follows: diff --git a/core/Tracker/Request.php b/core/Tracker/Request.php index 3c217f69211..df551e7d823 100644 --- a/core/Tracker/Request.php +++ b/core/Tracker/Request.php @@ -353,10 +353,11 @@ public function getCustomVariables($scope) return array(); } $customVariables = array(); + $maxCustomVars = CustomVariables::getMaxCustomVariables(); foreach ($customVar as $id => $keyValue) { $id = (int)$id; if ($id < 1 - || $id > CustomVariables::getMaxCustomVariables() + || $id > $maxCustomVars || count($keyValue) != 2 || (!is_string($keyValue[0]) && !is_numeric($keyValue[0])) ) { diff --git a/core/Tracker/Visit.php b/core/Tracker/Visit.php index 5926e8f1e2c..f18c6563108 100644 --- a/core/Tracker/Visit.php +++ b/core/Tracker/Visit.php @@ -513,7 +513,8 @@ protected function recognizeTheVisitor() // Custom Variables copied from Visit in potential later conversion if (!empty($selectCustomVariables)) { - for ($i = 1; $i <= CustomVariables::getMaxCustomVariables(); $i++) { + $maxCustomVariables = CustomVariables::getMaxCustomVariables(); + for ($i = 1; $i <= $maxCustomVariables; $i++) { if (isset($visitRow['custom_var_k' . $i]) && strlen($visitRow['custom_var_k' . $i]) ) { diff --git a/plugins/CustomVariables/Archiver.php b/plugins/CustomVariables/Archiver.php index f2bdd4d6da4..d7ae1ac0151 100644 --- a/plugins/CustomVariables/Archiver.php +++ b/plugins/CustomVariables/Archiver.php @@ -59,7 +59,8 @@ public function aggregateDayReport() { $this->dataArray = new DataArray(); - for ($i = 1; $i <= CustomVariables::getMaxCustomVariables(); $i++) { + $maxCustomVariables = CustomVariables::getMaxCustomVariables(); + for ($i = 1; $i <= $maxCustomVariables; $i++) { $this->aggregateCustomVariable($i); } diff --git a/plugins/CustomVariables/Commands/Info.php b/plugins/CustomVariables/Commands/Info.php new file mode 100644 index 00000000000..5ec0fa0b343 --- /dev/null +++ b/plugins/CustomVariables/Commands/Info.php @@ -0,0 +1,69 @@ +setName('customvariables:info'); + $this->setDescription('Get info about configured custom variables'); + } + + protected function execute(InputInterface $input, OutputInterface $output) + { + $maxVars = CustomVariables::getMaxCustomVariables(); + + if ($this->hasEverywhereSameAmountOfVariables()) { + $this->writeSuccessMessage($output, array( + 'Your Piwik is configured for ' . $maxVars . ' custom variables.' + )); + return; + } + + $output->writeln('There is a problem with your custom variables configuration:'); + $output->writeln('Some database tables miss custom variables columns.'); + $output->writeln(''); + $output->writeln('Your Piwik seems to be configured for ' . $maxVars . ' custom variables.'); + $output->writeln('Executing "./console set-max-custom-variables ' . $maxVars . '" might fix this issue.'); + $output->writeln('If not check the following tables whether they have the same columns starting with custom_var_: '); + foreach (Model::getScopes() as $scope) { + $output->writeln(Common::prefixTable($scope)); + } + } + + private function hasEverywhereSameAmountOfVariables() + { + $indexesBefore = null; + + foreach (Model::getScopes() as $scope) { + $model = new Model($scope); + $indexes = $model->getCustomVarIndexes(); + + if (is_null($indexesBefore)) { + $indexesBefore = $indexes; + } elseif ($indexes != $indexesBefore) { + return false; + } + } + + return true; + } +} diff --git a/plugins/CustomVariables/CustomVariables.php b/plugins/CustomVariables/CustomVariables.php index 6a7ae6618e5..b4e71517a34 100644 --- a/plugins/CustomVariables/CustomVariables.php +++ b/plugins/CustomVariables/CustomVariables.php @@ -96,6 +96,7 @@ public static function getMaxCustomVariables() public function addConsoleCommands(&$commands) { $commands[] = __NAMESPACE__ . '\\Commands\\SetNumberOfCustomVariables'; + $commands[] = __NAMESPACE__ . '\\Commands\\Info'; } /** @@ -127,7 +128,9 @@ public function getReportMetadata(&$reports) public function getSegmentsMetadata(&$segments) { - for ($i = 1; $i <= self::getMaxCustomVariables(); $i++) { + $maxCustomVariables = self::getMaxCustomVariables(); + + for ($i = 1; $i <= $maxCustomVariables; $i++) { $segments[] = array( 'type' => 'dimension', 'category' => 'CustomVariables_CustomVariables', diff --git a/plugins/CustomVariables/Model.php b/plugins/CustomVariables/Model.php index 60cbeac9470..c2c425b06e2 100644 --- a/plugins/CustomVariables/Model.php +++ b/plugins/CustomVariables/Model.php @@ -49,11 +49,9 @@ public function getScopeName() */ public function getCurrentNumCustomVars() { - $customVarColumns = $this->getCustomVarColumnNames(); + $indexes = $this->getCustomVarIndexes(); - $currentNumCustomVars = count($customVarColumns) / 2; - - return (int) $currentNumCustomVars; + return count($indexes); } /** @@ -70,23 +68,34 @@ public function getCurrentNumCustomVars() * @return int */ public function getHighestCustomVarIndex() + { + $indexes = $this->getCustomVarIndexes(); + + if (empty($indexes)) { + return 0; + } + + return max($indexes); + } + + public function getCustomVarIndexes() { $columns = $this->getCustomVarColumnNames(); if (empty($columns)) { - return 0; + return array(); } $indexes = array_map(function ($column) { return Model::getCustomVariableIndexFromFieldName($column); }, $columns); - return max($indexes); + return array_values(array_unique($indexes)); } private function getCustomVarColumnNames() { - $dbTable = Common::prefixTable($this->scope); + $dbTable = $this->getDbTableName(); $columns = Db::getColumnNamesFromTable($dbTable); $customVarColumns = array_filter($columns, function ($column) { @@ -98,7 +107,7 @@ private function getCustomVarColumnNames() public function removeCustomVariable() { - $dbTable = Common::prefixTable($this->scope); + $dbTable = $this->getDbTableName(); $index = $this->getHighestCustomVarIndex(); if ($index < 1) { @@ -113,7 +122,7 @@ public function removeCustomVariable() public function addCustomVariable() { - $dbTable = Common::prefixTable($this->scope); + $dbTable = $this->getDbTableName(); $index = $this->getHighestCustomVarIndex() + 1; $maxLen = CustomVariables::getMaxLengthCustomVariables(); @@ -123,6 +132,11 @@ public function addCustomVariable() return $index; } + private function getDbTableName() + { + return Common::prefixTable($this->scope); + } + public static function getCustomVariableIndexFromFieldName($fieldName) { $onlyNumber = str_replace(array('custom_var_k', 'custom_var_v'), '', $fieldName); diff --git a/plugins/CustomVariables/tests/ModelTest.php b/plugins/CustomVariables/tests/ModelTest.php index 1cf77ac152f..14d8647bf03 100644 --- a/plugins/CustomVariables/tests/ModelTest.php +++ b/plugins/CustomVariables/tests/ModelTest.php @@ -70,6 +70,20 @@ public function test_getCurrentNumCustomVars() $this->assertEquals(4, $this->getConversionScope()->getCurrentNumCustomVars()); } + public function test_getCustomVarIndexes() + { + $this->assertEquals(array(1,2,3,4,5), $this->getPageScope()->getCustomVarIndexes()); + $this->assertEquals(array(1,2,3,4,5), $this->getVisitScope()->getCustomVarIndexes()); + $this->assertEquals(array(1,2,3,4,5), $this->getConversionScope()->getCustomVarIndexes()); + + $this->getPageScope()->addCustomVariable(); + $this->getConversionScope()->removeCustomVariable(); + + $this->assertEquals(array(1,2,3,4,5,6), $this->getPageScope()->getCustomVarIndexes()); + $this->assertEquals(array(1,2,3,4,5), $this->getVisitScope()->getCustomVarIndexes()); + $this->assertEquals(array(1,2,3,4), $this->getConversionScope()->getCustomVarIndexes()); + } + public function test_getHighestCustomVarIndex_addCustomVariable_removeCustomVariable() { $this->assertEquals(5, $this->getPageScope()->getHighestCustomVarIndex()); diff --git a/plugins/Live/Visitor.php b/plugins/Live/Visitor.php index 37a8c20f771..9e13d9b514d 100644 --- a/plugins/Live/Visitor.php +++ b/plugins/Live/Visitor.php @@ -339,7 +339,10 @@ function getLongitude() function getCustomVariables() { $customVariables = array(); - for ($i = 1; $i <= CustomVariables::getMaxCustomVariables(); $i++) { + + $maxCustomVariables = CustomVariables::getMaxCustomVariables(); + + for ($i = 1; $i <= $maxCustomVariables; $i++) { if (!empty($this->details['custom_var_k' . $i])) { $customVariables[$i] = array( 'customVariableName' . $i => $this->details['custom_var_k' . $i], @@ -724,8 +727,10 @@ public static function enrichVisitorArrayWithActions($visitorDetailsArray, $acti { $idVisit = $visitorDetailsArray['idVisit']; + $maxCustomVariables = CustomVariables::getMaxCustomVariables(); + $sqlCustomVariables = ''; - for ($i = 1; $i <= CustomVariables::getMaxCustomVariables(); $i++) { + for ($i = 1; $i <= $maxCustomVariables; $i++) { $sqlCustomVariables .= ', custom_var_k' . $i . ', custom_var_v' . $i; } // The second join is a LEFT join to allow returning records that don't have a matching page title @@ -762,7 +767,10 @@ public static function enrichVisitorArrayWithActions($visitorDetailsArray, $acti foreach ($actionDetails as $actionIdx => &$actionDetail) { $actionDetail =& $actionDetails[$actionIdx]; $customVariablesPage = array(); - for ($i = 1; $i <= CustomVariables::getMaxCustomVariables(); $i++) { + + $maxCustomVariables = CustomVariables::getMaxCustomVariables(); + + for ($i = 1; $i <= $maxCustomVariables; $i++) { if (!empty($actionDetail['custom_var_k' . $i])) { $cvarKey = $actionDetail['custom_var_k' . $i]; $cvarKey = static::getCustomVariablePrettyKey($cvarKey); From 4ac53a8359a512eb4d4c066bf3ba0b006c3a2147 Mon Sep 17 00:00:00 2001 From: Thomas Steur Date: Thu, 3 Apr 2014 06:30:57 +0200 Subject: [PATCH 25/29] some more bug fixes --- plugins/API/Controller.php | 5 ++++- plugins/CoreAdminHome/Controller.php | 2 ++ plugins/CoreAdminHome/javascripts/jsTrackingGenerator.js | 5 +++-- plugins/CoreAdminHome/templates/trackingCodeGenerator.twig | 2 +- plugins/CustomVariables/Commands/Info.php | 2 +- .../CustomVariables/Commands/SetNumberOfCustomVariables.php | 4 ++-- plugins/CustomVariables/Model.php | 2 +- 7 files changed, 14 insertions(+), 8 deletions(-) diff --git a/plugins/API/Controller.php b/plugins/API/Controller.php index b80eb577a3a..349cdacbfbb 100644 --- a/plugins/API/Controller.php +++ b/plugins/API/Controller.php @@ -62,7 +62,10 @@ public function listSegments() $segment['type'] = 'dimension'; } - $onlyDisplay = array('customVariableName1', 'customVariableName2', 'customVariableValue1', 'customVariableValue2', 'customVariablePageName1', 'customVariablePageValue1'); + $onlyDisplay = array('customVariableName1', 'customVariableName2', + 'customVariableValue1', 'customVariableValue2', + 'customVariablePageName1', 'customVariablePageValue1'); + $customVariableWillBeDisplayed = in_array($segment['segment'], $onlyDisplay); // Don't display more than 4 custom variables name/value rows if ($segment['category'] == 'Custom Variables' diff --git a/plugins/CoreAdminHome/Controller.php b/plugins/CoreAdminHome/Controller.php index fbc351e7567..de24bf76f1a 100644 --- a/plugins/CoreAdminHome/Controller.php +++ b/plugins/CoreAdminHome/Controller.php @@ -19,6 +19,7 @@ use Piwik\Option; use Piwik\Piwik; use Piwik\Plugins\CorePluginsAdmin\UpdateCommunication; +use Piwik\Plugins\CustomVariables\CustomVariables; use Piwik\Plugins\LanguagesManager\API as APILanguagesManager; use Piwik\Plugins\LanguagesManager\LanguagesManager; use Piwik\Plugins\SitesManager\API as APISitesManager; @@ -210,6 +211,7 @@ public function trackingCodeGenerator() $view->defaultReportSiteName = Site::getNameFor($view->idSite); $view->defaultSiteRevenue = \Piwik\MetricsFormatter::getCurrencySymbol($view->idSite); + $view->maxCustomVariables = CustomVariables::getMaxCustomVariables(); $allUrls = APISitesManager::getInstance()->getSiteUrlsFromId($view->idSite); if (isset($allUrls[1])) { diff --git a/plugins/CoreAdminHome/javascripts/jsTrackingGenerator.js b/plugins/CoreAdminHome/javascripts/jsTrackingGenerator.js index 707a3eef946..03d854c1761 100644 --- a/plugins/CoreAdminHome/javascripts/jsTrackingGenerator.js +++ b/plugins/CoreAdminHome/javascripts/jsTrackingGenerator.js @@ -27,6 +27,7 @@ // get preloaded server-side data necessary for code generation var dataElement = $('#js-tracking-generator-data'), currencySymbols = JSON.parse(dataElement.attr('data-currencies')), + maxCustomVariables = parseInt(dataElement.attr('max-custom-variables'), 10), siteUrls = {}, siteCurrencies = {}, allGoals = {}, @@ -268,8 +269,8 @@ row.before(newRow); // hide add button if max # of custom variables has been reached - // (5 custom variables + 1 row for add new row) - if ($('tr', row.parent()).length == 6) { + // (X custom variables + 1 row for add new row) + if ($('tr', row.parent()).length == (maxCustomVariables + 1)) { $(this).hide(); } diff --git a/plugins/CoreAdminHome/templates/trackingCodeGenerator.twig b/plugins/CoreAdminHome/templates/trackingCodeGenerator.twig index 10ac52386a4..2666ec62ac7 100644 --- a/plugins/CoreAdminHome/templates/trackingCodeGenerator.twig +++ b/plugins/CoreAdminHome/templates/trackingCodeGenerator.twig @@ -7,7 +7,7 @@ {% endblock %} {% block content %} -
+

getArgument('maxCustomVars'); if (!is_numeric($maxCustomVars)) { - throw new \Exception('The number of available custom variables has to be number'); + throw new \Exception('The number of available custom variables has to be a number'); } $maxCustomVars = (int) $maxCustomVars; if ($maxCustomVars <= 0) { - throw new \Exception('There has to be at least 1 custom variable'); + throw new \Exception('There has to be at least one custom variable'); } return $maxCustomVars; diff --git a/plugins/CustomVariables/Model.php b/plugins/CustomVariables/Model.php index c2c425b06e2..07e7ca94f75 100644 --- a/plugins/CustomVariables/Model.php +++ b/plugins/CustomVariables/Model.php @@ -155,7 +155,7 @@ public static function install() { foreach (self::getScopes() as $scope) { $model = new Model($scope); - +return; try { for ($index = 0; $index < 5; $index++) { $model->addCustomVariable(); From 3ce67fb0ea90b7dbc2a94f3367ec33759b0c3005 Mon Sep 17 00:00:00 2001 From: Thomas Steur Date: Thu, 3 Apr 2014 06:33:48 +0200 Subject: [PATCH 26/29] forgot to remove return statement --- plugins/CustomVariables/Model.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/CustomVariables/Model.php b/plugins/CustomVariables/Model.php index 07e7ca94f75..c2c425b06e2 100644 --- a/plugins/CustomVariables/Model.php +++ b/plugins/CustomVariables/Model.php @@ -155,7 +155,7 @@ public static function install() { foreach (self::getScopes() as $scope) { $model = new Model($scope); -return; + try { for ($index = 0; $index < 5; $index++) { $model->addCustomVariable(); From b226529a062aa67cd93f72a6f596c0d08c790421 Mon Sep 17 00:00:00 2001 From: mattab Date: Thu, 3 Apr 2014 17:38:04 +1300 Subject: [PATCH 27/29] Tweak console commands descriptions --- plugins/CoreConsole/Commands/CoreArchiver.php | 2 +- plugins/CoreConsole/Commands/SyncUITestScreenshots.php | 4 ++-- plugins/CoreUpdater/Commands/Update.php | 3 ++- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/plugins/CoreConsole/Commands/CoreArchiver.php b/plugins/CoreConsole/Commands/CoreArchiver.php index fd7b566e233..eae8d80f514 100644 --- a/plugins/CoreConsole/Commands/CoreArchiver.php +++ b/plugins/CoreConsole/Commands/CoreArchiver.php @@ -34,7 +34,7 @@ protected function execute(InputInterface $input, OutputInterface $output) static public function configureArchiveCommand(ConsoleCommand $command) { $command->setName('core:archive'); - $command->setDescription("Runs the CLI archiver. It usually runs as a cron and is a useful tool for general maintenance, and pre-process reports for a Fast dashboard rendering."); + $command->setDescription("Runs the CLI archiver. It is an important tool for general maintenance and to keep Piwik very fast."); $command->setHelp("* It is recommended to run the script with the option --piwik-domain=[piwik-server-url] only. Other options are not required. * This script should be executed every hour via crontab, or as a daemon. * You can also run it via http:// by specifying the Super User &token_auth=XYZ as a parameter ('Web Cron'), diff --git a/plugins/CoreConsole/Commands/SyncUITestScreenshots.php b/plugins/CoreConsole/Commands/SyncUITestScreenshots.php index 639b7d3db5d..3deb5d72e18 100644 --- a/plugins/CoreConsole/Commands/SyncUITestScreenshots.php +++ b/plugins/CoreConsole/Commands/SyncUITestScreenshots.php @@ -22,8 +22,8 @@ class SyncUITestScreenshots extends ConsoleCommand protected function configure() { $this->setName('development:sync-ui-test-screenshots'); - $this->setDescription('This command is intended for Piwik core developers. It copies all processed screenshot ' - . 'tests on Travis to the expected screenshot directory.'); + $this->setDescription('For Piwik core devs. Copies screenshots ' + . 'from travis artifacts to tests/PHPUnit/UI/expected-ui-screenshots/'); $this->addArgument('buildnumber', InputArgument::REQUIRED, 'Travis build number you want to sync.'); $this->addArgument('screenshotsRegex', InputArgument::OPTIONAL, 'A regex to use when selecting screenshots to copy. If not supplied all screenshots are copied.', '.*'); diff --git a/plugins/CoreUpdater/Commands/Update.php b/plugins/CoreUpdater/Commands/Update.php index 226117df1ef..04171407672 100644 --- a/plugins/CoreUpdater/Commands/Update.php +++ b/plugins/CoreUpdater/Commands/Update.php @@ -27,7 +27,8 @@ class Update extends ConsoleCommand protected function configure() { $this->setName('core:update'); - $this->setDescription('Triggers the upgrades for Piwik core and plugins. Useful after Piwik core files or some plugins were updated to latest files.'); + + $this->setDescription('Triggers upgrades. Use it after Piwik core or any plugin files have been updated.'); $this->addOption('dry-run', null, InputOption::VALUE_NONE, 'Only prints out the SQL requests that would be executed during the upgrade'); } From 263b0700c9f700bc00749fbadc0f67f4bc478b26 Mon Sep 17 00:00:00 2001 From: diosmosis Date: Thu, 3 Apr 2014 06:49:36 +0100 Subject: [PATCH 28/29] Refs #4878, create update script for converting *_returning metrics to new segmented metrics. Removed Archiver and API. Old segmented VisitFrequency metrics will not be accessible. --- core/ArchiveProcessor/Rules.php | 2 +- core/Updater.php | 45 +++++++--- core/Updates/2.1.1-b11.php | 127 +++++++++++++++++++++++++++ core/Version.php | 2 +- plugins/CustomAlerts | 2 +- plugins/Goals/Controller.php | 2 +- plugins/VisitFrequency/API.php | 123 +++++++++++--------------- plugins/VisitFrequency/Archiver.php | 131 ---------------------------- 8 files changed, 216 insertions(+), 218 deletions(-) create mode 100644 core/Updates/2.1.1-b11.php delete mode 100644 plugins/VisitFrequency/Archiver.php diff --git a/core/ArchiveProcessor/Rules.php b/core/ArchiveProcessor/Rules.php index 02a8357ac68..13f2a346107 100644 --- a/core/ArchiveProcessor/Rules.php +++ b/core/ArchiveProcessor/Rules.php @@ -98,7 +98,7 @@ private static function getSegmentsToProcess($idSites) return $segmentsToProcess; } - private static function getDoneFlagArchiveContainsOnePlugin(Segment $segment, $plugin, $isSkipAggregationOfSubTables = false) + public static function getDoneFlagArchiveContainsOnePlugin(Segment $segment, $plugin, $isSkipAggregationOfSubTables = false) { $partial = self::isFlagArchivePartial($plugin, $isSkipAggregationOfSubTables); return 'done' . $segment->getHash() . '.' . $plugin . $partial ; diff --git a/core/Updater.php b/core/Updater.php index 59587cb19b8..a1f39fb71b4 100644 --- a/core/Updater.php +++ b/core/Updater.php @@ -287,16 +287,41 @@ public function getComponentsWithNewVersion() static function updateDatabase($file, $sqlarray) { foreach ($sqlarray as $update => $ignoreError) { - try { - Db::exec($update); - } catch (\Exception $e) { - if (($ignoreError === false) - || !Db::get()->isErrNo($e, $ignoreError) - ) { - $message = $file . ":\nError trying to execute the query '" . $update . "'.\nThe error was: " . $e->getMessage(); - throw new UpdaterErrorException($message); - } - } + self::executeMigrationQuery($update, $ignoreError, $file); + } + } + + /** + * Executes a database update query. + * + * @param string $updateSql Update SQL query. + * @param int|false $errorToIgnore A MySQL error code to ignore. + * @param string $file The Update file that's calling this method. + */ + public static function executeMigrationQuery($updateSql, $errorToIgnore, $file) + { + try { + Db::exec($updateSql); + } catch (\Exception $e) { + self::handleQueryError($e, $updateSql, $errorToIgnore, $file); + } + } + + /** + * Handle an error that is thrown from a database query. + * + * @param \Exception $e the exception thrown. + * @param string $updateSql Update SQL query. + * @param int|false $errorToIgnore A MySQL error code to ignore. + * @param string $file The Update file that's calling this method. + */ + public static function handleQueryError($e, $updateSql, $errorToIgnore, $file) + { + if (($errorToIgnore === false) + || !Db::get()->isErrNo($e, $errorToIgnore) + ) { + $message = $file . ":\nError trying to execute the query '" . $updateSql . "'.\nThe error was: " . $e->getMessage(); + throw new UpdaterErrorException($message); } } } diff --git a/core/Updates/2.1.1-b11.php b/core/Updates/2.1.1-b11.php new file mode 100644 index 00000000000..b38f92e3316 --- /dev/null +++ b/core/Updates/2.1.1-b11.php @@ -0,0 +1,127 @@ +getDatetime(); + + $archiveNumericTables = Db::get()->fetchCol("SHOW TABLES LIKE '%archive_numeric%'"); + + // for each numeric archive table, copy *_returning metrics to VisitsSummary metrics w/ the appropriate + // returning visit segment + foreach ($archiveNumericTables as $table) { + // get archives w/ *._returning + $sql = "SELECT idarchive, idsite, period, date1, date2 + FROM $table + WHERE name IN ('" . implode("','", $returningMetrics) . "') + GROUP BY idarchive"; + $idArchivesWithReturning = Db::fetchAll($sql); + + // get archives for visitssummary returning visitor segment + $sql = "SELECT idarchive, idsite, period, date1, date2 + FROM $table + WHERE name = ? + GROUP BY idarchive"; + $visitSummaryReturningSegmentDone = Rules::getDoneFlagArchiveContainsOnePlugin( + new Segment(VisitFrequencyApi::RETURNING_VISITOR_SEGMENT, $idSites = array()), 'VisitsSummary'); + $idArchivesWithVisitReturningSegment = Db::fetchAll($sql, array($visitSummaryReturningSegmentDone)); + + // collect info for new visitssummary archives have to be created to match archives w/ *._returning + // metrics + $missingIdArchives = array(); + $idArchiveMappings = array(); + foreach ($idArchivesWithReturning as $row) { + $withMetricsIdArchive = $row['idarchive']; + foreach ($idArchivesWithVisitReturningSegment as $segmentRow) { + if ($row['idsite'] == $segmentRow['idsite'] + && $row['period'] == $segmentRow['period'] + && $row['date1'] == $segmentRow['date1'] + && $row['date2'] == $segmentRow['date2'] + ) { + $idArchiveMappings[$withMetricsIdArchive] = $segmentRow['idarchive']; + } + } + + if (!isset($idArchiveMappings[$withMetricsIdArchive])) { + $missingIdArchives[$withMetricsIdArchive] = $row; + } + } + + // if there are missing idarchives, fill out new archive row values + if (!empty($missingIdArchives)) { + $newIdArchiveStart = Db::fetchOne("SELECT MAX(idarchive) FROM $table") + 1; + foreach ($missingIdArchives as $withMetricsIdArchive => &$rowToInsert) { + $idArchiveMappings[$withMetricsIdArchive] = $newIdArchiveStart; + + $rowToInsert['idarchive'] = $newIdArchiveStart; + $rowToInsert['ts_archived'] = $now; + $rowToInsert['name'] = $visitSummaryReturningSegmentDone; + $rowToInsert['value'] = ArchiveWriter::DONE_OK; + + ++$newIdArchiveStart; + } + + // add missing archives + try { + BatchInsert::tableInsertBatch($table, array_keys(reset($missingIdArchives)), $missingIdArchives, $throwException = false); + } catch (\Exception $ex) { + Updater::handleQueryError($ex, "", false, __FILE__); + } + } + + // update idarchive & name columns in rows with *._returning metrics + $updateSqlPrefix = "UPDATE $table + SET idarchive = CASE idarchive "; + $updateSqlSuffix = " END, name = CASE name "; + foreach ($returningMetrics as $metric) { + $newMetricName = substr($metric, 0, strlen($metric) - strlen(VisitFrequencyApi::COLUMN_SUFFIX)); + $updateSqlSuffix .= "WHEN '$metric' THEN '" . $newMetricName . "' "; + } + $updateSqlSuffix .= " END WHERE idarchive IN (" . implode(',', array_keys($idArchiveMappings)) . ") + AND name IN ('" . implode("','", $returningMetrics) . "')"; + + // update only 1000 rows at a time so we don't send too large an SQL query to MySQL + foreach (array_chunk($missingIdArchives, 1000, $preserveKeys = true) as $chunk) { + $idArchives = array(); + + $updateSql = $updateSqlPrefix; + foreach ($chunk as $withMetricsIdArchive => $row) { + $updateSql .= "WHEN $withMetricsIdArchive THEN {$row['idarchive']} "; + } + $updateSql .= $updateSqlSuffix; + + Updater::executeMigrationQuery($updateSql, false, __FILE__); + } + } + } +} diff --git a/core/Version.php b/core/Version.php index 889a5cd0654..fbe16426b5d 100644 --- a/core/Version.php +++ b/core/Version.php @@ -21,5 +21,5 @@ final class Version * The current Piwik version. * @var string */ - const VERSION = '2.1.1-b10'; + const VERSION = '2.1.1-b11'; } diff --git a/plugins/CustomAlerts b/plugins/CustomAlerts index a1125e96827..c9edd879003 160000 --- a/plugins/CustomAlerts +++ b/plugins/CustomAlerts @@ -1 +1 @@ -Subproject commit a1125e96827e12c7bd5c3d6107b9c162aace32f8 +Subproject commit c9edd879003ca7a9f75d81fe07ec6ff0c2340f91 diff --git a/plugins/Goals/Controller.php b/plugins/Goals/Controller.php index ba8a68efedb..2d64135948a 100644 --- a/plugins/Goals/Controller.php +++ b/plugins/Goals/Controller.php @@ -145,7 +145,7 @@ protected function getGoalReportView($idGoal = false) $view->topDimensions = $this->getTopDimensions($idGoal); // conversion rate for new and returning visitors - $segment = urldecode(\Piwik\Plugins\VisitFrequency\Archiver::RETURNING_VISITOR_SEGMENT); + $segment = urldecode(\Piwik\Plugins\VisitFrequency\API::RETURNING_VISITOR_SEGMENT); $conversionRateReturning = API::getInstance()->getConversionRate($this->idSite, Common::getRequestVar('period'), Common::getRequestVar('date'), $segment, $idGoal); $view->conversion_rate_returning = $this->formatConversionRate($conversionRateReturning); $segment = 'visitorType==new'; diff --git a/plugins/VisitFrequency/API.php b/plugins/VisitFrequency/API.php index 4c0f724398f..e837fee01d2 100644 --- a/plugins/VisitFrequency/API.php +++ b/plugins/VisitFrequency/API.php @@ -15,6 +15,7 @@ use Piwik\Archive; use Piwik\SegmentExpression; use Piwik\SettingsPiwik; +use Piwik\Plugins\VisitsSummary\API as APIVisitsSummary; /** * VisitFrequency API lets you access a list of metrics related to Returning Visitors. @@ -22,86 +23,62 @@ */ class API extends \Piwik\Plugin\API { + // visitorType==returning,visitorType==returningCustomer + const RETURNING_VISITOR_SEGMENT = "visitorType%3D%3Dreturning%2CvisitorType%3D%3DreturningCustomer"; + const COLUMN_SUFFIX = "_returning"; + + /** + * @param int $idSite + * @param string $period + * @param string $date + * @param bool|string $segment + * @param bool|array $columns + * @return mixed + */ public function get($idSite, $period, $date, $segment = false, $columns = false) { - $archive = Archive::build($idSite, $period, $date, $segment); - - // array values are comma separated - $columns = Piwik::getArrayFromApiParameter($columns); - $tempColumns = array(); - - $bounceRateReturningRequested = $averageVisitDurationReturningRequested = $actionsPerVisitReturningRequested = false; - if (!empty($columns)) { - // make sure base metrics are there for processed metrics - if (false !== ($bounceRateReturningRequested = array_search('bounce_rate_returning', $columns))) { - if (!in_array('nb_visits_returning', $columns)) { - $tempColumns[] = 'nb_visits_returning'; - } - - if (!in_array('bounce_count_returning', $columns)) { - $tempColumns[] = 'bounce_count_returning'; - } - - unset($columns[$bounceRateReturningRequested]); - } - - if (false !== ($actionsPerVisitReturningRequested = array_search('nb_actions_per_visit_returning', $columns))) { - if (!in_array('nb_actions_returning', $columns)) { - $tempColumns[] = 'nb_actions_returning'; - } - - if (!in_array('nb_visits_returning', $columns)) { - $tempColumns[] = 'nb_visits_returning'; - } - - unset($columns[$actionsPerVisitReturningRequested]); - } - - if (false !== ($averageVisitDurationReturningRequested = array_search('avg_time_on_site_returning', $columns))) { - if (!in_array('sum_visit_length_returning', $columns)) { - $tempColumns[] = 'sum_visit_length_returning'; - } - - if (!in_array('nb_visits_returning', $columns)) { - $tempColumns[] = 'nb_visits_returning'; - } - - unset($columns[$averageVisitDurationReturningRequested]); - } + $segment = $this->appendReturningVisitorSegment($segment); + + $this->unprefixColumns($columns); + $params = array( + 'idSite' => $idSite, + 'period' => $period, + 'date' => $date, + 'segment' => $segment, + 'columns' => implode(',', $columns), + 'format' => 'original', + 'serialize' => 0 // tests set this to 1 + ); + $table = Request::processRequest('VisitsSummary.get', $params); + $this->prefixColumns($table, $period); + return $table; + } - $tempColumns = array_unique($tempColumns); - $columns = array_merge($columns, $tempColumns); + protected function appendReturningVisitorSegment($segment) + { + if (empty($segment)) { + $segment = ''; } else { - $bounceRateReturningRequested = $averageVisitDurationReturningRequested = $actionsPerVisitReturningRequested = true; - $columns = array( - 'nb_visits_returning', - 'nb_actions_returning', - 'nb_visits_converted_returning', - 'bounce_count_returning', - 'sum_visit_length_returning', - 'max_actions_returning', - ); - - if (SettingsPiwik::isUniqueVisitorsEnabled($period)) { - $columns = array_merge(array('nb_uniq_visitors_returning'), $columns); - } + $segment .= urlencode(SegmentExpression::AND_DELIMITER); } - $dataTable = $archive->getDataTableFromNumeric($columns); + $segment .= self::RETURNING_VISITOR_SEGMENT; + return $segment; + } - // Process ratio metrics - if ($bounceRateReturningRequested !== false) { - $dataTable->filter('ColumnCallbackAddColumnPercentage', array('bounce_rate_returning', 'bounce_count_returning', 'nb_visits_returning', 0)); - } - if ($actionsPerVisitReturningRequested !== false) { - $dataTable->filter('ColumnCallbackAddColumnQuotient', array('nb_actions_per_visit_returning', 'nb_actions_returning', 'nb_visits_returning', 1)); - } - if ($averageVisitDurationReturningRequested !== false) { - $dataTable->filter('ColumnCallbackAddColumnQuotient', array('avg_time_on_site_returning', 'sum_visit_length_returning', 'nb_visits_returning', 0)); + protected function unprefixColumns(&$columns) + { + $columns = Piwik::getArrayFromApiParameter($columns); + foreach ($columns as &$column) { + $column = str_replace(self::COLUMN_SUFFIX, "", $column); } + } - // remove temporary metrics that were used to compute processed metrics - $dataTable->deleteColumns($tempColumns); - - return $dataTable; + protected function prefixColumns($table, $period) + { + $rename = array(); + foreach (APIVisitsSummary::getInstance()->getColumns($period) as $oldColumn) { + $rename[$oldColumn] = $oldColumn . self::COLUMN_SUFFIX; + } + $table->filter('ReplaceColumnNames', array($rename)); } } \ No newline at end of file diff --git a/plugins/VisitFrequency/Archiver.php b/plugins/VisitFrequency/Archiver.php deleted file mode 100644 index b6f9913346d..00000000000 --- a/plugins/VisitFrequency/Archiver.php +++ /dev/null @@ -1,131 +0,0 @@ -callVisitsSummaryApiAndArchive(); - } - - public function aggregateMultipleReports() - { - if (SettingsPiwik::isUniqueVisitorsEnabled($this->getProcessor()->getParams()->getPeriod()->getLabel())) { - $this->computeUniqueVisitsForNonDay(); - } - - $this->getProcessor()->aggregateNumericMetrics(self::$visitFrequencyPeriodMetrics); - } - - private function callVisitsSummaryApiAndArchive($columns = false) - { - $archiveParams = $this->getProcessor()->getParams(); - $periodLabel = $archiveParams->getPeriod()->getLabel(); - - $params = array( - 'idSite' => $archiveParams->getSite()->getId(), - 'period' => $periodLabel, - 'date' => $archiveParams->getPeriod()->getDateStart()->toString(), - 'segment' => $this->appendReturningVisitorSegment($archiveParams->getSegment()->getString()), - 'format' => 'original', - 'serialize' => 0 // make sure we don't serialize (in case serialize is in the query parameters) - ); - if ($columns) { - $params['columns'] = implode(",", $columns); - } - - $table = Request::processRequest('VisitsSummary.get', $params); - $this->suffixColumns($table, $periodLabel); - - if ($table->getRowsCount() > 0) { - $this->getProcessor()->insertNumericRecords($table->getFirstRow()->getColumns()); - } - } - - private function computeUniqueVisitsForNonDay() - { - // NOTE: we cannot call the VisitsSummary API from within period archiving for some reason. it results in - // a very hard to trace bug that breaks the OmniFixture severely (causes lots of data to not be shown). - // no idea why it causes such an error. - $oldParams = $this->getProcessor()->getParams(); - $site = $oldParams->getSite(); - $newSegment = $this->appendReturningVisitorSegment($oldParams->getSegment()->getString()); - $newParams = new ArchiveProcessorParams($site, $oldParams->getPeriod(), new Segment($newSegment, array($site->getId()))); - - $logAggregator = new LogAggregator($newParams); - $query = $logAggregator->queryVisitsByDimension(array(), false, array(), array(Metrics::INDEX_NB_UNIQ_VISITORS)); - $data = $query->fetch(); - $nbUniqVisitors = (float)$data[Metrics::INDEX_NB_UNIQ_VISITORS]; - - $this->getProcessor()->insertNumericRecord('nb_uniq_visitors_returning', $nbUniqVisitors); - } - - protected function appendReturningVisitorSegment($segment) - { - if (empty($segment)) { - $segment = ''; - } else { - $segment .= urlencode(SegmentExpression::AND_DELIMITER); - } - $segment .= self::RETURNING_VISITOR_SEGMENT; - return $segment; - } - - protected function unsuffixColumns(&$columns) - { - $columns = Piwik::getArrayFromApiParameter($columns); - foreach ($columns as &$column) { - $column = str_replace(self::COLUMN_SUFFIX, "", $column); - } - } - - protected function suffixColumns($table, $period) - { - $rename = array(); - foreach (APIVisitsSummary::getInstance()->getColumns($period) as $oldColumn) { - $rename[$oldColumn] = $oldColumn . self::COLUMN_SUFFIX; - } - $table->filter('ReplaceColumnNames', array($rename)); - } -} \ No newline at end of file From 644b63fd6035fd8d0b113e8ce71629af0378e738 Mon Sep 17 00:00:00 2001 From: mattab Date: Thu, 3 Apr 2014 22:38:21 +1300 Subject: [PATCH 29/29] Fix notice --- core/CronArchive.php | 4 ++-- plugins/CoreConsole/Commands/CoreArchiver.php | 4 ++-- plugins/CustomAlerts | 2 +- tests/PHPUnit/UI | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/core/CronArchive.php b/core/CronArchive.php index 015298f37ca..f77ae239832 100644 --- a/core/CronArchive.php +++ b/core/CronArchive.php @@ -540,7 +540,7 @@ private function archiveVisitsAndSegments($idsite, $period, $lastTimestampWebsit $url .= self::APPEND_TO_API_REQUEST; - $visitsInLastPeriods = 0; + $visitsInLastPeriods = $visitsLastPeriod = 0; $success = true; $urls = array(); @@ -1206,7 +1206,7 @@ public function __construct($message, $fullOutput = null) $this->fullOutput = $fullOutput; } - public function logAndExit(CronArchive $cronArchiver) + public function logAndExit(CronArchive$cronArchiver) { if ($cronArchiver->isCoreInited()) { $cronArchiver->logError($this->getMessage()); diff --git a/plugins/CoreConsole/Commands/CoreArchiver.php b/plugins/CoreConsole/Commands/CoreArchiver.php index eae8d80f514..0fe60d3bc96 100644 --- a/plugins/CoreConsole/Commands/CoreArchiver.php +++ b/plugins/CoreConsole/Commands/CoreArchiver.php @@ -46,8 +46,8 @@ static public function configureArchiveCommand(ConsoleCommand $command) $command->addOption('force-all-periods', null, InputOption::VALUE_OPTIONAL, "Limits archiving to websites with some traffic in the last [seconds] seconds. \nFor example --force-all-periods=86400 will archive websites that had visits in the last 24 hours. \nIf [seconds] is not specified, all websites with visits in the last " . CronArchive::ARCHIVE_SITES_WITH_TRAFFIC_SINCE . "\n seconds (" . round(CronArchive::ARCHIVE_SITES_WITH_TRAFFIC_SINCE / 86400) . " days) will be archived."); $command->addOption('force-timeout-for-periods', null, InputOption::VALUE_OPTIONAL, "The current week/ current month/ current year will be processed at most every [seconds].\nIf not specified, defaults to " . CronArchive::SECONDS_DELAY_BETWEEN_PERIOD_ARCHIVES . "."); $command->addOption('force-date-last-n', null, InputOption::VALUE_REQUIRED, "This script calls the API with period=lastN. You can force the N in lastN by specifying this value."); - $command->addOption("force-idsites", null, InputOption::VALUE_OPTIONAL, 'If specified, archiving will be processed only for these Sites Ids (comma separated)'); - $command->addOption("skip-idsites", null, InputOption::VALUE_OPTIONAL, 'If specified, archiving will be skipped for these websites (in case these website ids would have been archived).'); + $command->addOption('force-idsites', null, InputOption::VALUE_OPTIONAL, 'If specified, archiving will be processed only for these Sites Ids (comma separated)'); + $command->addOption('skip-idsites', null, InputOption::VALUE_OPTIONAL, 'If specified, archiving will be skipped for these websites (in case these website ids would have been archived).'); $command->addOption('disable-scheduled-tasks', null, InputOption::VALUE_NONE, "Skips executing Scheduled tasks (sending scheduled reports, db optimization, etc.)."); $command->addOption('xhprof', null, InputOption::VALUE_NONE, "Enables XHProf profiler for this archive.php run. Requires XHPRof (see tests/README.xhprof.md)."); $command->addOption('accept-invalid-ssl-certificate', null, InputOption::VALUE_NONE, "It is _NOT_ recommended to use this argument. Instead, you should use a valid SSL certificate!\nIt can be useful if you specified --url=https://... or if you are using Piwik with force_ssl=1"); diff --git a/plugins/CustomAlerts b/plugins/CustomAlerts index c9edd879003..a1125e96827 160000 --- a/plugins/CustomAlerts +++ b/plugins/CustomAlerts @@ -1 +1 @@ -Subproject commit c9edd879003ca7a9f75d81fe07ec6ff0c2340f91 +Subproject commit a1125e96827e12c7bd5c3d6107b9c162aace32f8 diff --git a/tests/PHPUnit/UI b/tests/PHPUnit/UI index 6e2b0993fda..a878862448d 160000 --- a/tests/PHPUnit/UI +++ b/tests/PHPUnit/UI @@ -1 +1 @@ -Subproject commit 6e2b0993fda17b6c4c2ac407c69fc09a1867dcd9 +Subproject commit a878862448d37eafb1c3896cc97acc553aca1f75