diff --git a/packages/fxa-content-server/app/scripts/lib/metrics.js b/packages/fxa-content-server/app/scripts/lib/metrics.js index 3981499a6d..1321f2d334 100644 --- a/packages/fxa-content-server/app/scripts/lib/metrics.js +++ b/packages/fxa-content-server/app/scripts/lib/metrics.js @@ -584,6 +584,17 @@ _.extend(Metrics.prototype, Backbone.Events, { return id; }, + /** + * Set the `service` parameter to use for all future metrics. + * This is useful in cases where don't learn the appropriate + * service value until after the app has been initialized. + * + * @param {String} [service] The service identifier + */ + setService(service) { + this._service = service || NOT_REPORTED_VALUE; + }, + /** * Set the view name prefix for metrics that contain a viewName. * This is used to differentiate between flows when the same diff --git a/packages/fxa-content-server/app/scripts/models/auth_brokers/fx-sync.js b/packages/fxa-content-server/app/scripts/models/auth_brokers/fx-sync.js index 46f20f955b..bea431710f 100644 --- a/packages/fxa-content-server/app/scripts/models/auth_brokers/fx-sync.js +++ b/packages/fxa-content-server/app/scripts/models/auth_brokers/fx-sync.js @@ -67,6 +67,7 @@ export default BaseAuthenticationBroker.extend({ // the service in the query parameter currently overrides the status message // this is due to backwards compatibility this.relier.set('service', response.clientId); + this._metrics.setService(response.clientId); } } const additionalEngineIds = diff --git a/packages/fxa-content-server/app/tests/spec/lib/metrics.js b/packages/fxa-content-server/app/tests/spec/lib/metrics.js index 7e96581b14..9735ea3ae8 100644 --- a/packages/fxa-content-server/app/tests/spec/lib/metrics.js +++ b/packages/fxa-content-server/app/tests/spec/lib/metrics.js @@ -704,6 +704,15 @@ describe('lib/metrics', () => { }); }); + describe('setService', function() { + it('sets the service identifier', function() { + metrics.setService('00112233445566'); + const { service } = metrics.getFilteredData(); + + assert.equal(service, '00112233445566'); + }); + }); + describe('isCollectionEnabled', () => { it('reports that collection is enabled if `isSampledUser===true`', () => { assert.isTrue(metrics.isCollectionEnabled()); diff --git a/packages/fxa-content-server/app/tests/spec/models/auth_brokers/fx-sync.js b/packages/fxa-content-server/app/tests/spec/models/auth_brokers/fx-sync.js index d33bfc3fc0..76538f88e4 100644 --- a/packages/fxa-content-server/app/tests/spec/models/auth_brokers/fx-sync.js +++ b/packages/fxa-content-server/app/tests/spec/models/auth_brokers/fx-sync.js @@ -84,33 +84,59 @@ describe('models/auth_brokers/fx-sync', () => { }); }); - describe('syncOptional capability', () => { - it('sets `syncOptional` to false if no multiService capability', () => { - relier.set('service', 'foo'); - broker.onFxaStatus({ capabilities: {} }); + describe('fxa_status response handling', () => { + describe('syncOptional capability', () => { + it('sets `syncOptional` to false if no multiService capability', () => { + relier.set('service', 'foo'); + broker.onFxaStatus({ capabilities: {} }); - assert.isFalse(broker.hasCapability('syncOptional')); + assert.isFalse(broker.hasCapability('syncOptional')); + }); + + it('sets `syncOptional` to false if multiService: false', () => { + relier.set('service', 'foo'); + broker.onFxaStatus({ capabilities: { multiService: false } }); + + assert.isFalse(broker.hasCapability('syncOptional')); + }); + + it('sets `syncOptional` to false if service===sync', () => { + relier.set('service', 'sync'); + broker.onFxaStatus({ capabilities: { multiService: true } }); + + assert.isFalse(broker.hasCapability('syncOptional')); + }); + + it('sets `syncOptional` to true if multiService and not sync', () => { + relier.set('service', 'foo'); + broker.onFxaStatus({ capabilities: { multiService: true } }); + + assert.isTrue(broker.hasCapability('syncOptional')); + }); }); - it('sets `syncOptional` to false if multiService: false', () => { - relier.set('service', 'foo'); - broker.onFxaStatus({ capabilities: { multiService: false } }); + describe('clientId', () => { + it('sets `service` based on `clientId` when not otherwise known', () => { + broker.onFxaStatus({ + capabilities: { multiService: true }, + clientId: 'fx-desktop', + }); - assert.isFalse(broker.hasCapability('syncOptional')); - }); + assert.equal(relier.get('service'), 'fx-desktop'); + assert.equal(metrics.getFilteredData().service, 'fx-desktop'); + }); - it('sets `syncOptional` to false if service===sync', () => { - relier.set('service', 'sync'); - broker.onFxaStatus({ capabilities: { multiService: true } }); + it('does not override an explicit `service` value obtained from query parameters', () => { + relier.set('service', 'sync'); + metrics.setService('sync'); + broker.onFxaStatus({ + capabilities: { multiService: true }, + clientId: 'fx-desktop', + }); - assert.isFalse(broker.hasCapability('syncOptional')); - }); - - it('sets `syncOptional` to true if multiService and not sync', () => { - relier.set('service', 'foo'); - broker.onFxaStatus({ capabilities: { multiService: true } }); - - assert.isTrue(broker.hasCapability('syncOptional')); + assert.equal(relier.get('service'), 'sync'); + assert.equal(metrics.getFilteredData().service, 'sync'); + }); }); });