[WO-565] Improve notifications

* Introduce 2 sec timeout for sent notifications
* Notify only for new messages in the inbox
* Close pending notes when a msg is marked unread in the inbox

.jshintrc

@@ -19,6 +19,7 @@
 
 	"predef": [
 			"console",
+			"Notification",
 			"importScripts",
 			"process",
 			"Event",

src/js/controller/mail-list.js

@@ -8,7 +8,8 @@ define(function(require) {
         emailDao, outboxBo, keychainDao, searchTimeout, firstSelect;
 
     var INIT_DISPLAY_LEN = 20,
-        SCROLL_DISPLAY_LEN = 10;
+        SCROLL_DISPLAY_LEN = 10,
+        FOLDER_TYPE_INBOX = 'Inbox';
 
     var MailListCtrl = function($scope, $routeParams) {
         //
@@ -19,6 +20,11 @@ define(function(require) {
         outboxBo = appController._outboxBo;
         keychainDao = appController._keychain;
 
+        /**
+         * Gathers unread notifications to be cancelled later
+         */
+        $scope.pendingNotifications = [];
+
         //
         // scope functions
         //
@@ -80,6 +86,13 @@ define(function(require) {
                     return;
                 }
 
+                // let's close pending notifications for unread messages in the inbox
+                if (currentFolder().type === FOLDER_TYPE_INBOX) {
+                    while ($scope.pendingNotifications.length) {
+                        notification.close($scope.pendingNotifications.shift());
+                    }
+                }
+
                 $scope.toggleUnread(email);
             }
         };
@@ -364,7 +377,7 @@ define(function(require) {
         //
 
         (emailDao || {}).onIncomingMessage = function(msgs) {
-            var popupId, popupTitle, popupMessage, unreadMsgs;
+            var note, title, message, unreadMsgs;
 
             unreadMsgs = msgs.filter(function(msg) {
                 return msg.unread;
@@ -374,34 +387,35 @@ define(function(require) {
                 return;
             }
 
-            popupId = '' + unreadMsgs[0].uid;
+            if (unreadMsgs.length === 1) {
-            if (unreadMsgs.length > 1) {
+                title = unreadMsgs[0].from[0].name || unreadMsgs[0].from[0].address;
-                popupTitle = unreadMsgs.length + ' new messages';
+                message = unreadMsgs[0].subject;
-                popupMessage = _.pluck(unreadMsgs, 'subject').join('\n');
             } else {
-                popupTitle = unreadMsgs[0].from[0].name || unreadMsgs[0].from[0].address;
+                title = unreadMsgs.length + ' new messages';
-                popupMessage = unreadMsgs[0].subject;
+                message = _.pluck(unreadMsgs, 'subject').join('\n');
-            }
-
-            notification.create({
-                id: popupId,
-                title: popupTitle,
-                message: popupMessage
-            });
-        };
-
-        notification.setOnClickedListener(function(uidString) {
-            var uid = parseInt(uidString, 10);
-
-            if (isNaN(uid)) {
-                return;
             }
 
+            note = notification.create({
+                title: title,
+                message: message,
+                onClick: function() {
+                    // force toggle into read mode when notification is clicked
                     firstSelect = false;
+
+                    // remove from pending notificatiosn
+                    var index = $scope.pendingNotifications.indexOf(note);
+                    if (index !== -1) {
+                        $scope.pendingNotifications.splice(index, 1);
+                    }
+
+                    // mark message as read
                     $scope.select(_.findWhere(currentFolder().messages, {
-                uid: uid
+                        uid: unreadMsgs[0].uid
                     }));
+                }
             });
+            $scope.pendingNotifications.push(note);
+        };
     };
 
     //

src/js/controller/navigation.js

@@ -106,9 +106,9 @@ define(function(require) {
 
         function sentNotification(email) {
             notification.create({
-                id: 'o' + email.id,
                 title: 'Message sent',
-                message: email.subject
+                message: email.subject,
+                timeout: 2000
             }, function() {});
         }
 

src/js/dao/email-dao.js

@@ -408,7 +408,11 @@ define(function(require) {
 
                     [].unshift.apply(folder.messages, messages); // add the new messages to the folder
                     updateUnreadCount(folder); // update the unread count
-                    self.onIncomingMessage(messages); // notify about new messages
+
+                    // notify about new messages only for the inbox
+                    if (folder.type === FOLDER_TYPE_INBOX) {
+                        self.onIncomingMessage(messages);
+                    }
                     done();
                 });
             }

src/js/util/notification.js

@@ -5,22 +5,52 @@ define(function(require) {
 
     var self = {};
 
-    self.create = function(options, callback) {
+    if (window.Notification) {
-        callback = callback || function() {};
+        self.hasPermission = Notification.permission === "granted";
-        if (window.chrome && chrome.notifications) {
-            chrome.notifications.create(options.id, {
-                type: 'basic',
-                title: options.title,
-                message: options.message,
-                iconUrl: chrome.runtime.getURL(cfg.iconPath)
-            }, callback);
     }
+
+    /**
+     * Creates a notification. Requests permission if not already granted
+     *
+     * @param {String} options.title The notification title
+     * @param {String} options.message The notification message
+     * @param {Number} options.timeout (optional) Timeout when the notification is closed in milliseconds
+     * @param {Function} options.onClick (optional) callback when the notification is clicked
+     * @returns {Notification} A notification instance
+     */
+    self.create = function(options) {
+        options.onClick = options.onClick || function() {};
+
+        if (!window.Notification) {
+            return;
+        }
+
+        if (!self.hasPermission) {
+            // don't wait until callback returns
+            Notification.requestPermission(function(permission) {
+                if (permission === "granted") {
+                    self.hasPermission = true;
+                }
+            });
+        }
+
+        var notification = new Notification(options.title, {
+            body: options.message,
+            icon: cfg.iconPath
+        });
+        notification.onclick = options.onClick;
+
+        if (options.timeout > 0) {
+            setTimeout(function() {
+                notification.close();
+            }, options.timeout);
+        }
+
+        return notification;
     };
 
-    self.setOnClickedListener = function(listener) {
+    self.close = function(notification) {
-        if (window.chrome && chrome.notifications) {
+        notification.close();
-            chrome.notifications.onClicked.addListener(listener);
-        }
     };
 
     return self;

test/integration/email-dao-test.js

@@ -367,11 +367,7 @@ define(function(require) {
                         }, function(err, folder) {
                             expect(err).to.not.exist;
                             expect(folder.exists).to.equal(1);
-                            emailDao.onIncomingMessage = function(messages) {
-                                expect(messages.length).to.equal(1);
-                                expect(messages[0].id).to.equal('b');
                             done();
-                            };
                         });
                     });
                 });

test/unit/email-dao-test.js

@@ -444,6 +444,27 @@ define(function(require) {
                     });
                 });
 
+                it('should not notify for other folders', function(done) {
+                    opts.folder = sentFolder;
+                    
+                    imapListStub.withArgs(opts).yieldsAsync(null, [message]);
+
+                    localStoreStub.withArgs({
+                        folder: sentFolder,
+                        emails: [message]
+                    }).yieldsAsync();
+
+                    dao.fetchMessages(opts, function(err) {
+                        expect(err).to.not.exist;
+                        expect(sentFolder.messages).to.contain(message);
+                        expect(notified).to.be.false;
+                        expect(localStoreStub.calledOnce).to.be.true;
+                        expect(imapListStub.calledOnce).to.be.true;
+
+                        done();
+                    });
+                });
+
                 it('should verify verification mails', function(done) {
                     message.subject = verificationSubject;
 

test/unit/mail-list-ctrl-test.js

@@ -15,7 +15,7 @@ define(function(require) {
 
     describe('Mail List controller unit test', function() {
         var scope, ctrl, origEmailDao, emailDaoMock, keychainMock, deviceStorageMock,
-            emailAddress, notificationClickedHandler, emails,
+            emailAddress, emails,
             hasChrome, hasSocket, hasRuntime, hasIdentity;
 
         beforeEach(function() {
@@ -37,10 +37,6 @@ define(function(require) {
                 window.chrome.identity = {};
             }
 
-            sinon.stub(notification, 'setOnClickedListener', function(func) {
-                notificationClickedHandler = func;
-            });
-
             emails = [{
                 unread: true
             }, {
@@ -86,8 +82,6 @@ define(function(require) {
         });
 
         afterEach(function() {
-            notification.setOnClickedListener.restore();
-
             if (!hasSocket) {
                 delete window.chrome.socket;
             }
@@ -260,6 +254,10 @@ define(function(require) {
                 scope._stopWatchTask();
             });
 
+            afterEach(function() {
+                notification.create.restore();
+            });
+
             it('should succeed for single mail', function(done) {
                 var mail = {
                     uid: 123,
@@ -271,14 +269,21 @@ define(function(require) {
                 };
 
                 sinon.stub(notification, 'create', function(opts) {
-                    expect(opts.id).to.equal('' + mail.uid);
                     expect(opts.title).to.equal(mail.from[0].address);
                     expect(opts.message).to.equal(mail.subject);
 
-                    notification.create.restore();
+                    opts.onClick();
+                    expect(scope.state.mailList.selected).to.equal(mail);
                     done();
                 });
 
+                scope.state.nav = {
+                    currentFolder: {
+                        type: 'asd',
+                        messages: [mail]
+                    }
+                };
+
                 emailDaoMock.onIncomingMessage([mail]);
             });
 
@@ -307,50 +312,23 @@ define(function(require) {
                 }];
 
                 sinon.stub(notification, 'create', function(opts) {
-                    expect(opts.id).to.equal('' + mails[0].uid);
                     expect(opts.title).to.equal('2 new messages');
                     expect(opts.message).to.equal(mails[0].subject + '\n' + mails[1].subject);
 
-                    notification.create.restore();
+                    opts.onClick();
+                    expect(scope.state.mailList.selected).to.equal(mails[0]);
                     done();
                 });
 
+                scope.state.nav = {
+                    currentFolder: {
+                        type: 'asd',
+                        messages: mails
+                    }
+                };
+
                 emailDaoMock.onIncomingMessage(mails);
             });
-
-            it('should focus mail when clicked', function() {
-                var mail = {
-                    uid: 123,
-                    from: [{
-                        address: 'asd'
-                    }],
-                    subject: 'asdasd',
-                    unread: true
-                };
-
-                scope.state.nav = {
-                    currentFolder: {
-                        type: 'asd',
-                        messages: [mail]
-                    }
-                };
-
-                notificationClickedHandler('123');
-                expect(scope.state.mailList.selected).to.equal(mail);
-            });
-
-            it('should not change focus mail when popup id is NaN', function() {
-                scope.state.nav = {
-                    currentFolder: {
-                        type: 'asd',
-                        messages: []
-                    }
-                };
-                var focus = scope.state.mailList.selected = {};
-
-                notificationClickedHandler('');
-                expect(scope.state.mailList.selected).to.equal(focus);
-            });
         });
 
         describe('getBody', function() {
@@ -368,6 +346,9 @@ define(function(require) {
 
         describe('select', function() {
             it('should decrypt, focus mark an unread mail as read', function() {
+                scope.pendingNotifications = ['asd'];
+                sinon.stub(notification, 'close');
+
                 var mail = {
                     from: [{
                         address: 'asd'
@@ -377,7 +358,7 @@ define(function(require) {
                 scope.state = {
                     nav: {
                         currentFolder: {
-                            type: 'asd'
+                            type: 'Inbox'
                         }
                     },
                     mailList: {},
@@ -393,6 +374,10 @@ define(function(require) {
                 expect(emailDaoMock.decryptBody.calledOnce).to.be.true;
                 expect(keychainMock.refreshKeyForUserId.calledOnce).to.be.true;
                 expect(scope.state.mailList.selected).to.equal(mail);
+                expect(notification.close.calledWith('asd')).to.be.true;
+                expect(notification.close.calledOnce).to.be.true;
+
+                notification.close.restore();
             });
 
             it('should decrypt and focus a read mail', function() {