🐛(backend) fix dimail call despite mailbox creation failure on our side

When we try to create a duplicate email, a request
to dimail is sent despite a reject on our side.
To solve this issue, we call mailbox creation first
to benefit from validation of our Django model.

Mailbox creation try was called too late after dimail call.
So in attempt to create a duplicated email a request
to dimail was sent despite a failure in our side.

CHANGELOG.md

@@ -14,6 +14,7 @@ and this project adheres to
 
 ### Fixed
 
+- 🐛(backend) fix dimail call despite mailbox creation failure on our side
 - 🧑‍💻(user) fix the User.language infinite migration #611
 
 ## [1.9.1] - 2024-12-18

src/backend/mailbox_manager/api/client/serializers.py

@@ -35,8 +35,7 @@ class MailboxSerializer(serializers.ModelSerializer):
         """
         Override create function to fire a request on mailbox creation.
         """
-        mailbox_status = enums.MailDomainStatusChoices.PENDING
-
+        mailbox = super().create(validated_data)
         if validated_data["domain"].status == enums.MailDomainStatusChoices.ENABLED:
             client = DimailAPIClient()
             # send new mailbox request to dimail
@@ -48,8 +47,8 @@ class MailboxSerializer(serializers.ModelSerializer):
             mailbox_data = json.loads(
                 response.content.decode("utf-8").replace("'", '"')
             )
-
-            mailbox_status = enums.MailDomainStatusChoices.ENABLED
+            mailbox.status = enums.MailDomainStatusChoices.ENABLED
+            mailbox.save()
 
             # send confirmation email
             client.notify_mailbox_creation(
@@ -57,7 +56,7 @@ class MailboxSerializer(serializers.ModelSerializer):
             )
 
         # actually save mailbox on our database
-        return models.Mailbox.objects.create(**validated_data, status=mailbox_status)
+        return mailbox
 
 
 class MailDomainSerializer(serializers.ModelSerializer):

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

@@ -269,6 +269,55 @@ def test_api_mailboxes__cannot_create_on_disabled_domain(role):
     ]
 
 
+def test_api_mailboxes__no_dimail_call_if_mailbox_creation_failed():
+    """Duplication case fails on our side at creation step thanks to unique_together
+    on Mailbox model and no dimail call should be made."""
+    mail_domain = factories.MailDomainEnabledFactory()
+    mailbox = factories.MailboxFactory(domain=mail_domain)
+    access = factories.MailDomainAccessFactory(
+        role=enums.MailDomainRoleChoices.ADMIN, domain=mail_domain
+    )
+
+    client = APIClient()
+    client.force_login(access.user)
+
+    # now we try to make the same mailbox
+    mailbox_data = serializers.MailboxSerializer(mailbox).data
+    with responses.RequestsMock(assert_all_requests_are_fired=False) as rsps:
+        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",
+        )
+        # try to create duplicate mailbox but django validation does not agree
+        assert response.status_code == status.HTTP_400_BAD_REQUEST
+        # no new mailbox should be created
+        assert models.Mailbox.objects.count() == 1
+        # no dimail call should be made
+        assert len(rsps.calls) == 0
+
+
 @pytest.mark.parametrize(
     "domain_status",
     [
@@ -524,7 +573,9 @@ def test_api_mailboxes__dimail_token_permission_denied():
         assert response.json() == {
             "detail": "Token denied. Please check your MAIL_PROVISIONING_API_CREDENTIALS."
         }
-        assert not models.Mailbox.objects.exists()
+        # mailbox was created in our side only and in pending status
+        mailbox = models.Mailbox.objects.get()
+        assert mailbox.status == enums.MailboxStatusChoices.PENDING
 
 
 def test_api_mailboxes__user_unrelated_to_domain():
@@ -569,7 +620,9 @@ def test_api_mailboxes__user_unrelated_to_domain():
         assert response.json() == {
             "detail": "Permission denied. Please check your MAIL_PROVISIONING_API_CREDENTIALS."
         }
-        assert not models.Mailbox.objects.exists()
+        # mailbox was created in our side only and in pending status
+        mailbox = models.Mailbox.objects.get()
+        assert mailbox.status == enums.MailboxStatusChoices.PENDING
 
 
 def test_api_mailboxes__handling_dimail_unexpected_error(caplog):
@@ -612,7 +665,10 @@ def test_api_mailboxes__handling_dimail_unexpected_error(caplog):
             assert response.json() == {
                 "detail": "Unexpected response from dimail: {'details': 'Internal server error'}"
             }
-        assert not models.Mailbox.objects.exists()
+
+        # mailbox was created in our side only and in pending status
+        mailbox = models.Mailbox.objects.get()
+        assert mailbox.status == enums.MailboxStatusChoices.PENDING
 
         # Check error logger was called
         assert caplog.records[0].levelname == "ERROR"