Use the VCards collection for roster contacts

Instead of saving the vcard data on the contact model itself

CHANGES.md

@@ -10,6 +10,12 @@
 - Support for rendering URLs sent according to XEP-0066 Out of Band Data.
 - Geo-URIs (e.g. from Conversations) are now replaced by links to openstreetmap (works in reverse also)
 
+### Bugfixes
+
+- Spoiler messages didn't include the message author's name.
+- Documentation includes utf-8 charset to make minfied versions compatible across platforms. #1017
+- #1026 Typing in MUC shows "Typing from another device"
+
 ### API changes
 - `_converse.api.vcard.get` now also accepts a `Backbone.Model` instance and
   has an additional `force` parameter to force fetching the vcard even if it
@@ -40,12 +46,10 @@
 - Extracted the views from `converse-muc.js` into `converse-muc-views.js` and
   where appropriate moved methods from the views into the models/collections.
   This makes MUC possible in headless mode.
+- Created a new core plugin `converse-roster.js` which contains the models for
+  roster-related data. Previously this code was in `converse-core.js`.
+- VCards are now stored separately from chats and roster contacts.
 
-### Bugfixes
-
-- Spoiler messages didn't include the message author's name.
-- Documentation includes utf-8 charset to make minfied versions compatible across platforms. #1017
-- #1026 Typing in MUC shows "Typing from another device"
 
 ## 3.3.4 (2018-03-05)
 

spec/chatbox.js

@@ -245,7 +245,7 @@
                 expect(_converse.chatboxes.length).toEqual(1);
 
                 chatbox = test_utils.openChatBoxFor(_converse, contact_jid);
-                $el = $(_converse.rosterview.el).find('a.open-chat:contains("'+chatbox.get('fullname')+'")');
+                $el = $(_converse.rosterview.el).find('a.open-chat:contains("'+chatbox.getDisplayName()+'")');
                 jid = $el.text().replace(/ /g,'.').toLowerCase() + '@localhost';
 
                 spyOn(_converse, 'emit');

spec/roster.js

