fix: Manipulate sort_timestamp to not be 0

This is to avoid sorting incoming messages that
are slightly in the past above system messages
about SecureJoin. SecureJoin messages are
timed according to smeared timestamp,
so even in the local tests they are in the future
by a few seconds.

src/chat.rs

@@ -477,12 +477,17 @@ impl ChatId {
     /// Adds message "Messages are end-to-end encrypted".
     pub(crate) async fn add_e2ee_notice(self, context: &Context, timestamp: i64) -> Result<()> {
         let text = stock_str::messages_e2ee_info_msg(context);
+
+        // Sort this notice to the very beginning of the chat.
+        // We don't want any message to appear before this notice
+        // which is normally added when encrypted chat is created.
+        let sort_timestamp = 0;
         add_info_msg_with_cmd(
             context,
             self,
             &text,
             SystemMessage::ChatE2ee,
-            Some(timestamp),
+            Some(sort_timestamp),
             timestamp,
             None,
             None,
@@ -925,6 +930,17 @@ SELECT id, rfc724_mid, pre_rfc724_mid, timestamp, ?, 1 FROM msgs WHERE chat_id=?
             .unwrap_or(0))
     }
 
+    /// Returns timestamp of us joining the chat if we are the member of the chat.
+    pub(crate) async fn join_timestamp(self, context: &Context) -> Result<Option<i64>> {
+        context
+            .sql
+            .query_get_value(
+                "SELECT add_timestamp FROM chats_contacts WHERE chat_id=? AND contact_id=?",
+                (self, ContactId::SELF),
+            )
+            .await
+    }
+
     /// Returns timestamp of the latest message in the chat,
     /// including hidden messages or a draft if there is one.
     pub(crate) async fn get_timestamp(self, context: &Context) -> Result<Option<i64>> {
@@ -1212,15 +1228,11 @@ SELECT id, rfc724_mid, pre_rfc724_mid, timestamp, ?, 1 FROM msgs WHERE chat_id=?
     /// corresponding event in case of a system message (usually the current system time).
     /// `always_sort_to_bottom` makes this adjust the returned timestamp up so that the message goes
     /// to the chat bottom.
-    /// `received` -- whether the message is received. Otherwise being sent.
-    /// `incoming` -- whether the message is incoming.
     pub(crate) async fn calc_sort_timestamp(
         self,
         context: &Context,
         message_timestamp: i64,
         always_sort_to_bottom: bool,
-        received: bool,
-        incoming: bool,
     ) -> Result<i64> {
         let mut sort_timestamp = cmp::min(message_timestamp, smeared_time(context));
 
@@ -1240,38 +1252,6 @@ SELECT id, rfc724_mid, pre_rfc724_mid, timestamp, ?, 1 FROM msgs WHERE chat_id=?
                     (self, MessageState::OutDraft),
                 )
                 .await?
-        } else if received {
-            // Received messages shouldn't mingle with just sent ones and appear somewhere in the
-            // middle of the chat, so we go after the newest non fresh message.
-            //
-            // But if a received outgoing message is older than some seen message, better sort the
-            // received message purely by timestamp. We could place it just before that seen
-            // message, but anyway the user may not notice it.
-            //
-            // NB: Received outgoing messages may break sorting of fresh incoming ones, but this
-            // shouldn't happen frequently. Seen incoming messages don't really break sorting of
-            // fresh ones, they rather mean that older incoming messages are actually seen as well.
-            context
-                .sql
-                .query_row_optional(
-                    "SELECT MAX(timestamp), MAX(IIF(state=?,timestamp_sent,0))
-                     FROM msgs
-                     WHERE chat_id=? AND hidden=0 AND state>?
-                     HAVING COUNT(*) > 0",
-                    (MessageState::InSeen, self, MessageState::InFresh),
-                    |row| {
-                        let ts: i64 = row.get(0)?;
-                        let ts_sent_seen: i64 = row.get(1)?;
-                        Ok((ts, ts_sent_seen))
-                    },
-                )
-                .await?
-                .and_then(|(ts, ts_sent_seen)| {
-                    match incoming || ts_sent_seen <= message_timestamp {
-                        true => Some(ts),
-                        false => None,
-                    }
-                })
         } else {
             None
         };
@@ -1282,7 +1262,16 @@ SELECT id, rfc724_mid, pre_rfc724_mid, timestamp, ?, 1 FROM msgs WHERE chat_id=?
             sort_timestamp = last_msg_time;
         }
 
