Ignore Drafts folder when scanning (#2454)

* Add failing test for #2369

* Completely ignore Drafts folder

fix #2369

* Also ignore messages that have the Draft flag set but are not in the Drafts folder

python/src/deltachat/direct_imap.py

@@ -251,7 +251,16 @@ class DirectImap:
             return res
 
     def append(self, folder, msg):
+        """Upload a message to *folder*.
+        Trailing whitespace or a linebreak at the beginning will be removed automatically.
+        """
         if msg.startswith("\n"):
             msg = msg[1:]
         msg = '\n'.join([s.lstrip() for s in msg.splitlines()])
         self.conn.append(folder, msg)
+
+    def get_uid_by_message_id(self, message_id):
+        msgs = self.conn.search(['HEADER', 'MESSAGE-ID', message_id])
+        if len(msgs) == 0:
+            raise Exception("Did not find message " + message_id + ", maybe you forgot to select the correct folder?")
+        return msgs[0]

python/tests/test_account.py

@@ -1319,9 +1319,11 @@ class TestOnlineAccount:
         assert not device_chat.can_send()
         assert device_chat.get_draft() is None
 
-    def test_dont_show_emails_in_draft_folder(self, acfactory):
+    def test_dont_show_emails_in_draft_folder(self, acfactory, lp):
         """Most mailboxes have a "Drafts" folder where constantly new emails appear but we don't actually want to show them.
-        So: If it's outgoing AND there is no Received header AND it's not in the sentbox, then ignore the email."""
+        So: If it's outgoing AND there is no Received header AND it's not in the sentbox, then ignore the email.
+
+        If the draft email is sent out later (i.e. moved to "Sent"), it must be shown."""
         ac1 = acfactory.get_online_configuring_account()
         ac1.set_config("show_emails", "2")
         ac1.create_contact("alice@example.com").create_chat()
@@ -1342,7 +1344,7 @@ class TestOnlineAccount:
             Message-ID: <aepiors@example.org>
             Content-Type: text/plain; charset=utf-8
 
-            message in Drafts
+            message in Drafts that is moved to Sent later
         """.format(ac1.get_config("configured_addr")))
         ac1.direct_imap.append("Sent", """
             From: ac1 <{}>
@@ -1355,6 +1357,7 @@ class TestOnlineAccount:
         """.format(ac1.get_config("configured_addr")))
 
         ac1.set_config("scan_all_folders_debounce_secs", "0")
+        lp.sec("All prepared, now let DC find the message")
         ac1.start_io()
 
         msg = ac1._evtracker.wait_next_messages_changed()
@@ -1365,6 +1368,18 @@ class TestOnlineAccount:
         assert msg.text == "subj – message in Sent"
         assert len(msg.chat.get_messages()) == 1
 
+        ac1.stop_io()
+        lp.sec("'Send out' the draft, i.e. move it to the Sent folder, and wait for DC to display it this time")
+        ac1.direct_imap.select_folder("Drafts")
+        uid = ac1.direct_imap.get_uid_by_message_id("aepiors@example.org")
+        ac1.direct_imap.conn.move(uid, "Sent")
+
+        ac1.start_io()
+        msg2 = ac1._evtracker.wait_next_messages_changed()
+
+        assert msg2.text == "subj – message in Drafts that is moved to Sent later"
+        assert len(msg.chat.get_messages()) == 2
+
     def test_prefer_encrypt(self, acfactory, lp):
         """Test quorum rule for encryption preference in 1:1 and group chat."""
         ac1, ac2, ac3 = acfactory.get_many_online_accounts(3)

src/config.rs

@@ -18,7 +18,18 @@ use crate::stock_str;
 
 /// The available configuration keys.
 #[derive(
