From 18447c14864a72d72181097b0bea00537935544d Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Fri, 20 Oct 2023 14:42:33 -0700 Subject: [PATCH 01/12] Formatting for nameservers --- src/registrar/models/domain.py | 38 ++++++++++++++++++++++ src/registrar/templates/domain_detail.html | 4 +-- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index c7d786426..ab0ad4b7b 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -2,6 +2,7 @@ from itertools import zip_longest import logging import ipaddress import re +import string from datetime import date from string import digits from typing import Optional @@ -228,6 +229,43 @@ class Domain(TimeStampedModel, DomainHelper): """ raise NotImplementedError() + @Cache + def format_nameservers(self): + """ + Formatting nameservers for display for this domain. + + Hosts are provided as a list of tuples, e.g. + + [("ns1.example.com",), ("ns1.example.gov", ["0.0.0.0"])] + + We want to display as: + ns1.example.gov + ns1.example.gov (0.0.0.0) + + Subordinate hosts (something.your-domain.gov) MUST have IP addresses, + while non-subordinate hosts MUST NOT. + + + """ + try: + hosts = self._get_property("hosts") + except Exception as err: + # Do not raise error when missing nameservers + # this is a standard occurence when a domain + # is first created + logger.info("Domain is missing nameservers %s" % err) + return [] + + hostList = [] + for host in hosts: + host_info = host["name"] + if len(host["addrs"]) > 0: + converter = string.maketrans("[]", "()") + host_info.append(host["addrs"].translate(converter, "'")) + hostList.append(host_info) + + return hostList + @Cache def nameservers(self) -> list[tuple[str, list]]: """ diff --git a/src/registrar/templates/domain_detail.html b/src/registrar/templates/domain_detail.html index e0d672093..c5394e433 100644 --- a/src/registrar/templates/domain_detail.html +++ b/src/registrar/templates/domain_detail.html @@ -28,8 +28,8 @@
{% url 'domain-dns-nameservers' pk=domain.id as url %} - {% if domain.nameservers|length > 0 %} - {% include "includes/summary_item.html" with title='DNS name servers' value=domain.nameservers list='true' edit_link=url %} + {% if domain.format_nameservers|length > 0 %} + {% include "includes/summary_item.html" with title='DNS name servers' value=domain.format_nameservers list='true' edit_link=url %} {% 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.

From ea567192d043ab995ca5ed0e0bcbca4211c31e95 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 23 Oct 2023 14:10:43 -0700 Subject: [PATCH 02/12] Convert bracket to parenthesis --- src/registrar/models/domain.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index ab0ad4b7b..c97bf1318 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -2,7 +2,6 @@ from itertools import zip_longest import logging import ipaddress import re -import string from datetime import date from string import digits from typing import Optional @@ -260,10 +259,8 @@ class Domain(TimeStampedModel, DomainHelper): for host in hosts: host_info = host["name"] if len(host["addrs"]) > 0: - converter = string.maketrans("[]", "()") - host_info.append(host["addrs"].translate(converter, "'")) + hostList.append(tuple(host["addrs"])) hostList.append(host_info) - return hostList @Cache @@ -291,7 +288,6 @@ class Domain(TimeStampedModel, DomainHelper): hostList = [] for host in hosts: hostList.append((host["name"], host["addrs"])) - return hostList def _create_host(self, host, addrs): From 9c939d4a84280f19821d9f907dbd5c3396d8a74d Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Mon, 23 Oct 2023 15:20:34 -0700 Subject: [PATCH 03/12] Reformat DNS name servers --- src/registrar/models/domain.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index c97bf1318..ce5198756 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -257,10 +257,8 @@ class Domain(TimeStampedModel, DomainHelper): hostList = [] for host in hosts: - host_info = host["name"] - if len(host["addrs"]) > 0: - hostList.append(tuple(host["addrs"])) - hostList.append(host_info) + host_info_str = f'{host["name"]} {host["addrs"] if len(host["addrs"]) > 0 else ""}' + hostList.append(host_info_str) return hostList @Cache From fbbde984090e05888e1c70f56c174c8dce9ad0a2 Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Tue, 24 Oct 2023 13:44:49 -0700 Subject: [PATCH 04/12] Format hosts to name (IP addresses) --- src/registrar/models/domain.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index ce5198756..38afa28ee 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -257,7 +257,11 @@ class Domain(TimeStampedModel, DomainHelper): hostList = [] for host in hosts: - host_info_str = f'{host["name"]} {host["addrs"] if len(host["addrs"]) > 0 else ""}' + # can remove str conversion when EPP.IP address display changes from IP object -> IP address + host_addrs_stringified = [str(addr) for addr in host["addrs"]] + host_addrs_str = "" + host_addrs_str = f'({", ".join(obj for obj in host_addrs_stringified)})' + host_info_str = f'{host["name"]} {host_addrs_str if len(host["addrs"]) > 0 else ""}' hostList.append(host_info_str) return hostList From 61d6dda59f91ef42994fc263964fceed39af86cc Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Tue, 24 Oct 2023 13:46:01 -0700 Subject: [PATCH 05/12] Clean up unused string initialization --- src/registrar/models/domain.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 38afa28ee..0a78d4ccf 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -259,7 +259,6 @@ class Domain(TimeStampedModel, DomainHelper): for host in hosts: # can remove str conversion when EPP.IP address display changes from IP object -> IP address host_addrs_stringified = [str(addr) for addr in host["addrs"]] - host_addrs_str = "" host_addrs_str = f'({", ".join(obj for obj in host_addrs_stringified)})' host_info_str = f'{host["name"]} {host_addrs_str if len(host["addrs"]) > 0 else ""}' hostList.append(host_info_str) From 6c26010fe535271e2e12dfee8eb5e33e69b8427f Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Tue, 24 Oct 2023 14:43:29 -0700 Subject: [PATCH 06/12] Resolve lint errors --- src/registrar/models/domain.py | 6 ++++-- src/registrar/tests/test_views.py | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 0a78d4ccf..9060ef90d 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -257,10 +257,12 @@ class Domain(TimeStampedModel, DomainHelper): hostList = [] for host in hosts: - # can remove str conversion when EPP.IP address display changes from IP object -> IP address + # can remove str conversion when EPP.IP address display + # changes from IP object -> IP address host_addrs_stringified = [str(addr) for addr in host["addrs"]] host_addrs_str = f'({", ".join(obj for obj in host_addrs_stringified)})' - host_info_str = f'{host["name"]} {host_addrs_str if len(host["addrs"]) > 0 else ""}' + host_info_str = f'{host["name"]} \ + {host_addrs_str if len(host["addrs"]) > 0 else ""}' hostList.append(host_info_str) return hostList diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 7cc616889..7b46b530e 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1392,6 +1392,8 @@ class TestDomainNameservers(TestDomainOverview): page = result.follow() self.assertContains(page, "The name servers for this domain have been updated") + # changed nameserver is reflected on domain overview page + @skip("Broken by adding registry connection fix in ticket 848") def test_domain_nameservers_form_invalid(self): """Can change domain's nameservers. From 5267a1af53bfdfbae828f39ca27696bd2dcdbf44 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 24 Oct 2023 15:47:24 -0700 Subject: [PATCH 07/12] Add test cases for displaying just nameserver and displaying nameserver AND ip --- src/registrar/tests/common.py | 40 +++++++++++++++++++++- src/registrar/tests/test_views.py | 56 +++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 1 deletion(-) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 8cd5fd6ba..463530e85 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -738,6 +738,7 @@ class MockEppLib(TestCase): "ns1.cats-are-superior3.com", ], ) + infoDomainNoHost = fakedEppObject( "my-nameserver.gov", cr_date=datetime.datetime(2023, 5, 25, 19, 45, 35), @@ -804,7 +805,20 @@ class MockEppLib(TestCase): infoDomainHasIP = fakedEppObject( "nameserverwithip.gov", cr_date=datetime.datetime(2023, 5, 25, 19, 45, 35), - contacts=[], + contacts=[ + common.DomainContact( + contact="securityContact", + type=PublicContact.ContactTypeChoices.SECURITY, + ), + common.DomainContact( + contact="technicalContact", + type=PublicContact.ContactTypeChoices.TECHNICAL, + ), + common.DomainContact( + contact="adminContact", + type=PublicContact.ContactTypeChoices.ADMINISTRATIVE, + ), + ], hosts=[ "ns1.nameserverwithip.gov", "ns2.nameserverwithip.gov", @@ -813,6 +827,29 @@ class MockEppLib(TestCase): addrs=["1.2.3.4", "2.3.4.5"], ) + justNameserver = fakedEppObject( + "justnameserver.com", + cr_date=datetime.datetime(2023, 5, 25, 19, 45, 35), + contacts=[ + common.DomainContact( + contact="securityContact", + type=PublicContact.ContactTypeChoices.SECURITY, + ), + common.DomainContact( + contact="technicalContact", + type=PublicContact.ContactTypeChoices.TECHNICAL, + ), + common.DomainContact( + contact="adminContact", + type=PublicContact.ContactTypeChoices.ADMINISTRATIVE, + ), + ], + hosts=[ + "ns1.justnameserver.com", + "ns2.justnameserver.com", + ], + ) + infoDomainCheckHostIPCombo = fakedEppObject( "nameserversubdomain.gov", cr_date=datetime.datetime(2023, 5, 25, 19, 45, 35), @@ -931,6 +968,7 @@ class MockEppLib(TestCase): "threenameserversDomain.gov": (self.infoDomainThreeHosts, None), "defaultsecurity.gov": (self.InfoDomainWithDefaultSecurityContact, None), "defaulttechnical.gov": (self.InfoDomainWithDefaultTechnicalContact, None), + "justnameserver.com": (self.justNameserver, None), } # Retrieve the corresponding values from the dictionary diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 7b46b530e..81afe1182 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1071,6 +1071,13 @@ class TestWithDomainPermissions(TestWithUser): def setUp(self): super().setUp() self.domain, _ = Domain.objects.get_or_create(name="igorville.gov") + 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_dsdata, _ = Domain.objects.get_or_create(name="dnssec-dsdata.gov") self.domain_multdsdata, _ = Domain.objects.get_or_create( name="dnssec-multdsdata.gov" @@ -1081,9 +1088,11 @@ class TestWithDomainPermissions(TestWithUser): self.domain_dnssec_none, _ = Domain.objects.get_or_create( name="dnssec-none.gov" ) + self.domain_information, _ = DomainInformation.objects.get_or_create( creator=self.user, domain=self.domain ) + DomainInformation.objects.get_or_create( creator=self.user, domain=self.domain_dsdata ) @@ -1096,9 +1105,17 @@ 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 + ) + self.role, _ = UserDomainRole.objects.get_or_create( user=self.user, domain=self.domain, role=UserDomainRole.Roles.MANAGER ) + UserDomainRole.objects.get_or_create( user=self.user, domain=self.domain_dsdata, role=UserDomainRole.Roles.MANAGER ) @@ -1117,6 +1134,16 @@ class TestWithDomainPermissions(TestWithUser): domain=self.domain_dnssec_none, role=UserDomainRole.Roles.MANAGER, ) + UserDomainRole.objects.get_or_create( + user=self.user, + domain=self.domain_with_ip, + role=UserDomainRole.Roles.MANAGER, + ) + UserDomainRole.objects.get_or_create( + user=self.user, + domain=self.domain_just_nameserver, + role=UserDomainRole.Roles.MANAGER, + ) def tearDown(self): try: @@ -1200,6 +1227,35 @@ class TestDomainOverview(TestWithDomainPermissions, WebTest): response = self.client.get(reverse("domain", kwargs={"pk": self.domain.id})) self.assertEqual(response.status_code, 403) + def test_domain_see_just_nameserver(self): + home_page = self.app.get("/") + self.assertContains(home_page, "justnameserver.com") + + # View nameserver on Domain Overview page + detail_page = self.app.get( + reverse("domain", kwargs={"pk": self.domain_just_nameserver.id}) + ) + + self.assertContains(detail_page, "justnameserver.com") + self.assertContains(detail_page, "ns1.justnameserver.com") + self.assertContains(detail_page, "ns2.justnameserver.com") + + def test_domain_see_nameserver_and_ip(self): + home_page = self.app.get("/") + self.assertContains(home_page, "nameserverwithip.gov") + + # View nameserver on Domain Overview page + detail_page = self.app.get( + reverse("domain", kwargs={"pk": self.domain_with_ip.id}) + ) + + self.assertContains(detail_page, "nameserverwithip.gov") + + self.assertContains(detail_page, "ns1.nameserverwithip.gov") + self.assertContains(detail_page, "ns2.nameserverwithip.gov") + self.assertContains(detail_page, "ns3.nameserverwithip.gov") + self.assertContains(detail_page, "(1.2.3.4, 2.3.4.5)") + class TestDomainUserManagement(TestDomainOverview): def test_domain_user_management(self): From 448cef084578e137210e6c681390d5b846a30ebb Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 24 Oct 2023 17:40:57 -0700 Subject: [PATCH 08/12] Remove a comment --- src/registrar/tests/test_views.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 81afe1182..4b9ac1dcb 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1448,8 +1448,6 @@ class TestDomainNameservers(TestDomainOverview): page = result.follow() self.assertContains(page, "The name servers for this domain have been updated") - # changed nameserver is reflected on domain overview page - @skip("Broken by adding registry connection fix in ticket 848") def test_domain_nameservers_form_invalid(self): """Can change domain's nameservers. From 1c99101c18788dfb2924492fbb39c7a0e64af030 Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Thu, 26 Oct 2023 12:16:01 -0700 Subject: [PATCH 09/12] Move nameserver formatting to view --- src/registrar/templates/domain_detail.html | 4 ++-- src/registrar/templates/includes/summary_item.html | 8 ++++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/registrar/templates/domain_detail.html b/src/registrar/templates/domain_detail.html index dfd583b1e..e220fe1aa 100644 --- a/src/registrar/templates/domain_detail.html +++ b/src/registrar/templates/domain_detail.html @@ -28,8 +28,8 @@
{% url 'domain-dns-nameservers' pk=domain.id as url %} - {% if domain.format_nameservers|length > 0 %} - {% include "includes/summary_item.html" with title='DNS name servers' value=domain.format_nameservers list='true' edit_link=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 %} {% 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.

diff --git a/src/registrar/templates/includes/summary_item.html b/src/registrar/templates/includes/summary_item.html index 6fcad0650..63eb62fb4 100644 --- a/src/registrar/templates/includes/summary_item.html +++ b/src/registrar/templates/includes/summary_item.html @@ -56,6 +56,14 @@ {% for item in value %} {% if users %}
  • {{ item.user.email }}
    • + {% elif domains %} +
  • {{ item.0 }} + {% if item.1 %} + ({% for addr in item.1 %} + {{addr}} + {% endfor %}) + {% endif %} +
    • {% else %}
  • {{ item }}
    • {% endif %} From eeaa4bfc27e50495170a1f418b438a5da0f4d815 Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Fri, 27 Oct 2023 09:46:25 -0700 Subject: [PATCH 10/12] Implement reformatting nameservers from model to views --- src/registrar/models/domain.py | 38 ------------------- .../templates/includes/summary_item.html | 11 ++++-- 2 files changed, 7 insertions(+), 42 deletions(-) diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 4ba2ae3d4..78aecb927 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -229,44 +229,6 @@ class Domain(TimeStampedModel, DomainHelper): """ raise NotImplementedError() - @Cache - def format_nameservers(self): - """ - Formatting nameservers for display for this domain. - - Hosts are provided as a list of tuples, e.g. - - [("ns1.example.com",), ("ns1.example.gov", ["0.0.0.0"])] - - We want to display as: - ns1.example.gov - ns1.example.gov (0.0.0.0) - - Subordinate hosts (something.your-domain.gov) MUST have IP addresses, - while non-subordinate hosts MUST NOT. - - - """ - try: - hosts = self._get_property("hosts") - except Exception as err: - # Do not raise error when missing nameservers - # this is a standard occurence when a domain - # is first created - logger.info("Domain is missing nameservers %s" % err) - return [] - - hostList = [] - for host in hosts: - # can remove str conversion when EPP.IP address display - # changes from IP object -> IP address - host_addrs_stringified = [str(addr) for addr in host["addrs"]] - host_addrs_str = f'({", ".join(obj for obj in host_addrs_stringified)})' - host_info_str = f'{host["name"]} \ - {host_addrs_str if len(host["addrs"]) > 0 else ""}' - hostList.append(host_info_str) - return hostList - @Cache def nameservers(self) -> list[tuple[str, list]]: """ diff --git a/src/registrar/templates/includes/summary_item.html b/src/registrar/templates/includes/summary_item.html index 63eb62fb4..f5b6c609d 100644 --- a/src/registrar/templates/includes/summary_item.html +++ b/src/registrar/templates/includes/summary_item.html @@ -57,11 +57,14 @@ {% if users %}
  • {{ item.user.email }}
    • {% elif domains %} -
  • {{ item.0 }} +
  • + {{ item.0 }} {% if item.1 %} - ({% for addr in item.1 %} - {{addr}} - {% endfor %}) + ({% spaceless %} + {% for addr in item.1 %} + {{addr}}{% if not forloop.last %}, {% endif %} + {% endfor %} + {% endspaceless %}) {% endif %}
    • {% else %} From e135de66fc3f531c641a822d591fc1ab23fa2df9 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Fri, 27 Oct 2023 11:25:58 -0700 Subject: [PATCH 11/12] Fix test bc of weird detail page whitespace --- src/registrar/tests/test_views.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 10b2adee2..0a472b885 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -1269,7 +1269,9 @@ class TestDomainOverview(TestWithDomainPermissions, WebTest): self.assertContains(detail_page, "ns1.nameserverwithip.gov") self.assertContains(detail_page, "ns2.nameserverwithip.gov") self.assertContains(detail_page, "ns3.nameserverwithip.gov") - self.assertContains(detail_page, "(1.2.3.4, 2.3.4.5)") + # Splitting IP addresses bc there is odd whitespace and can't strip text + self.assertContains(detail_page, "(1.2.3.4,") + self.assertContains(detail_page, "2.3.4.5)") class TestDomainManagers(TestDomainOverview): From f3547f24c070b3edf68614a96abb1270abc12b86 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Fri, 27 Oct 2023 11:32:29 -0700 Subject: [PATCH 12/12] Remove comment --- src/registrar/templates/includes/summary_item.html | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/templates/includes/summary_item.html b/src/registrar/templates/includes/summary_item.html index f5b6c609d..8a33bb1d5 100644 --- a/src/registrar/templates/includes/summary_item.html +++ b/src/registrar/templates/includes/summary_item.html @@ -43,7 +43,6 @@ {% else %} {% include "includes/contact.html" with contact=value %} {% endif %} - {% elif list %} {% if value|length == 1 %} {% if users %}