-        Ok(sort_timestamp)
+        if let Some(join_timestamp) = self.join_timestamp(context).await? {
+            // If we are the member of the chat, don't add messages
+            // before the timestamp of us joining it.
+            // This is needed to avoid sorting "Member added"
+            // or automatically sent bot welcome messages
+            // above SecureJoin system messages.
+            Ok(std::cmp::max(sort_timestamp, join_timestamp))
+        } else {
+            Ok(sort_timestamp)
+        }
     }
 }
 
@@ -4938,15 +4927,8 @@ pub(crate) async fn add_info_msg_with_cmd(
         ts
     } else {
         let sort_to_bottom = true;
-        let (received, incoming) = (false, false);
         chat_id
-            .calc_sort_timestamp(
-                context,
-                smeared_time(context),
-                sort_to_bottom,
-                received,
-                incoming,
-            )
+            .calc_sort_timestamp(context, smeared_time(context), sort_to_bottom)
             .await?
     };
 

src/chatlist.rs

@@ -142,7 +142,7 @@ impl Chatlist {
                    AND c.blocked!=1
                    AND c.id IN(SELECT chat_id FROM chats_contacts WHERE contact_id=?2 AND add_timestamp >= remove_timestamp)
                  GROUP BY c.id
-                 ORDER BY c.archived=?3 DESC, IFNULL(m.timestamp,c.created_timestamp) DESC, m.id DESC;",
+                 ORDER BY c.archived=?3 DESC, IFNULL(NULLIF(m.timestamp,0),c.created_timestamp) DESC, m.id DESC;",
                 (MessageState::OutDraft, query_contact_id, ChatVisibility::Pinned),
                 process_row,
             ).await?
@@ -168,7 +168,7 @@ impl Chatlist {
                    AND c.blocked!=1
                    AND c.archived=1
                  GROUP BY c.id
-                 ORDER BY IFNULL(m.timestamp,c.created_timestamp) DESC, m.id DESC;",
+                 ORDER BY IFNULL(NULLIF(m.timestamp,0),c.created_timestamp) DESC, m.id DESC;",
                     (MessageState::OutDraft,),
                     process_row,
                 )
@@ -204,7 +204,7 @@ impl Chatlist {
                    AND IFNULL(c.name_normalized,c.name) LIKE ?3
                    AND (NOT ?4 OR EXISTS (SELECT 1 FROM msgs m WHERE m.chat_id = c.id AND m.state == ?5 AND hidden=0))
                  GROUP BY c.id
-                 ORDER BY IFNULL(m.timestamp,c.created_timestamp) DESC, m.id DESC;",
+                 ORDER BY IFNULL(NULLIF(m.timestamp,0),c.created_timestamp) DESC, m.id DESC;",
                     (MessageState::OutDraft, skip_id, str_like_cmd, only_unread, MessageState::InFresh),
                     process_row,
                 )
