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

feat: Don't add "Failed to send message to ..." info messages to group chats

Browse Source 
A NDN may arrive days after the message is sent when it's already impossible to tell which message
wasn't delivered looking at the "Failed to send" info message, so it only clutters the chat and
makes the user think they tried to send some message recently which isn't true. Moreover, the info
message duplicates the info already displayed in the error message behind the exclamation mark and
info messages do not point to the message that is failed to be sent.

Moreover it works rarely because `mimeparser.rs` only parses recipients from `x-failed-recipients`,
so it likely only works for Gmail. Postfix does not add this `X-Failed-Recipients` header. Let's
remove this parsing too. Thanks to @link2xt for pointing this out.
This commit is contained in:
iequidoo
2024-12-06 00:31:34 -03:00
committed by iequidoo
parent abe81d0b84
commit 0501917e98
4 changed files with 13 additions and 87 deletions
Download Patch File Download Diff File Expand all files Collapse all files

78
src/mimeparser.rs
View File

@@ -17,10 +17,10 @@ use rand::distributions::{Alphanumeric, DistString};
use crate::aheader::{Aheader, EncryptPreference};
use crate::authres::handle_authres;
use crate::blob::BlobObject;
use crate::chat::{add_info_msg, ChatId};
use crate::chat::ChatId;
use crate::config::Config;
use crate::constants::{self, Chattype};
use crate::contact::{Contact, ContactId, Origin};
use crate::constants;
use crate::contact::ContactId;
use crate::context::Context;
use crate::decrypt::{
get_autocrypt_peerstate, get_encrypted_mime, keyring_from_peerstate, try_decrypt,
@@ -36,8 +36,7 @@ use crate::peerstate::Peerstate;
use crate::simplify::{simplify, SimplifiedText};
use crate::sync::SyncItems;
use crate::tools::{
create_smeared_timestamp, get_filemeta, parse_receive_headers, smeared_time, truncate_msg_text,
validate_id,
get_filemeta, parse_receive_headers, smeared_time, truncate_msg_text, validate_id,
};
use crate::{chatlist_events, location, stock_str, tools};
@@ -1665,18 +1664,8 @@ impl MimeMessage {
.get_header_value(HeaderDef::MessageId)
.and_then(|v| parse_message_id(&v).ok())
{
let mut to_list =
get_all_addresses_from_header(&report.headers, "x-failed-recipients");
let to = if to_list.len() != 1 {
// We do not know which recipient failed
None
} else {
to_list.pop()
};
return Ok(Some(DeliveryReport {
rfc724_mid: original_message_id,
failed_recipient: to.map(|s| s.addr),
failure,
}));
}
@@ -1774,7 +1763,6 @@ impl MimeMessage {
{
self.delivery_report = Some(DeliveryReport {
rfc724_mid: original_message_id,
failed_recipient: None,
failure: true,
})
}
@@ -1912,7 +1900,6 @@ pub(crate) struct Report {
#[derive(Debug)]
pub(crate) struct DeliveryReport {
pub rfc724_mid: String,
pub failed_recipient: Option<String>,
pub failure: bool,
}
@@ -2278,20 +2265,12 @@ async fn handle_ndn(
let msgs: Vec<_> = context
.sql
.query_map(
concat!(
"SELECT",
" m.id AS msg_id,",
" c.id AS chat_id,",
" c.type AS type",
" FROM msgs m LEFT JOIN chats c ON m.chat_id=c.id",
" WHERE rfc724_mid=? AND from_id=1",
),
"SELECT id FROM msgs
WHERE rfc724_mid=? AND from_id=1",
(&failed.rfc724_mid,),
|row| {
let msg_id: MsgId = row.get("msg_id")?;
let chat_id: ChatId = row.get("chat_id")?;
let chat_type: Chattype = row.get("type")?;
Ok((msg_id, chat_id, chat_type))
let msg_id: MsgId = row.get(0)?;
Ok(msg_id)
},
|rows| Ok(rows.collect::<Vec<_>>()),
)
@@ -2299,16 +2278,13 @@ async fn handle_ndn(
let error = if let Some(error) = error {
error
} else if let Some(failed_recipient) = &failed.failed_recipient {
format!("Delivery to {failed_recipient} failed.").clone()
} else {
"Delivery to at least one recipient failed.".to_string()
};
let err_msg = &error;
let mut first = true;
for msg in msgs {
let (msg_id, chat_id, chat_type) = msg?;
let msg_id = msg?;
let mut message = Message::load_from_db(context, msg_id).await?;
let aggregated_error = message
.error
@@ -2320,47 +2296,11 @@ async fn handle_ndn(
aggregated_error.as_ref().unwrap_or(err_msg),
)
.await?;
if first {
// Add only one info msg for all failed messages
ndn_maybe_add_info_msg(context, failed, chat_id, chat_type).await?;
}
first = false;
}
Ok(())
}
async fn ndn_maybe_add_info_msg(
context: &Context,
failed: &DeliveryReport,
chat_id: ChatId,
chat_type: Chattype,
) -> Result<()> {
match chat_type {
Chattype::Group | Chattype::Broadcast => {
if let Some(failed_recipient) = &failed.failed_recipient {
let contact_id =
Contact::lookup_id_by_addr(context, failed_recipient, Origin::Unknown)
.await?
.context("contact ID not found")?;
let contact = Contact::get_by_id(context, contact_id).await?;
// Tell the user which of the recipients failed if we know that (because in
// a group, this might otherwise be unclear)
let text = stock_str::failed_sending_to(context, contact.get_display_name()).await;
add_info_msg(context, chat_id, &text, create_smeared_timestamp(context)).await?;
context.emit_event(EventType::ChatModified(chat_id));
}
}
Chattype::Mailinglist => {
// ndn_maybe_add_info_msg() is about the case when delivery to the group failed.
// If we get an NDN for the mailing list, just issue a warning.
warn!(context, "ignoring NDN for mailing list.");
}
Chattype::Single => {}
}
Ok(())
}
#[cfg(test)]
mod tests {
use mailparse::ParsedMail;