mirror of
https://github.com/cisagov/manage.get.gov.git
synced 2025-05-16 17:47:02 +02:00
Small Refactoring
This commit is contained in:
parent
6538471091
commit
274e31cc37
5 changed files with 40 additions and 33 deletions
|
@ -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):
|
||||
|
||||
|
|
|
@ -2,8 +2,6 @@
|
|||
|
||||
{% block field_sets %}
|
||||
<div class="submit-row">
|
||||
|
||||
<input type="hidden" value="edit" name="analyst_action">
|
||||
<input type="submit" value="Edit Domain" name="_edit_domain">
|
||||
<input type="submit" value="Place hold" name="_place_client_hold">
|
||||
</div>
|
||||
|
|
|
@ -19,7 +19,7 @@
|
|||
|
||||
<div class="tablet:grid-col-9">
|
||||
<main id="main-content" class="grid-container">
|
||||
{% if not is_analyst_or_superuser or not analyst_action%}
|
||||
{% if not is_analyst_or_superuser or not analyst_action %}
|
||||
<a href="{% url 'home' %}" class="breadcrumb__back">
|
||||
<svg class="usa-icon" aria-hidden="true" focusable="false" role="img">
|
||||
<use xlink:href="{% static 'img/sprite.svg' %}#arrow_back"></use>
|
||||
|
@ -39,7 +39,6 @@
|
|||
<p class="margin-left-05 margin-top-0 margin-bottom-0 line-height-sans-1">
|
||||
Back to manage your domains
|
||||
</p>
|
||||
|
||||
</a>
|
||||
{% endif %}
|
||||
{# messages block is under the back breadcrumb link #}
|
||||
|
|
|
@ -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,
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue