From a0891997e5bddcad1bbc9da54e1af3aa82f5b0a7 Mon Sep 17 00:00:00 2001 From: iequidoo Date: Fri, 15 Nov 2024 19:23:49 -0300 Subject: [PATCH] feat: Mark reactions as IMAP-seen in marknoticed_chat() (#6210) When a reaction notification is shown in the UIs, there's an option "Mark Read", but the UIs are unaware of reactions message ids, moreover there are no `msgs` rows for reactions in the core, so the UIs just call `marknoticed_chat()` in this case. We don't want to introduce reactions message ids to the UIs (at least currently), but let's make reactions usual messages, just hidden, so that the existing `\Seen` flag synchronisation mechanism works for them, and mark all reactions in the chat as seen in marknoticed_chat(). --- src/chat.rs | 55 ++++++++++++++++++++++++++++++---------------- src/reaction.rs | 27 ++++++++++++++++++----- src/receive_imf.rs | 11 +++------- src/test_utils.rs | 13 +++++++++++ 4 files changed, 73 insertions(+), 33 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index c1d2f3d1d8..67a179bde5 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -3295,10 +3295,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() { @@ -3309,7 +3309,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), @@ -3319,20 +3319,38 @@ 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| { + let nr_msgs_noticed = conn.execute( + "UPDATE msgs + SET state=? + WHERE state=? + AND hidden=0 + AND chat_id=?", + (MessageState::InNoticed, MessageState::InFresh, chat_id), + )?; + let mut stmt = conn.prepare( + "SELECT id FROM msgs + WHERE state>=? AND state, _>>()?; + Ok((nr_msgs_noticed, hidden_msgs)) + }; + let (nr_msgs_noticed, hidden_msgs) = context.sql.call_write(conn_fn).await?; + if nr_msgs_noticed == 0 && hidden_msgs.is_empty() { + return Ok(()); + } + message::markseen_msgs(context, hidden_msgs).await?; } context.emit_event(EventType::MsgsNoticed(chat_id)); @@ -3377,7 +3395,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 6bb71f2a49..c9477b6a93 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}; @@ -641,7 +641,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?; @@ -657,6 +658,20 @@ Here's my footer -- bob@example.net" .await?; expect_incoming_reactions_event(&alice, alice_msg.sender_msg_id, *bob_id, "👍").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 @@ -695,7 +710,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; assert!(has_incoming_reactions_event(&alice).await); let chatlist = Chatlist::try_load(&bob, 0, None, None).await?; @@ -855,7 +870,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"); @@ -907,7 +922,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. @@ -929,7 +944,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 e4bbeac892..6e88a8a436 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -758,7 +758,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; @@ -1016,12 +1016,7 @@ async fn add_parts( } } - state = if seen - || fetching_existing_messages - || is_mdn - || is_reaction - || chat_id_blocked == Blocked::Yes - { + state = if seen || fetching_existing_messages || is_mdn || chat_id_blocked == Blocked::Yes { MessageState::InSeen } else { MessageState::InFresh @@ -1230,7 +1225,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(|| { diff --git a/src/test_utils.rs b/src/test_utils.rs index 4a8473bc05..4cb794a43a 100644 --- a/src/test_utils.rs +++ b/src/test_utils.rs @@ -607,6 +607,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(