Переглянути джерело

Fixes #1586. Not possible to kick someone with space in nick

Refactored moderation by moving certain methods to the model and
consolidating setting of roles and affiliations into new methods.
JC Brand 6 роки тому
батько
коміт
be0274f1f0
4 змінених файлів з 202 додано та 185 видалено
  1. 1 0
      CHANGES.md
  2. 29 29
      spec/muc.js
  3. 120 148
      src/converse-muc-views.js
  4. 52 8
      src/headless/converse-muc.js

+ 1 - 0
CHANGES.md

@@ -29,6 +29,7 @@
 - #1558: `this.get` is not a function error when `forward_messages` is set to `true`.
 - #1572: In `fullscreen` view mode the top is cut off on iOS
 - #1576: Converse gets stuck with spinner when logging out with `auto_login` set to `true`
+- #1586: Not possible to kick someone with a space in their nickname
 
 - **Breaking changes**:
 - Rename `muc_disable_moderator_commands` to [muc_disable_slash_commands](https://conversejs.org/docs/html/configuration.html#muc-disable-slash-commands).

+ 29 - 29
spec/muc.js

@@ -2802,7 +2802,7 @@
                 expect(_converse.connection.send).not.toHaveBeenCalled();
                 expect(view.el.querySelectorAll('.chat-error').length).toBe(1);
                 expect(view.el.querySelector('.chat-error').textContent.trim())
-                    .toBe(`Error: couldn't find a groupchat participant "chris"`)
+                    .toBe('Error: couldn\'t find a groupchat participant based on your arguments');
 
                 // Now test with an existing nick
                 textarea.value = '/member marc Welcome to the club!';
@@ -2993,7 +2993,7 @@
                 const view = _converse.chatboxviews.get('lounge@localhost');
                 spyOn(view.model, 'setAffiliation').and.callThrough();
                 spyOn(view, 'showErrorMessage').and.callThrough();
-                spyOn(view, 'validateRoleChangeCommand').and.callThrough();
+                spyOn(view, 'validateRoleOrAffiliationChangeArgs').and.callThrough();
 
                 let presence = $pres({
                         'from': 'lounge@localhost/annoyingGuy',
@@ -3015,7 +3015,7 @@
                     preventDefault: _.noop,
                     keyCode: 13
                 });
-                expect(view.validateRoleChangeCommand).toHaveBeenCalled();
+                expect(view.validateRoleOrAffiliationChangeArgs).toHaveBeenCalled();
                 expect(view.showErrorMessage).toHaveBeenCalledWith(
                     "Error: the \"owner\" command takes two arguments, the user's nickname and optionally a reason.");
                 expect(view.model.setAffiliation).not.toHaveBeenCalled();
@@ -3026,7 +3026,7 @@
                 view.onFormSubmitted(new Event('submit'));
 
                 expect(view.showErrorMessage).toHaveBeenCalledWith(
-                    'Error: couldn\'t find a groupchat participant "nobody"');
+                    "Error: couldn't find a groupchat participant based on your arguments");
                 expect(view.model.setAffiliation).not.toHaveBeenCalled();
 
                 // Call now with the correct of arguments.
@@ -3036,14 +3036,14 @@
                 textarea.value = '/owner annoyingGuy You\'re responsible';
                 view.onFormSubmitted(new Event('submit'));
 
