Skip to content
This repository was archived by the owner on Jan 17, 2023. It is now read-only.

Comments

Audit events sent to GA before public launch#690

Merged
ckprice merged 1 commit intomozilla:masterfrom
meandavejustice:615-ga-coverage
May 4, 2016
Merged

Audit events sent to GA before public launch#690
ckprice merged 1 commit intomozilla:masterfrom
meandavejustice:615-ga-coverage

Conversation

@meandavejustice
Copy link
Contributor

@meandavejustice meandavejustice commented May 2, 2016

#615 is blocked by #681

  • refs Make sure all the things are GA'd #615
  • added "Active Experiments Running" dimension to ga in landing-page.js
  • added "Number of Active Experiments Running" dimension to ga in landing-page.js
  • remove extra "logout" event handler in header-view.js since we already handle in settings-menu-view.js
  • added "wiki" event to ga in settings-menu.js
  • added "file issue" event to ga in settings-menu.js
  • added "tour" events to ga in experiment-tour-dialog-view.js, 'start', 'cancel', 'next', and 'back'
  • added "cancel tour" event to ga in experiment-tour-dialog-view.js
  • disable eslint on error logging in retire-page.js
  • update test/views/landing-page.js so work with app.me.installed.length check

},
body: ''
}).then(() => {
app.sendToGA('event', {
Copy link

Choose a reason for hiding this comment

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

I'm addressing this in my PR as well. Race?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We'll use yours, I'll remove this.

@ckprice
Copy link

ckprice commented May 3, 2016

Thanks Dave! A couple of small comments. Also wondering if we want to add events to the exiting survey links here and here in this PR? If not no worries we can merge this and can open a new issue for those.

/edit actually I can handle the call in views/retire-page.js in my PR.

@meandavejustice
Copy link
Contributor Author

@ckprice I've addressed your comments, though github didn't collapse them all


app.sendToGA('pageview', {
'dimension1': this.loggedIn
'dimension1': this.loggedIn,
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we get this stuff documented in README-METRICS.md - if not as part of this PR, then as part of a follow up in the very near future? Not sure what this dimension stuff is about, so I couldn't really speak to it in a review.

Copy link

Choose a reason for hiding this comment

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

Filed #696

eventAction: 'button click',
eventLabel: 'exit survey disabled',
newTab: true,
outboundURL: this.ev.target.getAttribute('href')
Copy link

Choose a reason for hiding this comment

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

this.ev is undefined

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh whoops, fixing now

- refs #615
- added "Active Experiments Running" dimension to ga in `landing-page.js`
- added "Number of Active Experiments Running" dimension to ga in `landing-page.js`
- remove extra "logout" event handler in header-view.js since we already handle in settings-menu-view.js
- added "wiki" event to ga in `settings-menu.js`
- added "file issue" event to ga in `settings-menu.js`
- added "tour" events to ga in `experiment-tour-dialog-view.js`, 'start', 'cancel', 'next', and 'back'
- added "cancel tour" event to ga in `experiment-tour-dialog-view.js`
- disable eslint on error logging in `retire-page.js`
- update `test/views/landing-page.js` so work with `app.me.installed.length` check
@ckprice
Copy link

ckprice commented May 4, 2016

r+!

@ckprice ckprice merged commit c4bd5ea into mozilla:master May 4, 2016
@meandavejustice meandavejustice deleted the 615-ga-coverage branch May 4, 2016 18:58
@ckprice ckprice mentioned this pull request May 4, 2016
4 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants