From 0ea37d71135c2d3e5c0574cb7371775240869347 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 2 Oct 2024 18:12:40 -0400 Subject: [PATCH] unit tests --- src/registrar/admin.py | 13 +++++++-- src/registrar/tests/test_admin_request.py | 35 +++++++---------------- 2 files changed, 21 insertions(+), 27 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 79e9d55b3..321014949 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2013,8 +2013,17 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): should_proceed = False return (obj, should_proceed) - request_is_not_approved = original_obj.status != models.DomainRequest.DomainRequestStatus.APPROVED - if request_is_not_approved and Domain.objects.filter(name=original_obj.requested_domain.name).exists(): + obj_is_not_approved = obj.status != models.DomainRequest.DomainRequestStatus.APPROVED + original_obj_is_not_approved = original_obj.status != models.DomainRequest.DomainRequestStatus.APPROVED + if obj_is_not_approved and not obj.domain_is_not_active(): + # If an admin tried to set an approved domain request to + # another status and the related domain is already + # active (READY), shortcut the action and throw a friendly + # error message. This action would still not go through + # shortcut or not as the rules are duplicated on the model, + # but the error would be an ugly Django error screen. + error_message = "This action is not permitted. The domain is already active." + if original_obj_is_not_approved and Domain.objects.filter(name=original_obj.requested_domain.name).exists(): # REDUNDANT CHECK: # This action (approving a request when the domain is active) # would still not go through check or not as the rules are diff --git a/src/registrar/tests/test_admin_request.py b/src/registrar/tests/test_admin_request.py index 6fd2da8db..a9b073472 100644 --- a/src/registrar/tests/test_admin_request.py +++ b/src/registrar/tests/test_admin_request.py @@ -25,7 +25,6 @@ from registrar.models import ( Portfolio, AllowedEmail, ) -from registrar.utility.errors import FSMErrorCodes from .common import ( MockSESClient, completed_domain_request, @@ -38,7 +37,6 @@ from .common import ( GenericTestHelper, ) from unittest.mock import patch -from django_fsm import TransitionNotAllowed from django.conf import settings import boto3_mocking # type: ignore @@ -1798,10 +1796,18 @@ class TestDomainRequestAdmin(MockEppLib): domain_request.rejection_reason = rejection_reason + self.admin.save_model(request, domain_request, None, True) + + # Assert that the error message was called with the correct argument if domain_is_active: - with self.assertRaises(TransitionNotAllowed): - self.admin.save_model(request, domain_request, None, True) + messages.error.assert_called_once_with( + request, + "This action is not permitted. The domain " + "is already active.", + ) else: + # Assert that the error message was never called + messages.error.assert_not_called() + self.assertEqual(domain_request.approved_domain, None) # Assert that Domain got Deleted @@ -1812,27 +1818,6 @@ class TestDomainRequestAdmin(MockEppLib): with self.assertRaises(DomainInformation.DoesNotExist): domain_information.refresh_from_db() - - # # Assert that the error message was called with the correct argument - # if domain_is_active: - # messages.error.assert_called_once_with( - # request, - # FSMErrorCodes.APPROVE_DOMAIN_IN_USE, - # ) - # else: - # # Assert that the error message was never called - # messages.error.assert_not_called() - - # self.assertEqual(domain_request.approved_domain, None) - - # # Assert that Domain got Deleted - # with self.assertRaises(Domain.DoesNotExist): - # domain.refresh_from_db() - - # # Assert that DomainInformation got Deleted - # with self.assertRaises(DomainInformation.DoesNotExist): - # domain_information.refresh_from_db() - def test_error_when_saving_approved_to_in_review_and_domain_is_active(self): self.trigger_saving_approved_to_another_state(True, DomainRequest.DomainRequestStatus.IN_REVIEW)