Test that roster groups are fetched before contacts

updates #1405

dist/converse.js

@@ -68022,10 +68022,6 @@ _converse_headless_converse_core__WEBPACK_IMPORTED_MODULE_0__["default"].plugins
         }
       } else {
         try {
-          /* Make sure not to run fetchRosterContacts async, since we need
-           * the contacts to exist before processing contacts presence,
-           * which might come in the same BOSH request.
-           */
           await _converse.rostergroups.fetchRosterGroups();
 
           _converse.emit('rosterGroupsFetched');
@@ -68379,7 +68375,7 @@ _converse_headless_converse_core__WEBPACK_IMPORTED_MODULE_0__["default"].plugins
 
         if (collection.length === 0 || this.rosterVersioningSupported() && !_converse.session.get('roster_fetched')) {
           _converse.send_initial_presence = true;
-          await _converse.roster.fetchFromServer();
+          return _converse.roster.fetchFromServer();
         } else {
           _converse.emit('cachedRoster', collection);
         }
@@ -68810,13 +68806,13 @@ _converse_headless_converse_core__WEBPACK_IMPORTED_MODULE_0__["default"].plugins
         * Returns a promise which resolves once the groups have been
         * returned.
         */
-        return new Promise((resolve, reject) => {
+        return new Promise(success => {
           this.fetch({
-            silent: true,
+            success,
             // We need to first have all groups before
             // we can start positioning them, so we set
             // 'silent' to true.
-            success: resolve
+            silent: true
           });
         });
       }

spec/bookmarks.js

@@ -362,7 +362,7 @@
 
 
         it("can be retrieved from the XMPP server", mock.initConverse(
-            ['send'], ['chatBoxesFetched', 'roomsPanelRendered', 'rosterGroupsFetched'], {},
+            {'connection': ['send']}, ['chatBoxesFetched', 'roomsPanelRendered', 'rosterGroupsFetched'], {},
             async function (done, _converse) {
 
             await test_utils.waitUntilDiscoConfirmed(
@@ -453,7 +453,7 @@
         describe("The rooms panel", function () {
 
             it("shows a list of bookmarks", mock.initConverse(
-                ['send'], ['rosterGroupsFetched'], {},
+                {'connection': ['send']}, ['rosterGroupsFetched'], {},
                 async function (done, _converse) {
 
                 await test_utils.waitUntilDiscoConfirmed(
@@ -541,7 +541,7 @@
 
 
             it("remembers the toggle state of the bookmarks list", mock.initConverse(
-                ['send'], ['rosterGroupsFetched'], {},
+                {'connection': ['send']}, ['rosterGroupsFetched'], {},
                 async function (done, _converse) {
 
                 test_utils.openControlBox();

spec/roomslist.js

@@ -48,7 +48,7 @@
 
         it("uses bookmarks to determine groupchat names",
             mock.initConverse(
-                ['send'], ['rosterGroupsFetched', 'chatBoxesFetched'], {'view_mode': 'fullscreen'},
+                {'connection': ['send']}, ['rosterGroupsFetched', 'chatBoxesFetched'], {'view_mode': 'fullscreen'},
                 async function (done, _converse) {
 
             await test_utils.openAndEnterChatRoom(_converse, 'lounge', 'localhost', 'dummy');

spec/roster.js

@@ -30,6 +30,34 @@
 
     describe("The Contacts Roster", function () {
 
+        it("is populated once we have registered a presence handler",
+            mock.initConverse(
+                {'_converse': ['emit']}, null, {},
+                async function (done, _converse) {
+
+            const IQs = _converse.connection.IQ_stanzas;
+            await test_utils.waitUntil(() => _converse.emit.calls.all().map(c => c.args[0]).includes('rosterGroupsFetched'));
+            const node = await test_utils.waitUntil(
+                () => _.filter(IQs, iq => iq.nodeTree.querySelector('iq query[xmlns="jabber:iq:roster"]')).pop());
+            expect(_converse.emit.calls.all().map(c => c.args[0]).includes('rosterContactsFetched')).toBeFalsy();
+
+            expect(node.toLocaleString()).toBe(
+                `<iq id="${node.nodeTree.getAttribute('id')}" type="get" xmlns="jabber:client">`+
+                    `<query xmlns="jabber:iq:roster"/>`+
+                `</iq>`);
+            const result = $iq({
+                'to': _converse.connection.jid,
+                'type': 'result',
+                'id': node.nodeTree.getAttribute('id')
+            }).c('query', {
+                'xmlns': 'jabber:iq:roster'
+            }).c('item', {'jid': 'nurse@example.com'}).up()
+              .c('item', {'jid': 'romeo@example.com'})
+            _converse.connection._dataRecv(test_utils.createRequest(result));
+            await test_utils.waitUntil(() => _converse.emit.calls.all().map(c => c.args[0]).includes('rosterContactsFetched'));
+            done();
+        }));
+
         it("supports roster versioning",
             mock.initConverse(
                 null, ['rosterGroupsFetched'], {},

src/headless/converse-roster.js

@@ -88,13 +88,8 @@ converse.plugins.add('converse-roster', {
                 }
             } else {
                 try {
-                    /* Make sure not to run fetchRosterContacts async, since we need
-                     * the contacts to exist before processing contacts presence,
-                     * which might come in the same BOSH request.
-                     */
                     await _converse.rostergroups.fetchRosterGroups();
                     _converse.emit('rosterGroupsFetched');
-
                     await _converse.roster.fetchRosterContacts();
                     _converse.emit('rosterContactsFetched');
                 } catch (reason) {
@@ -407,7 +402,7 @@ converse.plugins.add('converse-roster', {
                 if (collection.length === 0 ||
                         (this.rosterVersioningSupported() && !_converse.session.get('roster_fetched'))) {
                     _converse.send_initial_presence = true;
-                    await _converse.roster.fetchFromServer();
+                    return _converse.roster.fetchFromServer();
                 } else {
                     _converse.emit('cachedRoster', collection);
                 }
@@ -780,12 +775,13 @@ converse.plugins.add('converse-roster', {
                 * Returns a promise which resolves once the groups have been
                 * returned.
                 */
-                return new Promise((resolve, reject) => {
+                return new Promise(success => {
                     this.fetch({
-                        silent: true, // We need to first have all groups before
+                        success,
-                                    // we can start positioning them, so we set
+                        // We need to first have all groups before
-                                    // 'silent' to true.
+                        // we can start positioning them, so we set
-                        success: resolve
+                        // 'silent' to true.
+                        silent: true,
                     });
                 });
             }

tests/mock.js

@@ -167,10 +167,8 @@
         }
 
         const connection = mock.mock_connection();
-        if (!_.isUndefined(spies)) {
+        if (!_.isNil(spies)) {
-            _.forEach(spies, function (method) {
+            _.forEach(spies.connection, method => spyOn(connection, method));
-                spyOn(connection, method);
-            });
         }
 
         const _converse = await converse.initialize(_.extend({
@@ -186,6 +184,10 @@
             'debug': false
         }, settings || {}));
 
+        if (!_.isNil(spies)) {
+            _.forEach(spies._converse, method => spyOn(_converse, method).and.callThrough());
+        }
+
         _converse.ChatBoxViews.prototype.trimChat = function () {};
 
         _converse.api.vcard.get = function (model, force) {