Rework ActionsHandler

Rework ActionsHandler as static helper functions.

The functions are now invoked from ChatBarCache so there is no need to pass an actions handler object around qml simplifying the code.

autotests/actionshandlertest.cpp

@@ -14,7 +14,6 @@ class ActionsHandlerTest : public QObject
 
 private:
     Quotient::Connection *connection = Quotient::Connection::makeMockConnection(QStringLiteral("@bob:kde.org"));
-    ActionsHandler *actionsHandler = new ActionsHandler(this);
 
 private Q_SLOTS:
     void nullObject();
@@ -23,20 +22,19 @@ private Q_SLOTS:
 void ActionsHandlerTest::nullObject()
 {
     QTest::ignoreMessage(QtWarningMsg, "ActionsHandler::handleMessageEvent - called with m_room and/or chatBarCache set to nullptr.");
-    actionsHandler->handleMessageEvent(nullptr);
+    ActionsHandler::handleMessageEvent(nullptr, nullptr);
 
     auto chatBarCache = new ChatBarCache(this);
     QTest::ignoreMessage(QtWarningMsg, "ActionsHandler::handleMessageEvent - called with m_room and/or chatBarCache set to nullptr.");
-    actionsHandler->handleMessageEvent(chatBarCache);
+    ActionsHandler::handleMessageEvent(nullptr, chatBarCache);
 
     auto room = new TestUtils::TestRoom(connection, QStringLiteral("#myroom:kde.org"));
-    actionsHandler->setRoom(room);
     QTest::ignoreMessage(QtWarningMsg, "ActionsHandler::handleMessageEvent - called with m_room and/or chatBarCache set to nullptr.");
-    actionsHandler->handleMessageEvent(nullptr);
+    ActionsHandler::handleMessageEvent(room, nullptr);
 
     // The final one should throw no warning so we make sure.
     QTest::failOnWarning("ActionsHandler::handleMessageEvent - called with m_room and/or chatBarCache set to nullptr.");
-    actionsHandler->handleMessageEvent(chatBarCache);
+    ActionsHandler::handleMessageEvent(room, chatBarCache);
 }
 
 QTEST_GUILESS_MAIN(ActionsHandlerTest)

src/actionshandler.cpp

@@ -1,70 +1,48 @@
 // SPDX-FileCopyrightText: 2020 Carl Schwan <carlschwan@kde.org>
+// SPDX-FileCopyrightText: 2024 James Graham <james.h.graham@protonmail.com>
 // SPDX-License-Identifier: GPL-3.0-or-later
 
 #include "actionshandler.h"
 
-#include <Quotient/csapi/joining.h>
+#include "chatbarcache.h"
-#include <Quotient/events/roommemberevent.h>
-
-#include <cmark.h>
-
-#include <KLocalizedString>
-#include <QStringBuilder>
-
 #include "models/actionsmodel.h"
 #include "neochatconfig.h"
 #include "texthandler.h"
 
 using namespace Quotient;
+using namespace Qt::StringLiterals;
 
