Merge pull request #1598 from sarracini/gh1516

shield(metrics): Closes #1516 Add 'shield_variant' to common properties of pings
mozilla · Oct 20, 2016 · 0c67b15 · 0c67b15
2 parents 1c88d85 + dbac6de
commit 0c67b15
Show file tree

Hide file tree

Showing 5 changed files with 11 additions and 8 deletions.
diff --git a/addon/ActivityStreams.js b/addon/ActivityStreams.js
@@ -51,7 +51,6 @@ const DEFAULT_OPTIONS = {
   onRemoveWorker: null,
   placesCacheTimeout: 1800000, // every 30 minutes, rebuild/repopulate the cache
   recommendationTTL: 3600000, // every hour, get a new recommendation
-  shield_variant: "N/A",
   shareProvider: null,
   pageScraper: null,
   searchProvider: null,
@@ -503,7 +502,7 @@ ActivityStreams.prototype = {
   },
 
   _handleUserEvent({msg}) {
-    this._tabTracker.handleUserEvent(Object.assign(msg.data, {"shield_variant": this.options.shield_variant}));
+    this._tabTracker.handleUserEvent(msg.data);
   },
 
   _respondToRecommendationToggle() {

diff --git a/addon/TabTracker.js b/addon/TabTracker.js
@@ -22,9 +22,10 @@ const DEFAULT_PLACES_QUERIES = {
   getBookmarksSize() {return Promise.resolve(0);},
   getHistorySize() {return Promise.resolve(0);}
 };
-function TabTracker(clientID) {
+function TabTracker(options) {
   this._tabData = {};
-  this._clientID = clientID;
+  this._clientID = options.clientID;
+  this._shieldVariant = options.shield_variant;
   this.onOpen = this.onOpen.bind(this);
 
   this._onPrefChange = this._onPrefChange.bind(this);
@@ -87,6 +88,9 @@ TabTracker.prototype = {
     if (this._experimentID) {
       payload.experiment_id = this._experimentID;
     }
+    if (this._shieldVariant) {
+      payload.shield_variant = this._shieldVariant;
+    }
   },
 
   uninit() {

diff --git a/addon/main.js b/addon/main.js
@@ -32,7 +32,7 @@ Object.assign(exports, {
     Task.spawn(function*() {
       const clientID = yield ClientID.getClientID();
       options.clientID = clientID;
-      const tabTracker = new TabTracker(clientID);
+      const tabTracker = new TabTracker(options);
       const telemetrySender = new TelemetrySender();
 
       if (options.loadReason === "upgrade") {

diff --git a/test/lib/utils.js b/test/lib/utils.js
@@ -119,7 +119,7 @@ function getTestActivityStream(options = {}) {
   options.pageScraper = mockPageScraper;
   options.searchProvider = getTestSearchProvider();
   options.recommendationProvider = getTestRecommendationProvider();
-  const testTabTracker = new TabTracker(options.clientID);
+  const testTabTracker = new TabTracker(options);
   const testTelemetrySender = new TelemetrySender();
   let mockApp = new ActivityStreams(mockMetadataStore, testTabTracker, testTelemetrySender, options);
   mockApp.init();

diff --git a/test/test-TabTracker.js b/test/test-TabTracker.js
@@ -397,7 +397,7 @@ exports.test_TabTracker_action_pings = function*(assert) {
   app._handleUserEvent(eventData);
 
   let pingData = yield userEventPromise;
-  let additionalKeys = ["client_id", "addon_version", "locale", "action", "tab_id", "page", "shield_variant"];
+  let additionalKeys = ["client_id", "addon_version", "locale", "action", "tab_id", "page"];
   for (let key of additionalKeys) {
     assert.ok(pingData[key], `The ping has the additional key ${key}`);
   }
@@ -459,7 +459,7 @@ exports.test_TabTracker_unload_reason_with_user_action = function*(assert) {
     app._handleUserEvent(eventData);
 
     const eventPingData = yield userEventPromise;
-    const additionalKeys = ["client_id", "addon_version", "locale", "action", "tab_id", "page", "shield_variant"];
+    const additionalKeys = ["client_id", "addon_version", "locale", "action", "tab_id", "page"];
     for (let key of additionalKeys) {
       assert.ok(eventPingData[key], `The ping has the additional key ${key}`);
     }