Skip to content

Commit

Permalink
fix: markseen_msgs: Limit not yet downloaded messages state to InNoti…
Browse files Browse the repository at this point in the history
…ced (#2970)

This fixes sending MDNs for big messages when they are downloaded and really seen. Otherwise MDNs
are not sent for big encrypted messages because they "don't want MDN" until downloaded.
  • Loading branch information
iequidoo committed Oct 30, 2024
1 parent 5ef0a8d commit 0851201
Showing 1 changed file with 48 additions and 4 deletions.
52 changes: 48 additions & 4 deletions src/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1692,6 +1692,7 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec<MsgId>) -> Result<()>
m.id AS id,
m.chat_id AS chat_id,
m.state AS state,
m.download_state as download_state,
m.ephemeral_timer AS ephemeral_timer,
m.param AS param,
m.from_id AS from_id,
Expand All @@ -1707,6 +1708,7 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec<MsgId>) -> Result<()>
let id: MsgId = row.get("id")?;
let chat_id: ChatId = row.get("chat_id")?;
let state: MessageState = row.get("state")?;
let download_state: DownloadState = row.get("download_state")?;
let param: Params = row.get::<_, String>("param")?.parse().unwrap_or_default();
let from_id: ContactId = row.get("from_id")?;
let rfc724_mid: String = row.get("rfc724_mid")?;
Expand All @@ -1718,6 +1720,7 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec<MsgId>) -> Result<()>
id,
chat_id,
state,
download_state,
param,
from_id,
rfc724_mid,
Expand Down Expand Up @@ -1747,6 +1750,7 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec<MsgId>) -> Result<()>
id,
curr_chat_id,
curr_state,
curr_download_state,
curr_param,
curr_from_id,
curr_rfc724_mid,
Expand All @@ -1756,7 +1760,14 @@ 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 {
if curr_download_state != DownloadState::Done {
if curr_state == MessageState::InFresh {
// Don't mark partially downloaded messages as seen or send a read receipt since
// they are not really seen by the user.
update_msg_state(context, id, MessageState::InNoticed).await?;
updated_chat_ids.insert(curr_chat_id);
}
} else if curr_state == MessageState::InFresh || curr_state == MessageState::InNoticed {
update_msg_state(context, id, MessageState::InSeen).await?;
info!(context, "Seen message {}.", id);

Expand Down Expand Up @@ -2599,8 +2610,28 @@ mod tests {
assert!(!msg.param.get_bool(Param::WantsMdn).unwrap_or_default());
assert_eq!(msg.state, MessageState::InFresh);
markseen_msgs(alice, vec![msg.id]).await?;
let msg = Message::load_from_db(alice, msg.id).await?;
assert_eq!(msg.state, MessageState::InSeen);
// A not downloaded message can be seen only if it's seen on another device.
assert_eq!(msg.id.get_state(alice).await?, MessageState::InNoticed);
// Marking the message as seen again is a no op.
markseen_msgs(alice, vec![msg.id]).await?;
assert_eq!(msg.id.get_state(alice).await?, MessageState::InNoticed);

msg.id
.update_download_state(alice, DownloadState::InProgress)
.await?;
markseen_msgs(alice, vec![msg.id]).await?;
assert_eq!(msg.id.get_state(alice).await?, MessageState::InNoticed);
msg.id
.update_download_state(alice, DownloadState::Failure)
.await?;
markseen_msgs(alice, vec![msg.id]).await?;
assert_eq!(msg.id.get_state(alice).await?, MessageState::InNoticed);
msg.id
.update_download_state(alice, DownloadState::Undecipherable)
.await?;
markseen_msgs(alice, vec![msg.id]).await?;
assert_eq!(msg.id.get_state(alice).await?, MessageState::InNoticed);

assert!(
!alice
.sql
Expand All @@ -2609,12 +2640,25 @@ mod tests {
);

alice.set_config(Config::DownloadLimit, None).await?;
// Let's assume that Alice and Bob resolved the problem with encryption.
let old_msg = msg;
let msg = alice.recv_msg(&sent_msg).await;
assert_eq!(msg.chat_id, old_msg.chat_id);
assert_eq!(msg.download_state, DownloadState::Done);
assert!(msg.param.get_bool(Param::WantsMdn).unwrap_or_default());
assert!(msg.get_showpadlock());
// The message state mustn't be downgraded to `InFresh`.
assert_eq!(msg.state, MessageState::InNoticed);
markseen_msgs(alice, vec![msg.id]).await?;
let msg = Message::load_from_db(alice, msg.id).await?;
assert_eq!(msg.state, MessageState::InSeen);

assert_eq!(
alice
.sql
.count("SELECT COUNT(*) FROM smtp_mdns", ())
.await?,
1
);
Ok(())
}

Expand Down

0 comments on commit 0851201

Please sign in to comment.