Mirrors/chatmail-core
1
0
Fork 0
mirror of https://github.com/chatmail/core.git synced 2026-05-03 21:36:29 +03:00
Code Issues Packages Projects Releases Wiki Activity

feat: Mark reactions as IMAP-seen in marknoticed_chat() (#6210)

Browse Source 
When a reaction notification is shown in the UIs, there's an option "Mark Read", but the UIs are
unaware of reactions message ids, 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 received
reactions `InFresh` so that the existing `\Seen` flag synchronisation mechanism works for them, and
mark the last fresh hidden incoming message (reaction) in the chat as seen in
`chat::marknoticed_chat()` to trigger emitting `MsgsNoticed` on other devices.

There's a problem though that another device may have more reactions received and not yet seen
notifications are removed from it when handling `MsgsNoticed`, but the same problem already exists
for "usual" messages, so let's not solve it for now.
This commit is contained in:
iequidoo
2024-12-13 13:19:25 -03:00
parent 97b6a03801
commit 6dc7f5064a
4 changed files with 120 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
deltachat-rpc-client/tests/test_something.py
View File

@@ -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)

71
src/chat.rs
View File

@@ -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;
@@ -3243,15 +3244,16 @@ pub async fn get_chat_msgs_ex(
/// Marks all messages in the chat as noticed.
/// If the given chat-id is the archive-link, marks all messages in all archived chats as noticed.
pub async fn marknoticed_chat(context: &Context, chat_id: ChatId) -> Result<()> {
// "WHERE" below uses the index `(state, hidden, chat_id)`, see get_fresh_msg_cnt() for reasoning
// the additional SELECT statement may speed up things as no write-blocking is needed.
// `WHERE` statements below use the index `(state, hidden, chat_id)`, that's why we enumerate
// `hidden` values, see `get_fresh_msg_cnt()` for reasoning.
// The additional `SELECT` statement may speed up things as no write-blocking is needed.
if chat_id.is_archived_link() {
let chat_ids_in_archive = context
.sql
.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.archived=1",
WHERE m.state=10 AND m.hidden IN (0,1) AND m.chat_id>9 AND c.archived=1",
(),
|row| row.get::<_, ChatId>(0),
|ids| ids.collect::<Result<Vec<_>, _>>().map_err(Into::into),
@@ -3265,7 +3267,7 @@ pub async fn marknoticed_chat(context: &Context, chat_id: ChatId) -> Result<()>
.sql
.transaction(|transaction| {
let mut stmt = transaction.prepare(
"UPDATE msgs SET state=13 WHERE state=10 AND hidden=0 AND chat_id = ?",
"UPDATE msgs SET state=13 WHERE state=10 AND hidden IN (0,1) AND chat_id=?",
)?;
for chat_id_in_archive in &chat_ids_in_archive {
stmt.execute((chat_id_in_archive,))?;
@@ -3281,22 +3283,56 @@ pub async fn marknoticed_chat(context: &Context, chat_id: ChatId) -> Result<()>
}
} else {
start_chat_ephemeral_timers(context, chat_id).await?;
if context
.sql
.execute(
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);
let nr_msgs_noticed = conn.execute(
"UPDATE msgs
SET state=?
WHERE state=?
AND hidden=0
AND chat_id=?;",
SET state=?
WHERE state=? AND hidden IN (0,1) AND chat_id=?",
(MessageState::InNoticed, MessageState::InFresh, chat_id),
)
.await?
== 0
{
)?;
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,11 +3373,12 @@ pub(crate) async fn mark_old_messages_as_noticed(
.transaction(|transaction| {
let mut changed_chats = Vec::new();
for (_, msg) in msgs_by_chat {
// NB: Enumerate `hidden` values to employ the index `(state, hidden, chat_id)`.
let changed_rows = transaction.execute(
"UPDATE msgs
SET state=?
WHERE state=?
AND hidden=0
AND hidden IN (0,1)
AND chat_id=?
AND timestamp<=?;",
(

18
src/reaction.rs
View File

@@ -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};
@@ -664,7 +664,7 @@ Here's my footer -- bob@example.net"
let bob_reaction_msg = bob.pop_sent_msg().await;
let alice_reaction_msg = alice.recv_msg_hidden(&bob_reaction_msg).await;
assert_eq!(alice_reaction_msg.state, MessageState::InSeen);
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?;
@@ -681,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

19
src/receive_imf.rs
View File

@@ -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();
@@ -1021,11 +1030,8 @@ 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
// No check for `hidden` because only reactions are such and they should be `InFresh`.
{
MessageState::InSeen
} else {
@@ -1727,6 +1733,7 @@ RETURNING id
Ok(ReceivedMsg {
chat_id,
state,
hidden,
sort_timestamp,
msg_ids: created_db_entries,
needs_delete_job,