diff --git a/autotests/CMakeLists.txt b/autotests/CMakeLists.txt index c2efaf2e7..0a52904af 100644 --- a/autotests/CMakeLists.txt +++ b/autotests/CMakeLists.txt @@ -70,3 +70,9 @@ ecm_add_test( LINK_LIBRARIES neochat Qt::Test TEST_NAME pollhandlertest ) + +ecm_add_test( + reactionmodeltest.cpp + LINK_LIBRARIES neochat Qt::Test + TEST_NAME reactionmodeltest +) diff --git a/autotests/data/test-reactionmodel-extra-sync.json b/autotests/data/test-reactionmodel-extra-sync.json new file mode 100644 index 000000000..8158ca03d --- /dev/null +++ b/autotests/data/test-reactionmodel-extra-sync.json @@ -0,0 +1,44 @@ +{ + "timeline": { + "events": [ + { + "content": { + "m.relates_to": { + "event_id": "$153456789:example.org", + "key": "👍", + "rel_type": "m.annotation" + } + }, + "origin_server_ts": 1690322545183, + "room_id": "!jEsUZKDJdhlrceRyVU:example.org", + "sender": "@bob:example.org", + "type": "m.reaction", + "unsigned": { + "age": 390159121 + }, + "event_id": "$163456790:example.org", + "age": 390159121 + }, + { + "content": { + "m.relates_to": { + "event_id": "$153456789:example.org", + "key": "😆", + "rel_type": "m.annotation" + } + }, + "origin_server_ts": 1690322545184, + "room_id": "!jEsUZKDJdhlrceRyVU:example.org", + "sender": "@bob:example.org", + "type": "m.reaction", + "unsigned": { + "age": 390159122 + }, + "event_id": "$163456791:example.org", + "age": 390159122 + } + ], + "limited": true, + "prev_batch": "t34-23535_0_0" + } +} diff --git a/autotests/data/test-reactionmodel-sync.json b/autotests/data/test-reactionmodel-sync.json new file mode 100644 index 000000000..5d592b839 --- /dev/null +++ b/autotests/data/test-reactionmodel-sync.json @@ -0,0 +1,204 @@ +{ + "account_data": { + "events": [ + { + "content": { + "tags": { + "u.work": { + "order": 0.9 + } + } + }, + "type": "m.tag" + }, + { + "content": { + "custom_config_key": "custom_config_value" + }, + "type": "org.example.custom.room.config" + } + ] + }, + "ephemeral": { + "events": [ + { + "content": { + "user_ids": [ + "@alice:matrix.org", + "@bob:example.com" + ] + }, + "room_id": "!jEsUZKDJdhlrceRyVU:example.org", + "type": "m.typing" + }, + { + "content": { + "$153456789:example.org": { + "m.read": { + "@alice:matrix.org": { + "ts": 1436451550453 + } + } + } + }, + "type": "m.receipt" + }, + { + "content": { + "$1532735824654:example.org": { + "m.read": { + "@bob:example.com": { + "ts": 1436451550453 + } + } + } + }, + "type": "m.receipt" + }, + { + "content": { + "$1532735824654:example.org": { + "m.read": { + "@tim:example.com": { + "ts": 1436451550454 + } + } + } + }, + "type": "m.receipt" + }, + { + "content": { + "$1532735824654:example.org": { + "m.read": { + "@jeff:example.com": { + "ts": 1436451550455 + } + } + } + }, + "type": "m.receipt" + }, + { + "content": { + "$1532735824654:example.org": { + "m.read": { + "@tina:example.com": { + "ts": 1436451550456 + } + } + } + }, + "type": "m.receipt" + }, + { + "content": { + "$1532735824654:example.org": { + "m.read": { + "@sally:example.com": { + "ts": 1436451550457 + } + } + } + }, + "type": "m.receipt" + }, + { + "content": { + "$1532735824654:example.org": { + "m.read": { + "@fred:example.com": { + "ts": 1436451550458 + } + } + } + }, + "type": "m.receipt" + } + ] + }, + "state": { + "events": [ + { + "content": { + "avatar_url": "mxc://example.org/SEsfnsuifSDFSSEF", + "displayname": "Alice Margatroid", + "membership": "join", + "reason": "Looking for support" + }, + "event_id": "$143273582443PhrSn:example.org", + "origin_server_ts": 1432735824653, + "room_id": "!jEsUZKDJdhlrceRyVU:example.org", + "sender": "@example:example.org", + "state_key": "@alice:example.org", + "type": "m.room.member", + "unsigned": { + "age": 1234 + } + }, + { + "content": { + "displayname": "Look\nat\nme\nI\nput\nnewlines\nin\nmy\ndisplay name", + "membership": "join" + }, + "event_id": "$143273582443PhrSh:example.org", + "origin_server_ts": 1432735824659, + "room_id": "!jEsUZKDJdhlrceRyVU:example.org", + "sender": "@newline:example.org", + "state_key": "@newline:example.org", + "type": "m.room.member", + "unsigned": { + "age": 12345 + } + } + ] + }, + "summary": { + "m.heroes": [ + "@alice:example.com", + "@bob:example.com" + ], + "m.invited_member_count": 0, + "m.joined_member_count": 2 + }, + "timeline": { + "events": [ + { + "content": { + "body": "This is an example\ntext message", + "format": "org.matrix.custom.html", + "formatted_body": "This is an example
text message", + "msgtype": "m.text" + }, + "event_id": "$153456789:example.org", + "origin_server_ts": 1432735824654, + "room_id": "!jEsUZKDJdhlrceRyVU:example.org", + "sender": "@example:example.org", + "type": "m.room.message", + "unsigned": { + "age": 1232 + } + }, + { + "content": { + "m.relates_to": { + "event_id": "$153456789:example.org", + "key": "👍", + "rel_type": "m.annotation" + } + }, + "origin_server_ts": 1690322545182, + "room_id": "!jEsUZKDJdhlrceRyVU:example.org", + "sender": "@alice:matrix.org", + "type": "m.reaction", + "unsigned": { + "age": 390159120 + }, + "event_id": "$163456789:example.org", + "age": 390159120 + } + ], + "limited": true, + "prev_batch": "t34-23535_0_0" + } +} diff --git a/autotests/eventhandlertest.cpp b/autotests/eventhandlertest.cpp index 940c7e396..43c58e9d7 100644 --- a/autotests/eventhandlertest.cpp +++ b/autotests/eventhandlertest.cpp @@ -67,8 +67,6 @@ private Q_SLOTS: void nullMediaInfo(); void linkPreviewer(); void nullLinkPreviewer(); - void reactions(); - void nullReactions(); void hasReply(); void nullHasReply(); void replyId(); @@ -423,23 +421,6 @@ void EventHandlerTest::nullLinkPreviewer() QCOMPARE(noEventHandler.getLinkPreviewer(), nullptr); } -void EventHandlerTest::reactions() -{ - auto event = room->messageEvents().at(0).get(); - eventHandler.setEvent(event); - - QCOMPARE(eventHandler.getReactions()->rowCount(), 1); -} - -void EventHandlerTest::nullReactions() -{ - QTest::ignoreMessage(QtWarningMsg, "getReactions called with m_room set to nullptr."); - QCOMPARE(emptyHandler.getReactions(), nullptr); - - QTest::ignoreMessage(QtWarningMsg, "getReactions called with m_event set to nullptr."); - QCOMPARE(noEventHandler.getReactions(), nullptr); -} - void EventHandlerTest::hasReply() { auto event = room->messageEvents().at(5).get(); diff --git a/autotests/reactionmodeltest.cpp b/autotests/reactionmodeltest.cpp new file mode 100644 index 000000000..e807ae783 --- /dev/null +++ b/autotests/reactionmodeltest.cpp @@ -0,0 +1,83 @@ +// SPDX-FileCopyrightText: 2023 James Graham +// SPDX-License-Identifier: GPL-2.0-only OR GPL-3.0-only OR LicenseRef-KDE-Accepted-GPL + +#include +#include +#include + +#include "models/reactionmodel.h" + +#include + +#include "testutils.h" + +using namespace Quotient; + +class ReactionModelTest : public QObject +{ + Q_OBJECT + +private: + Connection *connection = nullptr; + TestUtils::TestRoom *room = nullptr; + +private Q_SLOTS: + void initTestCase(); + + void nullModel(); + void basicReaction(); + void newReaction(); +}; + +void ReactionModelTest::initTestCase() +{ + connection = Connection::makeMockConnection(QStringLiteral("@bob:kde.org")); + room = new TestUtils::TestRoom(connection, QStringLiteral("#myroom:kde.org"), QLatin1String("test-reactionmodel-sync.json")); +} + +void ReactionModelTest::nullModel() +{ + auto model = ReactionModel(nullptr, nullptr); + + QCOMPARE(model.rowCount(), 0); + QCOMPARE(model.data(model.index(0), ReactionModel::TextContentRole), QVariant()); +} + +void ReactionModelTest::basicReaction() +{ + auto event = eventCast(room->messageEvents().at(0).get()); + auto model = ReactionModel(event, room); + + QCOMPARE(model.rowCount(), 1); + QCOMPARE(model.data(model.index(0), ReactionModel::TextContentRole), QStringLiteral("👍")); + QCOMPARE(model.data(model.index(0), ReactionModel::ReactionRole), QStringLiteral("👍")); + QCOMPARE(model.data(model.index(0), ReactionModel::ToolTipRole), + QStringLiteral("@alice:matrix.org reacted with 👍")); + auto authorList = QVariantList{room->getUser(room->user(QStringLiteral("@alice:matrix.org")))}; + QCOMPARE(model.data(model.index(0), ReactionModel::AuthorsRole), authorList); + QCOMPARE(model.data(model.index(0), ReactionModel::HasLocalUser), false); +} + +void ReactionModelTest::newReaction() +{ + auto event = eventCast(room->messageEvents().at(0).get()); + auto model = new ReactionModel(event, room); + + QCOMPARE(model->rowCount(), 1); + QCOMPARE(model->data(model->index(0), ReactionModel::ToolTipRole), + QStringLiteral("@alice:matrix.org reacted with 👍")); + + QSignalSpy spy(model, SIGNAL(modelReset())); + + room->syncNewEvents(QLatin1String("test-reactionmodel-extra-sync.json")); + QCOMPARE(model->rowCount(), 2); + QCOMPARE(spy.count(), 2); // Once for each of the 2 new reactions. + QCOMPARE(model->data(model->index(1), ReactionModel::ReactionRole), QStringLiteral("😆")); + QCOMPARE(model->data(model->index(0), ReactionModel::ToolTipRole), + QStringLiteral("@alice:matrix.org and @bob:example.org reacted with 👍")); + + delete model; +} + +QTEST_MAIN(ReactionModelTest) +#include "reactionmodeltest.moc" diff --git a/src/eventhandler.cpp b/src/eventhandler.cpp index 2ab0898e0..9fcfdea43 100644 --- a/src/eventhandler.cpp +++ b/src/eventhandler.cpp @@ -811,60 +811,6 @@ QSharedPointer EventHandler::getLinkPreviewer() const } } -QSharedPointer EventHandler::getReactions() const -{ - if (m_room == nullptr) { - qCWarning(EventHandling) << "getReactions called with m_room set to nullptr."; - return nullptr; - } - if (m_event == nullptr) { - qCWarning(EventHandling) << "getReactions called with m_event set to nullptr."; - return nullptr; - } - if (!m_event->is()) { - qCWarning(EventHandling) << "getReactions called with on a non-message event."; - return nullptr; - } - - auto eventId = m_event->id(); - const auto &annotations = m_room->relatedEvents(eventId, EventRelation::AnnotationType); - if (annotations.isEmpty()) { - return nullptr; - }; - - QMap> reactions = {}; - for (const auto &a : annotations) { - if (a->isRedacted()) { // Just in case? - continue; - } - if (const auto &e = eventCast(a)) { - reactions[e->key()].append(m_room->user(e->senderId())); - } - } - - if (reactions.isEmpty()) { - return nullptr; - } - - QList res; - auto i = reactions.constBegin(); - while (i != reactions.constEnd()) { - QVariantList authors; - for (const auto &author : i.value()) { - authors.append(m_room->getUser(author)); - } - - res.append(ReactionModel::Reaction{i.key(), authors}); - ++i; - } - - if (res.size() > 0) { - return QSharedPointer(new ReactionModel(nullptr, res, m_room->localUser())); - } else { - return nullptr; - } -} - bool EventHandler::hasReply() const { if (m_event == nullptr) { diff --git a/src/eventhandler.h b/src/eventhandler.h index 5206edff4..93aa792da 100644 --- a/src/eventhandler.h +++ b/src/eventhandler.h @@ -241,15 +241,6 @@ public: */ QSharedPointer getLinkPreviewer() const; - /** - * @brief Return a ReactionModel object for the event. - * - * A nullptr will be returned for any event that doesn't have any links so the - * return should be null checked and an empty QVariantList (or other suitable - * empty mode) provided if null. - */ - QSharedPointer getReactions() const; - /** * @brief Whether the event is a reply to another in the timeline. */ diff --git a/src/models/messageeventmodel.cpp b/src/models/messageeventmodel.cpp index 2dbe85b56..0e6e48f31 100644 --- a/src/models/messageeventmodel.cpp +++ b/src/models/messageeventmodel.cpp @@ -72,6 +72,12 @@ QHash MessageEventModel::roleNames() const MessageEventModel::MessageEventModel(QObject *parent) : QAbstractListModel(parent) { + connect(this, &MessageEventModel::modelAboutToBeReset, this, [this]() { + resetting = true; + }); + connect(this, &MessageEventModel::modelReset, this, [this]() { + resetting = false; + }); } NeoChatRoom *MessageEventModel::room() const @@ -245,7 +251,7 @@ void MessageEventModel::setRoom(NeoChatRoom *room) m_currentRoom->createPollHandler(eventCast(eventIt->event())); } } - refreshEventRoles(eventId, {ReactionRole, ShowReactionsRole, Qt::DisplayRole}); + refreshEventRoles(eventId, {Qt::DisplayRole}); }); connect(m_currentRoom, &Room::changed, this, [this]() { for (auto it = m_currentRoom->messageEvents().rbegin(); it != m_currentRoom->messageEvents().rend(); ++it) { @@ -723,10 +729,28 @@ void MessageEventModel::createEventObjects(const Quotient::RoomMessageEvent *eve } else { m_linkPreviewers.remove(eventId); } - if (auto reactionModel = eventHandler.getReactions()) { - m_reactionModels[eventId] = reactionModel; + + // ReactionModel handles updates to add and remove reactions, we only need to + // handle adding and removing whole models here. + if (m_reactionModels.contains(eventId)) { + // If a model already exists but now has no reactions remove it + if (m_reactionModels[eventId]->rowCount() <= 0) { + m_reactionModels.remove(eventId); + if (!resetting) { + refreshEventRoles(eventId, {ReactionRole, ShowReactionsRole}); + } + } } else { - m_reactionModels.remove(eventId); + if (m_currentRoom->relatedEvents(*event, Quotient::EventRelation::AnnotationType).count() > 0) { + // If a model doesn't exist and there are reactions add it. + auto reactionModel = QSharedPointer(new ReactionModel(event, m_currentRoom)); + if (reactionModel->rowCount() > 0) { + m_reactionModels[eventId] = reactionModel; + if (!resetting) { + refreshEventRoles(eventId, {ReactionRole, ShowReactionsRole}); + } + } + } } } diff --git a/src/models/messageeventmodel.h b/src/models/messageeventmodel.h index 2a0ef9808..2b0c72e65 100644 --- a/src/models/messageeventmodel.h +++ b/src/models/messageeventmodel.h @@ -131,6 +131,7 @@ private: QString lastReadEventId; QPersistentModelIndex m_lastReadEventIndex; int rowBelowInserted = -1; + bool resetting = false; bool movingEvent = false; KFormat m_format; diff --git a/src/models/reactionmodel.cpp b/src/models/reactionmodel.cpp index 30bc0287e..87107d3ce 100644 --- a/src/models/reactionmodel.cpp +++ b/src/models/reactionmodel.cpp @@ -2,6 +2,7 @@ // SPDX-License-Identifier: GPL-2.0-only OR GPL-3.0-only OR LicenseRef-KDE-Accepted-GPL #include "reactionmodel.h" +#include "neochatroom.h" #include #ifdef HAVE_ICU @@ -15,11 +16,20 @@ #include -ReactionModel::ReactionModel(QObject *parent, QList reactions, Quotient::User *localUser) - : QAbstractListModel(parent) - , m_localUser(localUser) +ReactionModel::ReactionModel(const Quotient::RoomMessageEvent *event, const NeoChatRoom *room) + : QAbstractListModel(nullptr) + , m_room(room) + , m_event(event) { - setReactions(reactions); + if (m_event != nullptr && m_room != nullptr) { + connect(m_room, &NeoChatRoom::updatedEvent, this, [this](const QString &eventId) { + if (m_event->id() == eventId) { + updateReactions(); + } + }); + + updateReactions(); + } } QVariant ReactionModel::data(const QModelIndex &index, int role) const @@ -35,38 +45,11 @@ QVariant ReactionModel::data(const QModelIndex &index, int role) const const auto &reaction = m_reactions.at(index.row()); - const auto isEmoji = [](const QString &text) { -#ifdef HAVE_ICU - QTextBoundaryFinder finder(QTextBoundaryFinder::Grapheme, text); - int from = 0; - while (finder.toNextBoundary() != -1) { - auto to = finder.position(); - if (text[from].isSpace()) { - from = to; - continue; - } - - auto first = text.mid(from, to - from).toUcs4()[0]; - if (!u_hasBinaryProperty(first, UCHAR_EMOJI_PRESENTATION)) { - return false; - } - from = to; - } - return true; -#else - return false; -#endif - }; - - const auto reactionText = isEmoji(reaction.reaction) - ? QStringLiteral("") + reaction.reaction + QStringLiteral("") - : reaction.reaction; - if (role == TextContentRole) { if (reaction.authors.count() > 1) { - return QStringLiteral("%1 %2").arg(reactionText, QString::number(reaction.authors.count())); + return QStringLiteral("%1 %2").arg(reactionText(reaction.reaction), QString::number(reaction.authors.count())); } else { - return reactionText; + return reactionText(reaction.reaction); } } @@ -97,7 +80,7 @@ QVariant ReactionModel::data(const QModelIndex &index, int role) const "%2 reacted with %3", reaction.authors.count(), text, - reactionText); + reactionText(reaction.reaction)); return text; } @@ -107,7 +90,7 @@ QVariant ReactionModel::data(const QModelIndex &index, int role) const if (role == HasLocalUser) { for (auto author : reaction.authors) { - if (author.toMap()[QStringLiteral("id")] == m_localUser->id()) { + if (author.toMap()[QStringLiteral("id")] == m_room->localUser()->id()) { return true; } } @@ -123,11 +106,44 @@ int ReactionModel::rowCount(const QModelIndex &parent) const return m_reactions.count(); } -void ReactionModel::setReactions(QList reactions) +void ReactionModel::updateReactions() { beginResetModel(); + m_reactions.clear(); - m_reactions = reactions; + + const auto &annotations = m_room->relatedEvents(*m_event, Quotient::EventRelation::AnnotationType); + if (annotations.isEmpty()) { + endResetModel(); + return; + }; + + QMap> reactions = {}; + for (const auto &a : annotations) { + if (a->isRedacted()) { // Just in case? + continue; + } + if (const auto &e = eventCast(a)) { + reactions[e->key()].append(m_room->user(e->senderId())); + } + } + + if (reactions.isEmpty()) { + endResetModel(); + return; + } + + auto i = reactions.constBegin(); + while (i != reactions.constEnd()) { + QVariantList authors; + for (const auto &author : i.value()) { + authors.append(m_room->getUser(author)); + } + + m_reactions.append(ReactionModel::Reaction{i.key(), authors}); + ++i; + } + endResetModel(); } @@ -142,4 +158,32 @@ QHash ReactionModel::roleNames() const }; } +QString ReactionModel::reactionText(const QString &text) +{ + const auto isEmoji = [](const QString &text) { +#ifdef HAVE_ICU + QTextBoundaryFinder finder(QTextBoundaryFinder::Grapheme, text); + int from = 0; + while (finder.toNextBoundary() != -1) { + auto to = finder.position(); + if (text[from].isSpace()) { + from = to; + continue; + } + + auto first = text.mid(from, to - from).toUcs4()[0]; + if (!u_hasBinaryProperty(first, UCHAR_EMOJI_PRESENTATION)) { + return false; + } + from = to; + } + return true; +#else + return false; +#endif + }; + + return isEmoji(text) ? QStringLiteral("") + text + QStringLiteral("") : text; +} + #include "moc_reactionmodel.cpp" diff --git a/src/models/reactionmodel.h b/src/models/reactionmodel.h index 20b1d0780..52b3c13f6 100644 --- a/src/models/reactionmodel.h +++ b/src/models/reactionmodel.h @@ -3,8 +3,10 @@ #pragma once +#include "neochatroom.h" #include #include +#include namespace Quotient { @@ -41,7 +43,7 @@ public: HasLocalUser, /**< Whether the local user is in the list of authors. */ }; - explicit ReactionModel(QObject *parent = nullptr, QList reactions = {}, Quotient::User *localUser = nullptr); + explicit ReactionModel(const Quotient::RoomMessageEvent *event, const NeoChatRoom *room); /** * @brief Get the given role value at the given index. @@ -64,14 +66,12 @@ public: */ [[nodiscard]] QHash roleNames() const override; - /** - * @brief Set the reactions data in the model. - */ - void setReactions(QList reactions); - private: + const NeoChatRoom *m_room; + const Quotient::RoomMessageEvent *m_event; QList m_reactions; - Quotient::User *m_localUser; + void updateReactions(); + static QString reactionText(const QString &text); }; Q_DECLARE_METATYPE(ReactionModel *)