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

@@ -178,39 +178,45 @@ class DomainAdmin(ListHeaderAdmin):
         PLACE_HOLD = "_place_client_hold"
         REMOVE_HOLD = "_remove_client_hold"
         if PLACE_HOLD in request.POST:
-            try:
+            return self.do_place_client_hold(request, obj)
-                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(".")
         elif REMOVE_HOLD in request.POST:
-            try:
+            return self.do_remove_client_hold(request, obj)
-                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 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,

src/registrar/tests/common.py

@@ -434,6 +434,7 @@ def create_user():
     )
     staffuser.is_staff = True
     staffuser.save()
+    return staffuser
 
 
 def create_ready_domain():

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):