diff --git a/deltachat-rpc-client/tests/test_something.py b/deltachat-rpc-client/tests/test_something.py index 557f3cd8f..90fe36d52 100644 --- a/deltachat-rpc-client/tests/test_something.py +++ b/deltachat-rpc-client/tests/test_something.py @@ -1232,6 +1232,10 @@ def test_leave_and_delete_group(acfactory, log): def test_immediate_autodelete(acfactory, direct_imap, log): + """ + `bcc_self` is off by default, + so that messages are supposed to be immediately autodeleted + """ ac1, ac2 = acfactory.get_online_accounts(2) # "1" means delete immediately, while "0" means do not delete diff --git a/python/tests/test_1_online.py b/python/tests/test_1_online.py index 236da0cea..6b5418272 100644 --- a/python/tests/test_1_online.py +++ b/python/tests/test_1_online.py @@ -1073,6 +1073,8 @@ def test_send_receive_locations(acfactory, lp): def test_delete_multiple_messages(acfactory, lp): ac1, ac2 = acfactory.get_online_accounts(2) + # Make sure that messages are not immediately auto-deleted: + ac2.set_config("bcc_self", "1") chat12 = acfactory.get_accepted_chat(ac1, ac2) lp.sec("ac1: sending seven messages") diff --git a/src/ephemeral/ephemeral_tests.rs b/src/ephemeral/ephemeral_tests.rs index 663e752f6..543ad68b9 100644 --- a/src/ephemeral/ephemeral_tests.rs +++ b/src/ephemeral/ephemeral_tests.rs @@ -511,30 +511,6 @@ async fn test_delete_expired_imap_messages() -> Result<()> { 0 ); - // t.set_config(Config::DeleteServerAfter, Some(&*(25 * HOUR).to_string())) - // .await?; - // delete_expired_imap_messages(&t).await?; - // test_marked_for_deletion(&t, 1000).await?; - - // MsgId::new(1000) - // .update_download_state(&t, DownloadState::Available) - // .await?; - // t.sql - // .execute("UPDATE imap SET target=folder WHERE rfc724_mid='1000'", ()) - // .await?; - // delete_expired_imap_messages(&t).await?; - // test_marked_for_deletion(&t, 1000).await?; // Delete downloadable anyway. - // remove_uid(&t, 1000).await?; - - // t.set_config(Config::DeleteServerAfter, Some(&*(22 * HOUR).to_string())) - // .await?; - // delete_expired_imap_messages(&t).await?; - // test_marked_for_deletion(&t, 1010).await?; - // t.sql - // .execute("UPDATE imap SET target=folder WHERE rfc724_mid='1010'", ()) - // .await?; - // TODO check if removing this code makes the test fail - MsgId::new(1010) .update_download_state(&t, DownloadState::Available) .await?; @@ -547,11 +523,6 @@ async fn test_delete_expired_imap_messages() -> Result<()> { 0 ); - // TODO instead, test that setting bcc_self to 0 removes messages - // t.set_config(Config::DeleteServerAfter, Some("1")).await?; - // delete_expired_imap_messages(&t).await?; - // test_marked_for_deletion(&t, 3000).await?; - Ok(()) } diff --git a/src/receive_imf/receive_imf_tests.rs b/src/receive_imf/receive_imf_tests.rs index 622692862..8366ac92d 100644 --- a/src/receive_imf/receive_imf_tests.rs +++ b/src/receive_imf/receive_imf_tests.rs @@ -1977,17 +1977,6 @@ async fn test_no_smtp_job_for_self_chat() -> Result<()> { chat::send_msg(bob, chat_id, &mut msg).await?; assert!(bob.pop_sent_msg_opt(Duration::ZERO).await.is_none()); - bob.set_config_bool(Config::BccSelf, true).await?; - // TODO instead, test setting bcc_self - // bob.set_config(Config::DeleteServerAfter, Some("1")).await?; - // let mut msg = Message::new_text("Happy birthday to me".to_string()); - // chat::send_msg(bob, chat_id, &mut msg).await?; - // assert!(bob.pop_sent_msg_opt(Duration::ZERO).await.is_none()); - - // bob.set_config(Config::DeleteServerAfter, None).await?; - // let mut msg = Message::new_text("Happy birthday to me".to_string()); - // chat::send_msg(bob, chat_id, &mut msg).await?; - // assert!(bob.pop_sent_msg_opt(Duration::ZERO).await.is_some()); Ok(()) } diff --git a/src/smtp.rs b/src/smtp.rs index ef86ec944..5065fbdf8 100644 --- a/src/smtp.rs +++ b/src/smtp.rs @@ -699,31 +699,6 @@ pub(crate) async fn add_self_recipients( recipients: &mut Vec, encrypted: bool, ) -> Result<()> { - // Previous versions of Delta Chat did not send BCC self - // if DeleteServerAfter was set to immediately delete messages - // from the server. This is not the case anymore - // because BCC-self messages are also used to detect - // that message was sent if SMTP server is slow to respond - // and connection is frequently lost - // before receiving status line. NB: This is not a problem for chatmail servers, so `BccSelf` - // disabled by default is fine. - - // Seems like the correct replacement is `if true`. - - // Before my change, we're adding the self-recipient iff: - // - Messages are NOT deleted at once - // - OR there are recipients - // - // i.e. we skip adding the self-recipient iff: - // - Messages are deleted at once - // - AND there are no recipients - // probably because in this case, it's not necesary to send anything. - // - // Messages are deleted at once iff BccSelf is off in a chatmail profile now. - // But BccSelf is always on when this function is called. - // So, we can just remove the condition. - // TODO remove commented-out code - // if context.get_config_delete_server_after().await? != Some(0) || !recipients.is_empty() { // Avoid sending unencrypted messages to all transports, chatmail relays won't accept // them. Normally the user should have a non-chatmail primary transport to send unencrypted // messages. @@ -732,9 +707,14 @@ pub(crate) async fn add_self_recipients( recipients.push(addr); } } - // `from` must be the last addr, see `receive_imf_inner()` why. + // `from` must be the last addr + // because `receive_imf_inner()` marks the message as 'delivered' + // if it arrives to the self-server via `bcc_self`. + // This helps with marking messages as delivered + // if the server is slow and we never get an `OK` response + // before the connection times out. let from = context.get_primary_self_addr().await?; recipients.push(from); - // } + Ok(()) }