chatboxes: wait until messages are fixed before returning new chatbox

Fixes #1691

Author: jcbrand

CHANGES.md

@@ -1,15 +1,26 @@
 # Changelog
 
-## 5.0.4
+## 6.0.0 (Unreleased)
+
+- #1691 Fix `collection.chatbox is undefined` errors
+
+### Breaking changes
+
+- The following API methods now return promises:
+  * `_converse.api.chats.get`
+  * `_converse.api.chats.create` 
+  * `_converse.api.rooms.get`
+  * `_converse.api.rooms.create`
+
+## 5.0.4 (Unreleased)
 
 - Bugfix: Don't treat every duplicate message ID as a message correction; since some
   clients don't use globally unique ID's this causes false positives.
-- #1712: `TypeError: plugin._features is not a function`
-- Bugfix: process stanzas from mam one-by-one in order to correctly process message
-  receipts
-- #1714 Bugfix: Don't notify the user in case we're receiving a message delivery receipt only
-- add config option [allow_message_corrections](https://conversejs.org/docs/html/configuration.html#allow_message_corrections)
+- Bugfix: process stanzas from mam one-by-one in order to correctly process message receipts
+- Add config option [allow_message_corrections](https://conversejs.org/docs/html/configuration.html#allow_message_corrections)
   which, if set to `last`, limits editing of sent messages to the last message sent
+- #1712 `TypeError: plugin._features is not a function`
+- #1714 Bugfix: Don't notify the user in case we're receiving a message delivery receipt only
 
 ## 5.0.3 (2019-09-13)
 

spec/bookmarks.js

@@ -270,6 +270,7 @@
                     'nick': ' Othello'
                 });
                 expect(_converse.bookmarks.length).toBe(1);
+                await u.waitUntil(() => _converse.chatboxes.length >= 1);
                 expect(view.model.get('bookmarked')).toBeTruthy();
                 let bookmark_icon = await u.waitUntil(() => view.el.querySelector('.toggle-bookmark'));
                 expect(u.hasClass('button-on', bookmark_icon)).toBeTruthy();

spec/converse.js

@@ -262,26 +262,27 @@
                 _converse.api.trigger('rosterContactsFetched');
 
                 // Test on chat that doesn't exist.
-                expect(_converse.api.chats.get('[email protected]')).toBeFalsy();
+                let chat = await _converse.api.chats.get('[email protected]');
+                expect(chat).toBeFalsy();
                 const jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@montague.lit';
                 const jid2 = mock.cur_names[1].replace(/ /g,'.').toLowerCase() + '@montague.lit';
 
                 // Test on chat that's not open
-                let box = _converse.api.chats.get(jid);
-                expect(typeof box === 'undefined').toBeTruthy();
+                chat = await _converse.api.chats.get(jid);
+                expect(typeof chat === 'undefined').toBeTruthy();
                 expect(_converse.chatboxes.length).toBe(1);
 
                 // Test for one JID
-                box = await _converse.api.chats.open(jid);
-                expect(box instanceof Object).toBeTruthy();
-                expect(box.get('box_id')).toBe(`box-${btoa(jid)}`);
+                chat = await _converse.api.chats.open(jid);
+                expect(chat instanceof Object).toBeTruthy();
+                expect(chat.get('box_id')).toBe(`box-${btoa(jid)}`);
 
                 const view = _converse.chatboxviews.get(jid);
                 await u.waitUntil(() => u.isVisible(view.el));
                 // Test for multiple JIDs
                 test_utils.openChatBoxFor(_converse, jid2);
                 await u.waitUntil(() => _converse.chatboxes.length == 2);
-                const list = _converse.api.chats.get([jid, jid2]);
+                const list = await _converse.api.chats.get([jid, jid2]);
                 expect(Array.isArray(list)).toBeTruthy();
                 expect(list[0].get('box_id')).toBe(`box-${btoa(jid)}`);
                 expect(list[1].get('box_id')).toBe(`box-${btoa(jid2)}`);
@@ -299,13 +300,14 @@
                 const jid = mock.cur_names[0].replace(/ /g,'.').toLowerCase() + '@montague.lit';
                 const jid2 = mock.cur_names[1].replace(/ /g,'.').toLowerCase() + '@montague.lit';
                 // Test on chat that doesn't exist.
-                expect(_converse.api.chats.get('[email protected]')).toBeFalsy();
+                let chat = await _converse.api.chats.get('[email protected]');
+                expect(chat).toBeFalsy();
 
