From 63c70f439a65421a964a04a2b0749a9cebc0b827 Mon Sep 17 00:00:00 2001 From: opsb Date: Thu, 17 Sep 2015 14:33:50 +0200 Subject: [PATCH 1/3] Introduce metrics.context Any properties bound to metrics.context will be merged into options for any calls to the service API --- README.md | 24 ++++++++++++++++-------- addon/services/metrics.js | 23 +++++++++++------------ tests/unit/services/metrics-test.js | 12 +++++++++++- 3 files changed, 38 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index adda7a18..b91871e2 100644 --- a/README.md +++ b/README.md @@ -78,7 +78,7 @@ Adapter names are PascalCased. Refer to the [list of supported adapters](#curren The `metricsAdapters` option in `ENV` accepts an array of objects containing settings for each analytics service you want to use in your app in the following format: ```js -/** +/** * @param {String} name Adapter name * @param {Object} config Configuration options for the service */ @@ -148,22 +148,30 @@ metrics.trackPage('GoogleAnalytics', { There are 4 main methods implemented by the service, with the same argument signature: -- `trackPage([analyticsName], options)` - +- `trackPage([analyticsName], options)` + This is commonly used by analytics services to track page views. Due to the way Single Page Applications implement routing, you will need to call this on the `activate` hook of each route to track all page views. - `trackEvent([analyticsName], options)` - + This is a general purpose method for tracking a named event in your application. - `identify([analyticsName], options)` - + For analytics services that have identification functionality. - `alias([analyticsName], options)` - + For services that implement it, this method notifies the analytics service that an anonymous user now has a unique identifier. +##### Context + +Often you'll want to include things like `currentUser.name` with every event or page view that's tracked. Any properties that you bind to `metrics.context` will be merged into the options for every service call. + + set('metrics.context.userName', 'Jimbo'); + get('metrics').trackPage({page: 'page/1'}); + // => {userName: 'Jimbo', page: 'page/1'} + #### `link-to` API To use the augmented `link-to`, just use the same helper, but add some extra `metrics` attributes: @@ -185,7 +193,7 @@ ga('send', { }); ``` -To add an attribute, just prefix it with `metrics` and enter it in camelcase. +To add an attribute, just prefix it with `metrics` and enter it in camelcase. ### Lazy Initialization @@ -222,7 +230,7 @@ First, generate a new Metrics Adapter: $ ember generate metrics-adapter foo-bar ``` -This creates `app/metrics-adapters/foo-bar.js` and a unit test at `tests/unit/metrics-adapters/foo-bar-test.js`, which you should now customize. +This creates `app/metrics-adapters/foo-bar.js` and a unit test at `tests/unit/metrics-adapters/foo-bar-test.js`, which you should now customize. ### Required Methods diff --git a/addon/services/metrics.js b/addon/services/metrics.js index b5247768..13ab3790 100644 --- a/addon/services/metrics.js +++ b/addon/services/metrics.js @@ -8,17 +8,20 @@ const { warn, get, set, + merge, A: emberArray, String: { dasherize } } = Ember; export default Service.extend({ _adapters: {}, + context: null, init() { const adapters = getWithDefault(this, 'metricsAdapters', emberArray([])); this._super(...arguments); this.activateAdapters(adapters); + set(this, 'context', {}); }, identify(...args) { @@ -60,22 +63,18 @@ export default Service.extend({ invoke(methodName, ...args) { const adaptersObj = get(this, '_adapters'); - const adapterNames = Object.keys(adaptersObj); + const allAdapterNames = Object.keys(adaptersObj); + const [selectedAdapterNames, options] = args.length > 1 ? [[args[0]], args[1]] : [allAdapterNames, args[0]]; + const context = get(this, 'context'); + const mergedOptions = merge(context, options); - const adapters = adapterNames.map((adapterName) => { + const selectedAdapters = selectedAdapterNames.map((adapterName) => { return get(adaptersObj, adapterName); }); - if (args.length > 1) { - let [ adapterName, options ] = args; - const adapter = get(adaptersObj, adapterName); - - adapter[methodName](options); - } else { - adapters.forEach((adapter) => { - adapter[methodName](...args); - }); - } + selectedAdapters.forEach((adapter) => { + adapter[methodName](mergedOptions); + }); }, _activateAdapter(adapterOption = {}) { diff --git a/tests/unit/services/metrics-test.js b/tests/unit/services/metrics-test.js index d62036a4..45bee3cb 100644 --- a/tests/unit/services/metrics-test.js +++ b/tests/unit/services/metrics-test.js @@ -2,7 +2,7 @@ import Ember from 'ember'; import { moduleFor, test } from 'ember-qunit'; import sinon from 'sinon'; -const get = Ember.get; +const { get, set } = Ember; let sandbox, metricsAdapters; moduleFor('service:metrics', 'Unit | Service | metrics', { @@ -131,6 +131,16 @@ test('#invoke invokes the named method on a single activated adapter with no arg assert.ok(GoogleAnalyticsStub.calledOnce, 'it invoked the Google Analytics method'); }); +test('#invoke includes `context` properties', function(assert){ + const service = this.subject({ metricsAdapters }); + const GoogleAnalyticsSpy = sandbox.spy(get(service, '_adapters.GoogleAnalytics'), 'trackPage'); + + set(service, 'context.userName', "Jimbo"); + service.invoke('trackPage', 'GoogleAnalytics', {page: 'page/1', title: 'page one'}); + + assert.ok(GoogleAnalyticsSpy.calledWith({userName: "Jimbo", page: 'page/1', title: 'page one'})); +}); + test('it implements standard contracts', function(assert) { const service = this.subject({ metricsAdapters }); sandbox.stub(window.mixpanel); From bb4d24aac5a3a4e9f3215034bdc69d90f3dbe3b6 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Thu, 17 Sep 2015 10:23:55 -0400 Subject: [PATCH 2/3] Clean up Service implementation --- README.md | 16 ++-- addon/services/metrics.js | 140 +++++++++++++++++++--------- testem.json | 3 +- tests/unit/services/metrics-test.js | 6 +- 4 files changed, 108 insertions(+), 57 deletions(-) diff --git a/README.md b/README.md index b91871e2..450d9c82 100644 --- a/README.md +++ b/README.md @@ -142,6 +142,14 @@ metrics.trackPage('GoogleAnalytics', { }); ``` +#### Context +Often, you may want to include information like the current user's name with every event or page view that's tracked. Any properties that are set on `metrics.context` will be merged into options for every Service call. + +```js +Ember.set(this, 'metrics.context.userName', 'Jimbo'); +Ember.get(this, 'metrics').trackPage({ page: 'page/1' }); // { userName: 'Jimbo', page: 'page/1' } +``` + ### API #### Service API @@ -164,14 +172,6 @@ There are 4 main methods implemented by the service, with the same argument sign For services that implement it, this method notifies the analytics service that an anonymous user now has a unique identifier. -##### Context - -Often you'll want to include things like `currentUser.name` with every event or page view that's tracked. Any properties that you bind to `metrics.context` will be merged into the options for every service call. - - set('metrics.context.userName', 'Jimbo'); - get('metrics').trackPage({page: 'page/1'}); - // => {userName: 'Jimbo', page: 'page/1'} - #### `link-to` API To use the augmented `link-to`, just use the same helper, but add some extra `metrics` attributes: diff --git a/addon/services/metrics.js b/addon/services/metrics.js index 13ab3790..bd4abd8b 100644 --- a/addon/services/metrics.js +++ b/addon/services/metrics.js @@ -4,24 +4,52 @@ const { Service, getWithDefault, assert, - isNone, - warn, get, set, merge, A: emberArray, String: { dasherize } } = Ember; +const { keys } = Object; export default Service.extend({ - _adapters: {}, + /** + * Cached adapters to reduce multiple expensive lookups. + * + * @property _adapters + * @private + * @type Object + * @default null + */ + _adapters: null, + + /** + * Contextual information attached to each call to an adapter. Often you'll + * want to include things like `currentUser.name` with every event or page + * view that's tracked. Any properties that you bind to `metrics.context` + * will be merged into the options for every service call. + * + * @property context + * @type Object + * @default null + */ context: null, + /** + * When the Service is created, activate adapters that were specified in the + * configuration. This config is injected into the Service as + * `metricsAdapters`. + * + * @method init + * @param {Void} + * @return {Void} + */ init() { const adapters = getWithDefault(this, 'metricsAdapters', emberArray([])); - this._super(...arguments); - this.activateAdapters(adapters); + set(this, '_adapters', {}); set(this, 'context', {}); + this.activateAdapters(adapters); + this._super(...arguments); }, identify(...args) { @@ -40,20 +68,21 @@ export default Service.extend({ this.invoke('trackPage', ...args); }, + /** + * Instantiates the adapters specified in the configuration and caches them + * for future retrieval. + * + * @method activateAdapters + * @param {Array} adapterOptions + * @return {Object} instantiated adapters + */ activateAdapters(adapterOptions = []) { const cachedAdapters = get(this, '_adapters'); - let activatedAdapters = {}; + const activatedAdapters = {}; adapterOptions.forEach((adapterOption) => { const { name } = adapterOption; - let adapter; - - if (cachedAdapters[name]) { - warn(`[ember-metrics] Metrics adapter ${name} has already been activated.`); - adapter = cachedAdapters[name]; - } else { - adapter = this._activateAdapter(adapterOption); - } + const adapter = cachedAdapters[name] ? cachedAdapters[name] : this._activateAdapter(adapterOption); set(activatedAdapters, name, adapter); }); @@ -61,51 +90,74 @@ export default Service.extend({ return set(this, '_adapters', activatedAdapters); }, + /** + * Invokes a method across all activated adapters. + * + * @method invoke + * @param {String} methodName + * @param {Rest} args + * @return {Void} + */ invoke(methodName, ...args) { - const adaptersObj = get(this, '_adapters'); - const allAdapterNames = Object.keys(adaptersObj); + const cachedAdapters = get(this, '_adapters'); + const allAdapterNames = keys(cachedAdapters); const [selectedAdapterNames, options] = args.length > 1 ? [[args[0]], args[1]] : [allAdapterNames, args[0]]; - const context = get(this, 'context'); - const mergedOptions = merge(context, options); + const mergedOptions = merge(get(this, 'context'), options); - const selectedAdapters = selectedAdapterNames.map((adapterName) => { - return get(adaptersObj, adapterName); - }); + selectedAdapterNames + .map((adapterName) => get(cachedAdapters, adapterName)) + .forEach((adapter) => adapter[methodName](mergedOptions)); + }, - selectedAdapters.forEach((adapter) => { - adapter[methodName](mergedOptions); - }); + /** + * On teardown, destroy cached adapters together with the Service. + * + * @method willDestroy + * @param {Void} + * @return {Void} + */ + willDestroy() { + const cachedAdapters = get(this, '_adapters'); + + for (let adapterName in cachedAdapters) { + get(cachedAdapters, adapterName).destroy(); + } }, - _activateAdapter(adapterOption = {}) { - const metrics = this; - const { name, config } = adapterOption; + /** + * Instantiates an adapter if one is found. + * + * @method _activateAdapter + * @param {Object} + * @private + * @return {Adapter} + */ + _activateAdapter({ name, config } = {}) { const Adapter = this._lookupAdapter(name); assert(`[ember-metrics] Could not find metrics adapter ${name}.`, Adapter); - return Adapter.create({ metrics, config }); + return Adapter.create({ this, config }); }, - _lookupAdapter(adapterName = '') { - const container = get(this, 'container'); - - if (isNone(container)) { - return; - } + /** + * Looks up the adapter from the container. Prioritizes the consuming app's + * adapters over the addon's adapters. + * + * @method _lookupAdapter + * @param {String} adapterName + * @private + * @return {Adapter} a local adapter or an adapter from the addon + */ + _lookupAdapter(adapterName) { + const { container } = this; + + assert('[ember-metrics] The service is missing its container.', container); + assert('[ember-metrics] Could not find metrics adapter without a name.', adapterName); const dasherizedAdapterName = dasherize(adapterName); const availableAdapter = container.lookupFactory(`ember-metrics@metrics-adapter:${dasherizedAdapterName}`); const localAdapter = container.lookupFactory(`metrics-adapter:${dasherizedAdapterName}`); - const adapter = localAdapter ? localAdapter : availableAdapter; - return adapter; - }, - - willDestroy() { - const adapters = get(this, '_adapters'); - - for (let adapterName in adapters) { - get(adapters, adapterName).destroy(); - } + return localAdapter ? localAdapter : availableAdapter; } }); diff --git a/testem.json b/testem.json index 0f35392c..5de2d54d 100644 --- a/testem.json +++ b/testem.json @@ -6,7 +6,6 @@ "PhantomJS" ], "launch_in_dev": [ - "PhantomJS", - "Chrome" + "PhantomJS" ] } diff --git a/tests/unit/services/metrics-test.js b/tests/unit/services/metrics-test.js index 45bee3cb..809edbea 100644 --- a/tests/unit/services/metrics-test.js +++ b/tests/unit/services/metrics-test.js @@ -135,10 +135,10 @@ test('#invoke includes `context` properties', function(assert){ const service = this.subject({ metricsAdapters }); const GoogleAnalyticsSpy = sandbox.spy(get(service, '_adapters.GoogleAnalytics'), 'trackPage'); - set(service, 'context.userName', "Jimbo"); - service.invoke('trackPage', 'GoogleAnalytics', {page: 'page/1', title: 'page one'}); + set(service, 'context.userName', 'Jimbo'); + service.invoke('trackPage', 'GoogleAnalytics', { page: 'page/1', title: 'page one' }); - assert.ok(GoogleAnalyticsSpy.calledWith({userName: "Jimbo", page: 'page/1', title: 'page one'})); + assert.ok(GoogleAnalyticsSpy.calledWith({ userName: 'Jimbo', page: 'page/1', title: 'page one' }), 'it includes context properties'); }); test('it implements standard contracts', function(assert) { From a0939f4b26cc9d3e227970ea06c5b499cc05af55 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Thu, 17 Sep 2015 10:46:59 -0400 Subject: [PATCH 3/3] Version bump to 0.2.0 --- package.json | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 91b77bfb..521fd765 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "ember-metrics", - "version": "0.1.5", + "version": "0.2.0", "description": "Send data to multiple analytics integrations without re-implementing new API", "directories": { "doc": "doc", @@ -52,7 +52,9 @@ "analytics", "segment", "tracking", - "google analytics" + "google analytics", + "google tag manager", + "mixpanel" ], "dependencies": { "ember-cli-babel": "^5.1.3"