♻️(serializers) move dimail calls to serializers

we move all business logic from model to serializer.
all API calls (direct and from front) will keep on triggering
expected 3rd party calls while admin actions will uniquely trigger
modifications in our database.

CHANGELOG.md

@@ -11,8 +11,13 @@ and this project adheres to
 
 ### Added
 
-- ✨(domains) add endpoint to list and retrieve domain accesses @sdemagny #404
-- 🍱(dev) embark dimail-api as container by @mjeammet #366
+- ✨(domains) add endpoint to list and retrieve domain accesses #404
+- 🍱(dev) embark dimail-api as container #366
+
+
+### Changed
+
+- ♻️(serializers) move business logic to serializers #414 
 
 ## [1.1.0] - 2024-09-10
 

src/backend/mailbox_manager/api/serializers.py

@@ -5,6 +5,7 @@ from rest_framework import serializers
 from core.api.serializers import UserSerializer
 
 from mailbox_manager import enums, models
+from mailbox_manager.utils.dimail import DimailAPIClient
 
 
 class MailboxSerializer(serializers.ModelSerializer):
@@ -16,6 +17,14 @@ class MailboxSerializer(serializers.ModelSerializer):
         # everything is actually read-only as we do not allow update for now
         read_only_fields = ["id"]
 
+    def create(self, validated_data):
+        """
+        Override create function to fire a request on mailbox creation.
+        """
+        client = DimailAPIClient()
+        client.send_mailbox_request(validated_data)
+        return models.Mailbox.objects.create(**validated_data)
+
 
 class MailDomainSerializer(serializers.ModelSerializer):
     """Serialize mail domain."""

src/backend/mailbox_manager/factories.py

@@ -2,14 +2,10 @@
 Mailbox manager application factories
 """
 
-import re
-
 from django.utils.text import slugify
 
 import factory.fuzzy
-import responses
 from faker import Faker
-from rest_framework import status
 
 from core import factories as core_factories
 from core import models as core_models
@@ -79,37 +75,3 @@ class MailboxFactory(factory.django.DjangoModelFactory):
     )
     domain = factory.SubFactory(MailDomainEnabledFactory)
     secondary_email = factory.Faker("email")
-
-    @classmethod
-    def _create(cls, model_class, *args, use_mock=True, **kwargs):
-        domain = kwargs["domain"]
-        if use_mock and isinstance(domain, models.MailDomain):
-            with responses.RequestsMock() as rsps:
-                # Ensure successful response using "responses":
-                rsps.add(
-                    rsps.GET,
-                    re.compile(r".*/token/"),
-                    body='{"access_token": "domain_owner_token"}',
-                    status=status.HTTP_200_OK,
-                    content_type="application/json",
-                )
-                rsps.add(
-                    rsps.POST,
-                    re.compile(
-                        rf".*/domains/{domain.name}/mailboxes/{kwargs['local_part']}"
-                    ),
-                    body=str(
-                        {
-                            "email": f"{kwargs['local_part']}@{domain.name}",
-                            "password": "newpass",
-                            "uuid": "uuid",
-                        }
-                    ),
-                    status=status.HTTP_201_CREATED,
-                    content_type="application/json",
-                )
-
-                result = super()._create(model_class, *args, **kwargs)
-        else:
-            result = super()._create(model_class, *args, **kwargs)
-        return result

src/backend/mailbox_manager/models.py

@@ -4,14 +4,13 @@ Declare and configure the models for the People additional application : mailbox
 
 from django.conf import settings
 from django.core import exceptions, validators
-from django.db import models, transaction
+from django.db import models
 from django.utils.text import slugify
 from django.utils.translation import gettext_lazy as _
 
 from core.models import BaseModel
 
 from mailbox_manager.enums import MailDomainRoleChoices, MailDomainStatusChoices
-from mailbox_manager.utils.dimail import DimailAPIClient
 
 
 class MailDomain(BaseModel):
@@ -155,16 +154,12 @@ class Mailbox(BaseModel):
 
     def save(self, *args, **kwargs):
         """
