ESCA111/neochat
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Refactor reactions

Browse Source 
Currently we effectively create the reactions list in EventHandler then pass that data into a model. This reworks the model so that we just pass in a room and an event and it grabs it's own data. This means that:
- the functions in event handler are no longer required
- the model can update itself to add/remove reactions so no need to handle that in MessageEventModel
- MessageEventModel only needs to create new ReactionModels or remove old ones when no reactions exist anymore

A basic test suite has also been created for the ReactionModel
This commit is contained in:
James Graham
2024-01-06 17:50:32 +00:00
parent ad083f64b1
commit fad381c36f
11 changed files with 454 additions and 130 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
autotests/CMakeLists.txt
View File

@@ -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
)

44
autotests/data/test-reactionmodel-extra-sync.json Normal file
View File

@@ -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"
}
}

204
autotests/data/test-reactionmodel-sync.json Normal file
View File

@@ -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": "<b>This is an example<br>text message</b>",
"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"
}
}

19
autotests/eventhandlertest.cpp
View File

@@ -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();

83
autotests/reactionmodeltest.cpp Normal file
View File

@@ -0,0 +1,83 @@
// SPDX-FileCopyrightText: 2023 James Graham <james.h.graham@protonmail.com>
// SPDX-License-Identifier: GPL-2.0-only OR GPL-3.0-only OR LicenseRef-KDE-Accepted-GPL
#include <QObject>
#include <QSignalSpy>
#include <QTest>
#include "models/reactionmodel.h"
#include <Quotient/events/roommessageevent.h>
#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<const RoomMessageEvent>(room->messageEvents().at(0).get());
auto model = ReactionModel(event, room);
QCOMPARE(model.rowCount(), 1);
QCOMPARE(model.data(model.index(0), ReactionModel::TextContentRole), QStringLiteral("<span style=\"font-family: 'emoji';\">👍</span>"));
QCOMPARE(model.data(model.index(0), ReactionModel::ReactionRole), QStringLiteral("👍"));
QCOMPARE(model.data(model.index(0), ReactionModel::ToolTipRole),
QStringLiteral("@alice:matrix.org reacted with <span style=\"font-family: 'emoji';\">👍</span>"));
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<const RoomMessageEvent>(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 <span style=\"font-family: 'emoji';\">👍</span>"));
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 <span style=\"font-family: 'emoji';\">👍</span>"));
delete model;
}
QTEST_MAIN(ReactionModelTest)
#include "reactionmodeltest.moc"

54
src/eventhandler.cpp
View File

@@ -811,60 +811,6 @@ QSharedPointer<LinkPreviewer> EventHandler::getLinkPreviewer() const
}
}
QSharedPointer<ReactionModel> 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<RoomMessageEvent>()) {
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<QString, QList<User *>> reactions = {};
for (const auto &a : annotations) {
if (a->isRedacted()) { // Just in case?
continue;
}
if (const auto &e = eventCast<const ReactionEvent>(a)) {
reactions[e->key()].append(m_room->user(e->senderId()));
}
}
if (reactions.isEmpty()) {
return nullptr;
}
QList<ReactionModel::Reaction> 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<ReactionModel>(new ReactionModel(nullptr, res, m_room->localUser()));
} else {
return nullptr;
}
}
bool EventHandler::hasReply() const
{
if (m_event == nullptr) {

9
src/eventhandler.h
View File

@@ -241,15 +241,6 @@ public:
*/
QSharedPointer<LinkPreviewer> 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<ReactionModel> getReactions() const;
/**
* @brief Whether the event is a reply to another in the timeline.
*/

32
src/models/messageeventmodel.cpp
View File

@@ -72,6 +72,12 @@ QHash<int, QByteArray> 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<const PollStartEvent>(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<ReactionModel>(new ReactionModel(event, m_currentRoom));
if (reactionModel->rowCount() > 0) {
m_reactionModels[eventId] = reactionModel;
if (!resetting) {
refreshEventRoles(eventId, {ReactionRole, ShowReactionsRole});
}
}
}
}
}

1
src/models/messageeventmodel.h
View File

@@ -131,6 +131,7 @@ private:
QString lastReadEventId;
QPersistentModelIndex m_lastReadEventIndex;
int rowBelowInserted = -1;
bool resetting = false;
bool movingEvent = false;
KFormat m_format;

118
src/models/reactionmodel.cpp
View File

@@ -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 <QDebug>
#ifdef HAVE_ICU
@@ -15,11 +16,20 @@
#include <Quotient/user.h>
ReactionModel::ReactionModel(QObject *parent, QList<Reaction> 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("<span style=\"font-family: 'emoji';\">") + reaction.reaction + QStringLiteral("</span>")
: 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<Reaction> 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<QString, QList<Quotient::User *>> reactions = {};
for (const auto &a : annotations) {
if (a->isRedacted()) { // Just in case?
continue;
}
if (const auto &e = eventCast<const Quotient::ReactionEvent>(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<int, QByteArray> 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("<span style=\"font-family: 'emoji';\">") + text + QStringLiteral("</span>") : text;
}
#include "moc_reactionmodel.cpp"

14
src/models/reactionmodel.h
View File

@@ -3,8 +3,10 @@
#pragma once
#include "neochatroom.h"
#include <QAbstractListModel>
#include <QQmlEngine>
#include <Quotient/events/reactionevent.h>
namespace Quotient
{
@@ -41,7 +43,7 @@ public:
HasLocalUser, /**< Whether the local user is in the list of authors. */
};
explicit ReactionModel(QObject *parent = nullptr, QList<Reaction> 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<int, QByteArray> roleNames() const override;
/**
* @brief Set the reactions data in the model.
*/
void setReactions(QList<Reaction> reactions);
private:
const NeoChatRoom *m_room;
const Quotient::RoomMessageEvent *m_event;
QList<Reaction> m_reactions;
Quotient::User *m_localUser;
void updateReactions();
static QString reactionText(const QString &text);
};
Q_DECLARE_METATYPE(ReactionModel *)