-                expect(view.validateRoleChangeCommand.calls.count()).toBe(3);
+                expect(view.validateRoleOrAffiliationChangeArgs.calls.count()).toBe(3);
                 expect(view.model.setAffiliation).toHaveBeenCalled();
                 expect(view.showErrorMessage.calls.count()).toBe(2);
                 // Check that the member list now gets updated
                 expect(sent_IQ.toLocaleString()).toBe(
                     `<iq id="${IQ_id}" to="lounge@localhost" type="set" xmlns="jabber:client">`+
                         `<query xmlns="http://jabber.org/protocol/muc#admin">`+
-                            `<item affiliation="owner" jid="annoyingGuy">`+
+                            `<item affiliation="owner" jid="annoyingguy@localhost">`+
                                 `<reason>You&apos;re responsible</reason>`+
                             `</item>`+
                         `</query>`+
@@ -3081,7 +3081,7 @@
                 const view = _converse.chatboxviews.get('lounge@localhost');
                 spyOn(view.model, 'setAffiliation').and.callThrough();
                 spyOn(view, 'showErrorMessage').and.callThrough();
-                spyOn(view, 'validateRoleChangeCommand').and.callThrough();
+                spyOn(view, 'validateRoleOrAffiliationChangeArgs').and.callThrough();
 
                 let presence = $pres({
                         'from': 'lounge@localhost/annoyingGuy',
@@ -3103,7 +3103,7 @@
                     preventDefault: _.noop,
                     keyCode: 13
                 });
-                expect(view.validateRoleChangeCommand).toHaveBeenCalled();
+                expect(view.validateRoleOrAffiliationChangeArgs).toHaveBeenCalled();
                 expect(view.showErrorMessage).toHaveBeenCalledWith(
                     "Error: the \"ban\" command takes two arguments, the user's nickname and optionally a reason.");
                 expect(view.model.setAffiliation).not.toHaveBeenCalled();
@@ -3114,14 +3114,14 @@
                 textarea.value = '/ban annoyingGuy You\'re annoying';
                 view.onFormSubmitted(new Event('submit'));
 
-                expect(view.validateRoleChangeCommand.calls.count()).toBe(2);
+                expect(view.validateRoleOrAffiliationChangeArgs.calls.count()).toBe(2);
                 expect(view.showErrorMessage.calls.count()).toBe(1);
                 expect(view.model.setAffiliation).toHaveBeenCalled();
                 // Check that the member list now gets updated
                 expect(sent_IQ.toLocaleString()).toBe(
                     `<iq id="${IQ_id}" to="lounge@localhost" type="set" xmlns="jabber:client">`+
                         `<query xmlns="http://jabber.org/protocol/muc#admin">`+
-                            `<item affiliation="outcast" jid="annoyingGuy">`+
+                            `<item affiliation="outcast" jid="annoyingguy@localhost">`+
                                 `<reason>You&apos;re annoying</reason>`+
                             `</item>`+
                         `</query>`+
@@ -3145,7 +3145,7 @@
                 done();
             }));
 
-            it("accepts a /kick command to kick a user",
+            it("takes a /kick command to kick a user",
                 mock.initConverse(
                     null, ['rosterGroupsFetched'], {},
                     async function (done, _converse) {
@@ -3161,10 +3161,10 @@
                 const view = _converse.chatboxviews.get('lounge@localhost');
                 spyOn(view.model, 'setRole').and.callThrough();
                 spyOn(view, 'showErrorMessage').and.callThrough();
-                spyOn(view, 'validateRoleChangeCommand').and.callThrough();
+                spyOn(view, 'validateRoleOrAffiliationChangeArgs').and.callThrough();
 
                 let presence = $pres({
-                        'from': 'lounge@localhost/annoyingGuy',
+                        'from': 'lounge@localhost/annoying guy',
                         'id':'27C55F89-1C6A-459A-9EB5-77690145D624',
                         'to': 'dummy@localhost/desktop'
                     })
@@ -3183,7 +3183,7 @@
                     preventDefault: _.noop,
                     keyCode: 13
                 });
-                expect(view.validateRoleChangeCommand).toHaveBeenCalled();
+                expect(view.validateRoleOrAffiliationChangeArgs).toHaveBeenCalled();
                 expect(view.showErrorMessage).toHaveBeenCalledWith(
                     "Error: the \"kick\" command takes two arguments, the user's nickname and optionally a reason.");
                 expect(view.model.setRole).not.toHaveBeenCalled();
@@ -3191,16 +3191,16 @@
                 // XXX: Calling onFormSubmitted directly, trying
                 // again via triggering Event doesn't work for some weird
                 // reason.
-                textarea.value = '/kick annoyingGuy You\'re annoying';
+                textarea.value = '/kick annoying guy You\'re annoying';
                 view.onFormSubmitted(new Event('submit'));
 
-                expect(view.validateRoleChangeCommand.calls.count()).toBe(2);
+                expect(view.validateRoleOrAffiliationChangeArgs.calls.count()).toBe(2);
                 expect(view.showErrorMessage.calls.count()).toBe(1);
                 expect(view.model.setRole).toHaveBeenCalled();
                 expect(sent_IQ.toLocaleString()).toBe(
                     `<iq id="${IQ_id}" to="lounge@localhost" type="set" xmlns="jabber:client">`+
                         `<query xmlns="http://jabber.org/protocol/muc#admin">`+
-                            `<item nick="annoyingGuy" role="none">`+
+                            `<item nick="annoying guy" role="none">`+
                                 `<reason>You&apos;re annoying</reason>`+
                             `</item>`+
                         `</query>`+
@@ -3217,7 +3217,7 @@
                  *     </presence>
                  */
                 presence = $pres({
-                        'from': 'lounge@localhost/annoyingGuy',
+                        'from': 'lounge@localhost/annoying guy',
                         'to': 'dummy@localhost/desktop',
                         'type': 'unavailable'
                     })
@@ -3228,7 +3228,7 @@
                         }).up()
                         .c('status', {'code': '307'});
                 _converse.connection._dataRecv(test_utils.createRequest(presence));
-                expect(view.el.querySelectorAll('.chat-info')[3].textContent).toBe("annoyingGuy has been kicked out");
+                expect(view.el.querySelectorAll('.chat-info')[3].textContent).toBe("annoying guy has been kicked out");
                 expect(view.el.querySelectorAll('.chat-info').length).toBe(4);
                 done();
             }));
