Use the `parseMessageForCommands` hook to also parse MUC commands

Also fix issue where `model.getAllowedCommands()` prevents 3rd party
implementations.

src/plugins/chatview/message-form.js

@@ -162,11 +162,6 @@ export default class MessageForm extends ElementView {
         }
     }
 
-    async parseMessageForCommands (text) {
-        // Wrap util so that we can override in the MUC message-form component
-        return await parseMessageForCommands(this.model, text);
-    }
-
     async onFormSubmitted (ev) {
         ev?.preventDefault?.();
 
@@ -194,7 +189,7 @@ export default class MessageForm extends ElementView {
         textarea.setAttribute('disabled', 'disabled');
         this.querySelector('converse-emoji-dropdown')?.hideMenu();
 
-        const is_command = await this.parseMessageForCommands(message_text);
+        const is_command = await parseMessageForCommands(this.model, message_text);
         const message = is_command ? null : await this.model.sendMessage({'body': message_text, spoiler_hint});
         if (is_command || message) {
             hint_el.value = '';

src/plugins/chatview/utils.js

@@ -40,23 +40,23 @@ export async function clearMessages (chat) {
 
 
 export async function parseMessageForCommands (chat, text) {
-    /**
-     * *Hook* which allows plugins to add more commands to a chat's textbox.
-     * Data provided is the chatbox model and the text typed - {model, text}.
-     * Check `handled` to see if the hook was already handled.
-     * @event _converse#parseMessageForCommands
-     * @example
-     *  api.listen.on('parseMessageForCommands', (data, handled) {
-     *      if (!handled) {
-     *         const command = (data.text.match(/^\/([a-zA-Z]*) ?/) || ['']).pop().toLowerCase();
-     *         // custom code comes here
-     *      }
-     *      return handled;
-     *  }
-     */
     const match = text.replace(/^\s*/, '').match(/^\/(.*)\s*$/);
     if (match) {
         let handled = false;
+        /**
+         * *Hook* which allows plugins to add more commands to a chat's textbox.
+         * Data provided is the chatbox model and the text typed - {model, text}.
+         * Check `handled` to see if the hook was already handled.
+         * @event _converse#parseMessageForCommands
+         * @example
+         *  api.listen.on('parseMessageForCommands', (data, handled) {
+         *      if (!handled) {
+         *         const command = (data.text.match(/^\/([a-zA-Z]*) ?/) || ['']).pop().toLowerCase();
+         *         // custom code comes here
+         *      }
+         *      return handled;
+         *  }
+         */
         handled = await api.hook('parseMessageForCommands', {model: chat, text}, handled);
         if (handled) {
             return true;

src/plugins/muc-views/message-form.js

@@ -6,6 +6,12 @@ import { getAutoCompleteListItem, parseMessageForMUCCommands } from './utils.js'
 
 export default class MUCMessageForm extends MessageForm {
 
+    async connectedCallback () {
+        super.connectedCallback();
+        await this.model.initialized;
+        api.listen.on('parseMessageForCommands', parseMessageForMUCCommands);
+    }
+
     toHTML () {
         return tpl_muc_message_form(
             Object.assign(this.model.toJSON(), {
@@ -47,13 +53,6 @@ export default class MUCMessageForm extends MessageForm {
         this.mention_auto_complete.on('suggestion-box-selectcomplete', () => (this.auto_completing = false));
     }
 
-    /**
-     * @async
-     */
-    parseMessageForCommands (text) {
-        return parseMessageForMUCCommands(this.model, text);
-    }
-
     getAutoCompleteList () {
         return this.model.getAllKnownNicknames().map(nick => ({ 'label': nick, 'value': `@${nick}` }));
     }

src/plugins/muc-views/tests/muc.js

@@ -2499,8 +2499,7 @@ describe("Groupchats", function () {
                 preventDefault: function preventDefault () {},
                 keyCode: 13
             });
-            expect(_converse.connection.send).toHaveBeenCalled();
-            expect(Strophe.serialize(sent_stanza)).toBe(
+            await u.waitUntil(() => Strophe.serialize(sent_stanza) ===
                 `<iq id="${sent_stanza.getAttribute('id')}" to="lounge@muc.montague.lit" type="set" xmlns="jabber:client">`+
                     `<query xmlns="http://jabber.org/protocol/muc#admin">`+
                         `<item affiliation="member" jid="marc@montague.lit">`+
@@ -2593,10 +2592,6 @@ describe("Groupchats", function () {
         it("takes /topic to set the groupchat topic", mock.initConverse([], {}, async function (_converse) {
             await mock.openAndEnterChatRoom(_converse, 'lounge@montague.lit', 'romeo');
             const view = _converse.chatboxviews.get('lounge@montague.lit');
-            let sent_stanza;
-            spyOn(_converse.connection, 'send').and.callFake(function (stanza) {
-                sent_stanza = stanza;
-            });
             // Check the alias /topic
             const textarea = await u.waitUntil(() => view.querySelector('.chat-textarea'));
             textarea.value = '/topic This is the groupchat subject';
@@ -2606,8 +2601,8 @@ describe("Groupchats", function () {
                 preventDefault: function preventDefault () {},
                 keyCode: 13
             });
-            expect(_converse.connection.send).toHaveBeenCalled();
-            expect(sent_stanza.textContent.trim()).toBe('This is the groupchat subject');
+            const { sent_stanzas } = _converse.connection;
+            await u.waitUntil(() => sent_stanzas.filter(s => s.textContent.trim() === 'This is the groupchat subject'));
 
             // Check /subject
             textarea.value = '/subject This is a new subject';
@@ -2617,7 +2612,7 @@ describe("Groupchats", function () {
                 keyCode: 13
             });
 
-            expect(sent_stanza.textContent.trim()).toBe('This is a new subject');
+            let sent_stanza = await u.waitUntil(() => sent_stanzas.filter(s => s.textContent.trim() === 'This is a new subject').pop());
             expect(Strophe.serialize(sent_stanza).toLocaleString()).toBe(
                 '<message from="romeo@montague.lit/orchard" to="lounge@montague.lit" type="groupchat" xmlns="jabber:client">'+
                     '<subject xmlns="jabber:client">This is a new subject</subject>'+
@@ -2630,12 +2625,15 @@ describe("Groupchats", function () {
                 preventDefault: function preventDefault () {},
                 keyCode: 13
             });
-            expect(sent_stanza.textContent.trim()).toBe('This is yet another subject');
+            sent_stanza = await u.waitUntil(() => sent_stanzas.filter(s => s.textContent.trim() === 'This is yet another subject').pop());
             expect(Strophe.serialize(sent_stanza).toLocaleString()).toBe(
                 '<message from="romeo@montague.lit/orchard" to="lounge@montague.lit" type="groupchat" xmlns="jabber:client">'+
                     '<subject xmlns="jabber:client">This is yet another subject</subject>'+
                 '</message>');
 
+            while (sent_stanzas.length) {
+                sent_stanzas.pop();
+            }
             // Check unsetting the topic
             textarea.value = '/topic';
             message_form.onKeyDown({
@@ -2643,6 +2641,7 @@ describe("Groupchats", function () {
                 preventDefault: function preventDefault () {},
                 keyCode: 13
             });
+            sent_stanza = await u.waitUntil(() => sent_stanzas.pop());
             expect(Strophe.serialize(sent_stanza).toLocaleString()).toBe(
                 '<message from="romeo@montague.lit/orchard" to="lounge@montague.lit" type="groupchat" xmlns="jabber:client">'+
                     '<subject xmlns="jabber:client"></subject>'+
@@ -2698,7 +2697,7 @@ describe("Groupchats", function () {
                 preventDefault: function preventDefault () {},
                 keyCode: 13
             });
-            expect(view.model.validateRoleOrAffiliationChangeArgs).toHaveBeenCalled();
+            await u.waitUntil(() => view.model.validateRoleOrAffiliationChangeArgs.calls.count());
             const err_msg = await u.waitUntil(() => view.querySelector('.chat-error'));
             expect(err_msg.textContent.trim()).toBe(
                 "Error: the \"owner\" command takes two arguments, the user's nickname and optionally a reason.");
@@ -2725,7 +2724,7 @@ describe("Groupchats", function () {
             textarea.value = '/owner annoyingGuy You\'re responsible';
             message_form.onFormSubmitted(new Event('submit'));
 
-            expect(view.model.validateRoleOrAffiliationChangeArgs.calls.count()).toBe(3);
+            await u.waitUntil(() => view.model.validateRoleOrAffiliationChangeArgs.calls.count() === 3);
             // Check that the member list now gets updated
             expect(Strophe.serialize(sent_IQ)).toBe(
                 `<iq id="${IQ_id}" to="lounge@montague.lit" type="set" xmlns="jabber:client">`+
@@ -2787,7 +2786,7 @@ describe("Groupchats", function () {
                 preventDefault: function preventDefault () {},
                 keyCode: 13
             });
-            expect(view.model.validateRoleOrAffiliationChangeArgs).toHaveBeenCalled();
+            await u.waitUntil(() => view.model.validateRoleOrAffiliationChangeArgs.calls.count());
             await u.waitUntil(() => view.querySelector('.message:last-child')?.textContent?.trim() ===
                 "Error: the \"ban\" command takes two arguments, the user's nickname and optionally a reason.");
 
@@ -2802,7 +2801,7 @@ describe("Groupchats", function () {
             textarea.value = '/ban annoyingGuy You\'re annoying';
             message_form.onFormSubmitted(new Event('submit'));
 
-            expect(view.model.validateRoleOrAffiliationChangeArgs.calls.count()).toBe(2);
+            await u.waitUntil(() => view.model.validateRoleOrAffiliationChangeArgs.calls.count() === 2);
             // Check that the member list now gets updated
             expect(Strophe.serialize(sent_IQ)).toBe(
                 `<iq id="${IQ_id}" to="lounge@montague.lit" type="set" xmlns="jabber:client">`+
@@ -2886,7 +2885,7 @@ describe("Groupchats", function () {
                 preventDefault: function preventDefault () {},
                 keyCode: 13
             });
-            expect(view.model.validateRoleOrAffiliationChangeArgs).toHaveBeenCalled();
+            await u.waitUntil(() => view.model.validateRoleOrAffiliationChangeArgs.calls.count());
             await u.waitUntil(() => view.querySelector('.message:last-child')?.textContent?.trim() ===
                 "Error: the \"kick\" command takes two arguments, the user's nickname and optionally a reason.");
             expect(view.model.setRole).not.toHaveBeenCalled();
@@ -2897,7 +2896,7 @@ describe("Groupchats", function () {
             textarea.value = '/kick @annoying guy You\'re annoying';
             message_form.onFormSubmitted(new Event('submit'));
 
-            expect(view.model.validateRoleOrAffiliationChangeArgs.calls.count()).toBe(2);
+            await u.waitUntil(() => view.model.validateRoleOrAffiliationChangeArgs.calls.count() === 2);
             expect(view.model.setRole).toHaveBeenCalled();
             expect(Strophe.serialize(sent_IQ)).toBe(
                 `<iq id="${IQ_id}" to="lounge@montague.lit" type="set" xmlns="jabber:client">`+
@@ -2988,7 +2987,7 @@ describe("Groupchats", function () {
                 keyCode: 13
             });
 
-            expect(view.model.validateRoleOrAffiliationChangeArgs).toHaveBeenCalled();
+            await u.waitUntil(() => view.model.validateRoleOrAffiliationChangeArgs.calls.count());
             await u.waitUntil(() => view.querySelector('.message:last-child')?.textContent?.trim() ===
                 "Error: the \"op\" command takes two arguments, the user's nickname and optionally a reason.");
 
@@ -3000,7 +2999,7 @@ describe("Groupchats", function () {
             textarea.value = '/op trustworthyguy You\'re trustworthy';
             message_form.onFormSubmitted(new Event('submit'));
 
-            expect(view.model.validateRoleOrAffiliationChangeArgs.calls.count()).toBe(2);
+            await u.waitUntil(() => view.model.validateRoleOrAffiliationChangeArgs.calls.count() === 2);
             expect(view.model.setRole).toHaveBeenCalled();
             expect(Strophe.serialize(sent_IQ)).toBe(
                 `<iq id="${IQ_id}" to="lounge@montague.lit" type="set" xmlns="jabber:client">`+
@@ -3044,7 +3043,7 @@ describe("Groupchats", function () {
             textarea.value = '/deop trustworthyguy Perhaps not';
             message_form.onFormSubmitted(new Event('submit'));
 
-            expect(view.model.validateRoleOrAffiliationChangeArgs.calls.count()).toBe(3);
+            await u.waitUntil(() => view.model.validateRoleOrAffiliationChangeArgs.calls.count() === 3);
             expect(view.model.setRole).toHaveBeenCalled();
             expect(Strophe.serialize(sent_IQ)).toBe(
                 `<iq id="${IQ_id}" to="lounge@montague.lit" type="set" xmlns="jabber:client">`+
@@ -3127,7 +3126,7 @@ describe("Groupchats", function () {
                 keyCode: 13
             });
 
-            expect(view.model.validateRoleOrAffiliationChangeArgs).toHaveBeenCalled();
+            await u.waitUntil(() => view.model.validateRoleOrAffiliationChangeArgs.calls.count());
             await u.waitUntil(() => view.querySelector('.message:last-child')?.textContent?.trim() ===
                 "Error: the \"mute\" command takes two arguments, the user's nickname and optionally a reason.");
             expect(view.model.setRole).not.toHaveBeenCalled();
@@ -3138,7 +3137,7 @@ describe("Groupchats", function () {
             textarea.value = '/mute annoyingGuy You\'re annoying';
             message_form.onFormSubmitted(new Event('submit'));
 
-            expect(view.model.validateRoleOrAffiliationChangeArgs.calls.count()).toBe(2);
+            await u.waitUntil(() => view.model.validateRoleOrAffiliationChangeArgs.calls.count() === 2)
             expect(view.model.setRole).toHaveBeenCalled();
             expect(Strophe.serialize(sent_IQ)).toBe(
                 `<iq id="${IQ_id}" to="lounge@montague.lit" type="set" xmlns="jabber:client">`+
@@ -3179,7 +3178,7 @@ describe("Groupchats", function () {
             textarea.value = '/voice annoyingGuy Now you can talk again';
             message_form.onFormSubmitted(new Event('submit'));
 
-            expect(view.model.validateRoleOrAffiliationChangeArgs.calls.count()).toBe(3);
+            await u.waitUntil(() => view.model.validateRoleOrAffiliationChangeArgs.calls.count() === 3);
             expect(view.model.setRole).toHaveBeenCalled();
             expect(Strophe.serialize(sent_IQ)).toBe(
                 `<iq id="${IQ_id}" to="lounge@montague.lit" type="set" xmlns="jabber:client">`+

src/plugins/muc-views/utils.js

@@ -298,117 +298,102 @@ export function showOccupantModal (ev, occupant) {
 }
 
 
-export function parseMessageForMUCCommands (muc, text) {
-    if (
-        api.settings.get('muc_disable_slash_commands') &&
-        !Array.isArray(api.settings.get('muc_disable_slash_commands'))
-    ) {
-        return parseMessageForCommands(muc, text);
+export function parseMessageForMUCCommands (data, handled) {
+    if (handled || (
+            api.settings.get('muc_disable_slash_commands') &&
+            !Array.isArray(api.settings.get('muc_disable_slash_commands'))
+    )) {
+        return handled;
     }
+
+    let text = data.text;
     text = text.replace(/^\s*/, '');
     const command = (text.match(/^\/([a-zA-Z]*) ?/) || ['']).pop().toLowerCase();
     if (!command) {
         return false;
     }
 
+    const model = data.model;
     const args = text.slice(('/' + command).length + 1).trim();
-    if (!muc.getAllowedCommands().includes(command)) {
-        return false;
-    }
-
-    switch (command) {
-        case 'admin': {
-            verifyAndSetAffiliation(muc, command, args, ['owner']);
-            break;
-        }
-        case 'ban': {
-            verifyAndSetAffiliation(muc, command, args, ['admin', 'owner']);
-            break;
-        }
-        case 'modtools': {
-            showModeratorToolsModal(muc, args);
-            break;
-        }
-        case 'deop': {
-            // FIXME: /deop only applies to setting a moderators
-            // role to "participant" (which only admin/owner can
-            // do). Moderators can however set non-moderator's role
-            // to participant (e.g. visitor => participant).
-            // Currently we don't distinguish between these two
-            // cases.
-            setRole(muc, command, args, ['admin', 'owner']);
-            break;
+    const allowed_commands = model.getAllowedCommands() ?? [];
+
+    if (command === 'admin' && allowed_commands.includes(command)) {
+        verifyAndSetAffiliation(model, command, args, ['owner']);
+        return true;
+    } else if (command === 'ban' && allowed_commands.includes(command)) {
+        verifyAndSetAffiliation(model, command, args, ['admin', 'owner']);
+        return true;
+    } else if (command === 'modtools' && allowed_commands.includes(command)) {
+        showModeratorToolsModal(model, args);
+        return true;
+    } else if (command === 'deop' && allowed_commands.includes(command)) {
+        // FIXME: /deop only applies to setting a moderators
+        // role to "participant" (which only admin/owner can
+        // do). Moderators can however set non-moderator's role
+        // to participant (e.g. visitor => participant).
+        // Currently we don't distinguish between these two
+        // cases.
+        setRole(model, command, args, ['admin', 'owner']);
+        return true;
+    } else if (command === 'destroy' && allowed_commands.includes(command)) {
+        if (!model.verifyAffiliations(['owner'])) {
+            return true;
         }
-        case 'destroy': {
-            if (!muc.verifyAffiliations(['owner'])) {
-                break;
-            }
-            destroyMUC(muc).catch(e => muc.onCommandError(e));
-            break;
-        }
-        case 'help': {
-            muc.set({ 'show_help_messages': false }, { 'silent': true });
-            muc.set({ 'show_help_messages': true });
-            break;
-        }
-        case 'kick': {
-            setRole(muc, command, args, [], ['moderator']);
-            break;
-        }
-        case 'mute': {
-            setRole(muc, command, args, [], ['moderator']);
-            break;
-        }
-        case 'member': {
-            verifyAndSetAffiliation(muc, command, args, ['admin', 'owner']);
-            break;
-        }
-        case 'nick': {
-            if (!muc.verifyRoles(['visitor', 'participant', 'moderator'])) {
-                break;
-            } else if (args.length === 0) {
-                // e.g. Your nickname is "coolguy69"
-                const message = __('Your nickname is "%1$s"', muc.get('nick'));
-                muc.createMessage({ message, 'type': 'error' });
-            } else {
-                muc.setNickname(args);
-            }
-            break;
-        }
-        case 'owner':
-            verifyAndSetAffiliation(muc, command, args, ['owner']);
-            break;
-        case 'op': {
-            setRole(muc, command, args, ['admin', 'owner']);
-            break;
-        }
-        case 'register': {
-            if (args.length > 1) {
-                muc.createMessage({
-                    'message': __('Error: invalid number of arguments'),
-                    'type': 'error'
-                });
-            } else {
-                muc.registerNickname().then(err_msg => {
-                    err_msg && muc.createMessage({ 'message': err_msg, 'type': 'error' });
-                });
-            }
-            break;
-        }
-        case 'revoke': {
-            verifyAndSetAffiliation(muc, command, args, ['admin', 'owner']);
-            break;
+        destroyMUC(model).catch(e => model.onCommandError(e));
+        return true;
+    } else if (command === 'help' && allowed_commands.includes(command)) {
+        model.set({ 'show_help_messages': false }, { 'silent': true });
+        model.set({ 'show_help_messages': true });
+        return true;
+    } else if (command === 'kick' && allowed_commands.includes(command)) {
+        setRole(model, command, args, [], ['moderator']);
+        return true;
+    } else if (command === 'mute' && allowed_commands.includes(command)) {
+        setRole(model, command, args, [], ['moderator']);
+        return true;
+    } else if (command === 'member' && allowed_commands.includes(command)) {
+        verifyAndSetAffiliation(model, command, args, ['admin', 'owner']);
+        return true;
+    } else if (command === 'nick' && allowed_commands.includes(command)) {
+        if (!model.verifyRoles(['visitor', 'participant', 'moderator'])) {
+            return true;
+        } else if (args.length === 0) {
+            // e.g. Your nickname is "coolguy69"
+            const message = __('Your nickname is "%1$s"', model.get('nick'));
+            model.createMessage({ message, 'type': 'error' });
+        } else {
+            model.setNickname(args);
         }
-        case 'topic':
-        case 'subject':
-            muc.setSubject(args);
-            break;
-        case 'voice': {
-            setRole(muc, command, args, [], ['moderator']);
-            break;
+        return true;
+    } else if (command === 'owner' && allowed_commands.includes(command)) {
+        verifyAndSetAffiliation(model, command, args, ['owner']);
+        return true;
+    } else if (command === 'op' && allowed_commands.includes(command)) {
+        setRole(model, command, args, ['admin', 'owner']);
+        return true;
+    } else if (command === 'register' && allowed_commands.includes(command)) {
+        if (args.length > 1) {
+            model.createMessage({
+                'message': __('Error: invalid number of arguments'),
+                'type': 'error'
+            });
+        } else {
+            model.registerNickname().then(err_msg => {
+                err_msg && model.createMessage({ 'message': err_msg, 'type': 'error' });
+            });
         }
-        default:
-            return parseMessageForCommands(muc, text);
+        return true;
+    } else if (command === 'revoke' && allowed_commands.includes(command)) {
+        verifyAndSetAffiliation(model, command, args, ['admin', 'owner']);
+        return true;
+    } else if (command === 'topic' && allowed_commands.includes(command) ||
+            command === 'subject' && allowed_commands.includes(command)) {
+        model.setSubject(args);
+        return true;
+    } else if (command === 'voice' && allowed_commands.includes(command)) {
+        setRole(model, command, args, [], ['moderator']);
+        return true;
+    } else {
+        return false;
     }
-    return true;
 }