From ba6509c7a3a11e839ff376ba95717d0e2308841f Mon Sep 17 00:00:00 2001
From: Mark Stacey <markjstacey@gmail.com>
Date: Tue, 3 Nov 2020 17:11:24 -0330
Subject: [PATCH] Simplify handling of Segment keys (#9781)

It was getting rather complicated to keep track of which Segment keys
were set where, and under which name.

The build script now injects a key even in test environments, but it is
unused if `IN_TEST` is truthy. This should be functionally equivalent
to the old logic. I find this simpler mainly for two reasons: there is
one less intermediate variable to keep track of now, and the `IN_TEST`
check is now directly in the module where we're constructing the
`segment` instance, rather than being referenced at a distance in a
comment.

The old setup made it difficult to turn on metrics for specific e2e
tests as well, which will be done in a subsequent PR.
---
 development/build/scripts.js  | 31 +++++++++----------------------
 shared/modules/metametrics.js | 20 +++++++++++---------
 2 files changed, 20 insertions(+), 31 deletions(-)

diff --git a/development/build/scripts.js b/development/build/scripts.js
index 891978216..8efae39e4 100644
--- a/development/build/scripts.js
+++ b/development/build/scripts.js
@@ -356,24 +356,6 @@ function createScriptTasks({ browserPlatforms, livereload }) {
       throw new Error('Missing SENTRY_DSN environment variable')
     }
 
-    // When we're in the 'production' environment we will use a specific key only set in CI
-    // Otherwise we'll use the key from .metamaskrc or from the environment variable. If
-    // the value of SEGMENT_WRITE_KEY that we envify is undefined then no events will be tracked
-    // in the build. This is intentional so that developers can contribute to MetaMask without
-    // inflating event volume.
-    const SEGMENT_PROD_WRITE_KEY = opts.testing
-      ? undefined
-      : process.env.SEGMENT_PROD_WRITE_KEY
-    const SEGMENT_DEV_WRITE_KEY = opts.testing
-      ? undefined
-      : conf.SEGMENT_WRITE_KEY
-    const SEGMENT_PROD_LEGACY_WRITE_KEY = opts.testing
-      ? undefined
-      : process.env.SEGMENT_PROD_LEGACY_WRITE_KEY
-    const SEGMENT_DEV_LEGACY_WRITE_KEY = opts.testing
-      ? undefined
-      : conf.SEGMENT_LEGACY_WRITE_KEY
-
     // Inject variables into bundle
     bundler.transform(
       envify({
@@ -390,14 +372,19 @@ function createScriptTasks({ browserPlatforms, livereload }) {
         INFURA_PROJECT_ID: opts.testing
           ? '00000000000000000000000000000000'
           : conf.INFURA_PROJECT_ID,
+        // When we're in the 'production' environment we will use a specific key only set in CI
+        // Otherwise we'll use the key from .metamaskrc or from the environment variable. If
+        // the value of SEGMENT_WRITE_KEY that we envify is undefined then no events will be tracked
+        // in the build. This is intentional so that developers can contribute to MetaMask without
+        // inflating event volume.
         SEGMENT_WRITE_KEY:
           environment === 'production'
-            ? SEGMENT_PROD_WRITE_KEY
-            : SEGMENT_DEV_WRITE_KEY,
+            ? process.env.SEGMENT_PROD_WRITE_KEY
+            : conf.SEGMENT_WRITE_KEY,
         SEGMENT_LEGACY_WRITE_KEY:
           environment === 'production'
-            ? SEGMENT_PROD_LEGACY_WRITE_KEY
-            : SEGMENT_DEV_LEGACY_WRITE_KEY,
+            ? process.env.SEGMENT_PROD_LEGACY_WRITE_KEY
+            : conf.SEGMENT_LEGACY_WRITE_KEY,
       }),
       {
         global: true,
diff --git a/shared/modules/metametrics.js b/shared/modules/metametrics.js
index 78a80f048..73a6d8c0d 100644
--- a/shared/modules/metametrics.js
+++ b/shared/modules/metametrics.js
@@ -63,16 +63,18 @@ export function sendCountIsTrackable(sendCount) {
 // provided a SEGMENT_WRITE_KEY. This also holds true for test environments and
 // E2E, which is handled in the build process by never providing the SEGMENT_WRITE_KEY
 // when process.env.IN_TEST is truthy
-export const segment = process.env.SEGMENT_WRITE_KEY
-  ? new Analytics(process.env.SEGMENT_WRITE_KEY, { flushAt, flushInterval })
-  : segmentNoop
+export const segment =
+  process.env.IN_TEST || !process.env.SEGMENT_WRITE_KEY
+    ? segmentNoop
+    : new Analytics(process.env.SEGMENT_WRITE_KEY, { flushAt, flushInterval })
 
-export const segmentLegacy = process.env.SEGMENT_LEGACY_WRITE_KEY
-  ? new Analytics(process.env.SEGMENT_LEGACY_WRITE_KEY, {
-      flushAt,
-      flushInterval,
-    })
-  : segmentNoop
+export const segmentLegacy =
+  process.env.IN_TEST || !process.env.SEGMENT_LEGACY_WRITE_KEY
+    ? segmentNoop
+    : new Analytics(process.env.SEGMENT_LEGACY_WRITE_KEY, {
+        flushAt,
+        flushInterval,
+      })
 
 /**
  * We attach context to every meta metrics event that help to qualify our analytics.