feat: allow timeout-related commands to be used in multiple channels (#5402)

This changes the behaviour of the following commands:
 - `/ban`
 - `/timeout`
 - `/untimeout`
 - `/unban`

All of those commands now accept one or more `--channel` parameters to override which channel the action should take place in.
The `--channel` parameter accepts a channel ID or channel name with the same syntax as the other "user targets" do (e.g. `id:11148817` or `pajlada`)

examples
Ban user in the chat you're typing in:  
`/ban weeb123`

Ban user in the chat you're typing in, with a reason specified:  
`/ban weeb123 the ban reason`

Ban user in a separate chat, with a reason specified:  
`/ban --channel pajlada weeb123 the ban reason`

Ban user in two separate chats, with a reason specified:  
`/ban --channel pajlada --channel id:117166826 weeb123 the ban reason`


Timeout user in the chat you're typing in:  
`/timeout weeb123`

Timeout user in the chat you're typing in, with a reason specified:  
`/timeout weeb123 10m the timeout reason`

Timeout user in a separate chat, with a reason specified:  
`/timeout --channel pajlada weeb123 10m the timeout reason`

Timeout user in two separate chats, with a reason specified:  
`/timeout --channel pajlada --channel id:117166826 weeb123 10m the timeout reason`


Unban user in the chat you're typing in:  
`/unban weeb123`

Unban user in a separate chat:  
`/unban --channel pajlada weeb123`

Unban user in two separate chats:  
`/unban --channel pajlada --channel id:117166826 weeb123`

.gitmodules

@@ -41,3 +41,6 @@
 [submodule "tools/crash-handler"]
 	path = tools/crash-handler
 	url = https://github.com/Chatterino/crash-handler
+[submodule "lib/expected-lite"]
+	path = lib/expected-lite
+	url = https://github.com/martinmoene/expected-lite

CHANGELOG.md

@@ -7,6 +7,7 @@
 - Minor: Add option to customise Moderation buttons with images. (#5369)
 - Minor: Colored usernames now update on the fly when changing the "Color @usernames" setting. (#5300)
 - Minor: Added `flags.action` filter variable, allowing you to filter on `/me` messages. (#5397)
+- Minor: Added the ability for `/ban`, `/timeout`, `/unban`, and `/untimeout` to specify multiple channels to duplicate the action to. Example: `/timeout --channel id:11148817 --channel testaccount_420 forsen 7m game complaining`. (#5402)
 - Minor: The size of the emote popup is now saved. (#5415)
 - Minor: Added the ability to duplicate tabs. (#5277)
 - Minor: Improved error messages for channel update commands. (#5429)

benchmarks/src/RecentMessages.cpp

@@ -4,6 +4,7 @@
 #include "messages/Emote.hpp"
 #include "mocks/DisabledStreamerMode.hpp"
 #include "mocks/EmptyApplication.hpp"
+#include "mocks/LinkResolver.hpp"
 #include "mocks/TwitchIrcServer.hpp"
 #include "mocks/UserData.hpp"
 #include "providers/bttv/BttvEmotes.hpp"
@@ -99,10 +100,16 @@ public:
         return &this->streamerMode;
     }
 
+    ILinkResolver *getLinkResolver() override
+    {
+        return &this->linkResolver;
+    }
+
     AccountController accounts;
     Emotes emotes;
     mock::UserDataController userData;
     mock::MockTwitchIrcServer twitch;
+    mock::EmptyLinkResolver linkResolver;
     ChatterinoBadges chatterinoBadges;
     FfzBadges ffzBadges;
     SeventvBadges seventvBadges;

lib/expected-lite

@@ -0,0 +1 @@
+Subproject commit 3634b0a6d8dffcffad4d1355253d79290c0c754c

mocks/include/mocks/EmptyApplication.hpp

@@ -19,6 +19,11 @@ public:
 
     virtual ~EmptyApplication() = default;
 
+    bool isTest() const override
+    {
+        return true;
+    }
+
     const Paths &getPaths() override
     {
         return this->paths_;
@@ -137,7 +142,7 @@ public:
         return nullptr;
     }
 
-    Logging *getChatLogger() override
+    ILogging *getChatLogger() override
     {
         assert(!"getChatLogger was called without being initialized");
         return nullptr;

mocks/include/mocks/LinkResolver.hpp

@@ -0,0 +1,32 @@
+#pragma once
+
+#include "providers/links/LinkResolver.hpp"
+
+#include <gmock/gmock.h>
+#include <QString>
+#include <QStringList>
+
+namespace chatterino::mock {
+
+class LinkResolver : public ILinkResolver
+{
+public:
+    LinkResolver() = default;
+    ~LinkResolver() override = default;
+
+    MOCK_METHOD(void, resolve, (LinkInfo * info), (override));
+};
+
+class EmptyLinkResolver : public ILinkResolver
+{
+public:
+    EmptyLinkResolver() = default;
+    ~EmptyLinkResolver() override = default;
+
+    void resolve(LinkInfo *info) override
+    {
+        //
+    }
+};
+
+}  // namespace chatterino::mock

mocks/include/mocks/Logging.hpp

@@ -0,0 +1,36 @@
+#pragma once
+
+#include "singletons/Logging.hpp"
+
+#include <gmock/gmock.h>
+#include <QString>
+#include <QStringList>
+
+namespace chatterino::mock {
+
+class Logging : public ILogging
+{
+public:
+    Logging() = default;
+    ~Logging() override = default;
+
+    MOCK_METHOD(void, addMessage,
+                (const QString &channelName, MessagePtr message,
+                 const QString &platformName),
+                (override));
+};
+
+class EmptyLogging : public ILogging
+{
+public:
+    EmptyLogging() = default;
+    ~EmptyLogging() override = default;
+
+    void addMessage(const QString &channelName, MessagePtr message,
+                    const QString &platformName) override
+    {
+        //
+    }
+};
+
+}  // namespace chatterino::mock

resources/licenses/expected-lite.txt

@@ -0,0 +1,23 @@
+Boost Software License - Version 1.0 - August 17th, 2003
+
+Permission is hereby granted, free of charge, to any person or organization
+obtaining a copy of the software and accompanying documentation covered by
+this license (the "Software") to use, reproduce, display, distribute,
+execute, and transmit the Software, and to prepare derivative works of the
+Software, and to permit third-parties to whom the Software is furnished to
+do so, all subject to the following:
+
+The copyright notices in the Software and this entire statement, including
+the above license grant, this restriction and the following disclaimer,
+must be included in all copies of the Software, in whole or in part, and
+all derivative works of the Software, unless such copies or derivative
+works are solely in the form of machine-executable object code generated by
+a source language processor.
+
+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+FITNESS FOR A PARTICULAR PURPOSE, TITLE AND NON-INFRINGEMENT. IN NO EVENT
+SHALL THE COPYRIGHT HOLDERS OR ANYONE DISTRIBUTING THE SOFTWARE BE LIABLE
+FOR ANY DAMAGES OR OTHER LIABILITY, WHETHER IN CONTRACT, TORT OR OTHERWISE,
+ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+DEALINGS IN THE SOFTWARE.

src/Application.cpp

@@ -504,7 +504,7 @@ PubSub *Application::getTwitchPubSub()
     return this->twitchPubSub.get();
 }
 
-Logging *Application::getChatLogger()
+ILogging *Application::getChatLogger()
 {
     assertInGuiThread();
 

src/Application.hpp

@@ -35,6 +35,7 @@ class PluginController;
 
 class Theme;
 class WindowManager;
+class ILogging;
 class Logging;
 class Paths;
 class Emotes;
@@ -64,6 +65,8 @@ public:
 
     static IApplication *instance;
 
+    virtual bool isTest() const = 0;
+
     virtual const Paths &getPaths() = 0;
     virtual const Args &getArgs() = 0;
     virtual Theme *getThemes() = 0;
@@ -80,7 +83,7 @@ public:
     virtual ITwitchIrcServer *getTwitch() = 0;
     virtual IAbstractIrcServer *getTwitchAbstract() = 0;
     virtual PubSub *getTwitchPubSub() = 0;
-    virtual Logging *getChatLogger() = 0;
+    virtual ILogging *getChatLogger() = 0;
     virtual IChatterinoBadges *getChatterinoBadges() = 0;
     virtual FfzBadges *getFfzBadges() = 0;
     virtual SeventvBadges *getSeventvBadges() = 0;
@@ -121,6 +124,11 @@ public:
     Application &operator=(const Application &) = delete;
     Application &operator=(Application &&) = delete;
 
+    bool isTest() const override
+    {
+        return false;
+    }
+
     /**
      * In the interim, before we remove _exit(0); from RunGui.cpp,
      * this will destroy things we know can be destroyed
@@ -191,7 +199,7 @@ public:
     ITwitchIrcServer *getTwitch() override;
     IAbstractIrcServer *getTwitchAbstract() override;
     PubSub *getTwitchPubSub() override;
-    Logging *getChatLogger() override;
+    ILogging *getChatLogger() override;
     FfzBadges *getFfzBadges() override;
     SeventvBadges *getSeventvBadges() override;
     IUserDataController *getUserData() override;

src/CMakeLists.txt

@@ -107,6 +107,8 @@ set(SOURCE_FILES
         controllers/commands/builtin/twitch/UpdateChannel.hpp
         controllers/commands/builtin/twitch/UpdateColor.cpp
         controllers/commands/builtin/twitch/UpdateColor.hpp
+        controllers/commands/common/ChannelAction.cpp
+        controllers/commands/common/ChannelAction.hpp
         controllers/commands/CommandContext.hpp
         controllers/commands/CommandController.cpp
         controllers/commands/CommandController.hpp
@@ -1003,6 +1005,9 @@ target_include_directories(${LIBRARY_PROJECT} PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}
 # semver dependency https://github.com/Neargye/semver
 target_include_directories(${LIBRARY_PROJECT} PUBLIC ${CMAKE_SOURCE_DIR}/lib/semver/include)
 
+# expected-lite dependency https://github.com/martinmoene/expected-lite
+target_include_directories(${LIBRARY_PROJECT} PUBLIC ${CMAKE_SOURCE_DIR}/lib/expected-lite/include)
+
 # miniaudio dependency https://github.com/mackron/miniaudio
 if (USE_SYSTEM_MINIAUDIO)
     message(STATUS "Building with system miniaudio")

src/common/QLogging.cpp

@@ -11,6 +11,7 @@ Q_LOGGING_CATEGORY(chatterinoArgs, "chatterino.args", logThreshold);
 Q_LOGGING_CATEGORY(chatterinoBenchmark, "chatterino.benchmark", logThreshold);
 Q_LOGGING_CATEGORY(chatterinoBttv, "chatterino.bttv", logThreshold);
 Q_LOGGING_CATEGORY(chatterinoCache, "chatterino.cache", logThreshold);
+Q_LOGGING_CATEGORY(chatterinoCommands, "chatterino.commands", logThreshold);
 Q_LOGGING_CATEGORY(chatterinoCommon, "chatterino.common", logThreshold);
 Q_LOGGING_CATEGORY(chatterinoCrashhandler, "chatterino.crashhandler",
                    logThreshold);

src/common/QLogging.hpp

@@ -7,6 +7,7 @@ Q_DECLARE_LOGGING_CATEGORY(chatterinoArgs);
 Q_DECLARE_LOGGING_CATEGORY(chatterinoBenchmark);
 Q_DECLARE_LOGGING_CATEGORY(chatterinoBttv);
 Q_DECLARE_LOGGING_CATEGORY(chatterinoCache);
+Q_DECLARE_LOGGING_CATEGORY(chatterinoCommands);
 Q_DECLARE_LOGGING_CATEGORY(chatterinoCommon);
 Q_DECLARE_LOGGING_CATEGORY(chatterinoCrashhandler);
 Q_DECLARE_LOGGING_CATEGORY(chatterinoEmoji);
@@ -17,6 +18,7 @@ Q_DECLARE_LOGGING_CATEGORY(chatterinoHighlights);
 Q_DECLARE_LOGGING_CATEGORY(chatterinoHotkeys);
 Q_DECLARE_LOGGING_CATEGORY(chatterinoHTTP);
 Q_DECLARE_LOGGING_CATEGORY(chatterinoImage);
+Q_DECLARE_LOGGING_CATEGORY(chatterinoImageuploader);
 Q_DECLARE_LOGGING_CATEGORY(chatterinoIrc);
 Q_DECLARE_LOGGING_CATEGORY(chatterinoIvr);
 Q_DECLARE_LOGGING_CATEGORY(chatterinoLiveupdates);
@@ -26,7 +28,6 @@ Q_DECLARE_LOGGING_CATEGORY(chatterinoMessage);
 Q_DECLARE_LOGGING_CATEGORY(chatterinoNativeMessage);
 Q_DECLARE_LOGGING_CATEGORY(chatterinoNetwork);
 Q_DECLARE_LOGGING_CATEGORY(chatterinoNotification);
-Q_DECLARE_LOGGING_CATEGORY(chatterinoImageuploader);
 Q_DECLARE_LOGGING_CATEGORY(chatterinoPubSub);
 Q_DECLARE_LOGGING_CATEGORY(chatterinoRecentMessages);
 Q_DECLARE_LOGGING_CATEGORY(chatterinoSettings);

src/controllers/commands/builtin/twitch/Ban.cpp

@@ -1,13 +1,14 @@
 #include "controllers/commands/builtin/twitch/Ban.hpp"
 
 #include "Application.hpp"
+#include "common/QLogging.hpp"
 #include "controllers/accounts/AccountController.hpp"
 #include "controllers/commands/CommandContext.hpp"
+#include "controllers/commands/common/ChannelAction.hpp"
 #include "messages/MessageBuilder.hpp"
 #include "providers/twitch/api/Helix.hpp"
 #include "providers/twitch/TwitchAccount.hpp"
 #include "providers/twitch/TwitchChannel.hpp"
-#include "util/Twitch.hpp"
 
 namespace {
 
@@ -80,13 +81,12 @@ QString formatBanTimeoutError(const char *operation, HelixBanUserError error,
     return errorMessage;
 }
 
-void banUserByID(const ChannelPtr &channel, const TwitchChannel *twitchChannel,
+void banUserByID(const ChannelPtr &channel, const QString &channelID,
                  const QString &sourceUserID, const QString &targetUserID,
                  const QString &reason, const QString &displayName)
 {
     getHelix()->banUser(
-        twitchChannel->roomId(), sourceUserID, targetUserID, std::nullopt,
+        channelID, sourceUserID, targetUserID, std::nullopt, reason,
-        reason,
         [] {
             // No response for bans, they're emitted over pubsub/IRC instead
         },
@@ -97,14 +97,13 @@ void banUserByID(const ChannelPtr &channel, const TwitchChannel *twitchChannel,
         });
 }
 
-void timeoutUserByID(const ChannelPtr &channel,
+void timeoutUserByID(const ChannelPtr &channel, const QString &channelID,
-                     const TwitchChannel *twitchChannel,
                      const QString &sourceUserID, const QString &targetUserID,
                      int duration, const QString &reason,
                      const QString &displayName)
 {
     getHelix()->banUser(
-        twitchChannel->roomId(), sourceUserID, targetUserID, duration, reason,
+        channelID, sourceUserID, targetUserID, duration, reason,
         [] {
             // No response for timeouts, they're emitted over pubsub/IRC instead
         },
@@ -121,63 +120,108 @@ namespace chatterino::commands {
 
 QString sendBan(const CommandContext &ctx)
 {
-    const auto &words = ctx.words;
+    const auto command = QStringLiteral("/ban");
-    const auto &channel = ctx.channel;
+    const auto usage = QStringLiteral(
-    const auto *twitchChannel = ctx.twitchChannel;
+        R"(Usage: "/ban [options...] <username> [reason]" - Permanently prevent a user from chatting via their username. Reason is optional and will be shown to the target user and other moderators. Options: --channel <channel> to override which channel the ban takes place in (can be specified multiple times).)");
+    const auto actions = parseChannelAction(ctx, command, usage, false, true);
 
-    if (channel == nullptr)
+    if (!actions.has_value())
     {
+        if (ctx.channel != nullptr)
+        {
+            ctx.channel->addMessage(makeSystemMessage(actions.error()));
+        }
+        else
+        {
+            qCWarning(chatterinoCommands)
+                << "Error parsing command:" << actions.error();
+        }
+
         return "";
     }
 
-    if (twitchChannel == nullptr)
+    assert(!actions.value().empty());
-    {
-        channel->addMessage(makeSystemMessage(
-            QString("The /ban command only works in Twitch channels.")));
-        return "";
-    }
-
-    const auto *usageStr =
-        "Usage: \"/ban <username> [reason]\" - Permanently prevent a user "
-        "from chatting. Reason is optional and will be shown to the target "
-        "user and other moderators. Use \"/unban\" to remove a ban.";
-    if (words.size() < 2)
-    {
-        channel->addMessage(makeSystemMessage(usageStr));
-        return "";
-    }
 
     auto currentUser = getIApp()->getAccounts()->twitch.getCurrent();
     if (currentUser->isAnon())
     {
-        channel->addMessage(
+        ctx.channel->addMessage(
             makeSystemMessage("You must be logged in to ban someone!"));
         return "";
     }
 
-    const auto &rawTarget = words.at(1);
+    for (const auto &action : actions.value())
-    auto [targetUserName, targetUserID] = parseUserNameOrID(rawTarget);
+    {
-    auto reason = words.mid(2).join(' ');
+        const auto &reason = action.reason;
 
-    if (!targetUserID.isEmpty())
+        QStringList userLoginsToFetch;
-    {
+        QStringList userIDs;
-        banUserByID(channel, twitchChannel, currentUser->getUserId(),
+        if (action.target.id.isEmpty())
-                    targetUserID, reason, targetUserID);
+        {
-    }
+            assert(!action.target.login.isEmpty() &&
-    else
+                   "Ban Action target username AND user ID may not be "
-    {
+                   "empty at the same time");
-        getHelix()->getUserByName(
+            userLoginsToFetch.append(action.target.login);
-            targetUserName,
+        }
-            [channel, currentUser, twitchChannel,
+        else
-             reason](const auto &targetUser) {
+        {
-                banUserByID(channel, twitchChannel, currentUser->getUserId(),
+            // For hydration
-                            targetUser.id, reason, targetUser.displayName);
+            userIDs.append(action.target.id);
-            },
+        }
-            [channel, targetUserName{targetUserName}] {
+        if (action.channel.id.isEmpty())
-                // Equivalent error from IRC
+        {
-                channel->addMessage(makeSystemMessage(
+            assert(!action.channel.login.isEmpty() &&
-                    QString("Invalid username: %1").arg(targetUserName)));
+                   "Ban Action channel username AND user ID may not be "
-            });
+                   "empty at the same time");
+            userLoginsToFetch.append(action.channel.login);
+        }
+        else
+        {
+            // For hydration
+            userIDs.append(action.channel.id);
+        }
+
+        if (!userLoginsToFetch.isEmpty())
+        {
+            // At least 1 user ID needs to be resolved before we can take action
+            // userIDs is filled up with the data we already have to hydrate the action channel & action target
+            getHelix()->fetchUsers(
+                userIDs, userLoginsToFetch,
+                [channel{ctx.channel}, actionChannel{action.channel},
+                 actionTarget{action.target}, currentUser, reason,
+                 userLoginsToFetch](const auto &users) mutable {
+                    if (!actionChannel.hydrateFrom(users))
+                    {
+                        channel->addMessage(makeSystemMessage(
+                            QString("Failed to ban, bad channel name: %1")
+                                .arg(actionChannel.login)));
+                        return;
+                    }
+                    if (!actionTarget.hydrateFrom(users))
+                    {
+                        channel->addMessage(makeSystemMessage(
+                            QString("Failed to ban, bad target name: %1")
+                                .arg(actionTarget.login)));
+                        return;
+                    }
+
+                    banUserByID(channel, actionChannel.id,
+                                currentUser->getUserId(), actionTarget.id,
+                                reason, actionTarget.displayName);
+                },
+                [channel{ctx.channel}, userLoginsToFetch] {
+                    channel->addMessage(makeSystemMessage(
+                        QString("Failed to ban, bad username(s): %1")
+                            .arg(userLoginsToFetch.join(", "))));
+                });
+        }
+        else
+        {
+            // If both IDs are available, we do no hydration & just use the id as the display name
+            banUserByID(ctx.channel, action.channel.id,
+                        currentUser->getUserId(), action.target.id, reason,
+                        action.target.id);
+        }
     }
 
     return "";
@@ -221,87 +265,117 @@ QString sendBanById(const CommandContext &ctx)
     auto target = words.at(1);
     auto reason = words.mid(2).join(' ');
 
-    banUserByID(channel, twitchChannel, currentUser->getUserId(), target,
+    banUserByID(channel, twitchChannel->roomId(), currentUser->getUserId(),
-                reason, target);
+                target, reason, target);
 
     return "";
 }
 
 QString sendTimeout(const CommandContext &ctx)
 {
-    const auto &words = ctx.words;
+    const auto command = QStringLiteral("/timeout");
-    const auto &channel = ctx.channel;
+    const auto usage = QStringLiteral(
-    const auto *twitchChannel = ctx.twitchChannel;
+        R"(Usage: "/timeout [options...] <username> [duration][time unit] [reason]" - Temporarily prevent a user from chatting. Duration (optional, default=10 minutes) must be a positive integer; time unit (optional, default=s) must be one of s, m, h, d, w; maximum duration is 2 weeks. Combinations like 1d2h are also allowed. Reason is optional and will be shown to the target user and other moderators. Use "/untimeout" to remove a timeout. Options: --channel <channel> to override which channel the timeout takes place in (can be specified multiple times).)");
+    const auto actions = parseChannelAction(ctx, command, usage, true, true);
 
-    if (channel == nullptr)
+    if (!actions.has_value())
     {
+        if (ctx.channel != nullptr)
+        {
+            ctx.channel->addMessage(makeSystemMessage(actions.error()));
+        }
+        else
+        {
+            qCWarning(chatterinoCommands)
+                << "Error parsing command:" << actions.error();
+        }
+
         return "";
     }
 
-    if (twitchChannel == nullptr)
+    assert(!actions.value().empty());
-    {
-        channel->addMessage(makeSystemMessage(
-            QString("The /timeout command only works in Twitch channels.")));
-        return "";
-    }
-    const auto *usageStr =
-        "Usage: \"/timeout <username> [duration][time unit] [reason]\" - "
-        "Temporarily prevent a user from chatting. Duration (optional, "
-        "default=10 minutes) must be a positive integer; time unit "
-        "(optional, default=s) must be one of s, m, h, d, w; maximum "
-        "duration is 2 weeks. Combinations like 1d2h are also allowed. "
-        "Reason is optional and will be shown to the target user and other "
-        "moderators. Use \"/untimeout\" to remove a timeout.";
-    if (words.size() < 2)
-    {
-        channel->addMessage(makeSystemMessage(usageStr));
-        return "";
-    }
 
     auto currentUser = getIApp()->getAccounts()->twitch.getCurrent();
     if (currentUser->isAnon())
     {
-        channel->addMessage(
+        ctx.channel->addMessage(
             makeSystemMessage("You must be logged in to timeout someone!"));
         return "";
     }
 
-    const auto &rawTarget = words.at(1);
+    for (const auto &action : actions.value())
-    auto [targetUserName, targetUserID] = parseUserNameOrID(rawTarget);
-
-    int duration = 10 * 60;  // 10min
-    if (words.size() >= 3)
     {
-        duration = (int)parseDurationToSeconds(words.at(2));
+        const auto &reason = action.reason;
-        if (duration <= 0)
+
+        QStringList userLoginsToFetch;
+        QStringList userIDs;
+        if (action.target.id.isEmpty())
         {
-            channel->addMessage(makeSystemMessage(usageStr));
+            assert(!action.target.login.isEmpty() &&
-            return "";
+                   "Timeout Action target username AND user ID may not be "
+                   "empty at the same time");
+            userLoginsToFetch.append(action.target.login);
+        }
+        else
+        {
+            // For hydration
+            userIDs.append(action.target.id);
+        }
+        if (action.channel.id.isEmpty())
+        {
+            assert(!action.channel.login.isEmpty() &&
+                   "Timeout Action channel username AND user ID may not be "
+                   "empty at the same time");
+            userLoginsToFetch.append(action.channel.login);
+        }
+        else
+        {
+            // For hydration
+            userIDs.append(action.channel.id);
         }
-    }
-    auto reason = words.mid(3).join(' ');
 
-    if (!targetUserID.isEmpty())
+        if (!userLoginsToFetch.isEmpty())
-    {
+        {
-        timeoutUserByID(channel, twitchChannel, currentUser->getUserId(),
+            // At least 1 user ID needs to be resolved before we can take action
-                        targetUserID, duration, reason, targetUserID);
+            // userIDs is filled up with the data we already have to hydrate the action channel & action target
-    }
+            getHelix()->fetchUsers(
-    else
+                userIDs, userLoginsToFetch,
-    {
+                [channel{ctx.channel}, duration{action.duration},
-        getHelix()->getUserByName(
+                 actionChannel{action.channel}, actionTarget{action.target},
-            targetUserName,
+                 currentUser, reason,
-            [channel, currentUser, twitchChannel,
+                 userLoginsToFetch](const auto &users) mutable {
-             targetUserName{targetUserName}, duration,
+                    if (!actionChannel.hydrateFrom(users))
-             reason](const auto &targetUser) {
+                    {
-                timeoutUserByID(channel, twitchChannel,
+                        channel->addMessage(makeSystemMessage(
-                                currentUser->getUserId(), targetUser.id,
+                            QString("Failed to timeout, bad channel name: %1")
-                                duration, reason, targetUser.displayName);
+                                .arg(actionChannel.login)));
-            },
+                        return;
-            [channel, targetUserName{targetUserName}] {
+                    }
-                // Equivalent error from IRC
+                    if (!actionTarget.hydrateFrom(users))
-                channel->addMessage(makeSystemMessage(
+                    {
-                    QString("Invalid username: %1").arg(targetUserName)));
+                        channel->addMessage(makeSystemMessage(
-            });
+                            QString("Failed to timeout, bad target name: %1")
+                                .arg(actionTarget.login)));
+                        return;
+                    }
+
+                    timeoutUserByID(channel, actionChannel.id,
+                                    currentUser->getUserId(), actionTarget.id,
+                                    duration, reason, actionTarget.displayName);
+                },
+                [channel{ctx.channel}, userLoginsToFetch] {
+                    channel->addMessage(makeSystemMessage(
+                        QString("Failed to timeout, bad username(s): %1")
+                            .arg(userLoginsToFetch.join(", "))));
+                });
+        }
+        else
+        {
+            // If both IDs are available, we do no hydration & just use the id as the display name
+            timeoutUserByID(ctx.channel, action.channel.id,
+                            currentUser->getUserId(), action.target.id,
+                            action.duration, reason, action.target.id);
+        }
     }
 
     return "";

src/controllers/commands/builtin/twitch/Unban.cpp

@@ -1,24 +1,25 @@
+#include "controllers/commands/builtin/twitch/Unban.hpp"
+
 #include "Application.hpp"
+#include "common/Channel.hpp"
+#include "common/QLogging.hpp"
 #include "controllers/accounts/AccountController.hpp"
-#include "controllers/commands/builtin/twitch/Ban.hpp"
 #include "controllers/commands/CommandContext.hpp"
+#include "controllers/commands/common/ChannelAction.hpp"
 #include "messages/MessageBuilder.hpp"
 #include "providers/twitch/api/Helix.hpp"
 #include "providers/twitch/TwitchAccount.hpp"
-#include "providers/twitch/TwitchChannel.hpp"
-#include "util/Twitch.hpp"
 
 namespace {
 
 using namespace chatterino;
 
-void unbanUserByID(const ChannelPtr &channel,
+void unbanUserByID(const ChannelPtr &channel, const QString &channelID,
-                   const TwitchChannel *twitchChannel,
                    const QString &sourceUserID, const QString &targetUserID,
                    const QString &displayName)
 {
     getHelix()->unbanUser(
-        twitchChannel->roomId(), sourceUserID, targetUserID,
+        channelID, sourceUserID, targetUserID,
         [] {
             // No response for unbans, they're emitted over pubsub/IRC instead
         },
@@ -85,26 +86,28 @@ namespace chatterino::commands {
 
 QString unbanUser(const CommandContext &ctx)
 {
-    if (ctx.channel == nullptr)
+    const auto command = ctx.words.at(0).toLower();
+    const auto usage =
+        QStringLiteral(
+            R"(Usage: "%1 <username> - Removes a ban on a user. Options: --channel <channel> to override which channel the unban takes place in (can be specified multiple times).)")
+            .arg(command);
+    const auto actions = parseChannelAction(ctx, command, usage, false, false);
+    if (!actions.has_value())
     {
+        if (ctx.channel != nullptr)
+        {
+            ctx.channel->addMessage(makeSystemMessage(actions.error()));
+        }
+        else
+        {
+            qCWarning(chatterinoCommands)
+                << "Error parsing command:" << actions.error();
+        }
+
         return "";
     }
 
-    auto commandName = ctx.words.at(0).toLower();
+    assert(!actions.value().empty());
-    if (ctx.twitchChannel == nullptr)
-    {
-        ctx.channel->addMessage(makeSystemMessage(
-            QString("The %1 command only works in Twitch channels.")
-                .arg(commandName)));
-        return "";
-    }
-    if (ctx.words.size() < 2)
-    {
-        ctx.channel->addMessage(makeSystemMessage(
-            QString("Usage: \"%1 <username>\" - Removes a ban on a user.")
-                .arg(commandName)));
-        return "";
-    }
 
     auto currentUser = getIApp()->getAccounts()->twitch.getCurrent();
     if (currentUser->isAnon())
@@ -114,29 +117,78 @@ QString unbanUser(const CommandContext &ctx)
         return "";
     }
 
-    const auto &rawTarget = ctx.words.at(1);
+    for (const auto &action : actions.value())
-    auto [targetUserName, targetUserID] = parseUserNameOrID(rawTarget);
+    {
+        const auto &reason = action.reason;
 
-    if (!targetUserID.isEmpty())
+        QStringList userLoginsToFetch;
-    {
+        QStringList userIDs;
-        unbanUserByID(ctx.channel, ctx.twitchChannel, currentUser->getUserId(),
+        if (action.target.id.isEmpty())
-                      targetUserID, targetUserID);
+        {
-    }
+            assert(!action.target.login.isEmpty() &&
-    else
+                   "Unban Action target username AND user ID may not be "
-    {
+                   "empty at the same time");
-        getHelix()->getUserByName(
+            userLoginsToFetch.append(action.target.login);
-            targetUserName,
+        }
-            [channel{ctx.channel}, currentUser,
+        else
-             twitchChannel{ctx.twitchChannel},
+        {
-             targetUserName{targetUserName}](const auto &targetUser) {
+            // For hydration
-                unbanUserByID(channel, twitchChannel, currentUser->getUserId(),
+            userIDs.append(action.target.id);
-                              targetUser.id, targetUser.displayName);
+        }
-            },
+        if (action.channel.id.isEmpty())
-            [channel{ctx.channel}, targetUserName{targetUserName}] {
+        {
-                // Equivalent error from IRC
+            assert(!action.channel.login.isEmpty() &&
-                channel->addMessage(makeSystemMessage(
+                   "Unban Action channel username AND user ID may not be "
-                    QString("Invalid username: %1").arg(targetUserName)));
+                   "empty at the same time");
-            });
+            userLoginsToFetch.append(action.channel.login);
+        }
+        else
+        {
+            // For hydration
+            userIDs.append(action.channel.id);
+        }
+
+        if (!userLoginsToFetch.isEmpty())
+        {
+            // At least 1 user ID needs to be resolved before we can take action
+            // userIDs is filled up with the data we already have to hydrate the action channel & action target
+            getHelix()->fetchUsers(
+                userIDs, userLoginsToFetch,
+                [channel{ctx.channel}, actionChannel{action.channel},
+                 actionTarget{action.target}, currentUser, reason,
+                 userLoginsToFetch](const auto &users) mutable {
+                    if (!actionChannel.hydrateFrom(users))
+                    {
+                        channel->addMessage(makeSystemMessage(
+                            QString("Failed to timeout, bad channel name: %1")
+                                .arg(actionChannel.login)));
+                        return;
+                    }
+                    if (!actionTarget.hydrateFrom(users))
+                    {
+                        channel->addMessage(makeSystemMessage(
+                            QString("Failed to timeout, bad target name: %1")
+                                .arg(actionTarget.login)));
+                        return;
+                    }
+
+                    unbanUserByID(channel, actionChannel.id,
+                                  currentUser->getUserId(), actionTarget.id,
+                                  actionTarget.displayName);
+                },
+                [channel{ctx.channel}, userLoginsToFetch] {
+                    channel->addMessage(makeSystemMessage(
+                        QString("Failed to timeout, bad username(s): %1")
+                            .arg(userLoginsToFetch.join(", "))));
+                });
+        }
+        else
+        {
+            // If both IDs are available, we do no hydration & just use the id as the display name
+            unbanUserByID(ctx.channel, action.channel.id,
+                          currentUser->getUserId(), action.target.id,
+                          action.target.id);
+        }
     }
 
     return "";

src/controllers/commands/common/ChannelAction.cpp

@@ -0,0 +1,184 @@
+#include "controllers/commands/common/ChannelAction.hpp"
+
+#include "controllers/commands/CommandContext.hpp"
+#include "providers/twitch/api/Helix.hpp"
+#include "providers/twitch/TwitchChannel.hpp"
+#include "util/Helpers.hpp"
+#include "util/Twitch.hpp"
+
+#include <QCommandLineParser>
+#include <QStringBuilder>
+
+#include <algorithm>
+#include <ostream>
+#include <string>
+#include <vector>
+
+namespace chatterino::commands {
+
+bool IncompleteHelixUser::hydrateFrom(const std::vector<HelixUser> &users)
+{
+    // Find user in list based on our id or login
+    auto resolvedIt =
+        std::find_if(users.begin(), users.end(), [this](const auto &user) {
+            if (!this->login.isEmpty())
+            {
+                return user.login.compare(this->login, Qt::CaseInsensitive) ==
+                       0;
+            }
+            if (!this->id.isEmpty())
+            {
+                return user.id.compare(this->id, Qt::CaseInsensitive) == 0;
+            }
+            return false;
+        });
+    if (resolvedIt == users.end())
+    {
+        return false;
+    }
+    const auto &resolved = *resolvedIt;
+    this->id = resolved.id;
+    this->login = resolved.login;
+    this->displayName = resolved.displayName;
+    return true;
+}
+
+std::ostream &operator<<(std::ostream &os, const IncompleteHelixUser &u)
+{
+    os << "{id:" << u.id.toStdString() << ", login:" << u.login.toStdString()
+       << ", displayName:" << u.displayName.toStdString() << '}';
+    return os;
+}
+
+void PrintTo(const PerformChannelAction &a, std::ostream *os)
+{
+    *os << "{channel:" << a.channel << ", target:" << a.target
+        << ", reason:" << a.reason.toStdString()
+        << ", duration:" << std::to_string(a.duration) << '}';
+}
+
+nonstd::expected<std::vector<PerformChannelAction>, QString> parseChannelAction(
+    const CommandContext &ctx, const QString &command, const QString &usage,
+    bool withDuration, bool withReason)
+{
+    if (ctx.channel == nullptr)
+    {
+        // A ban action must be performed with a channel as a context
+        return nonstd::make_unexpected(
+            "A " % command %
+            " action must be performed with a channel as a context");
+    }
+
+    QCommandLineParser parser;
+    parser.setOptionsAfterPositionalArgumentsMode(
+        QCommandLineParser::ParseAsPositionalArguments);
+    parser.addPositionalArgument("username", "The name of the user to ban");
+    if (withDuration)
+    {
+        parser.addPositionalArgument("duration", "Duration of the action");
+    }
+    if (withReason)
+    {
+        parser.addPositionalArgument("reason", "The optional ban reason");
+    }
+    QCommandLineOption channelOption(
+        "channel", "Override which channel(s) to perform the action in",
+        "channel");
+    parser.addOptions({
+        channelOption,
+    });
+    parser.parse(ctx.words);
+
+    auto positionalArguments = parser.positionalArguments();
+    if (positionalArguments.isEmpty())
+    {
+        return nonstd::make_unexpected("Missing target - " % usage);
+    }
+
+    auto [targetUserName, targetUserID] =
+        parseUserNameOrID(positionalArguments.takeFirst());
+
+    PerformChannelAction base{
+        .target =
+            IncompleteHelixUser{
+                .id = targetUserID,
+                .login = targetUserName,
+                .displayName = "",
+            },
+        .duration = 0,
+    };
+
+    if (withDuration)
+    {
+        if (positionalArguments.isEmpty())
+        {
+            base.duration = 10 * 60;  // 10 min
+        }
+        else
+        {
+            auto durationStr = positionalArguments.takeFirst();
+            base.duration = (int)parseDurationToSeconds(durationStr);
+            if (base.duration <= 0)
+            {
+                return nonstd::make_unexpected("Invalid duration - " % usage);
+            }
+            if (withReason)
+            {
+                base.reason = positionalArguments.join(' ');
+            }
+        }
+    }
+    else
+    {
+        if (withReason)
+        {
+            base.reason = positionalArguments.join(' ');
+        }
+    }
+
+    std::vector<PerformChannelAction> actions;
+
+    auto overrideChannels = parser.values(channelOption);
+    if (overrideChannels.isEmpty())
+    {
+        if (ctx.twitchChannel == nullptr)
+        {
+            return nonstd::make_unexpected(
+                "The " % command % " command only works in Twitch channels");
+        }
+
+        actions.push_back(PerformChannelAction{
+            .channel =
+                {
+                    .id = ctx.twitchChannel->roomId(),
+                    .login = ctx.twitchChannel->getName(),
+                    .displayName = ctx.twitchChannel->getDisplayName(),
+                },
+            .target = base.target,
+            .reason = base.reason,
+            .duration = base.duration,
+        });
+    }
+    else
+    {
+        for (const auto &overrideChannelTarget : overrideChannels)
+        {
+            auto [channelUserName, channelUserID] =
+                parseUserNameOrID(overrideChannelTarget);
+            actions.push_back(PerformChannelAction{
+                .channel =
+                    {
+                        .id = channelUserID,
+                        .login = channelUserName,
+                    },
+                .target = base.target,
+                .reason = base.reason,
+                .duration = base.duration,
+            });
+        }
+    }
+
+    return actions;
+}
+
+}  // namespace chatterino::commands

src/controllers/commands/common/ChannelAction.hpp

@@ -0,0 +1,59 @@
+#pragma once
+
+#include <nonstd/expected.hpp>
+#include <QString>
+
+#include <ostream>
+#include <tuple>
+#include <vector>
+
+namespace chatterino {
+
+struct CommandContext;
+struct HelixUser;
+
+}  // namespace chatterino
+
+namespace chatterino::commands {
+
+struct IncompleteHelixUser {
+    QString id;
+    QString login;
+    QString displayName;
+
+    bool hydrateFrom(const std::vector<HelixUser> &users);
+
+    bool operator==(const IncompleteHelixUser &other) const
+    {
+        return std::tie(this->id, this->login, this->displayName) ==
+               std::tie(other.id, other.login, other.displayName);
+    }
+};
+
+struct PerformChannelAction {
+    // Channel to perform the action in
+    IncompleteHelixUser channel;
+    // Target to perform the action on
+    IncompleteHelixUser target;
+    QString reason;
+    int duration{};
+
+    bool operator==(const PerformChannelAction &other) const
+    {
+        return std::tie(this->channel, this->target, this->reason,
+                        this->duration) == std::tie(other.channel, other.target,
+                                                    other.reason,
+                                                    other.duration);
+    }
+};
+
+std::ostream &operator<<(std::ostream &os, const IncompleteHelixUser &u);
+// gtest printer
+// NOLINTNEXTLINE(readability-identifier-naming)
+void PrintTo(const PerformChannelAction &a, std::ostream *os);
+
+nonstd::expected<std::vector<PerformChannelAction>, QString> parseChannelAction(
+    const CommandContext &ctx, const QString &command, const QString &usage,
+    bool withDuration, bool withReason);
+
+}  // namespace chatterino::commands

src/providers/twitch/TwitchChannel.cpp

@@ -91,13 +91,6 @@ TwitchChannel::TwitchChannel(const QString &name)
 {
     qCDebug(chatterinoTwitch) << "[TwitchChannel" << name << "] Opened";
 
-    if (!getApp())
-    {
-        // This is intended for tests and benchmarks.
-        // Irc, Pubsub, live-updates, and live-notifications aren't mocked there.
-        return;
-    }
-
     this->bSignals_.emplace_back(
         getIApp()->getAccounts()->twitch.currentUserChanged.connect([this] {
             this->setMod(false);
@@ -231,13 +224,6 @@ TwitchChannel::TwitchChannel(const QString &name)
 
 TwitchChannel::~TwitchChannel()
 {
-    if (!getApp())
-    {
-        // This is for tests and benchmarks, where live-updates aren't mocked
-        // see comment in constructor.
-        return;
-    }
-
     getIApp()->getTwitch()->dropSeventvChannel(this->seventvUserID_,
                                                this->seventvEmoteSetID_);
 
@@ -586,6 +572,10 @@ void TwitchChannel::showLoginMessage()
 
 void TwitchChannel::roomIdChanged()
 {
+    if (getIApp()->isTest())
+    {
+        return;
+    }
     this->refreshPubSub();
     this->refreshBadges();
     this->refreshCheerEmotes();
@@ -792,7 +782,7 @@ void TwitchChannel::setRoomId(const QString &id)
     {
         *this->roomID_.access() = id;
         // This is intended for tests and benchmarks. See comment in constructor.
-        if (getApp())
+        if (!getIApp()->isTest())
         {
             this->roomIdChanged();
             this->loadRecentMessages();
@@ -1341,6 +1331,11 @@ void TwitchChannel::loadRecentMessagesReconnect()
 
 void TwitchChannel::refreshPubSub()
 {
+    if (getIApp()->isTest())
+    {
+        return;
+    }
+
     auto roomId = this->roomId();
     if (roomId.isEmpty())
     {

src/providers/twitch/TwitchChannel.hpp

@@ -463,6 +463,7 @@ private:
     friend class TwitchIrcServer;
     friend class TwitchMessageBuilder;
     friend class IrcMessageHandler;
+    friend class Commands_E2E_Test;
 };
 
 }  // namespace chatterino

src/singletons/Logging.hpp

@@ -16,13 +16,22 @@ struct Message;
 using MessagePtr = std::shared_ptr<const Message>;
 class LoggingChannel;
 
-class Logging
+class ILogging
+{
+public:
+    virtual ~ILogging() = default;
+
+    virtual void addMessage(const QString &channelName, MessagePtr message,
+                            const QString &platformName) = 0;
+};
+
+class Logging : public ILogging
 {
 public:
     Logging(Settings &settings);
 
     void addMessage(const QString &channelName, MessagePtr message,
-                    const QString &platformName);
+                    const QString &platformName) override;
 
 private:
     using PlatformName = QString;

src/widgets/settingspages/AboutPage.cpp

@@ -127,6 +127,9 @@ AboutPage::AboutPage()
             addLicense(form.getElement(), "Fluent icons",
                        "https://github.com/microsoft/fluentui-system-icons",
                        ":/licenses/fluenticons.txt");
+            addLicense(form.getElement(), "expected-lite",
+                       "https://github.com/martinmoene/expected-lite",
+                       ":/licenses/expected-lite.txt");
         }
 
         // Attributions

tests/CMakeLists.txt

@@ -44,6 +44,7 @@ set(test_SOURCES
     ${CMAKE_CURRENT_LIST_DIR}/src/QMagicEnum.cpp
     ${CMAKE_CURRENT_LIST_DIR}/src/ModerationAction.cpp
     ${CMAKE_CURRENT_LIST_DIR}/src/Scrollbar.cpp
+    ${CMAKE_CURRENT_LIST_DIR}/src/Commands.cpp
     # Add your new file above this line!
     )