fix: Sort old incoming messages below all outgoing ones (#4621)

If the Inbox is fetched before the Sentbox (as done currently), messages from the Sentbox will
correctly mingle with the Inbox messages in the end. So, this commit changes message ordering only
if we already have processed outgoing messages, e.g. if we just sent them in the chat as described
in #4621. Otherwise new incoming messages are displayed somewhere in the middle of the chat which
doesn't look usable.

src/receive_imf.rs

@@ -1400,17 +1400,19 @@ async fn calc_sort_timestamp(
             )
             .await?
     } else if incoming {
-        // get newest incoming non fresh message for this chat.
+        // get newest non fresh message for this chat.
 
-        // If a user hasn't been online for some time, the Inbox is
-        // fetched first and then the Sentbox. In order for Inbox
-        // and Sent messages to be allowed to mingle,
-        // outgoing messages are purely sorted by their sent timestamp.
+        // If a user hasn't been online for some time, the Inbox is fetched first and then the
+        // Sentbox. In order for Inbox and Sent messages to be allowed to mingle, outgoing messages
+        // are purely sorted by their sent timestamp. NB: The Inbox must be fetched first otherwise
+        // Inbox messages would be always below old Sentbox messages. We could take in the query
+        // below only incoming messages, but then new incoming messages would mingle with just sent
+        // outgoing ones and apear somewhere in the middle of the chat.
         context
             .sql
             .query_get_value(
-                "SELECT MAX(timestamp) FROM msgs WHERE chat_id=? AND state>? AND from_id!=?",
-                (chat_id, MessageState::InFresh, ContactId::SELF),
+                "SELECT MAX(timestamp) FROM msgs WHERE chat_id=? AND state>?",
+                (chat_id, MessageState::InFresh),
             )
             .await?
     } else {

src/tests/verified_chats.rs

@@ -480,6 +480,46 @@ async fn test_old_message_4() -> Result<()> {
     Ok(())
 }
 
+/// Alice is offline for some time.
+/// When they come online, first their sentbox is synced and then their inbox.
+/// This test tests that the messages are still in the right order.
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_old_message_5() -> Result<()> {
+    let alice = TestContext::new_alice().await;
+    let msg_sent = receive_imf(
+        &alice,
+        b"From: alice@example.org\n\
+          To: Bob <bob@example.net>\n\
+          Message-ID: <1234-2-4@example.org>\n\
+          Date: Sat, 07 Dec 2019 19:00:27 +0000\n\
+          \n\
+          Happy birthday, Bob!\n",
+        true,
+    )
+    .await?
+    .unwrap();
+
+    let msg_incoming = receive_imf(
+        &alice,
+        b"From: Bob <bob@example.net>\n\
+          To: alice@example.org\n\
+          Message-ID: <1234-2-3@example.org>\n\
+          Date: Sun, 07 Dec 2019 19:00:26 +0000\n\
+          \n\
+          Happy birthday to me, Alice!\n",
+        false,
+    )
+    .await?
+    .unwrap();
+
+    assert!(msg_sent.sort_timestamp == msg_incoming.sort_timestamp);
+    alice
+        .golden_test_chat(msg_sent.chat_id, "test_old_message_5")
+        .await;
+
+    Ok(())
+}
+
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn test_mdn_doesnt_disable_verification() -> Result<()> {
     let mut tcm = TestContextManager::new();

test-data/golden/test_old_message_5

@@ -0,0 +1,5 @@
+Single#Chat#10: Bob [bob@example.net] 
+--------------------------------------------------------------------------------
+Msg#10: Me (Contact#Contact#Self): Happy birthday, Bob!  √
+Msg#11:  (Contact#Contact#10): Happy birthday to me, Alice! [FRESH]
+--------------------------------------------------------------------------------