From c5192f8ade3f366995dcfbf3b9c76b0920e12d48 Mon Sep 17 00:00:00 2001
From: JC Brand <jc@opkode.com>
Date: Sun, 4 Aug 2013 15:39:46 +0200
Subject: [PATCH] Bugfix. Updates #32

Users without roster contacts didn't have XMPPStatus.initStatus called and
therefore had a default status of offline.

Fixed this by setting the status to online from the start. The fetch command
will get the true status from localStorage (if available).

Made sure that the rest of converse's onConnected code is called in the
callback from fetch (so that we know the user's status is correct).

Updated and fixed tests.
---
 CHANGES.rst      |   1 +
 converse.js      | 139 +++++++++++++++++++++++++----------------------
 spec/MainSpec.js |  17 ++++--
 tests_main.js    |   3 +-
 4 files changed, 90 insertions(+), 70 deletions(-)

diff --git a/CHANGES.rst b/CHANGES.rst
index 1128a00c2..8cf55088b 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -4,6 +4,7 @@ Changelog
 0.5.1 (Unreleased)
 ------------------
 
+- #32: Default status was offline when user didn't have contacts. [jcbrand]
 - Attach panels to the DOM upon initialize. [jcbrand]
 
 0.5.0 (2013-07-30)
diff --git a/converse.js b/converse.js
index f881df61c..d00a495be 100644
--- a/converse.js
+++ b/converse.js
@@ -1046,21 +1046,6 @@
                     this.$el.html(this.template(this.model.toJSON()));
                     this.contactspanel = new converse.ContactsPanel({'$parent': this.$el.find('#controlbox-panes')});
                     this.contactspanel.render();
-                    converse.xmppstatus = new converse.XMPPStatus();
-                    converse.xmppstatus.localStorage = new Backbone.LocalStorage(
-                        hex_sha1('converse.xmppstatus-'+converse.bare_jid));
-                    converse.xmppstatus.fetch({
-                        success: function (xmppstatus, resp) {
-                            if (!xmppstatus.get('fullname')) {
-                                converse.getVCard(
-                                    null, // No 'to' attr when getting one's own vCard
-                                    function (jid, fullname, image, image_type, url) {
-                                        converse.xmppstatus.save({'fullname': fullname});
-                                    }
-                                );
-                            }
-                        }
-                    });
                     converse.xmppstatusview = new converse.XMPPStatusView({'model': converse.xmppstatus});
                     converse.xmppstatusview.render();
                     this.roomspanel = new converse.RoomsPanel({'$parent': this.$el.find('#controlbox-panes')});
@@ -2029,7 +2014,7 @@
                     item;
 
                 if (this.isSelf(bare_jid)) {
-                    if ((converse.connection.jid !== jid)&&(presence_type !== 'unavailabe')) {
+                    if ((converse.connection.jid !== jid)&&(presence_type !== 'unavailable')) {
                         // Another resource has changed it's status, we'll update ours as well.
                         // FIXME: We should ideally differentiate between converse.js using
                         // resources and other resources (i.e Pidgin etc.)
@@ -2127,8 +2112,20 @@
                 this.model.on("destroy", function (item) { this.removeRosterItem(item); }, this);
 
                 this.$el.hide().html(this.template());
-                this.model.fetch({add: true}); // Get the cached roster items from localstorage
-                // XXX: is this necessary? this.initialSort();
+                this.model.fetch({
+                    add: true,
+                    success: function (model, resp, options) {
+                        if (resp.length === 0) {
+                            // The presence stanza is sent out once all
+                            // roster contacts have been added and rendered.
+                            // See RosterView's render method.
+                            //
+                            // If there aren't any roster contacts, we still
+                            // want to send a presence stanza, so we do it here.
+                            converse.xmppstatus.sendPresence();
+                        }
+                    },
+                }); // Get the cached roster items from localstorage
             },
 
             updateChatBox: function (item, changed) {
@@ -2195,9 +2192,8 @@
                         // options where all of the items are offline and now we can show the rosterView
                         item.set('sorted', true);
                         this.initialSort();
-                        this.$el.show(function () {
-                            converse.xmppstatus.initStatus();
-                        });
+                        this.$el.show();
+                        converse.xmppstatus.sendPresence();
                     }
                 }
                 // Hide the headings if there are no contacts under them
@@ -2228,22 +2224,24 @@
         this.XMPPStatus = Backbone.Model.extend({
             initialize: function () {
                 this.set({
-                    'status' : this.get('status'),
-                    'status_message' : this.get('status_message'),
-                    'fullname' : this.get('fullname')
+                    'status' : this.get('status') || 'online',
                 });
-            },
-
-            initStatus: function () {
-                var stat = this.get('status');
-                if (stat === undefined) {
-                    stat = 'online';
-                    this.save({status: stat});
-                }
-                this.sendPresence(stat);
+                this.on('change', $.proxy(function () {
+                    if (this.get('fullname') === undefined) {
+                        converse.getVCard(
+                            null, // No 'to' attr when getting one's own vCard
+                            $.proxy(function (jid, fullname, image, image_type, url) {
+                                this.save({'fullname': fullname});
+                            }, this)
+                        );
+                    }
+                }, this));
             },
 
             sendPresence: function (type) {
+                if (type === undefined) {
+                    type = this.get('status') || 'online';
+                }
                 var status_message = this.get('status_message'),
                     presence;
                 // Most of these presence types are actually not explicitly sent,
@@ -2390,7 +2388,7 @@
                 this.$el.html(this.choose_template());
                 this.$el.find('#fancy-xmpp-status-select')
                         .html(this.status_template({
-                            'status_message': __("I am %1$s", this.getPrettyStatus(chat_status)),
+                            'status_message': this.model.get('status_message') || __("I am %1$s", this.getPrettyStatus(chat_status)),
                             'chat_status': chat_status
                             }));
                 // iterate through all the <option> elements and add option values
@@ -2595,14 +2593,15 @@
             }
         };
 
-        this.onConnected = function (connection) {
-            this.connection = connection;
-            this.connection.xmlInput = function (body) { console.log(body); };
-            this.connection.xmlOutput = function (body) { console.log(body); };
-            this.bare_jid = Strophe.getBareJidFromJid(this.connection.jid);
-            this.domain = Strophe.getDomainFromJid(this.connection.jid);
-            this.features = new this.Features();
+        this.initStatus = function (callback) {
+            this.xmppstatus = new this.XMPPStatus();
+            var id = hex_sha1('converse.xmppstatus-'+this.bare_jid);
+            this.xmppstatus.id = id; // This appears to be necessary for backbone.localStorage
+            this.xmppstatus.localStorage = new Backbone.LocalStorage(id);
+            this.xmppstatus.fetch({success: callback, error: callback});
+        };
 
+        this.initRoster = function () {
             // Set up the roster
             this.roster = new this.RosterItems();
             this.roster.localStorage = new Backbone.LocalStorage(
@@ -2611,33 +2610,45 @@
                 $.proxy(this.roster.rosterHandler, this.roster),
                 null, 'presence', null);
             this.rosterview = new this.RosterView({'model':this.roster});
+        }
 
-            this.chatboxes.onConnected();
-
-            this.connection.addHandler(
-                $.proxy(this.roster.subscribeToSuggestedItems, this.roster),
-                'http://jabber.org/protocol/rosterx', 'message', null);
-
-            this.connection.roster.get($.proxy(function (a) {
+        this.onConnected = function (connection, callback) {
+            this.connection = connection;
+            this.connection.xmlInput = function (body) { console.log(body); };
+            this.connection.xmlOutput = function (body) { console.log(body); };
+            this.bare_jid = Strophe.getBareJidFromJid(this.connection.jid);
+            this.domain = Strophe.getDomainFromJid(this.connection.jid);
+            this.features = new this.Features();
+            this.initStatus($.proxy(function () {
+                this.initRoster();
+                this.chatboxes.onConnected();
                 this.connection.addHandler(
-                        $.proxy(function (presence) {
-                            this.presenceHandler(presence);
-                            return true;
-                        }, this.roster), null, 'presence', null);
-                this.connection.addHandler(
-                        $.proxy(function (message) {
-                            this.chatboxes.messageReceived(message);
-                            return true;
-                        }, this), null, 'message', 'chat');
-            }, this));
+                    $.proxy(this.roster.subscribeToSuggestedItems, this.roster),
+                    'http://jabber.org/protocol/rosterx', 'message', null);
 
-            $(window).on("blur focus", $.proxy(function(e) {
-                if ((this.windowState != e.type) && (e.type == 'focus')) {
-                    converse.clearMsgCounter();
+                this.connection.roster.get($.proxy(function (a) {
+                    this.connection.addHandler(
+                            $.proxy(function (presence) {
+                                this.presenceHandler(presence);
+                                return true;
+                            }, this.roster), null, 'presence', null);
+                    this.connection.addHandler(
+                            $.proxy(function (message) {
+                                this.chatboxes.messageReceived(message);
+                                return true;
+                            }, this), null, 'message', 'chat');
+                }, this));
+                $(window).on("blur focus", $.proxy(function(e) {
+                    if ((this.windowState != e.type) && (e.type == 'focus')) {
+                        converse.clearMsgCounter();
+                    }
+                    this.windowState = e.type;
+                },this));
+                this.giveFeedback(__('Online Contacts'));
+                if (callback) {
+                    callback();
                 }
-                this.windowState = e.type;
-            },this));
-            this.giveFeedback(__('Online Contacts'));
+            }, this));
         };
 
         // This is the end of the initialize method.
diff --git a/spec/MainSpec.js b/spec/MainSpec.js
index ec8bd1efc..11796e61a 100644
--- a/spec/MainSpec.js
+++ b/spec/MainSpec.js
@@ -56,6 +56,12 @@
             it("can be opened by clicking a DOM element with class 'toggle-online-users'", open_controlbox);
 
             describe("The Status Widget", $.proxy(function () {
+                it("shows the user's chat status, which is online by default", $.proxy(function () {
+                    var view = this.xmppstatusview;
+                    expect(view.$el.find('a.choose-xmpp-status').hasClass('online')).toBe(true);
+                    expect(view.$el.find('a.choose-xmpp-status').attr('data-value')).toBe('I am online');
+                }, converse));
+
                 it("can be used to set the current user's chat status", $.proxy(function () {
                     var view = this.xmppstatusview;
                     spyOn(view, 'toggleOptions').andCallThrough();
@@ -64,25 +70,26 @@
                     runs(function () {
                         view.$el.find('a.choose-xmpp-status').click();
                         expect(view.toggleOptions).toHaveBeenCalled();
-                        expect(view.$el.find('a.choose-xmpp-status').hasClass('online')).toBe(false);
                     });
                     waits(250);
                     runs(function () {
                         spyOn(view, 'updateStatusUI').andCallThrough();
                         view.initialize(); // Rebind events for spy
-                        view.$el.find('.dropdown dd ul li a').first().click();
+                        $(view.$el.find('.dropdown dd ul li a')[1]).click();
                         expect(view.setStatus).toHaveBeenCalled();
                     });
                     waits(250);
                     runs($.proxy(function () {
                         expect(view.updateStatusUI).toHaveBeenCalled();
-                        expect(view.$el.find('a.choose-xmpp-status').hasClass('online')).toBe(true);
-                        expect(view.$el.find('a.choose-xmpp-status').attr('data-value')).toBe('I am online');
+                        expect(view.$el.find('a.choose-xmpp-status').hasClass('online')).toBe(false);
+                        expect(view.$el.find('a.choose-xmpp-status').hasClass('dnd')).toBe(true);
+                        expect(view.$el.find('a.choose-xmpp-status').attr('data-value')).toBe('I am busy');
                     }, converse));
                 }, converse));
 
                 it("can be used to set a custom status message", $.proxy(function () {
                     var view = this.xmppstatusview;
+                    this.xmppstatus.save({'status': 'online'});
                     spyOn(view, 'setStatusMessage').andCallThrough();
                     spyOn(view, 'renderStatusChangeForm').andCallThrough();
                     view.delegateEvents(); // We need to rebind all events otherwise our spy won't be called
@@ -120,6 +127,7 @@
                 it("can be added to the roster and they will be sorted alphabetically", $.proxy(function () {
                     var i, t, is_last;
                     spyOn(this.rosterview, 'render').andCallThrough();
+                    spyOn(this.xmppstatus, 'sendPresence');
                     for (i=0; i<pend_names.length; i++) {
                         is_last = i===(pend_names.length-1);
                         this.roster.create({
@@ -133,6 +141,7 @@
                         // the last contact has been added.
                         if (is_last) {
                             expect(this.rosterview.$el.is(':visible')).toEqual(true);
+                            expect(this.xmppstatus.sendPresence).toHaveBeenCalled();
                         } else {
                             expect(this.rosterview.$el.is(':visible')).toEqual(false);
                         }
diff --git a/tests_main.js b/tests_main.js
index b0ef43ecf..428a9fc7c 100644
--- a/tests_main.js
+++ b/tests_main.js
@@ -75,7 +75,6 @@ require([
         auto_subscribe: false,
         animate: false
     });
-    converse.onConnected(mock_connection);
 
     // Jasmine stuff
     var jasmineEnv = jasmine.getEnv();
@@ -93,5 +92,5 @@ require([
         };
         jasmineEnv.updateInterval = 200;
     }
-    jasmineEnv.execute();
+    converse.onConnected(mock_connection, $.proxy(jasmineEnv.execute, jasmineEnv));
 });