Merge pull request #228 from whiteout-io/dev/WO-786

[WO-786] Select inbox after onConnect

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') {
+        uid = parseInt(uid, 10);
-            // no uid specified in url... select no message
+        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
         }));
     });
 

src/js/controller/app/navigation.js

@@ -35,15 +35,36 @@ 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('uid', null); // close the read mode
-        $location.search('folder', folderIndex);
+        $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]
+        // 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 +90,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 +101,6 @@ var NavigationCtrl = function($scope, $location, account, email, outbox, notific
             type: config.outboxMailboxType
         });
         ob.count = count;
-        $scope.$apply();
 
         email.refreshFolder({
             folder: ob
@@ -111,20 +126,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 +135,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.navigate(0);
-            $scope.$apply();
         }
     });
 
@@ -155,18 +155,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;

src/js/email/account.js

@@ -137,10 +137,12 @@ 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
         return;
@@ -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) {

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({
+            self.openFolder({
-                path: inbox.path
+                folder: inbox
-            }, callback);
+            }, 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();
+            });
         });
     });
 };

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;

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': {