feat(metrics): emit a flow.continued event for signinCodes

#1946
r=seanmonstar

lib/db.js

@@ -909,12 +909,12 @@ module.exports = (
       )
   }
 
-  DB.prototype.createSigninCode = function (uid) {
+  DB.prototype.createSigninCode = function (uid, flowId) {
     log.trace({ op: 'DB.createSigninCode' })
 
     return random(config.signinCodeSize)
       .then(code => {
-        const data = { uid, createdAt: Date.now() }
+        const data = { uid, createdAt: Date.now(), flowId }
         return this.pool.put(`/signinCodes/${code.toString('hex')}`, data)
           .then(() => code, err => {
             if (isRecordAlreadyExistsError(err)) {

lib/routes/signin-codes.js

@@ -51,6 +51,11 @@ module.exports = (log, db, customs) => {
           return db.consumeSigninCode(code)
             .then(result => {
               return request.emitMetricsEvent('signinCode.consumed')
+                .then(() => {
+                  if (result.flowId) {
+                    return request.emitMetricsEvent(`flow.continued.${result.flowId}`)
+                  }
+                })
                 .then(() => result)
             })
         }

lib/routes/sms.js

@@ -87,7 +87,8 @@ module.exports = (log, db, config, customs, sms) => {
 
         function createSigninCode () {
           if (request.app.features.has('signinCodes')) {
-            return db.createSigninCode(sessionToken.uid)
+            return request.gatherMetricsContext({})
+              .then(metricsContext => db.createSigninCode(sessionToken.uid, metricsContext.flow_id))
           }
         }
 

test/local/routes/signin-codes.js

@@ -12,8 +12,8 @@ const P = require('../../../lib/promise')
 describe('/signinCodes/consume:', () => {
   let log, db, customs, routes, route, request
 
-  describe('success:', () => {
-    beforeEach(() => setup())
+  describe('success, db does not return flowId:', () => {
+    beforeEach(() => setup({ db: { email: 'foo@bar' } }))
 
     it('called log.begin correctly', () => {
       assert.equal(log.begin.callCount, 1)
@@ -55,8 +55,42 @@ describe('/signinCodes/consume:', () => {
     })
   })
 
+  describe('success, db returns flowId:', () => {
+    beforeEach(() => setup({ db: { email: 'foo@bar', flowId: 'baz' } }))
+
+    it('called log.begin once', () => {
+      assert.equal(log.begin.callCount, 1)
+    })
+
+    it('called request.validateMetricsContext once', () => {
+      assert.equal(request.validateMetricsContext.callCount, 1)
+    })
+
+    it('called customs.checkIpOnly once', () => {
+      assert.equal(customs.checkIpOnly.callCount, 1)
+    })
+
+    it('called db.consumeSigninCode once', () => {
+      assert.equal(db.consumeSigninCode.callCount, 1)
+    })
+
+    it('called log.flowEvent correctly', () => {
+      assert.equal(log.flowEvent.callCount, 2)
+
+      let args = log.flowEvent.args[0]
+      assert.equal(args.length, 1)
+      assert.equal(args[0].event, 'signinCode.consumed')
+      assert.equal(args[0].flow_id, request.payload.metricsContext.flowId)
+
+      args = log.flowEvent.args[1]
+      assert.equal(args.length, 1)
+      assert.equal(args[0].event, 'flow.continued.baz')
+      assert.equal(args[0].flow_id, request.payload.metricsContext.flowId)
+    })
+  })
+
   describe('db error:', () => {
-    beforeEach(() => setup({ db: { consumeSigninCode: 'foo' } }))
+    beforeEach(() => setup(null, { db: { consumeSigninCode: 'foo' } }))
 
     it('called log.begin', () => {
       assert.equal(log.begin.callCount, 1)
@@ -80,7 +114,7 @@ describe('/signinCodes/consume:', () => {
   })
 
   describe('customs error:', () => {
-    beforeEach(() => setup({ customs: { checkIpOnly: 'foo' } }))
+    beforeEach(() => setup(null, { customs: { checkIpOnly: 'foo' } }))
 
     it('called log.begin', () => {
       assert.equal(log.begin.callCount, 1)
@@ -103,11 +137,12 @@ describe('/signinCodes/consume:', () => {
     })
   })
 
-  function setup (errors) {
+  function setup (results, errors) {
+    results = results || {}
     errors = errors || {}
 
     log = mocks.spyLog()
-    db = mocks.mockDB(null, errors.db)
+    db = mocks.mockDB(results.db, errors.db)
     customs = mocks.mockCustoms(errors.customs)
     routes = makeRoutes({ log, db, customs })
     route = getRoute(routes, '/signinCodes/consume')

test/local/routes/sms.js

@@ -100,8 +100,9 @@ describe('/sms with the signinCodes feature included in the payload', () => {
       it('called db.createSigninCode correctly', () => {
         assert.equal(db.createSigninCode.callCount, 1)
         const args = db.createSigninCode.args[0]
-        assert.equal(args.length, 1)
+        assert.equal(args.length, 2)
         assert.equal(args[0], 'bar')
+        assert.equal(args[1], request.payload.metricsContext.flowId)
       })
 
       it('called sms.send correctly', () => {

test/mocks.js

@@ -178,7 +178,15 @@ function mockDB (data, errors) {
         }
       ])
     }),
-    consumeSigninCode: optionallyThrow(errors, 'consumeSigninCode'),
+    consumeSigninCode: sinon.spy(() => {
+      if (errors.consumeSigninCode) {
+        return P.reject(errors.consumeSigninCode)
+      }
+      return P.resolve({
+        email: data.email,
+        flowId: data.flowId
+      })
+    }),
     createAccount: sinon.spy(() => {
       return P.resolve({
         uid: data.uid,

test/remote/db_tests.js

@@ -646,8 +646,9 @@ describe('remote db', function() {
 
   it('signinCodes', () => {
     let previousCode
+    const flowId = crypto.randomBytes(32)
 
-    // Create a signinCode
+    // Create a signinCode without a flowId
     return db.createSigninCode(ACCOUNT.uid)
       .then(code => {
         assert.ok(Buffer.isBuffer(code), 'db.createSigninCode should return a buffer')
@@ -667,8 +668,9 @@ describe('remote db', function() {
           callback(null, previousCode)
         })
 
-        // Create a signinCode with crypto.randomBytes rigged to return a duplicate
-        return db.createSigninCode(ACCOUNT.uid)
+        // Create a signinCode with crypto.randomBytes rigged to return a duplicate,
+        // and this time specifying a flowId
+        return db.createSigninCode(ACCOUNT.uid, flowId)
       })
       .then(code => {
         assert.ok(Buffer.isBuffer(code), 'db.createSigninCode should return a buffer')
@@ -682,7 +684,12 @@ describe('remote db', function() {
         ])
       })
       .then(results => {
-        results.forEach(result => assert.deepEqual(result, { email: ACCOUNT.email }, 'db.consumeSigninCode should return the email address'))
+        assert.deepEqual(results[0], { email: ACCOUNT.email }, 'db.consumeSigninCode should return the email address')
+        assert.equal(results[1].email, ACCOUNT.email, 'db.consumeSigninCode should return the email address')
+        if (results[1].flowId) {
+          // This assertion is conditional so that tests pass regardless of db version
+          assert.equal(results[1].flowId, flowId.toString('hex'), 'db.consumeSigninCode should return the flowId')
+        }
 
         // Attempt to consume a consumed signinCode
         return db.consumeSigninCode(previousCode)