feat: Don't mark messages as seen on IMAP if bcc_self=0 or bot=1

bcc_self=0 means that the user disabled it or it's a chatmail setup with the only device and w/o
backups. If a backup is made someday, old messages not marked as IMAP-seen aren't a problem and new
ones are marked as IMAP-seen.

For bots, particularly multi-device ones (which are a rare case), probably no sane logic can be
built on the "seen" message status. Moreover, if the user has additionally a bot that reads incoming
messages, it doesn't mean that the user doesn't want to read them too.

Also don't mark a message as IMAP-seen just if it already exists on the device as this doesn't mean
that it exists on another device.

Also don't mark MDNs and DSNs as IMAP-seen. Marking MDNs as seen is useless, they shouldn't be
displayed by any MUA. Auto-marking DSNs as seen should be avoided because the user may want to see
them in another MUA.

Author: iequidoo

deltachat-rpc-client/tests/test_multidevice.py

@@ -44,7 +44,6 @@ def test_one_account_send_bcc_setting(acfactory, log, direct_imap):
 
     # now make sure we are sending message to ourselves too
     assert self_addr in ev.msg
-    assert self_addr in ev.msg
 
     # BCC-self messages are marked as seen by the sender device.
     while True:

python/tests/test_1_online.py

@@ -162,6 +162,7 @@ def test_html_message(acfactory, lp):
 
 def test_webxdc_message(acfactory, data, lp):
     ac1, ac2 = acfactory.get_online_accounts(2)
+    ac2.set_config("bcc_self", "1")
     chat = acfactory.get_accepted_chat(ac1, ac2)
 
     lp.sec("ac1: prepare and send text message to ac2")
@@ -512,6 +513,8 @@ def test_send_and_receive_message_markseen(acfactory, lp):
     # make DC's life harder wrt to encodings
     ac1.set_config("displayname", "ä name")
 
+    ac2.set_config("bcc_self", "1")
+
     # clear any fresh device messages
     ac1.get_device_chat().mark_noticed()
     ac2.get_device_chat().mark_noticed()
@@ -569,9 +572,6 @@ def test_send_and_receive_message_markseen(acfactory, lp):
         assert ev.data2 > dc.const.DC_MSG_ID_LAST_SPECIAL
     lp.step("2")
 
-    # Check that ac1 marks the read receipt as read.
-    ac1._evtracker.get_info_contains("Marked messages .* in folder INBOX as seen.")
-
     assert msg1.is_out_mdn_received()
     assert msg3.is_out_mdn_received()
 
@@ -587,7 +587,7 @@ def test_send_and_receive_message_markseen(acfactory, lp):
 def test_moved_markseen(acfactory):
     """Test that message already moved to DeltaChat folder is marked as seen."""
     ac1 = acfactory.new_online_configuring_account()
-    ac2 = acfactory.new_online_configuring_account(mvbox_move=True)
+    ac2 = acfactory.new_online_configuring_account(mvbox_move=True, bcc_self=True)
     acfactory.bring_accounts_online()
 
     ac2.stop_io()
@@ -652,31 +652,35 @@ def test_message_override_sender_name(acfactory, lp):
 
 
 @pytest.mark.parametrize("mvbox_move", [True, False])
-def test_markseen_message_and_mdn(acfactory, mvbox_move):
+def test_markseen_message(acfactory, mvbox_move):
     # Please only change this test if you are very sure that it will still catch the issues it catches now.
     # We had so many problems with markseen, if in doubt, rather create another test, it can't harm.
     ac1 = acfactory.new_online_configuring_account(mvbox_move=mvbox_move)
     ac2 = acfactory.new_online_configuring_account(mvbox_move=mvbox_move)
     acfactory.bring_accounts_online()
-    # Do not send BCC to self, we only want to test MDN on ac1.
-    ac1.set_config("bcc_self", "0")
+    ac2.set_config("bcc_self", "1")
+    ac2.stop_io()
 
     acfactory.get_accepted_chat(ac1, ac2).send_text("hi")
+    # We only want to test MDN on ac1, so set "bcc_self" after sending.
+    ac1.set_config("bcc_self", "1")
+
+    ac2.start_io()
     msg = ac2._evtracker.wait_next_incoming_message()
 
     ac2.mark_seen_messages([msg])
 
     folder = "mvbox" if mvbox_move else "inbox"
-    for ac in [ac1, ac2]:
-        if mvbox_move:
-            ac._evtracker.get_info_contains("Marked messages [0-9]+ in folder DeltaChat as seen.")
-        else:
-            ac._evtracker.get_info_contains("Marked messages [0-9]+ in folder INBOX as seen.")
+    if mvbox_move:
+        ac2._evtracker.get_info_contains("Marked messages [0-9]+ in folder DeltaChat as seen.")
+    else:
+        ac2._evtracker.get_info_contains("Marked messages [0-9]+ in folder INBOX as seen.")
+    ac1._evtracker.get_matching("DC_EVENT_MSG_READ")
     ac1.direct_imap.select_config_folder(folder)
     ac2.direct_imap.select_config_folder(folder)
 
