Browse Source

converse-muc: Only send the delta when setting the member list.

As required by XEP-0045.

This requires that we first fetch the existing member list, compute the delta
and then send that if there is one.
JC Brand 8 years ago
parent
commit
6c725409c2
3 changed files with 179 additions and 5 deletions
  1. 38 2
      spec/chatroom.js
  2. 139 2
      src/converse-muc.js
  3. 2 1
      tests/mock.js

+ 38 - 2
spec/chatroom.js

@@ -1434,8 +1434,6 @@
                 var view = converse.chatboxviews.get('coven@chat.shakespeare.lit');
                 expect(view.model.get('membersonly')).toBeTruthy();
 
-                spyOn(view, 'setMemberList').andCallThrough();
-
                 test_utils.createContacts(converse, 'current');
 
                 var sent_stanza,
@@ -1452,6 +1450,43 @@
                 var reason = "Please join this chat room";
                 view.directInvite(invitee_jid, reason);
 
+                // Check that we first requested the member list
+                expect(sent_IQ.toLocaleString()).toBe(
+                    "<iq to='coven@chat.shakespeare.lit' type='get' xmlns='jabber:client' id='"+IQ_id+"'>"+
+                        "<query xmlns='http://jabber.org/protocol/muc#admin'>"+
+                            "<item affiliation='member'/>"+
+                        "</query>"+
+                    "</iq>");
+
+                /* Now the service sends the member list to the user
+                 *
+                 *  <iq from='coven@chat.shakespeare.lit'
+                 *      id='member3'
+                 *      to='crone1@shakespeare.lit/desktop'
+                 *      type='result'>
+                 *  <query xmlns='http://jabber.org/protocol/muc#admin'>
+                 *      <item affiliation='member'
+                 *          jid='hag66@shakespeare.lit'
+                 *          nick='thirdwitch'
+                 *          role='participant'/>
+                 *  </query>
+                 *  </iq>
+                 */
+                var member_list_stanza = $iq({
+                        'from': 'coven@chat.shakespeare.lit',
+                        'id': IQ_id,
+                        'to': 'dummy@localhost/resource',
+                        'type': 'result'
+                    }).c('query', {'xmlns': Strophe.NS.MUC_ADMIN})
+                        .c('item', {
+                            'affiliation': 'member',
+                            'jid': 'hag66@shakespeare.lit',
+                            'nick': 'thirdwitch',
+                            'role': 'participant'
+                        });
+                converse.connection._dataRecv(test_utils.createRequest(member_list_stanza));
+
+                // Check that the member list now gets updated
                 expect(sent_IQ.toLocaleString()).toBe(
                     "<iq to='coven@chat.shakespeare.lit' type='set' xmlns='jabber:client' id='"+IQ_id+"'>"+
                         "<query xmlns='http://jabber.org/protocol/muc#admin'>"+
@@ -1459,6 +1494,7 @@
                         "</query>"+
                     "</iq>");
 
+                // Finally check that the user gets invited.
                 expect(sent_stanza.toLocaleString()).toBe( // Strophe adds the xmlns attr (although not in spec)
                     "<message from='dummy@localhost/resource' to='"+invitee_jid+"' id='"+sent_id+"' xmlns='jabber:client'>"+
                         "<x xmlns='jabber:x:conference' jid='coven@chat.shakespeare.lit' reason='Please join this chat room'/>"+

+ 139 - 2
src/converse-muc.js

@@ -494,7 +494,93 @@
                     this.insertIntoTextArea(ev.target.textContent);
                 },
 
