From d38a41d73def40e19b85ef31fbed6c99c5d0317f Mon Sep 17 00:00:00 2001 From: iequidoo Date: Sat, 28 Feb 2026 13:14:25 -0300 Subject: [PATCH] fix: markseen_msgs(): Mark reactions to specified messages as seen too (#7884) This allows to remove notifications for reactions from other devices. NB: UIs should pass all messages to markseen_msgs(), incl. outgoing ones. markseen_msgs() should be called when a message comes into view or when a reaction for a message being in view arrives. Also don't emit `MsgsNoticed` from receive_imf_inner() if the chat still contains fresh hidden messages, i.e. include reactions into this logic, to avoid removing notifications for reactions until they are seen on another device. --- deltachat-rpc-client/tests/test_something.py | 17 +-- src/message.rs | 108 ++++++++++--------- src/reaction.rs | 33 ++++++ src/receive_imf.rs | 26 ++++- src/sql/migrations.rs | 9 ++ 5 files changed, 133 insertions(+), 60 deletions(-) diff --git a/deltachat-rpc-client/tests/test_something.py b/deltachat-rpc-client/tests/test_something.py index 9f41b09f6..7141c9ea8 100644 --- a/deltachat-rpc-client/tests/test_something.py +++ b/deltachat-rpc-client/tests/test_something.py @@ -413,27 +413,32 @@ def test_dont_move_sync_msgs(acfactory, direct_imap): time.sleep(1) -def test_reaction_seen_on_another_dev(acfactory) -> None: +@pytest.mark.parametrize("chat_noticed", [False, True]) +def test_reaction_seen_on_another_dev(acfactory, chat_noticed) -> None: alice, bob = acfactory.get_online_accounts(2) alice2 = alice.clone() alice2.start_io() alice_contact_bob = alice.create_contact(bob, "Bob") alice_chat_bob = alice_contact_bob.create_chat() - alice_chat_bob.send_text("Hello!") + alice_msg = 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() + bob_msg = bob.get_message_by_id(msg_id) + snapshot = bob_msg.get_snapshot() snapshot.chat.accept() - message.send_reaction("😎") + bob_msg.send_reaction("😎") for a in [alice, alice2]: a.wait_for_event(EventType.INCOMING_REACTION) alice2.clear_all_events() - alice_chat_bob.mark_noticed() + if chat_noticed: + alice_chat_bob.mark_noticed() + else: + # UIs are supposed to mark messages being in view as seen, not reactions themselves. + alice_msg.mark_seen() chat_id = alice2.wait_for_event(EventType.MSGS_NOTICED).chat_id alice2_chat_bob = alice2.create_chat(bob) assert chat_id == alice2_chat_bob.id diff --git a/src/message.rs b/src/message.rs index fb89466d4..eb3bbc5e6 100644 --- a/src/message.rs +++ b/src/message.rs @@ -1829,7 +1829,7 @@ pub async fn delete_msgs_ex( Ok(()) } -/// Marks requested messages as seen. +/// Marks requested messages and reactions to them as seen. pub async fn markseen_msgs(context: &Context, msg_ids: Vec) -> Result<()> { if msg_ids.is_empty() { return Ok(()); @@ -1843,10 +1843,18 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec) -> Result<()> let mut msgs = Vec::with_capacity(msg_ids.len()); for &id in &msg_ids { - if let Some(msg) = context + let Some(rfc724_mid): Option = context .sql - .query_row_optional( + .query_get_value("SELECT rfc724_mid FROM msgs WHERE id=?", (id,)) + .await? + else { + continue; + }; + context + .sql + .query_map( "SELECT + m.id AS id, m.chat_id AS chat_id, m.state AS state, m.ephemeral_timer AS ephemeral_timer, @@ -1857,9 +1865,11 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec) -> Result<()> c.archived AS archived, c.blocked AS blocked FROM msgs m LEFT JOIN chats c ON c.id=m.chat_id - WHERE m.id=? AND m.chat_id>9", - (id,), + WHERE (m.id=? OR m.mime_in_reply_to=? AND m.hidden=1) + AND m.chat_id>9 AND ?<=m.state AND m.state("param")?.parse().unwrap_or_default(); @@ -1884,11 +1894,14 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec) -> Result<()> ephemeral_timer, )) }, + |rows| { + for row in rows { + msgs.push(row?); + } + Ok(()) + }, ) - .await? - { - msgs.push(msg); - } + .await?; } if msgs @@ -1917,48 +1930,45 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec) -> Result<()> _curr_ephemeral_timer, ) in msgs { - if curr_state == MessageState::InFresh || curr_state == MessageState::InNoticed { - update_msg_state(context, id, MessageState::InSeen).await?; - info!(context, "Seen message {}.", id); + update_msg_state(context, id, MessageState::InSeen).await?; + info!(context, "Seen message {}.", id); - markseen_on_imap_table(context, &curr_rfc724_mid).await?; + markseen_on_imap_table(context, &curr_rfc724_mid).await?; - // Read receipts for system messages are never sent to contacts. - // These messages have no place to display received read receipt - // anyway. And since their text is locally generated, - // quoting them is dangerous as it may contain contact names. E.g., for original message - // "Group left by me", a read receipt will quote "Group left by ", and the name can - // be a display name stored in address book rather than the name sent in the From field by - // the user. - // - // We also don't send read receipts for contact requests. - // Read receipts will not be sent even after accepting the chat. - let to_id = if curr_blocked == Blocked::Not - && !curr_hidden - && curr_param.get_bool(Param::WantsMdn).unwrap_or_default() - && curr_param.get_cmd() == SystemMessage::Unknown - && context.should_send_mdns().await? - { - Some(curr_from_id) - } else if context.get_config_bool(Config::BccSelf).await? { - Some(ContactId::SELF) - } else { - None - }; - if let Some(to_id) = to_id { - context - .sql - .execute( - "INSERT INTO smtp_mdns (msg_id, from_id, rfc724_mid) VALUES(?, ?, ?)", - (id, to_id, curr_rfc724_mid), - ) - .await - .context("failed to insert into smtp_mdns")?; - context.scheduler.interrupt_smtp().await; - } - if !curr_hidden { - updated_chat_ids.insert(curr_chat_id); - } + // Read receipts for system messages are never sent to contacts. These messages have no + // place to display received read receipt anyway. And since their text is locally generated, + // quoting them is dangerous as it may contain contact names. E.g., for original message + // "Group left by me", a read receipt will quote "Group left by ", and the name can be + // a display name stored in address book rather than the name sent in the From field by the + // user. + // + // We also don't send read receipts for contact requests. Read receipts will not be sent + // even after accepting the chat. + let to_id = if curr_blocked == Blocked::Not + && !curr_hidden + && curr_param.get_bool(Param::WantsMdn).unwrap_or_default() + && curr_param.get_cmd() == SystemMessage::Unknown + && context.should_send_mdns().await? + { + Some(curr_from_id) + } else if context.get_config_bool(Config::BccSelf).await? { + Some(ContactId::SELF) + } else { + None + }; + if let Some(to_id) = to_id { + context + .sql + .execute( + "INSERT INTO smtp_mdns (msg_id, from_id, rfc724_mid) VALUES(?, ?, ?)", + (id, to_id, curr_rfc724_mid), + ) + .await + .context("failed to insert into smtp_mdns")?; + context.scheduler.interrupt_smtp().await; + } + if !curr_hidden { + updated_chat_ids.insert(curr_chat_id); } archived_chats_maybe_noticed |= curr_state == MessageState::InFresh && !curr_hidden diff --git a/src/reaction.rs b/src/reaction.rs index 7d96776f6..7dc387d1f 100644 --- a/src/reaction.rs +++ b/src/reaction.rs @@ -1108,4 +1108,37 @@ Content-Transfer-Encoding: 7bit\r Ok(()) } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn test_markseen_referenced_msg() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + let chat_id = alice.create_chat(bob).await.id; + + let alice_msg_id = send_text_msg(alice, chat_id, "foo".to_string()).await?; + let sent_msg = alice.pop_sent_msg().await; + let bob_msg = bob.recv_msg(&sent_msg).await; + bob_msg.chat_id.accept(bob).await?; + + send_reaction(bob, bob_msg.id, "👀").await?; + let sent_msg = bob.pop_sent_msg().await; + let alice_reaction = alice.recv_msg_hidden(&sent_msg).await; + assert_eq!(alice_reaction.state, MessageState::InFresh); + + markseen_msgs(alice, vec![alice_msg_id]).await?; + let alice_reaction = Message::load_from_db(alice, alice_reaction.id).await?; + assert_eq!(alice_reaction.state, MessageState::InSeen); + assert_eq!( + alice + .sql + .count( + "SELECT COUNT(*) FROM smtp_mdns WHERE from_id=?", + (ContactId::SELF,) + ) + .await?, + 1 + ); + Ok(()) + } } diff --git a/src/receive_imf.rs b/src/receive_imf.rs index f11526257..aa706dfd4 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -35,7 +35,8 @@ use crate::message::{ rfc724_mid_exists, }; use crate::mimeparser::{ - AvatarAction, GossipedKey, MimeMessage, PreMessageMode, SystemMessage, parse_message_ids, + AvatarAction, GossipedKey, MimeMessage, PreMessageMode, SystemMessage, parse_message_id, + parse_message_ids, }; use crate::param::{Param, Params}; use crate::peer_channels::{add_gossip_peer_from_header, insert_topic_stub, iroh_topic_from_str}; @@ -1023,7 +1024,7 @@ UPDATE config SET value=? WHERE keyname='configured_addr' AND value!=?1 " UPDATE msgs SET state=? WHERE state=? AND - hidden=0 AND + (hidden=0 OR hidden=1) AND chat_id=? AND timestamp