mirror of
https://github.com/chatmail/core.git
synced 2026-04-01 21:12:13 +03:00
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.
This commit is contained in:
@@ -2104,9 +2104,14 @@ int dc_resend_msgs (dc_context_t* context, const uint3
|
||||
|
||||
|
||||
/**
|
||||
* Mark messages as presented to the user.
|
||||
* Mark messages and reactions to them as presented to the user.
|
||||
* Typically, UIs call this function on scrolling through the message list,
|
||||
* when the messages are presented at least for a little moment.
|
||||
* UIs should pass all messages to this function, incl. outgoing and info ones, as this is used also
|
||||
* for synchronization and to track last position.
|
||||
* This should also be called when a reaction for a message being in view arrives.
|
||||
* If this is called for already presented messages, unless they have new reactions, nothing
|
||||
* happens.
|
||||
* The concrete action depends on the type of the chat and on the users settings
|
||||
* (dc_msgs_presented() may be a better name therefore, but well. :)
|
||||
*
|
||||
|
||||
@@ -1301,6 +1301,11 @@ impl CommandApi {
|
||||
/// Mark messages as presented to the user.
|
||||
/// Typically, UIs call this function on scrolling through the message list,
|
||||
/// when the messages are presented at least for a little moment.
|
||||
/// UIs should pass all messages to this function, incl. outgoing and info ones, as this is used
|
||||
/// also for synchronization and to track last position.
|
||||
/// This should also be called when a reaction for a message being in view arrives.
|
||||
/// If this is called for already presented messages, unless they have new reactions, nothing
|
||||
/// happens.
|
||||
/// The concrete action depends on the type of the chat and on the users settings
|
||||
/// (dc_msgs_presented() may be a better name therefore, but well. :)
|
||||
///
|
||||
|
||||
@@ -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
|
||||
|
||||
110
src/message.rs
110
src/message.rs
@@ -1829,7 +1829,9 @@ pub async fn delete_msgs_ex(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// Marks requested messages as seen.
|
||||
/// Marks requested messages and reactions to them as seen.
|
||||
/// This should be called when a message comes into view or when a reaction for a message being in
|
||||
/// view arrives.
|
||||
pub async fn markseen_msgs(context: &Context, msg_ids: Vec<MsgId>) -> Result<()> {
|
||||
if msg_ids.is_empty() {
|
||||
return Ok(());
|
||||
@@ -1843,10 +1845,18 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec<MsgId>) -> 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<String> = 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 +1867,11 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec<MsgId>) -> 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<?",
|
||||
(id, rfc724_mid, MessageState::InFresh, MessageState::InSeen),
|
||||
|row| {
|
||||
let id: MsgId = row.get("id")?;
|
||||
let chat_id: ChatId = row.get("chat_id")?;
|
||||
let state: MessageState = row.get("state")?;
|
||||
let param: Params = row.get::<_, String>("param")?.parse().unwrap_or_default();
|
||||
@@ -1884,11 +1896,14 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec<MsgId>) -> Result<()>
|
||||
ephemeral_timer,
|
||||
))
|
||||
},
|
||||
|rows| {
|
||||
for row in rows {
|
||||
msgs.push(row?);
|
||||
}
|
||||
Ok(())
|
||||
},
|
||||
)
|
||||
.await?
|
||||
{
|
||||
msgs.push(msg);
|
||||
}
|
||||
.await?;
|
||||
}
|
||||
|
||||
if msgs
|
||||
@@ -1917,48 +1932,45 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec<MsgId>) -> 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 <name>", 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 <name>", 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
|
||||
|
||||
@@ -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(())
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<?",
|
||||
(
|
||||
@@ -1035,7 +1036,18 @@ UPDATE msgs SET state=? WHERE
|
||||
)
|
||||
.await
|
||||
.context("UPDATE msgs.state")?;
|
||||
if chat_id.get_fresh_msg_cnt(context).await? == 0 {
|
||||
let n_fresh_msgs = context
|
||||
.sql
|
||||
.count(
|
||||
"
|
||||
SELECT COUNT(*) FROM msgs WHERE
|
||||
state=? AND
|
||||
(hidden=0 OR hidden=1) AND
|
||||
chat_id=?",
|
||||
(MessageState::InFresh, chat_id),
|
||||
)
|
||||
.await?;
|
||||
if n_fresh_msgs == 0 {
|
||||
// Removes all notifications for the chat in UIs.
|
||||
context.emit_event(EventType::MsgsNoticed(chat_id));
|
||||
} else {
|
||||
@@ -1993,9 +2005,13 @@ async fn add_parts(
|
||||
)
|
||||
.await?;
|
||||
|
||||
let mime_in_reply_to = mime_parser
|
||||
.get_header(HeaderDef::InReplyTo)
|
||||
.unwrap_or_default();
|
||||
let mime_in_reply_to = match mime_parser.get_header(HeaderDef::InReplyTo) {
|
||||
Some(in_reply_to) => parse_message_id(in_reply_to)
|
||||
.log_err(context)
|
||||
.ok()
|
||||
.unwrap_or_default(),
|
||||
None => "".to_string(),
|
||||
};
|
||||
let mime_references = mime_parser
|
||||
.get_header(HeaderDef::References)
|
||||
.unwrap_or_default();
|
||||
@@ -2122,7 +2138,7 @@ async fn add_parts(
|
||||
let is_incoming_fresh = mime_parser.incoming && !seen;
|
||||
set_msg_reaction(
|
||||
context,
|
||||
mime_in_reply_to,
|
||||
&mime_in_reply_to,
|
||||
chat_id,
|
||||
from_id,
|
||||
sort_timestamp,
|
||||
@@ -2264,7 +2280,7 @@ RETURNING id
|
||||
} else {
|
||||
Vec::new()
|
||||
},
|
||||
if trash { "" } else { mime_in_reply_to },
|
||||
if trash { "" } else { &mime_in_reply_to },
|
||||
if trash { "" } else { mime_references },
|
||||
!trash && save_mime_modified,
|
||||
if trash { "" } else { part.error.as_deref().unwrap_or_default() },
|
||||
|
||||
@@ -2343,6 +2343,15 @@ ALTER TABLE contacts ADD COLUMN name_normalized TEXT;
|
||||
.await?;
|
||||
}
|
||||
|
||||
inc_and_check(&mut migration_version, 149)?;
|
||||
if dbversion < migration_version {
|
||||
sql.execute_migration(
|
||||
"CREATE INDEX msgs_index10 ON msgs (mime_in_reply_to)",
|
||||
migration_version,
|
||||
)
|
||||
.await?;
|
||||
}
|
||||
|
||||
let new_version = sql
|
||||
.get_raw_config_int(VERSION_CFG)
|
||||
.await?
|
||||
|
||||
Reference in New Issue
Block a user