Merge pull request #1050 from modoboa/fix/delete_mailboxes_with_domain

Fixed mailbox removal.

modoboa/admin/management/commands/handle_mailbox_operations.py

@@ -2,10 +2,11 @@
 
 import logging
 import os
+import shutil
 
 from django.core.management.base import BaseCommand
 
-from param_tools import tools as param_tools
+from modoboa.parameters import tools as param_tools
 from modoboa.lib.sysutils import exec_cmd
 from modoboa.lib.exceptions import InternalError
 
@@ -52,13 +53,16 @@ class Command(BaseCommand):
             raise OperationError(output)
 
     def delete_mailbox(self, operation):
+        """Try to delete a mailbox tree on filesystem."""
         if not os.path.exists(operation.argument):
             return
-        code, output = exec_cmd(
-            "rm -r %s" % operation.argument
-        )
-        if code:
-            raise OperationError(output)
+
+        def onerror(function, path, excinfo):
+            """Handle errors."""
+            self.logger.critical(
+                "delete failed (reason: {})".format(excinfo))
+
+        shutil.rmtree(operation.argument, False, onerror)
 
     def check_pidfile(self, path):
         """Check if this command is already running
@@ -93,8 +97,8 @@ class Command(BaseCommand):
             try:
                 f(ope)
             except (OperationError, InternalError) as e:
-                self.logger.critical("%s failed (reason: %s)",
-                                     ope, str(e).encode("utf-8"))
+                self.logger.critical(
+                    "%s failed (reason: %s)", ope, str(e).decode("utf-8"))
             else:
                 self.logger.info("%s succeed", ope)
                 ope.delete()

modoboa/admin/models/domain.py

@@ -226,8 +226,7 @@ class Domain(AdminObject):
         if self.alias_set.count():
             ungrant_access_to_objects(self.alias_set.all())
         if param_tools.get_global_parameter("auto_account_removal"):
-            for account in User.objects.filter(mailbox__domain=self):
-                account.delete(fromuser, keepdir)
+            User.objects.filter(mailbox__domain=self).delete()
         elif self.mailbox_set.count():
             Quota.objects.filter(username__contains="@%s" % self.name).delete()
             ungrant_access_to_objects(self.mailbox_set.all())

modoboa/admin/models/mailbox.py

@@ -350,7 +350,6 @@ reversion.register(SenderAddress)
 
 
 class MailboxOperation(models.Model):
-
     """An operation on a mailbox."""
 
     mailbox = models.ForeignKey(Mailbox, blank=True, null=True)

modoboa/admin/tests/dovecot

@@ -0,0 +1,3 @@
+#!/bin/bash
+
+echo "2.12"

modoboa/admin/tests/test_domain.py

@@ -186,12 +186,18 @@ class DomainTestCase(ModoTestCase):
     def test_delete(self):
         """Test the removal of a domain."""
         dom = Domain.objects.get(name="test.com")
-        self.ajax_post(
-            reverse("admin:domain_delete", args=[dom.id]),
-            {}
-        )
+        self.ajax_post(reverse("admin:domain_delete", args=[dom.id]))
         with self.assertRaises(Domain.DoesNotExist):
             Domain.objects.get(pk=1)
+        self.assertTrue(
+            User.objects.filter(username="user@test.com").exists())
+        self.set_global_parameter("auto_account_removal", True)
+        dom = Domain.objects.get(name="test2.com")
+        self.ajax_post(reverse("admin:domain_delete", args=[dom.id]))
+        with self.assertRaises(Domain.DoesNotExist):
+            Domain.objects.get(pk=1)
+        self.assertFalse(
+            User.objects.filter(username="admin@test2.com").exists())
 
     def test_domain_counters(self):
         """Check counters at domain level."""

modoboa/admin/tests/test_mailbox_operations.py

@@ -0,0 +1,86 @@
+"""Management command tests."""
+
+import os
+import shutil
+import tempfile
+
+import mock
+
+from django.core.management import call_command
+from django.core.urlresolvers import reverse
+from django.test import override_settings
+
+from modoboa.lib.tests import ModoTestCase
+
+from .. import factories
+from .. import models
+
+
+@override_settings(
+    DOVECOT_LOOKUP_PATH=["{}/dovecot".format(os.path.dirname(__file__))])
+class MailboxOperationTestCase(ModoTestCase):
+    """Test management command."""
+
+    @classmethod
+    def setUpTestData(cls):
+        """Create test data."""
+        super(MailboxOperationTestCase, cls).setUpTestData()
+        factories.populate_database()
+
+    def setUp(self):
+        """Initiate test env."""
+        super(MailboxOperationTestCase, self).setUp()
+        self.workdir = tempfile.mkdtemp()
+        path = "{}/test.com/admin".format(self.workdir)
+        os.makedirs(path)
+        self.set_global_parameter("handle_mailboxes", True)
+        self.set_global_parameter("enable_admin_limits", False, app="limits")
+
+    def tearDown(self):
+        """Reset test env."""
+        shutil.rmtree(self.workdir)
+
+    @mock.patch("modoboa.admin.models.Mailbox.mail_home")
+    def test_delete_account(self, mail_home_mock):
+        """Check delete operation."""
+        path = "{}/test.com/admin".format(self.workdir)
+        mail_home_mock.__get__ = mock.Mock(return_value=path)
+        mb = models.Mailbox.objects.select_related("user").get(
+            address="admin", domain__name="test.com")
+        self.ajax_post(
+            reverse("admin:account_delete", args=[mb.user.pk]), {}
+        )
+        call_command("handle_mailbox_operations")
+        self.assertFalse(models.MailboxOperation.objects.exists())
+        self.assertFalse(os.path.exists(mb.mail_home))
+
+    @mock.patch("modoboa.admin.models.Mailbox.mail_home")
+    def test_rename_account(self, mail_home_mock):
+        """Check rename operation."""
+        path = "{}/test.com/admin".format(self.workdir)
+        mail_home_mock.__get__ = mock.Mock(return_value=path)
+        mb = models.Mailbox.objects.select_related("user").get(
+            address="admin", domain__name="test.com")
+        values = {
+            "username": "admin2@test.com", "role": "DomainAdmins",
+            "is_active": True, "email": "admin2@test.com"
+        }
+        self.ajax_post(
+            reverse("admin:account_change", args=[mb.user.pk]), values
+        )
+        path = "{}/test.com/admin2".format(self.workdir)
+        mail_home_mock.__get__ = mock.Mock(return_value=path)
+        call_command("handle_mailbox_operations")
+        self.assertFalse(models.MailboxOperation.objects.exists())
+        self.assertTrue(os.path.exists(mb.mail_home))
+
+    @mock.patch("modoboa.admin.models.Mailbox.mail_home")
+    def test_delete_domain(self, mail_home_mock):
+        """Check delete operation."""
+        path = "{}/test.com/admin".format(self.workdir)
+        mail_home_mock.__get__ = mock.Mock(return_value=path)
+        domain = models.Domain.objects.get(name="test.com")
+        self.ajax_post(reverse("admin:domain_delete", args=[domain.pk]))
+        call_command("handle_mailbox_operations")
+        self.assertFalse(models.MailboxOperation.objects.exists())
+        self.assertFalse(os.path.exists(path))

modoboa/admin/views/domain.py

@@ -135,7 +135,7 @@ def editdomain(request, dom_id):
 @login_required
 @permission_required("admin.delete_domain")
 def deldomain(request, dom_id):
-    keepdir = True if request.POST.get("keepdir", "false") == "true" else False
+    keepdir = request.POST.get("keepdir", "false") == "true"
     try:
         mb = Mailbox.objects.get(user__id=request.user.id)
     except Mailbox.DoesNotExist: