Skip to content

fix(mail): expunge source folder after IMAP move/spam/ham #66

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ThomasMms23 wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

fix(mail): expunge source folder after IMAP move/spam/ham #66

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 15 additions & 3 deletions app/module/mail/ModuleMail.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1641,7 +1641,7 @@ def _action_move(self, client: ClientMailServer, folder_name: str, mail_uid: str
raise RequestException("Missing or invalid destination folder for move action", err.ERROR_MISSING_ACTION_DATA)

client.copy_mail_to_mailbox(folder_name, mail_uid, destination)
client.add_flags_to_mail(folder_name, mail_uid, ['\\Deleted'])
self._flag_deleted_and_expunge(client, folder_name, mail_uid)

return {"action": "move", "mail_uid": mail_uid, "from_folder": folder_name, "to_folder": destination}

Expand All @@ -1658,7 +1658,7 @@ def _action_spam(self, client: ClientMailServer, folder_name: str, mail_uid: str
"""
junk_folder = self.domain_mail_folder_name.get(cs.MAIL_FOLDER_JUNK, "Junk")
client.copy_mail_to_mailbox(folder_name, mail_uid, junk_folder, create_dest=True)
client.add_flags_to_mail(folder_name, mail_uid, ['\\Deleted'])
self._flag_deleted_and_expunge(client, folder_name, mail_uid)
return {"action": "spam", "mail_uid": mail_uid, "moved_to": junk_folder}

def _action_ham(self, client: ClientMailServer, folder_name: str, mail_uid: str|list[str]) -> dict[str, Any]:
Expand All @@ -1675,7 +1675,7 @@ def _action_ham(self, client: ClientMailServer, folder_name: str, mail_uid: str|
inbox_folder = self.domain_mail_folder_name.get(cs.MAIL_FOLDER_INBOX, "INBOX")
junk_folder = self.domain_mail_folder_name.get(cs.MAIL_FOLDER_JUNK, "Junk")
client.copy_mail_to_mailbox(junk_folder, mail_uid, inbox_folder)
client.add_flags_to_mail(folder_name, mail_uid, ['\\Deleted'])
self._flag_deleted_and_expunge(client, folder_name, mail_uid)

return {"action": "ham", "mail_uid": mail_uid, "moved_to": inbox_folder}

Expand All @@ -1699,6 +1699,18 @@ def _action_copy(self, client: ClientMailServer, folder_name: str, mail_uid: str

return {"action": "copy", "mail_uid": mail_uid, "from_folder": folder_name, "to_folder": destination}

def _flag_deleted_and_expunge(
self, client: ClientMailServer, folder_name: str, mail_uid: str|list[str]
) -> None:
"""Flag mails as deleted then expunge the source folder.

IMAP COPY + ``\\Deleted`` is not a move until EXPUNGE: without it the
original stays visible in the source folder (and a later move copies it
again, producing duplicates).
"""
client.add_flags_to_mail(folder_name, mail_uid, ['\\Deleted'])
client.expunge_folder(folder_name, do_children=False)

def _action_delete(self, client: ClientMailServer, folder_name: str, mail_uid: str|list[str], account_id: str) -> dict[str, Any]:
"""Delete a mail or a list of mails, honoring the user's ``SOGO_U_MAIL_DELETE_BEHAVIOR`` preference.

Expand Down
12 changes: 11 additions & 1 deletion tests/test_module/test_mail/test_moduleMail.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ def __init__(self):
self.add_flags_calls = []
self.remove_flags_calls = []
self.delete_mails_by_uid_calls = []
self.expunge_folder_calls = []
self.set_acl_calls = []
self.delete_acl_calls = []

Expand All @@ -66,7 +67,8 @@ def delete_folder(self, folder_path, do_children=True):
if self.delete_folder_result is not None:
raise self.delete_folder_result

def expunge_folder(self, folder_path, do_subfolders=True):
def expunge_folder(self, folder_path, do_children=True):
self.expunge_folder_calls.append((folder_path, do_children))
return self.expunge_folder_result

def purge_folder(self, folder_path, before_date=None, do_children=False, permanently=False):
Expand Down Expand Up @@ -585,6 +587,7 @@ def test_perform_mail_action_move_success(monkeypatch):
assert result["to_folder"] == "Archive"
assert ("INBOX", "42", "Archive") in fake_client.copy_mail_to_mailbox_calls
assert ("INBOX", "42", ['\\Deleted']) in fake_client.add_flags_calls
assert ("INBOX", False) in fake_client.expunge_folder_calls


def test_perform_mail_action_move_missing_destination(monkeypatch):
Expand All @@ -610,6 +613,7 @@ def test_perform_mail_action_spam_success(monkeypatch):
assert result["moved_to"] == "Junk"
assert ("INBOX", "42", "Junk") in fake_client.copy_mail_to_mailbox_calls
assert ("INBOX", "42", ['\\Deleted']) in fake_client.add_flags_calls
assert ("INBOX", False) in fake_client.expunge_folder_calls


def test_perform_mail_action_ham_success(monkeypatch):
Expand All @@ -625,6 +629,7 @@ def test_perform_mail_action_ham_success(monkeypatch):
assert result["moved_to"] == "INBOX"
assert ("Junk", "42", "INBOX") in fake_client.copy_mail_to_mailbox_calls
assert ("Junk", "42", ['\\Deleted']) in fake_client.add_flags_calls
assert ("Junk", False) in fake_client.expunge_folder_calls


def test_perform_mail_action_copy_success(monkeypatch):
Expand All @@ -641,6 +646,7 @@ def test_perform_mail_action_copy_success(monkeypatch):
assert ("INBOX", "42", "Archive") in fake_client.copy_mail_to_mailbox_calls
# Copy should NOT delete the original mail
assert ("INBOX", "42", ['\\Deleted']) not in fake_client.add_flags_calls
assert fake_client.expunge_folder_calls == []


def test_perform_mail_action_copy_missing_destination(monkeypatch):
Expand Down Expand Up @@ -744,6 +750,7 @@ def test_perform_mail_batch_action_move_success(monkeypatch):
assert result["to_folder"] == "Archive"
assert ("INBOX", ["42", "43"], "Archive") in fake_client.copy_mail_to_mailbox_calls
assert ("INBOX", ["42", "43"], ['\\Deleted']) in fake_client.add_flags_calls
assert ("INBOX", False) in fake_client.expunge_folder_calls


def test_perform_mail_batch_action_move_missing_destination(monkeypatch):
Expand All @@ -769,6 +776,7 @@ def test_perform_mail_batch_action_spam_success(monkeypatch):
assert result["moved_to"] == "Junk"
assert ("INBOX", ["42", "43"], "Junk") in fake_client.copy_mail_to_mailbox_calls
assert ("INBOX", ["42", "43"], ['\\Deleted']) in fake_client.add_flags_calls
assert ("INBOX", False) in fake_client.expunge_folder_calls


def test_perform_mail_batch_action_ham_success(monkeypatch):
Expand All @@ -784,6 +792,7 @@ def test_perform_mail_batch_action_ham_success(monkeypatch):
assert result["moved_to"] == "INBOX"
assert ("Junk", ["42", "43"], "INBOX") in fake_client.copy_mail_to_mailbox_calls
assert ("Junk", ["42", "43"], ['\\Deleted']) in fake_client.add_flags_calls
assert ("Junk", False) in fake_client.expunge_folder_calls


def test_perform_mail_batch_action_copy_success(monkeypatch):
Expand All @@ -800,6 +809,7 @@ def test_perform_mail_batch_action_copy_success(monkeypatch):
assert ("INBOX", ["42", "43"], "Archive") in fake_client.copy_mail_to_mailbox_calls
# Copy should NOT delete the original mails
assert ("INBOX", ["42", "43"], ['\\Deleted']) not in fake_client.add_flags_calls
assert fake_client.expunge_folder_calls == []


def test_perform_mail_batch_action_copy_missing_destination(monkeypatch):
Expand Down
Loading