feat: remove MvboxMove

Author: link2xt

deltachat-ffi/deltachat.h

@@ -430,10 +430,6 @@ char*           dc_get_blobdir               (const dc_context_t* context);
  *                    1=send a copy of outgoing messages to self (default).
  *                    Sending messages to self is needed for a proper multi-account setup,
  *                    however, on the other hand, may lead to unwanted notifications in non-delta clients.
- * - `mvbox_move`   = 1=detect chat messages,
- *                    move them to the `DeltaChat` folder,
- *                    and watch the `DeltaChat` folder for updates (default),
- *                    0=do not move chat-messages
  * - `only_fetch_mvbox` = 1=Do not fetch messages from folders other than the
  *                    `DeltaChat` folder. Messages will still be fetched from the
  *                    spam folder.

deltachat-rpc-client/tests/test_folders.py

@@ -416,36 +416,26 @@ def test_mvbox_and_trash(acfactory, direct_imap, log):
 
 
 @pytest.mark.parametrize(
-    ("folder", "move", "expected_destination"),
+    ("folder", "expected_destination"),
     [
         (
             "xyz",
-            False,
             "xyz",
         ),  # Test that emails aren't found in a random folder
-        (
-            "xyz",
-            True,
-            "xyz",
-        ),  # ...emails are found in a random folder and downloaded without moving
         (
             "Spam",
-            False,
             "INBOX",
         ),  # ...emails are moved from the spam folder to the Inbox
     ],
 )
 # Testrun.org does not support the CREATE-SPECIAL-USE capability, which means that we can't create a folder with
 # the "\Junk" flag (see https://tools.ietf.org/html/rfc6154). So, we can't test spam folder detection by flag.
-def test_scan_folders(acfactory, log, direct_imap, folder, move, expected_destination):
+def test_scan_folders(acfactory, log, direct_imap, folder, expected_destination):
     """Delta Chat periodically scans all folders for new messages to make sure we don't miss any."""
-    variant = folder + "-" + str(move) + "-" + expected_destination
+    variant = folder + "-" + expected_destination
     log.section("Testing variant " + variant)
     ac1, ac2 = acfactory.get_online_accounts(2)
     ac1.set_config("delete_server_after", "0")
-    if move:
-        ac1.set_config("mvbox_move", "1")
-        ac1.bring_online()
 
     ac1.stop_io()
     ac1_direct_imap = direct_imap(ac1)

deltachat-rpc-client/tests/test_multitransport.py

@@ -31,27 +31,24 @@ def test_add_second_address(acfactory) -> None:
     account.delete_transport(second_addr)
     assert len(account.list_transports()) == 2
 
-    # Enabling mvbox_move or only_fetch_mvbox
+    # Enabling only_fetch_mvbox
     # is not allowed when multi-transport is enabled.
-    for option in ["mvbox_move", "only_fetch_mvbox"]:
-        with pytest.raises(JsonRpcError):
-            account.set_config(option, "1")
+    with pytest.raises(JsonRpcError):
+        account.set_config("only_fetch_mvbox", "1")
 
     with pytest.raises(JsonRpcError):
         account.set_config("show_emails", "0")
 
 
-@pytest.mark.parametrize("key", ["mvbox_move", "only_fetch_mvbox"])
-def test_no_second_transport_with_mvbox(acfactory, key) -> None:
+def test_no_second_transport_with_mvbox(acfactory) -> None:
     """Test that second transport cannot be configured if mvbox is used."""
     account = acfactory.new_configured_account()
     assert len(account.list_transports()) == 1
 
-    assert account.get_config("mvbox_move") == "0"
     assert account.get_config("only_fetch_mvbox") == "0"
 
     qr = acfactory.get_account_qr()
-    account.set_config(key, "1")
+    account.set_config("only_fetch_mvbox", "1")
 
     with pytest.raises(JsonRpcError):
         account.add_transport_from_qr(qr)
@@ -120,30 +117,6 @@ def test_change_address(acfactory) -> None:
     assert sender_addr2 == new_alice_addr
 
 
