Improve User Avatar Model Roles

- Update the message event and user models so that the full source url is output. - Separate the reply author into its own role - Create an empty user object that can be passed so that the QML code no longer needs to check if certain parameters exist. - Make avatarForMember return and empty QUrl if a valid avatar cannot be found and make use in the user and event models As well as cleaning up the QML this should also stop the QML Image: Media id '' doesn't follow server/mediaId pattern spam in the log.
2023-04-30 13:51:06 +00:00
parent 1bb03e115e
commit 2281fe6f8a
13 changed files with 83 additions and 45 deletions
--- a/src/models/messageeventmodel.cpp
+++ b/src/models/messageeventmodel.cpp
@@ -47,6 +47,7 @@ QHash<int, QByteArray> MessageEventModel::roleNames() const
    roles[FileMimetypeIcon] = "fileMimetypeIcon";
    roles[EventResolvedTypeRole] = "eventResolvedType";
    roles[IsReplyRole] = "isReply";
+    roles[ReplyAuthor] = "replyAuthor";
    roles[ReplyRole] = "reply";
    roles[ReplyIdRole] = "replyId";
    roles[ShowAuthorRole] = "showAuthor";
@@ -421,12 +422,24 @@ void MessageEventModel::fetchMore(const QModelIndex &parent)
    }
 }

