From c4ad02d4e3045d5de4268e142e4f9a6b76ca6a2c Mon Sep 17 00:00:00 2001 From: JC Brand Date: Fri, 26 Jul 2019 13:32:21 +0200 Subject: [PATCH] New config setting: `muc_fetch_members` --- CHANGES.md | 1 + docs/source/configuration.rst | 23 +++++++++ spec/muc.js | 88 ++++++++++++++++++++++++----------- src/headless/converse-muc.js | 61 +++++++++++++----------- src/headless/utils/muc.js | 5 +- tests/utils.js | 4 +- 6 files changed, 126 insertions(+), 56 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index e2aa12053..7c97301bb 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -21,6 +21,7 @@ - New config option [clear_messages_on_reconnection](https://conversejs.org/docs/html/configuration.html#clear-messages-on-reconnection) - New config option [enable_smacks](https://conversejs.org/docs/html/configuration.html#enable-smacks) - New config option [message_limit](https://conversejs.org/docs/html/configuration.html#message-limit) +- New config option [muc_fetch_members](https://conversejs.org/docs/html/configuration.html#muc-fetch-members) - New config option [muc_mention_autocomplete_min_chars](https://conversejs.org/docs/html/configuration.html#muc-mention-autocomplete-min-chars) - New config option [muc_show_join_leave_status](https://conversejs.org/docs/html/configuration.html#muc-show-join-leave-status) - New config option [singleton](https://conversejs.org/docs/html/configuration.html#singleton) diff --git a/docs/source/configuration.rst b/docs/source/configuration.rst index 6b901b295..dd3a63793 100644 --- a/docs/source/configuration.rst +++ b/docs/source/configuration.rst @@ -451,6 +451,7 @@ Example: .. _`bosh-service-url`: + bosh_service_url ---------------- @@ -954,6 +955,28 @@ other domains. If you want to restrict MUCs to only this domain, then set `locked_domain`_ to ``true``. + +muc_fetch_members +----------------- + +* Default: ``true`` + +Determines whether Converse.js will fetch the member lists for a MUC +(multi-user chat) when the user first enters it. + +Here's the relevant part from the MUC XEP: https://xmpp.org/extensions/xep-0045.html#getmemberlist + +The MUC service won't necessarily allow any user to fetch member lists, +but can usually be configured to do so. + +The member lists consists of three lists of users who have the affiliations +``member``, ``admin`` and ``owner`` respectively. + +By fetching member lists, Converse.js will always show these users as +participants of the MUC, which makes it behave a bit more like modern chat +apps. + + muc_history_max_stanzas ----------------------- diff --git a/spec/muc.js b/spec/muc.js index c99321f8c..68ca1f479 100644 --- a/spec/muc.js +++ b/spec/muc.js @@ -400,13 +400,34 @@ describe("A Groupchat", function () { + + describe("upon being entered", function () { + it("will fetch the member list if muc_fetch_members is true", + mock.initConverse( + null, ['rosterGroupsFetched'], {'muc_fetch_members': true}, + async function (done, _converse) { + + spyOn(_converse.ChatRoomOccupants.prototype, 'fetchMembers').and.callThrough(); + await test_utils.openAndEnterChatRoom(_converse, 'lounge@montague.lit', 'romeo'); + let view = _converse.chatboxviews.get('lounge@montague.lit'); + expect(view.model.occupants.fetchMembers).toHaveBeenCalled(); + + _converse.muc_fetch_members = false; + await test_utils.openAndEnterChatRoom(_converse, 'orchard@montague.lit', 'romeo'); + view = _converse.chatboxviews.get('orchard@montague.lit'); + expect(view.model.occupants.fetchMembers.calls.count()).toBe(1); + done(); + })); + }); + it("clears cached messages when it gets closed", mock.initConverse( null, ['rosterGroupsFetched'], {}, async function (done, _converse) { - await test_utils.openAndEnterChatRoom(_converse, 'lounge@montague.lit', 'romeo'); - const view = _converse.chatboxviews.get('lounge@montague.lit'); + const muc_jid = 'lounge@montague.lit'; + await test_utils.openAndEnterChatRoom(_converse, muc_jid , 'romeo'); + const view = _converse.chatboxviews.get(muc_jid); const message = 'Hello world', nick = mock.chatroom_names[0], msg = $msg({ @@ -4185,14 +4206,10 @@ null, ['rosterGroupsFetched', 'chatBoxesFetched'], {}, async function (done, _converse) { - var sent_IQs = [], IQ_ids = []; + spyOn(_converse.ChatRoomOccupants.prototype, 'fetchMembers').and.callThrough(); const sendIQ = _converse.connection.sendIQ; const IQ_stanzas = _converse.connection.IQ_stanzas; const muc_jid = 'coven@chat.shakespeare.lit'; - spyOn(_converse.connection, 'sendIQ').and.callFake(function (iq, callback, errback) { - sent_IQs.push(iq); - IQ_ids.push(sendIQ.bind(this)(iq, callback, errback)); - }); await _converse.api.rooms.open(muc_jid, {'nick': 'romeo'}); let stanza = await u.waitUntil(() => _.filter( @@ -4206,11 +4223,18 @@ ``+ ``); - const view = _converse.chatboxviews.get('coven@chat.shakespeare.lit'); + const sent_IQs = _converse.connection.IQ_stanzas; + const last_sent_IQ = sent_IQs.pop(); + expect(Strophe.serialize(last_sent_IQ)).toBe( + ``+ + ``+ + ``); + + const view = _converse.chatboxviews.get(muc_jid); // State that the chat is members-only via the features IQ const features_stanza = $iq({ from: 'coven@chat.shakespeare.lit', - 'id': IQ_ids.pop(), + 'id': last_sent_IQ.getAttribute('id'), 'to': 'romeo@montague.lit/desktop', 'type': 'result' }) @@ -4244,24 +4268,23 @@ // Check in reverse order that we requested all three lists // (member, owner and admin). - const admin_iq_id = IQ_ids.pop(); - const owner_iq_id = IQ_ids.pop(); - const member_iq_id = IQ_ids.pop(); - - expect(sent_IQs.pop().toLocaleString()).toBe( - ``+ + const admin_iq = sent_IQs.pop(); + const owner_iq = sent_IQs.pop(); + const member_iq = sent_IQs.pop(); + expect(Strophe.serialize(admin_iq)).toBe( + ``+ ``+ ``+ ``+ ``); - expect(sent_IQs.pop().toLocaleString()).toBe( - ``+ + expect(Strophe.serialize(owner_iq)).toBe( + ``+ ``+ ``+ ``+ ``); - expect(sent_IQs.pop().toLocaleString()).toBe( - ``+ + expect(Strophe.serialize(member_iq)).toBe( + ``+ ``+ ``+ ``+ @@ -4281,9 +4304,9 @@ * * */ - var member_list_stanza = $iq({ + const member_list_stanza = $iq({ 'from': 'coven@chat.shakespeare.lit', - 'id': member_iq_id, + 'id': member_iq.getAttribute('id'), 'to': 'romeo@montague.lit/orchard', 'type': 'result' }).c('query', {'xmlns': Strophe.NS.MUC_ADMIN}) @@ -4295,9 +4318,9 @@ }); _converse.connection._dataRecv(test_utils.createRequest(member_list_stanza)); - var admin_list_stanza = $iq({ + const admin_list_stanza = $iq({ 'from': 'coven@chat.shakespeare.lit', - 'id': admin_iq_id, + 'id': admin_iq.getAttribute('id'), 'to': 'romeo@montague.lit/orchard', 'type': 'result' }).c('query', {'xmlns': Strophe.NS.MUC_ADMIN}) @@ -4308,9 +4331,9 @@ }); _converse.connection._dataRecv(test_utils.createRequest(admin_list_stanza)); - var owner_list_stanza = $iq({ + const owner_list_stanza = $iq({ 'from': 'coven@chat.shakespeare.lit', - 'id': owner_iq_id, + 'id': owner_iq.getAttribute('id'), 'to': 'romeo@montague.lit/orchard', 'type': 'result' }).c('query', {'xmlns': Strophe.NS.MUC_ADMIN}) @@ -4319,20 +4342,31 @@ 'jid': 'crone1@shakespeare.lit', }); _converse.connection._dataRecv(test_utils.createRequest(owner_list_stanza)); - await u.waitUntil(() => IQ_ids.length, 300); + stanza = await u.waitUntil(() => _.filter( IQ_stanzas, iq => iq.querySelector( `iq[to="${muc_jid}"] query[xmlns="http://jabber.org/protocol/muc#admin"]` )).pop()); expect(stanza.outerHTML, - ``+ + ``+ ``+ ``+ `Please join this groupchat`+ ``+ ``+ ``); + + const result = $iq({ + 'from': 'coven@chat.shakespeare.lit', + 'id': stanza.getAttribute('id'), + 'to': 'romeo@montague.lit/orchard', + 'type': 'result' + }); + _converse.connection._dataRecv(test_utils.createRequest(result)); + + await u.waitUntil(() => view.model.occupants.fetchMembers.calls.count()); + // Finally check that the user gets invited. expect(sent_stanza.toLocaleString()).toBe( // Strophe adds the xmlns attr (although not in spec) ``+ diff --git a/src/headless/converse-muc.js b/src/headless/converse-muc.js index 910d3c8e0..1dffab259 100644 --- a/src/headless/converse-muc.js +++ b/src/headless/converse-muc.js @@ -116,6 +116,7 @@ converse.plugins.add('converse-muc', { 'auto_register_muc_nickname': false, 'locked_muc_domain': false, 'muc_domain': undefined, + 'muc_fetch_members': true, 'muc_history_max_stanzas': undefined, 'muc_instant_rooms': true, 'muc_nickname_from_jid': false @@ -417,7 +418,9 @@ converse.plugins.add('converse-muc', { async onConnectionStatusChanged () { if (this.get('connection_status') === converse.ROOMSTATUS.ENTERED) { - await this.occupants.fetchMembers(); + 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 @@ -856,18 +859,11 @@ converse.plugins.add('converse-muc', { * @param { object } members - A map of jids, affiliations and * optionally reasons. Only those entries with the * same affiliation as being currently set will be considered. - * @returns - * A promise which resolves and fails depending on the XMPP server response. + * @returns { Promise } A promise which resolves and fails depending on the XMPP server response. */ setAffiliation (affiliation, members) { - members = _.filter(members, (member) => - // We only want those members who have the right - // affiliation (or none, which implies the provided one). - _.isUndefined(member.affiliation) || - member.affiliation === affiliation - ); - const promises = _.map(members, _.bind(this.sendAffiliationIQ, this, affiliation)); - return Promise.all(promises); + members = members.filter(m => _.isUndefined(m.affiliation) || m.affiliation === affiliation); + return Promise.all(members.map(m => this.sendAffiliationIQ(affiliation, m))); }, /** @@ -1050,18 +1046,20 @@ converse.plugins.add('converse-muc', { }, /** - * Send IQ stanzas to the server to modify the - * affiliations in this groupchat. + * Send IQ stanzas to the server to modify affiliations for users in this groupchat. + * * See: https://xmpp.org/extensions/xep-0045.html#modifymember * @private * @method _converse.ChatRoom#setAffiliations - * @param { object } members - A map of jids, affiliations and optionally reasons - * @param { function } onSuccess - callback for a succesful response - * @param { function } onError - callback for an error response + * @param { Object[] } members + * @param { string } members[].jid - The JID of the user whose affiliation will change + * @param { Array } members[].affiliation - The new affiliation for this user + * @param { string } [members[].reason] - An optional reason for the affiliation change + * @returns { Promise } */ setAffiliations (members) { - const affiliations = _.uniq(_.map(members, 'affiliation')); - return Promise.all(_.map(affiliations, _.partial(this.setAffiliation.bind(this), _, members))); + const affiliations = _.uniq(members.map(m => m.affiliation)); + return Promise.all(affiliations.map(a => this.setAffiliation(a, members))); }, /** @@ -1101,10 +1099,15 @@ converse.plugins.add('converse-muc', { this.occupants.findWhere({'nick': nick_or_jid}); }, + /** + * Returns a map of JIDs that have the affiliations + * as provided. + * @private + * @method _converse.ChatRoom#getJidsWithAffiliations + * @param { string|array } affiliation - An array of affiliations or + * a string if only one affiliation. + */ async getJidsWithAffiliations (affiliations) { - /* Returns a map of JIDs that have the affiliations - * as provided. - */ if (_.isString(affiliations)) { affiliations = [affiliations]; } @@ -1135,11 +1138,17 @@ converse.plugins.add('converse-muc', { * updated or once it's been established there's no need * to update the list. */ - updateMemberLists (members, affiliations, deltaFunc) { - this.getJidsWithAffiliations(affiliations) - .then(old_members => this.setAffiliations(deltaFunc(members, old_members))) - .then(() => this.occupants.fetchMembers()) - .catch(_.partial(_converse.log, _, Strophe.LogLevel.ERROR)); + async updateMemberLists (members, affiliations, deltaFunc) { + try { + const old_members = await this.getJidsWithAffiliations(affiliations); + await this.setAffiliations(deltaFunc(members, old_members)); + } catch (e) { + _converse.log(e, Strophe.LogLevel.ERROR); + return; + } + if (_converse.muc_fetch_members) { + return this.occupants.fetchMembers(); + } }, /** diff --git a/src/headless/utils/muc.js b/src/headless/utils/muc.js index d0f343a6e..4c6a43eb1 100644 --- a/src/headless/utils/muc.js +++ b/src/headless/utils/muc.js @@ -39,10 +39,11 @@ const { Strophe, sizzle, _ } = converse.env; * to 'none'. * @param { array } new_list - Array containing the new affiliations * @param { array } old_list - Array containing the old affiliations + * @returns { array } */ u.computeAffiliationsDelta = function computeAffiliationsDelta (exclude_existing, remove_absentees, new_list, old_list) { - const new_jids = _.map(new_list, 'jid'); - const old_jids = _.map(old_list, 'jid'); + const new_jids = new_list.map(o => o.jid); + const old_jids = old_list.map(o => o.jid); // Get the new affiliations let delta = _.map( diff --git a/tests/utils.js b/tests/utils.js index 353ba5501..5440d325d 100644 --- a/tests/utils.js +++ b/tests/utils.js @@ -287,7 +287,9 @@ const view = _converse.chatboxviews.get(muc_jid); await u.waitUntil(() => (view.model.get('connection_status') === converse.ROOMSTATUS.ENTERED)); - await utils.returnMemberLists(_converse, muc_jid, members); + if (_converse.muc_fetch_members) { + await utils.returnMemberLists(_converse, muc_jid, members); + } }; utils.clearBrowserStorage = function () {