-        Override save function to fire a request on mailbox creation.
         Modification is forbidden for now.
         """
         self.full_clean()
 
         if self._state.adding:
-            with transaction.atomic():
-                client = DimailAPIClient()
-                client.send_mailbox_request(self)
-                return super().save(*args, **kwargs)
+            return super().save(*args, **kwargs)
 
         # Update is not implemented for now
         raise NotImplementedError()

src/backend/mailbox_manager/tests/api/mailboxes/test_api_mailboxes_create.py

@@ -4,6 +4,8 @@ Unit tests for the mailbox API
 
 import json
 import re
+from logging import Logger
+from unittest import mock
 
 from django.test.utils import override_settings
 
@@ -488,3 +490,59 @@ def test_api_mailboxes__handling_dimail_unexpected_error():
                 "detail": "Unexpected response from dimail: {'details': 'Internal server error'}"
             }
         assert not models.Mailbox.objects.exists()
+
+
+@mock.patch.object(Logger, "error")
+@mock.patch.object(Logger, "info")
+def test_api_mailboxes__send_correct_logger_infos(mock_info, mock_error):
+    """
+    Upon requesting mailbox creation, things are correctly logged
+    """
+    access = factories.MailDomainAccessFactory(role=enums.MailDomainRoleChoices.OWNER)
+
+    client = APIClient()
+    client.force_login(access.user)
+    mailbox_data = serializers.MailboxSerializer(
+        factories.MailboxFactory.build(domain=access.domain)
+    ).data
+
+    with responses.RequestsMock() as rsps:
+        # Ensure successful response using "responses":
+        rsps.add(
+            rsps.GET,
+            re.compile(r".*/token/"),
+            body='{"access_token": "domain_owner_token"}',
+            status=status.HTTP_200_OK,
+            content_type="application/json",
+        )
+        rsps.add(
+            rsps.POST,
+            re.compile(rf".*/domains/{access.domain.name}/mailboxes/"),
+            body=str(
+                {
+                    "email": f"{mailbox_data['local_part']}@{access.domain.name}",
+                    "password": "newpass",
+                    "uuid": "uuid",
+                }
+            ),
+            status=status.HTTP_201_CREATED,
+            content_type="application/json",
+        )
+
+        response = client.post(
+            f"/api/v1.0/mail-domains/{access.domain.slug}/mailboxes/",
+            mailbox_data,
+            format="json",
+        )
+        assert response.status_code == status.HTTP_201_CREATED
+
+    # Logger
+    assert not mock_error.called
+    assert mock_info.call_count == 3
+    assert mock_info.call_args_list[0][0] == (
+        "Token succesfully granted by mail-provisioning API.",
+    )
+    assert mock_info.call_args_list[1][0] == (
+        "Mailbox successfully created on domain %s",
+        access.domain.name,
+    )

src/backend/mailbox_manager/tests/test_models_mailboxes.py

@@ -2,20 +2,12 @@
 Unit tests for the mailbox model
 """
 
-import json
-import re
-from logging import Logger
-from unittest import mock
-
 from django.core import exceptions
 from django.test.utils import override_settings
 
 import pytest
-import responses
-from rest_framework import status
 
 from mailbox_manager import enums, factories, models
-from mailbox_manager.api import serializers
 
 pytestmark = pytest.mark.django_db
 
@@ -158,111 +150,3 @@ def test_models_mailboxes__dimail_no_credentials():
         match="Please configure MAIL_PROVISIONING_API_CREDENTIALS before creating any mailbox.",
     ):
         factories.MailboxFactory(domain=domain)
