Made username and color in AutoMod mod messages use correct values (#2967)

We use values given in pubsub messages and handle their edge-cases properly.

CHANGELOG.md

@@ -6,6 +6,7 @@
 - Minor: Channel name in `<channel> has gone offline. Exiting host mode.` messages is now clickable. (#2922)
 - Minor: Added `/openurl` command. Usage: `/openurl <URL>`. Opens the provided URL in the browser. (#2461, #2926)
 - Minor: Updated to Emoji v13.1 (#2958)
+- Minor: Sender username in automod messages shown to moderators shows correct color and display name. (#2967)
 - Bugfix: Now deleting cache files that weren't modified in the past 14 days. (#2947)
 - Bugfix: Fixed large timeout durations in moderation buttons overlapping with usernames or other buttons. (#2865, #2921)
 - Bugfix: Middle mouse click no longer scrolls in not fully populated usercards and splits. (#2933)

src/Application.cpp

@@ -207,7 +207,7 @@ void Application::initPubsub()
             }
 
             QString text =
-                QString("%1 cleared the chat").arg(action.source.name);
+                QString("%1 cleared the chat").arg(action.source.login);
 
             auto msg = makeSystemMessage(text);
             postToThread([chan, msg] {
@@ -226,15 +226,14 @@ void Application::initPubsub()
 
             QString text =
                 QString("%1 turned %2 %3 mode")
-                    .arg(action.source.name)
+                    .arg(action.source.login)
                     .arg(action.state == ModeChangedAction::State::On ? "on"
                                                                       : "off")
                     .arg(action.getModeName());
 
             if (action.duration > 0)
             {
-                text.append(" (" + QString::number(action.duration) +
+                text += QString(" (%1 seconds)").arg(action.duration);
-                            " seconds)");
             }
 
             auto msg = makeSystemMessage(text);
@@ -254,16 +253,10 @@ void Application::initPubsub()
 
             QString text;
 
-            if (action.modded)
+            text = QString("%1 %2 %3")
-            {
+                       .arg(action.source.login,
-                text = QString("%1 modded %2")
+                            (action.modded ? "modded" : "unmodded"),
-                           .arg(action.source.name, action.target.name);
+                            action.target.login);
-            }
-            else
-            {
-                text = QString("%1 unmodded %2")
-                           .arg(action.source.name, action.target.name);
-            }
 
             auto msg = makeSystemMessage(text);
             postToThread([chan, msg] {

src/messages/MessageBuilder.cpp

@@ -140,25 +140,26 @@ std::pair<MessagePtr, MessagePtr> makeAutomodMessage(
     // Builder for offender's message
     builder2.emplace<TimestampElement>();
     builder2.emplace<TwitchModerationElement>();
-    builder2.message().loginName = action.target.name;
+    builder2.message().loginName = action.target.login;
     builder2.message().flags.set(MessageFlag::PubSub);
 
     // sender username
     builder2
         .emplace<TextElement>(
-            action.target.name + ":", MessageElementFlag::BoldUsername,
+            action.target.displayName + ":", MessageElementFlag::BoldUsername,
-            MessageColor(QColor("red")), FontStyle::ChatMediumBold)
+            MessageColor(action.target.color), FontStyle::ChatMediumBold)
-        ->setLink({Link::UserInfo, action.target.name});
+        ->setLink({Link::UserInfo, action.target.login});
     builder2
-        .emplace<TextElement>(action.target.name + ":",
+        .emplace<TextElement>(action.target.displayName + ":",
                               MessageElementFlag::NonBoldUsername,
-                              MessageColor(QColor("red")))
+                              MessageColor(action.target.color))
-        ->setLink({Link::UserInfo, action.target.name});
+        ->setLink({Link::UserInfo, action.target.login});
     // sender's message caught by AutoMod
     builder2.emplace<TextElement>(action.message, MessageElementFlag::Text,
                                   MessageColor::Text);
     builder2.message().flags.set(MessageFlag::AutoMod);
-    auto text2 = QString("%1: %2").arg(action.target.name, action.message);
+    auto text2 =
+        QString("%1: %2").arg(action.target.displayName, action.message);
     builder2.message().messageText = text2;
     builder2.message().searchText = text2;
 
@@ -277,7 +278,7 @@ MessageBuilder::MessageBuilder(const BanAction &action, uint32_t count)
     this->emplace<TimestampElement>();
     this->message().flags.set(MessageFlag::System);
     this->message().flags.set(MessageFlag::Timeout);
-    this->message().timeoutUser = action.target.name;
+    this->message().timeoutUser = action.target.login;
     this->message().count = count;
 
     QString text;
@@ -296,13 +297,13 @@ MessageBuilder::MessageBuilder(const BanAction &action, uint32_t count)
                 text);
         }
 
-        if (!action.source.name.isEmpty())
+        if (!action.source.login.isEmpty())
         {
             this->emplaceSystemTextAndUpdate("by", text);
             this->emplaceSystemTextAndUpdate(
-                    action.source.name + (action.reason.isEmpty() ? "." : ":"),
+                    action.source.login + (action.reason.isEmpty() ? "." : ":"),
                     text)
-                ->setLink({Link::UserInfo, action.source.name});
+                ->setLink({Link::UserInfo, action.source.login});
         }
 
         if (!action.reason.isEmpty())
@@ -315,29 +316,30 @@ MessageBuilder::MessageBuilder(const BanAction &action, uint32_t count)
     {
         if (action.isBan())
         {
-            this->emplaceSystemTextAndUpdate(action.source.name, text)
+            this->emplaceSystemTextAndUpdate(action.source.login, text)
-                ->setLink({Link::UserInfo, action.source.name});
+                ->setLink({Link::UserInfo, action.source.login});
             this->emplaceSystemTextAndUpdate("banned", text);
             if (action.reason.isEmpty())
             {
-                this->emplaceSystemTextAndUpdate(action.target.name, text)
+                this->emplaceSystemTextAndUpdate(action.target.login, text)
-                    ->setLink({Link::UserInfo, action.target.name});
+                    ->setLink({Link::UserInfo, action.target.login});
             }
             else
             {
-                this->emplaceSystemTextAndUpdate(action.target.name + ":", text)
+                this->emplaceSystemTextAndUpdate(action.target.login + ":",
-                    ->setLink({Link::UserInfo, action.target.name});
+                                                 text)
+                    ->setLink({Link::UserInfo, action.target.login});
                 this->emplaceSystemTextAndUpdate(
                     QString("\"%1\".").arg(action.reason), text);
             }
         }
         else
         {
-            this->emplaceSystemTextAndUpdate(action.source.name, text)
+            this->emplaceSystemTextAndUpdate(action.source.login, text)
-                ->setLink({Link::UserInfo, action.source.name});
+                ->setLink({Link::UserInfo, action.source.login});
             this->emplaceSystemTextAndUpdate("timed out", text);
-            this->emplaceSystemTextAndUpdate(action.target.name, text)
+            this->emplaceSystemTextAndUpdate(action.target.login, text)
-                ->setLink({Link::UserInfo, action.target.name});
+                ->setLink({Link::UserInfo, action.target.login});
             if (action.reason.isEmpty())
             {
                 this->emplaceSystemTextAndUpdate(
@@ -371,16 +373,16 @@ MessageBuilder::MessageBuilder(const UnbanAction &action)
     this->message().flags.set(MessageFlag::System);
     this->message().flags.set(MessageFlag::Untimeout);
 
-    this->message().timeoutUser = action.target.name;
+    this->message().timeoutUser = action.target.login;
 
     QString text;
 
-    this->emplaceSystemTextAndUpdate(action.source.name, text)
+    this->emplaceSystemTextAndUpdate(action.source.login, text)
-        ->setLink({Link::UserInfo, action.source.name});
+        ->setLink({Link::UserInfo, action.source.login});
     this->emplaceSystemTextAndUpdate(
         action.wasBan() ? "unbanned" : "untimedout", text);
-    this->emplaceSystemTextAndUpdate(action.target.name, text)
+    this->emplaceSystemTextAndUpdate(action.target.login, text)
-        ->setLink({Link::UserInfo, action.target.name});
+        ->setLink({Link::UserInfo, action.target.login});
 
     this->message().messageText = text;
     this->message().searchText = text;
@@ -397,31 +399,31 @@ MessageBuilder::MessageBuilder(const AutomodUserAction &action)
     {
         case AutomodUserAction::AddPermitted: {
             text = QString("%1 added %2 as a permitted term on AutoMod.")
-                       .arg(action.source.name, action.message);
+                       .arg(action.source.login, action.message);
         }
         break;
 
         case AutomodUserAction::AddBlocked: {
             text = QString("%1 added %2 as a blocked term on AutoMod.")
-                       .arg(action.source.name, action.message);
+                       .arg(action.source.login, action.message);
         }
         break;
 
         case AutomodUserAction::RemovePermitted: {
             text = QString("%1 removed %2 as a permitted term on AutoMod.")
-                       .arg(action.source.name, action.message);
+                       .arg(action.source.login, action.message);
         }
         break;
 
         case AutomodUserAction::RemoveBlocked: {
             text = QString("%1 removed %2 as a blocked term on AutoMod.")
-                       .arg(action.source.name, action.message);
+                       .arg(action.source.login, action.message);
         }
         break;
 
         case AutomodUserAction::Properties: {
             text = QString("%1 modified the AutoMod properties.")
-                       .arg(action.source.name);
+                       .arg(action.source.login);
         }
         break;
     }

src/providers/twitch/PubsubActions.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <rapidjson/document.h>
+#include <QColor>
 #include <QString>
 
 #include <chrono>
@@ -10,7 +11,10 @@ namespace chatterino {
 
 struct ActionUser {
     QString id;
-    QString name;
+    QString login;
+    // displayName should be in format "login(localizedName)" for non-ascii usernames
+    QString displayName;
+    QColor color;
 };
 
 struct PubSubAction {

src/providers/twitch/PubsubClient.cpp

@@ -2,6 +2,7 @@
 
 #include "providers/twitch/PubsubActions.hpp"
 #include "providers/twitch/PubsubHelpers.hpp"
+#include "singletons/Settings.hpp"
 #include "util/Helpers.hpp"
 #include "util/RapidjsonHelpers.hpp"
 
@@ -333,7 +334,7 @@ PubSub::PubSub()
                 return;
             }
 
-            if (!rj::getSafe(args[0], action.target.name))
+            if (!rj::getSafe(args[0], action.target.login))
             {
                 return;
             }
@@ -398,7 +399,7 @@ PubSub::PubSub()
                 return;
             }
 
-            if (!rj::getSafe(args[0], action.target.name))
+            if (!rj::getSafe(args[0], action.target.login))
             {
                 return;
             }
@@ -444,7 +445,7 @@ PubSub::PubSub()
                 return;
             }
 
-            if (!rj::getSafe(args[0], action.target.name))
+            if (!rj::getSafe(args[0], action.target.login))
             {
                 return;
             }
@@ -484,7 +485,7 @@ PubSub::PubSub()
                 return;
             }
 
-            if (!rj::getSafe(args[0], action.target.name))
+            if (!rj::getSafe(args[0], action.target.login))
             {
                 return;
             }
@@ -524,7 +525,7 @@ PubSub::PubSub()
                 return;
             }
 
-            if (!rj::getSafe(args[0], action.target.name))
+            if (!rj::getSafe(args[0], action.target.login))
             {
                 return;
             }
@@ -556,7 +557,7 @@ PubSub::PubSub()
                 return;
             }
 
-            if (!rj::getSafe(args[0], action.target.name))
+            if (!rj::getSafe(args[0], action.target.login))
             {
                 return;
             }
@@ -588,7 +589,7 @@ PubSub::PubSub()
                     return;
                 }
 
-                if (!rj::getSafe(args[0], action.target.name))
+                if (!rj::getSafe(args[0], action.target.login))
                 {
                     return;
                 }
@@ -658,7 +659,7 @@ PubSub::PubSub()
                     return;
                 }
 
-                if (!rj::getSafe(data, "requester_login", action.source.name))
+                if (!rj::getSafe(data, "requester_login", action.source.login))
                 {
                     return;
                 }
@@ -686,7 +687,7 @@ PubSub::PubSub()
                     return;
                 }
 
-                if (!rj::getSafe(data, "requester_login", action.source.name))
+                if (!rj::getSafe(data, "requester_login", action.source.login))
                 {
                     return;
                 }
@@ -743,7 +744,7 @@ PubSub::PubSub()
                     return;
                 }
 
-                if (!rj::getSafe(data, "requester_login", action.source.name))
+                if (!rj::getSafe(data, "requester_login", action.source.login))
                 {
                     return;
                 }
@@ -802,7 +803,7 @@ PubSub::PubSub()
                     return;
                 }
 
-                if (!rj::getSafe(data, "requester_login", action.source.name))
+                if (!rj::getSafe(data, "requester_login", action.source.login))
                 {
                     return;
                 }
@@ -1367,7 +1368,7 @@ void PubSub::handleMessageResponse(const rapidjson::Value &outerData)
             QString status;
             if (!rj::getSafe(data, "status", status))
             {
-                qDebug() << "Failed to get status";
+                qCDebug(chatterinoPubsub) << "Failed to get status";
                 return;
             }
             if (status == "PENDING")
@@ -1377,20 +1378,22 @@ void PubSub::handleMessageResponse(const rapidjson::Value &outerData)
                 if (!rj::getSafeObject(data, "content_classification",
                                        classification))
                 {
-                    qDebug() << "Failed to get content_classification";
+                    qCDebug(chatterinoPubsub)
+                        << "Failed to get content_classification";
                     return;
                 }
 
                 QString contentCategory;
                 if (!rj::getSafe(classification, "category", contentCategory))
                 {
-                    qDebug() << "Failed to get content category";
+                    qCDebug(chatterinoPubsub)
+                        << "Failed to get content category";
                     return;
                 }
                 int contentLevel;
                 if (!rj::getSafe(classification, "level", contentLevel))
                 {
-                    qDebug() << "Failed to get content level";
+                    qCDebug(chatterinoPubsub) << "Failed to get content level";
                     return;
                 }
                 action.reason = QString("%1 level %2")
@@ -1400,25 +1403,26 @@ void PubSub::handleMessageResponse(const rapidjson::Value &outerData)
                 rapidjson::Value messageData;
                 if (!rj::getSafeObject(data, "message", messageData))
                 {
-                    qDebug() << "Failed to get message data";
+                    qCDebug(chatterinoPubsub) << "Failed to get message data";
                     return;
                 }
 
                 rapidjson::Value messageContent;
                 if (!rj::getSafeObject(messageData, "content", messageContent))
                 {
-                    qDebug() << "Failed to get message content";
+                    qCDebug(chatterinoPubsub)
+                        << "Failed to get message content";
                     return;
                 }
                 if (!rj::getSafe(messageData, "id", action.msgID))
                 {
-                    qDebug() << "Failed to get message id";
+                    qCDebug(chatterinoPubsub) << "Failed to get message id";
                     return;
                 }
 
                 if (!rj::getSafe(messageContent, "text", action.message))
                 {
-                    qDebug() << "Failed to get message text";
+                    qCDebug(chatterinoPubsub) << "Failed to get message text";
                     return;
                 }
 
@@ -1428,23 +1432,68 @@ void PubSub::handleMessageResponse(const rapidjson::Value &outerData)
                 rapidjson::Value senderData;
                 if (!rj::getSafeObject(messageData, "sender", senderData))
                 {
-                    qDebug() << "Failed to get sender";
+                    qCDebug(chatterinoPubsub) << "Failed to get sender";
                     return;
                 }
-                QString sender_id;
+                QString senderId;
-                if (!rj::getSafe(senderData, "user_id", sender_id))
+                if (!rj::getSafe(senderData, "user_id", senderId))
                 {
-                    qDebug() << "Failed to get sender user id";
+                    qCDebug(chatterinoPubsub) << "Failed to get sender user id";
                     return;
                 }
-                QString sender_login;
+                QString senderLogin;
-                if (!rj::getSafe(senderData, "login", sender_login))
+                if (!rj::getSafe(senderData, "login", senderLogin))
                 {
-                    qDebug() << "Failed to get sender login";
+                    qCDebug(chatterinoPubsub) << "Failed to get sender login";
                     return;
                 }
-                action.target = ActionUser{sender_id, sender_login};
+                QString senderDisplayName = senderLogin;
-                qDebug() << action.msgID;
+                bool hasLocalizedName = false;
+                if (rj::getSafe(senderData, "display_name", senderDisplayName))
+                {
+                    // check for non-ascii display names
+                    if (QString::compare(senderLogin, senderDisplayName,
+                                         Qt::CaseInsensitive) != 0)
+                    {
+                        hasLocalizedName = true;
+                    }
+                }
+                QColor senderColor;
+                QString senderColor_;
+                if (rj::getSafe(senderData, "chat_color", senderColor_))
+                {
+                    senderColor = QColor(senderColor_);
+                }
+                else if (getSettings()->colorizeNicknames)
+                {
+                    // color may be not present if user is a grey-name
+                    senderColor = getRandomColor(senderId);
+                }
+                // handle username style based on prefered setting
+                switch (getSettings()->usernameDisplayMode.getValue())
+                {
+                    case UsernameDisplayMode::Username: {
+                        if (hasLocalizedName)
+                        {
+                            senderDisplayName = senderLogin;
+                        }
+                        break;
+                    }
+                    case UsernameDisplayMode::LocalizedName: {
+                        break;
+                    }
+                    case UsernameDisplayMode::UsernameAndLocalizedName: {
+                        if (hasLocalizedName)
+                        {
+                            senderDisplayName = QString("%1(%2)").arg(
+                                senderLogin, senderDisplayName);
+                        }
+                        break;
+                    }
+                }
+
+                action.target = ActionUser{senderId, senderLogin,
+                                           senderDisplayName, senderColor};
                 this->signals_.moderation.automodMessage.invoke(action);
             }
             // "ALLOWED" and "DENIED" statuses remain unimplemented

src/providers/twitch/PubsubHelpers.cpp

@@ -37,7 +37,7 @@ const rapidjson::Value &getMsgID(const rapidjson::Value &data)
 
 bool getCreatedByUser(const rapidjson::Value &data, ActionUser &user)
 {
-    return rj::getSafe(data, "created_by", user.name) &&
+    return rj::getSafe(data, "created_by", user.login) &&
            rj::getSafe(data, "created_by_user_id", user.id);
 }
 
@@ -48,7 +48,7 @@ bool getTargetUser(const rapidjson::Value &data, ActionUser &user)
 
 bool getTargetUserName(const rapidjson::Value &data, ActionUser &user)
 {
-    return rj::getSafe(data, "target_user_login", user.name);
+    return rj::getSafe(data, "target_user_login", user.login);
 }
 
 rapidjson::Document createListenMessage(const std::vector<QString> &topicsVec,

src/providers/twitch/TwitchMessageBuilder.cpp

@@ -1416,17 +1416,19 @@ void TwitchMessageBuilder::deletionMessage(const DeleteAction &action,
     builder->message().flags.set(MessageFlag::Timeout);
 
     builder
-        ->emplace<TextElement>(action.source.name, MessageElementFlag::Username,
+        ->emplace<TextElement>(action.source.login,
+                               MessageElementFlag::Username,
                                MessageColor::System, FontStyle::ChatMediumBold)
-        ->setLink({Link::UserInfo, action.source.name});
+        ->setLink({Link::UserInfo, action.source.login});
     // TODO(mm2pl): If or when jumping to a single message gets implemented a link,
     // add a link to the originalMessage
     builder->emplace<TextElement>(
         "deleted message from", MessageElementFlag::Text, MessageColor::System);
     builder
-        ->emplace<TextElement>(action.target.name, MessageElementFlag::Username,
+        ->emplace<TextElement>(action.target.login,
+                               MessageElementFlag::Username,
                                MessageColor::System, FontStyle::ChatMediumBold)
-        ->setLink({Link::UserInfo, action.target.name});
+        ->setLink({Link::UserInfo, action.target.login});
     builder->emplace<TextElement>("saying:", MessageElementFlag::Text,
                                   MessageColor::System);
     if (action.messageText.length() > 50)