diff options
author | mattab <matthieu.aubry@gmail.com> | 2015-05-25 09:39:24 +0300 |
---|---|---|
committer | mattab <matthieu.aubry@gmail.com> | 2015-05-25 09:39:24 +0300 |
commit | 5e3f188fb5067871ac5285853d8286b94eae1979 (patch) | |
tree | d6dcb74ad869edc6196f0d0d740b17d7a7a46b86 | |
parent | fe3e5ec336669fc8cf95f750c46f4570056179fe (diff) |
For Outlinks and Download URLs, do not exclude URL parameters and store URLs untouched
Also adds a missing URL Encoding for action URLs.
(when outlink or download urls contained `&` and such characters breaking the URL this possibly corrupted the tracking request).
Fixes #6244
14 files changed, 74 insertions, 40 deletions
diff --git a/core/Tracker/Action.php b/core/Tracker/Action.php index 6775a0a65d..ca11a5d526 100644 --- a/core/Tracker/Action.php +++ b/core/Tracker/Action.php @@ -206,6 +206,12 @@ abstract class Action } } + protected function setActionUrlWithoutExcludingParameters($url) + { + $this->rawActionUrl = PageUrl::getUrlIfLookValid($url); + $this->actionUrl = PageUrl::getUrlIfLookValid($url); + } + abstract protected function getActionsToLookup(); protected function getUrlAndType() diff --git a/plugins/Actions/Actions/ActionClickUrl.php b/plugins/Actions/Actions/ActionClickUrl.php index 030c80f3c0..df93d154a1 100644 --- a/plugins/Actions/Actions/ActionClickUrl.php +++ b/plugins/Actions/Actions/ActionClickUrl.php @@ -24,7 +24,7 @@ class ActionClickUrl extends Action public function __construct(Request $request) { parent::__construct(self::TYPE_OUTLINK, $request); - $this->setActionUrl($request->getParam('link')); + $this->setActionUrlWithoutExcludingParameters($request->getParam('link')); } public static function shouldHandle(Request $request) diff --git a/plugins/Actions/Actions/ActionDownloadUrl.php b/plugins/Actions/Actions/ActionDownloadUrl.php index 5d01c07aca..be24519bfb 100644 --- a/plugins/Actions/Actions/ActionDownloadUrl.php +++ b/plugins/Actions/Actions/ActionDownloadUrl.php @@ -20,7 +20,7 @@ class ActionDownloadUrl extends Action public function __construct(Request $request) { parent::__construct(self::TYPE_DOWNLOAD, $request); - $this->setActionUrl($request->getParam('download')); + $this->setActionUrlWithoutExcludingParameters($request->getParam('download')); } public static function shouldHandle(Request $request) diff --git a/plugins/Actions/ArchivingHelper.php b/plugins/Actions/ArchivingHelper.php index 4854e2b017..b5735225b8 100644 --- a/plugins/Actions/ArchivingHelper.php +++ b/plugins/Actions/ArchivingHelper.php @@ -603,7 +603,12 @@ class ArchivingHelper $urlFragment = $matches[3]; if (in_array($type, array(Action::TYPE_DOWNLOAD, Action::TYPE_OUTLINK))) { - return array(trim($urlHost), '/' . trim($urlPath)); + $path = '/' . trim($urlPath); + if (!empty($urlFragment)) { + $path .= '#' . $urlFragment; + } + + return array(trim($urlHost), $path); } $name = $urlPath; diff --git a/tests/PHPUnit/Fixtures/OneVisitorTwoVisits.php b/tests/PHPUnit/Fixtures/OneVisitorTwoVisits.php index e7af179e10..822f6eb9ff 100644 --- a/tests/PHPUnit/Fixtures/OneVisitorTwoVisits.php +++ b/tests/PHPUnit/Fixtures/OneVisitorTwoVisits.php @@ -106,7 +106,9 @@ class OneVisitorTwoVisits extends Fixture // Click on external link after 6 minutes (3rd action) $t->setForceVisitDateTime(Date::factory($dateTime)->addHour(0.1)->getDatetime()); - self::checkResponse($t->doTrackAction('http://dev.piwik.org/svn', 'link')); + + // Testing Outlink that contains a URL Fragment + self::checkResponse($t->doTrackAction('https://outlinks.org/#!outlink-with-fragment-<script>', 'link')); // Click on file download after 12 minutes (4th action) $t->setForceVisitDateTime(Date::factory($dateTime)->addHour(0.2)->getDatetime()); @@ -114,7 +116,7 @@ class OneVisitorTwoVisits extends Fixture // Click on two more external links, one the same as before (5th & 6th actions) $t->setForceVisitDateTime(Date::factory($dateTime)->addHour(0.22)->getDateTime()); - self::checkResponse($t->doTrackAction('http://outlinks.org/other_outlink', 'link')); + self::checkResponse($t->doTrackAction('http://outlinks.org/other_outlink#fragment&pk_campaign=Open%20partnership', 'link')); $t->setForceVisitDateTime(Date::factory($dateTime)->addHour(0.25)->getDateTime()); self::checkResponse($t->doTrackAction('http://dev.piwik.org/svn', 'link')); diff --git a/tests/PHPUnit/System/BackwardsCompatibility1XTest.php b/tests/PHPUnit/System/BackwardsCompatibility1XTest.php index 561aa12708..74dbb05d73 100644 --- a/tests/PHPUnit/System/BackwardsCompatibility1XTest.php +++ b/tests/PHPUnit/System/BackwardsCompatibility1XTest.php @@ -124,6 +124,11 @@ class BackwardsCompatibility1XTest extends SystemTestCase // the Action.getPageTitles test fails for unknown reason, so skipping it // eg. https://travis-ci.org/piwik/piwik/jobs/24449365 'Action.getPageTitles', + + // Outlinks now tracked with URL Fragment which was not the case in 1.X + 'Actions.get', + 'Actions.getOutlink', + 'Actions.getOutlinks', ); $apiNotToCall = array_merge($apiNotToCall, $reportsToCompareSeparately); diff --git a/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits__Actions.getOutlink_day.xml b/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits__Actions.getOutlink_day.xml index 5971863326..1565685cb1 100644 --- a/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits__Actions.getOutlink_day.xml +++ b/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits__Actions.getOutlink_day.xml @@ -4,8 +4,8 @@ <label>/svn</label> <nb_visits>1</nb_visits> <nb_uniq_visitors>1</nb_uniq_visitors> - <nb_hits>2</nb_hits> - <sum_time_spent>540</sum_time_spent> + <nb_hits>1</nb_hits> + <sum_time_spent>180</sum_time_spent> <url>http://dev.piwik.org/svn</url> </row> </result>
\ No newline at end of file diff --git a/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits__Actions.getOutlinks_day.xml b/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits__Actions.getOutlinks_day.xml index 539d798e9b..c6795a704e 100644 --- a/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits__Actions.getOutlinks_day.xml +++ b/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits__Actions.getOutlinks_day.xml @@ -1,34 +1,42 @@ <?xml version="1.0" encoding="utf-8" ?> <result> <row> - <label>dev.piwik.org</label> - <nb_visits>1</nb_visits> + <label>outlinks.org</label> + <nb_visits>2</nb_visits> <nb_hits>2</nb_hits> - <sum_time_spent>540</sum_time_spent> + <sum_time_spent>468</sum_time_spent> <subtable> <row> - <label>/svn</label> + <label>/#!outlink-with-fragment-<script></label> <nb_visits>1</nb_visits> <nb_uniq_visitors>1</nb_uniq_visitors> - <nb_hits>2</nb_hits> - <sum_time_spent>540</sum_time_spent> - <url>http://dev.piwik.org/svn</url> + <nb_hits>1</nb_hits> + <sum_time_spent>360</sum_time_spent> + <url>https://outlinks.org/#!outlink-with-fragment-<script></url> + </row> + <row> + <label>/other_outlink#fragment&pk_campaign=Open partnership</label> + <nb_visits>1</nb_visits> + <nb_uniq_visitors>1</nb_uniq_visitors> + <nb_hits>1</nb_hits> + <sum_time_spent>108</sum_time_spent> + <url>http://outlinks.org/other_outlink#fragment&pk_campaign=Open%20partnership</url> </row> </subtable> </row> <row> - <label>outlinks.org</label> + <label>dev.piwik.org</label> <nb_visits>1</nb_visits> <nb_hits>1</nb_hits> - <sum_time_spent>108</sum_time_spent> + <sum_time_spent>180</sum_time_spent> <subtable> <row> - <label>/other_outlink</label> + <label>/svn</label> <nb_visits>1</nb_visits> <nb_uniq_visitors>1</nb_uniq_visitors> <nb_hits>1</nb_hits> - <sum_time_spent>108</sum_time_spent> - <url>http://outlinks.org/other_outlink</url> + <sum_time_spent>180</sum_time_spent> + <url>http://dev.piwik.org/svn</url> </row> </subtable> </row> diff --git a/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits__Actions.get_day.xml b/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits__Actions.get_day.xml index e04ea4296d..33dded56df 100644 --- a/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits__Actions.get_day.xml +++ b/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits__Actions.get_day.xml @@ -5,7 +5,7 @@ <nb_downloads>1</nb_downloads> <nb_uniq_downloads>1</nb_uniq_downloads> <nb_outlinks>3</nb_outlinks> - <nb_uniq_outlinks>2</nb_uniq_outlinks> + <nb_uniq_outlinks>3</nb_uniq_outlinks> <nb_searches>0</nb_searches> <nb_keywords>0</nb_keywords> <avg_time_generation>0.155</avg_time_generation> diff --git a/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits_csv__API.get_month.csv b/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits_csv__API.get_month.csv Binary files differindex 02f34e8e00..1b6a6591aa 100644 --- a/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits_csv__API.get_month.csv +++ b/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits_csv__API.get_month.csv diff --git a/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits_withCookieSupport__Actions.getOutlink_day.xml b/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits_withCookieSupport__Actions.getOutlink_day.xml index 0f328b414d..cc4024a67f 100644 --- a/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits_withCookieSupport__Actions.getOutlink_day.xml +++ b/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits_withCookieSupport__Actions.getOutlink_day.xml @@ -4,8 +4,8 @@ <label>/svn</label> <nb_visits>1</nb_visits> <nb_uniq_visitors>1</nb_uniq_visitors> - <nb_hits>2</nb_hits> - <sum_time_spent>432</sum_time_spent> + <nb_hits>1</nb_hits> + <sum_time_spent>72</sum_time_spent> <url>http://dev.piwik.org/svn</url> </row> </result>
\ No newline at end of file diff --git a/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits_withCookieSupport__Actions.getOutlinks_day.xml b/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits_withCookieSupport__Actions.getOutlinks_day.xml index 1767641648..01dadf7a92 100644 --- a/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits_withCookieSupport__Actions.getOutlinks_day.xml +++ b/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits_withCookieSupport__Actions.getOutlinks_day.xml @@ -1,34 +1,42 @@ <?xml version="1.0" encoding="utf-8" ?> <result> <row> - <label>dev.piwik.org</label> - <nb_visits>1</nb_visits> + <label>outlinks.org</label> + <nb_visits>2</nb_visits> <nb_hits>2</nb_hits> - <sum_time_spent>432</sum_time_spent> + <sum_time_spent>468</sum_time_spent> <subtable> <row> - <label>/svn</label> + <label>/#!outlink-with-fragment-<script></label> <nb_visits>1</nb_visits> <nb_uniq_visitors>1</nb_uniq_visitors> - <nb_hits>2</nb_hits> - <sum_time_spent>432</sum_time_spent> - <url>http://dev.piwik.org/svn</url> + <nb_hits>1</nb_hits> + <sum_time_spent>360</sum_time_spent> + <url>https://outlinks.org/#!outlink-with-fragment-<script></url> + </row> + <row> + <label>/other_outlink#fragment&pk_campaign=Open partnership</label> + <nb_visits>1</nb_visits> + <nb_uniq_visitors>1</nb_uniq_visitors> + <nb_hits>1</nb_hits> + <sum_time_spent>108</sum_time_spent> + <url>http://outlinks.org/other_outlink#fragment&pk_campaign=Open%20partnership</url> </row> </subtable> </row> <row> - <label>outlinks.org</label> + <label>dev.piwik.org</label> <nb_visits>1</nb_visits> <nb_hits>1</nb_hits> - <sum_time_spent>108</sum_time_spent> + <sum_time_spent>72</sum_time_spent> <subtable> <row> - <label>/other_outlink</label> + <label>/svn</label> <nb_visits>1</nb_visits> <nb_uniq_visitors>1</nb_uniq_visitors> <nb_hits>1</nb_hits> - <sum_time_spent>108</sum_time_spent> - <url>http://outlinks.org/other_outlink</url> + <sum_time_spent>72</sum_time_spent> + <url>http://dev.piwik.org/svn</url> </row> </subtable> </row> diff --git a/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits_withCookieSupport__Actions.get_day.xml b/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits_withCookieSupport__Actions.get_day.xml index 0eea12ac0b..17f402fd78 100644 --- a/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits_withCookieSupport__Actions.get_day.xml +++ b/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits_withCookieSupport__Actions.get_day.xml @@ -5,7 +5,7 @@ <nb_downloads>1</nb_downloads> <nb_uniq_downloads>1</nb_uniq_downloads> <nb_outlinks>3</nb_outlinks> - <nb_uniq_outlinks>2</nb_uniq_outlinks> + <nb_uniq_outlinks>3</nb_uniq_outlinks> <nb_searches>1</nb_searches> <nb_keywords>1</nb_keywords> <avg_time_generation>0.155</avg_time_generation> diff --git a/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits_withCookieSupport__Live.getLastVisitsDetails_day.xml b/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits_withCookieSupport__Live.getLastVisitsDetails_day.xml index adec229633..2550e4e26b 100644 --- a/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits_withCookieSupport__Live.getLastVisitsDetails_day.xml +++ b/tests/PHPUnit/System/expected/test_OneVisitorTwoVisits_withCookieSupport__Live.getLastVisitsDetails_day.xml @@ -21,7 +21,7 @@ <type>action</type> <url>http://example.org/store/purchase.htm</url> <pageTitle>Checkout/Purchasing...</pageTitle> - <pageIdAction>12</pageIdAction> + <pageIdAction>13</pageIdAction> <pageId>9</pageId> <generationTime>0.13s</generationTime> @@ -147,7 +147,7 @@ </row> <row> <type>outlink</type> - <url>http://dev.piwik.org/svn</url> + <url>https://outlinks.org/#!outlink-with-fragment-<script></url> <pageTitle /> <pageIdAction>5</pageIdAction> @@ -171,7 +171,7 @@ </row> <row> <type>outlink</type> - <url>http://outlinks.org/other_outlink</url> + <url>http://outlinks.org/other_outlink#fragment&pk_campaign=Open%20partnership</url> <pageTitle /> <pageIdAction>7</pageIdAction> @@ -185,7 +185,7 @@ <type>outlink</type> <url>http://dev.piwik.org/svn</url> <pageTitle /> - <pageIdAction>5</pageIdAction> + <pageIdAction>8</pageIdAction> <pageId>6</pageId> <timeSpent>72</timeSpent> @@ -221,7 +221,7 @@ <type>action</type> <url>http://example.org/index.htm</url> <pageTitle>Looking at homepage after site search...</pageTitle> - <pageIdAction>10</pageIdAction> + <pageIdAction>11</pageIdAction> <pageId>8</pageId> <generationTime>0.02s</generationTime> |