Code cleanup

Had some older code lying around

src/registrar/admin.py

@@ -171,24 +171,14 @@ class DomainAdmin(ListHeaderAdmin):
             return HttpResponseRedirect(reverse("domain", args=(obj.id,)))
         return super().response_change(request, obj)
 
-    # Sets domain_id as a context var
-    def change_view(self, request, object_id, form_url="", extra_context=None):
+    def change_view(self, request, object_id):
+        # If the analyst was recently editing
         if "analyst_action" in request.session:
-            # If an analyst performed an edit action,
             # delete the session variable
             del request.session["analyst_action"]
             # delete the associated location
             del request.session["analyst_action_location"]
-
-        extra_context = extra_context or {}
-        extra_context["domain_id"] = object_id
-
-        return super().change_view(
-            request,
-            object_id,
-            form_url,
-            extra_context=extra_context,
-        )
+        return super().change_view(request, object_id)
 
     def has_change_permission(self, request, obj=None):
         # Fixes a bug wherein users which are only is_staff
@@ -196,7 +186,6 @@ class DomainAdmin(ListHeaderAdmin):
         # but cannot access this page when it is a request of type POST.
         if request.user.is_staff:
             return True
-
         return super().has_change_permission(request, obj)
 
 

src/registrar/tests/test_admin.py

@@ -14,10 +14,9 @@ from registrar.models import (
     DomainInformation,
     User,
     DomainInvitation,
+    Domain
 )
-from registrar.models.domain import Domain
 from .common import (
-    AuditedAdminMockData,
     completed_application,
     generic_domain_object,
     mock_user,
@@ -25,11 +24,8 @@ from .common import (
     create_user,
     multiple_unalphabetical_domain_objects,
 )
-
 from django.contrib.sessions.backends.db import SessionStore
-
 from django.contrib.auth import get_user_model
-
 from django.conf import settings
 from unittest.mock import MagicMock
 import boto3_mocking  # type: ignore
@@ -651,7 +647,6 @@ class DomainSessionVariableTest(TestCase):
         self.factory = RequestFactory()
         self.admin = DomainAdmin(Domain, None)
         self.client = Client(HTTP_HOST="localhost:8080")
-        self.superuser = create_superuser()
 
     def test_session_variables_set_correctly(self):
         """Checks if session variables are being set correctly"""
@@ -660,12 +655,7 @@ class DomainSessionVariableTest(TestCase):
         self.client.login(username="superuser", password=p)
 
         dummy_domain_information: DomainInformation = generic_domain_object("information", "session")
-        request = self.factory.post(
-            reverse('admin:registrar_domain_change', args=(dummy_domain_information.domain.pk,)),
-            {'_edit_domain': 'true'},
-            follow=True
-        )
-
+        request = self.get_factory_post_edit_domain(dummy_domain_information.domain.pk)
         self.populate_session_values(request, dummy_domain_information.domain)
 
         self.assertEqual(request.session['analyst_action'], 'edit')
@@ -676,19 +666,13 @@ class DomainSessionVariableTest(TestCase):
         p = "adminpass"
         self.client.login(username="superuser", password=p)
 
-        # We need to create multiple of these to ensure data is consistent across all of them
-        dummy_domain_information_list: [DomainInformation] = multiple_unalphabetical_domain_objects("invitation")
+        dummy_domain_information_list: [DomainInformation] = multiple_unalphabetical_domain_objects("information")
         for item in dummy_domain_information_list:
-            request = self.factory.post(
-                reverse('admin:registrar_domain_change', args=(item.pk,)),
-                {'_edit_domain': 'true'},
-                follow=True
-            )
-
+            request = self.get_factory_post_edit_domain(item.domain.pk)
             self.populate_session_values(request, item)
 
             self.assertEqual(request.session['analyst_action'], 'edit')
-            self.assertEqual(request.session['analyst_action_location'], item.pk)
+            self.assertEqual(request.session['analyst_action_location'], item.domain.pk)
 
     def test_session_variables_concurrent_requests(self):
         """ Simulates two requests at once """
@@ -730,4 +714,14 @@ class DomainSessionVariableTest(TestCase):
         request.user = self.client
         request.session = SessionStore()
         request.session.create()
-        self.admin.response_change(request, domain_object)
+        self.admin.response_change(request, domain_object)
+
+    def get_factory_post_edit_domain(self, primary_key):
+        """Posts to registrar domain change
+        with the edit domain button 'clicked',
+        then returns the factory object"""
+        return self.factory.post(
+            reverse('admin:registrar_domain_change', args=(primary_key,)),
+            {'_edit_domain': 'true'},
+            follow=True
+        )

src/registrar/views/utility/mixins.py

@@ -2,9 +2,7 @@
 
 from django.contrib.auth.mixins import PermissionRequiredMixin
 
-from registrar.models import DomainApplication, DomainInvitation
-
-from registrar.models import DomainInformation, UserDomainRole
+from registrar.models import DomainApplication, DomainInvitation, DomainInformation, UserDomainRole
 import logging
 
 logger = logging.getLogger(__name__)
@@ -34,27 +32,20 @@ class DomainPermission(PermissionsLoginMixin):
             return False
 
         pk = self.kwargs["pk"]
-
         # If pk is none then something went very wrong...
         if pk is None:
             raise ValueError("Primary key is None")
 
-        # Checks if the creator is the user requesting this item
-
-        user_is_creator: bool = UserDomainRole.objects.filter(
-            user=self.request.user, domain__id=pk
-        ).exists()
-
         # user needs to have a role on the domain
-        if user_is_creator:
+        if UserDomainRole.objects.filter(
+            user=self.request.user, domain__id=pk
+        ).exists():
             return True
 
         # ticket 806
         requested_domain: DomainInformation = None
-
         try:
             requested_domain = DomainInformation.objects.get(id=pk)
-
         except DomainInformation.DoesNotExist:
             # Q: While testing, I saw that, application-wide, if you go to a domain
             # that does not exist, for example,
@@ -62,7 +53,7 @@ class DomainPermission(PermissionsLoginMixin):
             # the page throws a 403 error, instead of a 404.
             # Do we want it to throw a 404 instead?
             # Basically, should this be Http404()?
-            logger.warning(f"Domain with PK {pk} does not exist")
+            logger.debug(f"Domain with PK {pk} does not exist")
             return False
 
         # Analysts may manage domains, when they are in these statuses:
@@ -79,7 +70,6 @@ class DomainPermission(PermissionsLoginMixin):
         )
 
         session = self.request.session
-
         # Check if the user is attempting a valid edit action.
         # If analyst_action is present, analyst_action_location will be present.
         # if it isn't, then it either suggests tampering