From c3efeb11322a915f94e5018d147fc1811fcbfad8 Mon Sep 17 00:00:00 2001 From: Felix Hammerl Date: Thu, 11 Dec 2014 15:02:08 +0100 Subject: [PATCH 1/2] [WO-786] Select inbox after onConnect --- src/js/controller/app/mail-list.js | 8 ++- src/js/controller/app/navigation.js | 69 ++++++++++--------- src/js/email/account.js | 8 ++- src/js/email/email.js | 22 ++++-- .../controller/app/navigation-ctrl-test.js | 11 --- test/unit/email/email-dao-test.js | 4 +- 6 files changed, 66 insertions(+), 56 deletions(-) diff --git a/src/js/controller/app/mail-list.js b/src/js/controller/app/mail-list.js index 4de5e6c..a3ec8d6 100644 --- a/src/js/controller/app/mail-list.js +++ b/src/js/controller/app/mail-list.js @@ -37,14 +37,16 @@ var MailListCtrl = function($scope, $timeout, $location, $filter, status, notifi $scope.loc = $location; $scope.$watch('(loc.search()).uid', function(uid) { - if (typeof uid === 'undefined') { - // no uid specified in url... select no message + uid = parseInt(uid, 10); + if (isNaN(uid)) { + // no (or nonsensical) uid specified in url... select no message $scope.select(); return; } + // select the message specified by the uid in the url $scope.select(_.findWhere(currentFolder().messages, { - uid: typeof uid === 'string' ? parseInt(uid, 10) : uid + uid: uid })); }); diff --git a/src/js/controller/app/navigation.js b/src/js/controller/app/navigation.js index a122186..ec64c38 100644 --- a/src/js/controller/app/navigation.js +++ b/src/js/controller/app/navigation.js @@ -35,15 +35,38 @@ var NavigationCtrl = function($scope, $location, account, email, outbox, notific // url/history handling // + $scope.loc = $location; + /** * Close read mode and go to folder */ $scope.navigate = function(folderIndex) { - $location.search('uid', null); - $location.search('folder', folderIndex); + $location.search('uid', null); // close the read mode + $location.search('folder', folderIndex); // open the n-th folder }; - $scope.loc = $location; + // folder index url watcher + $scope.$watch('(loc.search()).folder', function(folderIndex) { + if (!$scope.account.folders || !$scope.account.folders.length) { + // there's no folder to navigate to + return; + } + + // normalize folder index to [0 ; $scope.account.folders.length - 1] + folderIndex = parseInt(folderIndex, 10); + if (isNaN(folderIndex) || folderIndex < 0 || folderIndex > ($scope.account.folders.length - 1)) { + // array index out of bounds or nonsensical data + $location.search('folder', 0); + return; + } + + // navigate to folder[folderIndex] + if ($scope.account.folders && $scope.account.folders.length > folderIndex) { + // navigate to the selected folder index + $scope.state.nav.currentFolder = $scope.account.folders[folderIndex]; + $scope.state.nav.toggle(false); + } + }); // nav open/close state url watcher $scope.$watch('(loc.search()).nav', function(open) { @@ -69,11 +92,6 @@ var NavigationCtrl = function($scope, $location, account, email, outbox, notific // scope functions // - $scope.openFolder = function(folder) { - $scope.state.nav.currentFolder = folder; - $scope.state.nav.toggle(false); - }; - $scope.onOutboxUpdate = function(err, count) { if (err) { dialog.error(err); @@ -85,7 +103,6 @@ var NavigationCtrl = function($scope, $location, account, email, outbox, notific type: config.outboxMailboxType }); ob.count = count; - $scope.$apply(); email.refreshFolder({ folder: ob @@ -111,20 +128,6 @@ var NavigationCtrl = function($scope, $location, account, email, outbox, notific // init folders initializeFolders(); - // folder index url watcher - $scope.$watch('(loc.search()).folder', function(folderIndex) { - if (typeof folderIndex === 'undefined') { - $location.search('folder', 0); // navigate to inbox by default - return; - } - // select current folder - folderIndex = typeof folderIndex === 'string' ? parseInt(folderIndex, 10) : folderIndex; - if ($scope.account.folders && $scope.account.folders.length > folderIndex) { - // navigate to the selected folder index - $scope.openFolder($scope.account.folders[folderIndex]); - } - }); - // connect imap/smtp clients on first startup account.onConnect(function(err) { if (err) { @@ -134,8 +137,7 @@ var NavigationCtrl = function($scope, $location, account, email, outbox, notific // select inbox if not yet selected if (!$scope.state.nav.currentFolder) { - $scope.openFolder($scope.account.folders[0]); - $scope.$apply(); + $location.search('folder', 0); } }); @@ -155,18 +157,17 @@ var NavigationCtrl = function($scope, $location, account, email, outbox, notific $scope.$root.account = account.list()[0]; // set notificatio handler for sent messages - outbox.onSent = sentNotification; + outbox.onSent = function(message) { + notification.create({ + title: 'Message sent', + message: message.subject, + timeout: NOTIFICATION_SENT_TIMEOUT + }, function() {}); + }; + // start checking outbox periodically outbox.startChecking($scope.onOutboxUpdate); } - - function sentNotification(message) { - notification.create({ - title: 'Message sent', - message: message.subject, - timeout: NOTIFICATION_SENT_TIMEOUT - }, function() {}); - } }; module.exports = NavigationCtrl; \ No newline at end of file diff --git a/src/js/email/account.js b/src/js/email/account.js index f31b494..8287821 100644 --- a/src/js/email/account.js +++ b/src/js/email/account.js @@ -137,9 +137,11 @@ Account.prototype.isOnline = function() { /** * Event that is called when the user agent goes online. This create new instances of the imap-client and pgp-mailer and connects to the mail server. */ -Account.prototype.onConnect = function() { +Account.prototype.onConnect = function(callback) { var self = this; var config = self._appConfig.config; + + callback = callback || self._dialog.error; if (!self.isOnline() || !self._emailDao || !self._emailDao._account) { // prevent connection infinite loop @@ -148,7 +150,7 @@ Account.prototype.onConnect = function() { self._auth.getCredentials(function(err, credentials) { if (err) { - self._dialog.error(err); + callback(err); return; } @@ -176,7 +178,7 @@ Account.prototype.onConnect = function() { imapClient: imapClient, pgpMailer: pgpMailer, ignoreUploadOnSent: self._emailDao.checkIgnoreUploadOnSent(credentials.imap.host) - }, self._dialog.error); + }, callback); } function onConnectionError(error) { diff --git a/src/js/email/email.js b/src/js/email/email.js index 9c3c0c5..cacb750 100644 --- a/src/js/email/email.js +++ b/src/js/email/email.js @@ -1197,18 +1197,32 @@ Email.prototype.onConnect = function(options, callback) { // set status to online after setting cache to prevent race condition self._account.online = true; - // set up the imap client to listen for changes in the inbox + // by default, select the inbox (if there is one) after connecting the imap client. + // this avoids race conditions between the listening imap connection and the one where the work is done var inbox = _.findWhere(self._account.folders, { type: FOLDER_TYPE_INBOX }); if (!inbox) { + // if there is no inbox, that's ok, too return callback(); } - self._imapClient.listenForChanges({ - path: inbox.path - }, callback); + self.openFolder({ + folder: inbox + }, function(err) { + if (err) { + callback(err); + return; + } + + // set up the imap client to listen for changes in the inbox + self._imapClient.listenForChanges({ + path: inbox.path + }, function() {}); + + callback(); + }); }); }); }; diff --git a/test/unit/controller/app/navigation-ctrl-test.js b/test/unit/controller/app/navigation-ctrl-test.js index 18f3d86..4db6b3b 100644 --- a/test/unit/controller/app/navigation-ctrl-test.js +++ b/test/unit/controller/app/navigation-ctrl-test.js @@ -57,7 +57,6 @@ describe('Navigation Controller unit test', function() { expect(scope.state).to.exist; expect(scope.state.lightbox).to.be.undefined; expect(scope.account.folders).to.not.be.empty; - expect(scope.openFolder).to.exist; }); }); @@ -71,16 +70,6 @@ describe('Navigation Controller unit test', function() { }); }); - describe('open folder', function() { - it('should work', function() { - scope.state.nav.open = true; - - scope.openFolder('asd'); - expect(scope.state.nav.currentFolder).to.equal('asd'); - expect(scope.state.nav.open).to.be.false; - }); - }); - describe('empty outbox', function() { it('should work', function() { var callback; diff --git a/test/unit/email/email-dao-test.js b/test/unit/email/email-dao-test.js index 08fbee6..ba6c443 100644 --- a/test/unit/email/email-dao-test.js +++ b/test/unit/email/email-dao-test.js @@ -1847,7 +1847,8 @@ describe('Email DAO unit tests', function() { modseq: '123' }]; imapClientStub.login.yieldsAsync(); - imapClientStub.listenForChanges.yieldsAsync(); + imapClientStub.selectMailbox.yields(); + imapClientStub.listenForChanges.yields(); initFoldersStub.yieldsAsync(); dao.onConnect({ @@ -1858,6 +1859,7 @@ describe('Email DAO unit tests', function() { expect(err).to.not.exist; expect(dao.ignoreUploadOnSent).to.be.false; expect(imapClientStub.login.calledOnce).to.be.true; + expect(imapClientStub.selectMailbox.calledOnce).to.be.true; expect(initFoldersStub.calledOnce).to.be.true; expect(imapClientStub.mailboxCache).to.deep.equal({ 'INBOX': { From 276a82e5efe87b366528e271d57ffe0b2a0a86b3 Mon Sep 17 00:00:00 2001 From: Tankred Hase Date: Fri, 12 Dec 2014 10:49:16 +0100 Subject: [PATCH 2/2] Review navigation --- src/js/controller/app/navigation.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/js/controller/app/navigation.js b/src/js/controller/app/navigation.js index ec64c38..4a90800 100644 --- a/src/js/controller/app/navigation.js +++ b/src/js/controller/app/navigation.js @@ -61,11 +61,9 @@ var NavigationCtrl = function($scope, $location, account, email, outbox, notific } // navigate to folder[folderIndex] - if ($scope.account.folders && $scope.account.folders.length > folderIndex) { - // navigate to the selected folder index - $scope.state.nav.currentFolder = $scope.account.folders[folderIndex]; - $scope.state.nav.toggle(false); - } + // navigate to the selected folder index + $scope.state.nav.currentFolder = $scope.account.folders[folderIndex]; + $scope.state.nav.toggle(false); }); // nav open/close state url watcher @@ -137,7 +135,7 @@ var NavigationCtrl = function($scope, $location, account, email, outbox, notific // select inbox if not yet selected if (!$scope.state.nav.currentFolder) { - $location.search('folder', 0); + $scope.navigate(0); } });