diff --git a/src/chat.rs b/src/chat.rs index 5032230eb1..9cc77d6a06 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -1218,37 +1218,34 @@ impl ChatId { ) .await? } 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. + // Received incoming 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. Received + // outgoing messages are allowed to mingle with seen messages though to avoid seen + // replies appearing before messages sent from another device (cases like the user + // sharing the account with others or bots are rare, so let them break sometimes). // // 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. + let states = match incoming { + true => "13, 16, 18, 20, 24, 26", // `> MessageState::InFresh` + false => "18, 20, 24, 26", // `> MessageState::InSeen` + }; context .sql .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", - (MessageState::InSeen, self, MessageState::InFresh), + &format!( + "SELECT MAX(timestamp) FROM msgs + WHERE state IN ({states}) AND hidden=0 AND chat_id=? + HAVING COUNT(*) > 0" + ), + (self,), |row| { let ts: i64 = row.get(0)?; - let ts_sent_seen: i64 = row.get(1)?; - Ok((ts, ts_sent_seen)) + Ok(ts) }, ) .await? - .and_then(|(ts, ts_sent_seen)| { - match incoming || ts_sent_seen <= message_timestamp { - true => Some(ts), - false => None, - } - }) } else { None }; diff --git a/src/events/payload.rs b/src/events/payload.rs index bf7e1fa35e..cea0fccd12 100644 --- a/src/events/payload.rs +++ b/src/events/payload.rs @@ -36,7 +36,7 @@ pub enum EventType { /// Emitted when an IMAP message has been moved ImapMessageMoved(String), - /// Emitted before going into IDLE on the Inbox folder. + /// Emitted before going into IDLE on any folder. ImapInboxIdle, /// Emitted when an new file in the $BLOBDIR was created diff --git a/src/message.rs b/src/message.rs index 8ae88f6bb6..2e69cac271 100644 --- a/src/message.rs +++ b/src/message.rs @@ -85,7 +85,7 @@ impl MsgId { .sql .query_row_optional( concat!( - "SELECT m.state, mdns.msg_id", + "SELECT m.state, m.from_id, mdns.msg_id", " FROM msgs m LEFT JOIN msgs_mdns mdns ON mdns.msg_id=m.id", " WHERE id=?", " LIMIT 1", @@ -93,8 +93,9 @@ impl MsgId { (self,), |row| { let state: MessageState = row.get(0)?; - let mdn_msg_id: Option = row.get(1)?; - Ok(state.with_mdns(mdn_msg_id.is_some())) + let from_id: ContactId = row.get(1)?; + let mdn_msg_id: Option = row.get(2)?; + Ok(state.with(from_id, mdn_msg_id.is_some())) }, ) .await? @@ -551,6 +552,7 @@ impl Message { } _ => String::new(), }; + let from_id = row.get("from_id")?; let msg = Message { id: row.get("id")?, rfc724_mid: row.get::<_, String>("rfc724mid")?, @@ -558,7 +560,7 @@ impl Message { .get::<_, Option>("mime_in_reply_to")? .and_then(|in_reply_to| parse_message_id(&in_reply_to).ok()), chat_id: row.get("chat_id")?, - from_id: row.get("from_id")?, + from_id, to_id: row.get("to_id")?, timestamp_sort: row.get("timestamp")?, timestamp_sent: row.get("timestamp_sent")?, @@ -566,7 +568,7 @@ impl Message { ephemeral_timer: row.get("ephemeral_timer")?, ephemeral_timestamp: row.get("ephemeral_timestamp")?, viewtype: row.get("type").unwrap_or_default(), - state: state.with_mdns(mdn_msg_id.is_some()), + state: state.with(from_id, mdn_msg_id.is_some()), download_state: row.get("download_state")?, error: Some(row.get::<_, String>("error")?) .filter(|error| !error.is_empty()), @@ -1366,7 +1368,7 @@ pub enum MessageState { OutDelivered = 26, /// Outgoing message read by the recipient (two checkmarks; this - /// requires goodwill on the receiver's side). Not used in the db for new messages. + /// requires goodwill on the receiver's side). API-only, not used in the db. OutMdnRcvd = 28, } @@ -1410,10 +1412,16 @@ impl MessageState { ) } - /// Returns adjusted message state if the message has MDNs. - pub(crate) fn with_mdns(self, has_mdns: bool) -> Self { + /// Returns adjusted message state. + pub(crate) fn with(mut self, from_id: ContactId, has_mdns: bool) -> Self { + if MessageState::InFresh <= self + && self <= MessageState::InSeen + && from_id == ContactId::SELF + { + self = MessageState::OutDelivered; + } if self == MessageState::OutDelivered && has_mdns { - return MessageState::OutMdnRcvd; + self = MessageState::OutMdnRcvd; } self } diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 9cfa675dc5..3407c6ca93 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -1694,7 +1694,7 @@ async fn add_parts( }; let state = if !mime_parser.incoming { - MessageState::OutDelivered + MessageState::InFresh } 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`. { @@ -1702,7 +1702,6 @@ async fn add_parts( } else { MessageState::InFresh }; - let in_fresh = state == MessageState::InFresh; let sort_to_bottom = false; let received = true; @@ -1999,7 +1998,7 @@ async fn add_parts( save_mime_modified |= mime_parser.is_mime_modified && !part_is_empty && !hidden; let save_mime_modified = save_mime_modified && parts.peek().is_none(); - let ephemeral_timestamp = if in_fresh { + let ephemeral_timestamp = if state == MessageState::InFresh { 0 } else { match ephemeral_timer { @@ -2111,6 +2110,8 @@ RETURNING id ensure_and_debug_assert!(!row_id.is_special(), "Rowid {row_id} is special"); created_db_entries.push(row_id); } + let has_mdns = false; + let state = state.with(from_id, has_mdns); // Maybe set logging xdc and add gossip topics for webxdcs. for (part, msg_id) in mime_parser.parts.iter().zip(&created_db_entries) { diff --git a/src/sql/migrations.rs b/src/sql/migrations.rs index 54aac6506b..c01726d764 100644 --- a/src/sql/migrations.rs +++ b/src/sql/migrations.rs @@ -1271,6 +1271,18 @@ CREATE INDEX gossip_timestamp_index ON gossip_timestamp (chat_id, fingerprint); .await?; } + inc_and_check(&mut migration_version, 135)?; + if dbversion < migration_version { + // Tweak the index for `chat::calc_sort_timestamp()`. + sql.execute_migration( + "UPDATE msgs SET state=26 WHERE state=28; + DROP INDEX IF EXISTS msgs_index7; + CREATE INDEX msgs_index7 ON msgs (state, hidden, chat_id, timestamp);", + migration_version, + ) + .await?; + } + let new_version = sql .get_raw_config_int(VERSION_CFG) .await? diff --git a/src/tests/verified_chats.rs b/src/tests/verified_chats.rs index c120d73baa..962e917fb2 100644 --- a/src/tests/verified_chats.rs +++ b/src/tests/verified_chats.rs @@ -245,9 +245,9 @@ async fn test_old_message_4() -> Result<()> { Ok(()) } -/// Alice is offline for some time. -/// When they come online, first their sentbox is synced and then their inbox. -/// This test tests that the messages are still in the right order. +/// Alice's device#0 is offline for some time. +/// When it comes online, it sees a message from another device and an incoming message. Messages +/// may come from different folders. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_old_message_5() -> Result<()> { let alice = TestContext::new_alice().await; @@ -277,7 +277,11 @@ async fn test_old_message_5() -> Result<()> { .await? .unwrap(); - assert!(msg_sent.sort_timestamp == msg_incoming.sort_timestamp); + // If the messages come from the same folder and `msg_sent` is sent by Alice, it's better to + // sort `msg_incoming` after it so that it's more visible. Messages coming from different + // folders are a rare case now, but if Alice shares her account with someone else or has some + // auto-reply bot, messages should be sorted just by "Date". + 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/test_old_message_5 b/test-data/golden/test_old_message_5 index 9a847dce0d..de808e859f 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 cf76258032..8accbb96b6 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 end-to-end encrypted. [NOTICED][INFO] Msg#11🔒: Me (Contact#Contact#Self): Test – This is encrypted, signed, and has an Autocrypt Header without prefer-encrypt=mutual. √ +Msg#10: info (Contact#Contact#Info): Messages are end-to-end encrypted. [NOTICED][INFO] --------------------------------------------------------------------------------