mam: distinguish between messages fetched before and after MUC join

spec/mam.js

@@ -19,7 +19,6 @@
             it("is queried when the user enters a new MUC",
                     mock.initConverse(['discoInitialized'], {'archived_messages_page_size': 2}, async function (done, _converse) {
 
-                spyOn(_converse.ChatBox.prototype, 'fetchArchivedMessages').and.callThrough();
                 const sent_IQs = _converse.connection.IQ_stanzas;
                 const muc_jid = 'orchard@chat.shakespeare.lit';
                 await test_utils.openAndEnterChatRoom(_converse, muc_jid, 'romeo');
@@ -196,7 +195,6 @@
                     </iq>`);
                 _converse.connection._dataRecv(test_utils.createRequest(result));
                 await u.waitUntil(() => view.model.messages.length === 5);
-                expect(view.model.fetchArchivedMessages.calls.count()).toBe(5);
                 const msg_els = view.content.querySelectorAll('.chat-msg__text');
                 expect(Array.from(msg_els).map(e => e.textContent).join(' ')).toBe("2nd Message 3rd Message 4th Message 5th Message 6th Message");
                 done();

spec/muc_messages.js

@@ -979,7 +979,7 @@
                 await u.waitUntil(() => view.el.querySelector('.chat-msg__text').textContent ===
                     'hello z3r0 gibson sw0rdf1sh, how are you?', 500);
 
-                const correction = _converse.connection.send.calls.all()[3].args[0];
+                const correction = _converse.connection.send.calls.all()[2].args[0];
                 expect(correction.toLocaleString())
                     .toBe(`<message from="romeo@montague.lit/orchard" id="${correction.nodeTree.getAttribute("id")}" `+
                             `to="lounge@montague.lit" type="groupchat" `+

src/headless/converse-mam.js

@@ -165,16 +165,6 @@ converse.plugins.add('converse-mam', {
 
 
         Object.assign(_converse.ChatRoom.prototype, {
-            fetchArchivedMessagesIfNecessary () {
-                const conn_status = this.get('connection_status');
-                if ((!_converse.muc_show_logs_before_join && conn_status !== converse.ROOMSTATUS.ENTERED) ||
-                        !this.features.get('mam_enabled') ||
-                        this.get('mam_initialized')) {
-                    return;
-                }
-                this.fetchNewestMessages();
-                this.save({'mam_initialized': true});
-            }
         });
 
 
@@ -219,7 +209,6 @@ converse.plugins.add('converse-mam', {
             }
         };
 
-        /************************ BEGIN Event Handlers ************************/
         function getMAMPrefsFromFeature (feature) {
             const prefs = feature.get('preferences') || {};
             if (feature.get('var') !== Strophe.NS.MAM || _converse.message_archiving === undefined) {
@@ -232,12 +221,29 @@ converse.plugins.add('converse-mam', {
             }
         }
 
+        function preMUCJoinMAMFetch (room) {
+            if (!_converse.muc_show_logs_before_join ||
+                    !room.features.get('mam_enabled') ||
+                    room.get('connection_status') !== converse.ROOMSTATUS.ENTERED ||
+                    room.get('prejoin_mam_fetched')) {
+                return;
+            }
+            room.fetchNewestMessages();
+            room.save({'prejoin_mam_fetched': true});
+        }
+
+        /************************ BEGIN Event Handlers ************************/
         _converse.api.listen.on('addClientFeatures', () => _converse.api.disco.own.features.add(Strophe.NS.MAM));
         _converse.api.listen.on('serviceDiscovered', getMAMPrefsFromFeature);
-        _converse.api.listen.on('enteredNewRoom', room => room.features.get('mam_enabled') && room.fetchNewestMessages());
+        _converse.api.listen.on('chatRoomOpened', view => {
-        _converse.api.listen.on('chatRoomOpened', (view) => {
+            if (_converse.muc_show_logs_before_join) {
-            view.listenTo(view.model, 'change:connection_status', () => view.model.fetchArchivedMessagesIfNecessary());
+                // If we want to show MAM logs before entering the MUC, we need
+                // to be informed once it's clear that this MUC supports MAM.
+                view.model.features.on('change:mam_enabled', preMUCJoinMAMFetch(view.model));
+            }
         });
+        _converse.api.listen.on('enteredNewRoom', room => room.features.get('mam_enabled') && room.fetchNewestMessages());
+
 
         _converse.api.listen.on('chatReconnected', chat => {
             // XXX: For MUCs, we listen to enteredNewRoom instead

src/headless/converse-muc.js

@@ -413,15 +413,9 @@ converse.plugins.add('converse-muc', {
                     } else if (_converse.clear_messages_on_reconnection) {
                         await this.clearMessages();
                     }
-                    if (!u.isPersistableModel(this)) {
-                        // XXX: Happens during tests, nothing to do if this
-                        // is a hanging chatbox (i.e. not in the collection anymore).
-                        return;
-                    }
                     this.join();
                 } else if (!(await this.rejoinIfNecessary())) {
                     // We've restored the room from cache and we're still joined.
-                    await this.fetchMessages();
                     this.features.fetch();
                 }
             },
@@ -431,6 +425,7 @@ converse.plugins.add('converse-muc', {
                     if (_converse.muc_fetch_members) {
                         await this.occupants.fetchMembers();
                     }
+                    await this.fetchMessages();
                     /**
                      * Triggered when the user has entered a new MUC
                      * @event _converse#enteredNewRoom

tests/utils.js

@@ -148,13 +148,15 @@
     utils.getRoomFeatures = async function (_converse, room, server, features=[]) {
         const muc_jid = `${room}@${server}`.toLowerCase();
         const stanzas = _converse.connection.IQ_stanzas;
-        const index = stanzas.length-1;
+        // XXX How necessary is this?
+        const index = stanzas.length-2;
         const stanza = await u.waitUntil(() => _.filter(
             stanzas.slice(index),
             iq => iq.querySelector(
                 `iq[to="${muc_jid}"] query[xmlns="http://jabber.org/protocol/disco#info"]`
             )).pop());
 
+
         const features_stanza = $iq({
             'from': muc_jid,
             'id': stanza.getAttribute('id'),
@@ -308,6 +310,7 @@
         if (_converse.muc_fetch_members) {
             await utils.returnMemberLists(_converse, muc_jid, members);
         }
+        await view.model.messages.fetched;
     };
 
     utils.clearChatBoxMessages = function (converse, jid) {