Only create new message models once messages have been fetched

Fixes #2241

spec/bookmarks.js

@@ -5,7 +5,8 @@ const { Strophe, u, sizzle, $iq } = converse.env;
 
 describe("A chat room", function () {
 
-    it("can be bookmarked", mock.initConverse(['rosterGroupsFetched'], {}, async function (done, _converse) {
+    it("can be bookmarked", mock.initConverse(
+            ['rosterGroupsFetched', 'chatBoxesFetched'], {}, async function (done, _converse) {
 
         await mock.waitUntilDiscoConfirmed(
             _converse, _converse.bare_jid,
@@ -13,17 +14,18 @@ describe("A chat room", function () {
             ['http://jabber.org/protocol/pubsub#publish-options']
         );
         const { u, $iq } = converse.env;
-        let sent_stanza, IQ_id;
-        const sendIQ = _converse.connection.sendIQ;
-        spyOn(_converse.connection, 'sendIQ').and.callFake(function (iq, callback, errback) {
-            sent_stanza = iq;
-            IQ_id = sendIQ.bind(this)(iq, callback, errback);
-        });
         spyOn(_converse.connection, 'getUniqueId').and.callThrough();
 
+        const nick = 'JC';
+        const muc_jid = 'theplay@conference.shakespeare.lit';
         await mock.openChatRoom(_converse, 'theplay', 'conference.shakespeare.lit', 'JC');
-        const jid = 'theplay@conference.shakespeare.lit';
-        const view = _converse.chatboxviews.get(jid);
+        await mock.getRoomFeatures(_converse, muc_jid, []);
+        await mock.waitForReservedNick(_converse, muc_jid, nick);
+        await mock.receiveOwnMUCPresence(_converse, muc_jid, nick);
+        const view = _converse.chatboxviews.get(muc_jid);
+        await u.waitUntil(() => (view.model.session.get('connection_status') === converse.ROOMSTATUS.ENTERED));
+        await mock.returnMemberLists(_converse, muc_jid, [], ['member', 'admin', 'owner']);
+
         spyOn(view, 'renderBookmarkForm').and.callThrough();
         spyOn(view, 'closeForm').and.callThrough();
         await u.waitUntil(() => view.el.querySelector('.toggle-bookmark') !== null);
@@ -77,12 +79,13 @@ describe("A chat room", function () {
         form.querySelector('input[name="autojoin"]').checked = 'checked';
         form.querySelector('input[name="nick"]').value = 'JC';
 
-        _converse.connection.IQ_stanzas = [];
-        view.el.querySelector('.btn-primary').click();
+        const IQ_stanzas = _converse.connection.IQ_stanzas;
+        view.el.querySelector('.muc-bookmark-form .btn-primary').click();
 
-        await u.waitUntil(() => sent_stanza);
+        const sent_stanza = await u.waitUntil(
+            () => IQ_stanzas.filter(s => sizzle('iq publish[node="storage:bookmarks"]', s).length).pop());
         expect(Strophe.serialize(sent_stanza)).toBe(
-            `<iq from="romeo@montague.lit/orchard" id="${IQ_id}" type="set" xmlns="jabber:client">`+
+            `<iq from="romeo@montague.lit/orchard" id="${sent_stanza.getAttribute('id')}" type="set" xmlns="jabber:client">`+
                 `<pubsub xmlns="http://jabber.org/protocol/pubsub">`+
                     `<publish node="storage:bookmarks">`+
                         `<item id="current">`+
@@ -110,13 +113,13 @@ describe("A chat room", function () {
             `</iq>`
         );
         /* Server acknowledges successful storage
-            *
-            * <iq to='juliet@capulet.lit/balcony' type='result' id='pip1'/>
-            */
+         *
+         * <iq to='juliet@capulet.lit/balcony' type='result' id='pip1'/>
+         */
         const stanza = $iq({
             'to':_converse.connection.jid,
             'type':'result',
-            'id':IQ_id
+            'id': sent_stanza.getAttribute('id')
         });
         _converse.connection._dataRecv(mock.createRequest(stanza));
         await u.waitUntil(() => view.model.get('bookmarked'));

spec/emojis.js

@@ -39,7 +39,7 @@ describe("Emojis", function () {
             const muc_jid = 'lounge@montague.lit';
             await mock.openAndEnterChatRoom(_converse, muc_jid, 'romeo');
             const view = _converse.chatboxviews.get(muc_jid);
-
+            await u.waitUntil(() => view.el.querySelector('converse-emoji-dropdown'));
             const textarea = view.el.querySelector('textarea.chat-textarea');
             textarea.value = ':gri';
 
@@ -107,7 +107,7 @@ describe("Emojis", function () {
             const muc_jid = 'lounge@montague.lit';
             await mock.openAndEnterChatRoom(_converse, muc_jid, 'romeo');
             const view = _converse.chatboxviews.get(muc_jid);
-
+            await u.waitUntil(() => view.el.querySelector('converse-emoji-dropdown'));
             const textarea = view.el.querySelector('textarea.chat-textarea');
             textarea.value = ':';
             // Press tab
@@ -157,7 +157,7 @@ describe("Emojis", function () {
             const muc_jid = 'lounge@montague.lit';
             await mock.openAndEnterChatRoom(_converse, muc_jid, 'romeo');
             const view = _converse.chatboxviews.get(muc_jid);
-
+            await u.waitUntil(() => view.el.querySelector('converse-emoji-dropdown'));
             const textarea = view.el.querySelector('textarea.chat-textarea');
             textarea.value = ':gri';
 
@@ -183,6 +183,7 @@ describe("Emojis", function () {
             textarea.value = ':';
             view.onKeyDown(tab_event);
             await u.waitUntil(() => u.isVisible(view.el.querySelector('.emoji-picker__lists')));
+            await u.waitUntil(() => input.value === ':');
             input.dispatchEvent(new KeyboardEvent('keydown', tab_event));
             await u.waitUntil(() => input.value === ':100:');
             await u.waitUntil(() => sizzle('.emojis-lists__container--search .insert-emoji:not(.hidden)', view.el).length === 1, 1000);
@@ -200,8 +201,8 @@ describe("Emojis", function () {
 
             const muc_jid = 'lounge@montague.lit';
             await mock.openAndEnterChatRoom(_converse, muc_jid, 'romeo');
-
             const view = _converse.chatboxviews.get(muc_jid);
+            await u.waitUntil(() => view.el.querySelector('converse-emoji-dropdown'));
             const toolbar = view.el.querySelector('converse-chat-toolbar');
             toolbar.querySelector('.toggle-emojis').click();
             await u.waitUntil(() => u.isVisible(view.el.querySelector('.emoji-picker__lists')));

spec/mentions.js

@@ -267,7 +267,6 @@ describe("A sent groupchat message", function () {
             done();
         }));
 
-
         it("properly encodes the URIs in sent out references",
             mock.initConverse(
                 ['rosterGroupsFetched'], {},
@@ -299,7 +298,7 @@ describe("A sent groupchat message", function () {
             spyOn(_converse.connection, 'send');
             view.onKeyDown(enter_event);
             await new Promise(resolve => view.model.messages.once('rendered', resolve));
-            const msg = _converse.connection.send.calls.all()[0].args[0];
+            const msg = _converse.connection.send.calls.all()[1].args[0];
             expect(msg.toLocaleString())
                 .toBe(`<message from="romeo@montague.lit/orchard" id="${msg.nodeTree.getAttribute("id")}" `+
                         `to="lounge@montague.lit" type="groupchat" `+
@@ -353,7 +352,7 @@ describe("A sent groupchat message", function () {
                     'hello <span class="mention">z3r0</span> <span class="mention">gibson</span> <span class="mention">mr.robot</span>, how are you?'
             );
 
-            const msg = _converse.connection.send.calls.all()[0].args[0];
+            const msg = _converse.connection.send.calls.all()[1].args[0];
             expect(msg.toLocaleString())
                 .toBe(`<message from="romeo@montague.lit/orchard" id="${msg.nodeTree.getAttribute("id")}" `+
                         `to="lounge@montague.lit" type="groupchat" `+
@@ -397,7 +396,7 @@ describe("A sent groupchat message", function () {
             done();
         }));
 
-        it("includes XEP-0372 references to that person",
+        it("includes a XEP-0372 references to that person",
             mock.initConverse(
                 ['rosterGroupsFetched'], {},
                     async function (done, _converse) {
@@ -432,7 +431,7 @@ describe("A sent groupchat message", function () {
             view.onKeyDown(enter_event);
             await new Promise(resolve => view.model.messages.once('rendered', resolve));
 
-            const msg = _converse.connection.send.calls.all()[0].args[0];
+            const msg = _converse.connection.send.calls.all()[1].args[0];
             expect(msg.toLocaleString())
                 .toBe(`<message from="romeo@montague.lit/orchard" id="${msg.nodeTree.getAttribute("id")}" `+
                         `to="lounge@montague.lit" type="groupchat" `+

spec/muc.js

@@ -363,7 +363,6 @@ describe("Groupchats", function () {
             mock.initConverse(
                 ['rosterGroupsFetched'], {
                     'clear_messages_on_reconnection': true,
-                    'loglevel': 'debug',
                     'enable_smacks': false
                 }, async function (done, _converse) {
 
@@ -880,6 +879,9 @@ describe("Groupchats", function () {
                 .c('status', {code: '100'});
             _converse.connection._dataRecv(mock.createRequest(presence));
 
+            await u.waitUntil(() => (view.model.session.get('connection_status') === converse.ROOMSTATUS.ENTERED));
+            await mock.returnMemberLists(_converse, muc_jid, [], ['member', 'admin', 'owner']);
+
             const num_info_msgs = await u.waitUntil(() => view.el.querySelectorAll('.chat-content .chat-info').length);
             expect(num_info_msgs).toBe(1);
             expect(sizzle('div.chat-info', view.content).pop().textContent.trim()).toBe("This groupchat is not anonymous");
@@ -1969,6 +1971,9 @@ describe("Groupchats", function () {
                 .c('status').attrs({code:'210'}).nodeTree;
 
             _converse.connection._dataRecv(mock.createRequest(presence));
+
+            await u.waitUntil(() => (view.model.session.get('connection_status') === converse.ROOMSTATUS.ENTERED));
+            await mock.returnMemberLists(_converse, muc_jid, [], ['member', 'admin', 'owner']);
             await u.waitUntil(() => view.el.querySelectorAll('.chat-content .chat-info').length);
             const info_text = sizzle('.chat-content .chat-info:first', view.el).pop().textContent.trim();
             expect(info_text).toBe('Your nickname has been automatically set to thirdwitch');

src/headless/converse-chat.js

@@ -355,6 +355,7 @@ converse.plugins.add('converse-chat', {
 
             initMessages () {
                 this.messages = new this.messagesCollection();
+                this.messages.fetched = u.getResolveablePromise();
                 this.messages.chatbox = this;
                 this.messages.browserStorage = _converse.createStore(this.getMessagesCacheKey());
                 this.listenTo(this.messages, 'change:upload', message => {
@@ -380,11 +381,11 @@ converse.plugins.add('converse-chat', {
             },
 
             fetchMessages () {
-                if (this.messages.fetched) {
+                if (this.messages.fetched_flag) {
                     log.info(`Not re-fetching messages for ${this.get('jid')}`);
                     return;
                 }
-                this.messages.fetched = u.getResolveablePromise();
+                this.messages.fetched_flag = true;
                 const resolve = this.messages.fetched.resolve;
                 this.messages.fetch({
                     'add': true,
@@ -439,8 +440,9 @@ converse.plugins.add('converse-chat', {
              * @param { Promise<MessageAttributes> } attrs - A promise which resolves to the message attributes
              */
             queueMessage (attrs) {
-                this.msg_chain = (this.msg_chain || this.messages.fetched);
-                this.msg_chain = this.msg_chain.then(() => this.onMessage(attrs));
+                this.msg_chain = (this.msg_chain || this.messages.fetched)
+                    .then(() => this.onMessage(attrs))
+                    .catch(e => log.error(e));
                 return this.msg_chain;
             },
 
@@ -485,7 +487,8 @@ converse.plugins.add('converse-chat', {
                     log.error(e);
                 } finally {
                     delete this.msg_chain;
-                    delete this.messages.fetched;
+                    delete this.messages.fetched_flag;
+                    this.messages.fetched = u.getResolveablePromise();
                 }
             },
 
@@ -1009,12 +1012,19 @@ converse.plugins.add('converse-chat', {
             },
 
             /**
+             * Queue the creation of a message, to make sure that we don't run
+             * into a race condition whereby we're creating a new message
+             * before the collection has been fetched.
              * @async
              * @private
-             * @method _converse.ChatBox#createMessage
+             * @method _converse.ChatRoom#queueMessageCreation
+             * @param { Object } attrs
              */
-            createMessage (attrs, options) {
-                return this.messages.create(attrs, Object.assign({'wait': true, 'promise':true}, options)).catch(e => log.error(e));
+            async createMessage (attrs, options) {
+                attrs.time = attrs.time || (new Date()).toISOString();
+                await this.messages.fetched;
+                const p = this.messages.create(attrs, Object.assign({'wait': true, 'promise':true}, options));
+                return p;
             },
 
             /**

src/headless/converse-chatboxes.js

@@ -89,7 +89,6 @@ converse.plugins.add('converse-chatboxes', {
             }
         });
 
-
         async function createChatBox (jid, attrs, Model) {
             jid = Strophe.getBareJidFromJid(jid.toLowerCase());
             Object.assign(attrs, {'jid': jid, 'id': jid});
@@ -106,7 +105,6 @@ converse.plugins.add('converse-chatboxes', {
                 return null;
             }
             _converse.chatboxes.add(chatbox);
-            await chatbox.messages.fetched;
             return chatbox;
         }
 

src/headless/converse-muc.js

@@ -427,8 +427,8 @@ converse.plugins.add('converse-muc', {
                 if (this.session.get('connection_status') === converse.ROOMSTATUS.ENTERED && (await this.isJoined())) {
                     // We've restored the room from cache and we're still joined.
                     await new Promise(resolve => this.features.fetch({'success': resolve, 'error': resolve}));
-                    await this.fetchOccupants();
-                    await this.fetchMessages();
+                    await this.fetchOccupants().catch(e => log.error(e));
+                    await this.fetchMessages().catch(e => log.error(e));
                     return true;
                 } else {
                     await this.clearCache();