diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 1bfda0b84..4034bf35b 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -884,14 +884,11 @@ def save_model(self, request, obj, form, change): if ( obj and original_obj.status == models.DomainApplication.ApplicationStatus.APPROVED - and ( - obj.status == models.DomainApplication.ApplicationStatus.REJECTED - or obj.status == models.DomainApplication.ApplicationStatus.INELIGIBLE - ) + and obj.status != models.DomainApplication.ApplicationStatus.APPROVED and not obj.domain_is_not_active() ): # If an admin tried to set an approved application to - # rejected or ineligible and the related domain is already + # another status and the related domain is already # active, 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, diff --git a/src/registrar/models/domain_application.py b/src/registrar/models/domain_application.py index 4c1a8e22c..c37fc19b5 100644 --- a/src/registrar/models/domain_application.py +++ b/src/registrar/models/domain_application.py @@ -578,6 +578,19 @@ def domain_is_not_active(self): return not self.approved_domain.is_active() return True + def delete_and_clean_up_domain(self, called_from): + try: + domain_state = self.approved_domain.state + # Only reject if it exists on EPP + if domain_state != Domain.State.UNKNOWN: + self.approved_domain.deletedInEpp() + self.approved_domain.save() + self.approved_domain.delete() + self.approved_domain = None + except Exception as err: + logger.error(err) + logger.error(f"Can't query an approved domain while attempting {called_from}") + def _send_status_update_email(self, new_status, email_template, email_template_subject, send_email=True): """Send a status update email to the submitter. @@ -657,11 +670,19 @@ def submit(self): ApplicationStatus.INELIGIBLE, ], target=ApplicationStatus.IN_REVIEW, + conditions=[domain_is_not_active], ) def in_review(self): """Investigate an application that has been submitted. - This action is logged.""" + This action is logged. + + As side effects this will delete the domain and domain_information + (will cascade) when they exist.""" + + if self.status == self.ApplicationStatus.APPROVED: + self.delete_and_clean_up_domain("in_review") + literal = DomainApplication.ApplicationStatus.IN_REVIEW # Check if the tuple exists, then grab its value in_review = literal if literal is not None else "In Review" @@ -676,11 +697,19 @@ def in_review(self): ApplicationStatus.INELIGIBLE, ], target=ApplicationStatus.ACTION_NEEDED, + conditions=[domain_is_not_active], ) def action_needed(self): """Send back an application that is under investigation or rejected. - This action is logged.""" + This action is logged. + + As side effects this will delete the domain and domain_information + (will cascade) when they exist.""" + + if self.status == self.ApplicationStatus.APPROVED: + self.delete_and_clean_up_domain("reject_with_prejudice") + literal = DomainApplication.ApplicationStatus.ACTION_NEEDED # Check if the tuple is setup correctly, then grab its value action_needed = literal if literal is not None else "Action Needed" @@ -752,18 +781,9 @@ def reject(self): As side effects this will delete the domain and domain_information (will cascade), and send an email notification.""" + if self.status == self.ApplicationStatus.APPROVED: - try: - domain_state = self.approved_domain.state - # Only reject if it exists on EPP - if domain_state != Domain.State.UNKNOWN: - self.approved_domain.deletedInEpp() - self.approved_domain.save() - self.approved_domain.delete() - self.approved_domain = None - except Exception as err: - logger.error(err) - logger.error("Can't query an approved domain while attempting a DA reject()") + self.delete_and_clean_up_domain("reject") self._send_status_update_email( "action needed", @@ -792,17 +812,7 @@ def reject_with_prejudice(self): and domain_information (will cascade) when they exist.""" if self.status == self.ApplicationStatus.APPROVED: - try: - domain_state = self.approved_domain.state - # Only reject if it exists on EPP - if domain_state != Domain.State.UNKNOWN: - self.approved_domain.deletedInEpp() - self.approved_domain.save() - self.approved_domain.delete() - self.approved_domain = None - except Exception as err: - logger.error(err) - logger.error("Can't query an approved domain while attempting a DA reject_with_prejudice()") + self.delete_and_clean_up_domain("reject_with_prejudice") self.creator.restrict_user() diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 86cc287e8..809949ef2 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -699,41 +699,13 @@ def test_change_view_with_restricted_creator(self): "Cannot edit an application with a restricted creator.", ) - @boto3_mocking.patching - def test_error_when_saving_approved_to_rejected_and_domain_is_active(self): - # Create an instance of the model - application = completed_application(status=DomainApplication.ApplicationStatus.APPROVED) - domain = Domain.objects.create(name=application.requested_domain.name) - application.approved_domain = domain - application.save() - - # Create a request object with a superuser - request = self.factory.post("/admin/registrar/domainapplication/{}/change/".format(application.pk)) - request.user = self.superuser - - # Define a custom implementation for is_active - def custom_is_active(self): - return True # Override to return True - - # Use ExitStack to combine patch contexts - with ExitStack() as stack: - # Patch Domain.is_active and django.contrib.messages.error simultaneously - stack.enter_context(patch.object(Domain, "is_active", custom_is_active)) - stack.enter_context(patch.object(messages, "error")) + def trigger_saving_approved_to_another_state(self, domain_is_active, another_state): + """Helper method that triggers domain request state changes from approved to another state, + with an associated domain that can be either active (READY) or not. - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - with less_console_noise(): - # Simulate saving the model - application.status = DomainApplication.ApplicationStatus.REJECTED - self.admin.save_model(request, application, None, True) + Used to test errors when saving a change with an active domain, also used to test side effects + when saving a change goes through.""" - # Assert that the error message was called with the correct argument - messages.error.assert_called_once_with( - request, - "This action is not permitted. The domain " + "is already active.", - ) - - def test_side_effects_when_saving_approved_to_rejected(self): # Create an instance of the model application = completed_application(status=DomainApplication.ApplicationStatus.APPROVED) domain = Domain.objects.create(name=application.requested_domain.name) @@ -747,46 +719,7 @@ def test_side_effects_when_saving_approved_to_rejected(self): # Define a custom implementation for is_active def custom_is_active(self): - return False # Override to return False - - # Use ExitStack to combine patch contexts - with ExitStack() as stack: - # Patch Domain.is_active and django.contrib.messages.error simultaneously - stack.enter_context(patch.object(Domain, "is_active", custom_is_active)) - stack.enter_context(patch.object(messages, "error")) - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - with less_console_noise(): - # Simulate saving the model - application.status = DomainApplication.ApplicationStatus.REJECTED - self.admin.save_model(request, application, None, True) - - # Assert that the error message was never called - messages.error.assert_not_called() - - self.assertEqual(application.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_ineligible_and_domain_is_active(self): - # Create an instance of the model - application = completed_application(status=DomainApplication.ApplicationStatus.APPROVED) - domain = Domain.objects.create(name=application.requested_domain.name) - application.approved_domain = domain - application.save() - - # Create a request object with a superuser - request = self.factory.post("/admin/registrar/domainapplication/{}/change/".format(application.pk)) - request.user = self.superuser - - # Define a custom implementation for is_active - def custom_is_active(self): - return True # Override to return True + return domain_is_active # Override to return True # Use ExitStack to combine patch contexts with ExitStack() as stack: @@ -794,54 +727,52 @@ def custom_is_active(self): stack.enter_context(patch.object(Domain, "is_active", custom_is_active)) stack.enter_context(patch.object(messages, "error")) - # Simulate saving the model - application.status = DomainApplication.ApplicationStatus.INELIGIBLE + application.status = another_state self.admin.save_model(request, application, None, True) # Assert that the error message was called with the correct argument - messages.error.assert_called_once_with( - request, - "This action is not permitted. The domain " + "is already active.", - ) + if domain_is_active: + 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() - def test_side_effects_when_saving_approved_to_ineligible(self): - # Create an instance of the model - application = completed_application(status=DomainApplication.ApplicationStatus.APPROVED) - domain = Domain.objects.create(name=application.requested_domain.name) - domain_information = DomainInformation.objects.create(creator=self.superuser, domain=domain) - application.approved_domain = domain - application.save() + self.assertEqual(application.approved_domain, None) - # Create a request object with a superuser - request = self.factory.post("/admin/registrar/domainapplication/{}/change/".format(application.pk)) - request.user = self.superuser + # Assert that Domain got Deleted + with self.assertRaises(Domain.DoesNotExist): + domain.refresh_from_db() - # Define a custom implementation for is_active - def custom_is_active(self): - return False # Override to return False + # Assert that DomainInformation got Deleted + with self.assertRaises(DomainInformation.DoesNotExist): + domain_information.refresh_from_db() - # Use ExitStack to combine patch contexts - with ExitStack() as stack: - # Patch Domain.is_active and django.contrib.messages.error simultaneously - stack.enter_context(patch.object(Domain, "is_active", custom_is_active)) - stack.enter_context(patch.object(messages, "error")) + def test_error_when_saving_approved_to_in_review_and_domain_is_active(self): + self.trigger_saving_approved_to_another_state(True, DomainApplication.ApplicationStatus.IN_REVIEW) - # Simulate saving the model - application.status = DomainApplication.ApplicationStatus.INELIGIBLE - self.admin.save_model(request, application, None, True) + def test_error_when_saving_approved_to_action_needed_and_domain_is_active(self): + self.trigger_saving_approved_to_another_state(True, DomainApplication.ApplicationStatus.ACTION_NEEDED) + + def test_error_when_saving_approved_to_rejected_and_domain_is_active(self): + self.trigger_saving_approved_to_another_state(True, DomainApplication.ApplicationStatus.REJECTED) - # Assert that the error message was never called - messages.error.assert_not_called() + def test_error_when_saving_approved_to_ineligible_and_domain_is_active(self): + self.trigger_saving_approved_to_another_state(True, DomainApplication.ApplicationStatus.INELIGIBLE) - self.assertEqual(application.approved_domain, None) + def test_side_effects_when_saving_approved_to_in_review(self): + self.trigger_saving_approved_to_another_state(False, DomainApplication.ApplicationStatus.IN_REVIEW) - # Assert that Domain got Deleted - with self.assertRaises(Domain.DoesNotExist): - domain.refresh_from_db() + def test_side_effects_when_saving_approved_to_action_needed(self): + self.trigger_saving_approved_to_another_state(False, DomainApplication.ApplicationStatus.ACTION_NEEDED) - # Assert that DomainInformation got Deleted - with self.assertRaises(DomainInformation.DoesNotExist): - domain_information.refresh_from_db() + def test_side_effects_when_saving_approved_to_rejected(self): + self.trigger_saving_approved_to_another_state(False, DomainApplication.ApplicationStatus.REJECTED) + + def test_side_effects_when_saving_approved_to_ineligible(self): + self.trigger_saving_approved_to_another_state(False, DomainApplication.ApplicationStatus.INELIGIBLE) def test_has_correct_filters(self): """ diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index 294ec70af..d7c8960f6 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -464,6 +464,46 @@ def test_reject_with_prejudice_transition_not_allowed(self): with self.assertRaises(exception_type): application.reject_with_prejudice() + def test_transition_not_allowed_approved_in_review_when_domain_is_active(self): + """Create an application with status approved, create a matching domain that + is active, and call in_review against transition rules""" + + domain = Domain.objects.create(name=self.approved_application.requested_domain.name) + self.approved_application.approved_domain = domain + self.approved_application.save() + + # Define a custom implementation for is_active + def custom_is_active(self): + return True # Override to return True + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): + with less_console_noise(): + # Use patch to temporarily replace is_active with the custom implementation + with patch.object(Domain, "is_active", custom_is_active): + # Now, when you call is_active on Domain, it will return True + with self.assertRaises(TransitionNotAllowed): + self.approved_application.in_review() + + def test_transition_not_allowed_approved_action_needed_when_domain_is_active(self): + """Create an application with status approved, create a matching domain that + is active, and call action_needed against transition rules""" + + domain = Domain.objects.create(name=self.approved_application.requested_domain.name) + self.approved_application.approved_domain = domain + self.approved_application.save() + + # Define a custom implementation for is_active + def custom_is_active(self): + return True # Override to return True + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): + with less_console_noise(): + # Use patch to temporarily replace is_active with the custom implementation + with patch.object(Domain, "is_active", custom_is_active): + # Now, when you call is_active on Domain, it will return True + with self.assertRaises(TransitionNotAllowed): + self.approved_application.action_needed() + def test_transition_not_allowed_approved_rejected_when_domain_is_active(self): """Create an application with status approved, create a matching domain that is active, and call reject against transition rules"""