Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
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
4 changes: 0 additions & 4 deletions deltachat-ffi/deltachat.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
37 changes: 5 additions & 32 deletions deltachat-rpc-client/tests/test_multitransport.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion python/src/deltachat/testplugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
20 changes: 10 additions & 10 deletions python/tests/test_3_offline.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
29 changes: 3 additions & 26 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -467,7 +462,6 @@ impl Config {
self,
Self::Displayname
| Self::MdnsEnabled
| Self::MvboxMove
| Self::ShowEmails
| Self::Selfavatar
| Self::Selfstatus,
Expand All @@ -476,10 +470,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)
}
}

Expand Down Expand Up @@ -597,9 +588,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::IsChatmail).await?)
self.get_config_bool(Config::OnlyFetchMvbox).await
}

/// Returns true if sync messages should be sent.
Expand Down Expand Up @@ -683,7 +672,6 @@ impl Context {
| Config::ProxyEnabled
| Config::BccSelf
| Config::MdnsEnabled
| Config::MvboxMove
| Config::OnlyFetchMvbox
| Config::Configured
| Config::Bot
Expand All @@ -708,12 +696,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");
}

Expand Down Expand Up @@ -795,12 +778,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");
Expand Down
8 changes: 4 additions & 4 deletions src/config/config_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 0 additions & 5 deletions src/configure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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\"."
Expand Down
2 changes: 0 additions & 2 deletions src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -956,7 +955,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,
Expand Down
107 changes: 1 addition & 106 deletions src/imap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1667,43 +1667,6 @@ impl Session {
}
Ok(())
}

/// Attempts to configure mvbox.
///
/// Tries to find any folder examining `folders` in the order they go.
/// This method does not use LIST command to ensure that
/// configuration works even if mailbox lookup is forbidden via Access Control List (see
/// <https://datatracker.ietf.org/doc/html/rfc4314>).
///
/// Returns first found folder name.
async fn configure_mvbox<'a>(
&mut self,
context: &Context,
folders: &[&'a str],
) -> Result<Option<&'a str>> {
// Close currently selected folder if needed.
// We are going to select folders using low-level EXAMINE operations below.
self.maybe_close_folder(context).await?;

for folder in folders {
info!(context, "Looking for MVBOX-folder \"{}\"...", &folder);
let res = self.examine(&folder).await;
if res.is_ok() {
info!(
context,
"MVBOX-folder {:?} successfully selected, using it.", &folder
);
self.close().await?;
// Before moving emails to the mvbox we need to remember its UIDVALIDITY, otherwise
// emails moved before that wouldn't be fetched but considered "old" instead.
let folder_exists = self.select_with_uidvalidity(context, folder).await?;
ensure!(folder_exists, "No MVBOX folder {:?}??", &folder);
return Ok(Some(folder));
}
}

Ok(None)
}
}

impl Imap {
Expand All @@ -1716,23 +1679,11 @@ impl Imap {
.list(Some(""), Some("*"))
.await
.context("list_folders failed")?;
let mut delimiter = ".".to_string();
let mut delimiter_is_default = true;
let mut folder_configs = BTreeMap::new();

while let Some(folder) = folders.try_next().await? {
info!(context, "Scanning folder: {:?}", folder);

// Update the delimiter iff there is a different one, but only once.
if let Some(d) = folder.delimiter()
&& delimiter_is_default
&& !d.is_empty()
&& delimiter != d
{
delimiter = d.to_string();
delimiter_is_default = false;
}

let folder_meaning = get_folder_meaning_by_attrs(folder.attributes());
let folder_name_meaning = get_folder_meaning_by_name(folder.name());
if let Some(config) = folder_meaning.to_config() {
Expand All @@ -1747,23 +1698,9 @@ impl Imap {
}
drop(folders);

info!(context, "Using \"{}\" as folder-delimiter.", delimiter);

let fallback_folder = format!("INBOX{delimiter}DeltaChat");
let mvbox_folder = session
.configure_mvbox(context, &["DeltaChat", &fallback_folder])
.await
.context("failed to configure mvbox")?;

context
.set_config_internal(Config::ConfiguredInboxFolder, Some("INBOX"))
.await?;
if let Some(mvbox_folder) = mvbox_folder {
info!(context, "Setting MVBOX FOLDER TO {}", &mvbox_folder);
context
.set_config_internal(Config::ConfiguredMvboxFolder, Some(mvbox_folder))
.await?;
}
for (config, name) in folder_configs {
context.set_config_internal(config, Some(&name)).await?;
}
Expand Down Expand Up @@ -1911,15 +1848,7 @@ async fn spam_target_folder_cfg(
return Ok(None);
}

if needs_move_to_mvbox(context, headers).await?
// If OnlyFetchMvbox is set, we don't want to move the message to
// the inbox where we wouldn't fetch it again:
|| context.get_config_bool(Config::OnlyFetchMvbox).await?
{
Ok(Some(Config::ConfiguredMvboxFolder))
} else {
Ok(Some(Config::ConfiguredInboxFolder))
}
Ok(Some(Config::ConfiguredInboxFolder))
}

/// Returns `ConfiguredInboxFolder` or `ConfiguredMvboxFolder` if
Expand All @@ -1936,10 +1865,6 @@ pub async fn target_folder_cfg(

if folder_meaning == FolderMeaning::Spam {
spam_target_folder_cfg(context, headers).await
} else if folder_meaning == FolderMeaning::Inbox
&& needs_move_to_mvbox(context, headers).await?
{
Ok(Some(Config::ConfiguredMvboxFolder))
} else {
Ok(None)
}
Expand All @@ -1960,36 +1885,6 @@ pub async fn target_folder(
}
}

async fn needs_move_to_mvbox(
context: &Context,
headers: &[mailparse::MailHeader<'_>],
) -> Result<bool> {
let has_chat_version = headers.get_header_value(HeaderDef::ChatVersion).is_some();
if !context.get_config_bool(Config::MvboxMove).await? {
return Ok(false);
}

if headers
.get_header_value(HeaderDef::AutocryptSetupMessage)
.is_some()
{
// do not move setup messages;
// there may be a non-delta device that wants to handle it
return Ok(false);
}

if has_chat_version {
Ok(true)
} else if let Some(parent) = get_prefetch_parent_message(context, headers).await? {
match parent.is_dc_message {
MessengerMessage::No => Ok(false),
MessengerMessage::Yes | MessengerMessage::Reply => Ok(true),
}
} else {
Ok(false)
}
}

/// Try to get the folder meaning by the name of the folder only used if the server does not support XLIST.
// TODO: lots languages missing - maybe there is a list somewhere on other MUAs?
// however, if we fail to find out the sent-folder,
Expand Down
Loading
Loading