Refactor cleaner separation between converse-vcard and other plugins

spec/messages.js

@@ -1712,7 +1712,7 @@
                         'message': msg_text
                     });
                     view.model.sendMessage(msg_text);
-                    await u.waitUntil(() => sizzle('.chat-msg .chat-msg__text', chat_content).length === 5);
+                    await u.waitUntil(() => sizzle('.chat-msg .chat-msg__text', chat_content).length === 4, 1000);
                     msg_txt = sizzle('.chat-msg:last .chat-msg__text', chat_content).pop().textContent;
                     expect(msg_txt).toEqual(msg_text);
 

spec/protocol.js

@@ -271,6 +271,7 @@
                         'jid': 'contact@example.org',
                         'subscription': 'to',
                         'name': 'Nicky'});
+
                 _converse.connection._dataRecv(test_utils.createRequest(stanza));
                 // Check that the IQ set was acknowledged.
                 expect(Strophe.serialize(sent_stanza)).toBe( // Strophe adds the xmlns attr (although not in spec)

spec/roster.js

@@ -484,10 +484,10 @@
 
         describe("Pending Contacts", function () {
 
-            function _addContacts (_converse) {
+            async function _addContacts (_converse) {
                 // Must be initialized, so that render is called and documentFragment set up.
-                test_utils.createContacts(_converse, 'pending');
-                test_utils.openControlBox();
+                test_utils.createContacts(_converse, 'pending').openControlBox()
+                await Promise.all(_converse.roster.forEach(contact => u.waitUntil(() => contact.vcard.get('fullname'))));
             }
 
             it("can be collapsed under their own header",
@@ -495,7 +495,7 @@
                     null, ['rosterGroupsFetched'], {},
                     async function (done, _converse) {
 
-                _addContacts(_converse);
+                await _addContacts(_converse);
                 await u.waitUntil(() => sizzle('.roster-group', _converse.rosterview.el).filter(u.isVisible).map(e => e.querySelector('li')).length, 1000);
                 await checkHeaderToggling.apply(
                     _converse,
@@ -667,8 +667,9 @@
         });
 
         describe("Existing Contacts", function () {
-            function _addContacts (_converse) {
+            async function _addContacts (_converse) {
                 test_utils.createContacts(_converse, 'current').openControlBox()
+                await Promise.all(_converse.roster.forEach(contact => u.waitUntil(() => contact.vcard.get('fullname'))));
             }
 
             it("can be collapsed under their own header",
@@ -676,7 +677,7 @@
                     null, ['rosterGroupsFetched'], {},
                     async function (done, _converse) {
 
-                _addContacts(_converse);
+                await _addContacts(_converse);
                 await u.waitUntil(() => sizzle('li', _converse.rosterview.el).filter(u.isVisible).length, 500);
                 await checkHeaderToggling.apply(_converse, [_converse.rosterview.el.querySelector('.roster-group')]);
                 done();
@@ -688,7 +689,7 @@
                     async function (done, _converse) {
 
                 _converse.roster_groups = false;
-                _addContacts(_converse);
+                await _addContacts(_converse);
                 await u.waitUntil(() => sizzle('li', _converse.rosterview.el).filter(u.isVisible).length, 500);
                 _converse.rosterview.el.querySelector('.roster-group a.group-toggle').click();
                 const name = "Romeo Montague";
@@ -710,25 +711,22 @@
                     null, ['rosterGroupsFetched'], {},
                     async function (done, _converse) {
 
-                let i;
                 test_utils.openControlBox();
                 spyOn(_converse.rosterview, 'update').and.callThrough();
-                for (i=0; i<mock.cur_names.length; i++) {
-                    _converse.roster.create({
-                        jid: mock.cur_names[i].replace(/ /g,'.').toLowerCase() + '@montague.lit',
+                await Promise.all(mock.cur_names.map(name => {
+                    const contact = _converse.roster.create({
+                        jid: name.replace(/ /g,'.').toLowerCase() + '@montague.lit',
                         subscription: 'both',
                         ask: null,
-                        fullname: mock.cur_names[i]
+                        fullname: name
                     });
-                    expect(_converse.rosterview.update).toHaveBeenCalled();
-                }
+                    return u.waitUntil(() => contact.vcard.get('fullname'));
+                }));
                 await u.waitUntil(() => sizzle('li', _converse.rosterview.el).length, 600);
                 // Check that they are sorted alphabetically
-                const t = _.reduce(
-                    _converse.rosterview.el.querySelectorAll('.roster-group .current-xmpp-contact.offline a.open-chat'),
-                    (result, value) => (result + value.textContent.trim()), '');
-
-                expect(t).toEqual(mock.cur_names.slice(0,i+1).sort().join(''));
+                const els = sizzle('.roster-group .current-xmpp-contact.offline a.open-chat', _converse.rosterview.el)
+                const t = els.reduce((result, value) => (result + value.textContent.trim()), '');
+                expect(t).toEqual(mock.cur_names.slice(0,mock.cur_names.length).sort().join(''));
                 done();
             }));
 
@@ -737,7 +735,7 @@
                     null, ['rosterGroupsFetched'], {},
                     async function (done, _converse) {
 
-                _addContacts(_converse);
+                await _addContacts(_converse);
                 await u.waitUntil(() => _converse.rosterview.el.querySelectorAll('li').length);
                 const name = mock.cur_names[0];
                 const jid = name.replace(/ /g,'.').toLowerCase() + '@montague.lit';
@@ -796,7 +794,7 @@
                     null, ['rosterGroupsFetched'], {},
                     async function (done, _converse) {
 
-                _addContacts(_converse);
+                await _addContacts(_converse);
                 await u.waitUntil(() => _converse.rosterview.el.querySelectorAll('.roster-group li').length, 700);
                 let jid, t;
                 spyOn(_converse.rosterview, 'update').and.callThrough();
@@ -818,7 +816,7 @@
                     null, ['rosterGroupsFetched'], {},
                     async function (done, _converse) {
 
-                _addContacts(_converse);
+                await _addContacts(_converse);
                 await u.waitUntil(() => sizzle('.roster-group li', _converse.rosterview.el).length, 700);
                 let jid, t;
                 spyOn(_converse.rosterview, 'update').and.callThrough();
@@ -840,7 +838,7 @@
                     null, ['rosterGroupsFetched'], {},
                     async function (done, _converse) {
 
-                _addContacts(_converse);
+                await _addContacts(_converse);
                 await u.waitUntil(() => sizzle('.roster-group li', _converse.rosterview.el).length, 700);
                 let jid, t;
                 spyOn(_converse.rosterview, 'update').and.callThrough();
@@ -862,7 +860,7 @@
                     null, ['rosterGroupsFetched'], {},
                     async function (done, _converse) {
 
-                _addContacts(_converse);
+                await _addContacts(_converse);
                 await u.waitUntil(() => sizzle('.roster-group li', _converse.rosterview.el).length, 700);
                 var jid, t;
                 spyOn(_converse.rosterview, 'update').and.callThrough();
@@ -886,7 +884,7 @@
                     null, ['rosterGroupsFetched'], {},
                     async function (done, _converse) {
 
-                _addContacts(_converse);
+                await _addContacts(_converse);
                 await u.waitUntil(() => sizzle('.roster-group li', _converse.rosterview.el).length, 500)
                 var jid, t;
                 spyOn(_converse.rosterview, 'update').and.callThrough();
@@ -910,7 +908,7 @@
                     null, ['rosterGroupsFetched'], {},
                     async function (done, _converse) {
 
-                _addContacts(_converse);
+                await _addContacts(_converse);
                 await u.waitUntil(() => sizzle('.roster-group li', _converse.rosterview.el).length, 700);
                 let i, jid;
                 for (i=0; i<3; i++) {
@@ -1009,15 +1007,16 @@
                     }
                 };
                 spyOn(_converse.rosterview, 'update').and.callThrough();
-                for (let i=0; i<mock.req_names.length; i++) {
-                    _converse.roster.create({
-                        jid: mock.req_names[i].replace(/ /g,'.').toLowerCase() + '@montague.lit',
+                await Promise.all(mock.req_names.map(name => {
+                    const contact = _converse.roster.create({
+                        jid: name.replace(/ /g,'.').toLowerCase() + '@montague.lit',
                         subscription: 'none',
                         ask: null,
                         requesting: true,
-                        nickname: mock.req_names[i]
+                        nickname: name
                     });
-                }
+                    return u.waitUntil(() => contact.vcard.get('fullname'));
+                }));
                 await u.waitUntil(() => _converse.rosterview.get('Contact requests').el.querySelectorAll('li').length, 700);
                 expect(_converse.rosterview.update).toHaveBeenCalled();
                 // Check that they are sorted alphabetically
@@ -1103,7 +1102,7 @@
                 const contact = _converse.roster.get(jid);
                 spyOn(window, 'confirm').and.returnValue(true);
                 spyOn(contact, 'unauthorize').and.callFake(function () { return contact; });
-                const req_contact = sizzle(".req-contact-name:contains('"+name+"')", _converse.rosterview.el).pop();
+                const req_contact = await u.waitUntil(() => sizzle(".req-contact-name:contains('"+name+"')", _converse.rosterview.el).pop());
                 req_contact.parentElement.parentElement.querySelector('.decline-xmpp-request').click();
                 expect(window.confirm).toHaveBeenCalled();
                 expect(contact.unauthorize).toHaveBeenCalled();
@@ -1202,14 +1201,14 @@
 
                 test_utils.createContacts(_converse, 'all').openControlBox();
                 await u.waitUntil(() => sizzle('.roster-group li', _converse.rosterview.el).length, 700);
-                for (let i=0; i<mock.cur_names.length; i++) {
-                    const name = mock.cur_names[i];
+                await Promise.all(mock.cur_names.map(async name => {
                     const jid = name.replace(/ /g,'.').toLowerCase() + '@montague.lit';
-                    const child = sizzle("li:contains('"+name+"')", _converse.rosterview.el).pop().firstElementChild;
+                    const el = await u.waitUntil(() => sizzle("li:contains('"+name+"')", _converse.rosterview.el).pop());
+                    const child = el.firstElementChild;
                     expect(child.textContent.trim()).toBe(name);
                     expect(child.getAttribute('title')).toContain(name);
                     expect(child.getAttribute('title')).toContain(jid);
-                }
+                }));
                 done();
             }));
         });

src/converse-profile.js

@@ -310,7 +310,8 @@ converse.plugins.add('converse-profile', {
 
         /******************** Event Handlers ********************/
 
-        _converse.api.listen.on('controlBoxPaneInitialized', (view) => {
+        _converse.api.listen.on('controlBoxPaneInitialized', async view => {
+            await _converse.api.waitUntil('statusInitialized');
             _converse.xmppstatusview = new _converse.XMPPStatusView({'model': _converse.xmppstatus});
             view.el.insertAdjacentElement('afterBegin', _converse.xmppstatusview.render().el);
         });

src/headless/converse-core.js

@@ -1288,6 +1288,10 @@ _converse.initialize = async function (settings, callback) {
             });
         },
 
+        getNickname () {
+            return _converse.nickname;
+        },
+
         constructPresence (type, status_message) {
             let presence;
             type = _.isString(type) ? type : (this.get('status') || _converse.default_state);

src/headless/converse-muc.js

@@ -222,7 +222,7 @@ converse.plugins.add('converse-muc', {
                 throw new Error(
                     "Can't call _converse.getDefaultMUCNickname before the statusInitialized has been fired.");
             }
-            const nick = _converse.nickname || (_converse.vcards ? _converse.xmppstatus.vcard.get('nickname') : undefined);
+            const nick = _converse.xmppstatus.getNickname();
             if (nick) {
                 return nick;
             } else if (_converse.muc_nickname_from_jid) {

src/headless/converse-roster.js

@@ -15,7 +15,7 @@ const u = converse.env.utils;
 
 converse.plugins.add('converse-roster', {
 
-    dependencies: ["converse-vcard"],
+    dependencies: [],
 
     initialize () {
         /* The initialize function gets called as soon as the plugin is
@@ -221,30 +221,12 @@ converse.plugins.add('converse-roster', {
         });
 
 
-        _converse.ModelWithVCardAndPresence = Backbone.Model.extend({
-            initialize () {
-                this.setVCard();
-                this.setPresence();
-            },
-
-            setVCard () {
-                if (!_converse.vcards) {
-                    // VCards aren't supported
-                    return;
-                }
-                const jid = this.get('jid');
-                this.vcard = _converse.vcards.findWhere({'jid': jid}) || _converse.vcards.create({'jid': jid});
-            },
-
-            setPresence () {
-                const jid = this.get('jid');
-                this.presence = _converse.presences.findWhere({'jid': jid}) || _converse.presences.create({'jid': jid});
-            }
-        });
-
-
-        _converse.RosterContact = _converse.ModelWithVCardAndPresence.extend({
-
+        /**
+         * @class
+         * @namespace _converse.RosterContact
+         * @memberOf _converse
+         */
+        _converse.RosterContact = Backbone.Model.extend({
             defaults: {
                 'chat_state': undefined,
                 'image': _converse.DEFAULT_IMAGE,
@@ -253,13 +235,10 @@ converse.plugins.add('converse-roster', {
                 'status': undefined,
             },
 
-            initialize (attributes) {
-                _converse.ModelWithVCardAndPresence.prototype.initialize.apply(this, arguments);
-
-                const { jid } = attributes,
-                    bare_jid = Strophe.getBareJidFromJid(jid).toLowerCase(),
-                    resource = Strophe.getResourceFromJid(jid);
-
+            async initialize (attributes) {
+                this.setPresence();
+                const { jid } = attributes;
+                const bare_jid = Strophe.getBareJidFromJid(jid).toLowerCase();
                 attributes.jid = bare_jid;
                 this.set(_.assignIn({
                     'groups': [],
@@ -276,24 +255,31 @@ converse.plugins.add('converse-roster', {
                  */
                 this.presence.on('change:show', () => _converse.api.trigger('contactPresenceChanged', this));
                 this.presence.on('change:show', () => this.trigger('presenceChanged'));
+                /**
+                 * Synchronous event which provides a hook for further initializing a RosterContact
+                 * @event _converse#rosterContactInitialized
+                 * @param { _converse.RosterContact } contact
+                 */
+                await _converse.api.trigger('rosterContactInitialized', this, {'Synchronous': true});
+            },
+
+            setPresence () {
+                const jid = this.get('jid');
+                this.presence = _converse.presences.findWhere({'jid': jid}) || _converse.presences.create({'jid': jid});
             },
 
             getDisplayName () {
+                // Gets overridden in converse-vcard where the fullname is may be returned
                 if (this.get('nickname')) {
                     return this.get('nickname');
-                } else if (this.vcard) {
-                    return this.vcard.getDisplayName();
                 } else {
                     return this.get('jid');
                 }
             },
 
             getFullname () {
-                if (this.vcard) {
-                    return this.vcard.get('fullname');
-                } else {
-                    return this.get('jid');
-                }
+                // Gets overridden in converse-vcard where the fullname may be returned
+                return this.get('jid');
             },
 
             /**
@@ -308,7 +294,7 @@ converse.plugins.add('converse-roster', {
                 if (message && message !== "") {
                     pres.c("status").t(message).up();
                 }
-                const nick = _converse.nickname || _converse.xmppstatus.vcard.get('nickname') || _converse.xmppstatus.vcard.get('fullname');
+                const nick = _converse.xmppstatus.getNickname();
                 if (nick) {
                     pres.c('nick', {'xmlns': Strophe.NS.NICK}).t(nick).up();
                 }
@@ -483,11 +469,11 @@ converse.plugins.add('converse-roster', {
             },
 
             subscribeToSuggestedItems (msg) {
-                _.each(msg.querySelectorAll('item'), function (item) {
+                Array.from(msg.querySelectorAll('item')).forEach(item => {
                     if (item.getAttribute('action') === 'add') {
                         _converse.roster.addAndSubscribe(
                             item.getAttribute('jid'),
-                            _converse.xmppstatus.vcard.get('nickname') || _converse.xmppstatus.vcard.get('fullname')
+                            _converse.xmppstatus.getNickname()
                         );
                     }
                 });

src/headless/converse-vcard.js

@@ -16,12 +16,46 @@ const u = converse.env.utils;
 
 converse.plugins.add('converse-vcard', {
 
+    dependencies: ["converse-roster"],
+
+    overrides: {
+        XMPPStatus: {
+            getNickname () {
+                const { _converse } = this.__super__;
+                const nick = this.__super__.getNickname.apply(this);
+                if (!nick && _converse.xmppstatus.vcard) {
+                    return _converse.xmppstatus.vcard.get('nickname') || _converse.xmppstatus.vcard.get('fullname');
+                } else {
+                    return nick;
+                }
+            }
+        },
+
+        RosterContact: {
+            getDisplayName () {
+                if (!this.get('nickname') && this.vcard) {
+                    return this.vcard.getDisplayName();
+                } else {
+                    return this.__super__.getDisplayName.apply(this);
+                }
+            },
+            getFullname () {
+                if (this.vcard) {
+                    return this.vcard.get('fullname');
+                } else {
+                    return this.__super__.getFullname.apply(this);
+                }
+            }
+        }
+    },
+
     initialize () {
         /* The initialize function gets called as soon as the plugin is
          * loaded by converse.js's plugin machinery.
          */
         const { _converse } = this;
 
+
         _converse.VCard = Backbone.Model.extend({
             defaults: {
                 'image': _converse.DEFAULT_IMAGE,
@@ -97,14 +131,6 @@ converse.plugins.add('converse-vcard', {
             return iq;
         }
 
-        function setVCard (jid, data) {
-            if (!jid) {
-                throw Error("No jid provided for the VCard data");
-            }
-            const vcard_el = Strophe.xmlHtmlNode(tpl_vcard(data)).firstElementChild;
-            return _converse.api.sendIQ(createStanza("set", jid, vcard_el));
-        }
-
         async function getVCard (_converse, jid) {
             const to = Strophe.getBareJidFromJid(jid) === _converse.bare_jid ? null : jid;
             let iq;
@@ -148,6 +174,19 @@ converse.plugins.add('converse-vcard', {
 
         _converse.api.listen.on('addClientFeatures', () => _converse.api.disco.own.features.add(Strophe.NS.VCARD));
 
+
+        function setVCardOnModel (model) {
+            // TODO: if we can make this method async and wait for the VCard to
+            // be updated, then we'll avoid unnecessary re-rendering of roster contacts.
+            const jid = model.get('jid');
+            model.vcard = _converse.vcards.findWhere({'jid': jid});
+            if (!model.vcard) {
+                model.vcard = _converse.vcards.create({'jid': jid});
+            }
+        }
+        _converse.api.listen.on('rosterContactInitialized', contact => setVCardOnModel(contact));
+
+
         /************************ BEGIN API ************************/
         Object.assign(_converse.api, {
             /**
@@ -177,7 +216,11 @@ converse.plugins.add('converse-vcard', {
                  * }).
                  */
                 set (jid, data) {
-                    return setVCard(jid, data);
+                    if (!jid) {
+                        throw Error("No jid provided for the VCard data");
+                    }
+                    const vcard_el = Strophe.xmlHtmlNode(tpl_vcard(data)).firstElementChild;
+                    return _converse.api.sendIQ(createStanza("set", jid, vcard_el));
                 },
 
                 /**
@@ -233,12 +276,10 @@ converse.plugins.add('converse-vcard', {
                  *     _converse.api.vcard.update(chatbox);
                  * });
                  */
-                update (model, force) {
-                    return this.get(model, force)
-                        .then(vcard => {
-                            delete vcard['stanza']
-                            model.save(vcard);
-                        });
+                async update (model, force) {
+                    const vcard = await this.get(model, force);
+                    delete vcard['stanza']
+                    model.save(vcard);
                 }
             }
         });

tests/mock.js

@@ -71,7 +71,7 @@
 
     // Names from http://www.fakenamegenerator.com/
     mock.req_names = [
-        'Escalus, Prince of Verona', 'The Nurse', 'Paris'
+        'Escalus, prince of Verona', 'The Nurse', 'Paris'
     ];
     mock.pend_names = [
         'Lord Capulet', 'Lady Capulet', 'Servant'