New issue

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

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

Already on GitHub? Sign in to your account

New "Event URL" segment `eventUrl` to segment on any Segment URL #12236

Merged
merged 10 commits into from Dec 1, 2017

Conversation

Projects
None yet
3 participants
@mattab
Member

mattab commented Oct 31, 2017

Useful for many use cases for example:

  • Creating Custom Reports such as "Top page URLs by Event action"
  • Segmenting by Event URL and view events triggered on a specific Page URL
  • Fixes #11131 Action URL segment could filter both Page URLs OR Event URLs (as advertised originally in the 2.16.0 changelog but it wasn't actually fully working yet)

TODO:

  • Check & fix build

@mattab mattab added the Enhancement label Oct 31, 2017

@mattab mattab added this to the 3.2.1 milestone Oct 31, 2017

@tsteur

This comment has been minimized.

Show comment
Hide comment
@tsteur

tsteur Nov 16, 2017

Member

I think tests need to be fixed but otherwise 👍

Member

tsteur commented Nov 16, 2017

I think tests need to be fixed but otherwise 👍

@sgiehl

This comment has been minimized.

Show comment
Hide comment
@sgiehl

sgiehl Nov 22, 2017

Member

Last test failing seems to be AutoSuggestAPITest. Could figure out how to fix or why it doesn't suggest any event urls as some events should have been tracked with urls.
@tsteur @mattab any idea on that, otherwise I need to investigate that a bit deeper as soon as I have some more time

Member

sgiehl commented Nov 22, 2017

Last test failing seems to be AutoSuggestAPITest. Could figure out how to fix or why it doesn't suggest any event urls as some events should have been tracked with urls.
@tsteur @mattab any idea on that, otherwise I need to investigate that a bit deeper as soon as I have some more time

@mattab

This comment has been minimized.

Show comment
Hide comment
@mattab

mattab Nov 29, 2017

Member

I noticed the same issue. @sgiehl could you investigate more and see if you can generate test data and how? would be great to merge this one asap

Member

mattab commented Nov 29, 2017

I noticed the same issue. @sgiehl could you investigate more and see if you can generate test data and how? would be great to merge this one asap

mattab and others added some commits Oct 31, 2017

New "Event URL" segment `eventUrl` to segment on any Segment URL
Useful for many use cases for example:

* Creating Custom Reports such as "Top page URLs by Event action"
* Segmenting by Event URL and view events triggered on a specific Page URL
* Fixes #11131 Action URL segment could filter both Page URLs OR Event URLs (as advertised originally in the 2.16.0 changelog but it wasn't actually fully working yet)
@@ -221,6 +221,7 @@ public static function flattenVisitorDetailsArray($visitorDetailsArray)
$flattenForActionType = array(
'outlink' => 'outlinkUrl',
'download' => 'downloadUrl',
'event' => 'eventUrl',

This comment has been minimized.

@sgiehl

sgiehl Nov 29, 2017

Member

this was needed to get suggested values for this segment

@sgiehl

sgiehl Nov 29, 2017

Member

this was needed to get suggested values for this segment

@sgiehl

This comment has been minimized.

Show comment
Hide comment
@sgiehl

sgiehl Nov 29, 2017

Member

I've now figured out why there weren't any suggest values. But unfortunately there still seems to be a bug in the segment itself. It does not seem to work, as there aren't any matches. Weren't able to get any results in segmented visitor logs. It seems the changes in TableLogActions (https://github.com/piwik/piwik/pull/12236/files#diff-fdad89cc205c1e002ab8f49d1cd3ab76) prevents any results. Not sure yet how to fix it. Guess I need to dig a bit deeper

Member

sgiehl commented Nov 29, 2017

I've now figured out why there weren't any suggest values. But unfortunately there still seems to be a bug in the segment itself. It does not seem to work, as there aren't any matches. Weren't able to get any results in segmented visitor logs. It seems the changes in TableLogActions (https://github.com/piwik/piwik/pull/12236/files#diff-fdad89cc205c1e002ab8f49d1cd3ab76) prevents any results. Not sure yet how to fix it. Guess I need to dig a bit deeper

@mattab

besides the test case not showing data, looks good 👍

@mattab mattab modified the milestones: 3.2.1, 3.2.2 Nov 30, 2017

@sgiehl

This comment has been minimized.

Show comment
Hide comment
@sgiehl

sgiehl Nov 30, 2017

Member

@mattab my last changes should fix it. the eventurl is stored without protocol, so it needs to be removed from suggested values.

Member

sgiehl commented Nov 30, 2017

@mattab my last changes should fix it. the eventurl is stored without protocol, so it needs to be removed from suggested values.

@mattab mattab modified the milestones: 3.2.2, 3.2.1 Nov 30, 2017

@mattab

This comment has been minimized.

Show comment
Hide comment
@mattab

mattab Nov 30, 2017

Member

Looks good @sgiehl

There is only one last failing test in CustomDimensions:

1) Piwik\Plugins\CustomDimensions\tests\System\ApiTest::testApi with data set #24 (array('API.getSegmentsMetadata'), array(1, '2013-01-23 01:23:45', array('year')))

Piwik\Plugins\CustomDimensions\tests\System\ApiTest: Differences with expected in '/home/travis/build/piwik/piwik/plugins/CustomDimensions/tests/System/processed/test___API.getSegmentsMetadata.xml'

Failed asserting that two DOM documents are equal.

--- Expected

+++ Actual

@@ @@

       <row>outlinkUrl</row>

+      <row>eventUrl</row>

@@ @@

     <segment>eventName</segment>

+  </row>

+  <row>

+    <type>dimension</type>

+    <category>Events</category>

+    <name>Event URL</name>

+    <segment>eventUrl</segment>

+    <acceptedValues>The URL must be URL encoded, for example: http%3A%2F%2Fexample.com%2Fpath%2Fpage%3Fquery</acceptedValues>

   </row>

   <row>

     <type>dimension</type>

Could you maybe merge the PR afterwards once tests green?

Member

mattab commented Nov 30, 2017

Looks good @sgiehl

There is only one last failing test in CustomDimensions:

1) Piwik\Plugins\CustomDimensions\tests\System\ApiTest::testApi with data set #24 (array('API.getSegmentsMetadata'), array(1, '2013-01-23 01:23:45', array('year')))

Piwik\Plugins\CustomDimensions\tests\System\ApiTest: Differences with expected in '/home/travis/build/piwik/piwik/plugins/CustomDimensions/tests/System/processed/test___API.getSegmentsMetadata.xml'

Failed asserting that two DOM documents are equal.

--- Expected

+++ Actual

@@ @@

       <row>outlinkUrl</row>

+      <row>eventUrl</row>

@@ @@

     <segment>eventName</segment>

+  </row>

+  <row>

+    <type>dimension</type>

+    <category>Events</category>

+    <name>Event URL</name>

+    <segment>eventUrl</segment>

+    <acceptedValues>The URL must be URL encoded, for example: http%3A%2F%2Fexample.com%2Fpath%2Fpage%3Fquery</acceptedValues>

   </row>

   <row>

     <type>dimension</type>

Could you maybe merge the PR afterwards once tests green?

@sgiehl

This comment has been minimized.

Show comment
Hide comment
@sgiehl

sgiehl Nov 30, 2017

Member
Member

sgiehl commented Nov 30, 2017

@mattab

This comment has been minimized.

Show comment
Hide comment
@mattab

mattab Dec 1, 2017

Member

Ok will merge now 👍 Thanks for the work on this!

Member

mattab commented Dec 1, 2017

Ok will merge now 👍 Thanks for the work on this!

@mattab mattab merged commit 40a8d65 into 3.x-dev Dec 1, 2017

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details

@mattab mattab deleted the eventUrl branch Dec 1, 2017

@matomo-org matomo-org deleted a comment from MatomoForumBot Dec 4, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment