fix: Order of messages if Sentbox is synced before Inbox

This fixes a Gmail-like scenario when outgoing messages are saved to Sentbox as well and it is fetched before Inbox by chance, by adding a new `OutRcvd` message state for received outgoing messages so that they can mingle with fresh incoming ones and appear after them in chats. As for `OutPending`, `OutDelivered` etc. messages, they are sent locally by the user and there's no need to make them more noticeable even if they are newer. All APIs still return `OutDelivered` instead of `OutRcvd` for compatibility.
deltachat · Oct 28, 2024 · 8bbee4a · 8bbee4a
1 parent 10aa308
commit 8bbee4a
Show file tree

Hide file tree

Showing 13 changed files with 43 additions and 25 deletions.
diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs
@@ -29,7 +29,7 @@ use deltachat::context::{Context, ContextBuilder};
 use deltachat::ephemeral::Timer as EphemeralTimer;
 use deltachat::imex::BackupProvider;
 use deltachat::key::preconfigure_keypair;
-use deltachat::message::MsgId;
+use deltachat::message::{MessageState, MsgId};
 use deltachat::qr_code_generator::{create_qr_svg, generate_backup_qr, get_securejoin_qr_svg};
 use deltachat::stock_str::StockMessage;
 use deltachat::webxdc::StatusUpdateSerial;
@@ -3294,7 +3294,11 @@ pub unsafe extern "C" fn dc_msg_get_state(msg: *mut dc_msg_t) -> libc::c_int {
         return 0;
     }
     let ffi_msg = &*msg;
-    ffi_msg.message.get_state() as libc::c_int
+    let state = match ffi_msg.message.get_state() {
+        MessageState::OutRcvd => MessageState::OutDelivered,
+        s => s,
+    };
+    state as libc::c_int
 }
 
 #[no_mangle]

diff --git a/deltachat-ffi/src/lot.rs b/deltachat-ffi/src/lot.rs
@@ -237,7 +237,7 @@ impl From<MessageState> for LotState {
             OutDraft => LotState::MsgOutDraft,
             OutPending => LotState::MsgOutPending,
             OutFailed => LotState::MsgOutFailed,
-            OutDelivered => LotState::MsgOutDelivered,
+            OutDelivered | OutRcvd => LotState::MsgOutDelivered,
             OutMdnRcvd => LotState::MsgOutMdnRcvd,
         }
     }

diff --git a/deltachat-jsonrpc/src/api/types/message.rs b/deltachat-jsonrpc/src/api/types/message.rs
@@ -7,6 +7,7 @@ use deltachat::contact::Contact;
 use deltachat::context::Context;
 use deltachat::download;
 use deltachat::message::Message;
+use deltachat::message::MessageState;
 use deltachat::message::MsgId;
 use deltachat::message::Viewtype;
 use deltachat::reaction::get_msg_reactions;
@@ -132,6 +133,12 @@ impl MessageObject {
 
         let parent_id = message.parent(context).await?.map(|m| m.get_id().to_u32());
 
+        let state = match message.get_state() {
+            MessageState::OutRcvd => MessageState::OutDelivered,
+            s => s,
+        }
+        .to_u32()
+        .context("state conversion to number failed")?;
         let download_state = message.download_state().into();
 
         let quote = if let Some(quoted_text) = message.quoted_text() {
@@ -193,10 +200,7 @@ impl MessageObject {
             has_location: message.has_location(),
             has_html: message.has_html(),
             view_type: message.get_viewtype().into(),
-            state: message
-                .get_state()
-                .to_u32()
-                .context("state conversion to number failed")?,
+            state,
             error: message.error(),
 
             timestamp: message.get_timestamp(),

diff --git a/src/chat.rs b/src/chat.rs
@@ -1443,9 +1443,14 @@ impl ChatId {
                 .query_row_optional(
                     "SELECT MAX(timestamp), MAX(IIF(state=?,timestamp_sent,0))
                      FROM msgs
-                     WHERE chat_id=? AND hidden=0 AND state>?
+                     WHERE chat_id=? AND hidden=0 AND state>? AND state!=?
                      HAVING COUNT(*) > 0",
-                    (MessageState::InSeen, self, MessageState::InFresh),
+                    (
+                        MessageState::InSeen,
+                        self,
+                        MessageState::InFresh,
+                        MessageState::OutRcvd,
+                    ),
                     |row| {
                         let ts: i64 = row.get(0)?;
                         let ts_sent_seen: i64 = row.get(1)?;
@@ -4262,6 +4267,7 @@ pub async fn resend_msgs(context: &Context, msg_ids: &[MsgId]) -> Result<()> {
             MessageState::OutPending
             | MessageState::OutFailed
             | MessageState::OutDelivered
+            | MessageState::OutRcvd
             | MessageState::OutMdnRcvd => {
                 message::update_msg_state(context, msg.id, MessageState::OutPending).await?
             }

diff --git a/src/message.rs b/src/message.rs
@@ -1367,6 +1367,9 @@ pub enum MessageState {
     /// the OutFailed state if we get such a hint from the server.
     OutDelivered = 26,
 
+    /// Received outgoing message sent by another MUA or device.
+    OutRcvd = 27,
+
     /// Outgoing message read by the recipient (two checkmarks; this
     /// requires goodwill on the receiver's side). Not used in the db for new messages.
     OutMdnRcvd = 28,
@@ -1387,6 +1390,7 @@ impl std::fmt::Display for MessageState {
                 Self::OutPending => "Pending",
                 Self::OutFailed => "Failed",
                 Self::OutDelivered => "Delivered",
+                Self::OutRcvd => "Other MUA",
                 Self::OutMdnRcvd => "Read",
             }
         )
@@ -1399,7 +1403,9 @@ impl MessageState {
         use MessageState::*;
         matches!(
             self,
-            OutPreparing | OutPending | OutDelivered | OutMdnRcvd // OutMdnRcvd can still fail because it could be a group message and only some recipients failed.
+            // OutMdnRcvd can still fail because it could be a group message and only some
+            // recipients failed.
+            OutPreparing | OutPending | OutDelivered | OutRcvd | OutMdnRcvd
         )
     }
 
@@ -1408,13 +1414,13 @@ impl MessageState {
         use MessageState::*;
         matches!(
             self,
-            OutPreparing | OutDraft | OutPending | OutFailed | OutDelivered | OutMdnRcvd
+            OutPreparing | OutDraft | OutPending | OutFailed | OutDelivered | OutRcvd | OutMdnRcvd
         )
     }
 
     /// Returns adjusted message state if the message has MDNs.
     pub(crate) fn with_mdns(self, has_mdns: bool) -> Self {
-        if self == MessageState::OutDelivered && has_mdns {
+        if has_mdns && self >= MessageState::OutDelivered {
             return MessageState::OutMdnRcvd;
         }
         self

diff --git a/src/mimeparser.rs b/src/mimeparser.rs
@@ -3778,7 +3778,7 @@ Message.
         )
         .await?;
         let msg = alice.get_last_msg().await;
-        assert_eq!(msg.state, MessageState::OutDelivered);
+        assert_eq!(msg.state, MessageState::OutRcvd);
 
         // Due to a bug in the old version running on the other device, Alice receives a read
         // receipt from self.
@@ -3817,7 +3817,7 @@ Message.
 
         // Check that the state has not changed to `MessageState::OutMdnRcvd`.
         let msg = Message::load_from_db(&alice, msg.id).await?;
-        assert_eq!(msg.state, MessageState::OutDelivered);
+        assert_eq!(msg.state, MessageState::OutRcvd);
 
         Ok(())
     }

diff --git a/src/reaction.rs b/src/reaction.rs
@@ -475,7 +475,7 @@ Can we chat at 1pm pacific, today?"
         )
         .await?;
         let msg = alice.get_last_msg().await;
-        assert_eq!(msg.state, MessageState::OutDelivered);
+        assert_eq!(msg.state, MessageState::OutRcvd);
         let reactions = get_msg_reactions(&alice, msg.id).await?;
         let contacts = reactions.contacts();
         assert_eq!(contacts.len(), 0);

diff --git a/src/receive_imf.rs b/src/receive_imf.rs
@@ -1030,9 +1030,7 @@ async fn add_parts(
     } else {
         // Outgoing
 
-        // the mail is on the IMAP server, probably it is also delivered.
-        // We cannot recreate other states (read, error).
-        state = MessageState::OutDelivered;
+        state = MessageState::OutRcvd;
         to_id = to_ids.first().copied().unwrap_or_default();
 
         let self_sent = to_ids.len() == 1 && to_ids.contains(&ContactId::SELF);

diff --git a/src/receive_imf/tests.rs b/src/receive_imf/tests.rs
@@ -326,7 +326,7 @@ async fn test_read_receipt_and_unarchive() -> Result<()> {
     let msg = get_chat_msg(&t, group_id, 0, 1).await;
     assert_eq!(msg.is_dc_message, MessengerMessage::Yes);
     assert_eq!(msg.text, "hello");
-    assert_eq!(msg.state, MessageState::OutDelivered);
+    assert_eq!(msg.state, MessageState::OutRcvd);
     let group = Chat::load_from_db(&t, group_id).await?;
     assert!(group.get_visibility() == ChatVisibility::Normal);
 
@@ -793,7 +793,7 @@ async fn test_parse_ndn(
         if error_msg.is_some() {
             MessageState::OutFailed
         } else {
-            MessageState::OutDelivered
+            MessageState::OutRcvd
         }
     );
 
@@ -4538,7 +4538,7 @@ async fn test_outgoing_msg_forgery() -> Result<()> {
 
     let sent_msg = malice.send_text(malice_chat_id, "hi from malice").await;
     let msg = alice.recv_msg(&sent_msg).await;
-    assert_eq!(msg.state, MessageState::OutDelivered);
+    assert_eq!(msg.state, MessageState::OutRcvd);
     assert!(!msg.get_showpadlock());
 
     Ok(())

diff --git a/src/tests/verified_chats.rs b/src/tests/verified_chats.rs
@@ -540,7 +540,7 @@ async fn test_old_message_5() -> Result<()> {
     .await?
     .unwrap();
 
-    assert!(msg_sent.sort_timestamp == msg_incoming.sort_timestamp);
+    assert!(msg_incoming.sort_timestamp < msg_sent.sort_timestamp);
     alice
         .golden_test_chat(msg_sent.chat_id, "test_old_message_5")
         .await;

diff --git a/test-data/golden/receive_imf_older_message_from_2nd_device b/test-data/golden/receive_imf_older_message_from_2nd_device
@@ -1,5 +1,5 @@
 Single#Chat#10: bob@example.net [bob@example.net] 
 --------------------------------------------------------------------------------
 Msg#10: Me (Contact#Contact#Self): We share this account  √
-Msg#11: Me (Contact#Contact#Self): I'm Alice too  √
+Msg#11: Me (Contact#Contact#Self): I'm Alice too 
 --------------------------------------------------------------------------------
diff --git a/test-data/golden/test_old_message_5 b/test-data/golden/test_old_message_5
@@ -1,5 +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]
+Msg#10: Me (Contact#Contact#Self): Happy birthday, Bob! 
 --------------------------------------------------------------------------------
diff --git a/test-data/golden/test_outgoing_mua_msg b/test-data/golden/test_outgoing_mua_msg
@@ -2,6 +2,6 @@ Single#Chat#10: bob@example.net [bob@example.net] 🛡️
 --------------------------------------------------------------------------------
 Msg#10: info (Contact#Contact#Info): Messages are guaranteed to be end-to-end encrypted from now on. [NOTICED][INFO 🛡️]
 Msg#11🔒:  (Contact#Contact#10): Heyho from DC [FRESH]
-Msg#12: Me (Contact#Contact#Self): One classical MUA message  √
+Msg#12: Me (Contact#Contact#Self): One classical MUA message 
 Msg#13🔒: Me (Contact#Contact#Self): Sending with DC again  √
 --------------------------------------------------------------------------------