-                const box = await _converse.api.chats.open(jid);
-                expect(box instanceof Object).toBeTruthy();
-                expect(box.get('box_id')).toBe(`box-${btoa(jid)}`);
+                chat = await _converse.api.chats.open(jid);
+                expect(chat instanceof Object).toBeTruthy();
+                expect(chat.get('box_id')).toBe(`box-${btoa(jid)}`);
                 expect(
-                    _.keys(box),
+                    Object.keys(chat),
                     ['close', 'endOTR', 'focus', 'get', 'initiateOTR', 'is_chatroom', 'maximize', 'minimize', 'open', 'set']
                 );
                 const view = _converse.chatboxviews.get(jid);

spec/messages.js

@@ -1047,7 +1047,7 @@
             const message = 'This message is sent from this chatbox';
             await test_utils.sendMessage(view, message);
 
-            const chatbox = _converse.api.chats.get(contact_jid);
+            const chatbox = await _converse.api.chats.get(contact_jid);
             expect(chatbox.messages.models.length, 1);
             const msg_object = chatbox.messages.models[0];
 

spec/muc.js

@@ -56,7 +56,7 @@
                 await u.waitUntil(() => _converse.rosterview.el.querySelectorAll('.roster-group .group-toggle').length, 300);
                 let muc_jid = '[email protected]';
                 await test_utils.openAndEnterChatRoom(_converse, muc_jid, 'romeo');
-                let room = _converse.api.rooms.get(muc_jid);
+                let room = await _converse.api.rooms.get(muc_jid);
                 expect(room instanceof Object).toBeTruthy();
 
                 let chatroomview = _converse.chatboxviews.get(muc_jid);
@@ -68,27 +68,27 @@
                 // Test with mixed case
                 muc_jid = '[email protected]';
                 await test_utils.openAndEnterChatRoom(_converse, muc_jid, 'romeo');
-                room = _converse.api.rooms.get(muc_jid);
+                room = await _converse.api.rooms.get(muc_jid);
                 expect(room instanceof Object).toBeTruthy();
                 chatroomview = _converse.chatboxviews.get(muc_jid.toLowerCase());
                 expect(u.isVisible(chatroomview.el)).toBeTruthy();
 
                 muc_jid = '[email protected]';
-                room = _converse.api.rooms.get(muc_jid);
+                room = await _converse.api.rooms.get(muc_jid);
                 expect(room instanceof Object).toBeTruthy();
                 chatroomview = _converse.chatboxviews.get(muc_jid.toLowerCase());
                 expect(u.isVisible(chatroomview.el)).toBeTruthy();
 
                 muc_jid = '[email protected]';
-                room = _converse.api.rooms.get(muc_jid);
+                room = await _converse.api.rooms.get(muc_jid);
                 expect(room instanceof Object).toBeTruthy();
                 chatroomview = _converse.chatboxviews.get(muc_jid.toLowerCase());
                 expect(u.isVisible(chatroomview.el)).toBeTruthy();
                 chatroomview.close();
 
                 // Non-existing room
                 muc_jid = '[email protected]';
-                room = _converse.api.rooms.get(muc_jid);
+                room = await _converse.api.rooms.get(muc_jid);
                 expect(typeof room === 'undefined').toBeTruthy();
                 done();
             }));

src/headless/converse-bookmarks.js

