Messages can get stuck in unread state

[gw-gh]

Messages in a channel should be set to "read" after a user has a chance to view them (and after a delay timer has expired). This is done in the QML by sending a "mark" API command with the timestamp of the last message in the messageListModel.

However, it seems that messages in some of my channels are not being put into the messageListModel in strict timestamp order. I don't know why this is happening - perhaps a change to the Slack API allows messages to be downloaded out of order, or perhaps this happens when messages are deleted or edited. So the latest message may not be the last one in the list.

The result is that the "mark" API is sent for the wrong message, and any messages with later timestamps will be stuck in "unread" state. This has the further effect of suppressing any new notifications for the channel, because notifications are only emitted when a new message is received if the channel's unread count is zero. Restarting the app does not make any difference - the mark is still sent for the wrong message.

The messy works-for-me patch below seems to restore messages being marked as read, and therefore also restores notifications. This is almost certainly not the best way to fix this but I needed something that worked in a hurry...

diff --git a/qml/pages/MessageListView.qml b/qml/pages/MessageListView.qml
index 8ca1e51..19468df 100644
--- a/qml/pages/MessageListView.qml
+++ b/qml/pages/MessageListView.qml
@@ -29,6 +29,28 @@ SilicaListView {
         return -1
     }
 
+    function sortedInsert(message) {
+        // is this the first message?
+        if (messageListModel.count == 0) {
+           messageListModel.append(message)
+           return
+       }
+        // optimisation: do we just go at the end?
+       if (messageListModel.get(messageListModel.count - 1).timestamp < message.timestamp) {
+           messageListModel.append(message)
+           return
+       }
+       // insert at correct sorted position
+        for (var i = 0; i < messageListModel.count; i++) {
+            if (messageListModel.get(i).timestamp >= message.timestamp) {
+                messageListModel.insert(i, message)
+                return
+            }
+       }
+       // should not get to here
+       messageListModel.append(message)
+    }
+
     function setLastRead(timestamp) {
         console.log("Setting lastRead to", timestamp)
 
@@ -277,7 +299,7 @@ SilicaListView {
                     messageListModel.set(index, message)
                 }
             } else {
-                messageListModel.append(message)
+                sortedInsert(message)
             }
 
             var isAtBottom = atBottom
@@ -285,7 +307,7 @@ SilicaListView {
                 listView.positionViewAtEnd()
 
                 if (appActive) {
-                    furthestRead = message.timestamp
+                    furthestRead = messageListModel.get(messageListModel.count - 1).timestamp
                     readTimer.restart()
                 }
             }
diff --git a/qml/pages/MessageLoader.js b/qml/pages/MessageLoader.js
index 225ae29..30fc70f 100644
--- a/qml/pages/MessageLoader.js
+++ b/qml/pages/MessageLoader.js
@@ -1,16 +1,39 @@
 WorkerScript.onMessage = function(message) {
+    function sortedInsert(model, message) {
+        // is this the first message?
+        if (model.count == 0) {
+           model.append(message)
+           return
+       }
+        // optimisation: do we just go at the end?
+       if (model.get(model.count - 1).timestamp < message.timestamp) {
+           model.append(message)
+           return
+       }
+       // insert at correct sorted position
+        for (var i = 0; i < model.count; i++) {
+            if (model.get(i).timestamp >= message.timestamp) {
+                model.insert(i, message)
+                return
+            }
+       }
+       // should not get to here
+       model.append(message)
+    }
+
+
     var messages = message.messages;
     var model = message.model;
 
     if (message.op === 'replace') {
         model.clear();
         messages.forEach(function(m) {
-            model.append(m);
+            sortedInsert(model, m);
         });
     }
     else if (message.op === 'prepend') {
         messages.reverse().forEach(function(m) {
-            model.insert(0, m);
+            sortedInsert(model, m);
         });
     }

[b100dian]

Hi @gw-gh , thanks for your deep analysis of what's going on!

You are probably right that there's something wrong with the order of messages, I myself fixed something around that in the master branch but it is unreleased (I assume you built the latest master and it had to be fixed with the message insertion in sorted order anyway?)

(Since you already have a working build, maybe you could also try to see if the error is happening on feature/storeage branch, since that is a very different depart to model messages that @danvratil started a while back and neither he nor myself got time to productize)