From 888ddda9e982e46e1b23eaa1783f4584bff904bb Mon Sep 17 00:00:00 2001
From: David Kennedy
Date: Fri, 1 Sep 2023 17:40:54 -0400
Subject: [PATCH 01/18] implemented add client hold and remove client hold;
extended change permission on domain admin to staff; conditionally display
buttons based on state; added states to domain to match state diagram
---
src/registrar/admin.py | 31 ++++++++++++++++---
src/registrar/models/domain.py | 20 ++++++++++--
.../django/admin/domain_change_form.html | 6 +++-
3 files changed, 49 insertions(+), 8 deletions(-)
diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 4696a15bf..f4b395524 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -164,7 +164,6 @@ class MyHostAdmin(AuditedAdmin):
inlines = [HostIPInline]
-
class DomainAdmin(ListHeaderAdmin):
"""Custom domain admin class to add extra buttons."""
@@ -175,10 +174,12 @@ class DomainAdmin(ListHeaderAdmin):
readonly_fields = ["state"]
def response_change(self, request, obj):
- ACTION_BUTTON = "_place_client_hold"
- if ACTION_BUTTON in request.POST:
+ 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:
@@ -191,9 +192,31 @@ class DomainAdmin(ListHeaderAdmin):
% obj.name,
)
return HttpResponseRedirect(".")
-
+ 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 super().response_change(request, obj)
+ 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):
"""Custom contact admin class to add search."""
diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py
index a7e46f888..2192bd370 100644
--- a/src/registrar/models/domain.py
+++ b/src/registrar/models/domain.py
@@ -2,7 +2,7 @@ import logging
from datetime import date
from string import digits
-from django_fsm import FSMField # type: ignore
+from django_fsm import FSMField, transition # type: ignore
from django.db import models
@@ -114,6 +114,12 @@ class Domain(TimeStampedModel, DomainHelper):
# the state is indeterminate
UNKNOWN = "unknown"
+ # the ready state for a domain object
+ READY = "ready"
+
+ # when a domain is on hold
+ ONHOLD="onhold"
+
class Cache(property):
"""
Python descriptor to turn class methods into properties.
@@ -311,13 +317,21 @@ class Domain(TimeStampedModel, DomainHelper):
"""Time to renew. Not implemented."""
raise NotImplementedError()
+ @transition(
+ field="state", source=[State.READY], target=State.ONHOLD
+ )
def place_client_hold(self):
"""This domain should not be active."""
- raise NotImplementedError("This is not implemented yet.")
+ # This method is changing the state of the domain in registrar
+ # TODO: implement EPP call
+ @transition(
+ field="state", source=[State.ONHOLD], target=State.READY
+ )
def remove_client_hold(self):
"""This domain is okay to be active."""
- raise NotImplementedError()
+ # This method is changing the state of the domain in registrar
+ # TODO: implement EPP call
def __str__(self) -> str:
return self.name
diff --git a/src/registrar/templates/django/admin/domain_change_form.html b/src/registrar/templates/django/admin/domain_change_form.html
index 5fa89f20a..933d7ae25 100644
--- a/src/registrar/templates/django/admin/domain_change_form.html
+++ b/src/registrar/templates/django/admin/domain_change_form.html
@@ -2,7 +2,11 @@
{% block field_sets %}
{{ block.super }}
{% endblock %}
\ No newline at end of file
From c52ccf8a9acba3b3b35566c9e1cf2f02a764ae31 Mon Sep 17 00:00:00 2001
From: David Kennedy
Date: Fri, 1 Sep 2023 17:48:30 -0400
Subject: [PATCH 02/18] fixed formatting for lint
---
src/registrar/admin.py | 2 ++
src/registrar/models/domain.py | 10 +++-------
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index f4b395524..5c621ee56 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -164,6 +164,7 @@ class MyHostAdmin(AuditedAdmin):
inlines = [HostIPInline]
+
class DomainAdmin(ListHeaderAdmin):
"""Custom domain admin class to add extra buttons."""
@@ -218,6 +219,7 @@ class DomainAdmin(ListHeaderAdmin):
return True
return super().has_change_permission(request, obj)
+
class ContactAdmin(ListHeaderAdmin):
"""Custom contact admin class to add search."""
diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py
index 2192bd370..9eb54b3d1 100644
--- a/src/registrar/models/domain.py
+++ b/src/registrar/models/domain.py
@@ -118,7 +118,7 @@ class Domain(TimeStampedModel, DomainHelper):
READY = "ready"
# when a domain is on hold
- ONHOLD="onhold"
+ ONHOLD = "onhold"
class Cache(property):
"""
@@ -317,17 +317,13 @@ class Domain(TimeStampedModel, DomainHelper):
"""Time to renew. Not implemented."""
raise NotImplementedError()
- @transition(
- field="state", source=[State.READY], target=State.ONHOLD
- )
+ @transition(field="state", source=[State.READY], target=State.ONHOLD)
def place_client_hold(self):
"""This domain should not be active."""
# This method is changing the state of the domain in registrar
# TODO: implement EPP call
- @transition(
- field="state", source=[State.ONHOLD], target=State.READY
- )
+ @transition(field="state", source=[State.ONHOLD], target=State.READY)
def remove_client_hold(self):
"""This domain is okay to be active."""
# This method is changing the state of the domain in registrar
From 744c6fa48a9aa54d0850a8e547d4e92131e03c9c Mon Sep 17 00:00:00 2001
From: David Kennedy
Date: Wed, 6 Sep 2023 17:06:55 -0400
Subject: [PATCH 03/18] added test for DomainAdmin to
test_place_and_remove_hold; added mock data for mock domain
---
src/registrar/tests/common.py | 9 +++++-
src/registrar/tests/test_admin.py | 49 +++++++++++++++++++++++++++++++
2 files changed, 57 insertions(+), 1 deletion(-)
diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py
index c4a2772b0..fbd359b75 100644
--- a/src/registrar/tests/common.py
+++ b/src/registrar/tests/common.py
@@ -427,11 +427,18 @@ def create_superuser():
def create_user():
User = get_user_model()
p = "userpass"
- return User.objects.create_user(
+ staffuser = User.objects.create_user(
username="staffuser",
email="user@example.com",
password=p,
)
+ staffuser.is_staff = True
+ staffuser.save()
+
+
+def create_ready_domain():
+ domain, _ = Domain.objects.get_or_create(name="city.gov", state=Domain.State.READY)
+ return domain
def completed_application(
diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index fc5478dd9..ca5a7b93c 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -2,12 +2,14 @@ from django.test import TestCase, RequestFactory, Client
from django.contrib.admin.sites import AdminSite
from registrar.admin import (
+ DomainAdmin,
DomainApplicationAdmin,
ListHeaderAdmin,
MyUserAdmin,
AuditedAdmin,
)
from registrar.models import (
+ Domain,
DomainApplication,
DomainInformation,
User,
@@ -18,6 +20,7 @@ from .common import (
mock_user,
create_superuser,
create_user,
+ create_ready_domain,
multiple_unalphabetical_domain_objects,
)
@@ -32,6 +35,52 @@ import logging
logger = logging.getLogger(__name__)
+class TestDomainAdmin(TestCase):
+ def setUp(self):
+ self.site = AdminSite()
+ self.factory = RequestFactory()
+ self.admin = DomainAdmin(model=Domain, admin_site=self.site)
+ self.client = Client(HTTP_HOST="localhost:8080")
+ self.staffuser = create_user()
+
+ def test_place_and_remove_hold(self):
+ domain = create_ready_domain()
+
+ # get admin page and assert Place Hold button
+ p = "userpass"
+ self.client.login(username="staffuser", password=p)
+ response = self.client.get(
+ "/admin/registrar/domain/{}/change/".format(domain.pk),
+ follow=True,
+ )
+ self.assertEqual(response.status_code, 200)
+ self.assertContains(response, domain.name)
+ self.assertContains(response, "Place hold")
+ self.assertNotContains(response, "Remove hold")
+
+ # submit place_client_hold and assert Remove Hold button
+ response = self.client.post(
+ "/admin/registrar/domain/{}/change/".format(domain.pk),
+ {"_place_client_hold": "Place hold", "name": domain.name},
+ follow=True,
+ )
+ self.assertEqual(response.status_code, 200)
+ self.assertContains(response, domain.name)
+ self.assertContains(response, "Remove hold")
+ self.assertNotContains(response, "Place hold")
+
+ # submit remove client hold and assert Place hold button
+ response = self.client.post(
+ "/admin/registrar/domain/{}/change/".format(domain.pk),
+ {"_remove_client_hold": "Remove hold", "name": domain.name},
+ follow=True,
+ )
+ self.assertEqual(response.status_code, 200)
+ self.assertContains(response, domain.name)
+ self.assertContains(response, "Place hold")
+ self.assertNotContains(response, "Remove hold")
+
+
class TestDomainApplicationAdmin(TestCase):
def setUp(self):
self.site = AdminSite()
From 5c41e4a28fffde503adfe181d4d71f1c74dbd17d Mon Sep 17 00:00:00 2001
From: David Kennedy
Date: Thu, 7 Sep 2023 07:18:38 -0400
Subject: [PATCH 04/18] 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):
From 0c1e8a2dda4a08b400710ab066edc5c4a6e1215b Mon Sep 17 00:00:00 2001
From: David Kennedy
Date: Thu, 7 Sep 2023 07:42:07 -0400
Subject: [PATCH 05/18] fixed a merge issue, and some code reformatting
---
src/registrar/admin.py | 5 +----
src/registrar/tests/test_admin.py | 1 -
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index 46ba254a6..e4619737e 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -212,10 +212,7 @@ class DomainAdmin(ListHeaderAdmin):
else:
self.message_user(
request,
- (
- "%s is ready. This domain is accessible on the public "
- "internet."
- )
+ ("%s is ready. This domain is accessible on the public internet.")
% obj.name,
)
return HttpResponseRedirect(".")
diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py
index e8b07ed5c..e556adcec 100644
--- a/src/registrar/tests/test_admin.py
+++ b/src/registrar/tests/test_admin.py
@@ -15,7 +15,6 @@ from registrar.models import (
DomainInformation,
User,
DomainInvitation,
- Domain,
)
from .common import (
completed_application,
From 627bb38f5a7680cd40a2a8daa5aea249d2242ab8 Mon Sep 17 00:00:00 2001
From: David Kennedy
Date: Thu, 7 Sep 2023 08:22:49 -0400
Subject: [PATCH 06/18] added dictionary of ACTION_BUTTONS for DomainAdmin to
admin.py
---
src/registrar/admin.py | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/registrar/admin.py b/src/registrar/admin.py
index e4619737e..4c9cd11a5 100644
--- a/src/registrar/admin.py
+++ b/src/registrar/admin.py
@@ -175,14 +175,16 @@ class DomainAdmin(ListHeaderAdmin):
readonly_fields = ["state"]
def response_change(self, request, obj):
- PLACE_HOLD = "_place_client_hold"
- REMOVE_HOLD = "_remove_client_hold"
- EDIT_DOMAIN = "_edit_domain"
- if PLACE_HOLD in request.POST:
+ ACTION_BUTTONS = {
+ "PLACE_HOLD": "_place_client_hold",
+ "REMOVE_HOLD": "_remove_client_hold",
+ "EDIT_DOMAIN": "_edit_domain",
+ }
+ if ACTION_BUTTONS["PLACE_HOLD"] in request.POST:
return self.do_place_client_hold(request, obj)
- elif REMOVE_HOLD in request.POST:
+ elif ACTION_BUTTONS["REMOVE_HOLD"] in request.POST:
return self.do_remove_client_hold(request, obj)
- elif EDIT_DOMAIN in request.POST:
+ elif ACTION_BUTTONS["EDIT_DOMAIN"] in request.POST:
return self.do_edit_domain(request, obj)
return super().response_change(request, obj)
From 3d254c0f90f0c32395a1b7ead4c9ab68d97d9d71 Mon Sep 17 00:00:00 2001
From: Katherine-Osos <119689946+Katherine-Osos@users.noreply.github.com>
Date: Thu, 7 Sep 2023 11:06:44 -0500
Subject: [PATCH 07/18] Update AO description to match updates in request form
---
src/registrar/templates/domain_authorizing_official.html | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/registrar/templates/domain_authorizing_official.html b/src/registrar/templates/domain_authorizing_official.html
index c08dbb237..c12f1f290 100644
--- a/src/registrar/templates/domain_authorizing_official.html
+++ b/src/registrar/templates/domain_authorizing_official.html
@@ -10,8 +10,7 @@
Authorizing official
Your authorizing official is the person within your organization who can
- authorize domain requests. This is generally the highest-ranking or
- highest-elected official in your organization. Read more about who can serve as an authorizing official.
+ authorize domain requests. This person must be in a role of significant, executive responsibility within the organization. Read more about who can serve as an authorizing official.