From ae0d404d12c5c1deb093770837def1bec6dd4a44 Mon Sep 17 00:00:00 2001 From: iequidoo Date: Fri, 11 Oct 2024 18:34:35 -0300 Subject: [PATCH] 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. 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-ffi/src/lib.rs | 8 ++++++-- deltachat-ffi/src/lot.rs | 2 +- deltachat-jsonrpc/src/api/types/message.rs | 12 ++++++++---- src/chat.rs | 15 ++++++++++----- src/message.rs | 12 +++++++++--- src/mimeparser/mimeparser_tests.rs | 4 ++-- src/reaction.rs | 2 +- src/receive_imf.rs | 2 +- src/receive_imf/receive_imf_tests.rs | 4 ++-- src/tests/verified_chats.rs | 2 +- .../receive_imf_older_message_from_2nd_device | 2 +- test-data/golden/test_old_message_5 | 2 +- test-data/golden/test_outgoing_encrypted_msg | 2 +- test-data/golden/test_outgoing_mua_msg | 2 +- 14 files changed, 45 insertions(+), 26 deletions(-) diff --git a/deltachat-ffi/src/lib.rs b/deltachat-ffi/src/lib.rs index 93894829a3..7e5b808f52 100644 --- 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; @@ -3344,7 +3344,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 index e77483ef7d..7837d1315e 100644 --- a/deltachat-ffi/src/lot.rs +++ b/deltachat-ffi/src/lot.rs @@ -240,7 +240,7 @@ impl From 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 index 3ad81bd12a..3d5627c621 100644 --- a/deltachat-jsonrpc/src/api/types/message.rs +++ b/deltachat-jsonrpc/src/api/types/message.rs @@ -9,6 +9,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; @@ -149,6 +150,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() { @@ -212,10 +219,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 index 37adccadf9..ef93596dfe 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -1427,17 +1427,21 @@ impl ChatId { // 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. + // NB: 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>? + 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)?; @@ -4444,6 +4448,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 index e801771833..b285bb92c8 100644 --- a/src/message.rs +++ b/src/message.rs @@ -1453,6 +1453,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, @@ -1473,6 +1476,7 @@ impl std::fmt::Display for MessageState { Self::OutPending => "Pending", Self::OutFailed => "Failed", Self::OutDelivered => "Delivered", + Self::OutRcvd => "Other MUA", Self::OutMdnRcvd => "Read", } ) @@ -1485,7 +1489,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 ) } @@ -1494,13 +1500,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/mimeparser_tests.rs b/src/mimeparser/mimeparser_tests.rs index 211176aee4..88a6bfb178 100644 --- a/src/mimeparser/mimeparser_tests.rs +++ b/src/mimeparser/mimeparser_tests.rs @@ -1522,7 +1522,7 @@ async fn test_ignore_read_receipt_to_self() -> Result<()> { ) .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. @@ -1561,7 +1561,7 @@ async fn test_ignore_read_receipt_to_self() -> Result<()> { // 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 index 3a13f4e98f..6ee08adc5c 100644 --- a/src/reaction.rs +++ b/src/reaction.rs @@ -476,7 +476,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 index 86d1577ace..5ad6beb0d7 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -1708,7 +1708,7 @@ async fn add_parts( }; let state = if !mime_parser.incoming { - MessageState::OutDelivered + MessageState::OutRcvd } else if seen || is_mdn || chat_id_blocked == Blocked::Yes || group_changes.silent // No check for `hidden` because only reactions are such and they should be `InFresh`. { diff --git a/src/receive_imf/receive_imf_tests.rs b/src/receive_imf/receive_imf_tests.rs index 829a2022eb..ca06f90885 100644 --- a/src/receive_imf/receive_imf_tests.rs +++ b/src/receive_imf/receive_imf_tests.rs @@ -684,7 +684,7 @@ async fn test_parse_ndn( if error_msg.is_some() { MessageState::OutFailed } else { - MessageState::OutDelivered + MessageState::OutRcvd } ); @@ -4312,7 +4312,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 index 7b6b18178c..e6d5b2b388 100644 --- a/src/tests/verified_chats.rs +++ b/src/tests/verified_chats.rs @@ -351,7 +351,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 index abc38ad339..04f2df83dd 100644 --- 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] Icon: 4138c52e5bc1c576cda7dd44d088c07.png -------------------------------------------------------------------------------- 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 index 624838a43c..fc240473a0 100644 --- 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] Icon: 4138c52e5bc1c576cda7dd44d088c07.png -------------------------------------------------------------------------------- -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_encrypted_msg b/test-data/golden/test_outgoing_encrypted_msg index cd3b205beb..5696571805 100644 --- a/test-data/golden/test_outgoing_encrypted_msg +++ b/test-data/golden/test_outgoing_encrypted_msg @@ -1,5 +1,5 @@ Single#Chat#10: bob@example.net [KEY 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🔒: Me (Contact#Contact#Self): Test – This is encrypted, signed, and has an Autocrypt Header without prefer-encrypt=mutual. √ +Msg#11🔒: Me (Contact#Contact#Self): Test – This is encrypted, signed, and has an Autocrypt Header without prefer-encrypt=mutual. -------------------------------------------------------------------------------- diff --git a/test-data/golden/test_outgoing_mua_msg b/test-data/golden/test_outgoing_mua_msg index 5d4a0f2ee9..7674b38e81 100644 --- a/test-data/golden/test_outgoing_mua_msg +++ b/test-data/golden/test_outgoing_mua_msg @@ -1,4 +1,4 @@ Single#Chat#11: bob@example.net [bob@example.net] Icon: 4138c52e5bc1c576cda7dd44d088c07.png -------------------------------------------------------------------------------- -Msg#12: Me (Contact#Contact#Self): One classical MUA message √ +Msg#12: Me (Contact#Contact#Self): One classical MUA message --------------------------------------------------------------------------------