From 6da9fdab2abb8f93ab6d3d11c4a95dc0dbc98a0f Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 22 Nov 2023 10:36:13 -0500 Subject: [PATCH 01/15] added is_editable to domain; added editable to domain_detail.html; added editable to domain_sidebar.html; added editable to summary_item.html; updated has_permission in domain views to check for domain editable --- src/registrar/models/domain.py | 8 ++++++++ src/registrar/templates/domain_detail.html | 14 +++++++------- src/registrar/templates/domain_sidebar.html | 2 ++ src/registrar/templates/includes/summary_item.html | 2 +- 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 37e78ec6e..93b50bf81 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -880,6 +880,14 @@ class Domain(TimeStampedModel, DomainHelper): """ return self.state == self.State.READY + def is_editable(self) -> bool: + """domain is editable unless state is on hold or deleted""" + return self.state in [ + self.State.UNKNOWN, + self.State.DNS_NEEDED, + self.State.READY, + ] + def transfer(self): """Going somewhere. Not implemented.""" raise NotImplementedError() diff --git a/src/registrar/templates/domain_detail.html b/src/registrar/templates/domain_detail.html index e220fe1aa..2bd6aa0ed 100644 --- a/src/registrar/templates/domain_detail.html +++ b/src/registrar/templates/domain_detail.html @@ -29,7 +29,7 @@ {% url 'domain-dns-nameservers' pk=domain.id as url %} {% if domain.nameservers|length > 0 %} - {% include "includes/summary_item.html" with title='DNS name servers' domains='true' value=domain.nameservers list='true' edit_link=url %} + {% include "includes/summary_item.html" with title='DNS name servers' domains='true' value=domain.nameservers list='true' edit_link=url editable=domain.is_editable %} {% else %}

DNS name servers

No DNS name servers have been added yet. Before your domain can be used we’ll need information about your domain name servers.

@@ -37,22 +37,22 @@ {% endif %} {% url 'domain-org-name-address' pk=domain.id as url %} - {% include "includes/summary_item.html" with title='Organization name and mailing address' value=domain.domain_info address='true' edit_link=url %} + {% include "includes/summary_item.html" with title='Organization name and mailing address' value=domain.domain_info address='true' edit_link=url editable=domain.is_editable %} {% url 'domain-authorizing-official' pk=domain.id as url %} - {% include "includes/summary_item.html" with title='Authorizing official' value=domain.domain_info.authorizing_official contact='true' edit_link=url %} + {% include "includes/summary_item.html" with title='Authorizing official' value=domain.domain_info.authorizing_official contact='true' edit_link=url editable=domain.is_editable %} {% url 'domain-your-contact-information' pk=domain.id as url %} - {% include "includes/summary_item.html" with title='Your contact information' value=request.user.contact contact='true' edit_link=url %} + {% include "includes/summary_item.html" with title='Your contact information' value=request.user.contact contact='true' edit_link=url editable=domain.is_editable %} {% url 'domain-security-email' pk=domain.id as url %} {% if security_email is not None and security_email != default_security_email%} - {% include "includes/summary_item.html" with title='Security email' value=security_email edit_link=url %} + {% include "includes/summary_item.html" with title='Security email' value=security_email edit_link=url editable=domain.is_editable %} {% else %} - {% include "includes/summary_item.html" with title='Security email' value='None provided' edit_link=url %} + {% include "includes/summary_item.html" with title='Security email' value='None provided' edit_link=url editable=domain.is_editable %} {% endif %} {% url 'domain-users' pk=domain.id as url %} - {% include "includes/summary_item.html" with title='Domain managers' users='true' list=True value=domain.permissions.all edit_link=url %} + {% include "includes/summary_item.html" with title='Domain managers' users='true' list=True value=domain.permissions.all edit_link=url editable=domain.is_editable %} {% endblock %} {# domain_content #} diff --git a/src/registrar/templates/domain_sidebar.html b/src/registrar/templates/domain_sidebar.html index 6be8f655d..c224d60c1 100644 --- a/src/registrar/templates/domain_sidebar.html +++ b/src/registrar/templates/domain_sidebar.html @@ -12,6 +12,7 @@ + {% if domain.is_editable %}
  • {% url 'domain-dns' pk=domain.id as url %} @@ -98,6 +99,7 @@ Domain managers
    • + {% endif %} diff --git a/src/registrar/templates/includes/summary_item.html b/src/registrar/templates/includes/summary_item.html index 8a33bb1d5..dea14553b 100644 --- a/src/registrar/templates/includes/summary_item.html +++ b/src/registrar/templates/includes/summary_item.html @@ -85,7 +85,7 @@ {% endif %} - {% if edit_link %} + {% if editable and edit_link %}
    Date: Wed, 22 Nov 2023 10:48:13 -0500 Subject: [PATCH 02/15] domain views has_permissions updated --- src/registrar/views/domain.py | 22 ++++++++++++++++++++++ src/registrar/views/utility/mixins.py | 17 +++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index 0b950ba7a..a9d0d4510 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -152,6 +152,28 @@ class DomainView(DomainBaseView): context["security_email"] = security_email return context + def in_editable_state(self, pk): + """Override in_editable_state from DomainPermission + Allow detail page to be editable""" + + requested_domain = None + if Domain.objects.filter(id=pk).exists(): + requested_domain = Domain.objects.get(id=pk) + + # if domain is editable return true + if requested_domain: + return True + return False + + def _get_domain(self, request): + """ + override get_domain for this view so that domain overview + always resets the cache for the domain object + """ + self.session = request.session + self.object = self.get_object() + self._update_session_with_domain() + class DomainOrgNameAddressView(DomainFormBaseView): """Organization name and mailing address view""" diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py index e37ff4927..596873cf3 100644 --- a/src/registrar/views/utility/mixins.py +++ b/src/registrar/views/utility/mixins.py @@ -3,6 +3,7 @@ from django.contrib.auth.mixins import PermissionRequiredMixin from registrar.models import ( + Domain, DomainApplication, DomainInvitation, DomainInformation, @@ -52,9 +53,25 @@ class DomainPermission(PermissionsLoginMixin): if not UserDomainRole.objects.filter(user=self.request.user, domain__id=pk).exists(): return False + # test if domain in editable state + if not self.in_editable_state(pk): + return False + # if we need to check more about the nature of role, do it here. return True + def in_editable_state(self, pk): + """Is the domain in an editable state""" + + requested_domain = None + if Domain.objects.filter(id=pk).exists(): + requested_domain = Domain.objects.get(id=pk) + + # if domain is editable return true + if requested_domain and requested_domain.is_editable(): + return True + return False + def can_access_other_user_domains(self, pk): """Checks to see if an authorized user (staff or superuser) can access a domain that they did not create or was invited to. From 3ec910c37a7a9c8c651750ccd9a2b66b6b3cb529 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 22 Nov 2023 11:18:54 -0500 Subject: [PATCH 03/15] wrote unit tests --- src/registrar/tests/test_views.py | 80 +++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 936c344f7..5b9ffa4bb 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1082,6 +1082,7 @@ class TestWithDomainPermissions(TestWithUser): self.domain_with_ip, _ = Domain.objects.get_or_create(name="nameserverwithip.gov") self.domain_just_nameserver, _ = Domain.objects.get_or_create(name="justnameserver.com") self.domain_no_information, _ = Domain.objects.get_or_create(name="noinformation.gov") + self.domain_on_hold, _ = Domain.objects.get_or_create(name="on-hold.gov", state=Domain.State.ON_HOLD) self.domain_dsdata, _ = Domain.objects.get_or_create(name="dnssec-dsdata.gov") self.domain_multdsdata, _ = Domain.objects.get_or_create(name="dnssec-multdsdata.gov") @@ -1096,6 +1097,7 @@ class TestWithDomainPermissions(TestWithUser): DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain_dnssec_none) DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain_with_ip) DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain_just_nameserver) + DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain_on_hold) self.role, _ = UserDomainRole.objects.get_or_create( user=self.user, domain=self.domain, role=UserDomainRole.Roles.MANAGER @@ -1124,6 +1126,9 @@ class TestWithDomainPermissions(TestWithUser): domain=self.domain_just_nameserver, role=UserDomainRole.Roles.MANAGER, ) + UserDomainRole.objects.get_or_create( + user=self.user, domain=self.domain_on_hold, role=UserDomainRole.Roles.MANAGER + ) def tearDown(self): try: @@ -1204,6 +1209,15 @@ class TestDomainOverview(TestWithDomainPermissions, WebTest): response = self.client.get(reverse("domain", kwargs={"pk": self.domain.id})) self.assertEqual(response.status_code, 403) + def test_domain_overview_allowed_for_on_hold(self): + """Test that the domain overview page displays for on hold domain""" + home_page = self.app.get("/") + self.assertContains(home_page, "on-hold.gov") + + # View domain overview page + detail_page = self.client.get(reverse("domain", kwargs={"pk": self.domain_on_hold.id})) + self.assertNotContains(detail_page, "Edit") + def test_domain_see_just_nameserver(self): home_page = self.app.get("/") self.assertContains(home_page, "justnameserver.com") @@ -1258,6 +1272,19 @@ class TestDomainManagers(TestDomainOverview): response = self.client.get(reverse("domain-users", kwargs={"pk": self.domain.id})) self.assertContains(response, "Domain managers") + def test_domain_users_blocked_for_on_hold(self): + """Test that the domain users page blocked for on hold domain""" + + # attempt to view domain users page + with less_console_noise(): + response = self.client.get(reverse("domain-users", kwargs={"pk": self.domain_on_hold.id})) + self.assertEqual(response.status_code, 403) + + # attempt to view domain users add page + with less_console_noise(): + response = self.client.get(reverse("domain-users-add", kwargs={"pk": self.domain_on_hold.id})) + self.assertEqual(response.status_code, 403) + def test_domain_managers_add_link(self): """Button to get to user add page works.""" management_page = self.app.get(reverse("domain-users", kwargs={"pk": self.domain.id})) @@ -1391,6 +1418,14 @@ class TestDomainNameservers(TestDomainOverview): page = self.client.get(reverse("domain-dns-nameservers", kwargs={"pk": self.domain.id})) self.assertContains(page, "DNS name servers") + def test_domain_nameservers_blocked_for_on_hold(self): + """Test that the domain nameservers page blocked for on hold domain""" + + # attempt to view domain nameservers page + with less_console_noise(): + response = self.client.get(reverse("domain-dns-nameservers", kwargs={"pk": self.domain_on_hold.id})) + self.assertEqual(response.status_code, 403) + def test_domain_nameservers_form_submit_one_nameserver(self): """Nameserver form submitted with one nameserver throws error. @@ -1606,6 +1641,14 @@ class TestDomainAuthorizingOfficial(TestDomainOverview): # once on the sidebar, once in the title self.assertContains(page, "Authorizing official", count=2) + def test_domain_authorizing_official_blocked_for_on_hold(self): + """Test that the domain authorizing official page blocked for on hold domain""" + + # attempt to view domain authorizing official page + with less_console_noise(): + response = self.client.get(reverse("domain-authorizing-official", kwargs={"pk": self.domain_on_hold.id})) + self.assertEqual(response.status_code, 403) + def test_domain_authorizing_official_content(self): """Authorizing official information appears on the page.""" self.domain_information.authorizing_official = Contact(first_name="Testy") @@ -1622,6 +1665,14 @@ class TestDomainOrganization(TestDomainOverview): # once on the sidebar, once in the page title, once as H1 self.assertContains(page, "Organization name and mailing address", count=3) + def test_domain_org_name_blocked_for_on_hold(self): + """Test that the domain org name page blocked for on hold domain""" + + # attempt to view domain org name page + with less_console_noise(): + response = self.client.get(reverse("domain-org-name-address", kwargs={"pk": self.domain_on_hold.id})) + self.assertEqual(response.status_code, 403) + def test_domain_org_name_address_content(self): """Org name and address information appears on the page.""" self.domain_information.organization_name = "Town of Igorville" @@ -1653,6 +1704,14 @@ class TestDomainContactInformation(TestDomainOverview): page = self.client.get(reverse("domain-your-contact-information", kwargs={"pk": self.domain.id})) self.assertContains(page, "Your contact information") + def test_domain_contact_information_blocked_for_on_hold(self): + """Test that the domain contact information page blocked for on hold domain""" + + # attempt to view domain contact information page + with less_console_noise(): + response = self.client.get(reverse("domain-your-contact-information", kwargs={"pk": self.domain_on_hold.id})) + self.assertEqual(response.status_code, 403) + def test_domain_your_contact_information_content(self): """Logged-in user's contact information appears on the page.""" self.user.contact.first_name = "Testy" @@ -1678,6 +1737,14 @@ class TestDomainSecurityEmail(TestDomainOverview): self.assertContains(page, "security@mail.gov") self.mockSendPatch.stop() + def test_domain_security_email_blocked_for_on_hold(self): + """Test that the domain security email page blocked for on hold domain""" + + # attempt to view domain security email page + with less_console_noise(): + response = self.client.get(reverse("domain-security-email", kwargs={"pk": self.domain_on_hold.id})) + self.assertEqual(response.status_code, 403) + def test_domain_security_email_no_security_contact(self): """Loads a domain with no defined security email. We should not show the default.""" @@ -1805,6 +1872,19 @@ class TestDomainDNSSEC(TestDomainOverview): page = self.client.get(reverse("domain-dns-dnssec", kwargs={"pk": self.domain.id})) self.assertContains(page, "Enable DNSSEC") + def test_domain_dnssec_blocked_for_on_hold(self): + """Test that the domain dnssec page blocked for on hold domain""" + + # attempt to view domain dnssec page + with less_console_noise(): + response = self.client.get(reverse("domain-dns-dnssec", kwargs={"pk": self.domain_on_hold.id})) + self.assertEqual(response.status_code, 403) + + # attempt to view domain dnssec dsdata page + with less_console_noise(): + response = self.client.get(reverse("domain-dns-dnssec-dsdata", kwargs={"pk": self.domain_on_hold.id})) + self.assertEqual(response.status_code, 403) + def test_dnssec_page_loads_with_data_in_domain(self): """DNSSEC overview page loads when domain has DNSSEC data and the template contains a button to disable DNSSEC.""" From 570c5c3020f148e1732a9d0a49bfc4c6cc919698 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 22 Nov 2023 11:50:18 -0500 Subject: [PATCH 04/15] formatted for linter --- src/registrar/tests/test_views.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 5b9ffa4bb..8818ae94f 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1213,11 +1213,11 @@ class TestDomainOverview(TestWithDomainPermissions, WebTest): """Test that the domain overview page displays for on hold domain""" home_page = self.app.get("/") self.assertContains(home_page, "on-hold.gov") - + # View domain overview page detail_page = self.client.get(reverse("domain", kwargs={"pk": self.domain_on_hold.id})) self.assertNotContains(detail_page, "Edit") - + def test_domain_see_just_nameserver(self): home_page = self.app.get("/") self.assertContains(home_page, "justnameserver.com") @@ -1274,7 +1274,7 @@ class TestDomainManagers(TestDomainOverview): def test_domain_users_blocked_for_on_hold(self): """Test that the domain users page blocked for on hold domain""" - + # attempt to view domain users page with less_console_noise(): response = self.client.get(reverse("domain-users", kwargs={"pk": self.domain_on_hold.id})) @@ -1420,7 +1420,7 @@ class TestDomainNameservers(TestDomainOverview): def test_domain_nameservers_blocked_for_on_hold(self): """Test that the domain nameservers page blocked for on hold domain""" - + # attempt to view domain nameservers page with less_console_noise(): response = self.client.get(reverse("domain-dns-nameservers", kwargs={"pk": self.domain_on_hold.id})) @@ -1643,7 +1643,7 @@ class TestDomainAuthorizingOfficial(TestDomainOverview): def test_domain_authorizing_official_blocked_for_on_hold(self): """Test that the domain authorizing official page blocked for on hold domain""" - + # attempt to view domain authorizing official page with less_console_noise(): response = self.client.get(reverse("domain-authorizing-official", kwargs={"pk": self.domain_on_hold.id})) @@ -1667,7 +1667,7 @@ class TestDomainOrganization(TestDomainOverview): def test_domain_org_name_blocked_for_on_hold(self): """Test that the domain org name page blocked for on hold domain""" - + # attempt to view domain org name page with less_console_noise(): response = self.client.get(reverse("domain-org-name-address", kwargs={"pk": self.domain_on_hold.id})) @@ -1706,10 +1706,12 @@ class TestDomainContactInformation(TestDomainOverview): def test_domain_contact_information_blocked_for_on_hold(self): """Test that the domain contact information page blocked for on hold domain""" - + # attempt to view domain contact information page with less_console_noise(): - response = self.client.get(reverse("domain-your-contact-information", kwargs={"pk": self.domain_on_hold.id})) + response = self.client.get( + reverse("domain-your-contact-information", kwargs={"pk": self.domain_on_hold.id}) + ) self.assertEqual(response.status_code, 403) def test_domain_your_contact_information_content(self): @@ -1739,7 +1741,7 @@ class TestDomainSecurityEmail(TestDomainOverview): def test_domain_security_email_blocked_for_on_hold(self): """Test that the domain security email page blocked for on hold domain""" - + # attempt to view domain security email page with less_console_noise(): response = self.client.get(reverse("domain-security-email", kwargs={"pk": self.domain_on_hold.id})) @@ -1874,7 +1876,7 @@ class TestDomainDNSSEC(TestDomainOverview): def test_domain_dnssec_blocked_for_on_hold(self): """Test that the domain dnssec page blocked for on hold domain""" - + # attempt to view domain dnssec page with less_console_noise(): response = self.client.get(reverse("domain-dns-dnssec", kwargs={"pk": self.domain_on_hold.id})) @@ -1884,7 +1886,7 @@ class TestDomainDNSSEC(TestDomainOverview): with less_console_noise(): response = self.client.get(reverse("domain-dns-dnssec-dsdata", kwargs={"pk": self.domain_on_hold.id})) self.assertEqual(response.status_code, 403) - + def test_dnssec_page_loads_with_data_in_domain(self): """DNSSEC overview page loads when domain has DNSSEC data and the template contains a button to disable DNSSEC.""" From 8300694a3a63760dc478f69fa605a0a776c7dd6a Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 22 Nov 2023 14:26:24 -0500 Subject: [PATCH 05/15] added test cases for deleted state; updated fetch_cache logic to properly handle deleted domains --- src/registrar/models/domain.py | 2 +- src/registrar/tests/test_views.py | 98 ++++++++++--------------------- 2 files changed, 31 insertions(+), 69 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 93b50bf81..d28008c9a 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -1196,7 +1196,7 @@ class Domain(TimeStampedModel, DomainHelper): logger.error(e) logger.error(e.code) raise e - if e.code == ErrorCode.OBJECT_DOES_NOT_EXIST: + if e.code == ErrorCode.OBJECT_DOES_NOT_EXIST and self.state != Domain.State.DELETED: # avoid infinite loop already_tried_to_create = True self.dns_needed_from_unknown() diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 8818ae94f..d6bad9cdf 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1083,6 +1083,7 @@ class TestWithDomainPermissions(TestWithUser): self.domain_just_nameserver, _ = Domain.objects.get_or_create(name="justnameserver.com") self.domain_no_information, _ = Domain.objects.get_or_create(name="noinformation.gov") self.domain_on_hold, _ = Domain.objects.get_or_create(name="on-hold.gov", state=Domain.State.ON_HOLD) + self.domain_deleted, _ = Domain.objects.get_or_create(name="deleted.gov", state=Domain.State.DELETED) self.domain_dsdata, _ = Domain.objects.get_or_create(name="dnssec-dsdata.gov") self.domain_multdsdata, _ = Domain.objects.get_or_create(name="dnssec-multdsdata.gov") @@ -1098,6 +1099,7 @@ class TestWithDomainPermissions(TestWithUser): DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain_with_ip) DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain_just_nameserver) DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain_on_hold) + DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain_deleted) self.role, _ = UserDomainRole.objects.get_or_create( user=self.user, domain=self.domain, role=UserDomainRole.Roles.MANAGER @@ -1129,6 +1131,9 @@ class TestWithDomainPermissions(TestWithUser): UserDomainRole.objects.get_or_create( user=self.user, domain=self.domain_on_hold, role=UserDomainRole.Roles.MANAGER ) + UserDomainRole.objects.get_or_create( + user=self.user, domain=self.domain_deleted, role=UserDomainRole.Roles.MANAGER + ) def tearDown(self): try: @@ -1182,6 +1187,31 @@ class TestDomainPermissions(TestWithDomainPermissions): response = self.client.get(reverse(view_name, kwargs={"pk": self.domain.id})) self.assertEqual(response.status_code, 403) + def test_domain_pages_blocked_for_on_hold_and_deleted(self): + """Test that the domain pages are blocked for on hold and deleted domains""" + + self.client.force_login(self.user) + for view_name in [ + "domain-users", + "domain-users-add", + "domain-dns", + "domain-dns-nameservers", + "domain-dns-dnssec", + "domain-dns-dnssec-dsdata", + "domain-org-name-address", + "domain-authorizing-official", + "domain-your-contact-information", + "domain-security-email", + ]: + for domain in [ + self.domain_on_hold, + self.domain_deleted, + ]: + with self.subTest(view_name=view_name, domain=domain): + with less_console_noise(): + response = self.client.get(reverse(view_name, kwargs={"pk": domain.id})) + self.assertEqual(response.status_code, 403) + class TestDomainOverview(TestWithDomainPermissions, WebTest): def setUp(self): @@ -1272,19 +1302,6 @@ class TestDomainManagers(TestDomainOverview): response = self.client.get(reverse("domain-users", kwargs={"pk": self.domain.id})) self.assertContains(response, "Domain managers") - def test_domain_users_blocked_for_on_hold(self): - """Test that the domain users page blocked for on hold domain""" - - # attempt to view domain users page - with less_console_noise(): - response = self.client.get(reverse("domain-users", kwargs={"pk": self.domain_on_hold.id})) - self.assertEqual(response.status_code, 403) - - # attempt to view domain users add page - with less_console_noise(): - response = self.client.get(reverse("domain-users-add", kwargs={"pk": self.domain_on_hold.id})) - self.assertEqual(response.status_code, 403) - def test_domain_managers_add_link(self): """Button to get to user add page works.""" management_page = self.app.get(reverse("domain-users", kwargs={"pk": self.domain.id})) @@ -1418,14 +1435,6 @@ class TestDomainNameservers(TestDomainOverview): page = self.client.get(reverse("domain-dns-nameservers", kwargs={"pk": self.domain.id})) self.assertContains(page, "DNS name servers") - def test_domain_nameservers_blocked_for_on_hold(self): - """Test that the domain nameservers page blocked for on hold domain""" - - # attempt to view domain nameservers page - with less_console_noise(): - response = self.client.get(reverse("domain-dns-nameservers", kwargs={"pk": self.domain_on_hold.id})) - self.assertEqual(response.status_code, 403) - def test_domain_nameservers_form_submit_one_nameserver(self): """Nameserver form submitted with one nameserver throws error. @@ -1641,14 +1650,6 @@ class TestDomainAuthorizingOfficial(TestDomainOverview): # once on the sidebar, once in the title self.assertContains(page, "Authorizing official", count=2) - def test_domain_authorizing_official_blocked_for_on_hold(self): - """Test that the domain authorizing official page blocked for on hold domain""" - - # attempt to view domain authorizing official page - with less_console_noise(): - response = self.client.get(reverse("domain-authorizing-official", kwargs={"pk": self.domain_on_hold.id})) - self.assertEqual(response.status_code, 403) - def test_domain_authorizing_official_content(self): """Authorizing official information appears on the page.""" self.domain_information.authorizing_official = Contact(first_name="Testy") @@ -1665,14 +1666,6 @@ class TestDomainOrganization(TestDomainOverview): # once on the sidebar, once in the page title, once as H1 self.assertContains(page, "Organization name and mailing address", count=3) - def test_domain_org_name_blocked_for_on_hold(self): - """Test that the domain org name page blocked for on hold domain""" - - # attempt to view domain org name page - with less_console_noise(): - response = self.client.get(reverse("domain-org-name-address", kwargs={"pk": self.domain_on_hold.id})) - self.assertEqual(response.status_code, 403) - def test_domain_org_name_address_content(self): """Org name and address information appears on the page.""" self.domain_information.organization_name = "Town of Igorville" @@ -1704,16 +1697,6 @@ class TestDomainContactInformation(TestDomainOverview): page = self.client.get(reverse("domain-your-contact-information", kwargs={"pk": self.domain.id})) self.assertContains(page, "Your contact information") - def test_domain_contact_information_blocked_for_on_hold(self): - """Test that the domain contact information page blocked for on hold domain""" - - # attempt to view domain contact information page - with less_console_noise(): - response = self.client.get( - reverse("domain-your-contact-information", kwargs={"pk": self.domain_on_hold.id}) - ) - self.assertEqual(response.status_code, 403) - def test_domain_your_contact_information_content(self): """Logged-in user's contact information appears on the page.""" self.user.contact.first_name = "Testy" @@ -1739,14 +1722,6 @@ class TestDomainSecurityEmail(TestDomainOverview): self.assertContains(page, "security@mail.gov") self.mockSendPatch.stop() - def test_domain_security_email_blocked_for_on_hold(self): - """Test that the domain security email page blocked for on hold domain""" - - # attempt to view domain security email page - with less_console_noise(): - response = self.client.get(reverse("domain-security-email", kwargs={"pk": self.domain_on_hold.id})) - self.assertEqual(response.status_code, 403) - def test_domain_security_email_no_security_contact(self): """Loads a domain with no defined security email. We should not show the default.""" @@ -1874,19 +1849,6 @@ class TestDomainDNSSEC(TestDomainOverview): page = self.client.get(reverse("domain-dns-dnssec", kwargs={"pk": self.domain.id})) self.assertContains(page, "Enable DNSSEC") - def test_domain_dnssec_blocked_for_on_hold(self): - """Test that the domain dnssec page blocked for on hold domain""" - - # attempt to view domain dnssec page - with less_console_noise(): - response = self.client.get(reverse("domain-dns-dnssec", kwargs={"pk": self.domain_on_hold.id})) - self.assertEqual(response.status_code, 403) - - # attempt to view domain dnssec dsdata page - with less_console_noise(): - response = self.client.get(reverse("domain-dns-dnssec-dsdata", kwargs={"pk": self.domain_on_hold.id})) - self.assertEqual(response.status_code, 403) - def test_dnssec_page_loads_with_data_in_domain(self): """DNSSEC overview page loads when domain has DNSSEC data and the template contains a button to disable DNSSEC.""" From 441a6027baf23d180792eab6d699b22cb978390b Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 22 Nov 2023 17:41:27 -0500 Subject: [PATCH 06/15] expiration date written to db on fetch_cache --- src/registrar/models/domain.py | 10 +++++++--- src/registrar/tests/common.py | 1 + src/registrar/tests/test_models_domain.py | 7 +++++++ 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 37e78ec6e..4cb2e72a3 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -214,9 +214,7 @@ class Domain(TimeStampedModel, DomainHelper): """Get or set the `ex_date` element from the registry. Additionally, update the expiration date in the registrar""" try: - self.expiration_date = self._get_property("ex_date") - self.save() - return self.expiration_date + return self._get_property("ex_date") except Exception as e: # exception raised during the save to registrar logger.error(f"error updating expiration date in registrar: {e}") @@ -1602,6 +1600,12 @@ class Domain(TimeStampedModel, DomainHelper): if old_cache_contacts is not None: cleaned["contacts"] = old_cache_contacts + # if expiration date from registry does not match what is in db, + # update the db + if "ex_date" in cleaned and cleaned["ex_date"] != self.expiration_date: + self.expiration_date = cleaned["ex_date"] + self.save() + self._cache = cleaned except RegistryError as e: diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 9a062106f..46081e98f 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -617,6 +617,7 @@ class MockEppLib(TestCase): common.Status(state="serverTransferProhibited", description="", lang="en"), common.Status(state="inactive", description="", lang="en"), ], + ex_date=datetime.date(2023, 5, 25), ) mockDataInfoContact = mockDataInfoDomain.dummyInfoContactResultData( "123", "123@mail.gov", datetime.datetime(2023, 5, 25, 19, 45, 35), "lastPw" diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index c75b1b935..4a2023243 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -1987,6 +1987,13 @@ class TestExpirationDate(MockEppLib): with self.assertRaises(RegistryError): self.domain_w_error.renew_domain() + def test_expiration_date_updated_on_info_domain_call(self): + """assert that expiration date in db is updated on info domain call""" + # force fetch_cache to be called + self.domain.statuses + test_date = datetime.date(2023, 5, 25) + self.assertEquals(self.domain.expiration_date, test_date) + class TestAnalystClientHold(MockEppLib): """Rule: Analysts may suspend or restore a domain by using client hold""" From 46677f4588458d7aa8c8157de17eef79888320bd Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 24 Nov 2023 07:43:25 -0700 Subject: [PATCH 07/15] Add filter + test skeleton --- src/registrar/admin.py | 4 ++++ src/registrar/tests/test_admin.py | 37 +++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index c059e5674..9921b1194 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -356,6 +356,10 @@ class DomainInvitationAdmin(ListHeaderAdmin): "email", "domain__name", ] + + # Filters + list_filter = ("status",) + search_help_text = "Search by email or domain." # Mark the FSM field 'status' as readonly diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index a7cbb5d33..1ad5f95da 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -8,6 +8,7 @@ from registrar.admin import ( DomainAdmin, DomainApplicationAdmin, DomainApplicationAdminForm, + DomainInvitationAdmin, ListHeaderAdmin, MyUserAdmin, AuditedAdmin, @@ -847,6 +848,42 @@ class TestDomainApplicationAdmin(MockEppLib): User.objects.all().delete() +class DomainInvitationAdminTest(TestCase): + def setUp(self): + self.site = AdminSite() + self.factory = RequestFactory() + self.admin = ListHeaderAdmin(model=DomainInvitationAdmin, admin_site=None) + self.client = Client(HTTP_HOST="localhost:8080") + self.superuser = create_superuser() + + def tearDown(self): + # delete any applications too + DomainInvitation.objects.all().delete() + DomainApplication.objects.all().delete() + User.objects.all().delete() + + def test_get_filters(self): + # Create a mock request object + request = self.factory.get("/admin/yourmodel/") + # Set the GET parameters for testing + request.GET = { + "status": "started", + "investigator": "Rachid Mrad", + "q": "search_value", + } + # Call the get_filters method + filters = self.admin.get_filters(request) + + # Assert the filters extracted from the request GET + self.assertEqual( + filters, + [ + {"parameter_name": "status", "parameter_value": "started"}, + {"parameter_name": "investigator", "parameter_value": "Rachid Mrad"}, + ], + ) + + class ListHeaderAdminTest(TestCase): def setUp(self): self.site = AdminSite() From 377786cccfbccfc3e55f551b20cd469eee457522 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 24 Nov 2023 14:16:34 -0700 Subject: [PATCH 08/15] Add test cases --- src/registrar/admin.py | 5 +++++ src/registrar/tests/test_admin.py | 34 ++++++++++++++----------------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 9921b1194..ff0f71426 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -343,6 +343,11 @@ class UserDomainRoleAdmin(ListHeaderAdmin): class DomainInvitationAdmin(ListHeaderAdmin): """Custom domain invitation admin class.""" + class Meta: + model = models.DomainInvitation + fields = "__all__" + + _meta = Meta() # Columns list_display = [ diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 1ad5f95da..089e569eb 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -850,9 +850,8 @@ class TestDomainApplicationAdmin(MockEppLib): class DomainInvitationAdminTest(TestCase): def setUp(self): - self.site = AdminSite() self.factory = RequestFactory() - self.admin = ListHeaderAdmin(model=DomainInvitationAdmin, admin_site=None) + self.admin = ListHeaderAdmin(model=DomainInvitationAdmin, admin_site=AdminSite()) self.client = Client(HTTP_HOST="localhost:8080") self.superuser = create_superuser() @@ -863,26 +862,23 @@ class DomainInvitationAdminTest(TestCase): User.objects.all().delete() def test_get_filters(self): - # Create a mock request object - request = self.factory.get("/admin/yourmodel/") - # Set the GET parameters for testing - request.GET = { - "status": "started", - "investigator": "Rachid Mrad", - "q": "search_value", - } - # Call the get_filters method - filters = self.admin.get_filters(request) + # Have to get creative to get past linter + p = "adminpass" + self.client.login(username="superuser", password=p) - # Assert the filters extracted from the request GET - self.assertEqual( - filters, - [ - {"parameter_name": "status", "parameter_value": "started"}, - {"parameter_name": "investigator", "parameter_value": "Rachid Mrad"}, - ], + # Mock a user + user = mock_user() + + response = self.client.get( + "/admin/registrar/domaininvitation/", + {}, + follow=True, ) + # Assert that the filters are added + self.assertContains(response, "invited", count=4) + self.assertContains(response, "retrieved", count=4) + class ListHeaderAdminTest(TestCase): def setUp(self): From e314a5ff77fd728f64b29791a666a2105fd3cca8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 27 Nov 2023 08:02:58 -0700 Subject: [PATCH 09/15] Update tests + linter --- src/registrar/admin.py | 3 ++- src/registrar/tests/test_admin.py | 22 ++++++++++++++-------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index ff0f71426..6585d602a 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -343,10 +343,11 @@ class UserDomainRoleAdmin(ListHeaderAdmin): class DomainInvitationAdmin(ListHeaderAdmin): """Custom domain invitation admin class.""" + class Meta: model = models.DomainInvitation fields = "__all__" - + _meta = Meta() # Columns diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 089e569eb..bff27a83d 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -849,26 +849,25 @@ class TestDomainApplicationAdmin(MockEppLib): class DomainInvitationAdminTest(TestCase): + """Tests for the DomainInvitation page""" + def setUp(self): + """Create a client object""" + self.client = Client(HTTP_HOST="localhost:8080") self.factory = RequestFactory() self.admin = ListHeaderAdmin(model=DomainInvitationAdmin, admin_site=AdminSite()) - self.client = Client(HTTP_HOST="localhost:8080") self.superuser = create_superuser() - + def tearDown(self): - # delete any applications too + """Delete all DomainInvitation objects""" DomainInvitation.objects.all().delete() - DomainApplication.objects.all().delete() - User.objects.all().delete() def test_get_filters(self): + """Ensures that our filters are displaying correctly""" # Have to get creative to get past linter p = "adminpass" self.client.login(username="superuser", password=p) - # Mock a user - user = mock_user() - response = self.client.get( "/admin/registrar/domaininvitation/", {}, @@ -879,6 +878,13 @@ class DomainInvitationAdminTest(TestCase): self.assertContains(response, "invited", count=4) self.assertContains(response, "retrieved", count=4) + # Check for the HTML context specificially + invited_html = 'invited' + retrieved_html = 'retrieved' + + self.assertContains(response, invited_html, count=1) + self.assertContains(response, retrieved_html, count=1) + class ListHeaderAdminTest(TestCase): def setUp(self): From 6ace0f2862ef8862c03851971c04f440159bee12 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 27 Nov 2023 13:02:26 -0700 Subject: [PATCH 10/15] Update test_reports.py --- src/registrar/tests/test_reports.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 14573ab65..52b971601 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -50,6 +50,7 @@ class ExportDataTest(TestCase): ) def tearDown(self): + # Dummy push - will remove Domain.objects.all().delete() DomainInformation.objects.all().delete() User.objects.all().delete() From 315638c020742c17d92c4933477a81bb65e0bd9e Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 27 Nov 2023 15:08:42 -0500 Subject: [PATCH 11/15] updated comments for code legibility --- src/registrar/views/domain.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/views/domain.py b/src/registrar/views/domain.py index a9d0d4510..3aac32531 100644 --- a/src/registrar/views/domain.py +++ b/src/registrar/views/domain.py @@ -154,13 +154,13 @@ class DomainView(DomainBaseView): def in_editable_state(self, pk): """Override in_editable_state from DomainPermission - Allow detail page to be editable""" + Allow detail page to be viewable""" requested_domain = None if Domain.objects.filter(id=pk).exists(): requested_domain = Domain.objects.get(id=pk) - # if domain is editable return true + # return true if the domain exists, this will allow the detail page to load if requested_domain: return True return False From abe35b9d633b7a2952e148a6cb6ad842539409b7 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 27 Nov 2023 18:02:43 -0500 Subject: [PATCH 12/15] changed the order of the permissions checking as the manage domain check was firing before editable check and allowing access --- src/registrar/views/utility/mixins.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/registrar/views/utility/mixins.py b/src/registrar/views/utility/mixins.py index 596873cf3..aaa2e849c 100644 --- a/src/registrar/views/utility/mixins.py +++ b/src/registrar/views/utility/mixins.py @@ -46,6 +46,10 @@ class DomainPermission(PermissionsLoginMixin): if pk is None: raise ValueError("Primary key is None") + # test if domain in editable state + if not self.in_editable_state(pk): + return False + if self.can_access_other_user_domains(pk): return True @@ -53,10 +57,6 @@ class DomainPermission(PermissionsLoginMixin): if not UserDomainRole.objects.filter(user=self.request.user, domain__id=pk).exists(): return False - # test if domain in editable state - if not self.in_editable_state(pk): - return False - # if we need to check more about the nature of role, do it here. return True From 20c13a5d2f97dfc8a43cf3d95e29d4fd8f94bf5f Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 27 Nov 2023 19:16:45 -0500 Subject: [PATCH 13/15] when in read only mode remove add dns name server button and text --- src/registrar/templates/domain_detail.html | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/registrar/templates/domain_detail.html b/src/registrar/templates/domain_detail.html index 2bd6aa0ed..8d471d57e 100644 --- a/src/registrar/templates/domain_detail.html +++ b/src/registrar/templates/domain_detail.html @@ -31,9 +31,11 @@ {% if domain.nameservers|length > 0 %} {% include "includes/summary_item.html" with title='DNS name servers' domains='true' value=domain.nameservers list='true' edit_link=url editable=domain.is_editable %} {% else %} + {% if domain.is_editable %}

    DNS name servers

    No DNS name servers have been added yet. Before your domain can be used we’ll need information about your domain name servers.

    - Add DNS name servers + Add DNS name servers + {% endif %} {% endif %} {% url 'domain-org-name-address' pk=domain.id as url %} From 3ce76de1ff61b370f6e24e145cc413b39abcfa80 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 27 Nov 2023 19:29:00 -0500 Subject: [PATCH 14/15] when in read only mode and no dns name servers, add empty section for dns name servers instead of button to add --- src/registrar/templates/domain_detail.html | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/registrar/templates/domain_detail.html b/src/registrar/templates/domain_detail.html index 8d471d57e..81a350f82 100644 --- a/src/registrar/templates/domain_detail.html +++ b/src/registrar/templates/domain_detail.html @@ -35,6 +35,8 @@

    DNS name servers

    No DNS name servers have been added yet. Before your domain can be used we’ll need information about your domain name servers.

    Add DNS name servers + {% else %} + {% include "includes/summary_item.html" with title='DNS name servers' domains='true' value='' edit_link=url editable=domain.is_editable %} {% endif %} {% endif %} From 128efb6a54b2a489268cab555720dd213a3716ef Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 27 Nov 2023 20:03:38 -0500 Subject: [PATCH 15/15] updated comment --- src/registrar/models/domain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 1e4ab0e17..94430fb36 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -212,7 +212,7 @@ class Domain(TimeStampedModel, DomainHelper): @Cache def registry_expiration_date(self) -> date: """Get or set the `ex_date` element from the registry. - Additionally, update the expiration date in the registrar""" + Additionally, _get_property updates the expiration date in the registrar""" try: return self._get_property("ex_date") except Exception as e: