From 9a4e89cee1e8237427588e2024bb2f07bf50135a Mon Sep 17 00:00:00 2001
From: Phil Booth <pmbooth@gmail.com>
Date: Tue, 18 Oct 2016 05:33:16 +0100
Subject: [PATCH] fix(logging): emit flow events for sign-in unblock, not
 activity events (#1508); r=seanmonstar,rfk

---
 lib/log.js                   |  2 --
 lib/routes/account.js        |  8 ++------
 test/local/account_routes.js | 24 +++++++++++++++++++++---
 3 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/lib/log.js b/lib/log.js
index 50be91c62..0b228d6d2 100644
--- a/lib/log.js
+++ b/lib/log.js
@@ -25,8 +25,6 @@ var ACTIVITY_FLOW_EVENTS = Object.keys(ALWAYS_ACTIVITY_FLOW_EVENTS)
   }, {
     // These activity events are flow events when there is a flowId
     'account.keyfetch': true,
-    'account.login.sentUnblockCode': true,
-    'account.login.confirmedUnblockCode': true,
     'account.signed': true
   })
 
diff --git a/lib/routes/account.js b/lib/routes/account.js
index b869a8fc5..eada0defc 100644
--- a/lib/routes/account.js
+++ b/lib/routes/account.js
@@ -461,9 +461,7 @@ module.exports = function (
                           throw error.invalidUnblockCode()
                         }
                         didSigninUnblock = true
-                        return log.activityEvent('account.login.confirmedUnblockCode', request, {
-                          uid: emailRecord.uid.toString('hex')
-                        })
+                        return log.flowEvent('account.login.confirmedUnblockCode', request)
                       }
                     )
                     .catch(
@@ -1627,9 +1625,7 @@ module.exports = function (
           .then(lookupAccount)
           .then(createUnblockCode)
           .then(mailUnblockCode)
-          .then(() => log.activityEvent('account.login.sentUnblockCode', request, {
-            uid: emailRecord.uid.toString('hex')
-          }))
+          .then(() => log.flowEvent('account.login.sentUnblockCode', request))
           .done(() => {
             reply({})
           }, reply)
diff --git a/test/local/account_routes.js b/test/local/account_routes.js
index 83640744f..724235f5f 100644
--- a/test/local/account_routes.js
+++ b/test/local/account_routes.js
@@ -784,6 +784,9 @@ test('/account/login', function (t) {
   mockLog.activityEvent = sinon.spy(function () {
     return P.resolve()
   })
+  mockLog.flowEvent = sinon.spy(function () {
+    return P.resolve()
+  })
   var mockMailer = mocks.mockMailer()
   var mockPush = mocks.mockPush()
   var mockCustoms = {
@@ -1355,10 +1358,15 @@ test('/account/login', function (t) {
           })
 
           t.test('valid code', (t) => {
-            t.plan(1)
+            t.plan(4)
             mockDB.consumeUnblockCode = () => P.resolve({ createdAt: Date.now() })
             return runTest(route, mockRequestWithUnblockCode, (res) => {
               t.ok(!(res instanceof Error), 'successful login')
+              t.equal(mockLog.flowEvent.callCount, 1, 'log.flowEvent was called once')
+              const args = mockLog.flowEvent.args[0]
+              t.equal(args.length, 2, 'log.flowEvent was passed two arguments')
+              t.equal(args[0], 'account.login.confirmedUnblockCode', 'first argument was event name')
+              mockLog.flowEvent.reset()
             })
           })
         })
@@ -1738,15 +1746,17 @@ test('/account/login/send_unblock_code', function (t) {
       allowedEmailAddresses: /^.*$/
     }
   }
+  const mockLog = mocks.spyLog()
   var accountRoutes = makeRoutes({
     config: config,
     db: mockDb,
+    log: mockLog,
     mailer: mockMailer
   })
   var route = getRoute(accountRoutes, '/account/login/send_unblock_code')
 
   t.test('signin unblock enabled', function (t) {
-    t.plan(9)
+    t.plan(12)
     config.signinUnblock.enabled = true
     return runTest(route, mockRequest, function (response) {
       t.ok(!(response instanceof Error), response.stack)
@@ -1763,16 +1773,24 @@ test('/account/login/send_unblock_code', function (t) {
       t.equal(mockMailer.sendUnblockCode.callCount, 1, 'called mailer.sendUnblockCode')
       var args = mockMailer.sendUnblockCode.args[0]
       t.equal(args.length, 3, 'mailer.sendUnblockCode called with 3 args')
+
+      t.equal(mockLog.flowEvent.callCount, 1, 'log.flowEvent was called once')
+      args = mockLog.flowEvent.args[0]
+      t.equal(args.length, 2, 'log.flowEvent was passed two arguments')
+      t.equal(args[0], 'account.login.sentUnblockCode', 'first argument was event name')
+      mockLog.flowEvent.reset()
     })
   })
 
   t.test('signin unblock disabled', function (t) {
-    t.plan(2)
+    t.plan(3)
     config.signinUnblock.enabled = false
 
     return runTest(route, mockRequest, function (err) {
       t.equal(err.output.statusCode, 503, 'correct status code is returned')
       t.equal(err.errno, error.ERRNO.FEATURE_NOT_ENABLED, 'correct errno is returned')
+
+      t.equal(mockLog.flowEvent.callCount, 0, 'log.flowEvent was not called')
     })
   })
 })