Merge pull request #6328 from MetaMask/clearNotices

Clear notices

app/scripts/lib/nodeify.js

@@ -1,5 +1,5 @@
 const promiseToCallback = require('promise-to-callback')
-const noop = function () {}
+const callbackNoop = function (err) { if (err) throw err }
 
 /**
  * A generator that returns a function which, when passed a promise, can treat that promise as a node style callback.
@@ -11,6 +11,7 @@ const noop = function () {}
  */
 module.exports = function nodeify (fn, context) {
   return function () {
+    // parse arguments
     const args = [].slice.call(arguments)
     const lastArg = args[args.length - 1]
     const lastArgIsCallback = typeof lastArg === 'function'
@@ -19,8 +20,16 @@ module.exports = function nodeify (fn, context) {
       callback = lastArg
       args.pop()
     } else {
-      callback = noop
+      callback = callbackNoop
     }
-    promiseToCallback(fn.apply(context, args))(callback)
+    // call the provided function and ensure result is a promise
+    let result
+    try {
+      result = Promise.resolve(fn.apply(context, args))
+    } catch (err) {
+      result = Promise.reject(err)
+    }
+    // wire up promise resolution to callback
+    promiseToCallback(result)(callback)
   }
 }

app/scripts/metamask-controller.js

@@ -473,6 +473,7 @@ module.exports = class MetamaskController extends EventEmitter {
       // notices
       checkNotices: noticeController.updateNoticesList.bind(noticeController),
       markNoticeRead: noticeController.markNoticeRead.bind(noticeController),
+      markAllNoticesRead: nodeify(noticeController.markAllNoticesRead, noticeController),
 
       approveProviderRequest: providerApprovalController.approveProviderRequest.bind(providerApprovalController),
       clearApprovedOrigins: providerApprovalController.clearApprovedOrigins.bind(providerApprovalController),

app/scripts/notice-controller.js

@@ -58,6 +58,18 @@ module.exports = class NoticeController extends EventEmitter {
     }
   }
 
+  markAllNoticesRead () {
+    const noticeList = this.getNoticesList()
+    noticeList.forEach(notice => {
+      notice.read = true
+      notice.body = ''
+    })
+    this.setNoticesList(noticeList)
+    const latestNotice = this.getNextUnreadNotice()
+    return latestNotice
+  }
+
+
   async updateNoticesList () {
     const newNotices = await this._retrieveNoticeData()
     const oldNotices = this.getNoticesList()

test/unit/app/controllers/notice-controller-test.js

@@ -39,6 +39,31 @@ describe('notice-controller', function () {
       })
     })
 
