zydronium/manage.get.gov
1
0
Fork 0
mirror of https://github.com/cisagov/manage.get.gov.git synced 2025-05-15 17:17:02 +02:00
Code Issues Projects Releases Packages Wiki Activity

Fix the approve() sets status to None bug by calling the transition methods on the current obj (as opposed to the old obj) in django admin

Browse source
This commit is contained in:
rachidatecs 2023-08-14 12:39:34 -04:00
parent a2277ab7b7
commit f7c9c1c42c
No known key found for this signature in database
GPG key ID: 3CEBBFA7325E5525
3 changed files with 65 additions and 67 deletions
Download patch file Download diff file Expand all files Collapse all files

27
src/registrar/admin.py
View file

@ -274,22 +274,27 @@ class DomainApplicationAdmin(ListHeaderAdmin):
pass
elif obj.status == models.DomainApplication.SUBMITTED:
# This is an fsm in model which will throw an error if the
# transition condition is violated, so we call it on the
# original object which has the right status value, and pass
# the updated object which contains the up-to-date data
# for the side effects (like an email send). Same
# comment applies to original_obj method calls below.
original_obj.submit(updated_domain_application=obj)
# transition condition is violated, so we roll back the
# status to what it was before the admn user changed it and
# let the fsm method set it. Same comment applies to
# transition method calls below.
obj.status = original_obj.status
obj.submit()
elif obj.status == models.DomainApplication.IN_REVIEW:
original_obj.in_review(updated_domain_application=obj)
obj.status = original_obj.status
obj.in_review()
elif obj.status == models.DomainApplication.ACTION_NEEDED:
original_obj.action_needed(updated_domain_application=obj)
obj.status = original_obj.status
obj.action_needed()
elif obj.status == models.DomainApplication.APPROVED:
original_obj.approve(updated_domain_application=obj)
obj.status = original_obj.status
obj.approve()
elif obj.status == models.DomainApplication.WITHDRAWN:
original_obj.withdraw()
obj.status = original_obj.status
obj.withdraw()
elif obj.status == models.DomainApplication.REJECTED:
original_obj.reject(updated_domain_application=obj)
obj.status = original_obj.status
obj.reject()
else:
logger.warning("Unknown status selected in django admin")

58
src/registrar/models/domain_application.py
View file

@ -504,15 +504,10 @@ class DomainApplication(TimeStampedModel):
@transition(
field="status", source=[STARTED, ACTION_NEEDED, WITHDRAWN], target=SUBMITTED
)
def submit(self, updated_domain_application=None):
def submit(self):
"""Submit an application that is started.
As a side effect, an email notification is sent.
This method is called in admin.py on the original application
which has the correct status value, but is passed the changed
application which has the up-to-date data that we'll use
in the email."""
As a side effect, an email notification is sent."""
# check our conditions here inside the `submit` method so that we
# can raise more informative exceptions
@ -528,16 +523,6 @@ class DomainApplication(TimeStampedModel):
if not DraftDomain.string_could_be_domain(self.requested_domain.name):
raise ValueError("Requested domain is not a valid domain name.")
if updated_domain_application is not None:
# A DomainApplication is being passed to this method (ie from admin)
updated_domain_application._send_status_update_email(
"submission confirmation",
"emails/submission_confirmation.txt",
"emails/submission_confirmation_subject.txt",
)
else:
# Or this method is called with the right application
# for context, ie from views/application.py
self._send_status_update_email(
"submission confirmation",
"emails/submission_confirmation.txt",
@ -545,29 +530,24 @@ class DomainApplication(TimeStampedModel):
)
@transition(field="status", source=SUBMITTED, target=IN_REVIEW)
def in_review(self, updated_domain_application):
def in_review(self):
"""Investigate an application that has been submitted.
As a side effect, an email notification is sent.
As a side effect, an email notification is sent."""
This method is called in admin.py on the original application
which has the correct status value, but is passed the changed
application which has the up-to-date data that we'll use
in the email."""
updated_domain_application._send_status_update_email(
self._send_status_update_email(
"application in review",
"emails/status_change_in_review.txt",
"emails/status_change_in_review_subject.txt",
)
@transition(field="status", source=[IN_REVIEW, REJECTED], target=ACTION_NEEDED)
def action_needed(self, updated_domain_application):
def action_needed(self):
"""Send back an application that is under investigation or rejected.
As a side effect, an email notification is sent, similar to in_review"""
As a side effect, an email notification is sent."""
updated_domain_application._send_status_update_email(
self._send_status_update_email(
"action needed",
"emails/status_change_action_needed.txt",
"emails/status_change_action_needed_subject.txt",
@ -576,19 +556,13 @@ class DomainApplication(TimeStampedModel):
@transition(
field="status", source=[SUBMITTED, IN_REVIEW, REJECTED], target=APPROVED
)
def approve(self, updated_domain_application=None):
def approve(self):
"""Approve an application that has been submitted.
This has substantial side-effects because it creates another database
object for the approved Domain and makes the user who created the
application into an admin on that domain. It also triggers an email
notification.
This method is called in admin.py on the original application
which has the correct status value, but is passed the changed
application which has the up-to-date data that we'll use
in the email.
"""
notification."""
# create the domain
Domain = apps.get_model("registrar.Domain")
@ -607,14 +581,6 @@ class DomainApplication(TimeStampedModel):
user=self.creator, domain=created_domain, role=UserDomainRole.Roles.ADMIN
)
if updated_domain_application is not None:
# A DomainApplication is being passed to this method (ie from admin)
updated_domain_application._send_status_update_email(
"application approved",
"emails/status_change_approved.txt",
"emails/status_change_approved_subject.txt",
)
else:
self._send_status_update_email(
"application approved",
"emails/status_change_approved.txt",
@ -626,12 +592,12 @@ class DomainApplication(TimeStampedModel):
"""Withdraw an application that has been submitted."""
@transition(field="status", source=[IN_REVIEW, APPROVED], target=REJECTED)
def reject(self, updated_domain_application):
def reject(self):
"""Reject an application that has been submitted.
As a side effect, an email notification is sent, similar to in_review"""
updated_domain_application._send_status_update_email(
self._send_status_update_email(
"action needed",
"emails/status_change_rejected.txt",
"emails/status_change_rejected_subject.txt",

27
src/registrar/tests/test_admin.py
View file

@ -153,6 +153,33 @@ class TestDomainApplicationAdmin(TestCase):
# Perform assertions on the mock call itself
mock_client_instance.send_email.assert_called_once()
def test_save_model_sets_approved_domain(self):
# make sure there is no user with this email
EMAIL = "mayor@igorville.gov"
User.objects.filter(email=EMAIL).delete()
# Create a sample application
application = completed_application(status=DomainApplication.IN_REVIEW)
# Create a mock request
request = self.factory.post(
"/admin/registrar/domainapplication/{}/change/".format(application.pk)
)
# Create an instance of the model admin
model_admin = DomainApplicationAdmin(DomainApplication, self.site)
# Modify the application's property
application.status = DomainApplication.APPROVED
# Use the model admin's save_model method
model_admin.save_model(request, application, form=None, change=True)
# Test that approved domain exists and equals requested domain
self.assertEqual(
application.requested_domain.name, application.approved_domain.name
)
@boto3_mocking.patching
def test_save_model_sends_action_needed_email(self):
# make sure there is no user with this email