-    Debug, Clone, Copy, PartialEq, Eq, Display, EnumString, AsRefStr, EnumIter, EnumProperty,
+    Debug,
+    Clone,
+    Copy,
+    PartialEq,
+    Eq,
+    Display,
+    EnumString,
+    AsRefStr,
+    EnumIter,
+    EnumProperty,
+    PartialOrd,
+    Ord,
 )]
 #[strum(serialize_all = "snake_case")]
 pub enum Config {

src/context.rs

@@ -538,19 +538,16 @@ impl Context {
 
     pub async fn is_sentbox(&self, folder_name: &str) -> Result<bool> {
         let sentbox = self.get_config(Config::ConfiguredSentboxFolder).await?;
-
         Ok(sentbox.as_deref() == Some(folder_name))
     }
 
     pub async fn is_mvbox(&self, folder_name: &str) -> Result<bool> {
         let mvbox = self.get_config(Config::ConfiguredMvboxFolder).await?;
-
         Ok(mvbox.as_deref() == Some(folder_name))
     }
 
     pub async fn is_spam_folder(&self, folder_name: &str) -> Result<bool> {
         let spam = self.get_config(Config::ConfiguredSpamFolder).await?;
-
         Ok(spam.as_deref() == Some(folder_name))
     }
 

src/dc_receive_imf.rs

@@ -2742,11 +2742,14 @@ mod tests {
 
         // Check that the ndn would be downloaded:
         let headers = mailparse::parse_mail(raw_ndn).unwrap().headers;
-        assert!(
+        assert!(crate::imap::prefetch_should_download(
-            crate::imap::prefetch_should_download(&t, &headers, ShowEmails::Off)
+            &t,
-                .await
+            &headers,
-                .unwrap()
+            std::iter::empty(),
-        );
+            ShowEmails::Off
+        )
+        .await
+        .unwrap());
 
         dc_receive_imf(&t, raw_ndn, "INBOX", 1, false)
             .await

src/imap.rs

@@ -14,7 +14,6 @@ use async_std::channel::Receiver;
 use async_std::prelude::*;
 use num_traits::FromPrimitive;
 
-use crate::chat;
 use crate::config::Config;
 use crate::constants::{
     Chattype, ShowEmails, Viewtype, DC_FETCH_EXISTING_MSGS_COUNT, DC_FOLDERS_CONFIGURED_VERSION,
@@ -36,6 +35,7 @@ use crate::param::Params;
 use crate::provider::Socket;
 use crate::scheduler::InterruptInfo;
 use crate::stock_str;
+use crate::{chat, constants::DC_CONTACT_ID_SELF};
 
 mod client;
 mod idle;
@@ -111,14 +111,27 @@ impl async_imap::Authenticator for OAuth2 {
     }
 }
 
-#[derive(Debug, PartialEq)]
+#[derive(Debug, PartialEq, Clone, Copy)]
 enum FolderMeaning {
     Unknown,
     Spam,
-    SentObjects,
+    Sent,
+    Drafts,
     Other,
 }
 
+impl FolderMeaning {
+    fn to_config(self) -> Option<Config> {
+        match self {
+            FolderMeaning::Unknown => None,
+            FolderMeaning::Spam => Some(Config::ConfiguredSpamFolder),
+            FolderMeaning::Sent => Some(Config::ConfiguredSentboxFolder),
+            FolderMeaning::Drafts => None,
+            FolderMeaning::Other => None,
+        }
+    }
+}
+
 #[derive(Debug)]
 struct ImapConfig {
     pub addr: String,
@@ -696,6 +709,7 @@ impl Imap {
                 current_uid,
                 &headers,
                 &msg_id,
+                msg.flags(),
                 folder,
                 show_emails,
             )
@@ -1300,9 +1314,8 @@ impl Imap {
 
             let mut delimiter = ".".to_string();
             let mut delimiter_is_default = true;
-            let mut sentbox_folder = None;
-            let mut spam_folder = None;
             let mut mvbox_folder = None;
+            let mut folder_configs = BTreeMap::new();
             let mut fallback_folder = get_fallback_folder(&delimiter);
 
             while let Some(folder) = folders.next().await {
@@ -1321,31 +1334,26 @@ impl Imap {
                 let folder_meaning = get_folder_meaning(&folder);
                 let folder_name_meaning = get_folder_meaning_by_name(folder.name());
                 if folder.name() == "DeltaChat" {
-                    // Always takes precendent
+                    // Always takes precedence
                     mvbox_folder = Some(folder.name().to_string());
                 } else if folder.name() == fallback_folder {
-                    // only set iff none has been already set
+                    // only set if none has been already set
                     if mvbox_folder.is_none() {
                         mvbox_folder = Some(folder.name().to_string());
                     }
-                } else if folder_meaning == FolderMeaning::SentObjects {
+                } else if let Some(config) = folder_meaning.to_config() {
-                    // Always takes precedent
+                    // Always takes precedence
-                    sentbox_folder = Some(folder.name().to_string());
+                    folder_configs.insert(config, folder.name().to_string());
-                } else if folder_meaning == FolderMeaning::Spam {
+                } else if let Some(config) = folder_name_meaning.to_config() {
-                    spam_folder = Some(folder.name().to_string());
+                    // only set if none has been already set
-                } else if folder_name_meaning == FolderMeaning::SentObjects {
+                    folder_configs
-                    // only set iff none has been already set
+                        .entry(config)
-                    if sentbox_folder.is_none() {
+                        .or_insert_with(|| folder.name().to_string());
-                        sentbox_folder = Some(folder.name().to_string());
-                    }
-                } else if folder_name_meaning == FolderMeaning::Spam && spam_folder.is_none() {
-                    spam_folder = Some(folder.name().to_string());
                 }
             }
             drop(folders);
 
             info!(context, "Using \"{}\" as folder-delimiter.", delimiter);
-            info!(context, "sentbox folder is {:?}", sentbox_folder);
 
             if mvbox_folder.is_none() && create_mvbox {
                 info!(context, "Creating MVBOX-folder \"DeltaChat\"...",);
@@ -1393,15 +1401,8 @@ impl Imap {
                     .set_config(Config::ConfiguredMvboxFolder, Some(mvbox_folder))
                     .await?;
             }
-            if let Some(ref sentbox_folder) = sentbox_folder {
+            for (config, name) in folder_configs {
-                context
+                context.set_config(config, Some(&name)).await?;
-                    .set_config(Config::ConfiguredSentboxFolder, Some(sentbox_folder))
-                    .await?;
-            }
-            if let Some(ref spam_folder) = spam_folder {
-                context
-                    .set_config(Config::ConfiguredSpamFolder, Some(spam_folder))
-                    .await?;
             }
             context
                 .sql
@@ -1474,29 +1475,50 @@ fn get_folder_meaning_by_name(folder_name: &str) -> FolderMeaning {
         "迷惑メール",
         "스팸",
     ];
+    const DRAFT_NAMES: &[&str] = &[
+        "Drafts",
+        "Kladder",
+        "Entw?rfe",
+        "Borradores",
+        "Brouillons",
+        "Bozze",
+        "Concepten",
+        "Wersje robocze",
+        "Rascunhos",
+        "Entwürfe",
+        "Koncepty",
+        "Kopie robocze",
+        "Taslaklar",
+        "Utkast",
+        "Πρόχειρα",
+        "Черновики",
+        "下書き",
+        "草稿",
+        "임시보관함",
+    ];
     let lower = folder_name.to_lowercase();
 
     if SENT_NAMES.iter().any(|s| s.to_lowercase() == lower) {
-        FolderMeaning::SentObjects
+        FolderMeaning::Sent
     } else if SPAM_NAMES.iter().any(|s| s.to_lowercase() == lower) {
         FolderMeaning::Spam
+    } else if DRAFT_NAMES.iter().any(|s| s.to_lowercase() == lower) {
+        FolderMeaning::Drafts
     } else {
         FolderMeaning::Unknown
     }
 }
 
 fn get_folder_meaning(folder_name: &Name) -> FolderMeaning {
-    let special_names = vec!["\\Trash", "\\Drafts"];
-
     for attr in folder_name.attributes() {
         if let NameAttribute::Custom(ref label) = attr {
-            if special_names.iter().any(|s| *s == label) {
+            match label.as_ref() {
-                return FolderMeaning::Other;
+                "\\Trash" => return FolderMeaning::Other,
-            } else if label == "\\Sent" {
+                "\\Sent" => return FolderMeaning::Sent,
-                return FolderMeaning::SentObjects;
+                "\\Spam" | "\\Junk" => return FolderMeaning::Spam,
-            } else if label == "\\Spam" || label == "\\Junk" {
+                "\\Drafts" => return FolderMeaning::Drafts,
-                return FolderMeaning::Spam;
+                _ => {}
-            }
+            };
         }
     }
     FolderMeaning::Unknown
@@ -1614,6 +1636,7 @@ fn prefetch_get_message_id(headers: &[mailparse::MailHeader]) -> Result<String>
 pub(crate) async fn prefetch_should_download(
     context: &Context,
     headers: &[mailparse::MailHeader<'_>],
+    mut flags: impl Iterator<Item = Flag<'_>>,
     show_emails: ShowEmails,
 ) -> Result<bool> {
     let is_chat_message = headers.get_header_value(HeaderDef::ChatVersion).is_some();
@@ -1651,12 +1674,17 @@ pub(crate) async fn prefetch_should_download(
         .get_header_value(HeaderDef::AutocryptSetupMessage)
         .is_some();
 
-    let (_contact_id, blocked_contact, origin) =
+    let (from_id, blocked_contact, origin) =
         from_field_to_contact_id(context, &mimeparser::get_from(headers), true).await?;
     // 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())
-    let accepted_contact = origin.is_known();
 
+    if flags.any(|f| f == Flag::Draft) && from_id == DC_CONTACT_ID_SELF {
+        info!(context, "Ignoring draft message");
+        return Ok(false);
+    }
+
+    let accepted_contact = origin.is_known();
     let show = is_autocrypt_setup_message
         || match show_emails {
             ShowEmails::Off => is_chat_message || is_reply_to_chat_message,
@@ -1675,6 +1703,7 @@ async fn message_needs_processing(
     current_uid: u32,
     headers: &[mailparse::MailHeader<'_>],
     msg_id: &str,
+    flags: impl Iterator<Item = Flag<'_>>,
     folder: &str,
     show_emails: ShowEmails,
 ) -> bool {
@@ -1698,7 +1727,7 @@ async fn message_needs_processing(
     // we do not know the message-id
     // or the message-id is missing (in this case, we create one in the further process)
     // or some other error happened
-    let show = match prefetch_should_download(context, headers, show_emails).await {
+    let show = match prefetch_should_download(context, headers, flags, show_emails).await {
         Ok(show) => show,
         Err(err) => {
             warn!(context, "prefetch_should_download error: {}", err);
@@ -1861,25 +1890,16 @@ mod tests {
     use crate::test_utils::TestContext;
     #[test]
     fn test_get_folder_meaning_by_name() {
-        assert_eq!(
+        assert_eq!(get_folder_meaning_by_name("Gesendet"), FolderMeaning::Sent);
-            get_folder_meaning_by_name("Gesendet"),
+        assert_eq!(get_folder_meaning_by_name("GESENDET"), FolderMeaning::Sent);
-            FolderMeaning::SentObjects
+        assert_eq!(get_folder_meaning_by_name("gesendet"), FolderMeaning::Sent);
-        );
-        assert_eq!(
-            get_folder_meaning_by_name("GESENDET"),
-            FolderMeaning::SentObjects
-        );
-        assert_eq!(
-            get_folder_meaning_by_name("gesendet"),
-            FolderMeaning::SentObjects
-        );
         assert_eq!(
             get_folder_meaning_by_name("Messages envoyés"),
-            FolderMeaning::SentObjects
+            FolderMeaning::Sent
         );
         assert_eq!(
             get_folder_meaning_by_name("mEsSaGes envoyÉs"),
-            FolderMeaning::SentObjects
+            FolderMeaning::Sent
         );
         assert_eq!(get_folder_meaning_by_name("xxx"), FolderMeaning::Unknown);
         assert_eq!(get_folder_meaning_by_name("SPAM"), FolderMeaning::Spam);

src/imap/scan_folders.rs

@@ -1,13 +1,13 @@
-use std::time::Instant;
+use std::{collections::BTreeMap, time::Instant};
 
 use anyhow::{Context as _, Result};
 
-use crate::config::Config;
-use crate::context::Context;
 use crate::imap::Imap;
+use crate::{config::Config, log::LogExt};
+use crate::{context::Context, imap::FolderMeaning};
 use async_std::prelude::*;
 
-use super::{get_folder_meaning, get_folder_meaning_by_name, FolderMeaning};
+use super::{get_folder_meaning, get_folder_meaning_by_name};
 
 impl Imap {
     pub async fn scan_folders(&mut self, context: &Context) -> Result<()> {
@@ -31,8 +31,7 @@ impl Imap {
         let folders: Vec<_> = session.list(Some(""), Some("*")).await?.collect().await;
         let watched_folders = get_watched_folders(context).await;
 
-        let mut sentbox_folder = None;
+        let mut folder_configs = BTreeMap::new();
-        let mut spam_folder = None;
 
         for folder in folders {
             let folder = match folder {
@@ -43,38 +42,41 @@ impl Imap {
                 }
             };
 
-            let foldername = folder.name();
             let folder_meaning = get_folder_meaning(&folder);
-            let folder_name_meaning = get_folder_meaning_by_name(foldername);
+            let folder_name_meaning = get_folder_meaning_by_name(folder.name());
 
-            if folder_meaning == FolderMeaning::SentObjects {
+            if let Some(config) = folder_meaning.to_config() {
-                // Always takes precedent
+                // Always takes precedence
-                sentbox_folder = Some(folder.name().to_string());
+                folder_configs.insert(config, folder.name().to_string());
-            } else if folder_meaning == FolderMeaning::Spam {
+            } else if let Some(config) = folder_name_meaning.to_config() {
-                spam_folder = Some(folder.name().to_string());
+                // only set if none has been already set
-            } else if folder_name_meaning == FolderMeaning::SentObjects {
+                folder_configs
-                // only set iff none has been already set
+                    .entry(config)
-                if sentbox_folder.is_none() {
+                    .or_insert_with(|| folder.name().to_string());
-                    sentbox_folder = Some(folder.name().to_string());
-                }
-            } else if folder_name_meaning == FolderMeaning::Spam && spam_folder.is_none() {
-                spam_folder = Some(folder.name().to_string());
             }
 
+            let is_drafts = folder_meaning == FolderMeaning::Drafts
+                || (folder_meaning == FolderMeaning::Unknown
+                    && folder_name_meaning == FolderMeaning::Drafts);
+
             // Don't scan folders that are watched anyway
-            if !watched_folders.contains(&foldername.to_string()) {
+            if !watched_folders.contains(&folder.name().to_string()) && !is_drafts {
-                if let Err(e) = self.fetch_new_messages(context, foldername, false).await {
+                self.fetch_new_messages(context, folder.name(), false)
-                    warn!(context, "Can't fetch new msgs in scanned folder: {:#}", e);
+                    .await
-                }
+                    .ok_or_log_msg(context, "Can't fetch new msgs in scanned folder");
             }
         }
 
-        context
+        // We iterate over both folder meanings to make sure that if e.g. the "Sent" folder was deleted,
-            .set_config(Config::ConfiguredSentboxFolder, sentbox_folder.as_deref())
+        // `ConfiguredSentboxFolder` is set to `None`:
-            .await?;
+        for config in &[
-        context
+            Config::ConfiguredSentboxFolder,
-            .set_config(Config::ConfiguredSpamFolder, spam_folder.as_deref())
+            Config::ConfiguredSpamFolder,
-            .await?;
+        ] {
+            context
+                .set_config(*config, folder_configs.get(config).map(|s| s.as_str()))
+                .await?;
+        }
 
         last_scan.replace(Instant::now());
         Ok(())