-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Conversation
eae314b
to
046ea36
Compare
Confirming the following events are being fired correctly:
|
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. |
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. |
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 ... )? |
I'm going to land this for now so we have at least parity. Additional props can come in another PR. |
Reader: Add stats tracking with Tracks
Adds in Tracks tracks, matching what we do in the native apps.
Fixes #1216
cc @robfelty @meremagee