From 9e7f8785490fb0a5c5d0c82e1cdd06fb6127998d Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 12 Apr 2024 15:24:50 -0400 Subject: [PATCH 01/21] allow clanking of first and or second nameserver if enough entries --- src/registrar/forms/domain.py | 36 +++++++++++--- src/registrar/tests/test_views_domain.py | 62 ++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 7 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 7b0ac2956..5b70d3e9b 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -83,25 +83,34 @@ class DomainNameserverForm(forms.Form): # after clean_fields. it is used to determine form level errors. # is_valid is typically called from view during a post cleaned_data = super().clean() + self.clean_empty_strings(cleaned_data) + server = cleaned_data.get("server", "") - # remove ANY spaces in the server field - server = server.replace(" ", "") - # lowercase the server - server = server.lower() + server = server.replace(" ", "").lower() cleaned_data["server"] = server - ip = cleaned_data.get("ip", None) - # remove ANY spaces in the ip field + + ip = cleaned_data.get("ip", "") ip = ip.replace(" ", "") cleaned_data["ip"] = ip + domain = cleaned_data.get("domain", "") ip_list = self.extract_ip_list(ip) - # validate if the form has a server or an ip + # Capture the server_value + server_value = self.cleaned_data["server"] + + # Validate if the form has a server or an ip if (ip and ip_list) or server: self.validate_nameserver_ip_combo(domain, server, ip_list) + # Re-set the server value: + # add_error which is called on validate_nameserver_ip_combo will clean-up (delete) any invalid data. + # We need that data because we need to know the total server entries (even if invalid) in the formset + # clean method where we determine whether a blank first and/or second entry should throw a required error. + self.cleaned_data["server"] = server_value + return cleaned_data def clean_empty_strings(self, cleaned_data): @@ -149,6 +158,19 @@ class BaseNameserverFormset(forms.BaseFormSet): """ Check for duplicate entries in the formset. """ + + # Check if there are at least two valid servers + valid_servers_count = sum( + 1 for form in self.forms if form.cleaned_data.get("server") and form.cleaned_data.get("server").strip() + ) + if valid_servers_count >= 2: + # If there are, remove the "At least two name servers are required" error from each form + # This will allow for successful submissions when the first or second entries are blanked + # but there are enough entries total + for form in self.forms: + if form.errors.get("server") == ["At least two name servers are required."]: + form.errors.pop("server") + if any(self.errors): # Don't bother validating the formset unless each form is valid on its own return diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index 064c5efdb..ba229fc10 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -974,6 +974,68 @@ class TestDomainNameservers(TestDomainOverview): page = result.follow() self.assertContains(page, "The name servers for this domain have been updated") + def test_domain_nameservers_can_blank_out_first_or_second_one_if_enough_entries(self): + """Nameserver form submits successfully with 2 valid inputs, even if the first or + second entries are blanked out. + + Uses self.app WebTest because we need to interact with forms. + """ + + nameserver1 = "" + nameserver2 = "ns2.igorville.gov" + nameserver3 = "ns3.igorville.gov" + valid_ip = "" + valid_ip_2 = "128.0.0.2" + valid_ip_3 = "128.0.0.3" + nameservers_page = self.app.get(reverse("domain-dns-nameservers", kwargs={"pk": self.domain.id})) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + nameservers_page.form["form-0-server"] = nameserver1 + nameservers_page.form["form-0-ip"] = valid_ip + nameservers_page.form["form-1-server"] = nameserver2 + nameservers_page.form["form-1-ip"] = valid_ip_2 + nameservers_page.form["form-2-server"] = nameserver3 + nameservers_page.form["form-2-ip"] = valid_ip_3 + with less_console_noise(): # swallow log warning message + result = nameservers_page.form.submit() + + # form submission was a successful post, response should be a 302 + self.assertEqual(result.status_code, 302) + self.assertEqual( + result["Location"], + reverse("domain-dns-nameservers", kwargs={"pk": self.domain.id}), + ) + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + nameservers_page = result.follow() + self.assertContains(nameservers_page, "The name servers for this domain have been updated") + + nameserver1 = "ns1.igorville.gov" + nameserver2 = "" + nameserver3 = "ns3.igorville.gov" + valid_ip = "128.0.0.1" + valid_ip_2 = "" + valid_ip_3 = "128.0.0.3" + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + nameservers_page.form["form-0-server"] = nameserver1 + nameservers_page.form["form-0-ip"] = valid_ip + nameservers_page.form["form-1-server"] = nameserver2 + nameservers_page.form["form-1-ip"] = valid_ip_2 + nameservers_page.form["form-2-server"] = nameserver3 + nameservers_page.form["form-2-ip"] = valid_ip_3 + with less_console_noise(): # swallow log warning message + result = nameservers_page.form.submit() + + # form submission was a successful post, response should be a 302 + self.assertEqual(result.status_code, 302) + self.assertEqual( + result["Location"], + reverse("domain-dns-nameservers", kwargs={"pk": self.domain.id}), + ) + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + nameservers_page = result.follow() + self.assertContains(nameservers_page, "The name servers for this domain have been updated") + def test_domain_nameservers_form_invalid(self): """Nameserver form does not submit with invalid data. From 0a17f396cb534011518b12f57504bc0c5ea040a5 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 12 Apr 2024 15:44:40 -0400 Subject: [PATCH 02/21] UX bug fixes --- src/registrar/assets/js/get-gov.js | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 587b95305..b2ed71bea 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -530,7 +530,7 @@ function hideDeletedForms() { let isDotgovDomain = document.querySelector(".dotgov-domain-form"); // The Nameservers formset features 2 required and 11 optionals if (isNameserversForm) { - cloneIndex = 2; + // cloneIndex = 2; formLabel = "Name server"; // DNSSEC: DS Data } else if (isDsDataForm) { @@ -766,3 +766,21 @@ function toggleTwoDomElements(ele1, ele2, index) { } })(); +/** + * An IIFE that listens to the other contacts radio form on DAs and toggles the contacts/no other contacts forms + * + */ +(function otherContactsFormListener() { + let isNameserversForm = document.querySelector(".nameservers-form"); + if (isNameserversForm) { + let forms = document.querySelectorAll(".repeatable-form"); + if (forms.length < 3) { + // Hide the delete buttons on the 2 nameservers + forms.forEach((form, index) => { + Array.from(form.querySelectorAll('.delete-record')).forEach((deleteButton) => { + deleteButton.setAttribute("disabled", "true"); + }); + }); + } + } +})(); From 28177030210adbfd40d2ec25e9691c3e4b83fa95 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 12 Apr 2024 15:46:11 -0400 Subject: [PATCH 03/21] update IIFE definition --- src/registrar/assets/js/get-gov.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index b2ed71bea..f2771e51c 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -767,10 +767,10 @@ function toggleTwoDomElements(ele1, ele2, index) { })(); /** - * An IIFE that listens to the other contacts radio form on DAs and toggles the contacts/no other contacts forms + * An IIFE that disables the delete buttons on nameserver forms on page load if < 3 forms * */ -(function otherContactsFormListener() { +(function nameserversFormListener() { let isNameserversForm = document.querySelector(".nameservers-form"); if (isNameserversForm) { let forms = document.querySelectorAll(".repeatable-form"); From 85bc9c272ede537a52ff283a039dc1b349959223 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 12 Apr 2024 15:47:47 -0400 Subject: [PATCH 04/21] cleanup --- src/registrar/assets/js/get-gov.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index f2771e51c..b4c41ecf1 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -776,7 +776,7 @@ function toggleTwoDomElements(ele1, ele2, index) { let forms = document.querySelectorAll(".repeatable-form"); if (forms.length < 3) { // Hide the delete buttons on the 2 nameservers - forms.forEach((form, index) => { + forms.forEach((form) => { Array.from(form.querySelectorAll('.delete-record')).forEach((deleteButton) => { deleteButton.setAttribute("disabled", "true"); }); From 27ef823b93d7cd7d3c60e21ea859fca11e21b0fc Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 12 Apr 2024 17:56:57 -0400 Subject: [PATCH 05/21] handle edge case of 0 and 1 empty and duplicate error --- src/registrar/forms/domain.py | 4 +- src/registrar/tests/common.py | 13 ++++++ src/registrar/tests/test_views_domain.py | 55 +++++++++++++++++++++++- 3 files changed, 68 insertions(+), 4 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 5b70d3e9b..165d32fd8 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -178,10 +178,10 @@ class BaseNameserverFormset(forms.BaseFormSet): data = [] duplicates = [] - for form in self.forms: + for index, form in enumerate(self.forms): if form.cleaned_data: value = form.cleaned_data["server"] - if value in data: + if value in data and not (form.cleaned_data.get("server", "").strip() == '' and index == 1): form.add_error( "server", NameserverError(code=nsErrorCodes.DUPLICATE_HOST, nameserver=value), diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index a0b0e774f..4b9461b65 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -1152,6 +1152,18 @@ class MockEppLib(TestCase): ], ) + infoDomainFourHosts = fakedEppObject( + "my-nameserver.gov", + cr_date=make_aware(datetime(2023, 5, 25, 19, 45, 35)), + contacts=[], + hosts=[ + "ns1.my-nameserver-1.com", + "ns1.my-nameserver-2.com", + "ns1.cats-are-superior3.com", + "ns1.explosive-chicken-nuggets.com", + ], + ) + infoDomainNoHost = fakedEppObject( "my-nameserver.gov", cr_date=make_aware(datetime(2023, 5, 25, 19, 45, 35)), @@ -1475,6 +1487,7 @@ class MockEppLib(TestCase): "namerserversubdomain.gov": (self.infoDomainCheckHostIPCombo, None), "freeman.gov": (self.InfoDomainWithContacts, None), "threenameserversDomain.gov": (self.infoDomainThreeHosts, None), + "fournameserversDomain.gov": (self.infoDomainFourHosts, None), "defaultsecurity.gov": (self.InfoDomainWithDefaultSecurityContact, None), "adomain2.gov": (self.InfoDomainWithVerisignSecurityContact, None), "defaulttechnical.gov": (self.InfoDomainWithDefaultTechnicalContact, None), diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index ba229fc10..eeab00ae3 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -5,7 +5,7 @@ from django.conf import settings from django.urls import reverse from django.contrib.auth import get_user_model -from .common import MockSESClient, create_user # type: ignore +from .common import MockEppLib, MockSESClient, create_user # type: ignore from django_webtest import WebTest # type: ignore import boto3_mocking # type: ignore @@ -727,7 +727,7 @@ class TestDomainManagers(TestDomainOverview): self.assertContains(home_page, self.domain.name) -class TestDomainNameservers(TestDomainOverview): +class TestDomainNameservers(TestDomainOverview, MockEppLib): def test_domain_nameservers(self): """Can load domain's nameservers page.""" page = self.client.get(reverse("domain-dns-nameservers", kwargs={"pk": self.domain.id})) @@ -1036,6 +1036,57 @@ class TestDomainNameservers(TestDomainOverview): nameservers_page = result.follow() self.assertContains(nameservers_page, "The name servers for this domain have been updated") + @skip('wip') + def test_domain_nameservers_can_blank_out_first_and_second_one_if_enough_entries(self): + """Nameserver form submits successfully with 2 valid inputs, even if the first and + second entries are blanked out. + + Uses self.app WebTest because we need to interact with forms. + """ + + # Submit a formset with 3 valid forms + # The returned page (after the redirect) will have 4 forms that we can use to test + # our use case. + + + infoDomainFourHosts, _ = Domain.objects.get_or_create(name="fournameserversDomain.gov", state=Domain.State.READY) + UserDomainRole.objects.get_or_create(user=self.user, domain=infoDomainFourHosts) + DomainInformation.objects.get_or_create(creator=self.user, domain=infoDomainFourHosts) + self.client.force_login(self.user) + + nameserver1 = "" + nameserver2 = "" + nameserver3 = "ns3.igorville.gov" + nameserver4 = "ns4.igorville.gov" + valid_ip = "" + valid_ip_2 = "" + valid_ip_3 = "128.0.0.3" + valid_ip_4 = "128.0.0.4" + nameservers_page = self.app.get(reverse("domain-dns-nameservers", kwargs={"pk": infoDomainFourHosts.id})) + print(nameservers_page.content.decode('utf-8')) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + nameservers_page.form["form-0-server"] = nameserver1 + nameservers_page.form["form-0-ip"] = valid_ip + nameservers_page.form["form-1-server"] = nameserver2 + nameservers_page.form["form-1-ip"] = valid_ip_2 + nameservers_page.form["form-2-server"] = nameserver3 + nameservers_page.form["form-2-ip"] = valid_ip_3 + nameservers_page.form["form-3-server"] = nameserver4 + nameservers_page.form["form-3-ip"] = valid_ip_4 + with less_console_noise(): # swallow log warning message + result = nameservers_page.form.submit() + + # form submission was a successful post, response should be a 302 + self.assertEqual(result.status_code, 302) + self.assertEqual( + result["Location"], + reverse("domain-dns-nameservers", kwargs={"pk": self.domain.id}), + ) + self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + nameservers_page = result.follow() + self.assertContains(nameservers_page, "The name servers for this domain have been updated") + def test_domain_nameservers_form_invalid(self): """Nameserver form does not submit with invalid data. From d05f016632252e0d1e97b7c27fdfeb5e8bb5edb7 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 12 Apr 2024 18:00:06 -0400 Subject: [PATCH 06/21] comment --- src/registrar/forms/domain.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 165d32fd8..b7c37277c 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -181,6 +181,9 @@ class BaseNameserverFormset(forms.BaseFormSet): for index, form in enumerate(self.forms): if form.cleaned_data: value = form.cleaned_data["server"] + # We need to make sure not to trigger the duplicate error in case the first and second nameservers are empty + # If there are enough records in the formset, that error is an unecessary blocker. If there aren't, the required + # error will block the submit. if value in data and not (form.cleaned_data.get("server", "").strip() == '' and index == 1): form.add_error( "server", From 2e86c167b9692b0e0f846f016f9bf2684320ef29 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 12 Apr 2024 21:40:41 -0400 Subject: [PATCH 07/21] Fix unit test for 2 empties --- src/registrar/forms/domain.py | 8 ++--- src/registrar/tests/common.py | 10 ++++--- src/registrar/tests/test_views_domain.py | 38 ++++++++++++++---------- 3 files changed, 32 insertions(+), 24 deletions(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index b7c37277c..8fc7a6497 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -181,10 +181,10 @@ class BaseNameserverFormset(forms.BaseFormSet): for index, form in enumerate(self.forms): if form.cleaned_data: value = form.cleaned_data["server"] - # We need to make sure not to trigger the duplicate error in case the first and second nameservers are empty - # If there are enough records in the formset, that error is an unecessary blocker. If there aren't, the required - # error will block the submit. - if value in data and not (form.cleaned_data.get("server", "").strip() == '' and index == 1): + # We need to make sure not to trigger the duplicate error in case the first and second nameservers + # are empty. If there are enough records in the formset, that error is an unecessary blocker. + # If there aren't, the required error will block the submit. + if value in data and not (form.cleaned_data.get("server", "").strip() == "" and index == 1): form.add_error( "server", NameserverError(code=nsErrorCodes.DUPLICATE_HOST, nameserver=value), diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 4b9461b65..07dc08f8a 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -1153,7 +1153,7 @@ class MockEppLib(TestCase): ) infoDomainFourHosts = fakedEppObject( - "my-nameserver.gov", + "fournameserversDomain.gov", cr_date=make_aware(datetime(2023, 5, 25, 19, 45, 35)), contacts=[], hosts=[ @@ -1464,7 +1464,9 @@ class MockEppLib(TestCase): ) def mockInfoDomainCommands(self, _request, cleaned): - request_name = getattr(_request, "name", None) + request_name = getattr(_request, "name", None).lower() + + print(request_name) # Define a dictionary to map request names to data and extension values request_mappings = { @@ -1486,8 +1488,8 @@ class MockEppLib(TestCase): "nameserverwithip.gov": (self.infoDomainHasIP, None), "namerserversubdomain.gov": (self.infoDomainCheckHostIPCombo, None), "freeman.gov": (self.InfoDomainWithContacts, None), - "threenameserversDomain.gov": (self.infoDomainThreeHosts, None), - "fournameserversDomain.gov": (self.infoDomainFourHosts, None), + "threenameserversdomain.gov": (self.infoDomainThreeHosts, None), + "fournameserversdomain.gov": (self.infoDomainFourHosts, None), "defaultsecurity.gov": (self.InfoDomainWithDefaultSecurityContact, None), "adomain2.gov": (self.InfoDomainWithVerisignSecurityContact, None), "defaulttechnical.gov": (self.InfoDomainWithDefaultTechnicalContact, None), diff --git a/src/registrar/tests/test_views_domain.py b/src/registrar/tests/test_views_domain.py index eeab00ae3..3a5ce7e7b 100644 --- a/src/registrar/tests/test_views_domain.py +++ b/src/registrar/tests/test_views_domain.py @@ -71,11 +71,14 @@ class TestWithDomainPermissions(TestWithUser): # that inherit this setUp self.domain_dnssec_none, _ = Domain.objects.get_or_create(name="dnssec-none.gov") + self.domain_with_four_nameservers, _ = Domain.objects.get_or_create(name="fournameserversDomain.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) DomainInformation.objects.get_or_create(creator=self.user, domain=self.domain_multdsdata) 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_four_nameservers) 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) @@ -98,6 +101,11 @@ class TestWithDomainPermissions(TestWithUser): domain=self.domain_dnssec_none, role=UserDomainRole.Roles.MANAGER, ) + UserDomainRole.objects.get_or_create( + user=self.user, + domain=self.domain_with_four_nameservers, + role=UserDomainRole.Roles.MANAGER, + ) UserDomainRole.objects.get_or_create( user=self.user, domain=self.domain_with_ip, @@ -1036,7 +1044,6 @@ class TestDomainNameservers(TestDomainOverview, MockEppLib): nameservers_page = result.follow() self.assertContains(nameservers_page, "The name servers for this domain have been updated") - @skip('wip') def test_domain_nameservers_can_blank_out_first_and_second_one_if_enough_entries(self): """Nameserver form submits successfully with 2 valid inputs, even if the first and second entries are blanked out. @@ -1044,28 +1051,27 @@ class TestDomainNameservers(TestDomainOverview, MockEppLib): Uses self.app WebTest because we need to interact with forms. """ - # Submit a formset with 3 valid forms - # The returned page (after the redirect) will have 4 forms that we can use to test - # our use case. - - - infoDomainFourHosts, _ = Domain.objects.get_or_create(name="fournameserversDomain.gov", state=Domain.State.READY) - UserDomainRole.objects.get_or_create(user=self.user, domain=infoDomainFourHosts) - DomainInformation.objects.get_or_create(creator=self.user, domain=infoDomainFourHosts) - self.client.force_login(self.user) - + # We need to start with a domain with 4 nameservers otherwise the formset in the test environment + # will only have 3 forms nameserver1 = "" nameserver2 = "" nameserver3 = "ns3.igorville.gov" nameserver4 = "ns4.igorville.gov" valid_ip = "" valid_ip_2 = "" - valid_ip_3 = "128.0.0.3" - valid_ip_4 = "128.0.0.4" - nameservers_page = self.app.get(reverse("domain-dns-nameservers", kwargs={"pk": infoDomainFourHosts.id})) - print(nameservers_page.content.decode('utf-8')) + valid_ip_3 = "" + valid_ip_4 = "" + nameservers_page = self.app.get( + reverse("domain-dns-nameservers", kwargs={"pk": self.domain_with_four_nameservers.id}) + ) + session_id = self.app.cookies[settings.SESSION_COOKIE_NAME] self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) + + # Minimal check to ensure the form is loaded correctly + self.assertEqual(nameservers_page.form["form-0-server"].value, "ns1.my-nameserver-1.com") + self.assertEqual(nameservers_page.form["form-3-server"].value, "ns1.explosive-chicken-nuggets.com") + nameservers_page.form["form-0-server"] = nameserver1 nameservers_page.form["form-0-ip"] = valid_ip nameservers_page.form["form-1-server"] = nameserver2 @@ -1081,7 +1087,7 @@ class TestDomainNameservers(TestDomainOverview, MockEppLib): self.assertEqual(result.status_code, 302) self.assertEqual( result["Location"], - reverse("domain-dns-nameservers", kwargs={"pk": self.domain.id}), + reverse("domain-dns-nameservers", kwargs={"pk": self.domain_with_four_nameservers.id}), ) self.app.set_cookie(settings.SESSION_COOKIE_NAME, session_id) nameservers_page = result.follow() From ffa7b59eb586d433d0aaa72a14be5e39e661949f Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Fri, 12 Apr 2024 21:52:16 -0400 Subject: [PATCH 08/21] cleanup --- src/registrar/forms/domain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/forms/domain.py b/src/registrar/forms/domain.py index 8fc7a6497..da1462bdb 100644 --- a/src/registrar/forms/domain.py +++ b/src/registrar/forms/domain.py @@ -99,7 +99,7 @@ class DomainNameserverForm(forms.Form): ip_list = self.extract_ip_list(ip) # Capture the server_value - server_value = self.cleaned_data["server"] + server_value = self.cleaned_data.get("server") # Validate if the form has a server or an ip if (ip and ip_list) or server: From 2ffeda3be7340e9294f9ece9f0cd2d3892a19281 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 15 Apr 2024 07:46:07 -0400 Subject: [PATCH 09/21] added column for Is user to Contact table in DJA --- src/registrar/admin.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 05bfc06b6..5116888dd 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -663,6 +663,7 @@ class ContactAdmin(ListHeaderAdmin): list_display = [ "contact", "email", + "user_exists", ] # this ordering effects the ordering of results # in autocomplete_fields for user @@ -679,6 +680,12 @@ class ContactAdmin(ListHeaderAdmin): change_form_template = "django/admin/email_clipboard_change_form.html" + def user_exists(self, obj): + """Check if the Contact has a related User""" + return obj.user is not None + user_exists.boolean = True + user_exists.short_description = "Is user" + # We name the custom prop 'contact' because linter # is not allowing a short_description attr on it # This gets around the linter limitation, for now. From 3909ba8212fdb9c7643af59f25d43d45c58defee Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 15 Apr 2024 07:53:24 -0400 Subject: [PATCH 10/21] formatted for readability --- src/registrar/admin.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 5116888dd..b9114f6f5 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -683,6 +683,7 @@ class ContactAdmin(ListHeaderAdmin): def user_exists(self, obj): """Check if the Contact has a related User""" return obj.user is not None + user_exists.boolean = True user_exists.short_description = "Is user" From 77d2c998d3d62d15fdab6c581ef9ad049d7cb4b4 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 15 Apr 2024 07:57:34 -0400 Subject: [PATCH 11/21] satisfying linter --- src/registrar/admin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index b9114f6f5..fe0730d31 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -684,8 +684,8 @@ class ContactAdmin(ListHeaderAdmin): """Check if the Contact has a related User""" return obj.user is not None - user_exists.boolean = True - user_exists.short_description = "Is user" + user_exists.boolean = True # type: ignore + user_exists.short_description = "Is user" # type: ignore # We name the custom prop 'contact' because linter # is not allowing a short_description attr on it From 5673d5951ef197ee6ef8edd2c4811bf16cecf605 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 15 Apr 2024 08:50:13 -0400 Subject: [PATCH 12/21] default filter applied when clicking domain requests from editing a domain request --- src/registrar/admin.py | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 05bfc06b6..efd8c29c9 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1435,12 +1435,35 @@ class DomainRequestAdmin(ListHeaderAdmin): """ Override changelist_view to set the selected value of status filter. """ + # there are two conditions which should set the default selected filter: + # 1 - there are no query parameters in the request and the request is the + # initial request for this view + # 2 - there are no query parameters in the request and the referring url is + # the change view for a domain request + should_apply_default_filter = False # use http_referer in order to distinguish between request as a link from another page # and request as a removal of all filters http_referer = request.META.get("HTTP_REFERER", "") # if there are no query parameters in the request - # and the request is the initial request for this view - if not bool(request.GET) and request.path not in http_referer: + if not bool(request.GET): + # if the request is the initial request for this view + if request.path not in http_referer: + should_apply_default_filter = True + # elif the request is a referral from changelist view or from + # domain request change view + elif request.path in http_referer: + # find the index to determine the referring url after the path + index = http_referer.find(request.path) + # Check if there is a character following the path in http_referer + if index + len(request.path) < len(http_referer): + next_char = http_referer[index + len(request.path)] + + # Check if the next character is a digit, if so, this indicates + # a change view for domain request + if next_char.isdigit(): + should_apply_default_filter = True + + if should_apply_default_filter: # modify the GET of the request to set the selected filter modified_get = copy.deepcopy(request.GET) modified_get["status__in"] = "submitted,in review,action needed" From 69c665e4fa893e6cb60f580ca1d32a3b49370ead Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 16 Apr 2024 11:12:17 -0400 Subject: [PATCH 13/21] made is_user column sortable --- src/registrar/admin.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index fe0730d31..f17aaf1b2 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -686,6 +686,7 @@ class ContactAdmin(ListHeaderAdmin): user_exists.boolean = True # type: ignore user_exists.short_description = "Is user" # type: ignore + user_exists.admin_order_field = 'user' # type: ignore # We name the custom prop 'contact' because linter # is not allowing a short_description attr on it From 72a816acdafc1f2f0e5f875d62dd8ac97fe2105d Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 16 Apr 2024 11:13:17 -0400 Subject: [PATCH 14/21] reformatted --- src/registrar/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index f17aaf1b2..17486e983 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -686,7 +686,7 @@ class ContactAdmin(ListHeaderAdmin): user_exists.boolean = True # type: ignore user_exists.short_description = "Is user" # type: ignore - user_exists.admin_order_field = 'user' # type: ignore + user_exists.admin_order_field = "user" # type: ignore # We name the custom prop 'contact' because linter # is not allowing a short_description attr on it From ee961c9cd801c876e950bdca5bb4d7c90deb4b5f Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 16 Apr 2024 12:52:48 -0400 Subject: [PATCH 15/21] changed icon to text for column --- src/registrar/admin.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 17486e983..b61b21f87 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -682,9 +682,8 @@ class ContactAdmin(ListHeaderAdmin): def user_exists(self, obj): """Check if the Contact has a related User""" - return obj.user is not None + return "Yes" if obj.user is not None else "No" - user_exists.boolean = True # type: ignore user_exists.short_description = "Is user" # type: ignore user_exists.admin_order_field = "user" # type: ignore From c1c428b6c3025fbf87f84cee049a3c15e7facf14 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 17 Apr 2024 12:40:27 -0400 Subject: [PATCH 16/21] updated code for readability --- src/registrar/admin.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index efd8c29c9..a0d2a0b6b 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1455,8 +1455,9 @@ class DomainRequestAdmin(ListHeaderAdmin): # find the index to determine the referring url after the path index = http_referer.find(request.path) # Check if there is a character following the path in http_referer - if index + len(request.path) < len(http_referer): - next_char = http_referer[index + len(request.path)] + next_char_index = index + len(request.path) + if index + next_char_index < len(http_referer): + next_char = http_referer[next_char_index] # Check if the next character is a digit, if so, this indicates # a change view for domain request From c0b95f22f9a5f65680f896386a09bffd0a50f251 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 17 Apr 2024 09:52:18 -0700 Subject: [PATCH 17/21] Add documentation on how to access the db and roll back a migration --- docs/developer/database-access.md | 7 +++++++ docs/developer/migration-troubleshooting.md | 15 +++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/docs/developer/database-access.md b/docs/developer/database-access.md index 859ef2fd6..e13f970b3 100644 --- a/docs/developer/database-access.md +++ b/docs/developer/database-access.md @@ -56,6 +56,13 @@ cf ssh getgov-ENVIRONMENT ./manage.py dumpdata ``` +## Access certain table in the database +1. `cf connect-to-service getgov-ENVIRONMENT getgov-ENVIRONMENT-database` gets you into whichever environments database you'd like +2. `\c [table name here that starts cgaws];` connects to the [cgaws...etc] table +3. `\dt` retrieves information about that table and displays it +4. Make sure the table you are looking for exists. For this example, we are looking for `django_migrations` +5. Run `SELECT * FROM django_migrations` to see everything that's in it! + ## Dropping and re-creating the database For your sandbox environment, it might be necessary to start the database over from scratch. diff --git a/docs/developer/migration-troubleshooting.md b/docs/developer/migration-troubleshooting.md index b90c02ae3..e2208f860 100644 --- a/docs/developer/migration-troubleshooting.md +++ b/docs/developer/migration-troubleshooting.md @@ -121,3 +121,18 @@ https://cisa-corp.slack.com/archives/C05BGB4L5NF/p1697810600723069 2. `./manage.py migrate model_name_here file_name_WITH_create` (run the last data creation migration AND ONLY THAT ONE) 3. `./manage.py migrate --fake model_name_here most_recent_file_name` (fake migrate the last migration in the migration list) 4. `./manage.py load` (rerun fixtures) + +### Scenario 9: Inconsistent Migration History +If you see `django.db.migrations.exceptions.InconsistentMigrationHistory` error, or when you run `./manage.py showmigrations` it looks like: + +[x] 0056_example_migration +[ ] 0057_other_migration +[x] 0058_some_other_migration + +1. Go to `database-access.md` to see the commands on how to access a certain table in the database. +2. In this case, we want to remove the migration "history" from the `django_migrations` table +3. Once you are in the `cgaws....` table, select the `django_migrations` table with the command `SELECT * FROM djangomigrations;` +4. Find the id of the "history" you want to delete. In this example, the id would be 58. +5. Run `DELETE FROM django_migrations WHERE id=58;` where 58 is an example id as seen above. +6. Go to your shell and run `./manage.py showmigrations` to make sure your migrations are now back to the right state + From 1bc4f002a9d7a1744e60150479f931bc4d554014 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 17 Apr 2024 09:58:01 -0700 Subject: [PATCH 18/21] Fix table name and add in semicolon for statement --- docs/developer/database-access.md | 4 ++-- docs/developer/migration-troubleshooting.md | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/developer/database-access.md b/docs/developer/database-access.md index e13f970b3..f77261bbb 100644 --- a/docs/developer/database-access.md +++ b/docs/developer/database-access.md @@ -58,10 +58,10 @@ cf ssh getgov-ENVIRONMENT ## Access certain table in the database 1. `cf connect-to-service getgov-ENVIRONMENT getgov-ENVIRONMENT-database` gets you into whichever environments database you'd like -2. `\c [table name here that starts cgaws];` connects to the [cgaws...etc] table +2. `\c [table name here that starts cgaws...etc];` connects to the [cgaws...etc] table 3. `\dt` retrieves information about that table and displays it 4. Make sure the table you are looking for exists. For this example, we are looking for `django_migrations` -5. Run `SELECT * FROM django_migrations` to see everything that's in it! +5. Run `SELECT * FROM django_migrations;` to see everything that's in it! ## Dropping and re-creating the database diff --git a/docs/developer/migration-troubleshooting.md b/docs/developer/migration-troubleshooting.md index e2208f860..51422d4c4 100644 --- a/docs/developer/migration-troubleshooting.md +++ b/docs/developer/migration-troubleshooting.md @@ -131,7 +131,7 @@ If you see `django.db.migrations.exceptions.InconsistentMigrationHistory` error, 1. Go to `database-access.md` to see the commands on how to access a certain table in the database. 2. In this case, we want to remove the migration "history" from the `django_migrations` table -3. Once you are in the `cgaws....` table, select the `django_migrations` table with the command `SELECT * FROM djangomigrations;` +3. Once you are in the `cgaws....` table, select the `django_migrations` table with the command `SELECT * FROM django_migrations;` 4. Find the id of the "history" you want to delete. In this example, the id would be 58. 5. Run `DELETE FROM django_migrations WHERE id=58;` where 58 is an example id as seen above. 6. Go to your shell and run `./manage.py showmigrations` to make sure your migrations are now back to the right state From e5105d076d1bccc63fbd14e6f3eaa2dcfb2b1468 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 17 Apr 2024 10:12:25 -0700 Subject: [PATCH 19/21] Point to the exact point in db access --- docs/developer/migration-troubleshooting.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/developer/migration-troubleshooting.md b/docs/developer/migration-troubleshooting.md index 51422d4c4..b2b8f8662 100644 --- a/docs/developer/migration-troubleshooting.md +++ b/docs/developer/migration-troubleshooting.md @@ -129,7 +129,7 @@ If you see `django.db.migrations.exceptions.InconsistentMigrationHistory` error, [ ] 0057_other_migration [x] 0058_some_other_migration -1. Go to `database-access.md` to see the commands on how to access a certain table in the database. +1. Go to [database-access.md](../database-access.md#access-certain-table-in-the-database) to see the commands on how to access a certain table in the database. 2. In this case, we want to remove the migration "history" from the `django_migrations` table 3. Once you are in the `cgaws....` table, select the `django_migrations` table with the command `SELECT * FROM django_migrations;` 4. Find the id of the "history" you want to delete. In this example, the id would be 58. From 3988ffd653bb29bbcf1245db433f35768d5b57fb Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 17 Apr 2024 10:18:15 -0700 Subject: [PATCH 20/21] Address feedback of wording --- docs/developer/migration-troubleshooting.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/developer/migration-troubleshooting.md b/docs/developer/migration-troubleshooting.md index b2b8f8662..f096a876a 100644 --- a/docs/developer/migration-troubleshooting.md +++ b/docs/developer/migration-troubleshooting.md @@ -132,7 +132,8 @@ If you see `django.db.migrations.exceptions.InconsistentMigrationHistory` error, 1. Go to [database-access.md](../database-access.md#access-certain-table-in-the-database) to see the commands on how to access a certain table in the database. 2. In this case, we want to remove the migration "history" from the `django_migrations` table 3. Once you are in the `cgaws....` table, select the `django_migrations` table with the command `SELECT * FROM django_migrations;` -4. Find the id of the "history" you want to delete. In this example, the id would be 58. -5. Run `DELETE FROM django_migrations WHERE id=58;` where 58 is an example id as seen above. -6. Go to your shell and run `./manage.py showmigrations` to make sure your migrations are now back to the right state +4. Find the id of the "history" you want to delete. This will be the one in the far left column. For this example, let's pretend the id is 101. +5. Run `DELETE FROM django_migrations WHERE id=101;` where 101 is an example id as seen above. +6. Go to your shell and run `./manage.py showmigrations` to make sure your migrations are now back to the right state. Most likely you will show several unapplied migrations. +7. If you still have unapplied migrations, run `./manage.py migrate`. If an error occurs saying one has already been applied, fake that particular migration `./manage.py migrate --fake model_name_here migration_number` and then run the normal `./manage.py migrate` command to then apply those migrations that come after the one that threw the error. From 436b87bc9654309de57ab840dcba2505f2a990a6 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 17 Apr 2024 10:20:55 -0700 Subject: [PATCH 21/21] Fix wording --- docs/developer/migration-troubleshooting.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/developer/migration-troubleshooting.md b/docs/developer/migration-troubleshooting.md index f096a876a..22a02503d 100644 --- a/docs/developer/migration-troubleshooting.md +++ b/docs/developer/migration-troubleshooting.md @@ -131,9 +131,9 @@ If you see `django.db.migrations.exceptions.InconsistentMigrationHistory` error, 1. Go to [database-access.md](../database-access.md#access-certain-table-in-the-database) to see the commands on how to access a certain table in the database. 2. In this case, we want to remove the migration "history" from the `django_migrations` table -3. Once you are in the `cgaws....` table, select the `django_migrations` table with the command `SELECT * FROM django_migrations;` +3. Once you are in the `cgaws...` table, select the `django_migrations` table with the command `SELECT * FROM django_migrations;` 4. Find the id of the "history" you want to delete. This will be the one in the far left column. For this example, let's pretend the id is 101. 5. Run `DELETE FROM django_migrations WHERE id=101;` where 101 is an example id as seen above. -6. Go to your shell and run `./manage.py showmigrations` to make sure your migrations are now back to the right state. Most likely you will show several unapplied migrations. +6. Go to your shell and run `./manage.py showmigrations` to make sure your migrations are now back to the right state. Most likely you will see several unapplied migrations. 7. If you still have unapplied migrations, run `./manage.py migrate`. If an error occurs saying one has already been applied, fake that particular migration `./manage.py migrate --fake model_name_here migration_number` and then run the normal `./manage.py migrate` command to then apply those migrations that come after the one that threw the error.