improve some code

src/backend/mailbox_manager/admin.py

@@ -1,15 +1,11 @@
 """Admin classes and registrations for People's mailbox manager app."""
 
-import logging
-
 from django.contrib import admin, messages
 from django.utils.translation import gettext_lazy as _
 
 from mailbox_manager import models
 from mailbox_manager.utils.dimail import DimailAPIClient
 
-logger = logging.getLogger(__name__)
-
 
 @admin.action(description=_("Synchronise from dimail"))
 def sync_mailboxes_from_dimail(modeladmin, request, queryset):  # pylint: disable=unused-argument
@@ -18,15 +14,12 @@ def sync_mailboxes_from_dimail(modeladmin, request, queryset):  # pylint: disabl
 
     for domain in queryset:
         try:
-            client.synchronize_mailboxes_from_dimail(domain.name)
+            client.synchronize_mailboxes_from_dimail(domain)
         except SystemError as err:
             messages.error(
                 request,
                 _(f"Synchronisation failed for {domain.name} with message: [{err}]"),
             )
-            logger.exception(
-                "Synchronisation failed for %s with message %s", domain.name, err
-            )
 
 
 class UserMailDomainAccessInline(admin.TabularInline):

src/backend/mailbox_manager/tests/test_utils_dimail_client.py

@@ -53,7 +53,7 @@ def test_dimail_synchronization__already_sync():
             status=status.HTTP_200_OK,
             content_type="application/json",
         )
-        dimail_client.synchronize_mailboxes_from_dimail(domain.name)
+        dimail_client.synchronize_mailboxes_from_dimail(domain)
 
     post_sync_mailboxes = models.Mailbox.objects.filter(domain=domain)
     assert post_sync_mailboxes.count() == 3
@@ -94,6 +94,6 @@ def test_dimail_synchronization__synchronize_mailboxes():
             status=status.HTTP_200_OK,
             content_type="application/json",
         )
-        dimail_client.synchronize_mailboxes_from_dimail(domain.name)
+        dimail_client.synchronize_mailboxes_from_dimail(domain)
 
     assert models.Mailbox.objects.exists()

src/backend/mailbox_manager/utils/dimail.py

@@ -167,20 +167,15 @@ def send_new_mailbox_notification(self, recipient, mailbox_data):
                 exception,
             )
 
-    def synchronize_mailboxes_from_dimail(self, domain_name):
+    def synchronize_mailboxes_from_dimail(self, domain):
         """Synchronize mailboxes from dimail - open xchange to our database.
         This is useful in case of acquisition of a pre-existing mail domain.
         Mailboxes created here are not new mailboxes and will not trigger mail notification."""
 
-        domain = models.MailDomain.objects.get(name=domain_name)
-        people_mailboxes = models.Mailbox.objects.filter(domain=domain)
-
-        headers = self.get_headers()
-
         try:
             response = session.get(
-                f"{self.API_URL}/domains/{domain_name}/mailboxes/",
-                headers=headers,
+                f"{self.API_URL}/domains/{domain.name}/mailboxes/",
+                headers=self.get_headers(),
                 verify=True,
                 timeout=10,
             )
@@ -195,26 +190,23 @@ def synchronize_mailboxes_from_dimail(self, domain_name):
         if response.status_code != status.HTTP_200_OK:
             return self.pass_dimail_unexpected_response(response)
 
-        content = ast.literal_eval(
+        dimail_mailboxes = ast.literal_eval(
             response.content.decode("utf-8")
         )  # format output str to proper list
 
-        for dimail_mailbox in content:
-            local_part = dimail_mailbox["email"].split("@")[0]
-
+        people_mailboxes = models.Mailbox.objects.filter(domain=domain)
+        for dimail_mailbox in dimail_mailboxes:
             if not dimail_mailbox["email"] in [
                 str(people_mailbox) for people_mailbox in people_mailboxes
             ]:
                 # creates a mailbox on our end
                 models.Mailbox.objects.create(
                     first_name=dimail_mailbox["givenName"],
                     last_name=dimail_mailbox["surName"],
-                    local_part=local_part,
+                    local_part=dimail_mailbox["email"].split("@")[0],
                     domain=domain,
                     secondary_email=dimail_mailbox[
                         "email"
                     ],  # secondary email is mandatory. Unfortunately, dimail doesn't store any.
                     # We temporarily give current email as secondary email.
                 )
-
-        return None