Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Content Tracking #6201

Merged
merged 66 commits into from Sep 13, 2014
Merged

Content Tracking #6201

merged 66 commits into from Sep 13, 2014

Conversation

tsteur
Copy link
Member

@tsteur tsteur commented Sep 12, 2014

refs #4996

tsteur and others added 30 commits August 21, 2014 13:18
…ssions should work (contentName => contentPiece and contentPiece => contentName). Next step: Matching interactions with impressions then this part is nearly done hopefully
…ally mandatory created a new column content_name for now (instead of existing idaction_name, will maybe remove it again). Otherwise it is hard to know whether it was a content action or not and the actual problem was that maybe other actions already define an idaction_name which might be different to content_name. We need the content_name along other actions to match an interaction with an impression. Next: The client side and the redirect...
…Sending the impressions to Piwik via bulk tracking
…. Makes it simpler and now the correct label is displayed in a subtable. I should not copy/paste code that I do not understand :)
…offer some APIs to manually track impressions and interactions (we might remove them later again, to be discussed)
… or not. This is quite a complex task and I do not think it can always work 100%. There are too many factors and ways to hide an element. For instance using a zIndex, zoom level, when a div is scrollable, CSS might not be applied yet at the time we scan etc. So far this should cover most cases but it must be clear it cannot work in all and there still needs to be a lot of testing. Also need to detect whether CSS was applied or not
…ly and a user does not have to add them manually to the viewDataTable
…iting this I noticed a few things that need to be changed and that do not work, also many possible new features for V2. Already updated a few things but will need to adjust more things before writing tests
tsteur and others added 21 commits September 9, 2014 15:37
…ty to track content via php tracker (not tested yet)
…e cross browser and php tests. A few more minor and easy to implement features as well
… except for cross browser support. I fixed many issues as I did not consider that link tracking may be disabled or enabled and the behavior should be different in such a case.
Conflicts:
	core/Metrics.php
	js/piwik.js
	piwik.js
	tests/javascript/index.php
…ost likely again as you would have to register subdomains etc as well). Also added some missing test files
…Safari + Opera and on my local phantomjs but not on travis phantomjs
…d do not allow known hosts automatically both to prevent issues on shared Piwik instances
tsteur added a commit that referenced this pull request Sep 13, 2014
@tsteur tsteur merged commit ddf202a into master Sep 13, 2014
@tsteur tsteur deleted the 4996_content_tracking branch September 13, 2014 13:35
@@ -134,7 +134,7 @@ public function getAnotherApiForTesting()
public function testCheckOtherTestsWereComplete()
{
// Check that only a few haven't been tested specifically (these are all custom variables slots since we only test slot 1, 2, 5 (see the fixture) and example dimension slots)
$maximumSegmentsToSkip = 14;
Copy link
Member

Choose a reason for hiding this comment

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

It would be cool to set test data for auto suggest for the new segment. It's a bit annoying it's true as it requires adding the new setters in ManyVisitsWithGeoIP fixture which is used by few other tests.. This test case AutoSuggest should have its own fixture for sure (my bad!)

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not finished yet. Only made the tests green to be able to merge as the feature itself should work

@mattab
Copy link
Member

mattab commented Sep 14, 2014

Great work. this will be an epic feature for sure! Looking forward to using it on piwik.org...

$model = new Model();

foreach ($siteIds as $siteId) {
$siteUrls = $model->getSiteUrlsFromId($siteId);
Copy link
Member

Choose a reason for hiding this comment

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

the data should be cached in the tracker cache and this not use Model directly, this will increase performance and prevent 1 added SQL on each content interaction tracking request

Copy link
Member Author

Choose a reason for hiding this comment

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

See todo list in documentation. I'm not finished yet!

mnapoli added a commit that referenced this pull request Oct 8, 2014
Furthermore, the link to the FAQ about this is now hidden when `displayLink` is `1`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants