diff --git a/src/backend/mailbox_manager/api/client/viewsets.py b/src/backend/mailbox_manager/api/client/viewsets.py index d8ab1ee..545bd94 100644 --- a/src/backend/mailbox_manager/api/client/viewsets.py +++ b/src/backend/mailbox_manager/api/client/viewsets.py @@ -2,7 +2,7 @@ from django.db.models import Q, Subquery -from rest_framework import exceptions, filters, mixins, viewsets +from rest_framework import exceptions, filters, mixins, status, viewsets from rest_framework.decorators import action from rest_framework.response import Response @@ -477,3 +477,19 @@ class AliasViewSet( slug=domain_slug ) super().perform_create(serializer) + + def destroy(self, request, *args, **kwargs): + """Override destroy method to send a delete request to dimail + and return clear message if domain out of sync.""" + instance = self.get_object() + self.perform_destroy(instance) + + client = DimailAPIClient() + dimail_response = client.delete_alias(instance) + if dimail_response.status_code == status.HTTP_404_NOT_FOUND: + return Response( + "Alias already deleted. Domain out of sync, please contact our support.", + status=status.HTTP_200_OK, + ) + + return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/src/backend/mailbox_manager/tests/api/aliases/test_api_aliases_delete.py b/src/backend/mailbox_manager/tests/api/aliases/test_api_aliases_delete.py index 8f5df93..14bade2 100644 --- a/src/backend/mailbox_manager/tests/api/aliases/test_api_aliases_delete.py +++ b/src/backend/mailbox_manager/tests/api/aliases/test_api_aliases_delete.py @@ -2,6 +2,7 @@ Tests for aliases API endpoint in People's app mailbox_manager. Focus on "list" action. """ +# pylint: disable=W0613 import re @@ -67,7 +68,8 @@ def test_api_aliases_delete__viewer_forbidden(): assert models.Alias.objects.count() == 1 -def test_api_aliases_delete__viewer_can_delete_self_alias(): +@responses.activate +def test_api_aliases_delete__viewer_can_delete_self_alias(dimail_token_ok): """ Authenticated users should be allowed to delete aliases when linking to their own mailbox. @@ -80,30 +82,8 @@ def test_api_aliases_delete__viewer_can_delete_self_alias(): domain=mail_domain, destination=authenticated_user.email ) - client = APIClient() - client.force_login(authenticated_user) - response = client.delete( - f"/api/v1.0/mail-domains/{mail_domain.slug}/aliases/{alias.local_part}/", - ) - - assert response.status_code == status.HTTP_204_NO_CONTENT - assert not models.Alias.objects.exists() - - -@responses.activate -def test_api_aliases_delete__administrators_allowed(): - """ - Administrators of a mail domain should be allowed to delete accesses excepted owner accesses. - """ - authenticated_user = core_factories.UserFactory() - mail_domain = factories.MailDomainFactory( - users=[(authenticated_user, enums.MailDomainRoleChoices.ADMIN)] - ) - alias = factories.AliasFactory(domain=mail_domain) - # Mock dimail response - responses.add( - responses.POST, + responses.delete( re.compile(r".*/aliases/"), status=status.HTTP_204_NO_CONTENT, content_type="application/json", @@ -117,3 +97,63 @@ def test_api_aliases_delete__administrators_allowed(): assert response.status_code == status.HTTP_204_NO_CONTENT assert not models.Alias.objects.exists() + + +@responses.activate +def test_api_aliases_delete__administrators_allowed(dimail_token_ok): + """ + Administrators of a mail domain should be allowed to delete accesses excepted owner accesses. + """ + authenticated_user = core_factories.UserFactory() + mail_domain = factories.MailDomainFactory( + users=[(authenticated_user, enums.MailDomainRoleChoices.ADMIN)] + ) + alias = factories.AliasFactory(domain=mail_domain) + + # Mock dimail response + responses.delete( + re.compile(r".*/aliases/"), + status=status.HTTP_204_NO_CONTENT, + content_type="application/json", + ) + + client = APIClient() + client.force_login(authenticated_user) + response = client.delete( + f"/api/v1.0/mail-domains/{mail_domain.slug}/aliases/{alias.local_part}/", + ) + + assert response.status_code == status.HTTP_204_NO_CONTENT + assert not models.Alias.objects.exists() + + +@responses.activate +def test_api_aliases_delete__404_out_of_sync(dimail_token_ok): + """ + Should return a clear message when dimail returns 404 not found. + """ + authenticated_user = core_factories.UserFactory() + mail_domain = factories.MailDomainFactory( + users=[(authenticated_user, enums.MailDomainRoleChoices.ADMIN)] + ) + alias = factories.AliasFactory(domain=mail_domain) + + # Mock dimail response + responses.delete( + re.compile(r".*/aliases/"), + json={"detail": "Not found"}, + status=status.HTTP_404_NOT_FOUND, + content_type="application/json", + ) + + client = APIClient() + client.force_login(authenticated_user) + response = client.delete( + f"/api/v1.0/mail-domains/{mail_domain.slug}/aliases/{alias.local_part}/", + ) + assert response.status_code == status.HTTP_200_OK + assert ( + response.json() + == "Alias already deleted. Domain out of sync, please contact our support." + ) + assert not models.Alias.objects.exists() diff --git a/src/backend/mailbox_manager/utils/dimail.py b/src/backend/mailbox_manager/utils/dimail.py index f9ba34a..f392933 100644 --- a/src/backend/mailbox_manager/utils/dimail.py +++ b/src/backend/mailbox_manager/utils/dimail.py @@ -747,7 +747,7 @@ class DimailAPIClient: ) raise error - if response.status_code == status.HTTP_201_CREATED: + if response.status_code == status.HTTP_204_NO_CONTENT: logger.info( "User %s removed destination %s from alias %s.", request_user, @@ -765,4 +765,14 @@ class DimailAPIClient: "Permission denied. Please check your MAIL_PROVISIONING_API_CREDENTIALS." ) + if response.status_code == status.HTTP_404_NOT_FOUND: + logger.error( + "[DIMAIL] 404, alias %s not found. Domain out of sync with dimail. Admin, please import aliases for domain %s", + str(alias.local_part), + str(alias.domain), + ) + # we don't raise error because we actually want this alias to be deleted + # to match dimail's states + return response + return self.raise_exception_for_unexpected_response(response)