diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 4ea37e6e8..503b6abbd 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -6,7 +6,6 @@ from django.http.response import HttpResponseRedirect
from django.urls import reverse
from . import models
-
logger = logging.getLogger(__name__)
@@ -162,7 +161,7 @@ class DomainAdmin(ListHeaderAdmin):
elif EDIT_DOMAIN in request.POST:
# We want to know, globally, when an edit action occurs
request.session["analyst_action"] = "edit"
- # Restricts this action to this domain only
+ # Restricts this action to this domain (pk) only
request.session["analyst_action_location"] = obj.id
return HttpResponseRedirect(reverse("domain", args=(obj.id,)))
@@ -179,6 +178,7 @@ class DomainAdmin(ListHeaderAdmin):
extra_context = extra_context or {}
extra_context["domain_id"] = object_id
+
return super().change_view(
request,
object_id,
@@ -186,6 +186,14 @@ class DomainAdmin(ListHeaderAdmin):
extra_context=extra_context,
)
+ def has_change_permission(self, request, obj=None):
+ # Fixes a bug wherein users which are only is_staff can access 'change' when GET,
+ # 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)
+
class ContactAdmin(ListHeaderAdmin):
diff --git a/src/registrar/templates/django/admin/domain_change_form.html b/src/registrar/templates/django/admin/domain_change_form.html
index 5461cc82a..6c5fc1384 100644
--- a/src/registrar/templates/django/admin/domain_change_form.html
+++ b/src/registrar/templates/django/admin/domain_change_form.html
@@ -2,8 +2,6 @@
{% block field_sets %}
- {% if not is_analyst_or_superuser or not analyst_action%}
+ {% if not is_analyst_or_superuser or not analyst_action %}
{% endif %}
{# messages block is under the back breadcrumb link #}
diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py
index 7644ef7cf..e0bf381cd 100644
--- a/src/registrar/views/utility/mixins.py
+++ b/src/registrar/views/utility/mixins.py
@@ -4,9 +4,9 @@ from django.contrib.auth.mixins import PermissionRequiredMixin
from django.http import Http404
from registrar.models import DomainApplication, DomainInvitation
-import logging
-from registrar.models.domain_information import DomainInformation
+from registrar.models import DomainInformation, UserDomainRole
+import logging
logger = logging.getLogger(__name__)
@@ -35,29 +35,36 @@ 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 null for Domain")
-
- requested_domain = None
-
- try:
- requested_domain = DomainInformation.objects.get(id=pk)
-
- # This should never happen in normal flow.
- # That said, it does need to be raised here.
- except DomainInformation.DoesNotExist:
- raise Http404()
+ raise ValueError("Primary key is None")
# Checks if the creator is the user requesting this item
- user_is_creator: bool = (
- requested_domain.creator.username == self.request.user.username
- )
+
+ 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:
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 (i.e: https://getgov-staging.app.cloud.gov/domain/73333),
+ # the page throws a 403 error,
+ # instead of a 404. This fixes that behaviour,
+ # but do we want it to throw a 403 instead?
+ # Basically, should this be logger.debug()?
+ raise Http404()
+
# Analysts may manage domains, when they are in these statuses:
valid_domain_statuses = [
DomainApplication.APPROVED,
diff --git a/src/registrar/views/utility/permission_views.py b/src/registrar/views/utility/permission_views.py
index 6a991d016..d7543d375 100644
--- a/src/registrar/views/utility/permission_views.py
+++ b/src/registrar/views/utility/permission_views.py
@@ -34,23 +34,20 @@ class DomainPermissionView(DomainPermission, DetailView, abc.ABC):
def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)
user = self.request.user
- # Q: is there a more efficent way to do this?
- # Searches by creator_id instead of creator,
- # should be slightly faster than by creator...
- is_original_creator = DomainInformation.objects.filter(
- creator_id=self.request.user.id, id=self.kwargs["pk"]
- ).exists()
- context["is_original_creator"] = is_original_creator
- context["is_analyst_or_superuser"] = user.is_superuser or user.is_staff
+ context["is_analyst_or_superuser"] = user.is_superuser or user.is_staff
# Flag to see if an analyst is attempting to make edits
if "analyst_action" in self.request.session:
context["analyst_action"] = self.request.session["analyst_action"]
+ context["analyst_action_location"] = self.request.session["analyst_action_location"]
return context
def log_analyst_form_actions(self, form_class_name, printable_object_info):
- """Generates a log for when 'analyst_action' exists on the session"""
+ """Generates a log for when key 'analyst_action' exists on the session.
+ Follows this format: f"{user_type} {self.request.user}
+ edited {form_class_name} in {printable_object_info}"
+ """
if "analyst_action" in self.request.session:
action = self.request.session["analyst_action"]
@@ -72,8 +69,6 @@ class DomainPermissionView(DomainPermission, DetailView, abc.ABC):
logger.info(
f"{user_type} {self.request.user} edited {form_class_name} in {printable_object_info}" # noqa
)
- else:
- logger.debug("'analyst_action' does not exist on the session")
# Abstract property enforces NotImplementedError on an attribute.
@property