diff --git a/deltachat-rpc-client/src/deltachat_rpc_client/const.py b/deltachat-rpc-client/src/deltachat_rpc_client/const.py index f2542f0eef..e06ba7f72f 100644 --- a/deltachat-rpc-client/src/deltachat_rpc_client/const.py +++ b/deltachat-rpc-client/src/deltachat_rpc_client/const.py @@ -39,6 +39,7 @@ class EventType(str, Enum): ERROR_SELF_NOT_IN_GROUP = "ErrorSelfNotInGroup" MSGS_CHANGED = "MsgsChanged" REACTIONS_CHANGED = "ReactionsChanged" + INCOMING_REACTION = "IncomingReaction" INCOMING_MSG = "IncomingMsg" INCOMING_MSG_BUNCH = "IncomingMsgBunch" MSGS_NOTICED = "MsgsNoticed" diff --git a/deltachat-rpc-client/tests/test_something.py b/deltachat-rpc-client/tests/test_something.py index cf1d422f29..f394e30c3e 100644 --- a/deltachat-rpc-client/tests/test_something.py +++ b/deltachat-rpc-client/tests/test_something.py @@ -285,6 +285,43 @@ def test_message(acfactory) -> None: assert reactions == snapshot.reactions +def test_reaction_seen_on_another_dev(acfactory, tmp_path) -> None: + alice, bob = acfactory.get_online_accounts(2) + alice.export_backup(tmp_path) + files = list(tmp_path.glob("*.tar")) + alice2 = acfactory.get_unconfigured_account() + alice2.import_backup(files[0]) + alice2.start_io() + + bob_addr = bob.get_config("addr") + alice_contact_bob = alice.create_contact(bob_addr, "Bob") + alice_chat_bob = alice_contact_bob.create_chat() + alice_chat_bob.send_text("Hello!") + + event = bob.wait_for_incoming_msg_event() + msg_id = event.msg_id + + message = bob.get_message_by_id(msg_id) + snapshot = message.get_snapshot() + snapshot.chat.accept() + message.send_reaction("😎") + for a in [alice, alice2]: + while True: + event = a.wait_for_event() + if event.kind == EventType.INCOMING_REACTION: + break + + alice_chat_bob.mark_noticed() + while True: + event = alice2.wait_for_event() + if event.kind == EventType.MSGS_NOTICED: + chat_id = event.chat_id + break + alice2_contact_bob = alice2.get_contact_by_addr(bob_addr) + alice2_chat_bob = alice2_contact_bob.create_chat() + assert chat_id == alice2_chat_bob.id + + def test_is_bot(acfactory) -> None: """Test that we can recognize messages submitted by bots.""" alice, bob = acfactory.get_online_accounts(2) diff --git a/src/chat.rs b/src/chat.rs index 320fffb854..433b19f470 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -39,6 +39,7 @@ use crate::mimeparser::SystemMessage; use crate::param::{Param, Params}; use crate::peerstate::Peerstate; use crate::receive_imf::ReceivedMsg; +use crate::rusqlite::OptionalExtension; use crate::securejoin::BobState; use crate::smtp::send_msg_to_smtp; use crate::sql; @@ -3255,10 +3256,10 @@ pub async fn marknoticed_chat(context: &Context, chat_id: ChatId) -> Result<()> .query_map( "SELECT DISTINCT(m.chat_id) FROM msgs m LEFT JOIN chats c ON m.chat_id=c.id - WHERE m.state=10 AND m.hidden=0 AND m.chat_id>9 AND c.blocked=0 AND c.archived=1", - (), + WHERE m.state=10 AND m.chat_id>9 AND c.blocked=0 AND c.archived=1", + (), |row| row.get::<_, ChatId>(0), - |ids| ids.collect::, _>>().map_err(Into::into) + |ids| ids.collect::, _>>().map_err(Into::into), ) .await?; if chat_ids_in_archive.is_empty() { @@ -3269,7 +3270,7 @@ pub async fn marknoticed_chat(context: &Context, chat_id: ChatId) -> Result<()> .sql .execute( &format!( - "UPDATE msgs SET state=13 WHERE state=10 AND hidden=0 AND chat_id IN ({});", + "UPDATE msgs SET state=13 WHERE state=10 AND chat_id IN ({});", sql::repeat_vars(chat_ids_in_archive.len()) ), rusqlite::params_from_iter(&chat_ids_in_archive), @@ -3279,20 +3280,61 @@ pub async fn marknoticed_chat(context: &Context, chat_id: ChatId) -> Result<()> context.emit_event(EventType::MsgsNoticed(chat_id_in_archive)); chatlist_events::emit_chatlist_item_changed(context, chat_id_in_archive); } - } else if context - .sql - .execute( - "UPDATE msgs - SET state=? - WHERE state=? - AND hidden=0 - AND chat_id=?;", - (MessageState::InNoticed, MessageState::InFresh, chat_id), - ) - .await? - == 0 - { - return Ok(()); + } else { + let conn_fn = |conn: &mut rusqlite::Connection| { + // This is to trigger emitting `MsgsNoticed` on other devices when reactions are noticed + // locally. We filter out `InNoticed` messages because they are normally a result of + // `mark_old_messages_as_noticed()` which happens on all devices anyway. Also we limit + // this to one message because the effect is the same anyway. + // + // Even if `message::markseen_msgs()` fails then, in the worst case other devices won't + // emit `MsgsNoticed` and app notifications won't be removed. The bigger problem is that + // another device may have more reactions received and not yet seen notifications are + // removed from it, but the same problem already exists for "usual" messages, so let's + // not solve it for now. + let mut stmt = conn.prepare( + "SELECT id, state FROM msgs + WHERE (state=? OR state=? OR state=?) + AND hidden=1 + AND chat_id=? + ORDER BY id DESC LIMIT 1", + )?; + let id_to_markseen = stmt + .query_row( + ( + MessageState::InFresh, + MessageState::InNoticed, + MessageState::InSeen, + chat_id, + ), + |row| { + let id: MsgId = row.get(0)?; + let state: MessageState = row.get(1)?; + Ok((id, state)) + }, + ) + .optional()? + .filter(|&(_, state)| state == MessageState::InFresh) + .map(|(id, _)| id); + + // NB: Enumerate `hidden` values to employ `msgs_index7`. + let nr_msgs_noticed = conn.execute( + "UPDATE msgs + SET state=? + WHERE state=? + AND (hidden=0 OR hidden=1) + AND chat_id=?", + (MessageState::InNoticed, MessageState::InFresh, chat_id), + )?; + Ok((nr_msgs_noticed, id_to_markseen)) + }; + let (nr_msgs_noticed, id_to_markseen) = context.sql.call_write(conn_fn).await?; + if nr_msgs_noticed == 0 { + return Ok(()); + } + if let Some(id) = id_to_markseen { + message::markseen_msgs(context, vec![id]).await?; + } } context.emit_event(EventType::MsgsNoticed(chat_id)); @@ -3337,7 +3379,6 @@ pub(crate) async fn mark_old_messages_as_noticed( "UPDATE msgs SET state=? WHERE state=? - AND hidden=0 AND chat_id=? AND timestamp<=?;", ( diff --git a/src/reaction.rs b/src/reaction.rs index f0f18b2598..464dcb5794 100644 --- a/src/reaction.rs +++ b/src/reaction.rs @@ -397,7 +397,7 @@ mod tests { use deltachat_contact_tools::ContactAddress; use super::*; - use crate::chat::{forward_msgs, get_chat_msgs, send_text_msg}; + use crate::chat::{forward_msgs, get_chat_msgs, marknoticed_chat, send_text_msg}; use crate::chatlist::Chatlist; use crate::config::Config; use crate::contact::{Contact, Origin}; @@ -663,7 +663,8 @@ Here's my footer -- bob@example.net" assert_eq!(get_chat_msgs(&bob, bob_msg.chat_id).await?.len(), 2); let bob_reaction_msg = bob.pop_sent_msg().await; - alice.recv_msg_trash(&bob_reaction_msg).await; + let alice_reaction_msg = alice.recv_msg_hidden(&bob_reaction_msg).await; + assert_eq!(alice_reaction_msg.state, MessageState::InFresh); assert_eq!(get_chat_msgs(&alice, chat_alice.id).await?.len(), 2); let reactions = get_msg_reactions(&alice, alice_msg.sender_msg_id).await?; @@ -680,6 +681,20 @@ Here's my footer -- bob@example.net" expect_incoming_reactions_event(&alice, alice_msg.sender_msg_id, *bob_id, "👍").await?; expect_no_unwanted_events(&alice).await; + marknoticed_chat(&alice, chat_alice.id).await?; + assert_eq!( + alice_reaction_msg.id.get_state(&alice).await?, + MessageState::InSeen + ); + // Reactions don't request MDNs. + assert_eq!( + alice + .sql + .count("SELECT COUNT(*) FROM smtp_mdns", ()) + .await?, + 0 + ); + // Alice reacts to own message. send_reaction(&alice, alice_msg.sender_msg_id, "👍 😀") .await @@ -719,7 +734,7 @@ Here's my footer -- bob@example.net" bob_msg1.chat_id.accept(&bob).await?; send_reaction(&bob, bob_msg1.id, "👍").await?; let bob_send_reaction = bob.pop_sent_msg().await; - alice.recv_msg_trash(&bob_send_reaction).await; + alice.recv_msg_hidden(&bob_send_reaction).await; expect_incoming_reactions_event(&alice, alice_msg1.sender_msg_id, alice_bob_id, "👍") .await?; expect_no_unwanted_events(&alice).await; @@ -882,7 +897,7 @@ Here's my footer -- bob@example.net" let bob_reaction_msg = bob.pop_sent_msg().await; // Alice receives a reaction. - alice.recv_msg_trash(&bob_reaction_msg).await; + alice.recv_msg_hidden(&bob_reaction_msg).await; let reactions = get_msg_reactions(&alice, alice_msg_id).await?; assert_eq!(reactions.to_string(), "👍1"); @@ -934,7 +949,7 @@ Here's my footer -- bob@example.net" { send_reaction(&alice2, alice2_msg.id, "👍").await?; let msg = alice2.pop_sent_msg().await; - alice1.recv_msg_trash(&msg).await; + alice1.recv_msg_hidden(&msg).await; } // Check that the status is still the same. @@ -956,7 +971,7 @@ Here's my footer -- bob@example.net" let alice1_msg = alice1.recv_msg(&alice0.pop_sent_msg().await).await; send_reaction(&alice0, alice0_msg_id, "👀").await?; - alice1.recv_msg_trash(&alice0.pop_sent_msg().await).await; + alice1.recv_msg_hidden(&alice0.pop_sent_msg().await).await; expect_reactions_changed_event(&alice0, chat_id, alice0_msg_id, ContactId::SELF).await?; expect_reactions_changed_event(&alice1, alice1_msg.chat_id, alice1_msg.id, ContactId::SELF) diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 439731e9b5..37525e5197 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -54,6 +54,9 @@ pub struct ReceivedMsg { /// Received message state. pub state: MessageState, + /// Whether the message is hidden. + pub hidden: bool, + /// Message timestamp for sorting. pub sort_timestamp: i64, @@ -192,6 +195,7 @@ pub(crate) async fn receive_imf_inner( return Ok(Some(ReceivedMsg { chat_id: DC_CHAT_ID_TRASH, state: MessageState::Undefined, + hidden: false, sort_timestamp: 0, msg_ids, needs_delete_job: false, @@ -373,6 +377,7 @@ pub(crate) async fn receive_imf_inner( received_msg = Some(ReceivedMsg { chat_id: DC_CHAT_ID_TRASH, state: MessageState::InSeen, + hidden: false, sort_timestamp: mime_parser.timestamp_sent, msg_ids: vec![msg_id], needs_delete_job: res == securejoin::HandshakeMessage::Done, @@ -611,7 +616,11 @@ pub(crate) async fn receive_imf_inner( } else if !chat_id.is_trash() { let fresh = received_msg.state == MessageState::InFresh; for msg_id in &received_msg.msg_ids { - chat_id.emit_msg_event(context, *msg_id, mime_parser.incoming && fresh); + chat_id.emit_msg_event( + context, + *msg_id, + mime_parser.incoming && fresh && !received_msg.hidden, + ); } } context.new_msgs_notify.notify_one(); @@ -760,7 +769,7 @@ async fn add_parts( // (of course, the user can add other chats manually later) let to_id: ContactId; let state: MessageState; - let mut hidden = false; + let mut hidden = is_reaction; let mut needs_delete_job = false; let mut restore_protection = false; @@ -1021,7 +1030,9 @@ async fn add_parts( state = if seen || fetching_existing_messages || is_mdn - || is_reaction + // Currently only reactions are hidden, so this check is just in case we have other + // hidden messages in the future to make `chat::marknoticed_chat()` no-op for them. + || (hidden && !is_reaction) || chat_id_blocked == Blocked::Yes { MessageState::InSeen @@ -1232,7 +1243,7 @@ async fn add_parts( } let orig_chat_id = chat_id; - let mut chat_id = if is_mdn || is_reaction { + let mut chat_id = if is_mdn { DC_CHAT_ID_TRASH } else { chat_id.unwrap_or_else(|| { @@ -1597,10 +1608,10 @@ RETURNING id state, is_dc_message, if trash { "" } else { msg }, - if trash { None } else { message::normalize_text(msg) }, - if trash { "" } else { &subject }, + if trash || hidden { None } else { message::normalize_text(msg) }, + if trash || hidden { "" } else { &subject }, // txt_raw might contain invalid utf8 - if trash { "" } else { &txt_raw }, + if trash || hidden { "" } else { &txt_raw }, if trash { "".to_string() } else { @@ -1616,7 +1627,7 @@ RETURNING id mime_in_reply_to, mime_references, save_mime_modified, - part.error.as_deref().unwrap_or_default(), + if trash || hidden { "" } else { part.error.as_deref().unwrap_or_default() }, ephemeral_timer, ephemeral_timestamp, if is_partial_download.is_some() { @@ -1626,7 +1637,7 @@ RETURNING id } else { DownloadState::Done }, - mime_parser.hop_info + if trash || hidden { "" } else { &mime_parser.hop_info }, ], |row| { let msg_id: MsgId = row.get(0)?; @@ -1733,6 +1744,7 @@ RETURNING id Ok(ReceivedMsg { chat_id, state, + hidden, sort_timestamp, msg_ids: created_db_entries, needs_delete_job, diff --git a/src/test_utils.rs b/src/test_utils.rs index e680d2025a..df42ca87ff 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -606,6 +606,19 @@ impl TestContext { msg } + /// Receive a message using the `receive_imf()` pipeline. Panics if it's not hidden. + pub async fn recv_msg_hidden(&self, msg: &SentMessage<'_>) -> Message { + let received = self + .recv_msg_opt(msg) + .await + .expect("receive_imf() seems not to have added a new message to the db"); + let msg = Message::load_from_db(self, *received.msg_ids.last().unwrap()) + .await + .unwrap(); + assert!(msg.hidden); + msg + } + /// Receive a message using the `receive_imf()` pipeline. This is similar /// to `recv_msg()`, but doesn't assume that the message is shown in the chat. pub async fn recv_msg_opt(