-ActionsHandler::ActionsHandler(QObject *parent)
+void ActionsHandler::handleMessageEvent(NeoChatRoom *room, ChatBarCache *chatBarCache)
-    : QObject(parent)
 {
-}
+    if (room == nullptr || chatBarCache == nullptr) {
-
-NeoChatRoom *ActionsHandler::room() const
-{
-    return m_room;
-}
-
-void ActionsHandler::setRoom(NeoChatRoom *room)
-{
-    if (m_room == room) {
-        return;
-    }
-
-    m_room = room;
-    Q_EMIT roomChanged();
-}
-
-void ActionsHandler::handleMessageEvent(ChatBarCache *chatBarCache)
-{
-    if (!m_room || !chatBarCache) {
         qWarning() << "ActionsHandler::handleMessageEvent - called with m_room and/or chatBarCache set to nullptr.";
         return;
     }
 
-    checkEffects(chatBarCache->text());
     if (!chatBarCache->attachmentPath().isEmpty()) {
         QUrl url(chatBarCache->attachmentPath());
         auto path = url.isLocalFile() ? url.toLocalFile() : url.toString();
-        m_room->uploadFile(QUrl(path), chatBarCache->text().isEmpty() ? path.mid(path.lastIndexOf(u'/') + 1) : chatBarCache->text());
+        room->uploadFile(QUrl(path), chatBarCache->text().isEmpty() ? path.mid(path.lastIndexOf(u'/') + 1) : chatBarCache->text());
         chatBarCache->setAttachmentPath({});
         chatBarCache->setText({});
         return;
     }
 
-    QString handledText = chatBarCache->text();
+    const auto handledText = handleMentions(chatBarCache);
-    handledText = handleMentions(handledText, chatBarCache->mentions());
+    const auto result = handleQuickEdit(room, handledText);
-    handleMessage(chatBarCache->text(), handledText, chatBarCache);
+    if (!result) {
+        handleMessage(room, handledText, chatBarCache);
+    }
 }
 
-QString ActionsHandler::handleMentions(QString handledText, QList<Mention> *mentions)
+QString ActionsHandler::handleMentions(ChatBarCache *chatBarCache)
 {
+    const auto mentions = chatBarCache->mentions();
     std::sort(mentions->begin(), mentions->end(), [](const auto &a, const auto &b) -> bool {
         return a.cursor.anchor() > b.cursor.anchor();
     });
 
+    auto handledText = chatBarCache->text();
     for (const auto &mention : *mentions) {
         if (mention.text.isEmpty() || mention.id.isEmpty()) {
             continue;
@@ -78,48 +56,60 @@ QString ActionsHandler::handleMentions(QString handledText, QList<Mention> *ment
     return handledText;
 }
 
-void ActionsHandler::handleMessage(const QString &text, QString handledText, ChatBarCache *chatBarCache)
+bool ActionsHandler::handleQuickEdit(NeoChatRoom *room, const QString &handledText)
 {
-    Q_ASSERT(m_room);
+    if (room == nullptr) {
+        return false;
+    }
+
     if (NeoChatConfig::allowQuickEdit()) {
         QRegularExpression sed(QStringLiteral("^s/([^/]*)/([^/]*)(/g)?$"));
-        auto match = sed.match(text);
+        auto match = sed.match(handledText);
         if (match.hasMatch()) {
             const QString regex = match.captured(1);
             const QString replacement = match.captured(2).toHtmlEscaped();
             const QString flags = match.captured(3);
 
-            for (auto it = m_room->messageEvents().crbegin(); it != m_room->messageEvents().crend(); it++) {
+            for (auto it = room->messageEvents().crbegin(); it != room->messageEvents().crend(); it++) {
                 if (const auto event = eventCast<const RoomMessageEvent>(&**it)) {
-                    if (event->senderId() == m_room->localMember().id() && event->hasTextContent()) {
+                    if (event->senderId() == room->localMember().id() && event->hasTextContent()) {
                         QString originalString;
                         if (event->content()) {
                             originalString = static_cast<const Quotient::EventContent::TextContent *>(event->content())->body;
                         } else {
                             originalString = event->plainBody();
                         }
-                        if (flags == "/g"_ls) {
+                        if (flags == "/g"_L1) {
-                            m_room->postHtmlMessage(handledText, originalString.replace(regex, replacement), event->msgtype(), {}, event->id());
+                            room->postHtmlMessage(handledText, originalString.replace(regex, replacement), event->msgtype(), {}, event->id());
                         } else {
-                            m_room->postHtmlMessage(handledText,
+                            room->postHtmlMessage(handledText,
-                                                    originalString.replace(originalString.indexOf(regex), regex.size(), replacement),
+                                                  originalString.replace(originalString.indexOf(regex), regex.size(), replacement),
-                                                    event->msgtype(),
+                                                  event->msgtype(),
-                                                    {},
+                                                  {},
-                                                    event->id());
+                                                  event->id());
                         }
-                        return;
+                        return true;
                     }
                 }
             }
         }
     }
+    return false;
+}
+
+void ActionsHandler::handleMessage(NeoChatRoom *room, QString handledText, ChatBarCache *chatBarCache)
+{
+    if (room == nullptr) {
+        return;
+    }
+
     auto messageType = RoomMessageEvent::MsgType::Text;
 
     if (handledText.startsWith(QLatin1Char('/'))) {
         for (const auto &action : ActionsModel::instance().allActions()) {
             if (handledText.indexOf(action.prefix) == 1
                 && (handledText.indexOf(" "_ls) == action.prefix.length() + 1 || handledText.length() == action.prefix.length() + 1)) {
-                handledText = action.handle(handledText.mid(action.prefix.length() + 1).trimmed(), m_room, chatBarCache);
+                handledText = action.handle(handledText.mid(action.prefix.length() + 1).trimmed(), room, chatBarCache);
                 if (action.messageType.has_value()) {
                     messageType = *action.messageType;
                 }
@@ -145,26 +135,7 @@ void ActionsHandler::handleMessage(const QString &text, QString handledText, Cha
         return;
     }
 
-    m_room->postMessage(text, handledText, messageType, chatBarCache->replyId(), chatBarCache->editId(), chatBarCache->threadId());
+    room->postMessage(chatBarCache->text(), handledText, messageType, chatBarCache->replyId(), chatBarCache->editId(), chatBarCache->threadId());
-}
-
-void ActionsHandler::checkEffects(const QString &text)
-{
-    std::optional<QString> effect = std::nullopt;
-    if (text.contains(QStringLiteral("\u2744"))) {
-        effect = QLatin1String("snowflake");
-    } else if (text.contains(QStringLiteral("\u1F386"))) {
-        effect = QLatin1String("fireworks");
-    } else if (text.contains(QStringLiteral("\u2F387"))) {
-        effect = QLatin1String("fireworks");
-    } else if (text.contains(QStringLiteral("\u1F389"))) {
-        effect = QLatin1String("confetti");
-    } else if (text.contains(QStringLiteral("\u1F38A"))) {
-        effect = QLatin1String("confetti");
-    }
-    if (effect.has_value()) {
-        Q_EMIT showEffect(*effect);
-    }
 }
 
 #include "moc_actionshandler.cpp"

src/actionshandler.h

@@ -1,22 +1,18 @@
 // SPDX-FileCopyrightText: 2020 Carl Schwan <carlschwan@kde.org>
+// SPDX-FileCopyrightText: 2024 James Graham <james.h.graham@protonmail.com>
 // SPDX-License-Identifier: GPL-3.0-or-later
 
 #pragma once
 
-#include <QObject>
+#include <QString>
-#include <QQmlEngine>
-
-#include <Quotient/events/roommessageevent.h>
-
-#include "chatbarcache.h"
-#include "neochatroom.h"
 
+class ChatBarCache;
 class NeoChatRoom;
 
 /**
  * @class ActionsHandler
  *
- * This class handles chat messages ready for posting to a room.
+ * This class contains functions to handle chat messages ready for posting to a room.
  *
  * Everything that needs to be done to prepare the message for posting in a room
  * including:
@@ -31,36 +27,17 @@ class NeoChatRoom;
  *
  * @sa ActionsModel, NeoChatRoom
  */
-class ActionsHandler : public QObject
+class ActionsHandler
 {
-    Q_OBJECT
-    QML_ELEMENT
-
-    /**
-     * @brief The room that messages will be sent to.
-     */
-    Q_PROPERTY(NeoChatRoom *room READ room WRITE setRoom NOTIFY roomChanged)
-
 public:
-    explicit ActionsHandler(QObject *parent = nullptr);
-
-    [[nodiscard]] NeoChatRoom *room() const;
-    void setRoom(NeoChatRoom *room);
-
-Q_SIGNALS:
-    void roomChanged();
-    void showEffect(const QString &effect);
-
-public Q_SLOTS:
     /**
      * @brief Pre-process text and send message event.
      */
-    void handleMessageEvent(ChatBarCache *chatBarCache);
+    static void handleMessageEvent(NeoChatRoom *room, ChatBarCache *chatBarCache);
 
 private:
-    QPointer<NeoChatRoom> m_room;
+    static QString handleMentions(ChatBarCache *chatBarCache);
-    void checkEffects(const QString &text);
+    static bool handleQuickEdit(NeoChatRoom *room, const QString &handledText);
 
-    QString handleMentions(QString handledText, QList<Mention> *mentions);
+    static void handleMessage(NeoChatRoom *room, QString handledText, ChatBarCache *chatBarCache);
-    void handleMessage(const QString &text, QString handledText, ChatBarCache *chatBarCache);
 };

src/chatbar/ChatBar.qml

@@ -53,14 +53,6 @@ QQC2.Control {
         }
     }
 
-    /**
-     * @brief The ActionsHandler object to use.
-     *
-     * This is expected to have the correct room set otherwise messages will be sent
-     * to the wrong room.
-     */
-    required property ActionsHandler actionsHandler
-
     /**
      * @brief The list of actions in the ChatBar.
      *
@@ -409,7 +401,7 @@ QQC2.Control {
         onChatBarCacheChanged: documentHandler.chatBarCache = chatBarCache
 
         function postMessage() {
-            root.actionsHandler.handleMessageEvent(_private.chatBarCache);
+            _private.chatBarCache.postMessage();
             repeatTimer.stop();
             root.currentRoom.markAllMessagesAsRead();
             textField.clear();

src/chatbarcache.cpp

@@ -5,6 +5,7 @@
 
 #include <Quotient/roommember.h>
 
+#include "actionshandler.h"
 #include "chatdocumenthandler.h"
 #include "eventhandler.h"
 #include "neochatroom.h"
@@ -259,4 +260,15 @@ void ChatBarCache::setSavedText(const QString &savedText)
     m_savedText = savedText;
 }
 
+void ChatBarCache::postMessage()
+{
+    auto room = dynamic_cast<NeoChatRoom *>(parent());
+    if (room == nullptr) {
+        qWarning() << "ChatBarCache created with incorrect parent, a NeoChatRoom must be set as the parent on creation.";
+        return;
+    }
+
+    ActionsHandler::handleMessageEvent(room, this);
+}
+
 #include "moc_chatbarcache.cpp"

src/chatbarcache.h

@@ -202,6 +202,11 @@ public:
      */
     void setSavedText(const QString &savedText);
 
+    /**
+     * @brief Post the contents of the cache as a message in the room.
+     */
+    Q_INVOKABLE void postMessage();
+
 Q_SIGNALS:
     void textChanged();
     void relationIdChanged(const QString &oldEventId, const QString &newEventId);

src/qml/RoomPage.qml

@@ -60,13 +60,6 @@ Kirigami.Page {
      */
     property MediaMessageFilterModel mediaMessageFilterModel: RoomManager.mediaMessageFilterModel
 
-    /**
-     * @brief The ActionsHandler object to use.
-     */
-    property ActionsHandler actionsHandler: ActionsHandler {
-        room: root.currentRoom
-    }
-
     property bool loading: !root.currentRoom || (root.currentRoom.timelineSize === 0 && !root.currentRoom.allHistoryLoaded)
 
     /// Disable cancel shortcut. Used by the separate window since it provides its own cancel implementation.
@@ -123,7 +116,6 @@ Kirigami.Page {
             page: root
             timelineModel: root.timelineModel
             messageFilterModel: root.messageFilterModel
-            actionsHandler: root.actionsHandler
             onFocusChatBar: {
                 if (chatBarLoader.item) {
                     chatBarLoader.item.forceActiveFocus();
@@ -182,7 +174,6 @@ Kirigami.Page {
             width: parent.width
             currentRoom: root.currentRoom
             connection: root.connection
-            actionsHandler: root.actionsHandler
             onMessageSent: {
                 if (!timelineViewLoader.item.atYEnd) {
                     timelineViewLoader.item.goToLastMessage();

src/qml/TimelineView.qml

@@ -43,14 +43,6 @@ QQC2.ScrollView {
      */
     required property MessageFilterModel messageFilterModel
 
-    /**
-     * @brief The ActionsHandler object to use.
-     *
-     * This is expected to have the correct room set otherwise messages will be sent
-     * to the wrong room.
-     */
-    required property ActionsHandler actionsHandler
-
     readonly property bool atYEnd: messageListView.atYEnd
 
     property alias interactive: messageListView.interactive
@@ -64,8 +56,6 @@ QQC2.ScrollView {
 
     ListView {
         id: messageListView
-        // So that delegates can access the actionsHandler properly.
-        readonly property ActionsHandler actionsHandler: root.actionsHandler
 
         readonly property int largestVisibleIndex: count > 0 ? indexAt(contentX + (width / 2), contentY + height - 1) : -1
         readonly property var sectionBannerItem: contentHeight >= height ? itemAtIndex(sectionBannerIndex()) : undefined

src/timeline/Bubble.qml

@@ -51,14 +51,6 @@ QQC2.Control {
      */
     required property var contentModel
 
-    /**
-     * @brief The ActionsHandler object to use.
-     *
-     * This is expected to have the correct room set otherwise messages will be sent
-     * to the wrong room.
-     */
-    property ActionsHandler actionsHandler
-
     /**
      * @brief Whether the bubble background should be shown.
      */
@@ -107,7 +99,6 @@ QQC2.Control {
                 delegate: MessageComponentChooser {
                     room: root.room
                     index: root.index
-                    actionsHandler: root.actionsHandler
                     timeline: root.timeline
                     maxContentWidth: root.maxContentWidth
 

src/timeline/ChatBarComponent.qml

@@ -27,14 +27,6 @@ QQC2.TextArea {
     required property ChatBarCache chatBarCache
     onChatBarCacheChanged: documentHandler.chatBarCache = chatBarCache
 
-    /**
-     * @brief The ActionsHandler object to use.
-     *
-     * This is expected to have the correct room set otherwise messages will be sent
-     * to the wrong room.
-     */
-    required property ActionsHandler actionsHandler
-
     /**
      * @brief The maximum width that the bubble's content can be.
      */
@@ -177,7 +169,7 @@ QQC2.TextArea {
         }
 
         function post() {
-            root.actionsHandler.handleMessageEvent(root.chatBarCache);
+            root.chatBarCache.postMessage();
             root.clear();
             root.chatBarCache.clearRelations();
         }

src/timeline/MessageComponentChooser.qml

@@ -22,14 +22,6 @@ DelegateChooser {
      */
     required property var index
 
-    /**
-     * @brief The ActionsHandler object to use.
-     *
-     * This is expected to have the correct room set otherwise messages will be sent
-     * to the wrong room.
-     */
-    required property ActionsHandler actionsHandler
-
     /**
      * @brief The timeline ListView this component is being used in.
      */
@@ -208,7 +200,6 @@ DelegateChooser {
         roleValue: MessageComponentType.ChatBar
         delegate: ChatBarComponent {
             room: root.room
-            actionsHandler: root.actionsHandler
             maxContentWidth: root.maxContentWidth
         }
     }

src/timeline/MessageDelegate.qml

@@ -295,7 +295,6 @@ TimelineDelegate {
                 } else {
                    return root.contentModel;
                 }
-                actionsHandler: root.ListView.view?.actionsHandler ?? null
                 timeline: root.ListView.view
 
                 showHighlight: root.showHighlight