@@ -3246,11 +3246,11 @@
                     sent_IQ = iq;
                     IQ_id = sendIQ.bind(this)(iq, callback, errback);
                 });
-                var view = _converse.chatboxviews.get('lounge@localhost');
+                const view = _converse.chatboxviews.get('lounge@localhost');
                 spyOn(view.model, 'setRole').and.callThrough();
                 spyOn(view, 'showErrorMessage').and.callThrough();
                 spyOn(view, 'showChatEvent').and.callThrough();
-                spyOn(view, 'validateRoleChangeCommand').and.callThrough();
+                spyOn(view, 'validateRoleOrAffiliationChangeArgs').and.callThrough();
 
                 // New user enters the groupchat
                 /* <presence
@@ -3262,7 +3262,7 @@
                  * </x>
                  * </presence>
                  */
-                var presence = $pres({
+                let presence = $pres({
                         'from': 'lounge@localhost/trustworthyguy',
                         'id':'27C55F89-1C6A-459A-9EB5-77690145D624',
                         'to': 'dummy@localhost/desktop'
@@ -3285,7 +3285,7 @@
                     keyCode: 13
                 });
 
-                expect(view.validateRoleChangeCommand).toHaveBeenCalled();
+                expect(view.validateRoleOrAffiliationChangeArgs).toHaveBeenCalled();
                 expect(view.showErrorMessage).toHaveBeenCalledWith(
                     "Error: the \"op\" command takes two arguments, the user's nickname and optionally a reason.");
 
@@ -3297,7 +3297,7 @@
                 textarea.value = '/op trustworthyguy You\'re trustworthy';
                 view.onFormSubmitted(new Event('submit'));
 
-                expect(view.validateRoleChangeCommand.calls.count()).toBe(2);
+                expect(view.validateRoleOrAffiliationChangeArgs.calls.count()).toBe(2);
                 expect(view.showErrorMessage.calls.count()).toBe(1);
                 expect(view.model.setRole).toHaveBeenCalled();
                 expect(sent_IQ.toLocaleString()).toBe(
@@ -3339,7 +3339,7 @@
                 textarea.value = '/deop trustworthyguy Perhaps not';
                 view.onFormSubmitted(new Event('submit'));
 
-                expect(view.validateRoleChangeCommand.calls.count()).toBe(3);
+                expect(view.validateRoleOrAffiliationChangeArgs.calls.count()).toBe(3);
                 expect(view.showChatEvent.calls.count()).toBe(1);
                 expect(view.model.setRole).toHaveBeenCalled();
                 expect(sent_IQ.toLocaleString()).toBe(
@@ -3392,7 +3392,7 @@
                 spyOn(view.model, 'setRole').and.callThrough();
                 spyOn(view, 'showErrorMessage').and.callThrough();
                 spyOn(view, 'showChatEvent').and.callThrough();
-                spyOn(view, 'validateRoleChangeCommand').and.callThrough();
+                spyOn(view, 'validateRoleOrAffiliationChangeArgs').and.callThrough();
 
                 // New user enters the groupchat
                 /* <presence
@@ -3427,7 +3427,7 @@
                     keyCode: 13
                 });
 
-                expect(view.validateRoleChangeCommand).toHaveBeenCalled();
+                expect(view.validateRoleOrAffiliationChangeArgs).toHaveBeenCalled();
                 expect(view.showErrorMessage).toHaveBeenCalledWith(
                     "Error: the \"mute\" command takes two arguments, the user's nickname and optionally a reason.");
                 expect(view.model.setRole).not.toHaveBeenCalled();
@@ -3438,7 +3438,7 @@
                 textarea.value = '/mute annoyingGuy You\'re annoying';
                 view.onFormSubmitted(new Event('submit'));
 
-                expect(view.validateRoleChangeCommand.calls.count()).toBe(2);
+                expect(view.validateRoleOrAffiliationChangeArgs.calls.count()).toBe(2);
                 expect(view.showErrorMessage.calls.count()).toBe(1);
                 expect(view.model.setRole).toHaveBeenCalled();
                 expect(sent_IQ.toLocaleString()).toBe(
@@ -3481,7 +3481,7 @@
                 textarea.value = '/voice annoyingGuy Now you can talk again';
                 view.onFormSubmitted(new Event('submit'));
 
-                expect(view.validateRoleChangeCommand.calls.count()).toBe(3);
+                expect(view.validateRoleOrAffiliationChangeArgs.calls.count()).toBe(3);
                 expect(view.showChatEvent.calls.count()).toBe(1);
                 expect(view.model.setRole).toHaveBeenCalled();
                 expect(sent_IQ.toLocaleString()).toBe(

+ 120 - 148
src/converse-muc-views.js

@@ -46,6 +46,21 @@ const ADMIN_COMMANDS = ['admin', 'ban', 'deop', 'destroy', 'member', 'op', 'revo
 const MODERATOR_COMMANDS = ['kick', 'mute', 'voice'];
 const VISITOR_COMMANDS = ['nick'];
 
+const COMMAND_TO_ROLE = {
+    'deop': 'participant',
+    'kick': 'none',
+    'mute': 'visitor',
+    'op': 'moderator',
+    'voice': 'participant'
+}
+const COMMAND_TO_AFFILIATION = {
+    'admin': 'admin',
+    'ban': 'outcast',
+    'member': 'member',
+    'owner': 'owner',
+    'revoke': 'none'
+}
+
 converse.plugins.add('converse-muc-views', {
     /* Dependencies are other plugins which might be
      * overridden or relied upon, and therefore need to be loaded before
@@ -96,8 +111,6 @@ converse.plugins.add('converse-muc-views', {
             }
         });
 
-        const OCCUPANT_NOT_FOUND = __("Could not find an occupant with that nickname");
-
 
         function renderRoomsPanel () {
             if (this.roomspanel && u.isVisible(this.roomspanel.el)) {
@@ -857,19 +870,18 @@ converse.plugins.add('converse-muc-views', {
                 }
             },
 
-            destroy (groupchat, reason, onSuccess, onError) {
-                const destroy = $build("destroy");
-                const iq = $iq({to: groupchat, type: "set"}).c("query", {xmlns: Strophe.NS.MUC_OWNER}).cnode(destroy.node);
-                if (reason && reason.length > 0) { iq.c("reason", reason); }
-                return _converse.api.sendIQ(iq);
-            },
-
             verifyRoles (roles, occupant, show_error=true) {
+                if (!Array.isArray(roles)) {
+                    throw new TypeError('roles must be an Array');
+                }
+                if (!roles.length) {
+                    return true;
+                }
                 if (!occupant) {
                     occupant = this.model.occupants.findWhere({'jid': _converse.bare_jid});
                 }
                 const role = occupant.get('role');
-                if (Array.isArray(roles) && roles.includes(role) || roles === role) {
+                if (roles.includes(role)) {
                     return true;
                 }
                 if (show_error) {
@@ -879,11 +891,17 @@ converse.plugins.add('converse-muc-views', {
             },
 
             verifyAffiliations (affiliations, occupant, show_error=true) {
+                if (!Array.isArray(affiliations)) {
+                    throw new TypeError('affiliations must be an Array');
+                }
+                if (!affiliations.length) {
+                    return true;
+                }
                 if (!occupant) {
                     occupant = this.model.occupants.findWhere({'jid': _converse.bare_jid});
                 }
                 const a = occupant.get('affiliation');
-                if (Array.isArray(affiliations) && affiliations.includes(a) || affiliations === a) {
+                if (affiliations.includes(a)) {
                     return true;
                 }
                 if (show_error) {
@@ -892,22 +910,81 @@ converse.plugins.add('converse-muc-views', {
                 return false;
             },
 
-            validateRoleChangeCommand (command, args) {
-                /* Check that a command to change a groupchat user's role or
-                 * affiliation has anough arguments.
-                 */
-                if (args.length < 1 || args.length > 2) {
+            validateRoleOrAffiliationChangeArgs (command, args) {
+                if (!args) {
                     this.showErrorMessage(
                         __('Error: the "%1$s" command takes two arguments, the user\'s nickname and optionally a reason.', command)
                     );
                     return false;
                 }
-                if (!(AFFILIATION_CHANGE_COMANDS.includes(command) && u.isValidJID(args[0])) &&
-                        !this.model.occupants.findWhere({'nick': args[0]}) &&
-                        !this.model.occupants.findWhere({'jid': args[0]})) {
-                    this.showErrorMessage(__('Error: couldn\'t find a groupchat participant "%1$s"', args[0]));
+                return true;
+            },
+
+            getNickOrJIDFromCommandArgs (args) {
+                const [text, references] = this.model.parseTextForReferences('@'+args);
+                if (!references.length) {
+                    this.showErrorMessage(__("Error: couldn't find a groupchat participant based on your arguments"));
+                    return false;
+                }
+                return references.pop();
+            },
+
+            setAffiliation (command, args, required_affiliations) {
+                const affiliation = COMMAND_TO_AFFILIATION[command];
+                if (!affiliation) {
+                    throw Error(`ChatRoomView#setAffiliation called with invalid command: ${command}`);
+                }
+                if (!this.verifyAffiliations(required_affiliations)) {
+                    return false;
+                }
+                if (!this.validateRoleOrAffiliationChangeArgs(command, args)) {
+                    return false;
+                }
+                const nick_or_jid = _.get(this.getNickOrJIDFromCommandArgs(args), 'value', null);
+                if (!nick_or_jid) {
+                    return false;
+                }
+                const reason = args.slice(nick_or_jid.length).trim();
+                // We're guaranteed to have an occupant due to getNickOrJIDFromCommandArgs
+                const occupant = this.model.getOccupant(nick_or_jid);
+                const attrs = {
+                    'jid': occupant.get('jid'),
+                    'reason': reason
+                }
+                if (_converse.auto_register_muc_nickname && occupant) {
+                    attrs['nick'] = occupant.get('nick');
+                }
+                this.model.setAffiliation(affiliation, [attrs])
+                    .then(() => this.model.occupants.fetchMembers())
+                    .catch(err => this.onCommandError(err));
+            },
+
+            getReason (args) {
+                return args.includes(',') ? args.slice(args.indexOf(',')+1).trim() : null;
+            },
+
+            setRole (command, args, required_affiliations=[], required_roles=[]) {
+                /* Check that a command to change a groupchat user's role or
+                 * affiliation has anough arguments.
+                 */
+                const role = COMMAND_TO_ROLE[command];
+                if (!role) {
+                    throw Error(`ChatRoomView#setRole called with invalid command: ${command}`);
+                }
+                if (!this.verifyAffiliations(required_affiliations) || !this.verifyRoles(required_roles)) {
+                    return false;
+                }
+                if (!this.validateRoleOrAffiliationChangeArgs(command, args)) {
+                    return false;
+                }
+                const nick_or_jid = _.get(this.getNickOrJIDFromCommandArgs(args), 'value', null);
+                if (!nick_or_jid) {
                     return false;
                 }
+                const reason = args.slice(nick_or_jid.length).trim();
+                // We're guaranteed to have an occupant due to getNickOrJIDFromCommandArgs
+                const occupant = this.model.getOccupant(nick_or_jid);
+                this.model.setRole(occupant, role, reason, undefined, this.onCommandError.bind(this));
                 return true;
             },
 
@@ -920,9 +997,12 @@ converse.plugins.add('converse-muc-views', {
                 if (_converse.muc_disable_slash_commands && !Array.isArray(_converse.muc_disable_slash_commands)) {
                     return _converse.ChatBoxView.prototype.parseMessageForCommands.apply(this, arguments);
                 }
-                const match = text.replace(/^\s*/, "").match(/^\/(.*?)(?: (.*))?$/) || [false, '', ''],
-                      args = match[2] && match[2].splitOnce(' ').filter(s => s) || [],
-                      command = match[1].toLowerCase();
+                text = text.replace(/^\s*/, "");
+                const command = (text.match(/^\/([a-zA-Z]*) ?/) || ['']).pop().toLowerCase();
+                if (!command) {
+                    return false;
+                }
+                const args = text.slice(('/'+command).length+1);
 
                 let disabled_commands = [];
                 if (Array.isArray(_converse.muc_disable_slash_commands)) {
@@ -934,29 +1014,11 @@ converse.plugins.add('converse-muc-views', {
 
                 switch (command) {
                     case 'admin': {
-                        if (!this.verifyAffiliations(['owner']) || !this.validateRoleChangeCommand(command, args)) {
-                            break;
-                        }
-                        this.model.setAffiliation('admin', [{
-                            'jid': args[0],
-                            'reason': args[1]
-                        }]).then(
-                            () => this.model.occupants.fetchMembers(),
-                            (err) => this.onCommandError(err)
-                        );
+                        this.setAffiliation(command, args, ['owner']);
                         break;
                     }
                     case 'ban': {
-                        if (!this.verifyAffiliations(['admin', 'owner']) || !this.validateRoleChangeCommand(command, args)) {
-                            break;
-                        }
-                        this.model.setAffiliation('outcast', [{
-                            'jid': args[0],
-                            'reason': args[1]
-                        }]).then(
-                            () => this.model.occupants.fetchMembers(),
-                            (err) => this.onCommandError(err)
-                        );
+                        this.setAffiliation(command, args, ['admin', 'owner']);
                         break;
                     }
                     case 'deop': {
@@ -966,24 +1028,14 @@ converse.plugins.add('converse-muc-views', {
                         // to participant (e.g. visitor => participant).
                         // Currently we don't distinguish between these two
                         // cases.
-                        if (!this.verifyAffiliations(['admin', 'owner']) || !this.validateRoleChangeCommand(command, args)) {
-                            break;
-                        }
-                        const occupant = this.model.getOccupantByNickname(args[0]);
-                        if (!occupant) {
-                            this.showErrorMessage(OCCUPANT_NOT_FOUND);
-                            break;
-                        }
-                        this.model.setRole(
-                            occupant, 'participant', args[1],
-                            undefined, this.onCommandError.bind(this));
+                        this.setRole(command, args, ['admin', 'owner']);
                         break;
                     }
                     case 'destroy': {
-                        if (!this.verifyAffiliations('owner')) {
+                        if (!this.verifyAffiliations(['owner'])) {
                             break;
                         }
-                        this.destroy(this.model.get('jid'), args[0])
+                        this.model.sendDestroyIQ(args)
                             .then(() => this.close())
                             .catch(e => this.onCommandError(e));
                         break;
@@ -995,12 +1047,12 @@ converse.plugins.add('converse-muc-views', {
                         // consideration.
                         let allowed_commands = ['clear', 'help', 'me', 'nick', 'subject', 'topic', 'register'];
                         const occupant = this.model.occupants.findWhere({'jid': _converse.bare_jid});
-                        if (this.verifyAffiliations('owner', occupant, false)) {
+                        if (this.verifyAffiliations(['owner'], occupant, false)) {
                             allowed_commands = allowed_commands.concat(OWNER_COMMANDS).concat(ADMIN_COMMANDS);
-                        } else if (this.verifyAffiliations('admin', occupant, false)) {
+                        } else if (this.verifyAffiliations(['admin'], occupant, false)) {
                             allowed_commands = allowed_commands.concat(ADMIN_COMMANDS);
                         }
-                        if (this.verifyRoles('moderator', occupant, false)) {
+                        if (this.verifyRoles(['moderator'], occupant, false)) {
                             allowed_commands = allowed_commands.concat(MODERATOR_COMMANDS).concat(VISITOR_COMMANDS);
                         } else if (!this.verifyRoles(['visitor', 'participant', 'moderator'], occupant, false)) {
                             allowed_commands = allowed_commands.concat(VISITOR_COMMANDS);
@@ -1030,49 +1082,15 @@ converse.plugins.add('converse-muc-views', {
                         );
                         break;
                     } case 'kick': {
-                        if (!this.verifyRoles(['moderator']) || !this.validateRoleChangeCommand(command, args)) {
-                            break;
-                        }
-                        const occupant = this.model.getOccupantByNickname(args[0]);
-                        if (!occupant) {
-                            this.showErrorMessage(OCCUPANT_NOT_FOUND);
-                            break;
-                        }
-                        this.model.setRole(
-                            occupant, 'none', args[1],
-                            undefined, this.onCommandError.bind(this));
+                        this.setRole(command, args, [], ['moderator']);
                         break;
                     }
                     case 'mute': {
-                        if (!this.verifyRoles(['moderator']) || !this.validateRoleChangeCommand(command, args)) {
-                            break;
-                        }
-                        const occupant = this.model.getOccupantByNickname(args[0]);
-                        if (!occupant) {
-                            this.showErrorMessage(OCCUPANT_NOT_FOUND);
-                            break;
-                        }
-                        this.model.setRole(
-                            occupant, 'visitor', args[1],
-                            undefined, this.onCommandError.bind(this));
+                        this.setRole(command, args, [], ['moderator']);
                         break;
                     }
                     case 'member': {
-                        if (!this.verifyAffiliations(['admin', 'owner']) || !this.validateRoleChangeCommand(command, args)) {
-                            break;
-                        }
-                        const occupant = this.model.occupants.findWhere({'nick': args[0]}) ||
-                                         this.model.occupants.findWhere({'jid': args[0]}),
-                              attrs = {
-                                'jid': occupant ? occupant.get('jid') : args[0],
-                                'reason': args[1]
-                              };
-                        if (_converse.auto_register_muc_nickname && occupant) {
-                            attrs['nick'] = occupant.get('nick');
-                        }
-                        this.model.setAffiliation('member', [attrs])
-                            .then(() => this.model.occupants.fetchMembers())
-                            .catch(err => this.onCommandError(err));
+                        this.setAffiliation(command, args, ['admin', 'owner']);
                         break;
                     }
                     case 'nick': {
@@ -1081,35 +1099,16 @@ converse.plugins.add('converse-muc-views', {
                         }
                         _converse.api.send($pres({
                             from: _converse.connection.jid,
-                            to: this.model.getRoomJIDAndNick(match[2]),
+                            to: this.model.getRoomJIDAndNick(args),
                             id: _converse.connection.getUniqueId()
                         }).tree());
                         break;
                     }
                     case 'owner':
-                        if (!this.verifyAffiliations(['owner']) || !this.validateRoleChangeCommand(command, args)) {
-                            break;
-                        }
-                        this.model.setAffiliation('owner', [{
-                            'jid': args[0],
-                            'reason': args[1]
-                        }]).then(
-                            () => this.model.occupants.fetchMembers(),
-                            (err) => this.onCommandError(err)
-                        );
+                        this.setAffiliation(command, args, ['owner']);
                         break;
                     case 'op': {
-                        if (!this.verifyAffiliations(['admin', 'owner']) || !this.validateRoleChangeCommand(command, args)) {
-                            break;
-                        }
-                        const occupant = this.model.getOccupantByNickname(args[0]);
-                        if (!occupant) {
-                            this.showErrorMessage();
-                            break;
-                        }
-                        this.model.setRole(
-                            occupant, 'moderator', args[1],
-                            undefined, this.onCommandError.bind(this));
+                        this.setRole(command, args, ['admin', 'owner']);
                         break;
                     }
                     case 'register': {
@@ -1123,42 +1122,15 @@ converse.plugins.add('converse-muc-views', {
                         break;
                     }
                     case 'revoke': {
-                        if (!this.verifyAffiliations(['admin', 'owner']) || !this.validateRoleChangeCommand(command, args)) {
-                            break;
-                        }
-                        this.model.setAffiliation('none', [{
-                            'jid': args[0],
-                            'reason': args[1]
-                        }]).then(
-                            () => this.model.occupants.fetchMembers(),
-                            (err) => this.onCommandError(err)
-                        );
+                        this.setAffiliation(command, args, ['admin', 'owner']);
                         break;
                     }
                     case 'topic':
                     case 'subject':
-                        // TODO: should be done via API call to _converse.api.rooms
-                        _converse.api.send(
-                            $msg({
-                                to: this.model.get('jid'),
-                                from: _converse.connection.jid,
-                                type: "groupchat"
-                            }).c("subject", {xmlns: "jabber:client"}).t(match[2] || "").tree()
-                        );
+                        this.model.setSubject(args);
                         break;
                     case 'voice': {
-                        if (!this.verifyRoles(['moderator']) || !this.validateRoleChangeCommand(command, args)) {
-                            break;
-                        }
-                        const occupant = this.model.getOccupantByNickname(args[0]);
-                        if (!occupant) {
-                            this.showErrorMessage(OCCUPANT_NOT_FOUND);
-                            break;
-                        }
-                        this.model.setRole(
-                            occupant, 'participant', args[1], undefined,
-                            this.onCommandError.bind(this)
-                        );
+                        this.setRole(command, args, [], ['moderator']);
                         break;
                     }
                     default:

+ 52 - 8
src/headless/converse-muc.js

@@ -361,7 +361,7 @@ converse.plugins.add('converse-muc', {
              * @private
              * @method _converse.ChatRoom#join
              * @param { String } nick - The user's nickname
-             * @param { String } password - Optional password, if required by the groupchat.
+             * @param { String } [password] - Optional password, if required by the groupchat.
              */
             async join (nick, password) {
                 if (this.get('connection_status') === converse.ROOMSTATUS.ENTERED) {
@@ -388,10 +388,36 @@ converse.plugins.add('converse-muc', {
                 return this;
             },
 
-            /* Leave the groupchat.
+            /**
+             * Sends an IQ stanza to the XMPP server to destroy this groupchat. Not
+             * to be confused with the {@link _converse.ChatRoom#destroy}
+             * method, which simply removes the room from the local browser storage cache.
+             * @private
+             * @method _converse.ChatRoom#sendDestroyIQ
+             * @param { string } [reason] - The reason for destroying the groupchat
+             * @param { string } [new_jid] - The JID of the new groupchat which
+             *      replaces this one.
+             */
+            sendDestroyIQ (reason, new_jid) {
+                const destroy = $build("destroy");
+                if (new_jid) {
+                    destroy.attrs({'jid': new_jid});
+                }
+                const iq = $iq({
+                    'to': this.get('jid'),
+                    'type': "set"
+                }).c("query", {'xmlns': Strophe.NS.MUC_OWNER}).cnode(destroy.node);
+                if (reason && reason.length > 0) {
+                    iq.c("reason", reason);
+                }
+                return _converse.api.sendIQ(iq);
+            },
+
+            /**
+             * Leave the groupchat.
              * @private
              * @method _converse.ChatRoom#leave
-             * @param { string } exit_msg - Optional message to indicate your reason for leaving
+             * @param { string } [exit_msg] - Message to indicate your reason for leaving
              */
             leave (exit_msg) {
                 this.features.destroy();
@@ -550,7 +576,7 @@ converse.plugins.add('converse-muc', {
              * @private
              * @method _converse.ChatRoom#directInvite
              * @param { String } recipient - JID of the person being invited
-             * @param { String } reason - Optional reason for the invitation
+             * @param { String } [reason] - Reason for the invitation
              */
             directInvite (recipient, reason) {
                 if (this.features.get('membersonly')) {
@@ -873,12 +899,14 @@ converse.plugins.add('converse-muc', {
 
             /**
              * @private
-             * @method _converse.ChatRoom#getOccupantByNickname
-             * @param { String } nick - The nickname of the occupant to be returned
+             * @method _converse.ChatRoom#getOccupant
+             * @param { String } nick_or_jid - The nickname or JID of the occupant to be returned
              * @returns { _converse.ChatRoomOccupant }
              */
-            getOccupantByNickname (nick) {
-                return this.occupants.findWhere({'nick': nick});
+            getOccupant (nick_or_jid) {
+                return (u.isValidJID(nick_or_jid) &&
+                    this.occupants.findWhere({'jid': nick_or_jid})) ||
+                    this.occupants.findWhere({'nick': nick_or_jid});
             },
 
             async getJidsWithAffiliations (affiliations) {
@@ -1146,6 +1174,22 @@ converse.plugins.add('converse-muc', {
                 return false;
             },
 
+            /**
+             * Set the subject for this {@link _converse.ChatRoom}
+             * @private
+             * @method _converse.ChatRoom#setSubject
+             * @param { String } value
+             */
+            setSubject(value='') {
+                _converse.api.send(
+                    $msg({
+                        to: this.get('jid'),
+                        from: _converse.connection.jid,
+                        type: "groupchat"
+                    }).c("subject", {xmlns: "jabber:client"}).t(value).tree()
+                );
+            },
+
             /**
              * Is this a chat state notification that can be ignored,
              * because it's old or because it's from us.