Skip to content
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
feat: create all contacts from the To field with IncomingUnknownTo or…
…igin

Contacts are created with IncomingUnknownTo origin instead of IncomingTo now
even if the message is from a known contact.

Removed IncomingTo, IncomingCc, OutgoingBcc, OutgoingCc, IncomingUnknownCc origins.
  • Loading branch information
link2xt committed May 17, 2025
commit 32b6a14c71f90557f12232ccc89f226c146ee4ee
16 changes: 1 addition & 15 deletions src/contact.rs
Original file line number Diff line number Diff line change
Expand Up @@ -501,14 +501,12 @@ pub enum Origin {
MailinglistAddress = 0x2,

/// Hidden on purpose, e.g. addresses with the word "noreply" in it
/// or past members of the groups.
Hidden = 0x8,

/// From: of incoming messages of unknown sender
IncomingUnknownFrom = 0x10,

/// Cc: of incoming messages of unknown sender
IncomingUnknownCc = 0x20,

/// To: of incoming messages of unknown sender
IncomingUnknownTo = 0x40,
Copy link
Collaborator

@Hocuri Hocuri Apr 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the case before this PR already, but, for my understanding: The order here determines which authnames are used, i.e. "better" origins overwrite the names of "worse" origins, with an exception that IncomingUnknownFrom may overwrite any other authname. (The origin also determines that contacts with at least IncomingReplyTo origin are shown in the contact list.)

I find the order super weird:

  • IncomingUnknownFrom has the lowest priority of all. I would expect it to have a high priority, because this is obviously the name that comes directly from this contact.
  • I would expect IncomingUnkwnownTo to have a lower priority, because it's a "gossiped" name - the displayname was in the "To" header in a message coming from some other contact.
  • Same for IncomingReplyTo, because it's not guaranteed that this message actually came from the contact who controls the IncomingReplyTo address.

Is there something I'm missing?

In any case, I'm not saying we have to change this now, can come after multi-transport.


Expand All @@ -522,21 +520,9 @@ pub enum Origin {
/// Contacts with at least this origin value are shown in the contact list.
IncomingReplyTo = 0x100,

/// Cc: of incoming message of known sender
IncomingCc = 0x200,

/// additional To:'s of incoming message of known sender
IncomingTo = 0x400,

/// a chat was manually created for this user, but no message yet sent
CreateChat = 0x800,

/// message sent by us
OutgoingBcc = 0x1000,

/// message sent by us
OutgoingCc = 0x2000,

/// message sent by us
OutgoingTo = 0x4000,

Expand Down
36 changes: 17 additions & 19 deletions src/imap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1885,20 +1885,19 @@ async fn should_move_out_of_spam(
None => return Ok(false),
};
// No chat found.
let (from_id, blocked_contact, _origin) =
match from_field_to_contact_id(context, &from, true)
.await
.context("from_field_to_contact_id")?
{
Some(res) => res,
None => {
warn!(
context,
"Contact with From address {:?} cannot exist, not moving out of spam", from
);
return Ok(false);
}
};
let (from_id, blocked_contact) = match from_field_to_contact_id(context, &from, true)
.await
.context("from_field_to_contact_id")?
{
Some(res) => res,
None => {
warn!(
context,
"Contact with From address {:?} cannot exist, not moving out of spam", from
);
return Ok(false);
}
};
if blocked_contact {
// Contact is blocked, leave the message in spam.
return Ok(false);
Expand Down Expand Up @@ -2243,11 +2242,10 @@ pub(crate) async fn prefetch_should_download(
Some(f) => f,
None => return Ok(false),
};
let (_from_id, blocked_contact, _origin) =
match from_field_to_contact_id(context, &from, true).await? {
Some(res) => res,
None => return Ok(false),
};
let (_from_id, blocked_contact) = match from_field_to_contact_id(context, &from, true).await? {
Some(res) => res,
None => return Ok(false),
};
// prevent_rename=true as this might be a mailing list message and in this case it would be bad if we rename the contact.
// (prevent_rename is the last argument of from_field_to_contact_id())

Expand Down
19 changes: 8 additions & 11 deletions src/receive_imf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ pub(crate) async fn receive_imf_inner(
// For example, GitHub sends messages from `[email protected]`,
// but uses display name of the user whose action generated the notification
// as the display name.
let (from_id, _from_id_blocked, incoming_origin) =
let (from_id, _from_id_blocked) =
match from_field_to_contact_id(context, &mime_parser.from, prevent_rename).await? {
Some(contact_id_res) => contact_id_res,
None => {
Expand All @@ -337,12 +337,10 @@ pub(crate) async fn receive_imf_inner(
let to_ids = add_or_lookup_contacts_by_address_list(
context,
&mime_parser.recipients,
if !mime_parser.incoming {
Origin::OutgoingTo
} else if incoming_origin.is_known() {
Origin::IncomingTo
} else {
if mime_parser.incoming {
Origin::IncomingUnknownTo
} else {
Origin::OutgoingTo
},
)
.await?;
Expand Down Expand Up @@ -646,7 +644,7 @@ pub(crate) async fn receive_imf_inner(

/// Converts "From" field to contact id.
///
/// Also returns whether it is blocked or not and its origin.
/// Also returns whether it is blocked or not.
///
/// * `prevent_rename`: if true, the display_name of this contact will not be changed. Useful for
/// mailing lists: In some mailing lists, many users write from the same address but with different
Expand All @@ -658,7 +656,7 @@ pub async fn from_field_to_contact_id(
context: &Context,
from: &SingleInfo,
prevent_rename: bool,
) -> Result<Option<(ContactId, bool, Origin)>> {
) -> Result<Option<(ContactId, bool)>> {
let display_name = if prevent_rename {
Some("")
} else {
Expand All @@ -684,12 +682,11 @@ pub async fn from_field_to_contact_id(
.await?;

if from_id == ContactId::SELF {
Ok(Some((ContactId::SELF, false, Origin::OutgoingBcc)))
Ok(Some((ContactId::SELF, false)))
} else {
let contact = Contact::get_by_id(context, from_id).await?;
let from_id_blocked = contact.blocked;
let incoming_origin = contact.origin;
Ok(Some((from_id, from_id_blocked, incoming_origin)))
Ok(Some((from_id, from_id_blocked)))
}
}

Expand Down
8 changes: 4 additions & 4 deletions src/receive_imf/receive_imf_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ async fn test_adhoc_group_outgoing_show_accepted_contact_unaccepted() -> Result<
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_adhoc_group_show_accepted_contact_known() {
let t = TestContext::new_alice().await;
t.set_config(Config::ShowEmails, Some("1")).await.unwrap();
t.set_config(Config::ShowEmails, Some("2")).await.unwrap();
Contact::create(&t, "Bob", "[email protected]").await.unwrap();
receive_imf(&t, GRP_MAIL, false).await.unwrap();

Expand All @@ -150,7 +150,7 @@ async fn test_adhoc_group_show_accepted_contact_known() {
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_adhoc_group_show_accepted_contact_accepted() {
let t = TestContext::new_alice().await;
t.set_config(Config::ShowEmails, Some("1")).await.unwrap();
t.set_config(Config::ShowEmails, Some("2")).await.unwrap();

// accept Bob by accepting a delta-message from Bob
receive_imf(&t, MSGRMSG, false).await.unwrap();
Expand Down Expand Up @@ -2319,7 +2319,7 @@ async fn test_ignore_footer_status_from_mailinglist() -> Result<()> {
&t,
"",
&ContactAddress::new("[email protected]").unwrap(),
Origin::IncomingUnknownCc,
Origin::IncomingUnknownTo,
)
.await?
.0;
Expand Down Expand Up @@ -3985,7 +3985,7 @@ async fn test_mua_user_adds_recipient_to_single_chat() -> Result<()> {
chat::get_chat_contacts(&alice, group_chat.id).await?.len(),
4
);
let fiona = Contact::lookup_id_by_addr(&alice, "[email protected]", Origin::IncomingTo)
let fiona = Contact::lookup_id_by_addr(&alice, "[email protected]", Origin::IncomingUnknownTo)
.await?
.unwrap();
assert!(chat::is_contact_in_chat(&alice, group_chat.id, fiona).await?);
Expand Down