From 40ce2858233dee60d9496e11977d8d57c98dbf30 Mon Sep 17 00:00:00 2001 From: Rasmus Karlsson Date: Sat, 18 May 2024 11:25:32 +0200 Subject: [PATCH 01/17] feat: allow /ban to be used in multiple channels at once example: `/ban --channel 11148817 --channel 117166826 forsen game complainer` this would ban forsen with the reason `game complainer` in the two listed channels --- .../commands/builtin/twitch/Ban.cpp | 157 ++++++++++++------ .../commands/builtin/twitch/Ban.hpp | 8 + 2 files changed, 112 insertions(+), 53 deletions(-) diff --git a/src/controllers/commands/builtin/twitch/Ban.cpp b/src/controllers/commands/builtin/twitch/Ban.cpp index 27b3d5a462d..b5921bd5a10 100644 --- a/src/controllers/commands/builtin/twitch/Ban.cpp +++ b/src/controllers/commands/builtin/twitch/Ban.cpp @@ -9,6 +9,8 @@ #include "providers/twitch/TwitchChannel.hpp" #include "util/Twitch.hpp" +#include + namespace { using namespace chatterino; @@ -80,13 +82,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, - reason, + channelID, sourceUserID, targetUserID, std::nullopt, reason, [] { // No response for bans, they're emitted over pubsub/IRC instead }, @@ -97,14 +98,13 @@ void banUserByID(const ChannelPtr &channel, const TwitchChannel *twitchChannel, }); } -void timeoutUserByID(const ChannelPtr &channel, - const TwitchChannel *twitchChannel, +void timeoutUserByID(const ChannelPtr &channel, const QString &channelID, 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 }, @@ -119,65 +119,115 @@ void timeoutUserByID(const ChannelPtr &channel, namespace chatterino::commands { -QString sendBan(const CommandContext &ctx) +std::vector parseBanCommand(const CommandContext &ctx) { - const auto &words = ctx.words; - const auto &channel = ctx.channel; - const auto *twitchChannel = ctx.twitchChannel; + if (ctx.channel == nullptr) + { + // A ban action must be performed with a channel as a context + return {}; + } - if (channel == nullptr) + QCommandLineParser parser; + parser.setOptionsAfterPositionalArgumentsMode( + QCommandLineParser::ParseAsPositionalArguments); + parser.addPositionalArgument("username", "The name of the user to ban"); + parser.addPositionalArgument("reason", "The optional ban reason"); + QCommandLineOption channelOption( + "channel", "Override which channel(s) to perform the action in", + "channel id"); + parser.addOptions({ + channelOption, + }); + parser.parse(ctx.words); + + const auto &positionalArguments = parser.positionalArguments(); + if (positionalArguments.isEmpty()) { - return ""; + return {}; } - if (twitchChannel == nullptr) + const auto &rawTarget = positionalArguments.first(); + const auto reason = positionalArguments.sliced(1).join(' '); + + auto overrideChannels = parser.values(channelOption); + if (overrideChannels.isEmpty()) { - channel->addMessage(makeSystemMessage( - QString("The /ban command only works in Twitch channels."))); - return ""; + if (ctx.twitchChannel == nullptr) + { + // No override channel specified, but the command was not run in a Twitch channel + return {}; + } + + return { + PerformChannelAction{ + .channelID = ctx.twitchChannel->roomId(), + .rawTarget = rawTarget, + .reason = reason, + }, + }; } - const auto *usageStr = - "Usage: \"/ban [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) + std::vector actions; + + for (const auto &overrideChannel : overrideChannels) { - channel->addMessage(makeSystemMessage(usageStr)); + actions.push_back(PerformChannelAction{ + .channelID = overrideChannel, + .rawTarget = rawTarget, + .reason = reason, + }); + } + + return actions; +} + +QString sendBan(const CommandContext &ctx) +{ + const auto actions = parseBanCommand(ctx); + + if (actions.empty()) + { + // TODO: better error message 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); - auto [targetUserName, targetUserID] = parseUserNameOrID(rawTarget); - auto reason = words.mid(2).join(' '); - - if (!targetUserID.isEmpty()) - { - banUserByID(channel, twitchChannel, currentUser->getUserId(), - targetUserID, reason, targetUserID); - } - else + for (const auto &action : actions) { - getHelix()->getUserByName( - targetUserName, - [channel, currentUser, twitchChannel, - reason](const auto &targetUser) { - banUserByID(channel, twitchChannel, currentUser->getUserId(), - targetUser.id, reason, targetUser.displayName); - }, - [channel, targetUserName{targetUserName}] { - // Equivalent error from IRC - channel->addMessage(makeSystemMessage( - QString("Invalid username: %1").arg(targetUserName))); - }); + const auto &reason = action.reason; + qInfo() << "banning" << action.rawTarget << "in" << action.channelID + << "with reason" << action.reason; + auto [targetUserName, targetUserID] = + parseUserNameOrID(action.rawTarget); + + if (!targetUserID.isEmpty()) + { + banUserByID(ctx.channel, action.channelID, currentUser->getUserId(), + targetUserID, reason, targetUserID); + } + else + { + getHelix()->getUserByName( + targetUserName, + [channel{ctx.channel}, channelID{action.channelID}, currentUser, + twitchChannel{ctx.twitchChannel}, + reason](const auto &targetUser) { + banUserByID(channel, channelID, currentUser->getUserId(), + targetUser.id, reason, targetUser.displayName); + }, + [channel{ctx.channel}, targetUserName{targetUserName}] { + // Equivalent error from IRC + channel->addMessage(makeSystemMessage( + QString("Invalid username: %1").arg(targetUserName))); + }); + } } return ""; @@ -221,8 +271,8 @@ QString sendBanById(const CommandContext &ctx) auto target = words.at(1); auto reason = words.mid(2).join(' '); - banUserByID(channel, twitchChannel, currentUser->getUserId(), target, - reason, target); + banUserByID(channel, twitchChannel->roomId(), currentUser->getUserId(), + target, reason, target); return ""; } @@ -283,19 +333,20 @@ QString sendTimeout(const CommandContext &ctx) if (!targetUserID.isEmpty()) { - timeoutUserByID(channel, twitchChannel, currentUser->getUserId(), - targetUserID, duration, reason, targetUserID); + timeoutUserByID(channel, twitchChannel->roomId(), + currentUser->getUserId(), targetUserID, duration, + reason, targetUserID); } else { getHelix()->getUserByName( targetUserName, - [channel, currentUser, twitchChannel, + [channel, currentUser, channelID{twitchChannel->roomId()}, targetUserName{targetUserName}, duration, reason](const auto &targetUser) { - timeoutUserByID(channel, twitchChannel, - currentUser->getUserId(), targetUser.id, - duration, reason, targetUser.displayName); + timeoutUserByID(channel, channelID, currentUser->getUserId(), + targetUser.id, duration, reason, + targetUser.displayName); }, [channel, targetUserName{targetUserName}] { // Equivalent error from IRC diff --git a/src/controllers/commands/builtin/twitch/Ban.hpp b/src/controllers/commands/builtin/twitch/Ban.hpp index 9ba72491087..ac373f0f27b 100644 --- a/src/controllers/commands/builtin/twitch/Ban.hpp +++ b/src/controllers/commands/builtin/twitch/Ban.hpp @@ -10,6 +10,14 @@ struct CommandContext; namespace chatterino::commands { +struct PerformChannelAction { + QString channelID; + QString rawTarget; + QString reason; +}; + +std::vector parseBanCommand(const CommandContext &ctx); + /// /ban QString sendBan(const CommandContext &ctx); /// /banid From 422c6ea5c81015355b003cb843955aa3e3aa5c73 Mon Sep 17 00:00:00 2001 From: Rasmus Karlsson Date: Sun, 19 May 2024 14:24:09 +0200 Subject: [PATCH 02/17] progress --- src/common/QLogging.cpp | 1 + src/common/QLogging.hpp | 5 +- .../commands/builtin/twitch/Ban.cpp | 257 ++++++++++-------- .../commands/builtin/twitch/Ban.hpp | 7 +- src/providers/twitch/TwitchChannel.cpp | 12 +- tests/CMakeLists.txt | 1 + tests/src/Commands.cpp | 50 ++++ 7 files changed, 211 insertions(+), 122 deletions(-) create mode 100644 tests/src/Commands.cpp diff --git a/src/common/QLogging.cpp b/src/common/QLogging.cpp index de4ef056c0c..a8cd8285d55 100644 --- a/src/common/QLogging.cpp +++ b/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); diff --git a/src/common/QLogging.hpp b/src/common/QLogging.hpp index 36daa0e1e92..d509354d476 100644 --- a/src/common/QLogging.hpp +++ b/src/common/QLogging.hpp @@ -7,16 +7,18 @@ 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); Q_DECLARE_LOGGING_CATEGORY(chatterinoEnv); Q_DECLARE_LOGGING_CATEGORY(chatterinoFfzemotes); +Q_DECLARE_LOGGING_CATEGORY(chatterinoHTTP); Q_DECLARE_LOGGING_CATEGORY(chatterinoHelper); 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); diff --git a/src/controllers/commands/builtin/twitch/Ban.cpp b/src/controllers/commands/builtin/twitch/Ban.cpp index b5921bd5a10..096210f3381 100644 --- a/src/controllers/commands/builtin/twitch/Ban.cpp +++ b/src/controllers/commands/builtin/twitch/Ban.cpp @@ -1,6 +1,7 @@ #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 "messages/MessageBuilder.hpp" @@ -10,6 +11,7 @@ #include "util/Twitch.hpp" #include +#include namespace { @@ -119,18 +121,26 @@ void timeoutUserByID(const ChannelPtr &channel, const QString &channelID, namespace chatterino::commands { -std::vector parseBanCommand(const CommandContext &ctx) +nonstd::expected, QString> parseChannelAction( + const CommandContext &ctx, const QString &command, const QString &usage, + bool withDuration) { if (ctx.channel == nullptr) { // A ban action must be performed with a channel as a context - return {}; + 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"); + } parser.addPositionalArgument("reason", "The optional ban reason"); QCommandLineOption channelOption( "channel", "Override which channel(s) to perform the action in", @@ -143,54 +153,94 @@ std::vector parseBanCommand(const CommandContext &ctx) const auto &positionalArguments = parser.positionalArguments(); if (positionalArguments.isEmpty()) { - return {}; + return nonstd::make_unexpected("Missing target - " % usage); } - const auto &rawTarget = positionalArguments.first(); - const auto reason = positionalArguments.sliced(1).join(' '); + PerformChannelAction base{ + .rawTarget = positionalArguments.first(), + .duration = 0, + }; - auto overrideChannels = parser.values(channelOption); - if (overrideChannels.isEmpty()) + if (withDuration) { - if (ctx.twitchChannel == nullptr) + auto durationStr = positionalArguments.value(1); + if (durationStr.isEmpty()) { - // No override channel specified, but the command was not run in a Twitch channel - return {}; + base.duration = 10 * 60; // 10 min } - - return { - PerformChannelAction{ - .channelID = ctx.twitchChannel->roomId(), - .rawTarget = rawTarget, - .reason = reason, - }, - }; + else + { + base.duration = (int)parseDurationToSeconds(durationStr); + if (base.duration <= 0) + { + return nonstd::make_unexpected("Invalid duration - " % usage); + } + } + base.reason = positionalArguments.sliced(2).join(' '); + } + else + { + base.reason = positionalArguments.sliced(1).join(' '); } std::vector actions; - for (const auto &overrideChannel : overrideChannels) + 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{ - .channelID = overrideChannel, - .rawTarget = rawTarget, - .reason = reason, + .channelID = ctx.twitchChannel->roomId(), + .rawTarget = base.rawTarget, + .reason = base.reason, + .duration = base.duration, }); } + else + { + for (const auto &overrideChannel : overrideChannels) + { + actions.push_back(PerformChannelAction{ + .channelID = overrideChannel, + .rawTarget = base.rawTarget, + .reason = base.reason, + .duration = base.duration, + }); + } + } return actions; } QString sendBan(const CommandContext &ctx) { - const auto actions = parseBanCommand(ctx); + const auto command = QStringLiteral("/ban"); + const auto usage = QStringLiteral( + R"(Usage: "/ban [options...] [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 to override which channel the ban takes place in (can be specified multiple times).)"); + const auto actions = parseChannelAction(ctx, command, usage, false); - if (actions.empty()) + if (!actions.has_value()) { - // TODO: better error message + if (ctx.channel != nullptr) + { + ctx.channel->addMessage(makeSystemMessage(actions.error())); + } + else + { + qCWarning(chatterinoCommands) + << "Error parsing command:" << actions.error(); + } + return ""; } + assert(!actions.value().empty()); + auto currentUser = getIApp()->getAccounts()->twitch.getCurrent(); if (currentUser->isAnon()) { @@ -199,11 +249,9 @@ QString sendBan(const CommandContext &ctx) return ""; } - for (const auto &action : actions) + for (const auto &action : actions.value()) { const auto &reason = action.reason; - qInfo() << "banning" << action.rawTarget << "in" << action.channelID - << "with reason" << action.reason; auto [targetUserName, targetUserID] = parseUserNameOrID(action.rawTarget); @@ -217,7 +265,6 @@ QString sendBan(const CommandContext &ctx) getHelix()->getUserByName( targetUserName, [channel{ctx.channel}, channelID{action.channelID}, currentUser, - twitchChannel{ctx.twitchChannel}, reason](const auto &targetUser) { banUserByID(channel, channelID, currentUser->getUserId(), targetUser.id, reason, targetUser.displayName); @@ -235,124 +282,108 @@ QString sendBan(const CommandContext &ctx) QString sendBanById(const CommandContext &ctx) { - const auto &words = ctx.words; - const auto &channel = ctx.channel; - const auto *twitchChannel = ctx.twitchChannel; + const auto command = QStringLiteral("/banid"); + const auto usage = QStringLiteral( + R"(Usage: "/banid [options...] [reason]" - Permanently prevent a user from chatting via their user ID. Reason is optional and will be shown to the target user and other moderators. Options: --channel to override which channel the ban takes place in (can be specified multiple times).)"); + const auto actions = parseChannelAction(ctx, command, usage, false); - if (channel == nullptr) - { - return ""; - } - if (twitchChannel == nullptr) + if (!actions.has_value()) { - channel->addMessage(makeSystemMessage( - QString("The /banid command only works in Twitch channels."))); - return ""; - } + if (ctx.channel != nullptr) + { + ctx.channel->addMessage(makeSystemMessage(actions.error())); + } + else + { + qCWarning(chatterinoCommands) + << "Error parsing command:" << actions.error(); + } - const auto *usageStr = - "Usage: \"/banid [reason]\" - Permanently prevent a user " - "from chatting via their user ID. Reason is optional and will be " - "shown to the target user and other moderators."; - if (words.size() < 2) - { - channel->addMessage(makeSystemMessage(usageStr)); return ""; } + assert(!actions.value().empty()); + auto currentUser = getIApp()->getAccounts()->twitch.getCurrent(); if (currentUser->isAnon()) { - channel->addMessage( + ctx.channel->addMessage( makeSystemMessage("You must be logged in to ban someone!")); return ""; } - auto target = words.at(1); - auto reason = words.mid(2).join(' '); - - banUserByID(channel, twitchChannel->roomId(), currentUser->getUserId(), - target, reason, target); + for (const auto &action : actions.value()) + { + const auto &reason = action.reason; + const auto &targetUserID = action.rawTarget; + banUserByID(ctx.channel, action.channelID, currentUser->getUserId(), + targetUserID, reason, targetUserID); + } return ""; } QString sendTimeout(const CommandContext &ctx) { - const auto &words = ctx.words; - const auto &channel = ctx.channel; - const auto *twitchChannel = ctx.twitchChannel; + const auto command = QStringLiteral("/timeout"); + const auto usage = QStringLiteral( + R"(Usage: "/timeout [options...] [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 to override which channel the ban takes place in (can be specified multiple times).)"); + const auto actions = parseChannelAction(ctx, command, usage, true); - if (channel == nullptr) + if (!actions.has_value()) { - return ""; - } + if (ctx.channel != nullptr) + { + ctx.channel->addMessage(makeSystemMessage(actions.error())); + } + else + { + qCWarning(chatterinoCommands) + << "Error parsing command:" << actions.error(); + } - if (twitchChannel == nullptr) - { - channel->addMessage(makeSystemMessage( - QString("The /timeout command only works in Twitch channels."))); - return ""; - } - const auto *usageStr = - "Usage: \"/timeout [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 ""; } + assert(!actions.value().empty()); + auto currentUser = getIApp()->getAccounts()->twitch.getCurrent(); if (currentUser->isAnon()) { - channel->addMessage( - makeSystemMessage("You must be logged in to timeout someone!")); + ctx.channel->addMessage( + makeSystemMessage("You must be logged in to ban someone!")); return ""; } - const auto &rawTarget = words.at(1); - auto [targetUserName, targetUserID] = parseUserNameOrID(rawTarget); - - int duration = 10 * 60; // 10min - if (words.size() >= 3) + for (const auto &action : actions.value()) { - duration = (int)parseDurationToSeconds(words.at(2)); - if (duration <= 0) + const auto &reason = action.reason; + const auto duration = action.duration; + auto [targetUserName, targetUserID] = + parseUserNameOrID(action.rawTarget); + + if (!targetUserID.isEmpty()) { - channel->addMessage(makeSystemMessage(usageStr)); - return ""; + timeoutUserByID(ctx.channel, action.channelID, + currentUser->getUserId(), targetUserID, duration, + reason, targetUserID); + } + else + { + getHelix()->getUserByName( + targetUserName, + [channel{ctx.channel}, channelID{action.channelID}, currentUser, + duration, reason](const auto &targetUser) { + timeoutUserByID(channel, channelID, + currentUser->getUserId(), targetUser.id, + duration, reason, targetUser.displayName); + }, + [channel{ctx.channel}, targetUserName{targetUserName}] { + // Equivalent error from IRC + channel->addMessage(makeSystemMessage( + QString("Invalid username: %1").arg(targetUserName))); + }); } - } - auto reason = words.mid(3).join(' '); - - if (!targetUserID.isEmpty()) - { - timeoutUserByID(channel, twitchChannel->roomId(), - currentUser->getUserId(), targetUserID, duration, - reason, targetUserID); - } - else - { - getHelix()->getUserByName( - targetUserName, - [channel, currentUser, channelID{twitchChannel->roomId()}, - targetUserName{targetUserName}, duration, - reason](const auto &targetUser) { - timeoutUserByID(channel, channelID, currentUser->getUserId(), - targetUser.id, duration, reason, - targetUser.displayName); - }, - [channel, targetUserName{targetUserName}] { - // Equivalent error from IRC - channel->addMessage(makeSystemMessage( - QString("Invalid username: %1").arg(targetUserName))); - }); } return ""; diff --git a/src/controllers/commands/builtin/twitch/Ban.hpp b/src/controllers/commands/builtin/twitch/Ban.hpp index ac373f0f27b..81c87d77d86 100644 --- a/src/controllers/commands/builtin/twitch/Ban.hpp +++ b/src/controllers/commands/builtin/twitch/Ban.hpp @@ -1,5 +1,7 @@ #pragma once +#include "expected.hpp" + #include namespace chatterino { @@ -14,9 +16,12 @@ struct PerformChannelAction { QString channelID; QString rawTarget; QString reason; + int duration; }; -std::vector parseBanCommand(const CommandContext &ctx); +nonstd::expected, QString> parseChannelAction( + const CommandContext &ctx, const QString &command, const QString &usage, + bool withDuration); /// /ban QString sendBan(const CommandContext &ctx); diff --git a/src/providers/twitch/TwitchChannel.cpp b/src/providers/twitch/TwitchChannel.cpp index 4339d6c2ec6..caac5dd185d 100644 --- a/src/providers/twitch/TwitchChannel.cpp +++ b/src/providers/twitch/TwitchChannel.cpp @@ -91,12 +91,12 @@ 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; - } + // 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] { diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 8288664dfd8..19ac9195b31 100644 --- a/tests/CMakeLists.txt +++ b/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! ) diff --git a/tests/src/Commands.cpp b/tests/src/Commands.cpp new file mode 100644 index 00000000000..2006b831b98 --- /dev/null +++ b/tests/src/Commands.cpp @@ -0,0 +1,50 @@ +#include "controllers/accounts/AccountController.hpp" +#include "controllers/commands/builtin/twitch/Ban.hpp" +#include "controllers/commands/CommandContext.hpp" +#include "mocks/Channel.hpp" +#include "mocks/EmptyApplication.hpp" +#include "providers/twitch/TwitchChannel.hpp" +#include "singletons/Settings.hpp" +#include "Test.hpp" + +using namespace chatterino; + +namespace { + +class MockApplication : mock::EmptyApplication +{ +public: + MockApplication() + : settings(this->settingsDir.filePath("settings.json")) + { + } + + AccountController *getAccounts() override + { + return &this->accounts; + } + + Settings settings; + AccountController accounts; +}; + +} // namespace + +TEST(Commands, parseBanCommand) +{ + MockApplication app; + + std::shared_ptr channel = + std::make_shared("forsen"); + CommandContext ctx{ + .words = {"/ban", "forsen"}, + .channel = channel, + .twitchChannel = channel.get(), + }; + auto oActions = commands::parseChannelAction(ctx, "/ban", "usage", false); + + ASSERT_TRUE(oActions.has_value()) << oActions.error(); + auto actions = *oActions; + ASSERT_EQ(actions.size(), 1); + ASSERT_EQ(actions.front().rawTarget, "forsen"); +} From f96aa26a2a3469876b98605b93ef5b7d5fc66a6d Mon Sep 17 00:00:00 2001 From: Rasmus Karlsson Date: Sat, 25 May 2024 14:29:28 +0200 Subject: [PATCH 03/17] Add expected-lite dependency as a submodule https://github.com/martinmoene/expected-lite --- .gitmodules | 3 +++ lib/expected-lite | 1 + src/CMakeLists.txt | 3 +++ src/widgets/settingspages/AboutPage.cpp | 3 +++ 4 files changed, 10 insertions(+) create mode 160000 lib/expected-lite diff --git a/.gitmodules b/.gitmodules index cb1235a8582..e58a5bbd49b 100644 --- a/.gitmodules +++ b/.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 diff --git a/lib/expected-lite b/lib/expected-lite new file mode 160000 index 00000000000..3634b0a6d8d --- /dev/null +++ b/lib/expected-lite @@ -0,0 +1 @@ +Subproject commit 3634b0a6d8dffcffad4d1355253d79290c0c754c diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 15806aae102..bd6a7b7225e 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -1002,6 +1002,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") diff --git a/src/widgets/settingspages/AboutPage.cpp b/src/widgets/settingspages/AboutPage.cpp index 89c985c5e8d..bc503ca6107 100644 --- a/src/widgets/settingspages/AboutPage.cpp +++ b/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 From e03c5e8cb5d136a0d04b7e60dd05784e7ed7a9ec Mon Sep 17 00:00:00 2001 From: Rasmus Karlsson Date: Sat, 25 May 2024 14:31:12 +0200 Subject: [PATCH 04/17] fix include path for real expected dependency --- src/controllers/commands/builtin/twitch/Ban.hpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/controllers/commands/builtin/twitch/Ban.hpp b/src/controllers/commands/builtin/twitch/Ban.hpp index 81c87d77d86..ebe0f638689 100644 --- a/src/controllers/commands/builtin/twitch/Ban.hpp +++ b/src/controllers/commands/builtin/twitch/Ban.hpp @@ -1,7 +1,6 @@ #pragma once -#include "expected.hpp" - +#include #include namespace chatterino { From f917a076efaaea56c09e4ac5cca62e0cab303070 Mon Sep 17 00:00:00 2001 From: Rasmus Karlsson Date: Sun, 2 Jun 2024 14:23:23 +0200 Subject: [PATCH 05/17] add expected-lite license, and fix it to work on /ban, /timeout, and /unban --- resources/licenses/expected-lite.txt | 23 + src/CMakeLists.txt | 2 + .../commands/builtin/twitch/Ban.cpp | 310 ++++--- .../commands/builtin/twitch/Ban.hpp | 12 - .../commands/builtin/twitch/Unban.cpp | 140 +++- .../commands/common/ChannelAction.cpp | 178 ++++ .../commands/common/ChannelAction.hpp | 55 ++ src/providers/twitch/TwitchChannel.cpp | 7 - tests/src/Commands.cpp | 774 +++++++++++++++++- 9 files changed, 1267 insertions(+), 234 deletions(-) create mode 100644 resources/licenses/expected-lite.txt create mode 100644 src/controllers/commands/common/ChannelAction.cpp create mode 100644 src/controllers/commands/common/ChannelAction.hpp diff --git a/resources/licenses/expected-lite.txt b/resources/licenses/expected-lite.txt new file mode 100644 index 00000000000..36b7cd93cdf --- /dev/null +++ b/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. diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index bd6a7b7225e..21229488922 100644 --- a/src/CMakeLists.txt +++ b/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 diff --git a/src/controllers/commands/builtin/twitch/Ban.cpp b/src/controllers/commands/builtin/twitch/Ban.cpp index 096210f3381..8effa08dda9 100644 --- a/src/controllers/commands/builtin/twitch/Ban.cpp +++ b/src/controllers/commands/builtin/twitch/Ban.cpp @@ -4,14 +4,11 @@ #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" - -#include -#include namespace { @@ -121,108 +118,12 @@ void timeoutUserByID(const ChannelPtr &channel, const QString &channelID, namespace chatterino::commands { -nonstd::expected, QString> parseChannelAction( - const CommandContext &ctx, const QString &command, const QString &usage, - bool withDuration) -{ - 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"); - } - parser.addPositionalArgument("reason", "The optional ban reason"); - QCommandLineOption channelOption( - "channel", "Override which channel(s) to perform the action in", - "channel id"); - parser.addOptions({ - channelOption, - }); - parser.parse(ctx.words); - - const auto &positionalArguments = parser.positionalArguments(); - if (positionalArguments.isEmpty()) - { - return nonstd::make_unexpected("Missing target - " % usage); - } - - PerformChannelAction base{ - .rawTarget = positionalArguments.first(), - .duration = 0, - }; - - if (withDuration) - { - auto durationStr = positionalArguments.value(1); - if (durationStr.isEmpty()) - { - base.duration = 10 * 60; // 10 min - } - else - { - base.duration = (int)parseDurationToSeconds(durationStr); - if (base.duration <= 0) - { - return nonstd::make_unexpected("Invalid duration - " % usage); - } - } - base.reason = positionalArguments.sliced(2).join(' '); - } - else - { - base.reason = positionalArguments.sliced(1).join(' '); - } - - std::vector 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{ - .channelID = ctx.twitchChannel->roomId(), - .rawTarget = base.rawTarget, - .reason = base.reason, - .duration = base.duration, - }); - } - else - { - for (const auto &overrideChannel : overrideChannels) - { - actions.push_back(PerformChannelAction{ - .channelID = overrideChannel, - .rawTarget = base.rawTarget, - .reason = base.reason, - .duration = base.duration, - }); - } - } - - return actions; -} - QString sendBan(const CommandContext &ctx) { const auto command = QStringLiteral("/ban"); const auto usage = QStringLiteral( - R"(Usage: "/ban [options...] [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 to override which channel the ban takes place in (can be specified multiple times).)"); - const auto actions = parseChannelAction(ctx, command, usage, false); + R"(Usage: "/ban [options...] [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 to override which channel the ban takes place in (can be specified multiple times).)"); + const auto actions = parseChannelAction(ctx, command, usage, false, true); if (!actions.has_value()) { @@ -252,29 +153,75 @@ QString sendBan(const CommandContext &ctx) for (const auto &action : actions.value()) { const auto &reason = action.reason; - auto [targetUserName, targetUserID] = - parseUserNameOrID(action.rawTarget); - if (!targetUserID.isEmpty()) + QStringList userLoginsToFetch; + QStringList userIDs; + if (action.target.id.isEmpty()) + { + assert(!action.target.login.isEmpty() && + "Ban Action target username AND user ID may not be " + "empty at the same time"); + userLoginsToFetch.append(action.target.login); + } + else { - banUserByID(ctx.channel, action.channelID, currentUser->getUserId(), - targetUserID, reason, targetUserID); + // For hydration + userIDs.append(action.target.id); + } + if (action.channel.id.isEmpty()) + { + assert(!action.channel.login.isEmpty() && + "Ban Action channel username AND user ID may not be " + "empty at the same time"); + userLoginsToFetch.append(action.channel.login); } else { - getHelix()->getUserByName( - targetUserName, - [channel{ctx.channel}, channelID{action.channelID}, currentUser, - reason](const auto &targetUser) { - banUserByID(channel, channelID, currentUser->getUserId(), - targetUser.id, reason, targetUser.displayName); + // 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}, targetUserName{targetUserName}] { - // Equivalent error from IRC + [channel{ctx.channel}, userLoginsToFetch] { channel->addMessage(makeSystemMessage( - QString("Invalid username: %1").arg(targetUserName))); + 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 ""; @@ -282,43 +229,44 @@ QString sendBan(const CommandContext &ctx) QString sendBanById(const CommandContext &ctx) { - const auto command = QStringLiteral("/banid"); - const auto usage = QStringLiteral( - R"(Usage: "/banid [options...] [reason]" - Permanently prevent a user from chatting via their user ID. Reason is optional and will be shown to the target user and other moderators. Options: --channel to override which channel the ban takes place in (can be specified multiple times).)"); - const auto actions = parseChannelAction(ctx, command, usage, false); + const auto &words = ctx.words; + const auto &channel = ctx.channel; + const auto *twitchChannel = ctx.twitchChannel; - if (!actions.has_value()) + if (channel == nullptr) { - if (ctx.channel != nullptr) - { - ctx.channel->addMessage(makeSystemMessage(actions.error())); - } - else - { - qCWarning(chatterinoCommands) - << "Error parsing command:" << actions.error(); - } - + return ""; + } + if (twitchChannel == nullptr) + { + channel->addMessage(makeSystemMessage( + QString("The /banid command only works in Twitch channels."))); return ""; } - assert(!actions.value().empty()); + const auto *usageStr = + "Usage: \"/banid [reason]\" - Permanently prevent a user " + "from chatting via their user ID. Reason is optional and will be " + "shown to the target user and other moderators."; + if (words.size() < 2) + { + channel->addMessage(makeSystemMessage(usageStr)); + return ""; + } auto currentUser = getIApp()->getAccounts()->twitch.getCurrent(); if (currentUser->isAnon()) { - ctx.channel->addMessage( + channel->addMessage( makeSystemMessage("You must be logged in to ban someone!")); return ""; } - for (const auto &action : actions.value()) - { - const auto &reason = action.reason; - const auto &targetUserID = action.rawTarget; - banUserByID(ctx.channel, action.channelID, currentUser->getUserId(), - targetUserID, reason, targetUserID); - } + auto target = words.at(1); + auto reason = words.mid(2).join(' '); + + banUserByID(channel, twitchChannel->roomId(), currentUser->getUserId(), + target, reason, target); return ""; } @@ -327,8 +275,8 @@ QString sendTimeout(const CommandContext &ctx) { const auto command = QStringLiteral("/timeout"); const auto usage = QStringLiteral( - R"(Usage: "/timeout [options...] [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 to override which channel the ban takes place in (can be specified multiple times).)"); - const auto actions = parseChannelAction(ctx, command, usage, true); + R"(Usage: "/timeout [options...] [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 to override which channel the unban takes place in (can be specified multiple times).)"); + const auto actions = parseChannelAction(ctx, command, usage, true, true); if (!actions.has_value()) { @@ -358,32 +306,76 @@ QString sendTimeout(const CommandContext &ctx) for (const auto &action : actions.value()) { const auto &reason = action.reason; - const auto duration = action.duration; - auto [targetUserName, targetUserID] = - parseUserNameOrID(action.rawTarget); - if (!targetUserID.isEmpty()) + QStringList userLoginsToFetch; + QStringList userIDs; + if (action.target.id.isEmpty()) + { + assert(!action.target.login.isEmpty() && + "Timeout Action target username AND user ID may not be " + "empty at the same time"); + userLoginsToFetch.append(action.target.login); + } + else { - timeoutUserByID(ctx.channel, action.channelID, - currentUser->getUserId(), targetUserID, duration, - reason, targetUserID); + // 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 { - getHelix()->getUserByName( - targetUserName, - [channel{ctx.channel}, channelID{action.channelID}, currentUser, - duration, reason](const auto &targetUser) { - timeoutUserByID(channel, channelID, - currentUser->getUserId(), targetUser.id, - duration, reason, targetUser.displayName); + // 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}, duration{action.duration}, + 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; + } + + timeoutUserByID(channel, actionChannel.id, + currentUser->getUserId(), actionTarget.id, + duration, reason, actionTarget.displayName); }, - [channel{ctx.channel}, targetUserName{targetUserName}] { - // Equivalent error from IRC + [channel{ctx.channel}, userLoginsToFetch] { channel->addMessage(makeSystemMessage( - QString("Invalid username: %1").arg(targetUserName))); + 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 ""; diff --git a/src/controllers/commands/builtin/twitch/Ban.hpp b/src/controllers/commands/builtin/twitch/Ban.hpp index ebe0f638689..9ba72491087 100644 --- a/src/controllers/commands/builtin/twitch/Ban.hpp +++ b/src/controllers/commands/builtin/twitch/Ban.hpp @@ -1,6 +1,5 @@ #pragma once -#include #include namespace chatterino { @@ -11,17 +10,6 @@ struct CommandContext; namespace chatterino::commands { -struct PerformChannelAction { - QString channelID; - QString rawTarget; - QString reason; - int duration; -}; - -nonstd::expected, QString> parseChannelAction( - const CommandContext &ctx, const QString &command, const QString &usage, - bool withDuration); - /// /ban QString sendBan(const CommandContext &ctx); /// /banid diff --git a/src/controllers/commands/builtin/twitch/Unban.cpp b/src/controllers/commands/builtin/twitch/Unban.cpp index e88008e8427..20f9282bc5f 100644 --- a/src/controllers/commands/builtin/twitch/Unban.cpp +++ b/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, - const TwitchChannel *twitchChannel, +void unbanUserByID(const ChannelPtr &channel, const QString &channelID, 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,27 +86,29 @@ 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 - Removes a ban on a user. Options: --channel to override which channel the timeout takes place in (can be specified multiple times).)") + .arg(command); + const auto actions = parseChannelAction(ctx, command, usage, false, false); + if (!actions.has_value()) { - return ""; - } + if (ctx.channel != nullptr) + { + ctx.channel->addMessage(makeSystemMessage(actions.error())); + } + else + { + qCWarning(chatterinoCommands) + << "Error parsing command:" << actions.error(); + } - auto commandName = ctx.words.at(0).toLower(); - 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 \" - Removes a ban on a user.") - .arg(commandName))); return ""; } + assert(!actions.value().empty()); + 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); - auto [targetUserName, targetUserID] = parseUserNameOrID(rawTarget); - - if (!targetUserID.isEmpty()) - { - unbanUserByID(ctx.channel, ctx.twitchChannel, currentUser->getUserId(), - targetUserID, targetUserID); - } - else + for (const auto &action : actions.value()) { - getHelix()->getUserByName( - targetUserName, - [channel{ctx.channel}, currentUser, - twitchChannel{ctx.twitchChannel}, - targetUserName{targetUserName}](const auto &targetUser) { - unbanUserByID(channel, twitchChannel, currentUser->getUserId(), - targetUser.id, targetUser.displayName); - }, - [channel{ctx.channel}, targetUserName{targetUserName}] { - // Equivalent error from IRC - channel->addMessage(makeSystemMessage( - QString("Invalid username: %1").arg(targetUserName))); - }); + const auto &reason = action.reason; + + QStringList userLoginsToFetch; + QStringList userIDs; + if (action.target.id.isEmpty()) + { + assert(!action.target.login.isEmpty() && + "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); + } + + 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 ""; diff --git a/src/controllers/commands/common/ChannelAction.cpp b/src/controllers/commands/common/ChannelAction.cpp new file mode 100644 index 00000000000..af816238ab7 --- /dev/null +++ b/src/controllers/commands/common/ChannelAction.cpp @@ -0,0 +1,178 @@ +#include "controllers/commands/common/ChannelAction.hpp" + +#include "controllers/commands/CommandContext.hpp" +#include "providers/twitch/api/Helix.hpp" +#include "providers/twitch/TwitchChannel.hpp" +#include "util/Twitch.hpp" + +#include +#include + +namespace chatterino::commands { + +bool IncompleteHelixUser::hydrateFrom(const std::vector &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, 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 id"); + parser.addOptions({ + channelOption, + }); + parser.parse(ctx.words); + + const auto &positionalArguments = parser.positionalArguments(); + if (positionalArguments.isEmpty()) + { + return nonstd::make_unexpected("Missing target - " % usage); + } + + auto [targetUserName, targetUserID] = + parseUserNameOrID(positionalArguments.first()); + + PerformChannelAction base{ + .target = + IncompleteHelixUser{ + .id = targetUserID, + .login = targetUserName, + .displayName = "", + }, + .duration = 0, + }; + + if (withDuration) + { + auto durationStr = positionalArguments.value(1); + if (durationStr.isEmpty()) + { + base.duration = 10 * 60; // 10 min + } + else + { + base.duration = (int)parseDurationToSeconds(durationStr); + if (base.duration <= 0) + { + return nonstd::make_unexpected("Invalid duration - " % usage); + } + if (withReason) + { + base.reason = positionalArguments.sliced(2).join(' '); + } + } + } + else + { + if (withReason) + { + base.reason = positionalArguments.sliced(1).join(' '); + } + } + + std::vector 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 diff --git a/src/controllers/commands/common/ChannelAction.hpp b/src/controllers/commands/common/ChannelAction.hpp new file mode 100644 index 00000000000..11f620f614e --- /dev/null +++ b/src/controllers/commands/common/ChannelAction.hpp @@ -0,0 +1,55 @@ +#pragma once + +#include +#include + +namespace chatterino { + +struct CommandContext; +struct HelixUser; + +} // namespace chatterino + +namespace chatterino::commands { + +struct IncompleteHelixUser { + QString id; + QString login; + QString displayName; + + bool hydrateFrom(const std::vector &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, QString> parseChannelAction( + const CommandContext &ctx, const QString &command, const QString &usage, + bool withDuration, bool withReason); + +} // namespace chatterino::commands diff --git a/src/providers/twitch/TwitchChannel.cpp b/src/providers/twitch/TwitchChannel.cpp index caac5dd185d..c86664b5ccf 100644 --- a/src/providers/twitch/TwitchChannel.cpp +++ b/src/providers/twitch/TwitchChannel.cpp @@ -231,13 +231,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_); diff --git a/tests/src/Commands.cpp b/tests/src/Commands.cpp index 2006b831b98..a0eb3cb1c01 100644 --- a/tests/src/Commands.cpp +++ b/tests/src/Commands.cpp @@ -1,12 +1,14 @@ #include "controllers/accounts/AccountController.hpp" -#include "controllers/commands/builtin/twitch/Ban.hpp" #include "controllers/commands/CommandContext.hpp" -#include "mocks/Channel.hpp" +#include "controllers/commands/common/ChannelAction.hpp" #include "mocks/EmptyApplication.hpp" +#include "mocks/TwitchIrcServer.hpp" #include "providers/twitch/TwitchChannel.hpp" #include "singletons/Settings.hpp" #include "Test.hpp" +#include + using namespace chatterino; namespace { @@ -19,6 +21,11 @@ class MockApplication : mock::EmptyApplication { } + ITwitchIrcServer *getTwitch() override + { + return &this->twitch; + } + AccountController *getAccounts() override { return &this->accounts; @@ -26,25 +33,768 @@ class MockApplication : mock::EmptyApplication Settings settings; AccountController accounts; + mock::MockTwitchIrcServer twitch; }; } // namespace -TEST(Commands, parseBanCommand) +TEST(Commands, parseBanActions) +{ + MockApplication app; + + std::shared_ptr channel = + std::make_shared("forsen"); + + CommandContext ctx{}; + + QString command("/ban"); + QString usage("usage string"); + bool withDuration = false; + bool withReason = true; + + struct Test { + CommandContext inputContext; + + std::vector expectedActions; + QString expectedError; + }; + + std::vector tests{ + { + // Normal ban with an added reason, with the user maybe trying to use the --channal parameter at the end, but it gets eaten by the reason + .inputContext = + { + .words = {"/ban", "forsen", "the", "ban", "reason", + "--channel", "xD"}, + .channel = channel, + .twitchChannel = channel.get(), + }, + .expectedActions = + { + { + .channel = + { + .login = "forsen", + .displayName = "forsen", + }, + .target = + { + .login = "forsen", + }, + .reason = "the ban reason --channel xD", + .duration = 0, + }, + }, + .expectedError = "", + }, + { + // Normal ban with an added reason + .inputContext = + { + .words = {"/ban", "forsen", "the", "ban", "reason"}, + .channel = channel, + .twitchChannel = channel.get(), + }, + .expectedActions = + { + { + .channel = + { + .login = "forsen", + .displayName = "forsen", + }, + .target = + { + .login = "forsen", + }, + .reason = "the ban reason", + .duration = 0, + }, + }, + .expectedError = "", + }, + { + // Normal ban without an added reason + .inputContext = + { + .words = {"/ban", "forsen"}, + .channel = channel, + .twitchChannel = channel.get(), + }, + .expectedActions = + { + { + .channel = + { + .login = "forsen", + .displayName = "forsen", + }, + .target = + { + .login = "forsen", + }, + .reason = "", + .duration = 0, + }, + }, + .expectedError = "", + }, + { + // User forgot to specify who to ban + .inputContext = + { + .words = {"/ban"}, + .channel = channel, + .twitchChannel = channel.get(), + }, + .expectedActions = {}, + .expectedError = "Missing target - " % usage, + }, + { + // User tried to use /ban outside of a channel context (shouldn't really be able to happen) + .inputContext = + { + .words = {"/ban"}, + }, + .expectedActions = {}, + .expectedError = + "A " % command % + " action must be performed with a channel as a context", + }, + { + // User tried to use /ban without a target, but with a --channel specified + .inputContext = + { + .words = {"/ban", "--channel", "pajlada"}, + .channel = channel, + .twitchChannel = channel.get(), + }, + .expectedActions = {}, + .expectedError = "Missing target - " % usage, + }, + { + // User overriding the ban to be done in the pajlada channel + .inputContext = + { + .words = {"/ban", "--channel", "pajlada", "forsen"}, + .channel = channel, + .twitchChannel = channel.get(), + }, + .expectedActions = + { + { + .channel = + { + .login = "pajlada", + }, + .target = + { + .login = "forsen", + }, + .reason = "", + .duration = 0, + }, + }, + .expectedError = "", + }, + { + // User overriding the ban to be done in the pajlada channel and in the channel with the id 11148817 + .inputContext = + { + .words = {"/ban", "--channel", "pajlada", "--channel", + "id:11148817", "forsen"}, + .channel = channel, + .twitchChannel = channel.get(), + }, + .expectedActions = + { + { + .channel = + { + .login = "pajlada", + }, + .target = + { + .login = "forsen", + }, + .reason = "", + .duration = 0, + }, + { + .channel = + { + .id = "11148817", + }, + .target = + { + .login = "forsen", + }, + .reason = "", + .duration = 0, + }, + }, + .expectedError = "", + }, + { + // User overriding the ban to be done in the pajlada channel and in the channel with the id 11148817, with a reason specified + .inputContext = + { + .words = {"/ban", "--channel", "pajlada", "--channel", + "id:11148817", "forsen", "the", "ban", "reason"}, + .channel = channel, + .twitchChannel = channel.get(), + }, + .expectedActions = + { + { + .channel = + { + .login = "pajlada", + }, + .target = + { + .login = "forsen", + }, + .reason = "the ban reason", + .duration = 0, + }, + { + .channel = + { + .id = "11148817", + }, + .target = + { + .login = "forsen", + }, + .reason = "the ban reason", + .duration = 0, + }, + }, + .expectedError = "", + }, + }; + + for (const auto &test : tests) + { + auto oActions = commands::parseChannelAction( + test.inputContext, command, usage, withDuration, withReason); + + if (!test.expectedActions.empty()) + { + ASSERT_TRUE(oActions.has_value()) << oActions.error(); + auto actions = *oActions; + ASSERT_EQ(actions.size(), test.expectedActions.size()); + ASSERT_EQ(actions, test.expectedActions); + } + else + { + ASSERT_FALSE(oActions.has_value()); + } + + if (!test.expectedError.isEmpty()) + { + ASSERT_FALSE(oActions.has_value()); + ASSERT_EQ(oActions.error(), test.expectedError); + } + } +} + +TEST(Commands, parseTimeoutActions) +{ + MockApplication app; + + std::shared_ptr channel = + std::make_shared("forsen"); + + CommandContext ctx{}; + + QString command("/timeout"); + QString usage("usage string"); + bool withDuration = true; + bool withReason = true; + + struct Test { + CommandContext inputContext; + + std::vector expectedActions; + QString expectedError; + }; + + std::vector tests{ + { + // Normal timeout without an added reason, with the default duration + .inputContext = + { + .words = {"/timeout", "forsen"}, + .channel = channel, + .twitchChannel = channel.get(), + }, + .expectedActions = + { + { + .channel = + { + .login = "forsen", + .displayName = "forsen", + }, + .target = + { + .login = "forsen", + }, + .reason = "", + .duration = 10 * 60, + }, + }, + .expectedError = "", + }, + { + // Normal timeout without an added reason, with a custom duration + .inputContext = + { + .words = {"/timeout", "forsen", "5m"}, + .channel = channel, + .twitchChannel = channel.get(), + }, + .expectedActions = + { + { + .channel = + { + .login = "forsen", + .displayName = "forsen", + }, + .target = + { + .login = "forsen", + }, + .reason = "", + .duration = 5 * 60, + }, + }, + .expectedError = "", + }, + { + // Normal timeout without an added reason, with a custom duration, with an added reason + .inputContext = + { + .words = {"/timeout", "forsen", "5m", "the", "timeout", + "reason"}, + .channel = channel, + .twitchChannel = channel.get(), + }, + .expectedActions = + { + { + .channel = + { + .login = "forsen", + .displayName = "forsen", + }, + .target = + { + .login = "forsen", + }, + .reason = "the timeout reason", + .duration = 5 * 60, + }, + }, + .expectedError = "", + }, + { + // Normal timeout without an added reason, with an added reason, but user forgot to specify a timeout duration so it fails + .inputContext = + { + .words = {"/timeout", "forsen", "the", "timeout", "reason"}, + .channel = channel, + .twitchChannel = channel.get(), + }, + .expectedActions = {}, + .expectedError = "Invalid duration - " % usage, + }, + { + // User forgot to specify who to timeout + .inputContext = + { + .words = {"/timeout"}, + .channel = channel, + .twitchChannel = channel.get(), + }, + .expectedActions = {}, + .expectedError = "Missing target - " % usage, + }, + { + // User tried to use /timeout outside of a channel context (shouldn't really be able to happen) + .inputContext = + { + .words = {"/timeout"}, + }, + .expectedActions = {}, + .expectedError = + "A " % command % + " action must be performed with a channel as a context", + }, + { + // User tried to use /timeout without a target, but with a --channel specified + .inputContext = + { + .words = {"/timeout", "--channel", "pajlada"}, + .channel = channel, + .twitchChannel = channel.get(), + }, + .expectedActions = {}, + .expectedError = "Missing target - " % usage, + }, + { + // User overriding the timeout to be done in the pajlada channel + .inputContext = + { + .words = {"/timeout", "--channel", "pajlada", "forsen"}, + .channel = channel, + .twitchChannel = channel.get(), + }, + .expectedActions = + { + { + .channel = + { + .login = "pajlada", + }, + .target = + { + .login = "forsen", + }, + .reason = "", + .duration = 10 * 60, + }, + }, + .expectedError = "", + }, + { + // User overriding the timeout to be done in the pajlada channel and in the channel with the id 11148817 + .inputContext = + { + .words = {"/timeout", "--channel", "pajlada", "--channel", + "id:11148817", "forsen"}, + .channel = channel, + .twitchChannel = channel.get(), + }, + .expectedActions = + { + { + .channel = + { + .login = "pajlada", + }, + .target = + { + .login = "forsen", + }, + .reason = "", + .duration = 10 * 60, + }, + { + .channel = + { + .id = "11148817", + }, + .target = + { + .login = "forsen", + }, + .reason = "", + .duration = 10 * 60, + }, + }, + .expectedError = "", + }, + { + // User overriding the timeout to be done in the pajlada channel and in the channel with the id 11148817, with a custom duration + .inputContext = + { + .words = {"/timeout", "--channel", "pajlada", "--channel", + "id:11148817", "forsen", "5m"}, + .channel = channel, + .twitchChannel = channel.get(), + }, + .expectedActions = + { + { + .channel = + { + .login = "pajlada", + }, + .target = + { + .login = "forsen", + }, + .reason = "", + .duration = 5 * 60, + }, + { + .channel = + { + .id = "11148817", + }, + .target = + { + .login = "forsen", + }, + .reason = "", + .duration = 5 * 60, + }, + }, + .expectedError = "", + }, + { + // User overriding the timeout to be done in the pajlada channel and in the channel with the id 11148817, with a reason specified + .inputContext = + { + .words = {"/timeout", "--channel", "pajlada", "--channel", + "id:11148817", "forsen", "10m", "the", "timeout", + "reason"}, + .channel = channel, + .twitchChannel = channel.get(), + }, + .expectedActions = + { + { + .channel = + { + .login = "pajlada", + }, + .target = + { + .login = "forsen", + }, + .reason = "the timeout reason", + .duration = 10 * 60, + }, + { + .channel = + { + .id = "11148817", + }, + .target = + { + .login = "forsen", + }, + .reason = "the timeout reason", + .duration = 10 * 60, + }, + }, + .expectedError = "", + }, + }; + + for (const auto &test : tests) + { + auto oActions = commands::parseChannelAction( + test.inputContext, command, usage, withDuration, withReason); + + if (!test.expectedActions.empty()) + { + ASSERT_TRUE(oActions.has_value()) << oActions.error(); + auto actions = *oActions; + ASSERT_EQ(actions.size(), test.expectedActions.size()); + ASSERT_EQ(actions, test.expectedActions); + } + else + { + ASSERT_FALSE(oActions.has_value()); + } + + if (!test.expectedError.isEmpty()) + { + ASSERT_FALSE(oActions.has_value()); + ASSERT_EQ(oActions.error(), test.expectedError); + } + } +} + +TEST(Commands, parseUnbanActions) { MockApplication app; std::shared_ptr channel = std::make_shared("forsen"); - CommandContext ctx{ - .words = {"/ban", "forsen"}, - .channel = channel, - .twitchChannel = channel.get(), + + CommandContext ctx{}; + + QString command("/unban"); + QString usage("usage string"); + bool withDuration = false; + bool withReason = false; + + struct Test { + CommandContext inputContext; + + std::vector expectedActions; + QString expectedError; }; - auto oActions = commands::parseChannelAction(ctx, "/ban", "usage", false); - ASSERT_TRUE(oActions.has_value()) << oActions.error(); - auto actions = *oActions; - ASSERT_EQ(actions.size(), 1); - ASSERT_EQ(actions.front().rawTarget, "forsen"); + std::vector tests{ + { + // Normal unban + .inputContext = + { + .words = {"/unban", "forsen"}, + .channel = channel, + .twitchChannel = channel.get(), + }, + .expectedActions = + { + { + .channel = + { + .login = "forsen", + .displayName = "forsen", + }, + .target = + { + .login = "forsen", + }, + }, + }, + .expectedError = "", + }, + { + // Normal unban but user input some random junk after the target + .inputContext = + { + .words = {"/unban", "forsen", "foo", "bar", "baz"}, + .channel = channel, + .twitchChannel = channel.get(), + }, + .expectedActions = + { + { + .channel = + { + .login = "forsen", + .displayName = "forsen", + }, + .target = + { + .login = "forsen", + }, + }, + }, + .expectedError = "", + }, + { + // User forgot to specify who to unban + .inputContext = + { + .words = {"/unban"}, + .channel = channel, + .twitchChannel = channel.get(), + }, + .expectedActions = {}, + .expectedError = "Missing target - " % usage, + }, + { + // User tried to use /unban outside of a channel context (shouldn't really be able to happen) + .inputContext = + { + .words = {"/unban"}, + }, + .expectedActions = {}, + .expectedError = + "A " % command % + " action must be performed with a channel as a context", + }, + { + // User tried to use /unban without a target, but with a --channel specified + .inputContext = + { + .words = {"/unban", "--channel", "pajlada"}, + .channel = channel, + .twitchChannel = channel.get(), + }, + .expectedActions = {}, + .expectedError = "Missing target - " % usage, + }, + { + // User overriding the unban to be done in the pajlada channel + .inputContext = + { + .words = {"/unban", "--channel", "pajlada", "forsen"}, + .channel = channel, + .twitchChannel = channel.get(), + }, + .expectedActions = + { + { + .channel = + { + .login = "pajlada", + }, + .target = + { + .login = "forsen", + }, + }, + }, + .expectedError = "", + }, + { + // User overriding the unban to be done in the pajlada channel and in the channel with the id 11148817 + .inputContext = + { + .words = {"/unban", "--channel", "pajlada", "--channel", + "id:11148817", "forsen"}, + .channel = channel, + .twitchChannel = channel.get(), + }, + .expectedActions = + { + { + .channel = + { + .login = "pajlada", + }, + .target = + { + .login = "forsen", + }, + }, + { + .channel = + { + .id = "11148817", + }, + .target = + { + .login = "forsen", + }, + }, + }, + .expectedError = "", + }, + }; + + for (const auto &test : tests) + { + auto oActions = commands::parseChannelAction( + test.inputContext, command, usage, withDuration, withReason); + + if (!test.expectedActions.empty()) + { + ASSERT_TRUE(oActions.has_value()) << oActions.error(); + auto actions = *oActions; + ASSERT_EQ(actions.size(), test.expectedActions.size()); + ASSERT_EQ(actions, test.expectedActions); + } + else + { + ASSERT_FALSE(oActions.has_value()); + } + + if (!test.expectedError.isEmpty()) + { + ASSERT_FALSE(oActions.has_value()); + ASSERT_EQ(oActions.error(), test.expectedError); + } + } } From 0849447aec071c3cf0964f303d4f96df7f425412 Mon Sep 17 00:00:00 2001 From: Rasmus Karlsson Date: Sun, 2 Jun 2024 14:25:12 +0200 Subject: [PATCH 06/17] Add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ac43b47a39..c259dc5d84b 100644 --- a/CHANGELOG.md +++ b/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) - Bugfix: Fixed tab move animation occasionally failing to start after closing a tab. (#5426) From e5587c17ad9bf765e7a4be52c1553451446a23ed Mon Sep 17 00:00:00 2001 From: Rasmus Karlsson Date: Sun, 2 Jun 2024 14:40:29 +0200 Subject: [PATCH 07/17] fix for qt5 --- src/controllers/commands/common/ChannelAction.cpp | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/controllers/commands/common/ChannelAction.cpp b/src/controllers/commands/common/ChannelAction.cpp index af816238ab7..2c9e80ea4a5 100644 --- a/src/controllers/commands/common/ChannelAction.cpp +++ b/src/controllers/commands/common/ChannelAction.cpp @@ -83,14 +83,14 @@ nonstd::expected, QString> parseChannelAction( }); parser.parse(ctx.words); - const auto &positionalArguments = parser.positionalArguments(); + auto positionalArguments = parser.positionalArguments(); if (positionalArguments.isEmpty()) { return nonstd::make_unexpected("Missing target - " % usage); } auto [targetUserName, targetUserID] = - parseUserNameOrID(positionalArguments.first()); + parseUserNameOrID(positionalArguments.takeFirst()); PerformChannelAction base{ .target = @@ -104,13 +104,13 @@ nonstd::expected, QString> parseChannelAction( if (withDuration) { - auto durationStr = positionalArguments.value(1); - if (durationStr.isEmpty()) + if (positionalArguments.isEmpty()) { base.duration = 10 * 60; // 10 min } else { + auto durationStr = positionalArguments.takeFirst(); base.duration = (int)parseDurationToSeconds(durationStr); if (base.duration <= 0) { @@ -118,7 +118,7 @@ nonstd::expected, QString> parseChannelAction( } if (withReason) { - base.reason = positionalArguments.sliced(2).join(' '); + base.reason = positionalArguments.join(' '); } } } @@ -126,7 +126,7 @@ nonstd::expected, QString> parseChannelAction( { if (withReason) { - base.reason = positionalArguments.sliced(1).join(' '); + base.reason = positionalArguments.join(' '); } } From d500459e8e4687a3428df7c50199f8a66ec2ebb0 Mon Sep 17 00:00:00 2001 From: Rasmus Karlsson Date: Sat, 15 Jun 2024 13:02:34 +0200 Subject: [PATCH 08/17] undo sorting --- src/common/QLogging.hpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/QLogging.hpp b/src/common/QLogging.hpp index d509354d476..b814bb33246 100644 --- a/src/common/QLogging.hpp +++ b/src/common/QLogging.hpp @@ -13,10 +13,10 @@ Q_DECLARE_LOGGING_CATEGORY(chatterinoCrashhandler); Q_DECLARE_LOGGING_CATEGORY(chatterinoEmoji); Q_DECLARE_LOGGING_CATEGORY(chatterinoEnv); Q_DECLARE_LOGGING_CATEGORY(chatterinoFfzemotes); -Q_DECLARE_LOGGING_CATEGORY(chatterinoHTTP); Q_DECLARE_LOGGING_CATEGORY(chatterinoHelper); 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); From bdf1788e9893f82ae466b447e1bf819a6d6602e0 Mon Sep 17 00:00:00 2001 From: Rasmus Karlsson Date: Sat, 15 Jun 2024 13:30:50 +0200 Subject: [PATCH 09/17] mock LinkResolver in RecentMessages benchmark --- benchmarks/src/RecentMessages.cpp | 7 ++++++ mocks/include/mocks/LinkResolver.hpp | 32 ++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) create mode 100644 mocks/include/mocks/LinkResolver.hpp diff --git a/benchmarks/src/RecentMessages.cpp b/benchmarks/src/RecentMessages.cpp index fd5fe0f1a14..7f2c0c7aca5 100644 --- a/benchmarks/src/RecentMessages.cpp +++ b/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 @@ class MockApplication : mock::EmptyApplication 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; diff --git a/mocks/include/mocks/LinkResolver.hpp b/mocks/include/mocks/LinkResolver.hpp new file mode 100644 index 00000000000..8a5682a3d1c --- /dev/null +++ b/mocks/include/mocks/LinkResolver.hpp @@ -0,0 +1,32 @@ +#pragma once + +#include "providers/links/LinkResolver.hpp" + +#include +#include +#include + +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 From 4dcdc782015cf71adc979b36a7e5ac71cc5fd64d Mon Sep 17 00:00:00 2001 From: Rasmus Karlsson Date: Sat, 15 Jun 2024 13:31:35 +0200 Subject: [PATCH 10/17] remove commented out code --- src/providers/twitch/TwitchChannel.cpp | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/providers/twitch/TwitchChannel.cpp b/src/providers/twitch/TwitchChannel.cpp index c86664b5ccf..1186402ad3c 100644 --- a/src/providers/twitch/TwitchChannel.cpp +++ b/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); From 5f8885fc6f686530abbeb57549caacd949407968 Mon Sep 17 00:00:00 2001 From: Rasmus Karlsson Date: Sat, 15 Jun 2024 13:41:28 +0200 Subject: [PATCH 11/17] fix cv paste error --- src/controllers/commands/builtin/twitch/Unban.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/controllers/commands/builtin/twitch/Unban.cpp b/src/controllers/commands/builtin/twitch/Unban.cpp index 20f9282bc5f..f47d701683f 100644 --- a/src/controllers/commands/builtin/twitch/Unban.cpp +++ b/src/controllers/commands/builtin/twitch/Unban.cpp @@ -89,7 +89,7 @@ QString unbanUser(const CommandContext &ctx) const auto command = ctx.words.at(0).toLower(); const auto usage = QStringLiteral( - R"(Usage: "%1 - Removes a ban on a user. Options: --channel to override which channel the timeout takes place in (can be specified multiple times).)") + R"(Usage: "%1 - Removes a ban on a user. Options: --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()) @@ -126,7 +126,7 @@ QString unbanUser(const CommandContext &ctx) if (action.target.id.isEmpty()) { assert(!action.target.login.isEmpty() && - "Timeout Action target username AND user ID may not be " + "Unban Action target username AND user ID may not be " "empty at the same time"); userLoginsToFetch.append(action.target.login); } @@ -138,7 +138,7 @@ QString unbanUser(const CommandContext &ctx) if (action.channel.id.isEmpty()) { assert(!action.channel.login.isEmpty() && - "Timeout Action channel username AND user ID may not be " + "Unban Action channel username AND user ID may not be " "empty at the same time"); userLoginsToFetch.append(action.channel.login); } From ae6d31b122b2563dc32fda59db10502383bcdfcc Mon Sep 17 00:00:00 2001 From: Rasmus Karlsson Date: Sat, 15 Jun 2024 15:42:43 +0200 Subject: [PATCH 12/17] use getIApp()->isTest() instead of checking for getApp() --- mocks/include/mocks/EmptyApplication.hpp | 7 ++++- mocks/include/mocks/Logging.hpp | 33 ++++++++++++++++++++++++ src/Application.cpp | 2 +- src/Application.hpp | 12 +++++++-- src/providers/twitch/TwitchChannel.cpp | 11 +++++++- src/singletons/Logging.hpp | 13 ++++++++-- 6 files changed, 71 insertions(+), 7 deletions(-) create mode 100644 mocks/include/mocks/Logging.hpp diff --git a/mocks/include/mocks/EmptyApplication.hpp b/mocks/include/mocks/EmptyApplication.hpp index 54906f56c75..233211bfa81 100644 --- a/mocks/include/mocks/EmptyApplication.hpp +++ b/mocks/include/mocks/EmptyApplication.hpp @@ -19,6 +19,11 @@ class EmptyApplication : public IApplication virtual ~EmptyApplication() = default; + bool isTest() const override + { + return true; + } + const Paths &getPaths() override { return this->paths_; @@ -137,7 +142,7 @@ class EmptyApplication : public IApplication return nullptr; } - Logging *getChatLogger() override + ILogging *getChatLogger() override { assert(!"getChatLogger was called without being initialized"); return nullptr; diff --git a/mocks/include/mocks/Logging.hpp b/mocks/include/mocks/Logging.hpp new file mode 100644 index 00000000000..f5315b1dd86 --- /dev/null +++ b/mocks/include/mocks/Logging.hpp @@ -0,0 +1,33 @@ +#pragma once + +#include "singletons/Logging.hpp" + +#include +#include +#include + +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 diff --git a/src/Application.cpp b/src/Application.cpp index 77663923f88..5a9323921a7 100644 --- a/src/Application.cpp +++ b/src/Application.cpp @@ -504,7 +504,7 @@ PubSub *Application::getTwitchPubSub() return this->twitchPubSub.get(); } -Logging *Application::getChatLogger() +ILogging *Application::getChatLogger() { assertInGuiThread(); diff --git a/src/Application.hpp b/src/Application.hpp index 795a9f6d976..d2c0e2facc3 100644 --- a/src/Application.hpp +++ b/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 @@ class IApplication 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 @@ class IApplication 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 @@ class Application : public IApplication 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 @@ class Application : public IApplication ITwitchIrcServer *getTwitch() override; IAbstractIrcServer *getTwitchAbstract() override; PubSub *getTwitchPubSub() override; - Logging *getChatLogger() override; + ILogging *getChatLogger() override; FfzBadges *getFfzBadges() override; SeventvBadges *getSeventvBadges() override; IUserDataController *getUserData() override; diff --git a/src/providers/twitch/TwitchChannel.cpp b/src/providers/twitch/TwitchChannel.cpp index 1186402ad3c..bb982c8fe3e 100644 --- a/src/providers/twitch/TwitchChannel.cpp +++ b/src/providers/twitch/TwitchChannel.cpp @@ -572,6 +572,10 @@ void TwitchChannel::showLoginMessage() void TwitchChannel::roomIdChanged() { + if (getIApp()->isTest()) + { + return; + } this->refreshPubSub(); this->refreshBadges(); this->refreshCheerEmotes(); @@ -778,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(); @@ -1327,6 +1331,11 @@ void TwitchChannel::loadRecentMessagesReconnect() void TwitchChannel::refreshPubSub() { + if (getIApp()->isTest()) + { + return; + } + auto roomId = this->roomId(); if (roomId.isEmpty()) { diff --git a/src/singletons/Logging.hpp b/src/singletons/Logging.hpp index edd1ac07fc9..af86a702dcf 100644 --- a/src/singletons/Logging.hpp +++ b/src/singletons/Logging.hpp @@ -16,13 +16,22 @@ struct Message; using MessagePtr = std::shared_ptr; 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; From f1c0e8271195f2ac4ece441aad4836acd1b9e907 Mon Sep 17 00:00:00 2001 From: Rasmus Karlsson Date: Sat, 15 Jun 2024 15:42:56 +0200 Subject: [PATCH 13/17] add more tests --- src/providers/twitch/TwitchChannel.hpp | 1 + tests/src/Commands.cpp | 257 +++++++++++++++++++++++++ 2 files changed, 258 insertions(+) diff --git a/src/providers/twitch/TwitchChannel.hpp b/src/providers/twitch/TwitchChannel.hpp index 2add5430213..611fea1f9a0 100644 --- a/src/providers/twitch/TwitchChannel.hpp +++ b/src/providers/twitch/TwitchChannel.hpp @@ -463,6 +463,7 @@ class TwitchChannel final : public Channel, public ChannelChatters friend class TwitchIrcServer; friend class TwitchMessageBuilder; friend class IrcMessageHandler; + friend class Commands_E2E_Test; }; } // namespace chatterino diff --git a/tests/src/Commands.cpp b/tests/src/Commands.cpp index a0eb3cb1c01..0f7ca62b6d6 100644 --- a/tests/src/Commands.cpp +++ b/tests/src/Commands.cpp @@ -1,9 +1,15 @@ #include "controllers/accounts/AccountController.hpp" +#include "controllers/commands/Command.hpp" #include "controllers/commands/CommandContext.hpp" +#include "controllers/commands/CommandController.hpp" #include "controllers/commands/common/ChannelAction.hpp" #include "mocks/EmptyApplication.hpp" +#include "mocks/Helix.hpp" +#include "mocks/Logging.hpp" #include "mocks/TwitchIrcServer.hpp" +#include "providers/twitch/TwitchAccount.hpp" #include "providers/twitch/TwitchChannel.hpp" +#include "singletons/Emotes.hpp" #include "singletons/Settings.hpp" #include "Test.hpp" @@ -11,6 +17,9 @@ using namespace chatterino; +using ::testing::_; +using ::testing::StrictMock; + namespace { class MockApplication : mock::EmptyApplication @@ -31,13 +40,33 @@ class MockApplication : mock::EmptyApplication return &this->accounts; } + CommandController *getCommands() override + { + return &this->commands; + } + + IEmotes *getEmotes() override + { + return &this->emotes; + } + + ILogging *getChatLogger() override + { + return &this->chatLogger; + } + Settings settings; AccountController accounts; + CommandController commands; mock::MockTwitchIrcServer twitch; + Emotes emotes; + mock::EmptyLogging chatLogger; }; } // namespace +namespace chatterino { + TEST(Commands, parseBanActions) { MockApplication app; @@ -798,3 +827,231 @@ TEST(Commands, parseUnbanActions) } } } + +TEST(Commands, E2E) +{ + ::testing::InSequence seq; + MockApplication app; + + app.commands.initialize(*getSettings(), getIApp()->getPaths()); + + QJsonObject pajlada; + pajlada["id"] = "11148817"; + pajlada["login"] = "pajlada"; + pajlada["display_name"] = "pajlada"; + pajlada["created_at"] = "2010-03-17T11:50:53Z"; + pajlada["description"] = " ͡° ͜ʖ ͡°)"; + pajlada["profile_image_url"] = + "https://static-cdn.jtvnw.net/jtv_user_pictures/" + "cbe986e3-06ad-4506-a3aa-eb05466c839c-profile_image-300x300.png"; + + QJsonObject testaccount420; + testaccount420["id"] = "117166826"; + testaccount420["login"] = "testaccount_420"; + testaccount420["display_name"] = "테스트계정420"; + testaccount420["created_at"] = "2016-02-27T18:55:59Z"; + testaccount420["description"] = ""; + testaccount420["profile_image_url"] = + "https://static-cdn.jtvnw.net/user-default-pictures-uv/" + "ead5c8b2-a4c9-4724-b1dd-9f00b46cbd3d-profile_image-300x300.png"; + + QJsonObject forsen; + forsen["id"] = "22484632"; + forsen["login"] = "forsen"; + forsen["display_name"] = "Forsen"; + forsen["created_at"] = "2011-05-19T00:28:28Z"; + forsen["description"] = + "Approach with caution! No roleplaying or tryharding allowed."; + forsen["profile_image_url"] = + "https://static-cdn.jtvnw.net/jtv_user_pictures/" + "forsen-profile_image-48b43e1e4f54b5c8-300x300.png"; + + std::shared_ptr channel = + std::make_shared("pajlada"); + channel->setRoomId("11148817"); + + StrictMock mockHelix; + initializeHelix(&mockHelix); + + EXPECT_CALL(mockHelix, update).Times(1); + EXPECT_CALL(mockHelix, loadBlocks).Times(1); + + auto account = std::make_shared( + testaccount420["login"].toString(), "token", "oauthclient", + testaccount420["id"].toString()); + getIApp()->getAccounts()->twitch.accounts.append(account); + getIApp()->getAccounts()->twitch.currentUsername = + testaccount420["login"].toString(); + getIApp()->getAccounts()->twitch.load(); + + // Simple single-channel ban + EXPECT_CALL(mockHelix, fetchUsers(QStringList{"11148817"}, + QStringList{"forsen"}, _, _)) + .WillOnce([=](auto, auto, auto success, auto) { + std::vector users{ + HelixUser(pajlada), + HelixUser(forsen), + }; + success(users); + }); + + EXPECT_CALL(mockHelix, + banUser(pajlada["id"].toString(), QString("117166826"), + forsen["id"].toString(), std::optional{}, + QString(""), _, _)) + .Times(1); + + getIApp()->getCommands()->execCommand("/ban forsen", channel, false); + + // Multi-channel ban + EXPECT_CALL(mockHelix, fetchUsers(QStringList{"11148817"}, + QStringList{"forsen"}, _, _)) + .WillOnce([=](auto, auto, auto success, auto) { + std::vector users{ + HelixUser(pajlada), + HelixUser(forsen), + }; + success(users); + }); + + EXPECT_CALL(mockHelix, banUser(pajlada["id"].toString(), + testaccount420["id"].toString(), + forsen["id"].toString(), + std::optional{}, QString(""), _, _)) + .Times(1); + + EXPECT_CALL(mockHelix, + fetchUsers(QStringList{}, + QStringList{"forsen", "testaccount_420"}, _, _)) + .WillOnce([=](auto, auto, auto success, auto) { + std::vector users{ + HelixUser(testaccount420), + HelixUser(forsen), + }; + success(users); + }); + + EXPECT_CALL(mockHelix, banUser(testaccount420["id"].toString(), + testaccount420["id"].toString(), + forsen["id"].toString(), + std::optional{}, QString(""), _, _)) + .Times(1); + + getIApp()->getCommands()->execCommand( + "/ban --channel id:11148817 --channel testaccount_420 forsen", channel, + false); + + // ID-based ban + EXPECT_CALL(mockHelix, + banUser(pajlada["id"].toString(), QString("117166826"), + forsen["id"].toString(), std::optional{}, + QString(""), _, _)) + .Times(1); + + getIApp()->getCommands()->execCommand("/ban id:22484632", channel, false); + + // ID-based redirected ban + EXPECT_CALL(mockHelix, + banUser(testaccount420["id"].toString(), QString("117166826"), + forsen["id"].toString(), std::optional{}, + QString(""), _, _)) + .Times(1); + + getIApp()->getCommands()->execCommand( + "/ban --channel id:117166826 id:22484632", channel, false); + + // name-based redirected ban + EXPECT_CALL(mockHelix, fetchUsers(QStringList{"22484632"}, + QStringList{"testaccount_420"}, _, _)) + .WillOnce([=](auto, auto, auto success, auto) { + std::vector users{ + HelixUser(testaccount420), + HelixUser(forsen), + }; + success(users); + }); + EXPECT_CALL(mockHelix, + banUser(testaccount420["id"].toString(), QString("117166826"), + forsen["id"].toString(), std::optional{}, + QString(""), _, _)) + .Times(1); + + getIApp()->getCommands()->execCommand( + "/ban --channel testaccount_420 id:22484632", channel, false); + + // Multi-channel timeout + EXPECT_CALL(mockHelix, fetchUsers(QStringList{"11148817"}, + QStringList{"forsen"}, _, _)) + .WillOnce([=](auto, auto, auto success, auto) { + std::vector users{ + HelixUser(pajlada), + HelixUser(forsen), + }; + success(users); + }); + + EXPECT_CALL(mockHelix, banUser(pajlada["id"].toString(), + testaccount420["id"].toString(), + forsen["id"].toString(), + std::optional{600}, QString(""), _, _)) + .Times(1); + + EXPECT_CALL(mockHelix, + fetchUsers(QStringList{}, + QStringList{"forsen", "testaccount_420"}, _, _)) + .WillOnce([=](auto, auto, auto success, auto) { + std::vector users{ + HelixUser(testaccount420), + HelixUser(forsen), + }; + success(users); + }); + + EXPECT_CALL(mockHelix, banUser(testaccount420["id"].toString(), + testaccount420["id"].toString(), + forsen["id"].toString(), + std::optional{600}, QString(""), _, _)) + .Times(1); + + getIApp()->getCommands()->execCommand( + "/timeout --channel id:11148817 --channel testaccount_420 forsen", + channel, false); + + // Multi-channel unban + EXPECT_CALL(mockHelix, fetchUsers(QStringList{"11148817"}, + QStringList{"forsen"}, _, _)) + .WillOnce([=](auto, auto, auto success, auto) { + std::vector users{ + HelixUser(pajlada), + HelixUser(forsen), + }; + success(users); + }); + + EXPECT_CALL(mockHelix, unbanUser(pajlada["id"].toString(), + testaccount420["id"].toString(), + forsen["id"].toString(), _, _)) + .Times(1); + + EXPECT_CALL(mockHelix, + fetchUsers(QStringList{}, + QStringList{"forsen", "testaccount_420"}, _, _)) + .WillOnce([=](auto, auto, auto success, auto) { + std::vector users{ + HelixUser(testaccount420), + HelixUser(forsen), + }; + success(users); + }); + + EXPECT_CALL(mockHelix, unbanUser(testaccount420["id"].toString(), + testaccount420["id"].toString(), + forsen["id"].toString(), _, _)) + .Times(1); + + getIApp()->getCommands()->execCommand( + "/unban --channel id:11148817 --channel testaccount_420 forsen", + channel, false); +} + +} // namespace chatterino From 1397aefd901d6d244b9aefe2c1204da45fb17713 Mon Sep 17 00:00:00 2001 From: Rasmus Karlsson Date: Sun, 16 Jun 2024 11:15:33 +0200 Subject: [PATCH 14/17] format --- mocks/include/mocks/Logging.hpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/mocks/include/mocks/Logging.hpp b/mocks/include/mocks/Logging.hpp index f5315b1dd86..8d444142bb6 100644 --- a/mocks/include/mocks/Logging.hpp +++ b/mocks/include/mocks/Logging.hpp @@ -14,8 +14,10 @@ class Logging : public ILogging Logging() = default; ~Logging() override = default; - MOCK_METHOD(void, addMessage, (const QString &channelName, MessagePtr message, - const QString &platformName), (override)); + MOCK_METHOD(void, addMessage, + (const QString &channelName, MessagePtr message, + const QString &platformName), + (override)); }; class EmptyLogging : public ILogging @@ -25,7 +27,8 @@ class EmptyLogging : public ILogging ~EmptyLogging() override = default; void addMessage(const QString &channelName, MessagePtr message, - const QString &platformName) override { + const QString &platformName) override + { // } }; From d43cf4c0c162c8309dcbb7f0ee1824cca02e85d1 Mon Sep 17 00:00:00 2001 From: Rasmus Karlsson Date: Sun, 16 Jun 2024 11:33:55 +0200 Subject: [PATCH 15/17] add missing direct includes, default-initialize duration, change channel id to channel --- src/controllers/commands/common/ChannelAction.cpp | 8 +++++++- src/controllers/commands/common/ChannelAction.hpp | 6 +++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/controllers/commands/common/ChannelAction.cpp b/src/controllers/commands/common/ChannelAction.cpp index 2c9e80ea4a5..4873859207d 100644 --- a/src/controllers/commands/common/ChannelAction.cpp +++ b/src/controllers/commands/common/ChannelAction.cpp @@ -3,11 +3,17 @@ #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 #include +#include +#include +#include +#include + namespace chatterino::commands { bool IncompleteHelixUser::hydrateFrom(const std::vector &users) @@ -77,7 +83,7 @@ nonstd::expected, QString> parseChannelAction( } QCommandLineOption channelOption( "channel", "Override which channel(s) to perform the action in", - "channel id"); + "channel"); parser.addOptions({ channelOption, }); diff --git a/src/controllers/commands/common/ChannelAction.hpp b/src/controllers/commands/common/ChannelAction.hpp index 11f620f614e..fd80eeb7954 100644 --- a/src/controllers/commands/common/ChannelAction.hpp +++ b/src/controllers/commands/common/ChannelAction.hpp @@ -3,6 +3,10 @@ #include #include +#include +#include +#include + namespace chatterino { struct CommandContext; @@ -32,7 +36,7 @@ struct PerformChannelAction { // Target to perform the action on IncompleteHelixUser target; QString reason; - int duration; + int duration{}; bool operator==(const PerformChannelAction &other) const { From a428fb49fef38010ce6699430676b4bee4a019fe Mon Sep 17 00:00:00 2001 From: Rasmus Karlsson Date: Sun, 16 Jun 2024 11:40:42 +0200 Subject: [PATCH 16/17] fix typo in /timeout usage string --- src/controllers/commands/builtin/twitch/Ban.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/controllers/commands/builtin/twitch/Ban.cpp b/src/controllers/commands/builtin/twitch/Ban.cpp index 8effa08dda9..bce3001f4b0 100644 --- a/src/controllers/commands/builtin/twitch/Ban.cpp +++ b/src/controllers/commands/builtin/twitch/Ban.cpp @@ -275,7 +275,7 @@ QString sendTimeout(const CommandContext &ctx) { const auto command = QStringLiteral("/timeout"); const auto usage = QStringLiteral( - R"(Usage: "/timeout [options...] [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 to override which channel the unban takes place in (can be specified multiple times).)"); + R"(Usage: "/timeout [options...] [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 to override which channel the timeout takes place in (can be specified multiple times).)"); const auto actions = parseChannelAction(ctx, command, usage, true, true); if (!actions.has_value()) @@ -299,7 +299,7 @@ QString sendTimeout(const CommandContext &ctx) if (currentUser->isAnon()) { ctx.channel->addMessage( - makeSystemMessage("You must be logged in to ban someone!")); + makeSystemMessage("You must be logged in to timeout someone!")); return ""; } From 9ac72ee0edabd0d43480bc770024dce2e5f60dd5 Mon Sep 17 00:00:00 2001 From: Rasmus Karlsson Date: Sun, 16 Jun 2024 11:50:21 +0200 Subject: [PATCH 17/17] comment typo --- tests/src/Commands.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/Commands.cpp b/tests/src/Commands.cpp index 0f7ca62b6d6..f180a718080 100644 --- a/tests/src/Commands.cpp +++ b/tests/src/Commands.cpp @@ -90,7 +90,7 @@ TEST(Commands, parseBanActions) std::vector tests{ { - // Normal ban with an added reason, with the user maybe trying to use the --channal parameter at the end, but it gets eaten by the reason + // Normal ban with an added reason, with the user maybe trying to use the --channel parameter at the end, but it gets eaten by the reason .inputContext = { .words = {"/ban", "forsen", "the", "ban", "reason",