Merge pull request #2007 from deltachat/tweak-protection-msg

do not say, a concrete user has enabled protection if we do not really know

src/chat.rs

@@ -228,29 +228,31 @@ impl ChatId {
     ///
     /// This sends the message with the protected status change to the chat,
     /// notifying the user on this device as well as the other users in the chat.
-    /// If `promoted` is false this means the chat only exists on this device so far
+    ///
-    /// and does not need to be sent out.
+    /// If `promote` is false this means, the message must not be sent out
-    /// In this case an local info message is added to the chat.
+    /// and only a local info message should be added to the chat.
+    /// This is used when protection is enabled implicitly or when a chat is not yet promoted.
     pub(crate) async fn add_protection_msg(
         self,
         context: &Context,
         protect: ProtectionStatus,
-        promoted: bool,
+        promote: bool,
         from_id: u32,
     ) -> Result<(), Error> {
         let msg_text = context.stock_protection_msg(protect, from_id).await;
+        let cmd = match protect {
+            ProtectionStatus::Protected => SystemMessage::ChatProtectionEnabled,
+            ProtectionStatus::Unprotected => SystemMessage::ChatProtectionDisabled,
+        };
 
-        if promoted {
+        if promote {
             let mut msg = Message::default();
             msg.viewtype = Viewtype::Text;
             msg.text = Some(msg_text);
-            msg.param.set_cmd(match protect {
+            msg.param.set_cmd(cmd);
-                ProtectionStatus::Protected => SystemMessage::ChatProtectionEnabled,
-                ProtectionStatus::Unprotected => SystemMessage::ChatProtectionDisabled,
-            });
             send_msg(context, self, &mut msg).await?;
         } else {
-            add_info_msg(context, self, msg_text).await;
+            add_info_msg_with_cmd(context, self, msg_text, cmd).await?;
         }
 
         Ok(())
@@ -2899,18 +2901,22 @@ pub(crate) async fn delete_and_reset_all_device_msgs(context: &Context) -> Resul
 /// Adds an informational message to chat.
 ///
 /// For example, it can be a message showing that a member was added to a group.
-pub(crate) async fn add_info_msg(context: &Context, chat_id: ChatId, text: impl AsRef<str>) {
+pub(crate) async fn add_info_msg_with_cmd(
+    context: &Context,
+    chat_id: ChatId,
+    text: impl AsRef<str>,
+    cmd: SystemMessage,
+) -> Result<MsgId, Error> {
     let rfc724_mid = dc_create_outgoing_rfc724_mid(None, "@device");
-    let ephemeral_timer = match chat_id.get_ephemeral_timer(context).await {
+    let ephemeral_timer = chat_id.get_ephemeral_timer(context).await?;
-        Err(e) => {
-            warn!(context, "Could not get timer for info msg: {}", e);
-            return;
-        }
-        Ok(ephemeral_timer) => ephemeral_timer,
-    };
 
-    if let Err(e) = context.sql.execute(
+    let mut param = Params::new();
-        "INSERT INTO msgs (chat_id,from_id,to_id, timestamp,type,state, txt,rfc724_mid,ephemeral_timer) VALUES (?,?,?, ?,?,?, ?,?,?);",
+    if cmd != SystemMessage::Unknown {
+        param.set_cmd(cmd)
+    }
+
+    context.sql.execute(
+        "INSERT INTO msgs (chat_id,from_id,to_id, timestamp,type,state, txt,rfc724_mid,ephemeral_timer, param) VALUES (?,?,?, ?,?,?, ?,?,?, ?);",
         paramsv![
             chat_id,
             DC_CONTACT_ID_INFO,
@@ -2920,22 +2926,25 @@ pub(crate) async fn add_info_msg(context: &Context, chat_id: ChatId, text: impl
             MessageState::InNoticed,
             text.as_ref().to_string(),
             rfc724_mid,
-            ephemeral_timer
+            ephemeral_timer,
+            param.to_string(),
         ]
-    ).await {
+    ).await?;
-        warn!(context, "Could not add info msg: {}", e);
-        return;
-    }
 
     let row_id = context
         .sql
         .get_rowid(context, "msgs", "rfc724_mid", &rfc724_mid)
         .await
         .unwrap_or_default();
-    context.emit_event(EventType::MsgsChanged {
+    let msg_id = MsgId::new(row_id);
-        chat_id,
+    context.emit_event(EventType::MsgsChanged { chat_id, msg_id });
-        msg_id: MsgId::new(row_id),
+    Ok(msg_id)
-    });
+}
+
+pub(crate) async fn add_info_msg(context: &Context, chat_id: ChatId, text: impl AsRef<str>) {
+    if let Err(e) = add_info_msg_with_cmd(context, chat_id, text, SystemMessage::Unknown).await {
+        warn!(context, "Could not add info msg: {}", e);
+    }
 }
 
 #[cfg(test)]
@@ -3590,4 +3599,116 @@ mod tests {
             false
         );
     }
+
+    #[async_std::test]
+    async fn test_add_info_msg() {
+        let t = TestContext::new().await;
+        let chat_id = create_group_chat(&t.ctx, ProtectionStatus::Unprotected, "foo")
+            .await
+            .unwrap();
+        add_info_msg(&t.ctx, chat_id, "foo info").await;
+
+        let msg = t.get_last_msg(chat_id).await;
+        assert_eq!(msg.get_chat_id(), chat_id);
+        assert_eq!(msg.get_viewtype(), Viewtype::Text);
+        assert_eq!(msg.get_text().unwrap(), "foo info");
+        assert!(msg.is_info());
+        assert_eq!(msg.get_info_type(), SystemMessage::Unknown);
+    }
+
+    #[async_std::test]
+    async fn test_add_info_msg_with_cmd() {
+        let t = TestContext::new().await;
+        let chat_id = create_group_chat(&t.ctx, ProtectionStatus::Unprotected, "foo")
+            .await
+            .unwrap();
+        let msg_id = add_info_msg_with_cmd(
+            &t.ctx,
+            chat_id,
+            "foo bar info",
+            SystemMessage::EphemeralTimerChanged,
+        )
+        .await
+        .unwrap();
+
+        let msg = Message::load_from_db(&t.ctx, msg_id).await.unwrap();
+        assert_eq!(msg.get_chat_id(), chat_id);
+        assert_eq!(msg.get_viewtype(), Viewtype::Text);
+        assert_eq!(msg.get_text().unwrap(), "foo bar info");
+        assert!(msg.is_info());
+        assert_eq!(msg.get_info_type(), SystemMessage::EphemeralTimerChanged);
+
+        let msg2 = t.get_last_msg(chat_id).await;
+        assert_eq!(msg.get_id(), msg2.get_id());
+    }
+
+    #[async_std::test]
+    async fn test_set_protection() {
+        let t = TestContext::new_alice().await;
+        let chat_id = create_group_chat(&t.ctx, ProtectionStatus::Unprotected, "foo")
+            .await
+            .unwrap();
+        let chat = Chat::load_from_db(&t.ctx, chat_id).await.unwrap();
+        assert!(!chat.is_protected());
+        assert!(chat.is_unpromoted());
+
+        // enable protection on unpromoted chat, the info-message is added via add_info_msg()
+        chat_id
+            .set_protection(&t.ctx, ProtectionStatus::Protected)
+            .await
+            .unwrap();
+
+        let chat = Chat::load_from_db(&t.ctx, chat_id).await.unwrap();
+        assert!(chat.is_protected());
+        assert!(chat.is_unpromoted());
+
+        let msgs = get_chat_msgs(&t.ctx, chat_id, 0, None).await;
+        assert_eq!(msgs.len(), 1);
+
+        let msg = t.get_last_msg(chat_id).await;
+        assert!(msg.is_info());
+        assert_eq!(msg.get_info_type(), SystemMessage::ChatProtectionEnabled);
+        assert_eq!(msg.get_state(), MessageState::InNoticed);
+
+        // disable protection again, still unpromoted
+        chat_id
+            .set_protection(&t.ctx, ProtectionStatus::Unprotected)
+            .await
+            .unwrap();
+
+        let chat = Chat::load_from_db(&t.ctx, chat_id).await.unwrap();
+        assert!(!chat.is_protected());
+        assert!(chat.is_unpromoted());
+
+        let msg = t.get_last_msg(chat_id).await;
+        assert!(msg.is_info());
+        assert_eq!(msg.get_info_type(), SystemMessage::ChatProtectionDisabled);
+        assert_eq!(msg.get_state(), MessageState::InNoticed);
+
+        // send a message, this switches to promoted state
+        send_text_msg(&t.ctx, chat_id, "hi!".to_string())
+            .await
+            .unwrap();
+
+        let chat = Chat::load_from_db(&t.ctx, chat_id).await.unwrap();
+        assert!(!chat.is_protected());
+        assert!(!chat.is_unpromoted());
+
+        let msgs = get_chat_msgs(&t.ctx, chat_id, 0, None).await;
+        assert_eq!(msgs.len(), 3);
+
+        // enable protection on promoted chat, the info-message is sent via send_msg() this time
+        chat_id
+            .set_protection(&t.ctx, ProtectionStatus::Protected)
+            .await
+            .unwrap();
+        let chat = Chat::load_from_db(&t.ctx, chat_id).await.unwrap();
+        assert!(chat.is_protected());
+        assert!(!chat.is_unpromoted());
+
+        let msg = t.get_last_msg(chat_id).await;
+        assert!(msg.is_info());
+        assert_eq!(msg.get_info_type(), SystemMessage::ChatProtectionEnabled);
+        assert_eq!(msg.get_state(), MessageState::OutDelivered); // as bcc-self is disabled and there is nobody else in the chat
+    }
 }

src/dc_receive_imf.rs

@@ -1245,8 +1245,10 @@ async fn create_or_lookup_group(
         recreate_member_list = true;
 
         if create_protected == ProtectionStatus::Protected {
+            // set from_id=0 as it is not clear that the sender of this random group message
+            // actually really has enabled chat-protection at some point.
             chat_id
-                .add_protection_msg(context, ProtectionStatus::Protected, false, from_id)
+                .add_protection_msg(context, ProtectionStatus::Protected, false, 0)
                 .await?;
         }
     }

src/test_utils.rs

@@ -9,13 +9,15 @@ use async_std::path::PathBuf;
 use async_std::sync::RwLock;
 use tempfile::{tempdir, TempDir};
 
-use crate::chat::ChatId;
+use crate::chat;
+use crate::chat::{ChatId, ChatItem};
 use crate::config::Config;
 use crate::context::Context;
 use crate::dc_receive_imf::dc_receive_imf;
 use crate::dc_tools::EmailAddress;
 use crate::job::Action;
 use crate::key::{self, DcKey};
+use crate::message::Message;
 use crate::mimeparser::MimeMessage;
 use crate::param::{Param, Params};
 
@@ -187,6 +189,19 @@ impl TestContext {
             .await
             .unwrap();
     }
+
+    /// Get the most recent message of a chat.
+    ///
+    /// Panics on errors or if the most recent message is a marker.
+    pub async fn get_last_msg(&self, chat_id: ChatId) -> Message {
+        let msgs = chat::get_chat_msgs(&self.ctx, chat_id, 0, None).await;
+        let msg_id = if let ChatItem::Message { msg_id } = msgs.last().unwrap() {
+            msg_id
+        } else {
+            panic!("Wrong item type");
+        };
+        Message::load_from_db(&self.ctx, *msg_id).await.unwrap()
+    }
 }
 
 /// A raw message as it was scheduled to be sent.