search

tomasbasham / ember-cli-analytics

An ember-cli addon to interface with analytics services and external integrations
MIT License
8 stars 9 forks source link

GA Integration: Support Event Dimensions and Metrics #1

Closed rjhilgefort closed 8 years ago

rjhilgefort commented 8 years ago

Google Analytics supports tracking custom dimensions and metrics through page views as well as events. The integration currently allows this through trackPage because there's not minipulation of the keys passed in. trackEvent modifies keys by prepending event (per spec)- it does not, however, look for the presence of the special dimension and metric keys. This PR adds support by not modifying those keys when found.

Here's a link to the GA spec on how they should be passed: https://developers.google.com/analytics/devguides/collection/analyticsjs/custom-dims-mets

rjhilgefort commented 8 years ago

The test appears to be failing on npm install and is unrelated to the changes I'm making in this PR.

tomasbasham commented 8 years ago

I guess there is an issue with travis. I have come across this before and it just sorted itself out eventually. I'll leave it another day before re-trying the test. However running tests locally shows an error:

not ok 5 PhantomJS 2.1 - JSHint - modules/ember-cli-analytics/integrations/google-analytics.js: should pass jshint
    ---
        actual: >
            false
        expected: >
            true
        message: >
            modules/ember-cli-analytics/integrations/google-analytics.js should pass jshint.
            modules/ember-cli-analytics/integrations/google-analytics.js: line 66, col 25, Expected '{' and instead saw 'key'.

            1 error
        Log: |
    ...
rjhilgefort commented 8 years ago

Updated based on feedback. All tests are passing locally. Fingers crossed Travis agrees.

tomasbasham commented 8 years ago

Great, man. Travis still being a nuisance. I'll jump on their support tomorrow if it is still failing. But I can see it all works 😀

tomasbasham commented 8 years ago

There we go. Travis has stopped throwing it's toys out of the pram. I'll merge this in ASAP

rjhilgefort commented 8 years ago

👍 and thanks for the repo!

rjhilgefort commented 8 years ago

@tomasbasham Don't plan on merging?

tomasbasham commented 7 years ago

I have merged it

rjhilgefort commented 7 years ago

👍