From 4f6f367f5e69fd4dcd77ed44ad8f9826ab789351 Mon Sep 17 00:00:00 2001
From: Phil Booth <pmbooth@gmail.com>
Date: Mon, 25 Sep 2017 08:11:43 +0100
Subject: [PATCH] fix(metrics): fix the data on email sent events

https://github.com/mozilla/fxa-auth-server/pull/2139
r=rfk
---
 lib/email/utils/helpers.js  | 11 ++++++-----
 test/local/email/utils.js   | 16 ++++++++++------
 test/local/senders/email.js | 10 ++++++----
 3 files changed, 22 insertions(+), 15 deletions(-)

diff --git a/lib/email/utils/helpers.js b/lib/email/utils/helpers.js
index 5a94d383..505b3c2f 100644
--- a/lib/email/utils/helpers.js
+++ b/lib/email/utils/helpers.js
@@ -75,10 +75,10 @@ function logEmailEventSent(log, message) {
   const emailEventInfo = {
     op: 'emailEvent',
     template: message.template,
-    type: 'sent'
+    type: 'sent',
+    flow_id: message.flowId
   }
 
-  emailEventInfo['flow_id'] = getHeaderValue('X-Flow-Id', message)
   emailEventInfo.locale = getHeaderValue('Content-Language', message)
 
   const addrs = [message.email].concat(message.ccEmails || [])
@@ -112,11 +112,12 @@ function logAmplitudeEvent (log, message, eventInfo) {
     query: {},
     payload: {}
   }, {
-    device_id: getHeaderValue('X-Device-Id', message),
+    device_id: message.deviceId || getHeaderValue('X-Device-Id', message),
     email_domain: eventInfo.domain,
-    service: getHeaderValue('X-Service-Id', message),
-    uid: getHeaderValue('X-Uid', message)
+    service: message.service || getHeaderValue('X-Service-Id', message),
+    uid: message.uid || getHeaderValue('X-Uid', message)
   }, {
+    flowBeginTime: message.flowBeginTime || getHeaderValue('X-Flow-Begin-Time', message),
     flow_id: eventInfo.flow_id,
     time: Date.now()
   })
diff --git a/test/local/email/utils.js b/test/local/email/utils.js
index e2ab6c91..bcbe4204 100644
--- a/test/local/email/utils.js
+++ b/test/local/email/utils.js
@@ -87,12 +87,13 @@ describe('email utils helpers', () => {
       ccEmails: [ 'bar@example.com', 'baz@example.com' ],
       template: 'verifyEmail',
       headers: [
-        { name: 'Content-Language', value: 'aaa' },
-        { name: 'X-Device-Id', value: 'bbb' },
-        { name: 'X-Flow-Id', value: 'ccc' },
-        { name: 'X-Service-Id', value: 'ddd' },
-        { name: 'X-Uid', value: 'eee' }
-      ]
+        { name: 'Content-Language', value: 'aaa' }
+      ],
+      deviceId: 'bbb',
+      flowBeginTime: 42,
+      flowId: 'ccc',
+      service: 'ddd',
+      uid: 'eee'
     })
     assert.equal(amplitude.callCount, 1)
     const args = amplitude.args[0]
@@ -118,6 +119,7 @@ describe('email utils helpers', () => {
       uid: 'eee'
     })
     assert.equal(args[3].flow_id, 'ccc')
+    assert.equal(args[3].flowBeginTime, 42)
     assert.ok(args[3].time > Date.now() - 1000)
   })
 
@@ -128,6 +130,7 @@ describe('email utils helpers', () => {
       headers: [
         { name: 'Content-Language', value: 'a' },
         { name: 'X-Device-Id', value: 'b' },
+        { name: 'X-Flow-Begin-Time', value: 1 },
         { name: 'X-Flow-Id', value: 'c' },
         { name: 'X-Service-Id', value: 'd' },
         { name: 'X-Template-Name', value: 'verifyLoginEmail' },
@@ -158,6 +161,7 @@ describe('email utils helpers', () => {
       uid: 'e'
     })
     assert.equal(args[3].flow_id, 'c')
+    assert.equal(args[3].flowBeginTime, 1)
   })
 
   describe('logErrorIfHeadersAreWeirdOrMissing', () => {
diff --git a/test/local/senders/email.js b/test/local/senders/email.js
index 0a14fc9a..ee8212e4 100644
--- a/test/local/senders/email.js
+++ b/test/local/senders/email.js
@@ -122,6 +122,10 @@ describe(
       })
     })
 
+    afterEach(() => {
+      mockLog.info.reset()
+    })
+
     messageTypes.forEach(
       function (type) {
         var message = {
@@ -631,10 +635,9 @@ describe(
     it(
       'logs emailEvent on send',
       function () {
-        mockLog.info.reset()
-
         var message = {
           email: 'test@restmail.net',
+          flowId: 'wibble',
           subject: 'subject',
           template: 'verifyLoginEmail',
           uid: 'foo'
@@ -646,6 +649,7 @@ describe(
             const emailEventLog = mockLog.info.getCalls()[2]
             assert.equal(emailEventLog.args[0].op, 'emailEvent', 'logs emailEvent')
             assert.equal(emailEventLog.args[0].domain, 'other', 'logs domain')
+            assert.equal(emailEventLog.args[0].flow_id, 'wibble', 'logs flow id')
             assert.equal(emailEventLog.args[0].template, 'verifyLoginEmail', 'logs correct template')
             assert.equal(emailEventLog.args[0].type, 'sent', 'logs correct type')
           })
@@ -655,8 +659,6 @@ describe(
     it(
       'rejects sendMail status',
       function () {
-        mailer.mailer.sendMail.reset()
-
         var message = {
           email: 'test@restmail.net',
           subject: 'subject',