-
-
-@override_settings(MAIL_PROVISIONING_API_CREDENTIALS="wrongCredentials")
-def test_models_mailboxes__dimail_token_permissions_denied():
-    """
-    Our API should raise a clear "Permission denied" error
-    if dimail returns a permission denied on /token/ endpoint.
-    """
-
-    domain = factories.MailDomainEnabledFactory()
-
-    with responses.RequestsMock() as rsps:
-        # Ensure successful response by scim provider using "responses":
-        rsps.add(
-            rsps.GET,
-            re.compile(r".*/token/"),
-            body='{"detail": "Permission denied"}',
-            status=status.HTTP_403_FORBIDDEN,
-            content_type="application/json",
-        )
-
-        with pytest.raises(
-            exceptions.PermissionDenied,
-            match="Token denied. Please check your MAIL_PROVISIONING_API_CREDENTIALS.",
-        ):
-            mailbox = factories.MailboxFactory(use_mock=False, domain=domain)
-            # Payload sent to mailbox provider
-            payload = json.loads(rsps.calls[1].request.body)
-            assert payload == {
-                "displayName": f"{mailbox.first_name} {mailbox.last_name}",
-                "givenName": mailbox.first_name,
-                "surName": mailbox.last_name,
-            }
-
-
-@mock.patch.object(Logger, "error")
-@mock.patch.object(Logger, "info")
-def test_models_mailboxes__create_mailbox_success(mock_info, mock_error):
-    """Creating a mailbox sends the expected information and get expected response before saving."""
-    domain = factories.MailDomainEnabledFactory()
-
-    # generate mailbox data before mailbox, to mock responses
-    mailbox_data = serializers.MailboxSerializer(
-        factories.MailboxFactory.build(domain=domain)
-    ).data
-
-    with responses.RequestsMock() as rsps:
-        # Ensure successful response using "responses":
-        rsps.add(
-            rsps.GET,
-            re.compile(r".*/token/"),
-            body='{"access_token": "domain_owner_token"}',
-            status=status.HTTP_200_OK,
-            content_type="application/json",
-        )
-        rsps.add(
-            rsps.POST,
-            re.compile(rf".*/domains/{domain.name}/mailboxes/"),
-            body=str(
-                {
-                    "email": f"{mailbox_data['local_part']}@{domain.name}",
-                    "password": "newpass",
-                    "uuid": "uuid",
-                }
-            ),
-            status=status.HTTP_201_CREATED,
-            content_type="application/json",
-        )
-
-        mailbox = factories.MailboxFactory(
-            use_mock=False, local_part=mailbox_data["local_part"], domain=domain
-        )
-
-        # Check headers
-        headers = rsps.calls[1].request.headers
-        assert headers["Content-Type"] == "application/json"
-
-        # Payload sent to mailbox provider
-        payload = json.loads(rsps.calls[1].request.body)
-        assert payload == {
-            "displayName": f"{mailbox.first_name} {mailbox.last_name}",
-            "givenName": mailbox.first_name,
-            "surName": mailbox.last_name,
-        }
-
-    # Logger
-    assert not mock_error.called
-    assert mock_info.call_count == 2
-    assert mock_info.call_args_list[0][0] == (
-        "Token succesfully granted by mail-provisioning API.",
-    )
-    assert mock_info.call_args_list[1][0] == (
-        "Mailbox successfully created on domain %s",
-        domain.name,
-    )
-    assert mock_info.call_args_list[1][1] == (
-        {
-            "extra": {
-                "response": str(
-                    {
-                        "email": f"{mailbox.local_part}@{domain.name}",
-                        "password": "newpass",
-                        "uuid": "uuid",
-                    }
-                )
-            }
-        }
-    )

src/backend/mailbox_manager/utils/dimail.py

@@ -64,15 +64,15 @@ class DimailAPIClient:
         """Send a CREATE mailbox request to mail provisioning API."""
 
         payload = {
-            "givenName": mailbox.first_name,
-            "surName": mailbox.last_name,
-            "displayName": f"{mailbox.first_name} {mailbox.last_name}",
+            "givenName": mailbox["first_name"],
+            "surName": mailbox["last_name"],
+            "displayName": f"{mailbox['first_name']} {mailbox['last_name']}",
         }
         headers = self.get_headers()
 
         try:
             response = session.post(
-                f"{self.API_URL}/domains/{mailbox.domain}/mailboxes/{mailbox.local_part}/",
+                f"{self.API_URL}/domains/{mailbox['domain']}/mailboxes/{mailbox['local_part']}/",
                 json=payload,
                 headers=headers,
                 verify=True,
@@ -93,7 +93,7 @@ class DimailAPIClient:
             # In the meantime, we log mailbox info (including password !)
             logger.info(
                 "Mailbox successfully created on domain %s",
-                mailbox.domain.name,
+                str(mailbox["domain"]),
                 extra=extra,
             )
             return response