diff --git a/src/chat.rs b/src/chat.rs index 7d845c9f6f..0c7a3958d3 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -616,8 +616,9 @@ impl ChatId { contact_id: Option, ) -> Result<()> { let sort_to_bottom = true; + let (received, incoming) = (false, false); let ts = self - .calc_sort_timestamp(context, timestamp_sent, sort_to_bottom, false) + .calc_sort_timestamp(context, timestamp_sent, sort_to_bottom, received, incoming) .await? // Always sort protection messages below `SystemMessage::SecurejoinWait{,Timeout}` ones // in case of race conditions. @@ -1392,12 +1393,14 @@ impl ChatId { /// corresponding event in case of a system message (usually the current system time). /// `always_sort_to_bottom` makes this ajust the returned timestamp up so that the message goes /// to the chat bottom. + /// `received` -- whether the message is received. Otherwise being sent. /// `incoming` -- whether the message is incoming. pub(crate) async fn calc_sort_timestamp( self, context: &Context, message_timestamp: i64, always_sort_to_bottom: bool, + received: bool, incoming: bool, ) -> Result { let mut sort_timestamp = cmp::min(message_timestamp, smeared_time(context)); @@ -1418,25 +1421,35 @@ impl ChatId { (self, MessageState::OutDraft), ) .await? - } else if incoming { - // get newest non fresh message for this chat. - - // If a user hasn't been online for some time, the Inbox is fetched first and then the - // Sentbox. In order for Inbox and Sent messages to be allowed to mingle, outgoing - // messages are purely sorted by their sent timestamp. NB: The Inbox must be fetched - // first otherwise Inbox messages would be always below old Sentbox messages. We could - // take in the query below only incoming messages, but then new incoming messages would - // mingle with just sent outgoing ones and apear somewhere in the middle of the chat. + } else if received { + // Received messages shouldn't mingle with just sent ones and appear somewhere in the + // middle of the chat, so we go after the newest non fresh message. + // + // But if a received outgoing message is older than some seen message, better sort the + // 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. context .sql - .query_get_value( - "SELECT MAX(timestamp) + .query_row_optional( + "SELECT MAX(timestamp), MAX(IIF(state=?,timestamp_sent,0)) FROM msgs WHERE chat_id=? AND hidden=0 AND state>? HAVING COUNT(*) > 0", - (self, MessageState::InFresh), + (MessageState::InSeen, self, MessageState::InFresh), + |row| { + let ts: i64 = row.get(0)?; + let ts_sent_incoming: i64 = row.get(1)?; + Ok((ts, ts_sent_incoming)) + }, ) .await? + .and_then(|(ts, ts_sent_incoming)| { + Some(ts).filter(|_| incoming || ts_sent_incoming <= message_timestamp) + }) } else { None }; diff --git a/src/receive_imf.rs b/src/receive_imf.rs index e19696a284..2849874c08 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -1255,11 +1255,13 @@ async fn add_parts( let in_fresh = state == MessageState::InFresh; let sort_to_bottom = false; + let received = true; let sort_timestamp = chat_id .calc_sort_timestamp( context, mime_parser.timestamp_sent, sort_to_bottom, + received, mime_parser.incoming, ) .await?; diff --git a/src/receive_imf/tests.rs b/src/receive_imf/tests.rs index f71ad1cf20..72e4054520 100644 --- a/src/receive_imf/tests.rs +++ b/src/receive_imf/tests.rs @@ -4810,6 +4810,36 @@ async fn test_protected_group_add_remove_member_missing_key() -> Result<()> { Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_older_message_from_2nd_device() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let chat_id = alice + .create_chat_with_contact("", "bob@example.net") + .await + .id; + alice.send_text(chat_id, "We share this account").await; + let received = receive_imf( + alice, + b"From: alice@example.org\n\ + To: bob@example.net\n\ + Message-ID: <1234-2-4@example.org>\n\ + Date: Sat, 07 Dec 1970 19:00:26 +0000\n\ + \n\ + I'm Alice too\n", + true, + ) + .await? + .unwrap(); + alice + .golden_test_chat( + received.chat_id, + "receive_imf_older_message_from_2nd_device", + ) + .await; + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_dont_create_adhoc_group_on_member_removal() -> Result<()> { let mut tcm = TestContextManager::new(); diff --git a/src/securejoin/bob.rs b/src/securejoin/bob.rs index b33dcc88a5..24c2191a46 100644 --- a/src/securejoin/bob.rs +++ b/src/securejoin/bob.rs @@ -73,8 +73,9 @@ pub(super) async fn start_protocol(context: &Context, invite: QrInvite) -> Resul // 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 = chat_id - .calc_sort_timestamp(context, 0, sort_to_bottom, false) + .calc_sort_timestamp(context, 0, sort_to_bottom, received, incoming) .await?; if chat_id.is_protected(context).await? == ProtectionStatus::Unprotected { let ts_start = time(); diff --git a/test-data/golden/receive_imf_older_message_from_2nd_device b/test-data/golden/receive_imf_older_message_from_2nd_device new file mode 100644 index 0000000000..13fe53ba44 --- /dev/null +++ b/test-data/golden/receive_imf_older_message_from_2nd_device @@ -0,0 +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 √ +--------------------------------------------------------------------------------