Skip to content

Commit

Permalink
feat: Mark reactions as IMAP-seen in marknoticed_chat() (#6210)
Browse files Browse the repository at this point in the history
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().
  • Loading branch information
iequidoo committed Nov 15, 2024
1 parent e117efa commit a089199
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 33 deletions.
55 changes: 36 additions & 19 deletions src/chat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Result<Vec<_>, _>>().map_err(Into::into)
|ids| ids.collect::<Result<Vec<_>, _>>().map_err(Into::into),
)
.await?;
if chat_ids_in_archive.is_empty() {
Expand All @@ -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),
Expand All @@ -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<?
AND hidden=1
AND chat_id=?",
)?;
let hidden_msgs = stmt
.query_map(
(MessageState::InFresh, MessageState::InSeen, chat_id),
|row| {
let id: MsgId = row.get(0)?;
Ok(id)
},
)?
.collect::<std::result::Result<Vec<_>, _>>()?;
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));
Expand Down Expand Up @@ -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<=?;",
(
Expand Down
27 changes: 21 additions & 6 deletions src/reaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -641,7 +641,8 @@ Here's my footer -- [email protected]"
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?;
Expand All @@ -657,6 +658,20 @@ Here's my footer -- [email protected]"
.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
Expand Down Expand Up @@ -695,7 +710,7 @@ Here's my footer -- [email protected]"
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?;
Expand Down Expand Up @@ -855,7 +870,7 @@ Here's my footer -- [email protected]"
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");
Expand Down Expand Up @@ -907,7 +922,7 @@ Here's my footer -- [email protected]"
{
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.
Expand All @@ -929,7 +944,7 @@ Here's my footer -- [email protected]"
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)
Expand Down
11 changes: 3 additions & 8 deletions src/receive_imf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(|| {
Expand Down
13 changes: 13 additions & 0 deletions src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down

0 comments on commit a089199

Please sign in to comment.