@@ -651,6 +651,8 @@
                     function (done, _converse) {
 
                 _addContacts(_converse);
+                return test_utils.waitUntil(() => _converse.roster.at(0).vcard.get('fullname'))
+                .then(function () {
                     var name;
                     spyOn(window, 'confirm').and.returnValue(true);
                     for (var i=0; i<mock.pend_names.length; i++) {
@@ -660,6 +662,7 @@
                     }
                     expect($(_converse.rosterview.el).find('#pending-xmpp-contacts').is(':visible')).toBeFalsy();
                     done();
+                });
             }));
 
             it("can be added to the roster and they will be sorted alphabetically",

src/converse-notification.js

@@ -157,7 +157,7 @@
                     }
                     roster_item = _converse.roster.get(from_jid);
                     if (!_.isUndefined(roster_item)) {
-                        title = __("%1$s says", roster_item.get('fullname'));
+                        title = __("%1$s says", roster_item.getDisplayName());
                     } else {
                         if (_converse.allow_non_roster_messaging) {
                             title = __("%1$s says", from_jid);
@@ -196,7 +196,7 @@
                 if (message === null) {
                     return;
                 }
-                const n = new Notification(contact.fullname, {
+                const n = new Notification(contact.getDisplayName(), {
                         body: message,
                         lang: _converse.locale,
                         icon: _converse.notification_icon
@@ -205,7 +205,7 @@
             };
 
             _converse.showContactRequestNotification = function (contact) {
-                const n = new Notification(contact.fullname, {
+                const n = new Notification(contact.getDisplayName(), {
                         body: __('wants to be your contact'),
                         lang: _converse.locale,
                         icon: _converse.notification_icon

src/converse-roster.js

@@ -13,6 +13,8 @@
 
     converse.plugins.add('converse-roster', {
 
+        dependencies: ["converse-vcard"],
+
         overrides: {
             clearSession () {
                 this.__super__.clearSession.apply(this, arguments);
@@ -121,7 +123,6 @@
 
                     attributes.jid = bare_jid;
                     this.set(_.assignIn({
-                        'fullname': bare_jid,
                         'groups': [],
                         'id': bare_jid,
                         'jid': bare_jid,
@@ -129,11 +130,24 @@
                         'user_id': Strophe.getNodeFromJid(jid)
                     }, attributes));
 
+                    this.vcard = _converse.vcards.findWhere({'jid': bare_jid});
+                    if (_.isNil(this.vcard)) {
+                        this.vcard = _converse.vcards.create({'jid': bare_jid});
+                    }
+
                     this.on('change:chat_status', function (item) {
                         _converse.emit('contactStatusChanged', item.attributes);
                     });
                 },
 
+                getDisplayName () {
+                    return this.vcard.get('fullname') || this.get('jid');
+                },
+
+                getFullname () {
+                    return this.vcard.get('fullname');
+                },
+
                 subscribe (message) {
                     /* Send a presence subscription request to this roster contact
                     *
@@ -297,8 +311,8 @@
                     const status1 = contact1.get('chat_status') || 'offline';
                     const status2 = contact2.get('chat_status') || 'offline';
                     if (_converse.STATUS_WEIGHTS[status1] === _converse.STATUS_WEIGHTS[status2]) {
-                        const name1 = (contact1.get('fullname') || contact1.get('jid')).toLowerCase();
-                        const name2 = (contact2.get('fullname') || contact2.get('jid')).toLowerCase();
+                        const name1 = (contact1.getDisplayName()).toLowerCase();
+                        const name2 = (contact2.getDisplayName()).toLowerCase();
                         return name1 < name2 ? -1 : (name1 > name2? 1 : 0);
                     } else  {
                         return _converse.STATUS_WEIGHTS[status1] < _converse.STATUS_WEIGHTS[status2] ? -1 : 1;
@@ -436,12 +450,11 @@
                     */
                     return new Promise((resolve, reject) => {
                         groups = groups || [];
-                        name = _.isEmpty(name)? jid: name;
                         this.sendContactAddIQ(jid, name, groups,
                             () => {
                                 const contact = this.create(_.assignIn({
                                     'ask': undefined,
-                                    'fullname': name,
+                                    'nickname': name,
                                     groups,
                                     jid,
                                     'requesting': false,
@@ -555,7 +568,7 @@
                         }
                         this.create({
                             'ask': ask,
-                            'fullname': item.getAttribute("name") || jid,
+                            'nickname': item.getAttribute("name"),
                             'groups': groups,
                             'jid': jid,
                             'subscription': subscription
@@ -585,7 +598,7 @@
                         'subscription': 'none',
                         'ask': null,
                         'requesting': true,
-                        'fullname': nickname
+                        'nickname': nickname
                     };
                     this.create(user_data);
                     _converse.emit('contactRequest', user_data);

src/converse-rosterview.js

@@ -368,9 +368,10 @@
 
                 initialize () {
                     this.model.on("change", this.render, this);
-                    this.model.on("remove", this.remove, this);
                     this.model.on("destroy", this.remove, this);
                     this.model.on("open", this.openChat, this);
+                    this.model.on("remove", this.remove, this);
+                    this.model.vcard.on('change:fullname', this.render, this);
                 },
 
                 render () {
@@ -412,19 +413,23 @@
                          *
                          *  So in both cases the user is a "pending" contact.
                          */
+                        const display_name = item.getDisplayName();
                         this.el.classList.add('pending-xmpp-contact');
                         this.el.innerHTML = tpl_pending_contact(
                             _.extend(item.toJSON(), {
-                                'desc_remove': __('Click to remove %1$s as a contact', item.get('fullname') || item.get('jid')),
+                                'display_name': display_name,
+                                'desc_remove': __('Click to remove %1$s as a contact', display_name),
                                 'allow_chat_pending_contacts': _converse.allow_chat_pending_contacts
                             })
                         );
                     } else if (requesting === true) {
+                        const display_name = item.getDisplayName();
                         this.el.classList.add('requesting-xmpp-contact');
                         this.el.innerHTML = tpl_requesting_contact(
                             _.extend(item.toJSON(), {
-                                'desc_accept': __("Click to accept the contact request from %1$s", item.get('fullname') || item.get('jid')),
-                                'desc_decline': __("Click to decline the contact request from %1$s", item.get('fullname') || item.get('jid')),
+                                'display_name': display_name,
+                                'desc_accept': __("Click to accept the contact request from %1$s", display_name),
+                                'desc_decline': __("Click to decline the contact request from %1$s", display_name),
                                 'allow_chat_pending_contacts': _converse.allow_chat_pending_contacts
                             })
                         );
@@ -449,12 +454,14 @@
                     } else if (chat_status === 'dnd') {
                         status_icon = 'fa-minus-circle';
                     }
+                    const display_name = item.getDisplayName();
                     this.el.innerHTML = tpl_roster_item(
                         _.extend(item.toJSON(), {
+                            'display_name': display_name,
                             'desc_status': STATUSES[chat_status],
                             'status_icon': status_icon,
-                            'desc_chat': __('Click to chat with %1$s (JID: %2$s)', item.get('fullname') || item.get('jid'), item.get('jid')),
-                            'desc_remove': __('Click to remove %1$s as a contact', item.get('fullname') || item.get('jid')),
+                            'desc_chat': __('Click to chat with %1$s (JID: %2$s)', display_name, item.get('jid')),
+                            'desc_remove': __('Click to remove %1$s as a contact', display_name),
                             'allow_contact_removal': _converse.allow_contact_removal,
                             'num_unread': item.get('num_unread') || 0
                         })
@@ -511,7 +518,7 @@
                     if (ev && ev.preventDefault) { ev.preventDefault(); }
                     _converse.roster.sendContactAddIQ(
                         this.model.get('jid'),
-                        this.model.get('fullname'),
+                        this.model.getFullname(),
                         [],
                         () => { this.model.authorize().subscribe(); }
                     );
@@ -633,8 +640,7 @@
                         }
                     } else  {
                         matches = this.model.contacts.filter((contact) => {
-                            const value = contact.get('fullname') || contact.get('jid');
-                            return !_.includes(value.toLowerCase(), q.toLowerCase());
+                            return !_.includes(contact.getDisplayName().toLowerCase(), q.toLowerCase());
                         });
                     }
                     return matches;

src/converse-vcard.js

-// Converse.js (A browser based XMPP chat client)
+// Converse.js
 // http://conversejs.org
 //
-// Copyright (c) 2012-2017, Jan-Carel Brand <jc@opkode.com>
+// Copyright (c) 2012-2018, the Converse.js developers
 // Licensed under the Mozilla Public License (MPLv2)
-//
-/*global define */
 
 (function (root, factory) {
     define(["converse-core", "crypto", "strophe.vcard"], factory);
@@ -36,11 +34,9 @@
     }
 
     function onVCardError (_converse, jid, iq, errback) {
-        const contact = _converse.roster.get(jid);
-        if (contact) {
-            contact.save({'vcard_updated': moment().format() });
+        if (errback) {
+            errback({'stanza': iq, 'jid': jid});
         }
-        if (errback) { errback({'stanza': iq, 'jid': jid}); }
     }
 
     function getVCard (_converse, jid) {
@@ -63,34 +59,6 @@
 
     converse.plugins.add('converse-vcard', {
 
-        // FIXME: After refactoring, the dependency switches, from
-        // converse-roster to converse-vcard
-        dependencies: ["converse-roster"],
-
-        overrides: {
-            // Overrides mentioned here will be picked up by converse.js's
-            // plugin architecture they will replace existing methods on the
-            // relevant objects or classes.
-            //
-            // New functions which don't exist yet can also be added.
-
-            RosterContacts: {
-                createRequestingContact (presence) {
-                    const { _converse } = this.__super__;
-                    const bare_jid = Strophe.getBareJidFromJid(presence.getAttribute('from'));
-
-                    _converse.api.vcard.get(bare_jid)
-                        .then(_.partial(_converse.createRequestingContactFromVCard, presence))
-                        .catch((vcard) => {
-                            _converse.log(
-                                `Error while retrieving vcard for ${vcard.jid}`,
-                                Strophe.LogLevel.WARN);
-                            _converse.createRequestingContactFromVCard(presence, vcard.stanza, vcard.jid);
-                        });
-                }
-            }
-        },
-
         initialize () {
             /* The initialize function gets called as soon as the plugin is
              * loaded by converse.js's plugin machinery.
@@ -106,29 +74,6 @@
             });
 
 
-            _converse.createRequestingContactFromVCard = function (presence, vcard) {
-                const bare_jid = Strophe.getBareJidFromJid(presence.getAttribute('from'));
-                let fullname = vcard.fullname;
-                if (!fullname) {
-                    const nick_el = sizzle(`nick[xmlns="${Strophe.NS.NICK}"]`, presence);
-                    fullname = nick_el.length ? nick_el[0].textContent : bare_jid;
-                }
-                const user_data = {
-                    'jid': bare_jid,
-                    'subscription': 'none',
-                    'ask': null,
-                    'requesting': true,
-                    'fullname': fullname,
-                    'image': vcard.image,
-                    'image_type': vcard.image_type,
-                    'image_hash': vcard.image_hash,
-                    'url': vcard.url,
-                    'vcard_updated': moment().format()
-                };
-                _converse.roster.create(user_data);
-                _converse.emit('contactRequest', user_data);
-            };
-
             /* Event handlers */
             _converse.initVCardCollection = function () {
                 _converse.vcards = new _converse.VCards();
@@ -142,10 +87,6 @@
                 _converse.connection.disco.addFeature(Strophe.NS.VCARD);
             });
 
-            _converse.on('initialized', () => {
-                _converse.roster.on("add", (contact) => _converse.api.vcard.update(contact));
-            });
-
             _converse.on('statusInitialized', function fetchOwnVCard () {
                 _converse.api.disco.supports(Strophe.NS.VCARD, _converse.domain)
                     .then((result) => {

src/templates/pending_contact.html

 {[ if (o.allow_chat_pending_contacts)  { ]}
 <a class="open-chat w-100" href="#">
 {[ } ]}
-<span class="pending-contact-name w-100" title="Name: {{{o.fullname}}} JID: {{{o.jid}}}">{{{o.fullname}}}</span> 
+<span class="pending-contact-name w-100" title="JID: {{{o.jid}}}">{{{o.display_name}}}</span> 
 {[ if (o.allow_chat_pending_contacts)  { ]}</a>
 {[ } ]}
 <a class="remove-xmpp-contact fa fa-trash" title="{{{o.desc_remove}}}" href="#"></a>

src/templates/requesting_contact.html

 {[ if (o.allow_chat_pending_contacts)  { ]}
 <a class="open-chat w-100"href="#">
 {[ } ]}
-<span class="req-contact-name w-100" title="Name: {{{o.fullname}}}
-JID: {{{o.jid}}}">{{{o.fullname}}}</span>
+<span class="req-contact-name w-100" title="JID: {{{o.jid}}}">{{{o.display_name}}}</span>
 {[ if (o.allow_chat_pending_contacts)  { ]}
 </a>
 {[ } ]}

src/templates/roster_item.html

@@ -4,7 +4,7 @@
     {[ if (o.num_unread) { ]}
     <span class="msgs-indicator">{{{ o.num_unread }}}</span>
     {[ } ]}
-    <span class="contact-name {[ if (o.num_unread) { ]} unread-msgs {[ } ]}">{{{o.fullname || o.jid}}}</span></a>
+    <span class="contact-name {[ if (o.num_unread) { ]} unread-msgs {[ } ]}">{{{o.display_name}}}</span></a>
 {[ if (o.allow_contact_removal) { ]}
 <a class="remove-xmpp-contact fa fa-trash" title="{{{o.desc_remove}}}" href="#"></a>
 {[ } ]}