-                setMemberList: function (members, onSuccess, onError) {
+                requestMemberList: function (onSuccess, onError, include_admins, include_owners) {
+                    /* Send an IQ stanza to the server, asking it for the
+                     * member-list of this room.
+                     *
+                     * See: http://xmpp.org/extensions/xep-0045.html#modifymember
+                     *
+                     * Parameters:
+                     *  (Boolean) include_admins: Whether the admin list should
+                     *      be included.
+                     *  (Boolean) include_owners: Whether the owner list should
+                     *      be included.
+                     *
+                     * Returns:
+                     *  A promise which resolves once the list has been
+                     *  retrieved.
+                     */
+                    var deferred = new $.Deferred();
+                    var iq = $iq({to: this.model.get('jid'), type: "get"})
+                        .c("query", {xmlns: Strophe.NS.MUC_ADMIN})
+                            .c("item", {'affiliation': 'member'}).up();
+                        if (include_admins) {
+                            iq.c("item", {'affiliation': 'admin'}).up();
+                        }
+                        if (include_owners) {
+                            iq.c("item", {'affiliation': 'owner'});
+                        }
+                    converse.connection.sendIQ(iq, deferred.resolve, deferred.reject);
+                    return deferred.promise();
+                },
+
+                parseMemberListIQ: function (iq) {
+                    /* Given an IQ stanza with a member list, create an array of member
+                     * objects.
+                     */
+                    return _.map(
+                        $(iq).find('query[xmlns="'+Strophe.NS.MUC_ADMIN+'"] item'),
+                        function (item) {
+                            return {
+                                'jid': item.getAttribute('jid'),
+                                'affiliation': item.getAttribute('affiliation'),
+                            };
+                        }
+                    );
+                },
+
+                computeMemberListDelta: function (new_list, old_list, remove_absentees) {
+                    /* Given two members lists (arrays of objects with jid and
+                     * affiliation properties), return a new list containing
+                     * those objects that are new, changed or removed
+                     * (depending on the 'remove_absentees' boolean).
+                     *
+                     * The objects for new and changed members stay the same,
+                     * for removed members, the affiliation is set to 'none'.
+                     *
+                     * Parameters:
+                     *  (Array) new_list: List of new objects with properties
+                     *      'jid' and 'affiliation'.
+                     *  (Array) new_list: List of old objects with properties
+                     *      'jid' and 'affiliation'.
+                     *  (Boolean) remove_absentees: Indicates whether members
+                     *      from the old list which are not in the new list
+                     *      should be considered removed (e.g. affiliation set
+                     *      to 'none').
+                     */
+                    var new_jids = _.pluck(new_list, 'jid');
+                    var old_jids = _.pluck(old_list, 'jid');
+                    var added_jids = _.without(new_jids, old_jids);
+                    var changed_jids = _.intersection(new_jids, old_jids);
+
+                    var list_delta = _.filter(new_list, function (item) {
+                            return _.contains(added_jids, item.jid) ||
+                                    _.contains(changed_jids, item.jid);
+                        });
+                    if (remove_absentees) {
+                        list_delta = list_delta.concat(
+                            _.map(_.without(old_jids, new_jids), function (jid) {
+                                    return {
+                                        'jid': jid,
+                                        'affiliation': 'none'
+                                    };
+                                })
+                            );
+                    }
+                    return list_delta;
+                },
+
+                sendMemberListStanza: function (onSuccess, onError, members) {
                     /* Send an IQ stanza to the server to modify the
                      * member-list of this room.
                      *
@@ -506,6 +592,12 @@
                      *  (Function) onSuccess: callback for a succesful response
                      *  (Function) onError: callback for an error response
                      */
+                    if (!members.length) {
+                        // Succesfully updated the list with zero new or
+                        // changed members :)
+                        onSuccess(null);
+                        return;
+                    }
                     var iq = $iq({to: this.model.get('jid'), type: "set"})
                         .c("query", {xmlns: Strophe.NS.MUC_ADMIN});
                     _.each(members, function (member) {
@@ -517,6 +609,51 @@
                     return converse.connection.sendIQ(iq, onSuccess, onError);
                 },
 
+                updateMemberList: function (members, remove_absentees,
+                                            include_admins, include_owners) {
+                    /* Fetch the member list (optionally including admins and
+                     * owners), then compute the delta between that list and
+                     * the passed in members, and if it exists, send the delta
+                     * to the XMPP server to update the member list.
+                     *
+                     * Parameters:
+                     *  (Array) members: Array of objects with properties 'jid'
+                     *      and 'affiliation'.
+                     *  (Boolean) remove_absentees: When computing the list
+                     *      delta, consider members from the old list that
+                     *      are not in the new list as removed (therefore their
+                     *      affiliation will get set to 'none').
+                     *  (Boolean) include_admins: Whether room admins are also
+                     *      considered.
+                     *  (Boolean) include_owners: Whether room owners are also
+                     *      considered.
+                     *
+                     * The include_admins and include_owners options have
+                     * access-control implications. Usually only room owners
+                     * can change those lists.
+                     *
+                     * Returns:
+                     *  A promise which is resolved once the list has been
+                     *  updated or once it's been established there's no need
+                     *  to update the list.
+                     */
+                    var deferred = new $.Deferred();
+                    var computeDelta = _.partial(
+                        this.computeMemberListDelta, members, _, remove_absentees
+                    );
+                    this.requestMemberList(include_admins, include_owners).then(
+                        _.compose(
+                            _.partial(
+                                this.sendMemberListStanza.bind(this),
+                                deferred.resolve,
+                                deferred.reject
+                            ),
+                            _.compose(computeDelta, this.parseMemberListIQ)
+                        )
+                    );
+                    return deferred.promise();
+                },
+
                 directInvite: function (recipient, reason) {
                     /* Send a direct invitation as per XEP-0249
                      *
@@ -528,7 +665,7 @@
                         // When inviting to a members-only room, we first add
                         // the person to the member list, otherwise they won't
                         // be able to join.
-                        this.setMemberList([{
+                        this.updateMemberList([{
                             'jid': recipient,
                             'affiliation': 'member'
                         }]);

+ 2 - 1
tests/mock.js

@@ -86,7 +86,8 @@
                 no_trimming: true,
                 auto_login: true,
                 jid: 'dummy@localhost',
-                password: 'secret'
+                password: 'secret',
+                debug: true
             }, settings || {}));
             converse.ChatBoxViews.prototype.trimChat = function () {};
             return func(converse);