From 49cadecd2d55fe522e8305abdfd7038eb207b39d Mon Sep 17 00:00:00 2001 From: Felix Hammerl Date: Mon, 28 Apr 2014 18:09:51 +0200 Subject: [PATCH] [WO-338] add notification for incoming unread mails --- package.json | 4 +- src/js/app-controller.js | 2 +- src/js/controller/mail-list.js | 130 ++++++++++++--------- src/js/controller/navigation.js | 4 +- src/js/dao/email-dao.js | 21 ++-- src/js/dao/email-sync.js | 1 + src/js/util/notification.js | 1 + test/new-unit/email-dao-test.js | 13 --- test/new-unit/email-sync-test.js | 15 ++- test/new-unit/mail-list-ctrl-test.js | 163 ++++++++++++++++----------- 10 files changed, 206 insertions(+), 148 deletions(-) diff --git a/package.json b/package.json index a94c883..8907844 100644 --- a/package.json +++ b/package.json @@ -11,7 +11,7 @@ }, "dependencies": { "crypto-lib": "https://github.com/whiteout-io/crypto-lib/tarball/v0.1.1", - "imap-client": "https://github.com/whiteout-io/imap-client/tarball/v0.2.4", + "imap-client": "https://github.com/whiteout-io/imap-client/tarball/dev/wo-338", "mailreader": "https://github.com/whiteout-io/mailreader/tarball/v0.2.2", "pgpmailer": "https://github.com/whiteout-io/pgpmailer/tarball/v0.2.2", "pgpbuilder": "https://github.com/whiteout-io/pgpbuilder/tarball/v0.2.3", @@ -36,4 +36,4 @@ "grunt-contrib-compress": "~0.5.2", "grunt-node-webkit-builder": "~0.1.17" } -} \ No newline at end of file +} diff --git a/src/js/app-controller.js b/src/js/app-controller.js index 97f6f97..844af7c 100644 --- a/src/js/app-controller.js +++ b/src/js/app-controller.js @@ -72,7 +72,7 @@ define(function(require) { self._keychain = keychain = new KeychainDAO(lawnchairDao, pubkeyDao); self._crypto = pgp = new PGP(); self._pgpbuilder = pgpbuilder = new PgpBuilder(); - emailSync = new EmailSync(keychain, userStorage); + self._emailSync = emailSync = new EmailSync(keychain, userStorage); self._emailDao = emailDao = new EmailDAO(keychain, pgp, userStorage, pgpbuilder, mailreader, emailSync); self._outboxBo = new OutboxBO(emailDao, keychain, userStorage); self._updateHandler = new UpdateHandler(appConfigStore, userStorage); diff --git a/src/js/controller/mail-list.js b/src/js/controller/mail-list.js index 3000861..8953aaf 100644 --- a/src/js/controller/mail-list.js +++ b/src/js/controller/mail-list.js @@ -5,9 +5,8 @@ define(function(require) { _ = require('underscore'), appController = require('js/app-controller'), IScroll = require('iscroll'), - str = require('js/app-config').string, notification = require('js/util/notification'), - emailDao, outboxBo; + emailDao, outboxBo, emailSync; var MailListCtrl = function($scope, $timeout) { // @@ -16,18 +15,58 @@ define(function(require) { emailDao = appController._emailDao; outboxBo = appController._outboxBo; + emailSync = appController._emailSync; + + emailDao.onNeedsSync = function(error, folder) { + if (error) { + $scope.onError(error); + return; + } + + $scope.synchronize({ + folder: folder + }); + }; + + emailSync.onIncomingMessage = function(msgs) { + var popupId, popupTitle, popupMessage, unreadMsgs; + + unreadMsgs = msgs.filter(function(msg) { + return msg.unread; + }); + + if (unreadMsgs.length === 0) { + return; + } + + popupId = '' + unreadMsgs[0].uid; + if (unreadMsgs.length > 1) { + popupTitle = unreadMsgs.length + ' new messages'; + popupMessage = _.pluck(unreadMsgs, 'subject').join('\n'); + } else { + popupTitle = unreadMsgs[0].from[0].name || unreadMsgs[0].from[0].address; + popupMessage = unreadMsgs[0].subject; + } + + notification.create({ + id: popupId, + title: popupTitle, + message: popupMessage + }); + }; + + notification.setOnClickedListener(function(uidString) { + var uid = parseInt(uidString, 10); + + if (isNaN(uid)) { + return; + } + + $scope.select(_.findWhere(currentFolder().messages, { + uid: uid + })); + }); - // push handler - if (emailDao) { - emailDao.onIncomingMessage = function(email) { - // sync - $scope.synchronize(function() { - // show notification - notificationForEmail(email); - }); - }; - notification.setOnClickedListener(notificationClicked); - } // // scope functions @@ -35,7 +74,7 @@ define(function(require) { $scope.getBody = function(email) { emailDao.getBody({ - folder: getFolder().path, + folder: currentFolder().path, message: email }, function(err) { if (err && err.code !== 42) { @@ -93,17 +132,20 @@ define(function(require) { /** * Synchronize the selected imap folder to local storage */ - $scope.synchronize = function(callback) { + $scope.synchronize = function(options) { updateStatus('Syncing ...'); + options = options || {}; + options.folder = options.folder || currentFolder().path; + // let email dao handle sync transparently - if ($scope.state.nav.currentFolder.type === 'Outbox') { + if (currentFolder().type === 'Outbox') { emailDao.syncOutbox({ - folder: getFolder().path + folder: currentFolder().path }, done); } else { emailDao.sync({ - folder: getFolder().path + folder: options.folder || currentFolder().path }, done); } @@ -127,18 +169,18 @@ define(function(require) { return; } - // sort emails - selectFirstMessage(); // display last update updateStatus('Last update: ', new Date()); + + // do not change the selection if we just updated another folder in the background + if (currentFolder().path === options.folder) { + selectFirstMessage(); + } + $scope.$apply(); // fetch visible bodies at the end of a successful sync $scope.loadVisibleBodies(); - - if (callback) { - callback(); - } } }; @@ -150,7 +192,7 @@ define(function(require) { return; } - if (getFolder().type === 'Outbox') { + if (currentFolder().type === 'Outbox') { $scope.onError({ errMsg: 'Deleting messages from the outbox is not yet supported.' }); @@ -161,18 +203,18 @@ define(function(require) { $scope.synchronize(); function removeAndShowNext() { - var index = getFolder().messages.indexOf(email); + var index = currentFolder().messages.indexOf(email); // show the next mail - if (getFolder().messages.length > 1) { + if (currentFolder().messages.length > 1) { // if we're about to delete the last entry of the array, show the previous (i.e. the one below in the list), // otherwise show the next one (i.e. the one above in the list) - $scope.select(_.last(getFolder().messages) === email ? getFolder().messages[index - 1] : getFolder().messages[index + 1]); + $scope.select(_.last(currentFolder().messages) === email ? currentFolder().messages[index - 1] : currentFolder().messages[index + 1]); } else { // if we have only one email in the array, show nothing $scope.select(); $scope.state.mailList.selected = undefined; } - getFolder().messages.splice(index, 1); + currentFolder().messages.splice(index, 1); } }; @@ -190,14 +232,14 @@ define(function(require) { * List emails from folder when user changes folder */ $scope._stopWatchTask = $scope.$watch('state.nav.currentFolder', function() { - if (!getFolder()) { + if (!currentFolder()) { return; } // development... display dummy mail objects if (!window.chrome || !chrome.identity) { updateStatus('Last update: ', new Date()); - getFolder().messages = createDummyMails(); + currentFolder().messages = createDummyMails(); selectFirstMessage(); return; } @@ -229,30 +271,6 @@ define(function(require) { // helper functions // - function notificationClicked(uidString) { - var email, uid = parseInt(uidString, 10); - - if (isNaN(uid)) { - return; - } - - email = _.findWhere(getFolder().messages, { - uid: uid - }); - - if (email) { - $scope.select(email); - } - } - - function notificationForEmail(email) { - notification.create({ - id: '' + email.uid, - title: email.from[0].name || email.from[0].address, - message: email.subject.replace(str.subjectPrefix, '') - }, function() {}); - } - function updateStatus(lbl, time) { $scope.lastUpdateLbl = lbl; $scope.lastUpdate = (time) ? time : ''; @@ -272,7 +290,7 @@ define(function(require) { } } - function getFolder() { + function currentFolder() { return $scope.state.nav.currentFolder; } }; diff --git a/src/js/controller/navigation.js b/src/js/controller/navigation.js index 44510f1..e97e42b 100644 --- a/src/js/controller/navigation.js +++ b/src/js/controller/navigation.js @@ -59,6 +59,7 @@ define(function(require) { // init folders initFolders(); + // select inbox as the current folder on init if ($scope.account.folders && $scope.account.folders.length > 0) { $scope.openFolder($scope.account.folders[0]); @@ -89,8 +90,7 @@ define(function(require) { outboxBo.onSent = sentNotification; // start checking outbox periodically outboxBo.startChecking($scope.onOutboxUpdate); - // make function available globally for write controller - $scope.emptyOutbox = outboxBo._processOutbox.bind(outboxBo); + return; } diff --git a/src/js/dao/email-dao.js b/src/js/dao/email-dao.js index 6704f0b..d98e7f1 100644 --- a/src/js/dao/email-dao.js +++ b/src/js/dao/email-dao.js @@ -86,13 +86,6 @@ define(function(require) { self._imapClient = options.imapClient; self._pgpMailer = options.pgpMailer; - // delegation-esque pattern to mitigate between node-style events and plain js - self._imapClient.onIncomingMessage = function(message) { - if (typeof self.onIncomingMessage === 'function') { - self.onIncomingMessage(message); - } - }; - // notify emailSync self._emailSync.onConnect({ imapClient: self._imapClient @@ -122,6 +115,20 @@ define(function(require) { return; } + var inbox = _.findWhere(folders, { + type: 'Inbox' + }); + + if (inbox) { + self._imapClient.listenForChanges({ + path: inbox.path + },function(error, path) { + if (typeof self.onNeedsSync === 'function') { + self.onNeedsSync(error, path); + } + }); + } + self._account.folders = folders; callback(); diff --git a/src/js/dao/email-sync.js b/src/js/dao/email-sync.js index fc9c29a..374c730 100644 --- a/src/js/dao/email-sync.js +++ b/src/js/dao/email-sync.js @@ -448,6 +448,7 @@ define(function(require) { // if persisting worked, add them to the messages array folder.messages = folder.messages.concat(messages); + self.onIncomingMessage(messages); doDeltaF4(); }); } diff --git a/src/js/util/notification.js b/src/js/util/notification.js index a5715f4..eec10aa 100644 --- a/src/js/util/notification.js +++ b/src/js/util/notification.js @@ -6,6 +6,7 @@ define(function(require) { var self = {}; self.create = function(options, callback) { + callback = callback || function() {}; if (window.chrome && chrome.notifications) { chrome.notifications.create(options.id, { type: 'basic', diff --git a/test/new-unit/email-dao-test.js b/test/new-unit/email-dao-test.js index 56eeca8..c9e5563 100644 --- a/test/new-unit/email-dao-test.js +++ b/test/new-unit/email-dao-test.js @@ -159,19 +159,6 @@ define(function(require) { }); }); - describe('push', function() { - it('should work', function(done) { - var o = {}; - - dao.onIncomingMessage = function(obj) { - expect(obj).to.equal(o); - done(); - }; - - dao._imapClient.onIncomingMessage(o); - }); - }); - describe('init', function() { beforeEach(function() { delete dao._account; diff --git a/test/new-unit/email-sync-test.js b/test/new-unit/email-sync-test.js index 340ab67..dc0f94d 100644 --- a/test/new-unit/email-sync-test.js +++ b/test/new-unit/email-sync-test.js @@ -800,7 +800,7 @@ define(function(require) { }); it('should fetch messages downstream from the remote', function(done) { - var invocations, folder, localListStub, imapSearchStub, localStoreStub, imapListMessagesStub; + var invocations, folder, localListStub, imapSearchStub, localStoreStub, imapListMessagesStub, incomingMessagesCalled; invocations = 0; folder = 'FOLDAAAA'; @@ -842,6 +842,13 @@ define(function(require) { emails: [dummyEncryptedMail] }).yields(); + incomingMessagesCalled = false; + emailSync.onIncomingMessage = function(msgs) { + incomingMessagesCalled = true; + expect(msgs).to.not.be.empty; + }; + + emailSync.sync({ folder: folder }, function(err) { @@ -860,6 +867,8 @@ define(function(require) { expect(localListStub.calledOnce).to.be.true; expect(imapSearchStub.calledThrice).to.be.true; expect(localStoreStub.calledOnce).to.be.true; + expect(incomingMessagesCalled).to.be.true; + done(); }); }); @@ -1076,6 +1085,8 @@ define(function(require) { emails: [verificationMail] }).yields(); + emailSync.onIncomingMessage = function() {}; + emailSync.sync({ folder: folder }, function(err) { @@ -1146,6 +1157,8 @@ define(function(require) { }); imapDeleteStub = sinon.stub(emailSync, '_imapDeleteMessage').yields({}); + emailSync.onIncomingMessage = function() {}; + emailSync.sync({ folder: folder }, function(err) { diff --git a/test/new-unit/mail-list-ctrl-test.js b/test/new-unit/mail-list-ctrl-test.js index cd0d213..deb52ed 100644 --- a/test/new-unit/mail-list-ctrl-test.js +++ b/test/new-unit/mail-list-ctrl-test.js @@ -6,35 +6,26 @@ define(function(require) { mocks = require('angularMocks'), MailListCtrl = require('js/controller/mail-list'), EmailDAO = require('js/dao/email-dao'), + EmailSync = require('js/dao/email-sync'), DeviceStorageDAO = require('js/dao/devicestorage-dao'), KeychainDAO = require('js/dao/keychain-dao'), - appController = require('js/app-controller'); + appController = require('js/app-controller'), + notification = require('js/util/notification'); chai.Assertion.includeStack = true; describe('Mail List controller unit test', function() { - var scope, ctrl, origEmailDao, emailDaoMock, keychainMock, deviceStorageMock, + var scope, ctrl, origEmailDao, origEmailSync, emailDaoMock, emailSyncMock, keychainMock, deviceStorageMock, emailAddress, notificationClickedHandler, emails, - hasChrome, hasNotifications, hasSocket, hasRuntime, hasIdentity; + hasChrome, hasSocket, hasRuntime, hasIdentity; beforeEach(function() { hasChrome = !! window.chrome; - hasNotifications = !! window.chrome.notifications; hasSocket = !! window.chrome.socket; hasIdentity = !! window.chrome.identity; if (!hasChrome) { window.chrome = {}; } - if (!hasNotifications) { - window.chrome.notifications = { - onClicked: { - addListener: function(handler) { - notificationClickedHandler = handler; - } - }, - create: function() {} - }; - } if (!hasSocket) { window.chrome.socket = {}; } @@ -47,6 +38,10 @@ define(function(require) { window.chrome.identity = {}; } + sinon.stub(notification, 'setOnClickedListener', function(func) { + notificationClickedHandler = func; + }); + emails = [{ unread: true }, { @@ -59,8 +54,11 @@ define(function(require) { }; origEmailDao = appController._emailDao; + origEmailSync = appController._emailSync; emailDaoMock = sinon.createStubInstance(EmailDAO); + emailSyncMock = sinon.createStubInstance(EmailSync); appController._emailDao = emailDaoMock; + appController._emailSync = emailSyncMock; emailAddress = 'fred@foo.com'; emailDaoMock._account = { emailAddress: emailAddress, @@ -91,9 +89,8 @@ define(function(require) { }); afterEach(function() { - if (!hasNotifications) { - delete window.chrome.notifications; - } + notification.setOnClickedListener.restore(); + if (!hasSocket) { delete window.chrome.socket; } @@ -109,6 +106,7 @@ define(function(require) { // restore the module appController._emailDao = origEmailDao; + appController._emailSync = origEmailDao; }); describe('scope variables', function() { @@ -117,72 +115,105 @@ define(function(require) { expect(scope.synchronize).to.exist; expect(scope.remove).to.exist; expect(scope.state.mailList).to.exist; - // expect(emailDaoMock.onIncomingMessage).to.exist; }); }); describe('push notification', function() { - it('should focus mail and not mark it read', function(done) { - var uid, mail, currentFolder; - + beforeEach(function() { scope._stopWatchTask(); - - uid = 123; - mail = { - uid: uid, - from: [{ - address: 'asd' - }], - subject: '[whiteout] asdasd', - unread: true - }; - currentFolder = 'asd'; - scope.state.nav = { - currentFolder: currentFolder - }; - scope.state.read = { - toggle: function() {} - }; - scope.emails = [mail]; - emailDaoMock.sync.yieldsAsync(); - window.chrome.notifications.create = function(id, opts) { - expect(id).to.equal('123'); - expect(opts.type).to.equal('basic'); - expect(opts.message).to.equal('asdasd'); - expect(opts.title).to.equal('asd'); - done(); - }; - - emailDaoMock.onIncomingMessage(mail); }); - }); - describe('clicking push notification', function() { - it('should focus mail', function() { - var mail, currentFolder; - - scope._stopWatchTask(); - - mail = { + it('should succeed for single mail', function(done) { + var mail = { uid: 123, from: [{ address: 'asd' }], - subject: '[whiteout] asdasd', + subject: 'this is the subject!', unread: true }; - currentFolder = { - type: 'asd', - messages: [mail] + + 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(); + done(); + }); + + emailSyncMock.onIncomingMessage([mail]); + }); + + it('should succeed for multiple mails', function(done) { + var mails = [{ + uid: 1, + from: [{ + address: 'asd' + }], + subject: 'this is the subject!', + unread: true + }, { + uid: 2, + from: [{ + address: 'qwe' + }], + subject: 'this is the other subject!', + unread: true + }, { + uid: 3, + from: [{ + address: 'qwe' + }], + subject: 'this is the other subject!', + unread: false + }]; + + 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(); + done(); + }); + + emailSyncMock.onIncomingMessage(mails); + }); + + it('should focus mail when clicked', function() { + var mail = { + uid: 123, + from: [{ + address: 'asd' + }], + subject: 'asdasd', + unread: true }; + scope.state.nav = { - currentFolder: currentFolder + 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('synchronize', function() { @@ -200,15 +231,15 @@ define(function(require) { currentFolder: currentFolder }; - var loadVisibleBodiesStub = sinon.stub(scope, 'loadVisibleBodies'); - - scope.synchronize(function() { + var loadVisibleBodiesStub = sinon.stub(scope, 'loadVisibleBodies', function() { expect(scope.state.nav.currentFolder.messages).to.deep.equal(emails); expect(loadVisibleBodiesStub.calledOnce).to.be.true; loadVisibleBodiesStub.restore(); + done(); }); + scope.synchronize(); }); });