Fix duplicate messages when redacting.

include/libqmatrixclient

@@ -1 +1 @@
-Subproject commit 98751495f1990dccf285e3b4739f86de7b7f68fd
+Subproject commit 8bd8aaf0858bb0a0ebcac8c3d29cfbb20279164d

qml/component/MessageDelegate.qml

@@ -1,6 +1,7 @@
 import QtQuick 2.9
 import QtQuick.Controls 2.2
 import QtQuick.Controls.Material 2.2
+import Matrique 0.1
 
 Item {
     id: messageDelegate
@@ -13,7 +14,7 @@ Item {
     readonly property bool isMessage: eventType === "message" || eventType === "notice"
     readonly property bool isFile: eventType === "video" || eventType === "audio" || eventType === "file" || eventType === "image"
 
-    visible: eventType != "redaction"
+    visible: marks !== EventStatus.Hidden
 
     z: -5
     width: delegateLoader.width

src/messageeventmodel.cpp

@@ -1,7 +1,6 @@
 #include "messageeventmodel.h"
 
 #include <QtCore/QDebug>
-#include <QtCore/QSettings>
 #include <QtQml>  // for qmlRegisterType()
 
 #include <connection.h>
@@ -24,22 +23,27 @@ QHash<int, QByteArray> MessageEventModel::roleNames() const {
   roles[AboveAuthorRole] = "aboveAuthor";
   roles[ContentRole] = "content";
   roles[ContentTypeRole] = "contentType";
+  roles[HighlightRole] = "highlight";
   roles[ReadMarkerRole] = "readMarker";
   roles[SpecialMarksRole] = "marks";
   roles[LongOperationRole] = "progressInfo";
+  roles[AnnotationRole] = "annotation";
   roles[EventResolvedTypeRole] = "eventResolvedType";
   roles[PlainTextRole] = "plainText";
   return roles;
 }
 
+MessageEventModel::~MessageEventModel() {}
+
 MessageEventModel::MessageEventModel(QObject* parent)
     : QAbstractListModel(parent), m_currentRoom(nullptr) {
-  qmlRegisterType<QMatrixClient::FileTransferInfo>();
+  using namespace QMatrixClient;
-  qRegisterMetaType<QMatrixClient::FileTransferInfo>();
+  qmlRegisterType<FileTransferInfo>();
+  qRegisterMetaType<FileTransferInfo>();
+  qmlRegisterUncreatableType<EventStatus>(
+      "Matrique", 0, 1, "EventStatus", "EventStatus is not an creatable type");
 }
 
-MessageEventModel::~MessageEventModel() {}
-
 void MessageEventModel::setRoom(QMatrixClient::Room* room) {
   if (room == m_currentRoom) return;
 
@@ -52,6 +56,7 @@ void MessageEventModel::setRoom(QMatrixClient::Room* room) {
   m_currentRoom = room;
   if (room) {
     lastReadEventId = room->readMarkerEventId();
+
     using namespace QMatrixClient;
     connect(m_currentRoom, &Room::aboutToAddNewMessages, this,
             [=](RoomEventsRange events) {
@@ -61,17 +66,16 @@ void MessageEventModel::setRoom(QMatrixClient::Room* room) {
             });
     connect(m_currentRoom, &Room::aboutToAddHistoricalMessages, this,
             [=](RoomEventsRange events) {
-              if (rowCount() > 0) nextNewerRow = rowCount() - 1;  // See #312
+              if (rowCount() > 0)
+                rowBelowInserted = rowCount() - 1;  // See #312
               beginInsertRows(QModelIndex(), rowCount(),
                               rowCount() + int(events.size()) - 1);
             });
     connect(m_currentRoom, &Room::addedMessages, this, [=] {
-      if (nextNewerRow > -1) {
-        const auto idx = index(nextNewerRow);
-        emit dataChanged(idx, idx);
-        nextNewerRow = -1;
-      }
       endInsertRows();
+      if (rowBelowInserted > -1)
+        refreshEventRoles(rowBelowInserted,
+                          {AboveAuthorRole, AboveSectionRole});
     });
     connect(m_currentRoom, &Room::pendingEventAboutToAdd, this,
             [this] { beginInsertRows({}, 0, 0); });
@@ -79,27 +83,38 @@ void MessageEventModel::setRoom(QMatrixClient::Room* room) {
             &MessageEventModel::endInsertRows);
     connect(m_currentRoom, &Room::pendingEventAboutToMerge, this,
             [this](RoomEvent*, int i) {
-              const auto timelineBaseIdx =
+              if (i == 0) return;  // No need to move anything, just refresh
-                  int(m_currentRoom->pendingEvents().size());
+
-              if (i + 1 == timelineBaseIdx) return;  // No need to move anything
+              movingEvent = true;
-              mergingEcho = true;
+              // Reverse i because row 0 is bottommost in the model
-              Q_ASSERT(beginMoveRows({}, i, i, {}, timelineBaseIdx));
+              const auto row = timelineBaseIndex() - i - 1;
+              Q_ASSERT(beginMoveRows({}, row, row, {}, timelineBaseIndex()));
             });
     connect(m_currentRoom, &Room::pendingEventMerged, this, [this] {
-      if (mergingEcho) {
+      if (movingEvent) {
         endMoveRows();
-        mergingEcho = false;
+        movingEvent = false;
       }
-      refreshEventRoles(int(m_currentRoom->pendingEvents().size()),
+      refreshRow(timelineBaseIndex());        // Refresh the looks
-                        {SpecialMarksRole});
+      if (m_currentRoom->timelineSize() > 1)  // Refresh above
+        refreshEventRoles(timelineBaseIndex() + 1/*,
+                                          {ReadMarkerRole}*/);
+      if (timelineBaseIndex() > 0)  // Refresh below, see #312
+        refreshEventRoles(timelineBaseIndex() - 1,
+                          {AboveAuthorRole, AboveSectionRole});
     });
     connect(m_currentRoom, &Room::pendingEventChanged, this,
-            [this](int i) { refreshEventRoles(i, {SpecialMarksRole}); });
+            &MessageEventModel::refreshRow);
+    connect(m_currentRoom, &Room::pendingEventAboutToDiscard, this,
+            [this](int i) { beginRemoveRows({}, i, i); });
+    connect(m_currentRoom, &Room::pendingEventDiscarded, this,
+            &MessageEventModel::endRemoveRows);
     connect(m_currentRoom, &Room::readMarkerMoved, this, [this] {
       refreshEventRoles(
-          std::exchange(lastReadEventId, m_currentRoom->readMarkerEventId()),
+                std::exchange(lastReadEventId,
-          {ReadMarkerRole});
+                              m_currentRoom->readMarkerEventId())/*,
-      refreshEventRoles(lastReadEventId, {ReadMarkerRole});
+                {ReadMarkerRole}*/);
+      refreshEventRoles(lastReadEventId /*, {ReadMarkerRole}*/);
     });
     connect(
         m_currentRoom, &Room::replacedEvent, this,
@@ -120,11 +135,16 @@ void MessageEventModel::setRoom(QMatrixClient::Room* room) {
 }
 
 void MessageEventModel::refreshEvent(const QString& eventId) {
-  refreshEventRoles(eventId, {});
+  refreshEventRoles(eventId);
 }
 
-void MessageEventModel::refreshEventRoles(const int row,
+void MessageEventModel::refreshRow(int row) { refreshEventRoles(row); }
-                                          const QVector<int>& roles) {
+
+int MessageEventModel::timelineBaseIndex() const {
+  return m_currentRoom ? int(m_currentRoom->pendingEvents().size()) : 0;
+}
+
+void MessageEventModel::refreshEventRoles(int row, const QVector<int>& roles) {
   const auto idx = index(row);
   emit dataChanged(idx, idx, roles);
 }
@@ -132,8 +152,13 @@ void MessageEventModel::refreshEventRoles(const int row,
 void MessageEventModel::refreshEventRoles(const QString& eventId,
                                           const QVector<int>& roles) {
   const auto it = m_currentRoom->findInTimeline(eventId);
-  if (it != m_currentRoom->timelineEdge())
+  if (it == m_currentRoom->timelineEdge()) {
-    refreshEventRoles(it - m_currentRoom->messageEvents().rbegin(), roles);
+    qWarning() << "Trying to refresh inexistent event:" << eventId;
+    return;
+  }
+  refreshEventRoles(
+      it - m_currentRoom->messageEvents().rbegin() + timelineBaseIndex(),
+      roles);
 }
 
 inline bool hasValidTimestamp(const QMatrixClient::TimelineItem& ti) {
@@ -161,9 +186,8 @@ QDateTime MessageEventModel::makeMessageTimestamp(
   return {};
 }
 
-QString MessageEventModel::makeDateString(
+QString MessageEventModel::renderDate(QDateTime timestamp) const {
-    const QMatrixClient::Room::rev_iter_t& baseIt) const {
+  auto date = timestamp.toLocalTime().date();
-  auto date = makeMessageTimestamp(baseIt).toLocalTime().date();
   if (QMatrixClient::SettingsGroup("UI")
           .value("banner_human_friendly_date", true)
           .toBool()) {
@@ -181,20 +205,20 @@ int MessageEventModel::rowCount(const QModelIndex& parent) const {
   return m_currentRoom->timelineSize();
 }
 
-QVariant MessageEventModel::data(const QModelIndex& index, int role) const {
+QVariant MessageEventModel::data(const QModelIndex& idx, int role) const {
-  const auto row = index.row();
+  const auto row = idx.row();
 
   if (!m_currentRoom || row < 0 ||
       row >= int(m_currentRoom->pendingEvents().size()) +
                  m_currentRoom->timelineSize())
     return {};
 
-  const auto timelineBaseIdx = int(m_currentRoom->pendingEvents().size());
+  bool isPending = row < timelineBaseIndex();
   const auto timelineIt = m_currentRoom->messageEvents().crbegin() +
-                          std::max(-1, row - timelineBaseIdx);
+                          std::max(0, row - timelineBaseIndex());
-  const auto& evt = row < timelineBaseIdx
+  const auto pendingIt = m_currentRoom->pendingEvents().crbegin() +
-                        ? *m_currentRoom->pendingEvents()[size_t(row)]
+                         std::min(row, timelineBaseIndex());
-                        : *timelineIt->event();
+  const auto& evt = isPending ? *pendingIt->event() : *timelineIt->event();
 
   using namespace QMatrixClient;
   if (role == Qt::DisplayRole) {
@@ -399,6 +423,7 @@ QVariant MessageEventModel::data(const QModelIndex& index, int role) const {
         tr("Unknown Event"));
   }
 
+
   if (role == Qt::ToolTipRole) {
     return evt.originalJson();
   }
@@ -412,15 +437,10 @@ QVariant MessageEventModel::data(const QModelIndex& index, int role) const {
           return "notice";
         case MessageEventType::Image:
           return "image";
-        case MessageEventType::File:
-        case MessageEventType::Audio:
-        case MessageEventType::Video:
-          return "file";
         default:
-          return "message";
+          return e->hasFileContent() ? "file" : "message";
       }
     }
-    if (is<RedactionEvent>(evt)) return "redaction";
     if (evt.isStateEvent()) return "state";
 
     return "other";
@@ -431,9 +451,8 @@ QVariant MessageEventModel::data(const QModelIndex& index, int role) const {
 
   if (role == AuthorRole) {
     // FIXME: It shouldn't be User, it should be its state "as of event"
-    return QVariant::fromValue(row < timelineBaseIdx
+    return QVariant::fromValue(isPending ? m_currentRoom->localUser()
-                                   ? m_currentRoom->localUser()
+                                         : m_currentRoom->user(evt.senderId()));
-                                   : m_currentRoom->user(evt.senderId()));
   }
 
   if (role == ContentTypeRole) {
@@ -466,16 +485,20 @@ QVariant MessageEventModel::data(const QModelIndex& index, int role) const {
   if (role == ReadMarkerRole) return evt.id() == lastReadEventId;
 
   if (role == SpecialMarksRole) {
-    if (row < timelineBaseIdx)
+    if (isPending) return pendingIt->deliveryStatus();
-      return evt.id().isEmpty() ? "unsent" : "unsynced";
 
     if (evt.isStateEvent() &&
-        static_cast<const StateEventBase&>(evt).repeatsState())
+        static_cast<const StateEventBase&>(evt).repeatsState() &&
-      return "noop";
+        !Settings().value("UI/show_noop_events", false).toBool())
-    return evt.isRedacted() ? "redacted" : "";
+      return EventStatus::Hidden;
+
+    if (is<RedactionEvent>(evt)) return EventStatus::Hidden;
+
+    return evt.isRedacted() ? EventStatus::Redacted : EventStatus::Normal;
   }
 
-  if (role == EventIdRole) return evt.id();
+  if (role == EventIdRole)
+    return !evt.id().isEmpty() ? evt.id() : evt.transactionId();
 
   if (role == LongOperationRole) {
     if (auto e = eventCast<const RoomMessageEvent>(&evt))
@@ -483,30 +506,21 @@ QVariant MessageEventModel::data(const QModelIndex& index, int role) const {
         return QVariant::fromValue(m_currentRoom->fileTransferInfo(e->id()));
   }
 
-  if (row >= timelineBaseIdx - 1)  // The timeline and the topmost unsynced
+  if (role == AnnotationRole)
-  {
+    if (isPending) return pendingIt->annotation();
-    if (role == TimeRole)
-      return row < timelineBaseIdx ? QDateTime::currentDateTimeUtc()
-                                   : makeMessageTimestamp(timelineIt);
 
-    if (role == SectionRole)
+  if (role == TimeRole || role == SectionRole) {
-      return row < timelineBaseIdx
+    auto ts =
-                 ? tr("Today")
+        isPending ? pendingIt->lastUpdated() : makeMessageTimestamp(timelineIt);
-                 : makeDateString(
+    return role == TimeRole ? QVariant(ts) : renderDate(ts);
-                       timelineIt);  // FIXME: move date rendering to QML
-
-    // FIXME: shouldn't be here, because #312
-    auto aboveEventIt = timelineIt + 1;
-    if (aboveEventIt != m_currentRoom->timelineEdge()) {
-      if (role == AboveSectionRole) return makeDateString(aboveEventIt);
-
-      if (role == AboveAuthorRole)
-        return QVariant::fromValue(
-            m_currentRoom->user((*aboveEventIt)->senderId()));
-
-      if (role == AboveTimeRole) return makeMessageTimestamp(aboveEventIt);
-    }
   }
 
-  return QVariant();
+  if (role == AboveSectionRole || role == AboveAuthorRole || role == AboveTimeRole)
+    for (auto r = row + 1; r < rowCount(); ++r) {
+      auto i = index(r);
+      if (data(i, SpecialMarksRole) != EventStatus::Hidden)
+        return data(i, role == AboveSectionRole ? SectionRole : role == AboveAuthorRole ? AuthorRole : TimeRole);
+    }
+
+  return {};
 }

src/messageeventmodel.h

@@ -25,6 +25,7 @@ class MessageEventModel : public QAbstractListModel {
     ReadMarkerRole,
     SpecialMarksRole,
     LongOperationRole,
+    AnnotationRole,
     PlainTextRole,
     // For debugging
     EventResolvedTypeRole,
@@ -36,25 +37,27 @@ class MessageEventModel : public QAbstractListModel {
   QMatrixClient::Room* getRoom() { return m_currentRoom; }
   void setRoom(QMatrixClient::Room* room);
 
-  Q_INVOKABLE int rowCount(
+  int rowCount(const QModelIndex& parent = QModelIndex()) const override;
-      const QModelIndex& parent = QModelIndex()) const override;
+  QVariant data(const QModelIndex& index, int role = Qt::DisplayRole) const override;
-  QVariant data(const QModelIndex& index, int role) const override;
   QHash<int, QByteArray> roleNames() const;
 
  private slots:
   void refreshEvent(const QString& eventId);
+  void refreshRow(int row);
 
  private:
   QMatrixClient::Room* m_currentRoom = nullptr;
   QString lastReadEventId;
-  bool mergingEcho = 0;
+  int rowBelowInserted = -1;
-  int nextNewerRow = -1;
+  bool movingEvent = 0;
 
+  int timelineBaseIndex() const;
   QDateTime makeMessageTimestamp(
       const QMatrixClient::Room::rev_iter_t& baseIt) const;
-  QString makeDateString(const QMatrixClient::Room::rev_iter_t& baseIt) const;
+  QString renderDate(QDateTime timestamp) const;
-  void refreshEventRoles(const int row, const QVector<int>& roles);
+  void refreshEventRoles(int row, const QVector<int>& roles = {});
-  void refreshEventRoles(const QString& eventId, const QVector<int>& roles);
+  void refreshEventRoles(const QString& eventId,
+                         const QVector<int>& roles = {});
 
  signals:
   void roomChanged();