converse-muc: Fetch messages in the initialize method

Lately we've been fetching messages only after entering the MUC, so that
we already have occupants to attach to them (due to `fetchMembers` being
called before) and thereby avoid rerenders.

I've now moved message fetching into the `initialize` method and added
missing event handlers for attaching/removing the occupant from a
message as it comes online or goes offline.

We still avoid (some) rerenders because we fetch and wait for cached
occupants before fetching cached messages and we wait for `fetchMembers`
before triggering `enteredNewRoom` which causes MAM messages to be
fetched.

spec/muc.js

@@ -587,8 +587,16 @@
                     null, ['rosterGroupsFetched', 'chatBoxesFetched'], {},
                     async function (done, _converse) {
 
-                await test_utils.openChatRoom(_converse, "coven", 'chat.shakespeare.lit');
-                const view = _converse.chatboxviews.get('coven@chat.shakespeare.lit');
+                const sent_IQs = _converse.connection.IQ_stanzas;
+                const muc_jid = 'coven@chat.shakespeare.lit';
+                const room = Strophe.getNodeFromJid(muc_jid);
+                const server = Strophe.getDomainFromJid(muc_jid);
+                const nick = 'romeo';
+                await _converse.api.rooms.open(muc_jid);
+                await test_utils.getRoomFeatures(_converse, room, server);
+                await test_utils.waitForReservedNick(_converse, muc_jid, nick);
+
+                const view = _converse.chatboxviews.get(muc_jid);
                 const chat_content = view.el.querySelector('.chat-content');
                 /* <presence to="romeo@montague.lit/_converse.js-29092160"
                  *           from="coven@chat.shakespeare.lit/some1">

spec/muc_messages.js

@@ -180,6 +180,62 @@
             expect(view.model.messages.last().occupant.get('role')).toBe('moderator');
             expect(view.el.querySelectorAll('.chat-msg').length).toBe(3);
             expect(sizzle('.chat-msg', view.el).pop().classList.value.trim()).toBe('message chat-msg groupchat moderator owner');
+
+
+            const add_events = view.model.occupants._events.add.length;
+            msg = $msg({
+                from: 'lounge@montague.lit/some1',
+                id: (new Date()).getTime(),
+                to: 'romeo@montague.lit',
+                type: 'groupchat'
+            }).c('body').t('Message from someone not in the MUC right now').tree();
+            await view.model.onMessage(msg);
+            await new Promise((resolve, reject) => view.once('messageInserted', resolve));
+            expect(view.model.messages.last().occupant).toBeUndefined();
+            // Check that there's a new "add" event handler, for when the occupant appears.
+            expect(view.model.occupants._events.add.length).toBe(add_events+1);
+
+            // Check that the occupant gets added/removed to the message as it
+            // gets removed or added.
+            presence = $pres({
+                    to:'romeo@montague.lit/orchard',
+                    from:'lounge@montague.lit/some1',
+                    id: u.getUniqueId()
+            }).c('x').attrs({xmlns:'http://jabber.org/protocol/muc#user'})
+                .c('item').attrs({jid: 'some1@montague.lit/orchard'});
+            _converse.connection._dataRecv(test_utils.createRequest(presence));
+            await u.waitUntil(() => view.model.messages.last().occupant);
+            expect(view.model.messages.last().get('message')).toBe('Message from someone not in the MUC right now');
+            expect(view.model.messages.last().occupant.get('nick')).toBe('some1');
+            // Check that the "add" event handler was removed.
+            expect(view.model.occupants._events.add.length).toBe(add_events);
+
+            presence = $pres({
+                    to:'romeo@montague.lit/orchard',
+                    type: 'unavailable',
+                    from:'lounge@montague.lit/some1',
+                    id: u.getUniqueId()
+            }).c('x').attrs({xmlns:'http://jabber.org/protocol/muc#user'})
+                .c('item').attrs({jid: 'some1@montague.lit/orchard'});
+            _converse.connection._dataRecv(test_utils.createRequest(presence));
+            await u.waitUntil(() => !view.model.messages.last().occupant);
+            expect(view.model.messages.last().get('message')).toBe('Message from someone not in the MUC right now');
+            expect(view.model.messages.last().occupant).toBeUndefined();
+            // Check that there's a new "add" event handler, for when the occupant appears.
+            expect(view.model.occupants._events.add.length).toBe(add_events+1);
+
+            presence = $pres({
+                    to:'romeo@montague.lit/orchard',
+                    from:'lounge@montague.lit/some1',
+                    id: u.getUniqueId()
+            }).c('x').attrs({xmlns:'http://jabber.org/protocol/muc#user'})
+                .c('item').attrs({jid: 'some1@montague.lit/orchard'});
+            _converse.connection._dataRecv(test_utils.createRequest(presence));
+            await u.waitUntil(() => view.model.messages.last().occupant);
+            expect(view.model.messages.last().get('message')).toBe('Message from someone not in the MUC right now');
+            expect(view.model.messages.last().occupant.get('nick')).toBe('some1');
+            // Check that the "add" event handler was removed.
+            expect(view.model.occupants._events.add.length).toBe(add_events);
             done();
         }));
 

src/headless/converse-chatboxes.js

@@ -354,6 +354,7 @@ converse.plugins.add('converse-chatboxes', {
                     'success': _.flow(this.afterMessagesFetched.bind(this), resolve),
                     'error': _.flow(this.afterMessagesFetched.bind(this), resolve)
                 });
+                return this.messages.fetched;
             },
 
             clearMessages () {

src/headless/converse-muc.js

@@ -264,18 +264,38 @@ converse.plugins.add('converse-muc', {
                         }
                     }, 10000);
                 } else {
-                    this.occupantAdded = u.getResolveablePromise();
                     this.setOccupant();
                     this.setVCard();
                 }
             },
 
+            onOccupantRemoved (occupant) {
+                delete this.occupant;
+                const chatbox = _.get(this, 'collection.chatbox');
+                chatbox.occupants.on('add', this.onOccupantAdded, this);
+            },
+
+            onOccupantAdded (occupant) {
+                if (occupant.get('nick') === Strophe.getResourceFromJid(this.get('from'))) {
+                    this.occupant = occupant;
+                    this.occupant.on('destroy', this.onOccupantRemoved, this);
+                    const chatbox = _.get(this, 'collection.chatbox');
+                    chatbox.occupants.off('add', this.onOccupantAdded, this);
+                }
+            },
+
             setOccupant () {
+                if (this.get('type') !== 'groupchat') { return; }
                 const chatbox = _.get(this, 'collection.chatbox');
                 if (!chatbox) { return; }
                 const nick = Strophe.getResourceFromJid(this.get('from'));
                 this.occupant = chatbox.occupants.findWhere({'nick': nick});
-                this.occupantAdded.resolve();
+                if (this.occupant) {
+                    this.occupant.on('destroy', this.onOccupantRemoved, this);
+                } else {
+                    chatbox.occupants.on('add', this.onOccupantAdded, this);
+                }
+
             },
 
             getVCardForChatroomOccupant () {
@@ -373,7 +393,7 @@ converse.plugins.add('converse-muc', {
                 }
             },
 
-            initialize() {
+            async initialize() {
                 if (_converse.vcards) {
                     this.vcard = _converse.vcards.findWhere({'jid': this.get('jid')}) ||
                         _converse.vcards.create({'jid': this.get('jid')});
@@ -384,9 +404,11 @@ converse.plugins.add('converse-muc', {
                 this.on('change:chat_state', this.sendChatState, this);
                 this.on('change:connection_status', this.onConnectionStatusChanged, this);
 
-                this.initOccupants();
-                this.registerHandlers();
                 this.initMessages();
+                this.registerHandlers();
+
+                await this.initOccupants();
+                await this.fetchMessages();
                 this.enterRoom();
             },
 
@@ -412,7 +434,6 @@ converse.plugins.add('converse-muc', {
                 } else if (!(await this.rejoinIfNecessary())) {
                     // We've restored the room from cache and we're still joined.
                     this.features.fetch();
-                    this.fetchMessages();
                 }
             },
 
@@ -421,15 +442,8 @@ converse.plugins.add('converse-muc', {
                     if (_converse.muc_fetch_members) {
                         await this.occupants.fetchMembers();
                     }
-                    // It's possible to fetch messages before entering a MUC,
-                    // but we don't support this use-case currently. By
-                    // fetching messages after members we can immediately
-                    // assign an occupant to the message before rendering it,
-                    // thereby avoiding re-renders (and therefore DOM reflows).
-                    this.fetchMessages();
-
                     /**
-                     * Triggered when the user has entered a new MUC and *after* cached messages have been fetched.
+                     * Triggered when the user has entered a new MUC
                      * @event _converse#enteredNewRoom
                      * @type { _converse.ChatRoom}
                      * @example _converse.api.listen.on('enteredNewRoom', model => { ... });
@@ -480,12 +494,11 @@ converse.plugins.add('converse-muc', {
                         'error': resolve
                     });
                 });
+                return this.occupants.fetched;
             },
 
             registerHandlers () {
-                /* Register presence and message handlers for this chat
-                 * groupchat
-                 */
+                // Register presence and message handlers for this groupchat
                 const room_jid = this.get('jid');
                 this.removeHandlers();
                 this.presence_handler = _converse.connection.addHandler(stanza => {
@@ -2288,3 +2301,5 @@ converse.plugins.add('converse-muc', {
         /************************ END API ************************/
     }
 });
+
+