Only create new message models once messages have been fetched

Fixes #2241

spec/bookmarks.js

@@ -5,25 +5,27 @@ 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,
             [{'category': 'pubsub', 'type': 'pep'}],
             ['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')));

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;
             },
 
@@ -486,6 +488,7 @@ converse.plugins.add('converse-chat', {
                 } finally {
                     delete this.msg_chain;
                     delete this.messages.fetched;
+                    delete this.messages.fetched_flag;
                 }
             },
 
@@ -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;
         }