1 files changed, 34 insertions, 20 deletions

diff --git a/src/plugins/Matrix.cpp b/src/plugins/Matrix.cpp
index ff0d498..7de8af9 100644
--- a/src/plugins/Matrix.cpp
+++ b/src/plugins/Matrix.cpp
@@ -55,7 +55,7 @@ namespace QuickMedia {
         return user->read_marker_event_id;
     }
 
-    void RoomData::prepend_messages_reverse(std::vector<std::shared_ptr<Message>> new_messages) {
+    void RoomData::prepend_messages_reverse(const std::vector<std::shared_ptr<Message>> &new_messages) {
         std::lock_guard<std::mutex> lock(room_mutex);
         for(auto it = new_messages.begin(); it != new_messages.end(); ++it) {
             if(message_by_event_id.find((*it)->event_id) == message_by_event_id.end()) {
@@ -65,7 +65,7 @@ namespace QuickMedia {
         }
     }
 
-    void RoomData::append_messages(std::vector<std::shared_ptr<Message>> new_messages) {
+    void RoomData::append_messages(const std::vector<std::shared_ptr<Message>> &new_messages) {
         std::lock_guard<std::mutex> lock(room_mutex);
         for(auto it = new_messages.begin(); it != new_messages.end(); ++it) {
             if(message_by_event_id.find((*it)->event_id) == message_by_event_id.end()) {
@@ -208,6 +208,8 @@ namespace QuickMedia {
                 events_set_room_name(events_json, room);
             }
 
+            const rapidjson::Value &ephemeral_json = GetMember(it.value, "ephemeral");
+
             const rapidjson::Value &timeline_json = GetMember(it.value, "timeline");
             if(timeline_json.IsObject()) {
                 if(room->prev_batch.empty()) {
@@ -217,7 +219,7 @@ namespace QuickMedia {
                         room->prev_batch = prev_batch_json.GetString();
                 }
 
-                // TODO: Is there no better way to check for notifications? this is not robust...
+                // TODO: Use /_matrix/client/r0/notifications ? or remove this and always look for displayname/user_id in messages
                 bool has_unread_notifications = false;
                 const rapidjson::Value &unread_notification_json = GetMember(it.value, "unread_notifications");
                 if(unread_notification_json.IsObject()) {
@@ -228,14 +230,18 @@ namespace QuickMedia {
 
                 const rapidjson::Value &events_json = GetMember(timeline_json, "events");
                 events_add_user_info(events_json, room);
-                events_add_messages(events_json, room, MessageDirection::AFTER, &room_messages, has_unread_notifications);
                 events_set_room_name(events_json, room);
-            }
-
-            const rapidjson::Value &ephemeral_json = GetMember(it.value, "ephemeral");
-            if(ephemeral_json.IsObject()) {
-                const rapidjson::Value &events_json = GetMember(ephemeral_json, "events");
-                events_add_user_read_markers(events_json, room);
+                // We want to do this before adding messages to know if a message that mentions us is a new mention
+                if(ephemeral_json.IsObject()) {
+                    const rapidjson::Value &events_json = GetMember(ephemeral_json, "events");
+                    events_add_user_read_markers(events_json, room);
+                }
+                events_add_messages(events_json, room, MessageDirection::AFTER, &room_messages, has_unread_notifications);
+            } else {
+                if(ephemeral_json.IsObject()) {
+                    const rapidjson::Value &events_json = GetMember(ephemeral_json, "events");
+                    events_add_user_read_markers(events_json, room);
+                }
             }
         }
 
@@ -457,25 +463,33 @@ namespace QuickMedia {
 
         for(const rapidjson::Value &event_item_json : events_json.GetArray()) {
             std::shared_ptr<Message> new_message = parse_message_event(event_item_json, room_data);
-            if(!new_message)
-                continue;
-
-            // TODO: Is @room ok? shouldn't we also check if the user has permission to do @room? (only when notifications are limited to @mentions)
-            if(has_unread_notifications && me)
-                new_message->mentions_me = message_contains_user_mention(new_message->body, me->display_name) || message_contains_user_mention(new_message->body, me->user_id) || message_contains_user_mention(new_message->body, "@room");
-
-            new_messages.push_back(std::move(new_message));
+            if(new_message)
+                new_messages.push_back(std::move(new_message));
         }
 
+        if(new_messages.empty())
+            return;
+
         // TODO: Add directly to this instead when set? otherwise add to new_messages
         if(room_messages)
             (*room_messages)[room_data] = new_messages;
 
         // TODO: Loop and std::move instead? doesn't insert create copies?
         if(message_dir == MessageDirection::BEFORE) {
-            room_data->prepend_messages_reverse(std::move(new_messages));
+            room_data->prepend_messages_reverse(new_messages);
         } else if(message_dir == MessageDirection::AFTER) {
-            room_data->append_messages(std::move(new_messages));
+            room_data->append_messages(new_messages);
+        }
+
+        std::shared_ptr<Message> read_marker_message;
+        if(me)
+            read_marker_message = room_data->get_message_by_id(room_data->get_user_read_marker(me));
+
+        for(auto &message : new_messages) {
+            // TODO: Is @room ok? shouldn't we also check if the user has permission to do @room? (only when notifications are limited to @mentions)
+            // TODO: Is comparing against read marker timestamp ok enough?
+            if(has_unread_notifications && me && (!read_marker_message || read_marker_message->timestamp < message->timestamp))
+                message->mentions_me = message_contains_user_mention(message->body, me->display_name) || message_contains_user_mention(message->body, me->user_id) || message_contains_user_mention(message->body, "@room");
         }
     }