@@ -99,19 +99,23 @@ converse.plugins.add('converse-bookmarks', {
             comparator: (item) => item.get('name').toLowerCase(),
 
             initialize () {
-                this.on('add', _.flow(this.openBookmarkedRoom, this.markRoomAsBookmarked));
+                this.on('add', bm => this.openBookmarkedRoom(bm)
+                    .then(bm => this.markRoomAsBookmarked(bm))
+                    .catch(e => _converse.log(e, Strophe.LogLevel.FATAL))
+                );
+
                 this.on('remove', this.markRoomAsUnbookmarked, this);
                 this.on('remove', this.sendBookmarkStanza, this);
 
-                const storage = _converse.config.get('storage'),
-                      cache_key = `converse.room-bookmarks${_converse.bare_jid}`;
+                const storage = _converse.config.get('storage');
+                const cache_key = `converse.room-bookmarks${_converse.bare_jid}`;
                 this.fetched_flag = cache_key+'fetched';
                 this.browserStorage = new BrowserStorage[storage](cache_key);
             },
 
-            openBookmarkedRoom (bookmark) {
+            async openBookmarkedRoom (bookmark) {
                 if ( _converse.muc_respect_autojoin && bookmark.get('autojoin')) {
-                    const groupchat = _converse.api.rooms.create(bookmark.get('jid'), bookmark.get('nick'));
+                    const groupchat = await _converse.api.rooms.create(bookmark.get('jid'), bookmark.get('nick'));
                     groupchat.maybeShow();
                 }
                 return bookmark;

src/headless/converse-chatboxes.js

@@ -326,9 +326,9 @@ converse.plugins.add('converse-chatboxes', {
 
             initMessages () {
                 this.messages = new this.messagesCollection();
+                this.messages.chatbox = this;
                 const storage = _converse.config.get('storage');
                 this.messages.browserStorage = new BrowserStorage[storage](this.getMessagesCacheKey());
-                this.messages.chatbox = this;
                 this.listenTo(this.messages, 'change:upload', message => {
                     if (message.get('upload') === _converse.SUCCESS) {
                         _converse.api.send(this.createMessageStanza(message));
@@ -1167,7 +1167,7 @@ converse.plugins.add('converse-chatboxes', {
                 if (utils.isSameBareJID(from_jid, _converse.bare_jid)) {
                     return;
                 }
-                const chatbox = this.getChatBox(from_jid);
+                const chatbox = await this.getChatBox(from_jid);
                 if (!chatbox) {
                     return;
                 }
@@ -1203,7 +1203,7 @@ converse.plugins.add('converse-chatboxes', {
             /**
              * Handler method for all incoming single-user chat "message" stanzas.
              * @private
-             * @method _converse.ChatBox#onMessage
+             * @method _converse.ChatBoxes#onMessage
              * @param { XMLElement } stanza - The incoming message stanza
              */
             async onMessage (stanza) {
@@ -1279,7 +1279,7 @@ converse.plugins.add('converse-chatboxes', {
                 // Get chat box, but only create when the message has something to show to the user
                 const has_body = sizzle(`body, encrypted[xmlns="${Strophe.NS.OMEMO}"]`, stanza).length > 0;
                 const roster_nick = get(contact, 'attributes.nickname');
-                const chatbox = this.getChatBox(contact_jid, {'nickname': roster_nick}, has_body);
+                const chatbox = await this.getChatBox(contact_jid, {'nickname': roster_nick}, has_body);
 
                 if (chatbox) {
                     const message = await chatbox.getDuplicateMessage(stanza);
@@ -1319,7 +1319,7 @@ converse.plugins.add('converse-chatboxes', {
              * @param { object } attrs - Optional chat box atributes. If the
              *  chat box already exists, its attributes will be updated.
              */
-            getChatBox (jid, attrs={}, create) {
+            async getChatBox (jid, attrs={}, create) {
                 if (isObject(jid)) {
                     create = attrs;
                     attrs = jid;
@@ -1337,6 +1337,7 @@ converse.plugins.add('converse-chatboxes', {
                             _converse.log(response.responseText);
                         }
                     });
+                    await chatbox.messages.fetched;
                     if (!chatbox.isValid()) {
                         chatbox.destroy();
                         return null;
@@ -1470,7 +1471,7 @@ converse.plugins.add('converse-chatboxes', {
                  * // To open a single chat, provide the JID of the contact you're chatting with in that chat:
                  * converse.plugins.add('myplugin', {
                  *     initialize: function() {
-                 *         var _converse = this._converse;
+                 *         const _converse = this._converse;
                  *         // Note, buddy@example.org must be in your contacts roster!
                  *         _converse.api.chats.open('[email protected]').then(chat => {
                  *             // Now you can do something with the chat model
@@ -1482,7 +1483,7 @@ converse.plugins.add('converse-chatboxes', {
                  * // To open an array of chats, provide an array of JIDs:
                  * converse.plugins.add('myplugin', {
                  *     initialize: function () {
-                 *         var _converse = this._converse;
+                 *         const _converse = this._converse;
                  *         // Note, these users must first be in your contacts roster!
                  *         _converse.api.chats.open(['buddy1@example.com', '[email protected]']).then(chats => {
                  *             // Now you can do something with the chat models
@@ -1518,7 +1519,7 @@ converse.plugins.add('converse-chatboxes', {
                  *
                  * @method _converse.api.chats.get
                  * @param {String|string[]} jids - e.g. '[email protected]' or ['[email protected]', '[email protected]']
-                 * @returns {_converse.ChatBox}
+                 * @returns { Promise<_converse.ChatBox> }
                  *
                  * @example
                  * // To return a single chat, provide the JID of the contact you're chatting with in that chat:
@@ -1535,19 +1536,13 @@ converse.plugins.add('converse-chatboxes', {
                  */
                 get (jids) {
                     if (jids === undefined) {
-                        const result = [];
-                        _converse.chatboxes.each(function (chatbox) {
-                            // FIXME: Leaky abstraction from MUC. We need to add a
-                            // base type for chat boxes, and check for that.
-                            if (chatbox.get('type') !== _converse.CHATROOMS_TYPE) {
-                                result.push(chatbox);
-                            }
-                        });
-                        return result;
+                        // FIXME: Leaky abstraction from MUC. We need to add a
+                        // base type for chat boxes, and check for that.
+                        return _converse.chatboxes.filter(c => (c.get('type') !== _converse.CHATROOMS_TYPE));
                     } else if (isString(jids)) {
                         return _converse.chatboxes.getChatBox(jids);
                     }
-                    return jids.map(jid => _converse.chatboxes.getChatBox(jid, {}, true));
+                    return Promise.all(jids.map(jid => _converse.chatboxes.getChatBox(jid, {}, true)));
                 }
             }
         });

src/headless/converse-muc.js

@@ -230,14 +230,14 @@ converse.plugins.add('converse-muc', {
             }
         }
 
-        function openChatRoom (jid, settings) {
+        async function openChatRoom (jid, settings) {
             /* Opens a groupchat, making sure that certain attributes
              * are correct, for example that the "type" is set to
              * "chatroom".
              */
             settings.type = _converse.CHATROOMS_TYPE;
             settings.id = jid;
-            const chatbox = _converse.chatboxes.getChatBox(jid, settings, true);
+            const chatbox = await _converse.chatboxes.getChatBox(jid, settings, true);
             chatbox.maybeShow(true);
             return chatbox;
         }
@@ -2042,7 +2042,7 @@ converse.plugins.add('converse-muc', {
          * @method _converse.ChatRoom#onDirectMUCInvitation
          * @param { XMLElement } message - The message stanza containing the invitation.
          */
-        _converse.onDirectMUCInvitation = function (message) {
+        _converse.onDirectMUCInvitation = async function (message) {
             const x_el = sizzle('x[xmlns="jabber:x:conference"]', message).pop(),
                 from = Strophe.getBareJidFromJid(message.getAttribute('from')),
                 room_jid = x_el.getAttribute('jid'),
@@ -2068,7 +2068,7 @@ converse.plugins.add('converse-muc', {
                 }
             }
             if (result === true) {
-                const chatroom = openChatRoom(room_jid, {'password': x_el.getAttribute('password') });
+                const chatroom = await openChatRoom(room_jid, {'password': x_el.getAttribute('password') });
                 if (chatroom.get('connection_status') === converse.ROOMSTATUS.DISCONNECTED) {
                     _converse.chatboxes.get(room_jid).join();
                 }
@@ -2198,6 +2198,7 @@ converse.plugins.add('converse-muc', {
                  * @param {(string[]|string)} jid|jids The JID or array of
                  *     JIDs of the chatroom(s) to create
                  * @param {object} [attrs] attrs The room attributes
+                 * @returns {Promise} Promise which resolves with the Backbone.Model representing the chat.
                  */
                 create (jids, attrs={}) {
                     attrs = _.isString(attrs) ? {'nick': attrs} : (attrs || {});
@@ -2241,6 +2242,7 @@ converse.plugins.add('converse-muc', {
                  *   another chat already in the foreground.
                  *   Set `force` to `true` if you want to force the room to be
                  *   maximized or shown.
+                 * @returns {Promise} Promise which resolves with the Backbone.Model representing the chat.
                  *
                  * @example
                  * this._converse.api.rooms.open('[email protected]')
@@ -2277,13 +2279,14 @@ converse.plugins.add('converse-muc', {
                         _converse.log(err_msg, Strophe.LogLevel.ERROR);
                         throw(new TypeError(err_msg));
                     } else if (_.isString(jids)) {
-                        const room = _converse.api.rooms.create(jids, attrs);
-                        if (room) {
-                            room.maybeShow(force);
-                        }
+                        const room = await _converse.api.rooms.create(jids, attrs);
+                        room && room.maybeShow(force);
                         return room;
                     } else {
-                        return jids.map(jid => _converse.api.rooms.create(jid, attrs).maybeShow(force));
+                        return jids.map(async jid => {
+                            const room = await _converse.api.rooms.create(jid, attrs);
+                            room.maybeShow(force);
+                        });
                     }
                 },
 
@@ -2300,6 +2303,7 @@ converse.plugins.add('converse-muc', {
                  *     the user's JID will be used.
                  * @param {boolean} create A boolean indicating whether the room should be created
                  *     if not found (default: `false`)
+                 * @returns {Promise} Promise which resolves with the Backbone.Model representing the chat.
                  * @example
                  * _converse.api.waitUntil('roomsAutoJoined').then(() => {
                  *     const create_if_not_found = true;