From 95b2a159303c3f36b91ad7936b224c826d3a4871 Mon Sep 17 00:00:00 2001
From: iequidoo <dgreshilov@gmail.com>
Date: Wed, 16 Aug 2023 22:59:46 -0300
Subject: [PATCH] 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                  | 16 +++++++-----
 src/tests/verified_chats.rs         | 40 +++++++++++++++++++++++++++++
 test-data/golden/test_old_message_5 |  5 ++++
 3 files changed, 54 insertions(+), 7 deletions(-)
 create mode 100644 test-data/golden/test_old_message_5

diff --git a/src/receive_imf.rs b/src/receive_imf.rs
index 7547e49ab..4a9d8311b 100644
--- a/src/receive_imf.rs
+++ b/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 {
diff --git a/src/tests/verified_chats.rs b/src/tests/verified_chats.rs
index cc58dcada..202bd11d4 100644
--- a/src/tests/verified_chats.rs
+++ b/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();
diff --git a/test-data/golden/test_old_message_5 b/test-data/golden/test_old_message_5
new file mode 100644
index 000000000..75d3c0af0
--- /dev/null
+++ b/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]
+--------------------------------------------------------------------------------