diff --git a/src/registrar/models/domain_application.py b/src/registrar/models/domain_application.py index da801ce3d..f96ec1040 100644 --- a/src/registrar/models/domain_application.py +++ b/src/registrar/models/domain_application.py @@ -1,6 +1,4 @@ from __future__ import annotations -from json import JSONDecodeError -import json from typing import Union import logging @@ -14,7 +12,6 @@ from registrar.models.domain import Domain from .utility.time_stamped_model import TimeStampedModel from ..utility.email import send_templated_email, EmailSendingError from itertools import chain -from auditlog.models import LogEntry # type: ignore logger = logging.getLogger(__name__) @@ -568,26 +565,6 @@ class DomainApplication(TimeStampedModel): except Exception: return "" - def has_previously_had_a_status_of(self, status): - """Return True if this request has previously had the status of {passed param}.""" - - log_entries = LogEntry.objects.get_for_object(self) - - for entry in log_entries: - try: - changes_dict = json.loads(entry.changes) - # changes_dict will look like {'status': ['withdrawn', 'submitted']}, - # henceforth the len(changes_dict.get('status', [])) == 2 - status_change = changes_dict.get("status", []) - if len(status_change) == 2 and status_change[1] == status: - return True - except JSONDecodeError: - logger.warning( - "JSON decode error while parsing logs for domain requests in has_previously_had_a_status_of" - ) - - return False - def domain_is_not_active(self): if self.approved_domain: return not self.approved_domain.is_active() @@ -656,8 +633,8 @@ class DomainApplication(TimeStampedModel): self.submission_date = timezone.now().date() self.save() - # Limit email notifications for this transition to the first time the request transitions to this status - if not self.has_previously_had_a_status_of(DomainApplication.ApplicationStatus.SUBMITTED): + # Limit email notifications to transitions from Started and Withdrawn + if self.status == self.ApplicationStatus.STARTED or self.status == self.ApplicationStatus.WITHDRAWN: self._send_status_update_email( "submission confirmation", "emails/submission_confirmation.txt", @@ -738,14 +715,12 @@ class DomainApplication(TimeStampedModel): user=self.creator, domain=created_domain, role=UserDomainRole.Roles.MANAGER ) - # Limit email notifications for this transition to the first time the request transitions to this status - if not self.has_previously_had_a_status_of(DomainApplication.ApplicationStatus.APPROVED): - self._send_status_update_email( - "application approved", - "emails/status_change_approved.txt", - "emails/status_change_approved_subject.txt", - send_email, - ) + self._send_status_update_email( + "application approved", + "emails/status_change_approved.txt", + "emails/status_change_approved_subject.txt", + send_email, + ) @transition( field="status", @@ -755,13 +730,11 @@ class DomainApplication(TimeStampedModel): def withdraw(self): """Withdraw an application that has been submitted.""" - # Limit email notifications for this transition to the first time the request transitions to this status - if not self.has_previously_had_a_status_of(DomainApplication.ApplicationStatus.WITHDRAWN): - self._send_status_update_email( - "withdraw", - "emails/domain_request_withdrawn.txt", - "emails/domain_request_withdrawn_subject.txt", - ) + self._send_status_update_email( + "withdraw", + "emails/domain_request_withdrawn.txt", + "emails/domain_request_withdrawn_subject.txt", + ) @transition( field="status", @@ -787,13 +760,11 @@ class DomainApplication(TimeStampedModel): logger.error(err) logger.error("Can't query an approved domain while attempting a DA reject()") - # Limit email notifications for this transition to the first time the request transitions to this status - if not self.has_previously_had_a_status_of(DomainApplication.ApplicationStatus.REJECTED): - self._send_status_update_email( - "action needed", - "emails/status_change_rejected.txt", - "emails/status_change_rejected_subject.txt", - ) + self._send_status_update_email( + "action needed", + "emails/status_change_rejected.txt", + "emails/status_change_rejected_subject.txt", + ) @transition( field="status", diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 7773cb60b..83f777189 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -456,8 +456,11 @@ class TestDomainApplicationAdmin(MockEppLib): self.assertIn(expected_string, email_body) def test_save_model_sends_submitted_email(self): - """When transitioning to submitted the first time (and the first time only) on a domain request, - an email is sent out.""" + """When transitioning to submitted from started or withdrawn on a domain request, + an email is sent out. + + When transitioning to submitted from dns needed or in review on a domain request, + no email is sent out.""" # Ensure there is no user with this email EMAIL = "mayor@igorville.gov" @@ -466,7 +469,7 @@ class TestDomainApplicationAdmin(MockEppLib): # Create a sample application application = completed_application() - # Test Submitted Status + # Test Submitted Status from started self.transition_state_and_send_email(application, DomainApplication.ApplicationStatus.SUBMITTED) self.assert_email_is_accurate("We received your .gov domain request.", 0, EMAIL) self.assertEqual(len(self.mock_client.EMAILS_SENT), 1) @@ -478,13 +481,33 @@ class TestDomainApplicationAdmin(MockEppLib): ) self.assertEqual(len(self.mock_client.EMAILS_SENT), 2) - # Test Submitted Status Again (No new email should be sent) + # Test Submitted Status Again (from withdrawn) self.transition_state_and_send_email(application, DomainApplication.ApplicationStatus.SUBMITTED) - self.assertEqual(len(self.mock_client.EMAILS_SENT), 2) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 3) + + # Move it to IN_REVIEW + self.transition_state_and_send_email(application, DomainApplication.ApplicationStatus.IN_REVIEW) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 3) + + # Test Submitted Status Again from in IN_REVIEW, no new email should be sent + self.transition_state_and_send_email(application, DomainApplication.ApplicationStatus.SUBMITTED) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 3) + + # Move it to IN_REVIEW + self.transition_state_and_send_email(application, DomainApplication.ApplicationStatus.IN_REVIEW) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 3) + + # Move it to ACTION_NEEDED + self.transition_state_and_send_email(application, DomainApplication.ApplicationStatus.ACTION_NEEDED) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 3) + + # Test Submitted Status Again from in ACTION_NEEDED, no new email should be sent + self.transition_state_and_send_email(application, DomainApplication.ApplicationStatus.SUBMITTED) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 3) def test_save_model_sends_approved_email(self): - """When transitioning to approved the first time (and the first time only) on a domain request, - an email is sent out.""" + """When transitioning to approved on a domain request, + an email is sent out every time.""" # Ensure there is no user with this email EMAIL = "mayor@igorville.gov" @@ -505,11 +528,11 @@ class TestDomainApplicationAdmin(MockEppLib): # Test Submitted Status Again (No new email should be sent) self.transition_state_and_send_email(application, DomainApplication.ApplicationStatus.APPROVED) - self.assertEqual(len(self.mock_client.EMAILS_SENT), 2) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 3) def test_save_model_sends_rejected_email(self): - """When transitioning to rejected the first time (and the first time only) on a domain request, - an email is sent out.""" + """When transitioning to rejected on a domain request, + an email is sent out every time.""" # Ensure there is no user with this email EMAIL = "mayor@igorville.gov" @@ -530,11 +553,11 @@ class TestDomainApplicationAdmin(MockEppLib): # Test Submitted Status Again (No new email should be sent) self.transition_state_and_send_email(application, DomainApplication.ApplicationStatus.REJECTED) - self.assertEqual(len(self.mock_client.EMAILS_SENT), 2) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 3) def test_save_model_sends_withdrawn_email(self): - """When transitioning to withdrawn the first time (and the first time only) on a domain request, - an email is sent out.""" + """When transitioning to withdrawn on a domain request, + an email is sent out every time.""" # Ensure there is no user with this email EMAIL = "mayor@igorville.gov" @@ -557,7 +580,7 @@ class TestDomainApplicationAdmin(MockEppLib): # Test Submitted Status Again (No new email should be sent) self.transition_state_and_send_email(application, DomainApplication.ApplicationStatus.WITHDRAWN) - self.assertEqual(len(self.mock_client.EMAILS_SENT), 2) + self.assertEqual(len(self.mock_client.EMAILS_SENT), 3) def test_save_model_sets_approved_domain(self): # make sure there is no user with this email diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index a1b22373d..66c8d04f8 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1,6 +1,6 @@ from django.test import TestCase from django.db.utils import IntegrityError -from unittest.mock import MagicMock, patch +from unittest.mock import patch from registrar.models import ( Contact, @@ -154,30 +154,7 @@ class TestDomainApplication(TestCase): application.submit() self.assertEqual(application.status, application.ApplicationStatus.SUBMITTED) - @patch("auditlog.models.LogEntry.objects.get_for_object") - def test_has_previously_had_a_status_of_returns_true(self, mock_get_for_object): - """Set up mock LogEntry.objects.get_for_object to return a log entry with the desired status""" - - log_entry_with_status = MagicMock(changes='{"status": ["previous_status", "desired_status"]}') - mock_get_for_object.return_value = [log_entry_with_status] - - result = self.started_application.has_previously_had_a_status_of("desired_status") - - self.assertTrue(result) - - @patch("auditlog.models.LogEntry.objects.get_for_object") - def test_has_previously_had_a_status_of_returns_false(self, mock_get_for_object): - """Set up mock LogEntry.objects.get_for_object to return a log entry - with a different status than the desired status""" - - log_entry_with_status = MagicMock(changes='{"status": ["previous_status", "different_status"]}') - mock_get_for_object.return_value = [log_entry_with_status] - - result = self.started_application.has_previously_had_a_status_of("desired_status") - - self.assertFalse(result) - - def test_submit_sends_email(self): + def test_submit_from_started_sends_email(self): """Create an application and submit it and see if email was sent.""" # submitter's email is mayor@igorville.gov @@ -199,17 +176,55 @@ class TestDomainApplication(TestCase): 0, ) - @patch("auditlog.models.LogEntry.objects.get_for_object") - def test_submit_does_not_send_email_if_submitted_previously(self, mock_get_for_object): - """Create an application, make it so it was submitted previously, submit it, - and see that an email was not sent.""" + def test_submit_from_withdrawn_sends_email(self): + """Create a withdrawn application and submit it and see if email was sent.""" # submitter's email is mayor@igorville.gov - application = completed_application() + application = completed_application(status=DomainApplication.ApplicationStatus.WITHDRAWN) - # Mock the logs - log_entry_with_status = MagicMock(changes='{"status": ["started", "submitted"]}') - mock_get_for_object.return_value = [log_entry_with_status] + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): + with less_console_noise(): + application.submit() + + # check to see if an email was sent + self.assertGreater( + len( + [ + email + for email in MockSESClient.EMAILS_SENT + if "mayor@igorville.gov" in email["kwargs"]["Destination"]["ToAddresses"] + ] + ), + 0, + ) + + def test_submit_from_action_needed_does_not_send_email(self): + """Create a withdrawn application and submit it and see if email was sent.""" + + # submitter's email is mayor@igorville.gov + application = completed_application(status=DomainApplication.ApplicationStatus.ACTION_NEEDED) + + with boto3_mocking.clients.handler_for("sesv2", self.mock_client): + with less_console_noise(): + application.submit() + + # check to see if an email was sent + self.assertEqual( + len( + [ + email + for email in MockSESClient.EMAILS_SENT + if "mayor@igorville.gov" in email["kwargs"]["Destination"]["ToAddresses"] + ] + ), + 0, + ) + + def test_submit_from_in_review_does_not_send_email(self): + """Create a withdrawn application and submit it and see if email was sent.""" + + # submitter's email is mayor@igorville.gov + application = completed_application(status=DomainApplication.ApplicationStatus.IN_REVIEW) with boto3_mocking.clients.handler_for("sesv2", self.mock_client): with less_console_noise(): @@ -249,34 +264,6 @@ class TestDomainApplication(TestCase): 0, ) - @patch("auditlog.models.LogEntry.objects.get_for_object") - def test_approve_does_not_send_email_if_approved_previously(self, mock_get_for_object): - """Create an application, make it so it was approved previously, approve it, - and see that an email was not sent.""" - - # submitter's email is mayor@igorville.gov - application = completed_application(status=DomainApplication.ApplicationStatus.IN_REVIEW) - - # Mock the logs - log_entry_with_status = MagicMock(changes='{"status": ["submitted", "approved"]}') - mock_get_for_object.return_value = [log_entry_with_status] - - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - with less_console_noise(): - application.approve() - - # check to see if an email was sent - self.assertEqual( - len( - [ - email - for email in MockSESClient.EMAILS_SENT - if "mayor@igorville.gov" in email["kwargs"]["Destination"]["ToAddresses"] - ] - ), - 0, - ) - def test_withdraw_sends_email(self): """Create an application and withdraw it and see if email was sent.""" @@ -299,34 +286,6 @@ class TestDomainApplication(TestCase): 0, ) - @patch("auditlog.models.LogEntry.objects.get_for_object") - def test_withdraw_does_not_send_email_if_withdrawn_previously(self, mock_get_for_object): - """Create an application, make it so it was withdrawn previously, withdraw it, - and see that an email was not sent.""" - - # submitter's email is mayor@igorville.gov - application = completed_application(status=DomainApplication.ApplicationStatus.IN_REVIEW) - - # Mock the logs - log_entry_with_status = MagicMock(changes='{"status": ["submitted", "withdrawn"]}') - mock_get_for_object.return_value = [log_entry_with_status] - - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - with less_console_noise(): - application.withdraw() - - # check to see if an email was sent - self.assertEqual( - len( - [ - email - for email in MockSESClient.EMAILS_SENT - if "mayor@igorville.gov" in email["kwargs"]["Destination"]["ToAddresses"] - ] - ), - 0, - ) - def test_reject_sends_email(self): """Create an application and reject it and see if email was sent.""" @@ -349,34 +308,6 @@ class TestDomainApplication(TestCase): 0, ) - @patch("auditlog.models.LogEntry.objects.get_for_object") - def test_reject_does_not_send_email_if_rejected_previously(self, mock_get_for_object): - """Create an application, make it so it was rejected previously, reject it, - and see that an email was not sent.""" - - # submitter's email is mayor@igorville.gov - application = completed_application(status=DomainApplication.ApplicationStatus.APPROVED) - - # Mock the logs - log_entry_with_status = MagicMock(changes='{"status": ["submitted", "rejected"]}') - mock_get_for_object.return_value = [log_entry_with_status] - - with boto3_mocking.clients.handler_for("sesv2", self.mock_client): - with less_console_noise(): - application.reject() - - # check to see if an email was sent - self.assertEqual( - len( - [ - email - for email in MockSESClient.EMAILS_SENT - if "mayor@igorville.gov" in email["kwargs"]["Destination"]["ToAddresses"] - ] - ), - 0, - ) - def test_submit_transition_allowed(self): """ Test that calling submit from allowable statuses does raises TransitionNotAllowed.