+    describe('#markAllNoticesRead', () => {
+      it('marks all notices read', async () => {
+        const testList = [{
+          id: 0,
+          read: false,
+          title: 'Notice 1',
+        }, {
+          id: 1,
+          read: false,
+          title: 'Notice 2',
+        }, {
+          id: 2,
+          read: false,
+          title: 'Notice 3',
+        }]
+
+        noticeController.setNoticesList(testList)
+
+        noticeController.markAllNoticesRead()
+
+        const unreadNotices = noticeController.getUnreadNotices()
+        assert.equal(unreadNotices.length, 0)
+      })
+    })
+
     describe('#getNextUnreadNotice', function () {
       it('should retrieve the latest unread notice', function (done) {
         var testList = [

test/unit/app/nodeify-test.js

@@ -2,16 +2,16 @@ const assert = require('assert')
 const nodeify = require('../../../app/scripts/lib/nodeify')
 
 describe('nodeify', function () {
-  var obj = {
+  const obj = {
     foo: 'bar',
     promiseFunc: function (a) {
-      var solution = this.foo + a
+      const solution = this.foo + a
       return Promise.resolve(solution)
     },
   }
 
   it('should retain original context', function (done) {
-    var nodified = nodeify(obj.promiseFunc, obj)
+    const nodified = nodeify(obj.promiseFunc, obj)
     nodified('baz', function (err, res) {
       if (!err) {
         assert.equal(res, 'barbaz')
@@ -22,7 +22,7 @@ describe('nodeify', function () {
     })
   })
 
-  it('should allow the last argument to not be a function', function (done) {
+  it('no callback - should allow the last argument to not be a function', function (done) {
     const nodified = nodeify(obj.promiseFunc, obj)
     try {
       nodified('baz')
@@ -31,4 +31,45 @@ describe('nodeify', function () {
       done(new Error('should not have thrown if the last argument is not a function'))
     }
   })
+
+  it('no callback - should asyncly throw an error if underlying function does', function (done) {
+    const nodified = nodeify(async () => { throw new Error('boom!') }, obj)
+    process.prependOnceListener('uncaughtException', function (err) {
+      assert.ok(err, 'got expected error')
+      assert.ok(err.message.includes('boom!'), 'got expected error message')
+      done()
+    })
+    try {
+      nodified('baz')
+    } catch (err) {
+      done(new Error('should not have thrown an error synchronously'))
+    }
+  })
+
+  it('sync functions - returns value', function (done) {
+    const nodified = nodeify(() => 42)
+    try {
+      nodified((err, result) => {
+        if (err) return done(new Error(`should not have thrown any error: ${err.message}`))
+        assert.equal(42, result, 'got expected result')
+      })
+      done()
+    } catch (err) {
+      done(new Error(`should not have thrown any error: ${err.message}`))
+    }
+  })
+
+  it('sync functions - handles errors', function (done) {
+    const nodified = nodeify(() => { throw new Error('boom!') })
+    try {
+      nodified((err, result) => {
+        if (result) return done(new Error('should not have returned any result'))
+        assert.ok(err, 'got expected error')
+        assert.ok(err.message.includes('boom!'), 'got expected error message')
+      })
+      done()
+    } catch (err) {
+      done(new Error(`should not have thrown any error: ${err.message}`))
+    }
+  })
 })

test/unit/ui/app/actions.spec.js

@@ -1308,6 +1308,29 @@ describe('Actions', () => {
     })
   })
 
+  describe('#setCompletedOnboarding', () => {
+    let markAllNoticesReadSpy, completeOnboardingSpy
+
+    beforeEach(() => {
+      markAllNoticesReadSpy = sinon.stub(background, 'markAllNoticesRead')
+      markAllNoticesReadSpy.callsFake(cb => cb())
+      completeOnboardingSpy = sinon.stub(background, 'completeOnboarding')
+      completeOnboardingSpy.callsFake(cb => cb())
+    })
+
+    after(() => {
+      markAllNoticesReadSpy.restore()
+      completeOnboardingSpy.restore()
+    })
+
+    it('completing onboarding marks all notices as read', async () => {
+      const store = mockStore()
+      await store.dispatch(actions.setCompletedOnboarding())
+      assert.equal(markAllNoticesReadSpy.callCount, 1)
+      assert.equal(completeOnboardingSpy.callCount, 1)
+    })
+  })
+
   describe('#updateNetworkNonce', () => {
     let getTransactionCountSpy
 

ui/app/store/actions.js

@@ -2488,21 +2488,27 @@ function setShowFiatConversionOnTestnetsPreference (value) {
 }
 
 function setCompletedOnboarding () {
-  return dispatch => {
+  return async dispatch => {
     dispatch(actions.showLoadingIndication())
-    return new Promise((resolve, reject) => {
-      background.completeOnboarding(err => {
-        dispatch(actions.hideLoadingIndication())
 
-        if (err) {
+    try {
+      await pify(background.markAllNoticesRead).call(background)
+    } catch (err) {
       dispatch(actions.displayWarning(err.message))
-          return reject(err)
+      throw err
+    }
+
+    dispatch(actions.clearNotices())
+
+    try {
+      await pify(background.completeOnboarding).call(background)
+    } catch (err) {
+      dispatch(actions.displayWarning(err.message))
+      throw err
     }
 
     dispatch(actions.completeOnboarding())
-        resolve()
+    dispatch(actions.hideLoadingIndication())
-      })
-    })
   }
 }