-@pytest.mark.parametrize("is_chatmail", ["0", "1"])
-def test_mvbox_move_first_transport(acfactory, is_chatmail) -> None:
-    """Test that mvbox_move is disabled by default even for non-chatmail accounts.
-    Disabling mvbox_move is required to be able to setup a second transport.
-    """
-    account = acfactory.get_unconfigured_account()
-
-    account.set_config("fix_is_chatmail", "1")
-    account.set_config("is_chatmail", is_chatmail)
-
-    # The default value when the setting is unset is "1".
-    # This is not changed for compatibility with old databases
-    # imported from backups.
-    assert account.get_config("mvbox_move") == "1"
-
-    qr = acfactory.get_account_qr()
-    account.add_transport_from_qr(qr)
-
-    # Once the first transport is set up,
-    # mvbox_move is disabled.
-    assert account.get_config("mvbox_move") == "0"
-    assert account.get_config("is_chatmail") == is_chatmail
-
-
 def test_reconfigure_transport(acfactory) -> None:
     """Test that reconfiguring the transport works
     even if settings not supported for multi-transport

python/src/deltachat/testplugin.py

@@ -522,7 +522,6 @@ def prepare_account_from_liveconfig(self, configdict) -> Account:
         ac = self.get_unconfigured_account()
         assert "addr" in configdict and "mail_pw" in configdict, configdict
         configdict.setdefault("bcc_self", False)
-        configdict.setdefault("mvbox_move", False)
         configdict.setdefault("sync_msgs", False)
         configdict.setdefault("delete_server_after", 0)
         ac.update_config(configdict)

python/tests/test_3_offline.py

@@ -52,19 +52,19 @@ def test_wrong_config_keys(self, acfactory):
 
     def test_set_config_int_conversion(self, acfactory):
         ac1 = acfactory.get_unconfigured_account()
-        ac1.set_config("mvbox_move", False)
-        assert ac1.get_config("mvbox_move") == "0"
-        ac1.set_config("mvbox_move", True)
-        assert ac1.get_config("mvbox_move") == "1"
-        ac1.set_config("mvbox_move", 0)
-        assert ac1.get_config("mvbox_move") == "0"
-        ac1.set_config("mvbox_move", 1)
-        assert ac1.get_config("mvbox_move") == "1"
+        ac1.set_config("bcc_self", False)
+        assert ac1.get_config("bcc_self") == "0"
+        ac1.set_config("bcc_self", True)
+        assert ac1.get_config("bcc_self") == "1"
+        ac1.set_config("bcc_self", 0)
+        assert ac1.get_config("bcc_self") == "0"
+        ac1.set_config("bcc_self", 1)
+        assert ac1.get_config("bcc_self") == "1"
 
     def test_update_config(self, acfactory):
         ac1 = acfactory.get_unconfigured_account()
-        ac1.update_config({"mvbox_move": False})
-        assert ac1.get_config("mvbox_move") == "0"
+        ac1.update_config({"bcc_self": True})
+        assert ac1.get_config("bcc_self") == "1"
 
     def test_has_bccself(self, acfactory):
         ac1 = acfactory.get_unconfigured_account()

src/config.rs

@@ -155,11 +155,6 @@ pub enum Config {
     #[strum(props(default = "1"))]
     MdnsEnabled,
 
-    /// True if chat messages should be moved to a separate folder. Auto-sent messages like sync
-    /// ones are moved there anyway.
-    #[strum(props(default = "1"))]
-    MvboxMove,
-
     /// Watch for new messages in the "Mvbox" (aka DeltaChat folder) only.
     ///
     /// This will not entirely disable other folders, e.g. the spam folder will also still
@@ -478,7 +473,6 @@ impl Config {
             self,
             Self::Displayname
                 | Self::MdnsEnabled
-                | Self::MvboxMove
                 | Self::ShowEmails
                 | Self::Selfavatar
                 | Self::Selfstatus,
@@ -487,10 +481,7 @@ impl Config {
 
     /// Whether the config option needs an IO scheduler restart to take effect.
     pub(crate) fn needs_io_restart(&self) -> bool {
-        matches!(
-            self,
-            Config::MvboxMove | Config::OnlyFetchMvbox | Config::ConfiguredAddr
-        )
+        matches!(self, Config::OnlyFetchMvbox | Config::ConfiguredAddr)
     }
 }
 
@@ -608,8 +599,7 @@ impl Context {
 
     /// Returns true if movebox ("DeltaChat" folder) should be watched.
     pub(crate) async fn should_watch_mvbox(&self) -> Result<bool> {
-        Ok(self.get_config_bool(Config::MvboxMove).await?
-            || self.get_config_bool(Config::OnlyFetchMvbox).await?)
+        self.get_config_bool(Config::OnlyFetchMvbox).await
     }
 
     /// Returns true if sync messages should be sent.
@@ -693,7 +683,6 @@ impl Context {
             | Config::ProxyEnabled
             | Config::BccSelf
             | Config::MdnsEnabled
-            | Config::MvboxMove
             | Config::OnlyFetchMvbox
             | Config::DeleteToTrash
             | Config::Configured
@@ -719,12 +708,7 @@ impl Context {
         Self::check_config(key, value)?;
 
         let n_transports = self.count_transports().await?;
-        if n_transports > 1
-            && matches!(
-                key,
-                Config::MvboxMove | Config::OnlyFetchMvbox | Config::ShowEmails
-            )
-        {
+        if n_transports > 1 && matches!(key, Config::OnlyFetchMvbox | Config::ShowEmails) {
             bail!("Cannot reconfigure {key} when multiple transports are configured");
         }
 
@@ -806,12 +790,6 @@ impl Context {
                     .set_raw_config(key.as_ref(), value.map(|s| s.to_lowercase()).as_deref())
                     .await?;
             }
-            Config::MvboxMove => {
-                self.sql.set_raw_config(key.as_ref(), value).await?;
-                self.sql
-                    .set_raw_config(constants::DC_FOLDERS_CONFIGURED_KEY, None)
-                    .await?;
-            }
             Config::ConfiguredAddr => {
                 let Some(addr) = value else {
                     bail!("Cannot unset configured_addr");

src/config/config_tests.rs

@@ -196,11 +196,11 @@ async fn test_sync() -> Result<()> {
     sync(&alice0, &alice1).await;
     assert_eq!(alice1.get_config_bool(Config::MdnsEnabled).await?, false);
 
-    for key in [Config::ShowEmails, Config::MvboxMove] {
-        let val = alice0.get_config_bool(key).await?;
-        alice0.set_config_bool(key, !val).await?;
+    {
+        let val = alice0.get_config_bool(Config::ShowEmails).await?;
+        alice0.set_config_bool(Config::ShowEmails, !val).await?;
         sync(&alice0, &alice1).await;
-        assert_eq!(alice1.get_config_bool(key).await?, !val);
+        assert_eq!(alice1.get_config_bool(Config::ShowEmails).await?, !val);
     }
 
     // `Config::SyncMsgs` mustn't be synced.

src/configure.rs

@@ -281,11 +281,6 @@ impl Context {
                     "To use additional relays, disable the legacy option \"Settings / Advanced / Only Fetch from DeltaChat Folder\"."
                 );
             }
-            if self.get_config(Config::MvboxMove).await?.as_deref() != Some("0") {
-                bail!(
-                    "To use additional relays, disable the legacy option \"Settings / Advanced / Move automatically to DeltaChat Folder\"."
-                );
-            }
             if self.get_config(Config::ShowEmails).await?.as_deref() != Some("2") {
                 bail!(
                     "To use additional relays, set the legacy option \"Settings / Advanced / Show Classic Emails\" to \"All\"."

src/context.rs

@@ -864,7 +864,6 @@ impl Context {
             Err(err) => format!("<key failure: {err}>"),
         };
 
-        let mvbox_move = self.get_config_int(Config::MvboxMove).await?;
         let only_fetch_mvbox = self.get_config_int(Config::OnlyFetchMvbox).await?;
         let folders_configured = self
             .sql
@@ -960,7 +959,6 @@ impl Context {
                 .await?
                 .to_string(),
         );
-        res.insert("mvbox_move", mvbox_move.to_string());
         res.insert("only_fetch_mvbox", only_fetch_mvbox.to_string());
         res.insert(
             constants::DC_FOLDERS_CONFIGURED_KEY,