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

Reader: Add stats tracking with Tracks #1244

Merged
merged 3 commits into from
Dec 3, 2015
Merged

Conversation

blowery
Copy link
Contributor

@blowery blowery commented Dec 3, 2015

Adds in Tracks tracks, matching what we do in the native apps.

Fixes #1216

cc @robfelty @meremagee

@blowery blowery added [Status] In Progress [Feature] Reader The reader site on Calypso. labels Dec 3, 2015
@blowery blowery self-assigned this Dec 3, 2015
@blowery blowery added this to the Reader: December Bug Scrub milestone Dec 3, 2015
@blowery blowery force-pushed the add/reader-tracks-stats branch from eae314b to 046ea36 Compare December 3, 2015 19:22
@blowery blowery added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Type] Task and removed [Status] In Progress labels Dec 3, 2015
@bluefuton
Copy link
Contributor

Confirming the following events are being fired correctly:

  • calypso_reader_article_commented_on
  • calypso_reader_infinite_scroll_performed
  • calypso_reader_blog_preview
  • calypso_reader_tag_loaded
  • calypso_reader_list_loaded
  • calypso_reader_discover_viewed
  • calypso_reader_article_opened
  • calypso_reader_article_liked / calypso_reader_article_unliked
  • calypso_reader_reader_list_followed / calypso_reader_reader_list_unfollowed
  • calypso_reader_reader_tag_followed / calypso_reader_reader_tag_unfollowed

@robfelty
Copy link
Contributor

robfelty commented Dec 3, 2015

Great progress!

I looked at the first one and it is not clear to me that we are recording which article a user commented on, which is something I think we should include.

Same comment applies to calypso_reader_article_opened (and probably the others too) - We should probably include the blog_id and post_id of the article.

Also, how is the siteidlabel being set? For stuff like posts, we currently have either 'wpcom' or 'jetpack'. For commenting on an article from the reader, should the siteidlabel always be 'wpcom'? Or should it correspond to the status of the article being commented on (an article hosted on wpcom, a jetpack site, or maybe even a non-jetpack site). I'm not even sure that is possible, but it could be an interesting thing to look at overall reader usage.

@blowery
Copy link
Contributor Author

blowery commented Dec 3, 2015

Also, how is the siteidlabel being set?

No idea. I think it's done inside the guts of the tracks client. We're not setting it explicitly. We just pass an event name and a set of props, which appear in event props.

@blowery
Copy link
Contributor Author

blowery commented Dec 3, 2015

I looked at the first one and it is not clear to me that we are recording which article a user commented on, which is something I think we should include.

I think it could be useful too, but the native apps are not currently passing it. Should we just pass it as blog_ID (or blogID or blogId or ... )?

@blowery
Copy link
Contributor Author

blowery commented Dec 3, 2015

I'm going to land this for now so we have at least parity. Additional props can come in another PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Reader The reader site on Calypso. [Type] Task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants