From 5c41e4a28fffde503adfe181d4d71f1c74dbd17d Mon Sep 17 00:00:00 2001
From: David Kennedy <david.kennedy@associates.cisa.dhs.gov>
Date: Thu, 7 Sep 2023 07:18:38 -0400
Subject: [PATCH] refactored DomainAdmin response_change into multiple methods;
 updated create_user in common.py for properly setting is_staff for test
 users; added tearDown for TestDomainAdmin

---
 src/registrar/admin.py            | 66 +++++++++++++++++--------------
 src/registrar/tests/common.py     |  1 +
 src/registrar/tests/test_admin.py |  5 +++
 3 files changed, 42 insertions(+), 30 deletions(-)

diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 5c621ee56..2f55b1061 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -178,39 +178,45 @@ class DomainAdmin(ListHeaderAdmin):
         PLACE_HOLD = "_place_client_hold"
         REMOVE_HOLD = "_remove_client_hold"
         if PLACE_HOLD in request.POST:
-            try:
-                obj.place_client_hold()
-                obj.save()
-            except Exception as err:
-                self.message_user(request, err, messages.ERROR)
-            else:
-                self.message_user(
-                    request,
-                    (
-                        "%s is in client hold. This domain is no longer accessible on"
-                        " the public internet."
-                    )
-                    % obj.name,
-                )
-            return HttpResponseRedirect(".")
+            return self.do_place_client_hold(request, obj)
         elif REMOVE_HOLD in request.POST:
-            try:
-                obj.remove_client_hold()
-                obj.save()
-            except Exception as err:
-                self.message_user(request, err, messages.ERROR)
-            else:
-                self.message_user(
-                    request,
-                    (
-                        "%s is ready. This domain is accessible on the public "
-                        "internet."
-                    )
-                    % obj.name,
-                )
-            return HttpResponseRedirect(".")
+            return self.do_remove_client_hold(request, obj)
         return super().response_change(request, obj)
 
+    def do_place_client_hold(self, request, obj):
+        try:
+            obj.place_client_hold()
+            obj.save()
+        except Exception as err:
+            self.message_user(request, err, messages.ERROR)
+        else:
+            self.message_user(
+                request,
+                (
+                    "%s is in client hold. This domain is no longer accessible on"
+                    " the public internet."
+                )
+                % obj.name,
+            )
+        return HttpResponseRedirect(".")
+
+    def do_remove_client_hold(self, request, obj):
+        try:
+            obj.remove_client_hold()
+            obj.save()
+        except Exception as err:
+            self.message_user(request, err, messages.ERROR)
+        else:
+            self.message_user(
+                request,
+                (
+                    "%s is ready. This domain is accessible on the public "
+                    "internet."
+                )
+                % obj.name,
+            )
+        return HttpResponseRedirect(".")
+
     def has_change_permission(self, request, obj=None):
         # Fixes a bug wherein users which are only is_staff
         # can access 'change' when GET,
diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py
index fbd359b75..8f8e70ec3 100644
--- a/src/registrar/tests/common.py
+++ b/src/registrar/tests/common.py
@@ -434,6 +434,7 @@ def create_user():
     )
     staffuser.is_staff = True
     staffuser.save()
+    return staffuser
 
 
 def create_ready_domain():
diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index ca5a7b93c..421b02994 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -41,6 +41,7 @@ class TestDomainAdmin(TestCase):
         self.factory = RequestFactory()
         self.admin = DomainAdmin(model=Domain, admin_site=self.site)
         self.client = Client(HTTP_HOST="localhost:8080")
+        self.superuser = create_superuser()
         self.staffuser = create_user()
 
     def test_place_and_remove_hold(self):
@@ -80,6 +81,10 @@ class TestDomainAdmin(TestCase):
         self.assertContains(response, "Place hold")
         self.assertNotContains(response, "Remove hold")
 
+    def tearDown(self):
+        Domain.objects.all().delete()
+        User.objects.all().delete()
+
 
 class TestDomainApplicationAdmin(TestCase):
     def setUp(self):