fix: Sort most system messages to the bottom of the chat

deltachat-ffi/deltachat.h

@@ -7712,7 +7712,7 @@ void dc_event_unref(dc_event_t* event);
 /// Used in summaries.
 #define DC_STR_REACTED_BY 177
 
-/// "Establishing guaranteed end-to-end encryption, please wait…"
+/// "Establishing connection, please wait…"
 ///
 /// Used as info message.
 #define DC_STR_SECUREJOIN_WAIT 190

deltachat-rpc-client/tests/test_securejoin.py

@@ -158,29 +158,29 @@ def check_account(ac, contact, inviter_side, please_wait_info_msg=False):
         chat = get_broadcast(ac)
         chat_msgs = chat.get_messages()
 
+        encrypted_msg = chat_msgs.pop(0).get_snapshot()
+        assert encrypted_msg.text == "Messages are end-to-end encrypted."
+        assert encrypted_msg.is_info
+
         if please_wait_info_msg:
             first_msg = chat_msgs.pop(0).get_snapshot()
-            assert first_msg.text == "Establishing guaranteed end-to-end encryption, please wait…"
+            assert first_msg.text == "Establishing connection, please wait…"
             assert first_msg.is_info
 
-        encrypted_msg = chat_msgs[0].get_snapshot()
-        assert encrypted_msg.text == "Messages are end-to-end encrypted."
-        assert encrypted_msg.is_info
-
-        member_added_msg = chat_msgs[1].get_snapshot()
+        member_added_msg = chat_msgs.pop(0).get_snapshot()
         if inviter_side:
             assert member_added_msg.text == f"Member {contact_snapshot.display_name} added."
         else:
             assert member_added_msg.text == "You joined the channel."
         assert member_added_msg.is_info
 
-        hello_msg = chat_msgs[2].get_snapshot()
+        hello_msg = chat_msgs.pop(0).get_snapshot()
         assert hello_msg.text == "Hello everyone!"
         assert not hello_msg.is_info
         assert hello_msg.show_padlock
         assert hello_msg.error is None
 
-        assert len(chat_msgs) == 3
+        assert len(chat_msgs) == 0
 
         chat_snapshot = chat.get_full_snapshot()
         assert chat_snapshot.is_encrypted

deltachat-rpc-client/tests/test_something.py

@@ -867,15 +867,15 @@ def check_account(ac, contact, inviter_side, please_wait_info_msg=False):
         contact_snapshot = contact.get_snapshot()
         chat_msgs = chat.get_messages()
 
-        if please_wait_info_msg:
-            first_msg = chat_msgs.pop(0).get_snapshot()
-            assert first_msg.text == "Establishing guaranteed end-to-end encryption, please wait…"
-            assert first_msg.is_info
-
         encrypted_msg = chat_msgs.pop(0).get_snapshot()
         assert encrypted_msg.text == "Messages are end-to-end encrypted."
         assert encrypted_msg.is_info
 
+        if please_wait_info_msg:
+            first_msg = chat_msgs.pop(0).get_snapshot()
+            assert first_msg.text == "Establishing connection, please wait…"
+            assert first_msg.is_info
+
         member_added_msg = chat_msgs.pop(0).get_snapshot()
         if inviter_side:
             assert member_added_msg.text == f"Member {contact_snapshot.display_name} added."

src/chat.rs