-    # Check that the mdn is marked as seen
-    assert len(list(ac1.direct_imap.conn.fetch(AND(seen=True)))) == 1
+    # Check that the mdn isn't marked as seen
+    assert len(list(ac1.direct_imap.conn.fetch(AND(seen=True)))) == 0
     # Check original message is marked as seen
     assert len(list(ac2.direct_imap.conn.fetch(AND(seen=True)))) == 1
 
@@ -715,15 +719,17 @@ def test_mdn_asymmetric(acfactory, lp):
     chat = ac1.create_chat(ac2)
     ac2.create_chat(ac1)
 
-    # make sure mdns are enabled (usually enabled by default already)
-    ac1.set_config("mdns_enabled", "1")
-    ac2.set_config("mdns_enabled", "1")
+    ac1.set_config("bcc_self", "1")
+    ac2.set_config("bcc_self", "1")
 
     lp.sec("sending text message from ac1 to ac2")
     msg_out = chat.send_text("message1")
 
     assert len(chat.get_messages()) == 1 + E2EE_INFO_MSGS
 
+    # Wait for the message to be marked as seen on IMAP.
+    ac1._evtracker.get_info_contains("Marked messages [0-9]+ in folder DeltaChat as seen.")
+
     lp.sec("disable ac1 MDNs")
     ac1.set_config("mdns_enabled", "0")
 
@@ -735,16 +741,17 @@ def test_mdn_asymmetric(acfactory, lp):
     lp.sec("ac2: mark incoming message as seen")
     ac2.mark_seen_messages([msg])
 
+    # Wait for the message to be marked as seen on IMAP.
+    ac2._evtracker.get_info_contains("Marked messages [0-9]+ in folder INBOX as seen.")
+
     lp.sec("ac1: waiting for incoming activity")
     # MDN should be moved even though MDNs are already disabled
     ac1._evtracker.get_matching("DC_EVENT_IMAP_MESSAGE_MOVED")
 
     assert len(chat.get_messages()) == 1 + E2EE_INFO_MSGS
 
-    # Wait for the message to be marked as seen on IMAP.
-    ac1._evtracker.get_info_contains("Marked messages [0-9]+ in folder DeltaChat as seen.")
-
     # MDN is received even though MDNs are already disabled
+    ac1._evtracker.get_matching("DC_EVENT_MSG_READ")
     assert msg_out.is_out_mdn_received()
 
     ac1.direct_imap.select_config_folder("mvbox")

src/imap.rs

@@ -2248,11 +2248,16 @@ pub(crate) async fn prefetch_should_download(
     message_id: &str,
     mut flags: impl Iterator<Item = Flag<'_>>,
 ) -> Result<bool> {
-    if message::rfc724_mid_exists(context, message_id)
-        .await?
-        .is_some()
+    if let Some((.., seen)) = message::rfc724_mid_exists_ex(
+        context,
+        message_id,
+        "state>=16", // `InSeen`
+    )
+    .await?
     {
-        markseen_on_imap_table(context, message_id).await?;
+        if seen {
+            markseen_on_imap_table(context, message_id).await?;
+        }
         return Ok(false);
     }
 
@@ -2397,6 +2402,11 @@ async fn mark_seen_by_uid(
 /// Schedule marking the message as Seen on IMAP by adding all known IMAP messages corresponding to
 /// the given Message-ID to `imap_markseen` table.
 pub(crate) async fn markseen_on_imap_table(context: &Context, message_id: &str) -> Result<()> {
+    if !context.get_config_bool(Config::BccSelf).await?
+        || context.get_config_bool(Config::Bot).await?
+    {
+        return Ok(());
+    }
     context
         .sql
         .execute(

src/receive_imf.rs

@@ -993,11 +993,6 @@ pub(crate) async fn receive_imf_inner(
                 )
                 .await?;
         }
-        if target.is_none() && !mime_parser.mdn_reports.is_empty() && mime_parser.has_chat_version()
-        {
-            // This is a Delta Chat MDN. Mark as read.
-            markseen_on_imap_table(context, rfc724_mid_orig).await?;
-        }
     }
 
     if mime_parser.is_call() {
@@ -1145,8 +1140,9 @@ async fn decide_chat_assignment(
         info!(context, "Message is an MDN (TRASH).");
         true
     } else if mime_parser.delivery_report.is_some() {
+        // Auto-marking DSNs as IMAP-seen should be avoided because the user may want to see them in
+        // another MUA.
         info!(context, "Message is a DSN (TRASH).");
-        markseen_on_imap_table(context, rfc724_mid).await.ok();
         true
     } else if mime_parser.get_header(HeaderDef::ChatEdit).is_some()
         || mime_parser.get_header(HeaderDef::ChatDelete).is_some()