From ff44ee2f1ef132b8f6a7ba4b85968ee4445aaeda Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Fri, 20 Oct 2023 12:28:47 -0700 Subject: [PATCH 01/14] Fix logic of availability API --- src/api/tests/test_available.py | 47 ++++++++++--------- src/api/views.py | 14 +++--- src/registrar/models/utility/domain_helper.py | 4 +- src/registrar/templates/domain_detail.html | 2 +- src/registrar/tests/common.py | 8 ++-- 5 files changed, 39 insertions(+), 36 deletions(-) diff --git a/src/api/tests/test_available.py b/src/api/tests/test_available.py index 9eab17bf7..52d0d962c 100644 --- a/src/api/tests/test_available.py +++ b/src/api/tests/test_available.py @@ -5,7 +5,7 @@ import json from django.contrib.auth import get_user_model from django.test import RequestFactory -from ..views import available, in_domains +from ..views import available, check_domain_available from .common import less_console_noise from registrar.tests.common import MockEppLib from unittest.mock import call @@ -37,10 +37,10 @@ class AvailableViewTest(MockEppLib): response_object = json.loads(response.content) self.assertIn("available", response_object) - def test_in_domains_makes_calls_(self): + def test_domain_available_makes_calls_(self): """Domain searches successfully make correct mock EPP calls""" - gsa_available = in_domains("gsa.gov") - igorville_available = in_domains("igorvilleremixed.gov") + gsa_available = check_domain_available("gsa.gov") + igorville_available = check_domain_available("igorville.gov") """Domain searches successfully make mock EPP calls""" self.mockedSendFunction.assert_has_calls( @@ -53,29 +53,32 @@ class AvailableViewTest(MockEppLib): ), call( commands.CheckDomain( - ["igorvilleremixed.gov"], + ["igorville.gov"], ), cleaned=True, ), ] ) """Domain searches return correct availability results""" - self.assertTrue(gsa_available) - self.assertFalse(igorville_available) + self.assertFalse(gsa_available) + self.assertTrue(igorville_available) - def test_in_domains_capitalized(self): + def test_domain_available_capitalized(self): """Domain searches work without case sensitivity""" - self.assertTrue(in_domains("gsa.gov")) - # input is lowercased so GSA.GOV should be found - self.assertTrue(in_domains("GSA.gov")) + self.assertFalse(check_domain_available("gsa.gov")) + self.assertTrue(check_domain_available("igorville.gov")) + # input is lowercased so GSA.GOV should also not be available + self.assertFalse(check_domain_available("GSA.gov")) + # input is lowercased so IGORVILLE.GOV should also not be available + self.assertFalse(check_domain_available("IGORVILLE.gov")) - def test_in_domains_dotgov(self): + def test_domain_available_dotgov(self): """Domain searches work without trailing .gov""" - self.assertTrue(in_domains("gsa")) + self.assertFalse(check_domain_available("gsa")) # input is lowercased so GSA.GOV should be found - self.assertTrue(in_domains("GSA")) - # This domain should not have been registered - self.assertFalse(in_domains("igorvilleremixed")) + self.assertFalse(check_domain_available("GSA")) + # This domain should be available to register + self.assertTrue(check_domain_available("igorville")) def test_not_available_domain(self): """gsa.gov is not available""" @@ -85,17 +88,17 @@ class AvailableViewTest(MockEppLib): self.assertFalse(json.loads(response.content)["available"]) def test_available_domain(self): - """igorvilleremixed.gov is still available""" - request = self.factory.get(API_BASE_PATH + "igorvilleremixed.gov") + """igorville.gov is still available""" + request = self.factory.get(API_BASE_PATH + "igorville.gov") request.user = self.user - response = available(request, domain="igorvilleremixed.gov") + response = available(request, domain="igorville.gov") self.assertTrue(json.loads(response.content)["available"]) def test_available_domain_dotgov(self): - """igorvilleremixed.gov is still available even without the .gov suffix""" - request = self.factory.get(API_BASE_PATH + "igorvilleremixed") + """igorville.gov is still available even without the .gov suffix""" + request = self.factory.get(API_BASE_PATH + "igorville") request.user = self.user - response = available(request, domain="igorvilleremixed") + response = available(request, domain="igorville") self.assertTrue(json.loads(response.content)["available"]) def test_error_handling(self): diff --git a/src/api/views.py b/src/api/views.py index e8b8431de..87607a71a 100644 --- a/src/api/views.py +++ b/src/api/views.py @@ -50,8 +50,8 @@ def _domains(): return domains -def in_domains(domain): - """Return true if the given domain is in the domains list. +def check_domain_available(domain): + """Return true if the given domain is available. The given domain is lowercased to match against the domains list. If the given domain doesn't end with .gov, ".gov" is added when looking for @@ -83,11 +83,11 @@ def available(request, domain=""): {"available": False, "message": DOMAIN_API_MESSAGES["invalid"]} ) # a domain is available if it is NOT in the list of current domains - if in_domains(domain): - return JsonResponse( - {"available": False, "message": DOMAIN_API_MESSAGES["unavailable"]} - ) - else: + if check_domain_available(domain): return JsonResponse( {"available": True, "message": DOMAIN_API_MESSAGES["success"]} ) + else: + return JsonResponse( + {"available": False, "message": DOMAIN_API_MESSAGES["unavailable"]} + ) diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index 8f5737915..1a77e44b1 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -1,6 +1,6 @@ import re -from api.views import in_domains +from api.views import check_domain_available from registrar.utility import errors @@ -44,7 +44,7 @@ class DomainHelper: raise errors.ExtraDotsError() if not DomainHelper.string_could_be_domain(domain + ".gov"): raise ValueError() - if in_domains(domain): + if not check_domain_available(domain): raise errors.DomainUnavailableError() return domain diff --git a/src/registrar/templates/domain_detail.html b/src/registrar/templates/domain_detail.html index e0d672093..bb137a91f 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' value=domain.nameservers list='true' edit_link=url %} + {% include "includes/summary_item.html" with title='DNS name servers' value=domain.nameservers list='true' edit_link=url only %} {% else %}
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/tests/common.py b/src/registrar/tests/common.py index 8cd5fd6ba..a2370a20d 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -834,11 +834,11 @@ class MockEppLib(TestCase): def mockCheckDomainCommand(self, _request, cleaned): if "gsa.gov" in getattr(_request, "names", None): - return self._mockDomainName("gsa.gov", True) + return self._mockDomainName("gsa.gov", False) elif "GSA.gov" in getattr(_request, "names", None): - return self._mockDomainName("GSA.gov", True) - elif "igorvilleremixed.gov" in getattr(_request, "names", None): - return self._mockDomainName("igorvilleremixed.gov", False) + return self._mockDomainName("GSA.gov", False) + elif "igorville.gov" in getattr(_request, "names", None): + return self._mockDomainName("igorvilleremixed.gov", True) elif "errordomain.gov" in getattr(_request, "names", None): raise RegistryError("Registry cannot find domain availability.") else: From 9c7bbd31ec0e588cbd55407f1a5a64da42f33632 Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Fri, 20 Oct 2023 15:03:07 -0700 Subject: [PATCH 02/14] Make login not required to access available API --- src/api/views.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/api/views.py b/src/api/views.py index 87607a71a..a80aff643 100644 --- a/src/api/views.py +++ b/src/api/views.py @@ -5,6 +5,8 @@ from django.http import JsonResponse import requests +from login_required import login_not_required + from cachetools.func import ttl_cache @@ -58,14 +60,18 @@ def check_domain_available(domain): a match. """ Domain = apps.get_model("registrar.Domain") - if domain.endswith(".gov"): - return Domain.available(domain) - else: - # domain search string doesn't end with .gov, add it on here - return Domain.available(domain + ".gov") + try: + if domain.endswith(".gov"): + return Domain.available(domain) + else: + # domain search string doesn't end with .gov, add it on here + return Domain.available(domain + ".gov") + except: + return False @require_http_methods(["GET"]) +@login_not_required def available(request, domain=""): """Is a given domain available or not. From c75891ee9077043807b676fa8ccc6712bfa9559f Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Fri, 20 Oct 2023 15:57:27 -0700 Subject: [PATCH 03/14] Modify tests for availability API to return false on error --- src/api/tests/test_available.py | 7 +++---- src/api/views.py | 19 +++++++++++-------- src/registrar/tests/test_url_auth.py | 1 + 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/api/tests/test_available.py b/src/api/tests/test_available.py index 52d0d962c..c7253fa35 100644 --- a/src/api/tests/test_available.py +++ b/src/api/tests/test_available.py @@ -108,10 +108,9 @@ class AvailableViewTest(MockEppLib): request.user = self.user response = available(request, domain=bad_string) self.assertFalse(json.loads(response.content)["available"]) - # domain set to raise error successfully raises error - with self.assertRaises(RegistryError): - error_domain_available = available(request, "errordomain.gov") - self.assertFalse(json.loads(error_domain_available.content)["available"]) + # domain set to raise error returns false for availability + error_domain_available = available(request, "errordomain.gov") + self.assertFalse(json.loads(error_domain_available.content)["available"]) class AvailableAPITest(MockEppLib): diff --git a/src/api/views.py b/src/api/views.py index a80aff643..94a5ea9f9 100644 --- a/src/api/views.py +++ b/src/api/views.py @@ -89,11 +89,14 @@ def available(request, domain=""): {"available": False, "message": DOMAIN_API_MESSAGES["invalid"]} ) # a domain is available if it is NOT in the list of current domains - if check_domain_available(domain): - return JsonResponse( - {"available": True, "message": DOMAIN_API_MESSAGES["success"]} - ) - else: - return JsonResponse( - {"available": False, "message": DOMAIN_API_MESSAGES["unavailable"]} - ) + try: + if check_domain_available(domain): + return JsonResponse( + {"available": True, "message": DOMAIN_API_MESSAGES["success"]} + ) + else: + return JsonResponse( + {"available": False, "message": DOMAIN_API_MESSAGES["unavailable"]} + ) + except: + raise RegistryError("Registry cannot find domain availability.") diff --git a/src/registrar/tests/test_url_auth.py b/src/registrar/tests/test_url_auth.py index 17ad2c329..2a04e8030 100644 --- a/src/registrar/tests/test_url_auth.py +++ b/src/registrar/tests/test_url_auth.py @@ -116,6 +116,7 @@ class TestURLAuth(TestCase): "/openid/callback", "/openid/callback/login/", "/openid/callback/logout/", + "/api/v1/available" ] def assertURLIsProtectedByAuth(self, url): From 15b16583cd8a6e894dbfa069d829dc7677d9fffa Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Fri, 20 Oct 2023 16:10:15 -0700 Subject: [PATCH 04/14] Add mocked responses for test_forms cases --- src/api/views.py | 5 ++++- src/registrar/tests/common.py | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/api/views.py b/src/api/views.py index 94a5ea9f9..158280b11 100644 --- a/src/api/views.py +++ b/src/api/views.py @@ -25,6 +25,7 @@ DOMAIN_API_MESSAGES = { "invalid": "Enter a domain using only letters," " numbers, or hyphens (though we don't recommend using hyphens).", "success": "That domain is available!", + "error": "Error finding domain availability." } @@ -99,4 +100,6 @@ def available(request, domain=""): {"available": False, "message": DOMAIN_API_MESSAGES["unavailable"]} ) except: - raise RegistryError("Registry cannot find domain availability.") + return JsonResponse( + {"available": False, "message": DOMAIN_API_MESSAGES["error"]} + ) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index a2370a20d..53d15a91c 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -839,6 +839,8 @@ class MockEppLib(TestCase): return self._mockDomainName("GSA.gov", False) elif "igorville.gov" in getattr(_request, "names", None): return self._mockDomainName("igorvilleremixed.gov", True) + elif "top-level-agency.gov" in getattr(_request, "names", None): + return self._mockDomainName("top-level-agency.gov", True) elif "errordomain.gov" in getattr(_request, "names", None): raise RegistryError("Registry cannot find domain availability.") else: From a30ed4725ad528d9789234a438a61b55b91e1cff Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Fri, 20 Oct 2023 16:15:03 -0700 Subject: [PATCH 05/14] Fix linting errors --- src/api/tests/test_available.py | 1 - src/api/views.py | 10 +++++----- src/registrar/tests/test_url_auth.py | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/api/tests/test_available.py b/src/api/tests/test_available.py index c7253fa35..524fd689a 100644 --- a/src/api/tests/test_available.py +++ b/src/api/tests/test_available.py @@ -12,7 +12,6 @@ from unittest.mock import call from epplibwrapper import ( commands, - RegistryError, ) API_BASE_PATH = "/api/v1/available/" diff --git a/src/api/views.py b/src/api/views.py index 158280b11..694bea349 100644 --- a/src/api/views.py +++ b/src/api/views.py @@ -25,7 +25,7 @@ DOMAIN_API_MESSAGES = { "invalid": "Enter a domain using only letters," " numbers, or hyphens (though we don't recommend using hyphens).", "success": "That domain is available!", - "error": "Error finding domain availability." + "error": "Error finding domain availability.", } @@ -67,7 +67,7 @@ def check_domain_available(domain): else: # domain search string doesn't end with .gov, add it on here return Domain.available(domain + ".gov") - except: + except Exception: return False @@ -99,7 +99,7 @@ def available(request, domain=""): return JsonResponse( {"available": False, "message": DOMAIN_API_MESSAGES["unavailable"]} ) - except: + except Exception: return JsonResponse( - {"available": False, "message": DOMAIN_API_MESSAGES["error"]} - ) + {"available": False, "message": DOMAIN_API_MESSAGES["error"]} + ) diff --git a/src/registrar/tests/test_url_auth.py b/src/registrar/tests/test_url_auth.py index 2a04e8030..32fc78ca1 100644 --- a/src/registrar/tests/test_url_auth.py +++ b/src/registrar/tests/test_url_auth.py @@ -116,7 +116,7 @@ class TestURLAuth(TestCase): "/openid/callback", "/openid/callback/login/", "/openid/callback/logout/", - "/api/v1/available" + "/api/v1/available", ] def assertURLIsProtectedByAuth(self, url): From 5c9fbf483eb4b3064ed4574952d66af4413539e6 Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Mon, 23 Oct 2023 09:50:35 -0700 Subject: [PATCH 06/14] Add url to ignore_urls list --- src/registrar/templates/domain_detail.html | 2 +- src/registrar/tests/test_url_auth.py | 4 ++-- src/registrar/tests/test_views.py | 3 +++ 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/registrar/templates/domain_detail.html b/src/registrar/templates/domain_detail.html index bb137a91f..e0d672093 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' value=domain.nameservers list='true' edit_link=url only %} + {% include "includes/summary_item.html" with title='DNS name servers' value=domain.nameservers list='true' edit_link=url %} {% else %}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/tests/test_url_auth.py b/src/registrar/tests/test_url_auth.py index 32fc78ca1..19af605a5 100644 --- a/src/registrar/tests/test_url_auth.py +++ b/src/registrar/tests/test_url_auth.py @@ -110,13 +110,13 @@ class TestURLAuth(TestCase): # Note that the trailing slash is wobbly depending on how the URL was defined. IGNORE_URLS = [ # These are the OIDC auth endpoints that always need - # to be public. + # to be public. Use the exact URLs that will be tested. "/openid/login/", "/openid/logout/", "/openid/callback", "/openid/callback/login/", "/openid/callback/logout/", - "/api/v1/available", + "/api/v1/available/whitehouse.gov", ] def assertURLIsProtectedByAuth(self, url): diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 7cc616889..7250a9d8c 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -291,6 +291,9 @@ class DomainApplicationTests(TestWithUser, WebTest): dotgov_result = dotgov_form.submit() # validate that data from this step are being saved application = DomainApplication.objects.get() # there's only one + print("dotgov result: ", dotgov_form) + print("application: ", application) + print("application requested domain: ", application.requested_domain) self.assertEqual(application.requested_domain.name, "city.gov") self.assertEqual( application.alternative_domains.filter(website="city1.gov").count(), 1 From 5ae8ad6a5ffba512eacd84bd964199fab5a12ab2 Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Mon, 23 Oct 2023 13:03:35 -0700 Subject: [PATCH 07/14] Inherit MockEppLib to DomainApplicationTests --- src/registrar/tests/test_views.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index 7250a9d8c..ae8386c5c 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -118,7 +118,7 @@ class LoggedInTests(TestWithUser): self.assertEqual(response.status_code, 403) -class DomainApplicationTests(TestWithUser, WebTest): +class DomainApplicationTests(TestWithUser, WebTest, MockEppLib): """Webtests for domain application to test filling and submitting.""" @@ -291,9 +291,6 @@ class DomainApplicationTests(TestWithUser, WebTest): dotgov_result = dotgov_form.submit() # validate that data from this step are being saved application = DomainApplication.objects.get() # there's only one - print("dotgov result: ", dotgov_form) - print("application: ", application) - print("application requested domain: ", application.requested_domain) self.assertEqual(application.requested_domain.name, "city.gov") self.assertEqual( application.alternative_domains.filter(website="city1.gov").count(), 1 From 67433d45024daa493d957f0826e3c3544783739b Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Mon, 23 Oct 2023 14:00:55 -0700 Subject: [PATCH 08/14] Add city.gov test cases to Check Domain mocks --- src/registrar/tests/common.py | 4 ++++ src/registrar/tests/test_views.py | 5 ++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 53d15a91c..596b551e4 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -841,6 +841,10 @@ class MockEppLib(TestCase): return self._mockDomainName("igorvilleremixed.gov", True) elif "top-level-agency.gov" in getattr(_request, "names", None): return self._mockDomainName("top-level-agency.gov", True) + elif "city.gov" in getattr(_request, "names", None): + return self._mockDomainName("city.gov", True) + elif "city1.gov" in getattr(_request, "names", None): + return self._mockDomainName("city1.gov", True) elif "errordomain.gov" in getattr(_request, "names", None): raise RegistryError("Registry cannot find domain availability.") else: diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index ae8386c5c..b89fb519e 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -118,7 +118,7 @@ class LoggedInTests(TestWithUser): self.assertEqual(response.status_code, 403) -class DomainApplicationTests(TestWithUser, WebTest, MockEppLib): +class DomainApplicationTests(TestWithUser, WebTest): """Webtests for domain application to test filling and submitting.""" @@ -291,6 +291,9 @@ class DomainApplicationTests(TestWithUser, WebTest, MockEppLib): dotgov_result = dotgov_form.submit() # validate that data from this step are being saved application = DomainApplication.objects.get() # there's only one + # print("application: ", application) + print("application requested domains ", application.requested_domain) + print("application alternative domains ", application.alternative_domains) self.assertEqual(application.requested_domain.name, "city.gov") self.assertEqual( application.alternative_domains.filter(website="city1.gov").count(), 1 From 421603df2c47bbefb3a96da252af1645e7058f25 Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Mon, 23 Oct 2023 14:04:15 -0700 Subject: [PATCH 09/14] Remove test print statements --- src/registrar/tests/test_views.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/registrar/tests/test_views.py b/src/registrar/tests/test_views.py index b89fb519e..7cc616889 100644 --- a/src/registrar/tests/test_views.py +++ b/src/registrar/tests/test_views.py @@ -291,9 +291,6 @@ class DomainApplicationTests(TestWithUser, WebTest): dotgov_result = dotgov_form.submit() # validate that data from this step are being saved application = DomainApplication.objects.get() # there's only one - # print("application: ", application) - print("application requested domains ", application.requested_domain) - print("application alternative domains ", application.alternative_domains) self.assertEqual(application.requested_domain.name, "city.gov") self.assertEqual( application.alternative_domains.filter(website="city1.gov").count(), 1 From bde3653fb8670dacab6bcafdfad23e3722404c31 Mon Sep 17 00:00:00 2001 From: Neil Martinsen-Burrell