-inline QVariantMap userAtEvent(NeoChatUser *user, NeoChatRoom *room, const RoomEvent &evt)
+static const QVariantMap emptyUser = {
+    {"isLocalUser", false},
+    {"id", QString()},
+    {"avatarSource", QUrl()},
+    {"avatarMediaId", QString()},
+    {"avatarUrl", QString()},
+    {"displayName", QString()},
+    {"display", QString()},
+    {"color", QColor()},
+    {"object", QVariant()},
+};
+
+inline QVariantMap userInContext(NeoChatUser *user, NeoChatRoom *room)
 {
-    Q_UNUSED(evt)
    return QVariantMap{
        {"isLocalUser", user->id() == room->localUser()->id()},
        {"id", user->id()},
+        {"avatarSource", room->avatarForMember(user)},
        {"avatarMediaId", user->avatarMediaId(room)},
        {"avatarUrl", user->avatarUrl(room)},
        {"displayName", user->displayname(room)},
@@ -551,7 +564,7 @@ QVariant MessageEventModel::data(const QModelIndex &idx, int role) const

    if (role == AuthorRole) {
        auto author = static_cast<NeoChatUser *>(isPending ? m_currentRoom->localUser() : m_currentRoom->user(evt.senderId()));
-        return userAtEvent(author, m_currentRoom, evt);
+        return userInContext(author, m_currentRoom);
    }

    if (role == ContentTypeRole) {
@@ -690,21 +703,19 @@ QVariant MessageEventModel::data(const QModelIndex &idx, int role) const
        return evt.contentJson()["m.relates_to"].toObject()["m.in_reply_to"].toObject()["event_id"].toString();
    }

-    if (role == ReplyRole) {
-        const QString &replyEventId = evt.contentJson()["m.relates_to"].toObject()["m.in_reply_to"].toObject()["event_id"].toString();
-        if (replyEventId.isEmpty()) {
-            return {};
-        };
-        const auto replyIt = m_currentRoom->findInTimeline(replyEventId);
-        const RoomEvent *replyPtr = replyIt != m_currentRoom->historyEdge() ? &**replyIt : nullptr;
-        if (!replyPtr) {
-            for (const auto &e : m_extraEvents) {
-                if (e->id() == replyEventId) {
-                    replyPtr = e.get();
-                    break;
-                }
-            }
+    if (role == ReplyAuthor) {
+        auto replyPtr = getReplyForEvent(evt);
+
+        if (replyPtr) {
+            auto replyUser = static_cast<NeoChatUser *>(m_currentRoom->user(replyPtr->senderId()));
+            return userInContext(replyUser, m_currentRoom);
+        } else {
+            return emptyUser;
        }
+    }
+
+    if (role == ReplyRole) {
+        auto replyPtr = getReplyForEvent(evt);
        if (!replyPtr) {
            return {};
        }
@@ -753,11 +764,12 @@ QVariant MessageEventModel::data(const QModelIndex &idx, int role) const
            content = QVariant::fromValue(e->image().originalJson);
        }

-        return QVariantMap{{"eventId", replyEventId},
-                           {"display", m_currentRoom->eventToString(*replyPtr, Qt::RichText)},
-                           {"content", content},
-                           {"type", type},
-                           {"author", userAtEvent(static_cast<NeoChatUser *>(m_currentRoom->user(replyPtr->senderId())), m_currentRoom, evt)}};
+        return QVariantMap{
+            {"eventId", replyPtr->id()},
+            {"display", m_currentRoom->eventToString(*replyPtr, Qt::RichText)},
+            {"content", content},
+            {"type", type},
+        };
    }

    if (role == ShowAuthorRole) {
@@ -835,7 +847,7 @@ QVariant MessageEventModel::data(const QModelIndex &idx, int role) const
 #else
            auto user = static_cast<NeoChatUser *>(userId);
 #endif
-            users += userAtEvent(user, m_currentRoom, evt);
+            users += userInContext(user, m_currentRoom);
        }

        return users;
@@ -901,7 +913,7 @@ QVariant MessageEventModel::data(const QModelIndex &idx, int role) const
        while (i != reactions.constEnd()) {
            QVariantList authors;
            for (auto author : i.value()) {
-                authors.append(userAtEvent(author, m_currentRoom, evt));
+                authors.append(userInContext(author, m_currentRoom));
            }
            bool hasLocalUser = i.value().contains(static_cast<NeoChatUser *>(m_currentRoom->localUser()));
            res.append(QVariantMap{{"reaction", i.key()}, {"count", i.value().count()}, {"authors", authors}, {"hasLocalUser", hasLocalUser}});
@@ -1088,3 +1100,23 @@ void MessageEventModel::loadReply(const QModelIndex &index)
        Q_EMIT dataChanged(persistentIndex, persistentIndex, {ReplyRole});
    });
 }
+
+const RoomEvent *MessageEventModel::getReplyForEvent(const RoomEvent &event) const
+{
+    const QString &replyEventId = event.contentJson()["m.relates_to"].toObject()["m.in_reply_to"].toObject()["event_id"].toString();
+    if (replyEventId.isEmpty()) {
+        return {};
+    };
+
+    const auto replyIt = m_currentRoom->findInTimeline(replyEventId);
+    const RoomEvent *replyPtr = replyIt != m_currentRoom->historyEdge() ? &**replyIt : nullptr;
+    if (!replyPtr) {
+        for (const auto &e : m_extraEvents) {
+            if (e->id() == replyEventId) {
+                replyPtr = e.get();
+                break;
+            }
+        }
+    }
+    return replyPtr;
+}
--- a/src/models/messageeventmodel.h
+++ b/src/models/messageeventmodel.h
@@ -75,6 +75,7 @@ public:
        FileMimetypeIcon, /**< The icon name for the mime type of a file. */

        IsReplyRole, /**< Is the message a reply to another event. */
+        ReplyAuthor, /**< The author of the event that was replied to. */
        ReplyRole, /**< The content data of the message that was replied to. */
        ReplyIdRole, /**< The matrix ID of the message that was replied to. */

@@ -192,6 +193,8 @@ private:
    int refreshEventRoles(const QString &eventId, const QVector<int> &roles = {});
    void moveReadMarker(const QString &toEventId);

+    const Quotient::RoomEvent *getReplyForEvent(const Quotient::RoomEvent &event) const;
+
    std::vector<Quotient::event_ptr_tt<Quotient::RoomEvent>> m_extraEvents;

 Q_SIGNALS:
--- a/src/models/userlistmodel.cpp
+++ b/src/models/userlistmodel.cpp
@@ -71,7 +71,8 @@ QVariant UserListModel::data(const QModelIndex &index, int role) const
        return user->id();
    }
    if (role == AvatarRole) {
-        return user->avatarMediaId(m_currentRoom);
+        auto neoChatUser = static_cast<NeoChatUser *>(user);
+        return m_currentRoom->avatarForMember(neoChatUser);
    }
    if (role == ObjectRole) {
        return QVariant::fromValue(user);
--- a/src/neochatroom.cpp
+++ b/src/neochatroom.cpp
@@ -1921,7 +1921,12 @@ QByteArray NeoChatRoom::roomAcountDataJson(const QString &eventType)
 QUrl NeoChatRoom::avatarForMember(NeoChatUser *user) const
 {
 #ifdef QUOTIENT_07
-    return connection()->makeMediaUrl(memberAvatarUrl(user->id()));
+    auto avatar = connection()->makeMediaUrl(memberAvatarUrl(user->id()));
+    if (avatar.isValid() && avatar.scheme() == QStringLiteral("mxc")) {
+        return avatar;
+    } else {
+        return QUrl();
+    }
 #else
    QUrl url(QStringLiteral("mxc://%1").arg(user->avatarMediaId()));
    QUrlQuery q(url.query());
--- a/src/qml/Component/ChatBox/ReplyPane.qml
+++ b/src/qml/Component/ChatBox/ReplyPane.qml
@@ -14,7 +14,7 @@ GridLayout {
    id: root
    property string userName
    property color userColor: Kirigami.Theme.highlightColor
-    property var userAvatar: ""
+    property url userAvatar: ""
    property var text

    rows: 3
--- a/src/qml/Component/NeochatMaximizeComponent.qml
+++ b/src/qml/Component/NeochatMaximizeComponent.qml
@@ -35,7 +35,7 @@ Components.AlbumMaximizeComponent {
            implicitHeight: Kirigami.Units.iconSizes.medium

            name: modelData.author.name ?? modelData.author.displayName
-            source: modelData.author.avatarMediaId ? ("image://mxc/" + modelData.author.avatarMediaId) : ""
+            source: modelData.author.avatarSource
            color: modelData.author.color
        }
        ColumnLayout {
--- a/src/qml/Component/Timeline/AvatarFlow.qml
+++ b/src/qml/Component/Timeline/AvatarFlow.qml
@@ -21,7 +21,7 @@ Flow {
            implicitHeight: avatarSize

            name: modelData.displayName
-            source: modelData.avatarMediaId ? ("image://mxc/" + modelData.avatarMediaId) : ""
+            source: modelData.avatarSource
            color: modelData.color
        }
    }
--- a/src/qml/Component/Timeline/LocationDelegate.qml
+++ b/src/qml/Component/Timeline/LocationDelegate.qml
@@ -71,7 +71,7 @@ TimelineContainer {
                        width: height
                        height: parent.height / 3 + 1
                        name: model.author.name ?? model.author.displayName
-                        source: model.author.avatarMediaId ? ("image://mxc/" + model.author.avatarMediaId) : ""
+                        source: model.author.avatarSource
                        color: model.author.color
                    }
                }
--- a/src/qml/Component/Timeline/StateDelegate.qml
+++ b/src/qml/Component/Timeline/StateDelegate.qml
@@ -82,7 +82,7 @@ QQC2.Control {
                        implicitHeight: Kirigami.Units.iconSizes.small

                        name: modelData.displayName
-                        source: modelData.avatarMediaId ? ("image://mxc/" + modelData.avatarMediaId) : ""
+                        source: modelData.avatarSource
                        color: modelData.color
                    }
                }
@@ -121,7 +121,7 @@ QQC2.Control {
                visible: !columnLayout.folded || stateEventRepeater.count === 1

                name: modelData.author.displayName
-                avatar: modelData.author.avatarMediaId ? ("image://mxc/" + modelData.author.avatarMediaId) : ""
+                avatar: modelData.author.avatarSource
                color: modelData.author.color
                text: `<style>a {text-decoration: none;}</style><a href="https://matrix.to/#/${modelData.author.id}" style="color: ${modelData.author.color}">${modelData.authorDisplayName}</a> ${modelData.text}`

--- a/src/qml/Component/Timeline/TimelineContainer.qml
+++ b/src/qml/Component/Timeline/TimelineContainer.qml
@@ -135,7 +135,7 @@ ColumnLayout {
                Config.showAvatarInTimeline &&
                (Config.compactLayout || !showUserMessageOnRight)
            name: model.author.name ?? model.author.displayName
-            source: visible && model.author.avatarMediaId ? ("image://mxc/" + model.author.avatarMediaId) : ""
+            source: model.author.avatarSource
            color: model.author.color

            MouseArea {
@@ -144,9 +144,7 @@ ColumnLayout {
                    userDetailDialog.createObject(QQC2.ApplicationWindow.overlay, {
                        room: currentRoom,
                        user: author.object,
-                        displayName: author.displayName,
-                        avatarMediaId: author.avatarMediaId,
-                        avatarUrl: author.avatarUrl
+                        displayName: author.displayName
                    }).open();
                }
                cursorShape: Qt.PointingHandCursor
@@ -231,8 +229,7 @@ ColumnLayout {
                                        room: currentRoom,
                                        user: author.object,
                                        displayName: author.displayName,
-                                        avatarMediaId: author.avatarMediaId,
-                                        avatarUrl: author.avatarUrl
+                                        avatarSource: author.avatarSource
                                    }).open();
                                }
                            }
@@ -260,9 +257,9 @@ ColumnLayout {
                        visible: active

                        sourceComponent: ReplyComponent {
-                            name: currentRoom.htmlSafeMemberName(reply.author.id)
-                            avatar: reply.author.avatarMediaId ? ("image://mxc/" + reply.author.avatarMediaId) : ""
-                            color: reply.author.color
+                            name: currentRoom.htmlSafeMemberName(model.replyAuthor.id)
+                            avatar: model.replyAuthor.avatarSource
+                            color: model.replyAuthor.color
                        }

                        Connections {
--- a/src/qml/Dialog/UserDetailDialog.qml
+++ b/src/qml/Dialog/UserDetailDialog.qml
@@ -19,7 +19,7 @@ Kirigami.OverlaySheet {
    property var user

    property string displayName: user.displayName
-    readonly property string avatar: room.avatarForMember(user)
+    readonly property url avatar: room.avatarForMember(user)

    parent: applicationWindow().overlay

@@ -45,7 +45,7 @@ Kirigami.OverlaySheet {
                Layout.preferredHeight: Kirigami.Units.iconSizes.huge

                name: displayName
-                source: avatar ?? ""
+                source: avatar
                color: user.color
            }

--- a/src/qml/Menu/Timeline/MessageDelegateContextMenu.qml
+++ b/src/qml/Menu/Timeline/MessageDelegateContextMenu.qml
@@ -226,7 +226,7 @@ Loader {
                        spacing: Kirigami.Units.largeSpacing
                        Kirigami.Avatar {
                            id: avatar
-                            source: author.avatarMediaId ? ("image://mxc/" + author.avatarMediaId) : ""
+                            source: author.avatarSource
                            Layout.preferredWidth: Kirigami.Units.gridUnit * 3
                            Layout.preferredHeight: Kirigami.Units.gridUnit * 3
                            Layout.alignment: Qt.AlignTop
--- a/src/qml/Panel/RoomDrawer.qml
+++ b/src/qml/Panel/RoomDrawer.qml
@@ -258,7 +258,7 @@ Kirigami.OverlayDrawer {
                            implicitWidth: height
                            sourceSize.height: Kirigami.Units.gridUnit + Kirigami.Units.smallSpacing * 2.5
                            sourceSize.width: Kirigami.Units.gridUnit + Kirigami.Units.smallSpacing * 2.5
-                            source: avatar ? ("image://mxc/" + avatar) : ""
+                            source: avatar
                            name: model.userId
                        }