fix: Don't make message OutDelivered after successful resending to new broadcast member

Follow-up to 970222f. We shouldn't update the message state to `OutDelivered` if message sending
failed before and the message was re-sent successfully only to a new broadcast member.

src/chat/chat_tests.rs

@@ -3049,6 +3049,31 @@ async fn test_broadcast_resend_to_new_member() -> Result<()> {
     Ok(())
 }
 
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_broadcast_resend_failed_msg_to_new_member() -> Result<()> {
+    let mut tcm = TestContextManager::new();
+    let alice = &tcm.alice().await;
+    let bob = &tcm.bob().await;
+    let fiona = &tcm.fiona().await;
+    let alice_bc_id = create_broadcast(alice, "bc".to_string()).await?;
+    let qr = get_securejoin_qr(alice, Some(alice_bc_id)).await.unwrap();
+
+    tcm.exec_securejoin_qr(bob, alice, &qr).await;
+    let alice_msg_id = alice.send_text(alice_bc_id, "text").await.sender_msg_id;
+    let mut msg = Message::load_from_db(alice, alice_msg_id).await?;
+    message::set_msg_failed(alice, &mut msg, "error").await?;
+    let fiona_bc_id = tcm.exec_securejoin_qr(fiona, alice, &qr).await;
+    let resent_msg = alice.pop_sent_msg().await;
+    let fiona_msg = fiona.recv_msg(&resent_msg).await;
+    assert_eq!(fiona_msg.chat_id, fiona_bc_id);
+    assert_eq!(fiona_msg.text, "text");
+    assert_eq!(
+        alice_msg_id.get_state(alice).await?,
+        MessageState::OutFailed
+    );
+    Ok(())
+}
+
 /// - Alice has multiple devices
 /// - Alice creates a broadcast and sends a message into it
 /// - Alice's second device sees the broadcast

src/message.rs

@@ -140,8 +140,20 @@ SELECT ?1, rfc724_mid, pre_rfc724_mid, timestamp, ?, ? FROM msgs WHERE id=?1
         Ok(())
     }
 
-    pub(crate) async fn set_delivered(self, context: &Context) -> Result<()> {
-        update_msg_state(context, self, MessageState::OutDelivered).await?;
+    /// Returns whether the message state is updated to `OutDelivered`.
+    pub(crate) async fn set_delivered(self, context: &Context) -> Result<bool> {
+        if context
+            .sql
+            .execute(
+                // Only update `OutPending` i.e. if the message is (re-)sent to all chat members.
+                "UPDATE msgs SET state=?, error='' WHERE id=? AND state=?",
+                (MessageState::OutDelivered, self, MessageState::OutPending),
+            )
+            .await?
+            == 0
+        {
+            return Ok(false);
+        }
         let chat_id: Option<ChatId> = context
             .sql
             .query_get_value("SELECT chat_id FROM msgs WHERE id=?", (self,))
@@ -153,7 +165,7 @@ SELECT ?1, rfc724_mid, pre_rfc724_mid, timestamp, ?, ? FROM msgs WHERE id=?1
         if let Some(chat_id) = chat_id {
             chatlist_events::emit_chatlist_item_changed(context, chat_id);
         }
-        Ok(())
+        Ok(true)
     }
 
     /// Bad evil escape hatch.
@@ -1414,6 +1426,9 @@ pub enum MessageState {
     /// The user has pressed the "send" button but the message is not
     /// yet sent and is pending in some way. Maybe we're offline (no
     /// checkmark).
+    ///
+    /// This state means that the message is being (re-)sent to all chat members. It shalln't be
+    /// used e.g. for resending only to a new broadcast member.
     OutPending = 20,
 
     /// *Unrecoverable* error (*recoverable* errors result in pending

src/test_utils.rs

@@ -38,7 +38,7 @@ use crate::events::{Event, EventEmitter, EventType, Events};
 use crate::key::{self, DcKey, self_fingerprint};
 use crate::log::warn;
 use crate::login_param::EnteredLoginParam;
-use crate::message::{Message, MessageState, MsgId, update_msg_state};
+use crate::message::{Message, MessageState, MsgId};
 use crate::mimeparser::{MimeMessage, SystemMessage};
 use crate::pgp::SeipdVersion;
 use crate::receive_imf::{ReceivedMsg, receive_imf};
@@ -692,10 +692,11 @@ ORDER BY id"
         if !msg_has_pending_smtp_job(self, msg_id)
             .await
             .expect("Failed to check for more jobs")
-        {
-            update_msg_state(&self.ctx, msg_id, MessageState::OutDelivered)
+            && msg_id
+                .set_delivered(self)
                 .await
-                .expect("failed to update message state");
+                .expect("MsgId::set_delivered")
+        {
             self.sql
                 .execute(
                     "UPDATE msgs SET timestamp_sent=? WHERE id=?",
@@ -772,9 +773,11 @@ ORDER BY id"
             .execute("DELETE FROM smtp WHERE msg_id=?", (msg_id,))
             .await
             .expect("Delete smtp jobs");
-        update_msg_state(&self.ctx, msg_id, MessageState::OutDelivered)
+        if msg_id
+            .set_delivered(self)
             .await
-            .expect("Update message state");
+            .expect("MsgId::set_delivered")
+        {
             self.sql
                 .execute(
                     "UPDATE msgs SET timestamp_sent=? WHERE id=?",
@@ -782,6 +785,7 @@ ORDER BY id"
                 )
                 .await
                 .expect("Update timestamp_sent");
+        }
         sent_msgs
     }