@@ -301,7 +301,7 @@ impl ChatId {
         let chat = Chat::load_from_db(context, chat_id).await?;
 
         if chat.is_encrypted(context).await? {
-            chat_id.add_encrypted_msg(context, timestamp).await?;
+            chat_id.add_e2ee_notice(context, timestamp).await?;
         }
 
         info!(
@@ -462,19 +462,15 @@ impl ChatId {
     }
 
     /// Adds message "Messages are end-to-end encrypted".
-    pub(crate) async fn add_encrypted_msg(
-        self,
-        context: &Context,
-        timestamp_sort: i64,
-    ) -> Result<()> {
+    pub(crate) async fn add_e2ee_notice(self, context: &Context, timestamp: i64) -> Result<()> {
         let text = stock_str::messages_e2e_encrypted(context).await;
         add_info_msg_with_cmd(
             context,
             self,
             &text,
             SystemMessage::ChatE2ee,
-            timestamp_sort,
-            None,
+            Some(timestamp),
+            timestamp,
             None,
             None,
             None,
@@ -2425,7 +2421,7 @@ impl ChatIdBlocked {
             && !chat.param.exists(Param::Devicetalk)
             && !chat.param.exists(Param::Selftalk)
         {
-            chat_id.add_encrypted_msg(context, smeared_time).await?;
+            chat_id.add_e2ee_notice(context, smeared_time).await?;
         }
 
         Ok(Self {
@@ -3452,7 +3448,7 @@ pub(crate) async fn create_group_ex(
 
     if !grpid.is_empty() {
         // Add "Messages are end-to-end encrypted." message.
-        chat_id.add_encrypted_msg(context, timestamp).await?;
+        chat_id.add_e2ee_notice(context, timestamp).await?;
     }
 
     if !context.get_config_bool(Config::Bot).await?
@@ -3465,7 +3461,7 @@ pub(crate) async fn create_group_ex(
             // Add "Messages in this chat use classic email and are not encrypted." message.
             stock_str::chat_unencrypted_explanation(context).await
         };
-        add_info_msg(context, chat_id, &text, create_smeared_timestamp(context)).await?;
+        add_info_msg(context, chat_id, &text).await?;
     }
     if let (true, true) = (sync.into(), !grpid.is_empty()) {
         let id = SyncId::Grpid(grpid);
@@ -3533,7 +3529,7 @@ pub(crate) async fn create_out_broadcast_ex(
         Ok(chat_id)
     };
     let chat_id = context.sql.transaction(trans_fn).await?;
-    chat_id.add_encrypted_msg(context, timestamp).await?;
+    chat_id.add_e2ee_notice(context, timestamp).await?;
 
     context.emit_msgs_changed_without_ids();
     chatlist_events::emit_chatlist_changed(context);
@@ -4616,9 +4612,11 @@ pub(crate) async fn add_info_msg_with_cmd(
     chat_id: ChatId,
     text: &str,
     cmd: SystemMessage,
-    timestamp_sort: i64,
-    // Timestamp to show to the user (if this is None, `timestamp_sort` will be shown to the user)
-    timestamp_sent_rcvd: Option<i64>,
+    // Timestamp where in the chat the message will be sorted.
+    // If this is None, the message will be sorted to the bottom.
+    timestamp_sort: Option<i64>,
+    // Timestamp to show to the user
+    timestamp_sent_rcvd: i64,
     parent: Option<&Message>,
     from_id: Option<ContactId>,
     added_removed_id: Option<ContactId>,
@@ -4634,6 +4632,22 @@ pub(crate) async fn add_info_msg_with_cmd(
         param.set(Param::ContactAddedRemoved, contact_id.to_u32().to_string());
     }
 
+    let timestamp_sort = if let Some(ts) = timestamp_sort {
+        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,
+            )
+            .await?
+    };
+
     let row_id =
     context.sql.insert(
         "INSERT INTO msgs (chat_id,from_id,to_id,timestamp,timestamp_sent,timestamp_rcvd,type,state,txt,txt_normalized,rfc724_mid,ephemeral_timer,param,mime_in_reply_to)
@@ -4643,8 +4657,8 @@ pub(crate) async fn add_info_msg_with_cmd(
             from_id.unwrap_or(ContactId::INFO),
             ContactId::INFO,
             timestamp_sort,
-            timestamp_sent_rcvd.unwrap_or(0),
-            timestamp_sent_rcvd.unwrap_or(0),
+            timestamp_sent_rcvd,
+            timestamp_sent_rcvd,
             Viewtype::Text,
             MessageState::InNoticed,
             text,
@@ -4664,19 +4678,14 @@ pub(crate) async fn add_info_msg_with_cmd(
 }
 
 /// Adds info message with a given text and `timestamp` to the chat.
-pub(crate) async fn add_info_msg(
-    context: &Context,
-    chat_id: ChatId,
-    text: &str,
-    timestamp: i64,
-) -> Result<MsgId> {
+pub(crate) async fn add_info_msg(context: &Context, chat_id: ChatId, text: &str) -> Result<MsgId> {
     add_info_msg_with_cmd(
         context,
         chat_id,
         text,
         SystemMessage::Unknown,
-        timestamp,
         None,
+        time(),
         None,
         None,
         None,

src/chat/chat_tests.rs

@@ -1238,7 +1238,7 @@ async fn test_unarchive_if_muted() -> Result<()> {
     chat_id.set_visibility(&t, ChatVisibility::Archived).await?;
     set_muted(&t, chat_id, MuteDuration::Forever).await?;
     send_text_msg(&t, chat_id, "out".to_string()).await?;
-    add_info_msg(&t, chat_id, "info", time()).await?;
+    add_info_msg(&t, chat_id, "info").await?;
     assert_eq!(get_archived_cnt(&t).await?, 1);
 
     // finally, unarchive on sending to not muted chat
@@ -1637,7 +1637,7 @@ async fn test_set_mute_duration() {
 async fn test_add_info_msg() -> Result<()> {
     let t = TestContext::new().await;
     let chat_id = create_group(&t, "foo").await?;
-    add_info_msg(&t, chat_id, "foo info", time()).await?;
+    add_info_msg(&t, chat_id, "foo info").await?;
 
     let msg = t.get_last_msg_in(chat_id).await;
     assert_eq!(msg.get_chat_id(), chat_id);
@@ -1659,8 +1659,8 @@ async fn test_add_info_msg_with_cmd() -> Result<()> {
         chat_id,
         "foo bar info",
         SystemMessage::EphemeralTimerChanged,
-        time(),
         None,
+        time(),
         None,
         None,
         None,
@@ -4507,7 +4507,7 @@ async fn test_info_not_referenced() -> Result<()> {
 
     let bob_received_message = tcm.send_recv_accept(alice, bob, "Hi!").await;
     let bob_chat_id = bob_received_message.chat_id;
-    add_info_msg(bob, bob_chat_id, "Some info", create_smeared_timestamp(bob)).await?;
+    add_info_msg(bob, bob_chat_id, "Some info").await?;
 
     // Bob sends a message.
     // This message should reference Alice's "Hi!" message and not the info message.

src/location.rs

@@ -294,7 +294,7 @@ pub async fn send_locations_to_chat(
             .unwrap_or_default();
     } else if 0 == seconds && is_sending_locations_before {
         let stock_str = stock_str::msg_location_disabled(context).await;
-        chat::add_info_msg(context, chat_id, &stock_str, now).await?;
+        chat::add_info_msg(context, chat_id, &stock_str).await?;
     }
     context.emit_event(EventType::ChatModified(chat_id));
     chatlist_events::emit_chatlist_item_changed(context, chat_id);
@@ -849,7 +849,7 @@ async fn maybe_send_locations(context: &Context) -> Result<Option<u64>> {
                 .context("failed to disable location streaming")?;
 
             let stock_str = stock_str::msg_location_disabled(context).await;
-            chat::add_info_msg(context, chat_id, &stock_str, now).await?;
+            chat::add_info_msg(context, chat_id, &stock_str).await?;
             context.emit_event(EventType::ChatModified(chat_id));
             chatlist_events::emit_chatlist_item_changed(context, chat_id);
         }

src/mimefactory.rs

@@ -1239,7 +1239,7 @@ impl MimeFactory {
                         // created before we had symmetric encryption,
                         // we show an error message.
                         let text = BROADCAST_INCOMPATIBILITY_MSG;
-                        chat::add_info_msg(context, chat.id, text, time()).await?;
+                        chat::add_info_msg(context, chat.id, text).await?;
                         bail!(text);
                     }
                     secret

src/receive_imf.rs

@@ -1777,11 +1777,16 @@ async fn add_parts(
                     "Updated ephemeral timer to {ephemeral_timer:?} for chat {chat_id}."
                 );
                 if mime_parser.is_system_message != SystemMessage::EphemeralTimerChanged {
-                    chat::add_info_msg(
+                    chat::add_info_msg_with_cmd(
                         context,
                         chat_id,
                         &stock_ephemeral_timer_changed(context, ephemeral_timer, from_id).await,
-                        sort_timestamp,
+                        SystemMessage::Unknown,
+                        Some(sort_timestamp),
+                        mime_parser.timestamp_sent,
+                        None,
+                        None,
+                        None,
                     )
                     .await?;
                 }
@@ -1889,8 +1894,8 @@ async fn add_parts(
             chat_id,
             &group_changes_msg,
             cmd,
-            sort_timestamp,
-            None,
+            Some(sort_timestamp),
+            mime_parser.timestamp_sent,
             None,
             None,
             added_removed_id,

src/securejoin.rs

@@ -118,7 +118,7 @@ pub async fn get_securejoin_qr(context: &Context, chat: Option<ChatId>) -> Resul
                         chat.id,
                     );
                     let text = BROADCAST_INCOMPATIBILITY_MSG;
-                    add_info_msg(context, chat.id, text, time()).await?;
+                    add_info_msg(context, chat.id, text).await?;
                     bail!(text.to_string());
                 }
             }

src/securejoin/bob.rs

@@ -127,7 +127,7 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul
         QrInvite::Group { .. } => {
             let joining_chat_id = joining_chat_id(context, &invite, private_chat_id).await?;
             let msg = stock_str::secure_join_started(context, invite.contact_id()).await;
-            chat::add_info_msg(context, joining_chat_id, &msg, time()).await?;
+            chat::add_info_msg(context, joining_chat_id, &msg).await?;
             Ok(joining_chat_id)
         }
         QrInvite::Broadcast { .. } => {
@@ -145,31 +145,23 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul
 
             // If we were not in the broadcast channel before, show a 'please wait' info message.
             // Since we don't have any specific stock string for this,
-            // use the generic `Establishing guaranteed end-to-end encryption, please wait…`
+            // use the generic `Establishing connection, please wait…`
             if !is_contact_in_chat(context, joining_chat_id, ContactId::SELF).await? {
                 let msg = stock_str::securejoin_wait(context).await;
-                chat::add_info_msg(context, joining_chat_id, &msg, time()).await?;
+                chat::add_info_msg(context, joining_chat_id, &msg).await?;
             }
             Ok(joining_chat_id)
         }
         QrInvite::Contact { .. } => {
             // For setup-contact the BobState already ensured the 1:1 chat exists because it
             // uses it to send the handshake messages.
-            // Calculate the sort timestamp before checking the chat protection status so that if we
-            // race with its change, we don't add our message below the protection message.
-            let sort_to_bottom = true;
-            let (received, incoming) = (false, false);
-            let ts_sort = private_chat_id
-                .calc_sort_timestamp(context, 0, sort_to_bottom, received, incoming)
-                .await?;
-            let ts_start = time();
             chat::add_info_msg_with_cmd(
                 context,
                 private_chat_id,
                 &stock_str::securejoin_wait(context).await,
                 SystemMessage::SecurejoinWait,
-                ts_sort,
-                Some(ts_start),
+                None,
+                time(),
                 None,
                 None,
                 None,
@@ -243,7 +235,7 @@ pub(super) async fn handle_auth_required(
                 let contact_id = invite.contact_id();
                 let msg = stock_str::secure_join_replies(context, contact_id).await;
                 let chat_id = joining_chat_id(context, &invite, chat_id).await?;
-                chat::add_info_msg(context, chat_id, &msg, time()).await?;
+                chat::add_info_msg(context, chat_id, &msg).await?;
             }
         }
 

src/smtp.rs

@@ -443,11 +443,11 @@ pub(crate) async fn send_msg_to_smtp(
                         chat_id,
                         &text,
                         crate::mimeparser::SystemMessage::InvalidUnencryptedMail,
+                        Some(timestamp_sort),
                         timestamp_sort,
                         None,
                         None,
                         None,
-                        None,
                     )
                     .await?;
                 };

src/stock_str.rs

@@ -392,7 +392,7 @@ pub enum StockMessage {
     #[strum(props(fallback = "Member %1$s removed."))]
     MsgDelMember = 178,
 
-    #[strum(props(fallback = "Establishing guaranteed end-to-end encryption, please wait…"))]
+    #[strum(props(fallback = "Establishing connection, please wait…"))]
     SecurejoinWait = 190,
 
     #[strum(props(fallback = "❤️ Seems you're enjoying Delta Chat!
@@ -811,7 +811,7 @@ pub(crate) async fn secure_join_replies(context: &Context, contact_id: ContactId
         .replace1(&contact_id.get_stock_name(context).await)
 }
 
-/// Stock string: `Establishing guaranteed end-to-end encryption, please wait…`.
+/// Stock string: `Establishing connection, please wait…`.
 pub(crate) async fn securejoin_wait(context: &Context) -> String {
     translated(context, StockMessage::SecurejoinWait).await
 }