@@ -253,7 +253,7 @@ impl Chatlist {
                        AND NOT c.archived=?
                        AND (c.type!=? OR c.id IN(SELECT chat_id FROM chats_contacts WHERE contact_id=? AND add_timestamp >= remove_timestamp))
                      GROUP BY c.id
-                     ORDER BY c.id=? DESC, c.archived=? DESC, IFNULL(m.timestamp,c.created_timestamp) DESC, m.id DESC;",
+                     ORDER BY c.id=? DESC, c.archived=? DESC, IFNULL(NULLIF(m.timestamp,0),c.created_timestamp) DESC, m.id DESC;",
                     (
                         MessageState::OutDraft, skip_id, ChatVisibility::Archived,
                         Chattype::Group, ContactId::SELF,
@@ -279,7 +279,7 @@ impl Chatlist {
                        AND (c.blocked=0 OR c.blocked=2)
                        AND NOT c.archived=?
                      GROUP BY c.id
-                     ORDER BY c.id=0 DESC, c.archived=? DESC, IFNULL(m.timestamp,c.created_timestamp) DESC, m.id DESC;",
+                     ORDER BY c.id=0 DESC, c.archived=? DESC, IFNULL(NULLIF(m.timestamp,0),c.created_timestamp) DESC, m.id DESC;",
                     (MessageState::OutDraft, skip_id, ChatVisibility::Archived, ChatVisibility::Pinned),
                     process_row,
                 ).await?

src/message.rs

@@ -815,7 +815,11 @@ impl Message {
 
     /// Returns the timestamp of the message for sorting.
     pub fn get_sort_timestamp(&self) -> i64 {
-        self.timestamp_sort
+        if self.timestamp_sort != 0 {
+            self.timestamp_sort
+        } else {
+            self.timestamp_sent
+        }
     }
 
     /// Returns the text of the message.

src/receive_imf.rs

@@ -1835,6 +1835,16 @@ async fn add_parts(
         }
     }
 
+    // Sort message to the bottom if we are not in the chat
+    // so if we are added via QR code scan
+    // the message about our addition goes after all the info messages.
+    // Info messages are sorted by local smeared_timestamp()
+    // which advances quickly during SecureJoin,
+    // while "member added" message may have older timestamp
+    // corresponding to the sender clock.
+    // In practice inviter clock may even be slightly in the past.
+    let sort_to_bottom = !chat.is_self_in_chat(context).await?;
+
     let is_location_kml = mime_parser.location_kml.is_some();
     let mut group_changes = match chat.typ {
         _ if chat.id.is_special() => GroupChangesInfo::default(),
@@ -1885,16 +1895,8 @@ async fn add_parts(
     };
     let in_fresh = state == MessageState::InFresh;
 
-    let sort_to_bottom = false;
-    let received = true;
     let sort_timestamp = chat_id
-        .calc_sort_timestamp(
-            context,
-            mime_parser.timestamp_sent,
-            sort_to_bottom,
-            received,
-            mime_parser.incoming,
-        )
+        .calc_sort_timestamp(context, mime_parser.timestamp_sent, sort_to_bottom)
         .await?;
 
     // Apply ephemeral timer changes to the chat.

src/receive_imf/receive_imf_tests.rs

@@ -819,9 +819,12 @@ async fn load_imf_email(context: &Context, imf_raw: &[u8]) -> Message {
         .set_config(Config::ShowEmails, Some("2"))
         .await
         .unwrap();
-    receive_imf(context, imf_raw, false).await.unwrap();
-    let chats = Chatlist::try_load(context, 0, None, None).await.unwrap();
-    let msg_id = chats.get_msg_id(0).unwrap().unwrap();
+    let received_msg = receive_imf(context, imf_raw, false)
+        .await
+        .expect("receive_imf failure")
+        .expect("No message received");
+    assert_eq!(received_msg.msg_ids.len(), 1);
+    let msg_id = received_msg.msg_ids[0];
     Message::load_from_db(context, msg_id).await.unwrap()
 }
 
@@ -2872,9 +2875,8 @@ async fn test_rfc1847_encapsulation() -> Result<()> {
 
     // Alice sends a message to Bob using Thunderbird.
     let raw = include_bytes!("../../test-data/message/rfc1847_encapsulation.eml");
-    receive_imf(bob, raw, false).await?;
 
-    let msg = bob.get_last_msg().await;
+    let msg = load_imf_email(bob, raw).await;
     assert!(msg.get_showpadlock());
 
     Ok(())
@@ -3082,8 +3084,8 @@ async fn test_auto_accept_for_bots() -> Result<()> {
 async fn test_auto_accept_group_for_bots() -> Result<()> {
     let t = TestContext::new_alice().await;
     t.set_config(Config::Bot, Some("1")).await.unwrap();
-    receive_imf(&t, GRP_MAIL, false).await?;
-    let msg = t.get_last_msg().await;
+    let msg = load_imf_email(&t, GRP_MAIL).await;
+
     let chat = chat::Chat::load_from_db(&t, msg.chat_id).await?;
     assert!(!chat.is_contact_request());
     Ok(())
@@ -3556,9 +3558,9 @@ async fn test_messed_up_message_id() -> Result<()> {
     let t = TestContext::new_bob().await;
 
     let raw = include_bytes!("../../test-data/message/messed_up_message_id.eml");
-    receive_imf(&t, raw, false).await?;
+    let msg = load_imf_email(&t, raw).await;
     assert_eq!(
-        t.get_last_msg().await.rfc724_mid,
+        msg.rfc724_mid,
         "0bb9ffe1-2596-d997-95b4-1fef8cc4808e@example.org"
     );
 

src/tests/verified_chats.rs

@@ -246,46 +246,6 @@ async fn test_old_message_4() -> Result<()> {
     Ok(())
 }
 
-/// Alice is offline for some time.
-/// When they come online, first their mvbox 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();

src/tools/tools_tests.rs

@@ -64,9 +64,11 @@ DKIM Results: Passed=true";
 
 async fn check_parse_receive_headers_integration(raw: &[u8], expected: &str) {
     let t = TestContext::new_alice().await;
-    receive_imf(&t, raw, false).await.unwrap();
-    let msg = t.get_last_msg().await;
-    let msg_info = msg.id.get_info(&t).await.unwrap();
+    let received = receive_imf(&t, raw, false).await.unwrap().unwrap();
+
+    assert_eq!(received.msg_ids.len(), 1);
+    let msg_id = received.msg_ids[0];
+    let msg_info = msg_id.get_info(&t).await.unwrap();
 
     // Ignore the first rows of the msg_info because they contain a
     // received time that depends on the test time which makes it impossible to

test-data/golden/receive_imf_delayed_removal_is_ignored

@@ -2,9 +2,9 @@ Group#Chat#1001: Group [5 member(s)]
 --------------------------------------------------------------------------------
 Msg#1001: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO]
 Msg#1002🔒: Me (Contact#Contact#Self): populate  √
+Msg#1007🔒:  (Contact#Contact#1001): Member fiona@example.net removed by bob@example.net. [FRESH][INFO]
 Msg#1003: info (Contact#Contact#Info): Member dom@example.net added. [NOTICED][INFO]
 Msg#1004: info (Contact#Contact#Info): Member fiona@example.net removed. [NOTICED][INFO]
 Msg#1005🔒:  (Contact#Contact#1001): Member elena@example.net added by bob@example.net. [FRESH][INFO]
 Msg#1006🔒: Me (Contact#Contact#Self): You added member fiona@example.net. [INFO] o
-Msg#1007🔒:  (Contact#Contact#1001): Member fiona@example.net removed by bob@example.net. [FRESH][INFO]
 --------------------------------------------------------------------------------

test-data/golden/receive_imf_older_message_from_2nd_device

@@ -1,5 +1,5 @@
 Single#Chat#1001: bob@example.net [bob@example.net] Icon: 4138c52e5bc1c576cda7dd44d088c07.png
 --------------------------------------------------------------------------------
-Msg#1001: Me (Contact#Contact#Self): We share this account  √
 Msg#1002: Me (Contact#Contact#Self): I'm Alice too  √
+Msg#1001: Me (Contact#Contact#Self): We share this account  √
 --------------------------------------------------------------------------------

test-data/golden/test_old_message_5

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

test-data/golden/test_sync_broadcast_alice1

@@ -1,7 +1,7 @@
 OutBroadcast#Chat#1001: Channel [0 member(s)]
 --------------------------------------------------------------------------------
 Msg#1001: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO]
-Msg#1006🔒: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √
 Msg#1008🔒: Me (Contact#Contact#Self): hi  √
+Msg#1006🔒: Me (Contact#Contact#Self): Member bob@example.net added. [INFO] √
 Msg#1009🔒: Me (Contact#Contact#Self): You removed member bob@example.net. [INFO] √
 --------------------------------------------------------------------------------