From 02456e92971528e5ac93a978916f13a33e357f83 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 8 Jan 2024 14:20:49 -0700 Subject: [PATCH 01/33] Update views.py --- src/api/views.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/api/views.py b/src/api/views.py index a7dd7600a..e40924708 100644 --- a/src/api/views.py +++ b/src/api/views.py @@ -88,10 +88,17 @@ def available(request, domain=""): """ domain = request.GET.get("domain", "") DraftDomain = apps.get_model("registrar.DraftDomain") + if domain is None or domain.strip() == "": + # TODO - change this... should it be the regular required? + return JsonResponse({"available": False, "code": "invalid", "message": "This field is required"}) # validate that the given domain could be a domain name and fail early if # not. if not (DraftDomain.string_could_be_domain(domain) or DraftDomain.string_could_be_domain(domain + ".gov")): - return JsonResponse({"available": False, "code": "invalid", "message": DOMAIN_API_MESSAGES["invalid"]}) + print(f"What is the domain at this point? {domain}") + if "." in domain: + return JsonResponse({"available": False, "code": "invalid", "message": DOMAIN_API_MESSAGES["extra_dots"]}) + else: + return JsonResponse({"available": False, "code": "invalid", "message": DOMAIN_API_MESSAGES["invalid"]}) # a domain is available if it is NOT in the list of current domains try: if check_domain_available(domain): From 89431b111db153f098bd2b946ab6988ea101e75f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Jan 2024 11:41:55 -0700 Subject: [PATCH 02/33] Unify error messages under one banner --- src/api/views.py | 33 ++---- src/registrar/forms/application_wizard.py | 28 +---- src/registrar/models/utility/domain_helper.py | 102 ++++++++++++++++-- src/registrar/utility/errors.py | 2 + 4 files changed, 107 insertions(+), 58 deletions(-) diff --git a/src/api/views.py b/src/api/views.py index e40924708..4bec29b80 100644 --- a/src/api/views.py +++ b/src/api/views.py @@ -1,7 +1,7 @@ """Internal API views""" from django.apps import apps from django.views.decorators.http import require_http_methods -from django.http import HttpResponse, JsonResponse +from django.http import HttpResponse from django.utils.safestring import mark_safe from registrar.templatetags.url_helpers import public_site_url @@ -71,6 +71,7 @@ def check_domain_available(domain): a match. If check fails, throws a RegistryError. """ Domain = apps.get_model("registrar.Domain") + if domain.endswith(".gov"): return Domain.available(domain) else: @@ -86,29 +87,15 @@ def available(request, domain=""): Response is a JSON dictionary with the key "available" and value true or false. """ + Domain = apps.get_model("registrar.Domain") domain = request.GET.get("domain", "") - DraftDomain = apps.get_model("registrar.DraftDomain") - if domain is None or domain.strip() == "": - # TODO - change this... should it be the regular required? - return JsonResponse({"available": False, "code": "invalid", "message": "This field is required"}) - # validate that the given domain could be a domain name and fail early if - # not. - if not (DraftDomain.string_could_be_domain(domain) or DraftDomain.string_could_be_domain(domain + ".gov")): - print(f"What is the domain at this point? {domain}") - if "." in domain: - return JsonResponse({"available": False, "code": "invalid", "message": DOMAIN_API_MESSAGES["extra_dots"]}) - else: - return JsonResponse({"available": False, "code": "invalid", "message": DOMAIN_API_MESSAGES["invalid"]}) - # a domain is available if it is NOT in the list of current domains - try: - if check_domain_available(domain): - return JsonResponse({"available": True, "code": "success", "message": DOMAIN_API_MESSAGES["success"]}) - else: - return JsonResponse( - {"available": False, "code": "unavailable", "message": DOMAIN_API_MESSAGES["unavailable"]} - ) - except Exception: - return JsonResponse({"available": False, "code": "error", "message": DOMAIN_API_MESSAGES["error"]}) + + json_response = Domain.validate_and_handle_errors( + domain=domain, + error_return_type="JSON_RESPONSE", + display_success=True, + ) + return json_response @require_http_methods(["GET"]) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index fcf6bda7a..00f832d59 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -384,17 +384,8 @@ CurrentSitesFormSet = forms.formset_factory( class AlternativeDomainForm(RegistrarForm): def clean_alternative_domain(self): """Validation code for domain names.""" - try: - requested = self.cleaned_data.get("alternative_domain", None) - validated = DraftDomain.validate(requested, blank_ok=True) - except errors.ExtraDotsError: - raise forms.ValidationError(DOMAIN_API_MESSAGES["extra_dots"], code="extra_dots") - except errors.DomainUnavailableError: - raise forms.ValidationError(DOMAIN_API_MESSAGES["unavailable"], code="unavailable") - except errors.RegistrySystemError: - raise forms.ValidationError(DOMAIN_API_MESSAGES["error"], code="error") - except ValueError: - raise forms.ValidationError(DOMAIN_API_MESSAGES["invalid"], code="invalid") + requested = self.cleaned_data.get("alternative_domain", None) + validated = DraftDomain.validate_and_handle_errors(requested, "FORM_VALIDATION_ERROR") return validated alternative_domain = forms.CharField( @@ -469,19 +460,8 @@ class DotGovDomainForm(RegistrarForm): def clean_requested_domain(self): """Validation code for domain names.""" - try: - requested = self.cleaned_data.get("requested_domain", None) - validated = DraftDomain.validate(requested) - except errors.BlankValueError: - raise forms.ValidationError(DOMAIN_API_MESSAGES["required"], code="required") - except errors.ExtraDotsError: - raise forms.ValidationError(DOMAIN_API_MESSAGES["extra_dots"], code="extra_dots") - except errors.DomainUnavailableError: - raise forms.ValidationError(DOMAIN_API_MESSAGES["unavailable"], code="unavailable") - except errors.RegistrySystemError: - raise forms.ValidationError(DOMAIN_API_MESSAGES["error"], code="error") - except ValueError: - raise forms.ValidationError(DOMAIN_API_MESSAGES["invalid"], code="invalid") + requested = self.cleaned_data.get("requested_domain", None) + validated = DraftDomain.validate_and_handle_errors(requested, "FORM_VALIDATION_ERROR") return validated requested_domain = forms.CharField(label="What .gov domain do you want?") diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index e43661b1d..7993d0f90 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -1,9 +1,16 @@ +from enum import Enum import re -from api.views import check_domain_available +from django import forms +from django.http import JsonResponse + +from api.views import DOMAIN_API_MESSAGES, check_domain_available from registrar.utility import errors from epplibwrapper.errors import RegistryError +class ValidationErrorReturnType(Enum): + JSON_RESPONSE = "JSON_RESPONSE" + FORM_VALIDATION_ERROR = "FORM_VALIDATION_ERROR" class DomainHelper: """Utility functions and constants for domain names.""" @@ -28,16 +35,10 @@ class DomainHelper: if domain is None: raise errors.BlankValueError() if not isinstance(domain, str): - raise ValueError("Domain name must be a string") - domain = domain.lower().strip() - if domain == "" and not blank_ok: - raise errors.BlankValueError() - if domain.endswith(".gov"): - domain = domain[:-4] - if "." in domain: - raise errors.ExtraDotsError() - if not DomainHelper.string_could_be_domain(domain + ".gov"): - raise ValueError() + raise errors.InvalidDomainError("Domain name must be a string") + + domain = DomainHelper._parse_domain_string(domain, blank_ok) + try: if not check_domain_available(domain): raise errors.DomainUnavailableError() @@ -45,6 +46,85 @@ class DomainHelper: raise errors.RegistrySystemError() from err return domain + @staticmethod + def _parse_domain_string(domain: str, blank_ok) -> str: + """Parses '.gov' out of the domain_name string, and does some validation on it""" + domain = domain.lower().strip() + + if domain == "" and not blank_ok: + raise errors.BlankValueError() + + if domain.endswith(".gov"): + domain = domain[:-4] + + if "." in domain: + raise errors.ExtraDotsError() + + if not DomainHelper.string_could_be_domain(domain + ".gov"): + raise errors.InvalidDomainError() + + @classmethod + def validate_and_handle_errors(cls, domain: str, error_return_type: str, display_success: bool = False): + """Runs validate() and catches possible exceptions.""" + try: + validated = cls.validate(domain) + except errors.BlankValueError: + return DomainHelper._return_form_error_or_json_response( + error_return_type, code="required" + ) + except errors.ExtraDotsError: + return DomainHelper._return_form_error_or_json_response( + error_return_type, code="extra_dots" + ) + except errors.DomainUnavailableError: + return DomainHelper._return_form_error_or_json_response( + error_return_type, code="unavailable" + ) + except errors.RegistrySystemError: + return DomainHelper._return_form_error_or_json_response( + error_return_type, code="error" + ) + except errors.InvalidDomainError: + return DomainHelper._return_form_error_or_json_response( + error_return_type, code="invalid" + ) + except Exception: + return DomainHelper._return_form_error_or_json_response( + error_return_type, code="error" + ) + else: + if display_success: + return DomainHelper._return_form_error_or_json_response( + error_return_type, code="success", available=True + ) + else: + return validated + + @staticmethod + def _return_form_error_or_json_response(return_type, code, available=False): + print(f"What is the code? {code}") + if return_type == "JSON_RESPONSE": + print("in the return context") + return JsonResponse( + {"available": available, "code": code, "message": DOMAIN_API_MESSAGES[code]} + ) + + if return_type == "FORM_VALIDATION_ERROR": + raise forms.ValidationError(DOMAIN_API_MESSAGES[code], code=code) + + # Why is this not working?? + """ + match return_type: + case ValidationErrorReturnType.FORM_VALIDATION_ERROR: + raise forms.ValidationError(DOMAIN_API_MESSAGES[code], code=code) + case ValidationErrorReturnType.JSON_RESPONSE: + return JsonResponse( + {"available": available, "code": code, "message": DOMAIN_API_MESSAGES[code]} + ) + case _: + raise ValueError("Invalid return type specified") + """ + @classmethod def sld(cls, domain: str): """ diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index 455419236..199997cc2 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -16,6 +16,8 @@ class DomainUnavailableError(ValueError): class RegistrySystemError(ValueError): pass +class InvalidDomainError(ValueError): + pass class ActionNotAllowed(Exception): """User accessed an action that is not From a7fa332d36827f8bfc97b90bba493477e11e58d1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Jan 2024 12:17:33 -0700 Subject: [PATCH 03/33] Update domain_helper.py --- src/registrar/models/utility/domain_helper.py | 27 ++++++------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index 7993d0f90..018087f7b 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -37,18 +37,6 @@ class DomainHelper: if not isinstance(domain, str): raise errors.InvalidDomainError("Domain name must be a string") - domain = DomainHelper._parse_domain_string(domain, blank_ok) - - try: - if not check_domain_available(domain): - raise errors.DomainUnavailableError() - except RegistryError as err: - raise errors.RegistrySystemError() from err - return domain - - @staticmethod - def _parse_domain_string(domain: str, blank_ok) -> str: - """Parses '.gov' out of the domain_name string, and does some validation on it""" domain = domain.lower().strip() if domain == "" and not blank_ok: @@ -63,6 +51,13 @@ class DomainHelper: if not DomainHelper.string_could_be_domain(domain + ".gov"): raise errors.InvalidDomainError() + try: + if not check_domain_available(domain): + raise errors.DomainUnavailableError() + except RegistryError as err: + raise errors.RegistrySystemError() from err + return domain + @classmethod def validate_and_handle_errors(cls, domain: str, error_return_type: str, display_success: bool = False): """Runs validate() and catches possible exceptions.""" @@ -88,10 +83,6 @@ class DomainHelper: return DomainHelper._return_form_error_or_json_response( error_return_type, code="invalid" ) - except Exception: - return DomainHelper._return_form_error_or_json_response( - error_return_type, code="error" - ) else: if display_success: return DomainHelper._return_form_error_or_json_response( @@ -102,11 +93,9 @@ class DomainHelper: @staticmethod def _return_form_error_or_json_response(return_type, code, available=False): - print(f"What is the code? {code}") if return_type == "JSON_RESPONSE": - print("in the return context") return JsonResponse( - {"available": available, "code": code, "message": DOMAIN_API_MESSAGES[code]} + {"available": available, "code": code, "message": DOMAIN_API_MESSAGES[code]} ) if return_type == "FORM_VALIDATION_ERROR": From 5b2eeee54771528df541bfe49a6e475de699ffbf Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Jan 2024 12:27:38 -0700 Subject: [PATCH 04/33] Add enums.py --- .../utility/extra_transition_domain_helper.py | 4 ++- .../commands/utility/terminal_helper.py | 21 +-------------- src/registrar/models/utility/domain_helper.py | 9 ++----- src/registrar/utility/enums.py | 27 +++++++++++++++++++ 4 files changed, 33 insertions(+), 28 deletions(-) create mode 100644 src/registrar/utility/enums.py diff --git a/src/registrar/management/commands/utility/extra_transition_domain_helper.py b/src/registrar/management/commands/utility/extra_transition_domain_helper.py index 54f68d5c8..755c9b98a 100644 --- a/src/registrar/management/commands/utility/extra_transition_domain_helper.py +++ b/src/registrar/management/commands/utility/extra_transition_domain_helper.py @@ -11,6 +11,7 @@ import os import sys from typing import Dict, List from django.core.paginator import Paginator +from registrar.utility.enums import LogCode from registrar.models.transition_domain import TransitionDomain from registrar.management.commands.utility.load_organization_error import ( LoadOrganizationError, @@ -28,7 +29,8 @@ from .epp_data_containers import ( ) from .transition_domain_arguments import TransitionDomainArguments -from .terminal_helper import TerminalColors, TerminalHelper, LogCode +from .terminal_helper import TerminalColors, TerminalHelper + logger = logging.getLogger(__name__) diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index 85bfc8193..3ae9ff3cd 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -1,29 +1,10 @@ -from enum import Enum import logging import sys from typing import List - +from registrar.utility.enums import LogCode logger = logging.getLogger(__name__) -class LogCode(Enum): - """Stores the desired log severity - - Overview of error codes: - - 1 ERROR - - 2 WARNING - - 3 INFO - - 4 DEBUG - - 5 DEFAULT - """ - - ERROR = 1 - WARNING = 2 - INFO = 3 - DEBUG = 4 - DEFAULT = 5 - - class TerminalColors: """Colors for terminal outputs (makes reading the logs WAY easier)""" diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index 018087f7b..28eaa391e 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -8,9 +8,6 @@ from api.views import DOMAIN_API_MESSAGES, check_domain_available from registrar.utility import errors from epplibwrapper.errors import RegistryError -class ValidationErrorReturnType(Enum): - JSON_RESPONSE = "JSON_RESPONSE" - FORM_VALIDATION_ERROR = "FORM_VALIDATION_ERROR" class DomainHelper: """Utility functions and constants for domain names.""" @@ -102,17 +99,15 @@ class DomainHelper: raise forms.ValidationError(DOMAIN_API_MESSAGES[code], code=code) # Why is this not working?? - """ match return_type: - case ValidationErrorReturnType.FORM_VALIDATION_ERROR: + case ValidationErrorReturnType.FORM_VALIDATION_ERROR.value: raise forms.ValidationError(DOMAIN_API_MESSAGES[code], code=code) - case ValidationErrorReturnType.JSON_RESPONSE: + case ValidationErrorReturnType.JSON_RESPONSE.value: return JsonResponse( {"available": available, "code": code, "message": DOMAIN_API_MESSAGES[code]} ) case _: raise ValueError("Invalid return type specified") - """ @classmethod def sld(cls, domain: str): diff --git a/src/registrar/utility/enums.py b/src/registrar/utility/enums.py new file mode 100644 index 000000000..64411f33e --- /dev/null +++ b/src/registrar/utility/enums.py @@ -0,0 +1,27 @@ +"""Used for holding various enums""" + +from enum import Enum + + +class ValidationErrorReturnType(Enum): + """Determines the return value of the validate_and_handle_errors class""" + JSON_RESPONSE = "JSON_RESPONSE" + FORM_VALIDATION_ERROR = "FORM_VALIDATION_ERROR" + + +class LogCode(Enum): + """Stores the desired log severity + + Overview of error codes: + - 1 ERROR + - 2 WARNING + - 3 INFO + - 4 DEBUG + - 5 DEFAULT + """ + + ERROR = 1 + WARNING = 2 + INFO = 3 + DEBUG = 4 + DEFAULT = 5 \ No newline at end of file From 91ed4a598c73e330ae2dff5159a135a91a144316 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Jan 2024 12:30:23 -0700 Subject: [PATCH 05/33] Hook up enum --- src/api/views.py | 3 ++- src/registrar/forms/application_wizard.py | 5 +++-- src/registrar/models/utility/domain_helper.py | 18 +++++------------- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/api/views.py b/src/api/views.py index 4bec29b80..b89e2629d 100644 --- a/src/api/views.py +++ b/src/api/views.py @@ -5,6 +5,7 @@ from django.http import HttpResponse from django.utils.safestring import mark_safe from registrar.templatetags.url_helpers import public_site_url +from registrar.utility.enums import ValidationErrorReturnType from registrar.utility.errors import GenericError, GenericErrorCodes import requests @@ -92,7 +93,7 @@ def available(request, domain=""): json_response = Domain.validate_and_handle_errors( domain=domain, - error_return_type="JSON_RESPONSE", + error_return_type=ValidationErrorReturnType.JSON_RESPONSE, display_success=True, ) return json_response diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 00f832d59..aa583a10c 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -13,6 +13,7 @@ from api.views import DOMAIN_API_MESSAGES from registrar.models import Contact, DomainApplication, DraftDomain, Domain from registrar.templatetags.url_helpers import public_site_url from registrar.utility import errors +from registrar.utility.enums import ValidationErrorReturnType logger = logging.getLogger(__name__) @@ -385,7 +386,7 @@ class AlternativeDomainForm(RegistrarForm): def clean_alternative_domain(self): """Validation code for domain names.""" requested = self.cleaned_data.get("alternative_domain", None) - validated = DraftDomain.validate_and_handle_errors(requested, "FORM_VALIDATION_ERROR") + validated = DraftDomain.validate_and_handle_errors(requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR) return validated alternative_domain = forms.CharField( @@ -461,7 +462,7 @@ class DotGovDomainForm(RegistrarForm): def clean_requested_domain(self): """Validation code for domain names.""" requested = self.cleaned_data.get("requested_domain", None) - validated = DraftDomain.validate_and_handle_errors(requested, "FORM_VALIDATION_ERROR") + validated = DraftDomain.validate_and_handle_errors(requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR) return validated requested_domain = forms.CharField(label="What .gov domain do you want?") diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index 28eaa391e..cf2369567 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -7,6 +7,7 @@ from django.http import JsonResponse from api.views import DOMAIN_API_MESSAGES, check_domain_available from registrar.utility import errors from epplibwrapper.errors import RegistryError +from registrar.utility.enums import ValidationErrorReturnType class DomainHelper: @@ -56,7 +57,7 @@ class DomainHelper: return domain @classmethod - def validate_and_handle_errors(cls, domain: str, error_return_type: str, display_success: bool = False): + def validate_and_handle_errors(cls, domain, error_return_type, display_success = False): """Runs validate() and catches possible exceptions.""" try: validated = cls.validate(domain) @@ -89,20 +90,11 @@ class DomainHelper: return validated @staticmethod - def _return_form_error_or_json_response(return_type, code, available=False): - if return_type == "JSON_RESPONSE": - return JsonResponse( - {"available": available, "code": code, "message": DOMAIN_API_MESSAGES[code]} - ) - - if return_type == "FORM_VALIDATION_ERROR": - raise forms.ValidationError(DOMAIN_API_MESSAGES[code], code=code) - - # Why is this not working?? + def _return_form_error_or_json_response(return_type: ValidationErrorReturnType, code, available=False): match return_type: - case ValidationErrorReturnType.FORM_VALIDATION_ERROR.value: + case ValidationErrorReturnType.FORM_VALIDATION_ERROR: raise forms.ValidationError(DOMAIN_API_MESSAGES[code], code=code) - case ValidationErrorReturnType.JSON_RESPONSE.value: + case ValidationErrorReturnType.JSON_RESPONSE: return JsonResponse( {"available": available, "code": code, "message": DOMAIN_API_MESSAGES[code]} ) From ab6cbb93c645f011b7bbcadf16cfa11ed3f33caf Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Jan 2024 13:31:32 -0700 Subject: [PATCH 06/33] Code cleanup --- src/registrar/forms/application_wizard.py | 7 +- .../commands/utility/terminal_helper.py | 1 + src/registrar/models/utility/domain_helper.py | 72 +++++++++++++------ src/registrar/utility/enums.py | 3 +- src/registrar/utility/errors.py | 2 + 5 files changed, 59 insertions(+), 26 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index aa583a10c..2eb359984 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -8,11 +8,8 @@ from django import forms from django.core.validators import RegexValidator, MaxLengthValidator from django.utils.safestring import mark_safe -from api.views import DOMAIN_API_MESSAGES - from registrar.models import Contact, DomainApplication, DraftDomain, Domain from registrar.templatetags.url_helpers import public_site_url -from registrar.utility import errors from registrar.utility.enums import ValidationErrorReturnType logger = logging.getLogger(__name__) @@ -386,7 +383,9 @@ class AlternativeDomainForm(RegistrarForm): def clean_alternative_domain(self): """Validation code for domain names.""" requested = self.cleaned_data.get("alternative_domain", None) - validated = DraftDomain.validate_and_handle_errors(requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR) + validated = DraftDomain.validate_and_handle_errors( + requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR, prevent_blank=False + ) return validated alternative_domain = forms.CharField( diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index 3ae9ff3cd..c24bd9616 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -2,6 +2,7 @@ import logging import sys from typing import List from registrar.utility.enums import LogCode + logger = logging.getLogger(__name__) diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index cf2369567..eb174d814 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -57,30 +57,44 @@ class DomainHelper: return domain @classmethod - def validate_and_handle_errors(cls, domain, error_return_type, display_success = False): - """Runs validate() and catches possible exceptions.""" + def validate_and_handle_errors(cls, domain, error_return_type, prevent_blank=True, display_success=False): + """ + Validates the provided domain and handles any validation errors. + + This method attempts to validate the domain using the `validate` method. If validation fails, + it catches the exception and returns an appropriate error response. The type of the error response + (JSON response or form validation error) is determined by the `error_return_type` parameter. + + If validation is successful and `display_success` is True, it returns a success response. + Otherwise, it returns the validated domain. + + Args: + domain (str): The domain to validate. + error_return_type (ValidationErrorReturnType): The type of error response to return if validation fails. + prevent_blank (bool, optional): Whether to return an exception if the input is blank. Defaults to True. + display_success (bool, optional): Whether to return a success response if validation is successful. Defaults to False. + + Returns: + The error response if validation fails, + the success response if validation is successful and `display_success` is True, + or the validated domain otherwise. + """ # noqa + try: validated = cls.validate(domain) except errors.BlankValueError: - return DomainHelper._return_form_error_or_json_response( - error_return_type, code="required" - ) + if not prevent_blank: + return DomainHelper._return_form_error_or_json_response(error_return_type, code="required") + else: + return validated except errors.ExtraDotsError: - return DomainHelper._return_form_error_or_json_response( - error_return_type, code="extra_dots" - ) + return DomainHelper._return_form_error_or_json_response(error_return_type, code="extra_dots") except errors.DomainUnavailableError: - return DomainHelper._return_form_error_or_json_response( - error_return_type, code="unavailable" - ) + return DomainHelper._return_form_error_or_json_response(error_return_type, code="unavailable") except errors.RegistrySystemError: - return DomainHelper._return_form_error_or_json_response( - error_return_type, code="error" - ) + return DomainHelper._return_form_error_or_json_response(error_return_type, code="error") except errors.InvalidDomainError: - return DomainHelper._return_form_error_or_json_response( - error_return_type, code="invalid" - ) + return DomainHelper._return_form_error_or_json_response(error_return_type, code="invalid") else: if display_success: return DomainHelper._return_form_error_or_json_response( @@ -88,16 +102,32 @@ class DomainHelper: ) else: return validated - + @staticmethod def _return_form_error_or_json_response(return_type: ValidationErrorReturnType, code, available=False): + """ + Returns an error response based on the `return_type`. + + If `return_type` is `FORM_VALIDATION_ERROR`, raises a form validation error. + If `return_type` is `JSON_RESPONSE`, returns a JSON response with 'available', 'code', and 'message' fields. + If `return_type` is neither, raises a ValueError. + + Args: + return_type (ValidationErrorReturnType): The type of error response. + code (str): The error code for the error message. + available (bool, optional): Availability, only used for JSON responses. Defaults to False. + + Returns: + A JSON response or a form validation error. + + Raises: + ValueError: If `return_type` is neither `FORM_VALIDATION_ERROR` nor `JSON_RESPONSE`. + """ # noqa match return_type: case ValidationErrorReturnType.FORM_VALIDATION_ERROR: raise forms.ValidationError(DOMAIN_API_MESSAGES[code], code=code) case ValidationErrorReturnType.JSON_RESPONSE: - return JsonResponse( - {"available": available, "code": code, "message": DOMAIN_API_MESSAGES[code]} - ) + return JsonResponse({"available": available, "code": code, "message": DOMAIN_API_MESSAGES[code]}) case _: raise ValueError("Invalid return type specified") diff --git a/src/registrar/utility/enums.py b/src/registrar/utility/enums.py index 64411f33e..6aa4f4044 100644 --- a/src/registrar/utility/enums.py +++ b/src/registrar/utility/enums.py @@ -5,6 +5,7 @@ from enum import Enum class ValidationErrorReturnType(Enum): """Determines the return value of the validate_and_handle_errors class""" + JSON_RESPONSE = "JSON_RESPONSE" FORM_VALIDATION_ERROR = "FORM_VALIDATION_ERROR" @@ -24,4 +25,4 @@ class LogCode(Enum): WARNING = 2 INFO = 3 DEBUG = 4 - DEFAULT = 5 \ No newline at end of file + DEFAULT = 5 diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index 199997cc2..bac18d076 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -16,9 +16,11 @@ class DomainUnavailableError(ValueError): class RegistrySystemError(ValueError): pass + class InvalidDomainError(ValueError): pass + class ActionNotAllowed(Exception): """User accessed an action that is not allowed by the current state""" From 6b6aed8f2406e7fa1ed433c41a64fac90922cb62 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Jan 2024 13:36:09 -0700 Subject: [PATCH 07/33] Fix blanks --- src/registrar/forms/application_wizard.py | 2 +- src/registrar/models/utility/domain_helper.py | 11 ++++------- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 2eb359984..22335c1c8 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -384,7 +384,7 @@ class AlternativeDomainForm(RegistrarForm): """Validation code for domain names.""" requested = self.cleaned_data.get("alternative_domain", None) validated = DraftDomain.validate_and_handle_errors( - requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR, prevent_blank=False + requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR, blank_ok=True ) return validated diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index eb174d814..e4d3c094c 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -57,7 +57,7 @@ class DomainHelper: return domain @classmethod - def validate_and_handle_errors(cls, domain, error_return_type, prevent_blank=True, display_success=False): + def validate_and_handle_errors(cls, domain, error_return_type, blank_ok=False, display_success=False): """ Validates the provided domain and handles any validation errors. @@ -71,7 +71,7 @@ class DomainHelper: Args: domain (str): The domain to validate. error_return_type (ValidationErrorReturnType): The type of error response to return if validation fails. - prevent_blank (bool, optional): Whether to return an exception if the input is blank. Defaults to True. + blank_ok (bool, optional): Whether to return an exception if the input is blank. Defaults to False. display_success (bool, optional): Whether to return a success response if validation is successful. Defaults to False. Returns: @@ -81,12 +81,9 @@ class DomainHelper: """ # noqa try: - validated = cls.validate(domain) + validated = cls.validate(domain, blank_ok) except errors.BlankValueError: - if not prevent_blank: - return DomainHelper._return_form_error_or_json_response(error_return_type, code="required") - else: - return validated + return DomainHelper._return_form_error_or_json_response(error_return_type, code="required") except errors.ExtraDotsError: return DomainHelper._return_form_error_or_json_response(error_return_type, code="extra_dots") except errors.DomainUnavailableError: From ce361cdd3b6a9d682227ff4fba96300fcc1695c0 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Jan 2024 14:07:41 -0700 Subject: [PATCH 08/33] Return tuple and do error mapping --- src/api/views.py | 5 +- src/registrar/forms/application_wizard.py | 4 +- src/registrar/models/utility/domain_helper.py | 46 +++++++++---------- 3 files changed, 25 insertions(+), 30 deletions(-) diff --git a/src/api/views.py b/src/api/views.py index b89e2629d..a98bd88a9 100644 --- a/src/api/views.py +++ b/src/api/views.py @@ -91,10 +91,9 @@ def available(request, domain=""): Domain = apps.get_model("registrar.Domain") domain = request.GET.get("domain", "") - json_response = Domain.validate_and_handle_errors( + _, json_response = Domain.validate_and_handle_errors( domain=domain, - error_return_type=ValidationErrorReturnType.JSON_RESPONSE, - display_success=True, + error_return_type=ValidationErrorReturnType.JSON_RESPONSE, ) return json_response diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index 22335c1c8..acd1d1cfc 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -383,7 +383,7 @@ class AlternativeDomainForm(RegistrarForm): def clean_alternative_domain(self): """Validation code for domain names.""" requested = self.cleaned_data.get("alternative_domain", None) - validated = DraftDomain.validate_and_handle_errors( + validated, _ = DraftDomain.validate_and_handle_errors( requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR, blank_ok=True ) return validated @@ -461,7 +461,7 @@ class DotGovDomainForm(RegistrarForm): def clean_requested_domain(self): """Validation code for domain names.""" requested = self.cleaned_data.get("requested_domain", None) - validated = DraftDomain.validate_and_handle_errors(requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR) + validated, _ = DraftDomain.validate_and_handle_errors(requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR) return validated requested_domain = forms.CharField(label="What .gov domain do you want?") diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index e4d3c094c..df7aa2b7e 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -57,7 +57,7 @@ class DomainHelper: return domain @classmethod - def validate_and_handle_errors(cls, domain, error_return_type, blank_ok=False, display_success=False): + def validate_and_handle_errors(cls, domain, error_return_type, blank_ok=False): """ Validates the provided domain and handles any validation errors. @@ -65,40 +65,36 @@ class DomainHelper: it catches the exception and returns an appropriate error response. The type of the error response (JSON response or form validation error) is determined by the `error_return_type` parameter. - If validation is successful and `display_success` is True, it returns a success response. - Otherwise, it returns the validated domain. Args: domain (str): The domain to validate. error_return_type (ValidationErrorReturnType): The type of error response to return if validation fails. blank_ok (bool, optional): Whether to return an exception if the input is blank. Defaults to False. - display_success (bool, optional): Whether to return a success response if validation is successful. Defaults to False. - Returns: - The error response if validation fails, - the success response if validation is successful and `display_success` is True, - or the validated domain otherwise. + A tuple of the validated domain name, and the response """ # noqa - + error_map = { + errors.BlankValueError: "required", + errors.ExtraDotsError: "extra_dots", + errors.DomainUnavailableError: "unavailable", + errors.RegistrySystemError: "error", + errors.InvalidDomainError: "invalid", + } + validated = None + response = None try: validated = cls.validate(domain, blank_ok) - except errors.BlankValueError: - return DomainHelper._return_form_error_or_json_response(error_return_type, code="required") - except errors.ExtraDotsError: - return DomainHelper._return_form_error_or_json_response(error_return_type, code="extra_dots") - except errors.DomainUnavailableError: - return DomainHelper._return_form_error_or_json_response(error_return_type, code="unavailable") - except errors.RegistrySystemError: - return DomainHelper._return_form_error_or_json_response(error_return_type, code="error") - except errors.InvalidDomainError: - return DomainHelper._return_form_error_or_json_response(error_return_type, code="invalid") + # Get a list of each possible exception, and the code to return + except tuple(error_map.keys()) as error: + # For each exception, determine which code should be returned + response = DomainHelper._return_form_error_or_json_response( + error_return_type, code=error_map.get(type(error)) + ) else: - if display_success: - return DomainHelper._return_form_error_or_json_response( - error_return_type, code="success", available=True - ) - else: - return validated + response = DomainHelper._return_form_error_or_json_response( + error_return_type, code="success", available=True + ) + return (validated, response) @staticmethod def _return_form_error_or_json_response(return_type: ValidationErrorReturnType, code, available=False): From 05ebe98adb98501130c541ce42fcaa935e3679c3 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Jan 2024 14:18:32 -0700 Subject: [PATCH 09/33] Update domain_helper.py --- src/registrar/models/utility/domain_helper.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index df7aa2b7e..78e27477b 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -91,9 +91,10 @@ class DomainHelper: error_return_type, code=error_map.get(type(error)) ) else: - response = DomainHelper._return_form_error_or_json_response( - error_return_type, code="success", available=True - ) + if error_return_type != ValidationErrorReturnType.FORM_VALIDATION_ERROR: + response = DomainHelper._return_form_error_or_json_response( + error_return_type, code="success", available=True + ) return (validated, response) @staticmethod From 45e994ea05e9f2a92d835ecd42a16b1a1d037e30 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Jan 2024 14:22:50 -0700 Subject: [PATCH 10/33] Linting --- src/api/views.py | 2 +- src/registrar/forms/application_wizard.py | 4 +++- src/registrar/models/utility/domain_helper.py | 1 - 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/api/views.py b/src/api/views.py index a98bd88a9..24960ff1c 100644 --- a/src/api/views.py +++ b/src/api/views.py @@ -92,7 +92,7 @@ def available(request, domain=""): domain = request.GET.get("domain", "") _, json_response = Domain.validate_and_handle_errors( - domain=domain, + domain=domain, error_return_type=ValidationErrorReturnType.JSON_RESPONSE, ) return json_response diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index acd1d1cfc..c29225ca6 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -461,7 +461,9 @@ class DotGovDomainForm(RegistrarForm): def clean_requested_domain(self): """Validation code for domain names.""" requested = self.cleaned_data.get("requested_domain", None) - validated, _ = DraftDomain.validate_and_handle_errors(requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR) + validated, _ = DraftDomain.validate_and_handle_errors( + requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR + ) return validated requested_domain = forms.CharField(label="What .gov domain do you want?") diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index 78e27477b..58629f213 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -1,4 +1,3 @@ -from enum import Enum import re from django import forms From 7045e5dcadf1efd206854af4f381391fde50c9ae Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Jan 2024 14:40:53 -0700 Subject: [PATCH 11/33] Exclude blank fields --- src/registrar/assets/js/get-gov.js | 9 +++++++++ src/registrar/templates/application_dotgov_domain.html | 8 +++++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index a2a99e104..92b6a1e46 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -134,10 +134,19 @@ function _checkDomainAvailability(el) { const callback = (response) => { toggleInputValidity(el, (response && response.available), msg=response.message); announce(el.id, response.message); + + // Determines if we ignore the field if it is just blank + ignore_blank = el.classList.contains("blank-ok") if (el.validity.valid) { el.classList.add('usa-input--success'); // use of `parentElement` due to .gov inputs being wrapped in www/.gov decoration inlineToast(el.parentElement, el.id, SUCCESS, response.message); + } else if (ignore_blank && response.code == "required"){ + // Visually remove the error + error = "usa-input--error" + if (el.classList.contains(error)){ + el.classList.remove(error) + } } else { inlineToast(el.parentElement, el.id, ERROR, response.message); } diff --git a/src/registrar/templates/application_dotgov_domain.html b/src/registrar/templates/application_dotgov_domain.html index bd3c4a473..ab5504264 100644 --- a/src/registrar/templates/application_dotgov_domain.html +++ b/src/registrar/templates/application_dotgov_domain.html @@ -73,9 +73,11 @@ {# attr_validate / validate="domain" invokes code in get-gov.js #} {# attr_auto_validate likewise triggers behavior in get-gov.js #} {% with append_gov=True attr_validate="domain" attr_auto_validate=True %} - {% for form in forms.1 %} - {% input_with_errors form.alternative_domain %} - {% endfor %} + {% with add_class="blank-ok" %} + {% for form in forms.1 %} + {% input_with_errors form.alternative_domain %} + {% endfor %} + {% endwith %} {% endwith %} {% endwith %} From 5cdbafeeb68a546b50357ace4c732541f7f330d0 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Jan 2024 14:55:18 -0700 Subject: [PATCH 12/33] Improve comments --- src/registrar/models/utility/domain_helper.py | 31 +++++++++++++------ src/registrar/utility/errors.py | 2 ++ 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index 58629f213..6c85cb884 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -58,20 +58,21 @@ class DomainHelper: @classmethod def validate_and_handle_errors(cls, domain, error_return_type, blank_ok=False): """ - Validates the provided domain and handles any validation errors. - - This method attempts to validate the domain using the `validate` method. If validation fails, - it catches the exception and returns an appropriate error response. The type of the error response - (JSON response or form validation error) is determined by the `error_return_type` parameter. + Validates a domain and returns an appropriate response based on the validation result. + This method uses the `validate` method to validate the domain. If validation fails, it catches the exception, + maps it to a corresponding error code, and returns a response based on the `error_return_type` parameter. Args: domain (str): The domain to validate. - error_return_type (ValidationErrorReturnType): The type of error response to return if validation fails. - blank_ok (bool, optional): Whether to return an exception if the input is blank. Defaults to False. + error_return_type (ValidationErrorReturnType): Determines the type of response (JSON or form validation error). + blank_ok (bool, optional): If True, blank input does not raise an exception. Defaults to False. + Returns: - A tuple of the validated domain name, and the response + tuple: The validated domain (or None if validation failed), and the response (success or error). """ # noqa + + # Map each exception to a corresponding error code error_map = { errors.BlankValueError: "required", errors.ExtraDotsError: "extra_dots", @@ -79,21 +80,31 @@ class DomainHelper: errors.RegistrySystemError: "error", errors.InvalidDomainError: "invalid", } + validated = None response = None + try: + # Attempt to validate the domain validated = cls.validate(domain, blank_ok) + # Get a list of each possible exception, and the code to return except tuple(error_map.keys()) as error: - # For each exception, determine which code should be returned + # If an error is caught, get its type + error_type = type(error) + + # Generate the response based on the error code and return type response = DomainHelper._return_form_error_or_json_response( - error_return_type, code=error_map.get(type(error)) + error_return_type, code=error_map.get(error_type) ) else: + # For form validation, we do not need to display the success message if error_return_type != ValidationErrorReturnType.FORM_VALIDATION_ERROR: response = DomainHelper._return_form_error_or_json_response( error_return_type, code="success", available=True ) + + # Return the validated domain and the response (either error or success) return (validated, response) @staticmethod diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index bac18d076..21158e58a 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -18,6 +18,8 @@ class RegistrySystemError(ValueError): class InvalidDomainError(ValueError): + """Error class for situations where an invalid domain is supplied""" + pass From 1b292ce640a64e66d3603bea8bd64acad70322e8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Jan 2024 15:01:41 -0700 Subject: [PATCH 13/33] Name refactor --- src/api/views.py | 4 ++-- src/registrar/forms/application_wizard.py | 9 +++++--- src/registrar/models/utility/domain_helper.py | 22 +++++++++---------- src/registrar/utility/enums.py | 2 +- 4 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/api/views.py b/src/api/views.py index 24960ff1c..9096c41ea 100644 --- a/src/api/views.py +++ b/src/api/views.py @@ -5,7 +5,7 @@ from django.http import HttpResponse from django.utils.safestring import mark_safe from registrar.templatetags.url_helpers import public_site_url -from registrar.utility.enums import ValidationErrorReturnType +from registrar.utility.enums import ValidationReturnType from registrar.utility.errors import GenericError, GenericErrorCodes import requests @@ -93,7 +93,7 @@ def available(request, domain=""): _, json_response = Domain.validate_and_handle_errors( domain=domain, - error_return_type=ValidationErrorReturnType.JSON_RESPONSE, + return_type=ValidationReturnType.JSON_RESPONSE, ) return json_response diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index c29225ca6..e318ee5aa 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -10,7 +10,7 @@ from django.utils.safestring import mark_safe from registrar.models import Contact, DomainApplication, DraftDomain, Domain from registrar.templatetags.url_helpers import public_site_url -from registrar.utility.enums import ValidationErrorReturnType +from registrar.utility.enums import ValidationReturnType logger = logging.getLogger(__name__) @@ -384,7 +384,9 @@ class AlternativeDomainForm(RegistrarForm): """Validation code for domain names.""" requested = self.cleaned_data.get("alternative_domain", None) validated, _ = DraftDomain.validate_and_handle_errors( - requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR, blank_ok=True + domain=requested, + return_type=ValidationReturnType.FORM_VALIDATION_ERROR, + blank_ok=True, ) return validated @@ -462,7 +464,8 @@ class DotGovDomainForm(RegistrarForm): """Validation code for domain names.""" requested = self.cleaned_data.get("requested_domain", None) validated, _ = DraftDomain.validate_and_handle_errors( - requested, ValidationErrorReturnType.FORM_VALIDATION_ERROR + domain=requested, + return_type=ValidationReturnType.FORM_VALIDATION_ERROR, ) return validated diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index 6c85cb884..5647fe49e 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -6,7 +6,7 @@ from django.http import JsonResponse from api.views import DOMAIN_API_MESSAGES, check_domain_available from registrar.utility import errors from epplibwrapper.errors import RegistryError -from registrar.utility.enums import ValidationErrorReturnType +from registrar.utility.enums import ValidationReturnType class DomainHelper: @@ -56,16 +56,16 @@ class DomainHelper: return domain @classmethod - def validate_and_handle_errors(cls, domain, error_return_type, blank_ok=False): + def validate_and_handle_errors(cls, domain, return_type, blank_ok=False): """ Validates a domain and returns an appropriate response based on the validation result. This method uses the `validate` method to validate the domain. If validation fails, it catches the exception, - maps it to a corresponding error code, and returns a response based on the `error_return_type` parameter. + maps it to a corresponding error code, and returns a response based on the `return_type` parameter. Args: domain (str): The domain to validate. - error_return_type (ValidationErrorReturnType): Determines the type of response (JSON or form validation error). + return_type (ValidationReturnType): Determines the type of response (JSON or form validation error). blank_ok (bool, optional): If True, blank input does not raise an exception. Defaults to False. Returns: @@ -95,20 +95,20 @@ class DomainHelper: # Generate the response based on the error code and return type response = DomainHelper._return_form_error_or_json_response( - error_return_type, code=error_map.get(error_type) + return_type, code=error_map.get(error_type) ) else: # For form validation, we do not need to display the success message - if error_return_type != ValidationErrorReturnType.FORM_VALIDATION_ERROR: + if return_type != ValidationReturnType.FORM_VALIDATION_ERROR: response = DomainHelper._return_form_error_or_json_response( - error_return_type, code="success", available=True + return_type, code="success", available=True ) # Return the validated domain and the response (either error or success) return (validated, response) @staticmethod - def _return_form_error_or_json_response(return_type: ValidationErrorReturnType, code, available=False): + def _return_form_error_or_json_response(return_type: ValidationReturnType, code, available=False): """ Returns an error response based on the `return_type`. @@ -117,7 +117,7 @@ class DomainHelper: If `return_type` is neither, raises a ValueError. Args: - return_type (ValidationErrorReturnType): The type of error response. + return_type (ValidationReturnType): The type of error response. code (str): The error code for the error message. available (bool, optional): Availability, only used for JSON responses. Defaults to False. @@ -128,9 +128,9 @@ class DomainHelper: ValueError: If `return_type` is neither `FORM_VALIDATION_ERROR` nor `JSON_RESPONSE`. """ # noqa match return_type: - case ValidationErrorReturnType.FORM_VALIDATION_ERROR: + case ValidationReturnType.FORM_VALIDATION_ERROR: raise forms.ValidationError(DOMAIN_API_MESSAGES[code], code=code) - case ValidationErrorReturnType.JSON_RESPONSE: + case ValidationReturnType.JSON_RESPONSE: return JsonResponse({"available": available, "code": code, "message": DOMAIN_API_MESSAGES[code]}) case _: raise ValueError("Invalid return type specified") diff --git a/src/registrar/utility/enums.py b/src/registrar/utility/enums.py index 6aa4f4044..51f6523c5 100644 --- a/src/registrar/utility/enums.py +++ b/src/registrar/utility/enums.py @@ -3,7 +3,7 @@ from enum import Enum -class ValidationErrorReturnType(Enum): +class ValidationReturnType(Enum): """Determines the return value of the validate_and_handle_errors class""" JSON_RESPONSE = "JSON_RESPONSE" From cdf46044d7b57d697ba1204109a9a03d2ac89ba1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 10 Jan 2024 11:06:40 -0700 Subject: [PATCH 14/33] Unit tests --- src/registrar/tests/test_forms.py | 109 ++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/src/registrar/tests/test_forms.py b/src/registrar/tests/test_forms.py index e0afb2d71..0187a7636 100644 --- a/src/registrar/tests/test_forms.py +++ b/src/registrar/tests/test_forms.py @@ -1,8 +1,12 @@ """Test form validation requirements.""" +import json from django.test import TestCase, RequestFactory +from django.urls import reverse +from api.views import available from registrar.forms.application_wizard import ( + AlternativeDomainForm, CurrentSitesForm, DotGovDomainForm, AuthorizingOfficialForm, @@ -23,6 +27,7 @@ from django.contrib.auth import get_user_model class TestFormValidation(MockEppLib): def setUp(self): super().setUp() + self.API_BASE_PATH = "/api/v1/available/?domain=" self.user = get_user_model().objects.create(username="username") self.factory = RequestFactory() @@ -74,6 +79,110 @@ class TestFormValidation(MockEppLib): ["Enter the .gov domain you want without any periods."], ) + def test_requested_domain_errors_consistent(self): + """Tests if the errors on submit and with the check availability buttons are consistent + for requested_domains + """ + test_cases = [ + # extra_dots + ("top-level-agency.com", "Enter the .gov domain you want without any periods."), + # invalid + ( + "underscores_forever", + "Enter a domain using only letters, numbers, " "or hyphens (though we don't recommend using hyphens).", + ), + # required + ("", "Enter the .gov domain you want. Don’t include “www” or “.gov.”"), + # unavailable + ( + "whitehouse.gov", + "That domain isn’t available. Read more about " + "choosing your .gov domain.", + ), + ] + + for domain, expected_error in test_cases: + with self.subTest(domain=domain, error=expected_error): + form = DotGovDomainForm(data={"requested_domain": domain}) + + form_error = list(form.errors["requested_domain"]) + + # Ensure the form returns what we expect + self.assertEqual( + form_error, + [expected_error], + ) + + request = self.factory.get(self.API_BASE_PATH + domain) + request.user = self.user + response = available(request, domain=domain) + + # Ensure that we're getting the right kind of response + self.assertContains(response, "available") + + response_object = json.loads(response.content) + + json_error = response_object["message"] + # Test if the message is what we expect + self.assertEqual(json_error, expected_error) + + # While its implied, + # for good measure, test if the two objects are equal anyway + self.assertEqual([json_error], form_error) + + def test_alternate_domain_errors_consistent(self): + """Tests if the errors on submit and with the check availability buttons are consistent + for alternative_domains + """ + test_cases = [ + # extra_dots + ("top-level-agency.com", "Enter the .gov domain you want without any periods."), + # invalid + ( + "underscores_forever", + "Enter a domain using only letters, numbers, " "or hyphens (though we don't recommend using hyphens).", + ), + # required + ("", "Enter the .gov domain you want. Don’t include “www” or “.gov.”"), + # unavailable + ( + "whitehouse.gov", + "That domain isn’t available. Read more about " + "choosing your .gov domain.", + ), + ] + + for domain, expected_error in test_cases: + with self.subTest(domain=domain, error=expected_error): + form = AlternativeDomainForm(data={"alternative_domain": domain}) + + form_error = list(form.errors["alternative_domain"]) + + # Ensure the form returns what we expect + self.assertEqual( + form_error, + [expected_error], + ) + + request = self.factory.get(self.API_BASE_PATH + domain) + request.user = self.user + response = available(request, domain=domain) + + # Ensure that we're getting the right kind of response + self.assertContains(response, "available") + + response_object = json.loads(response.content) + + json_error = response_object["message"] + # Test if the message is what we expect + self.assertEqual(json_error, expected_error) + + # While its implied, + # for good measure, test if the two objects are equal anyway + self.assertEqual([json_error], form_error) + def test_requested_domain_two_dots_invalid(self): """don't accept domains that are subdomains""" form = DotGovDomainForm(data={"requested_domain": "sub.top-level-agency.gov"}) From 659fff322e38a3e766a4a6318b524ae46dca5a24 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 10 Jan 2024 12:04:13 -0700 Subject: [PATCH 15/33] Add unit tests --- src/registrar/forms/application_wizard.py | 14 ++++++++++---- src/registrar/models/utility/domain_helper.py | 8 ++------ src/registrar/tests/test_forms.py | 10 ++++++---- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/registrar/forms/application_wizard.py b/src/registrar/forms/application_wizard.py index e318ee5aa..34bfb1731 100644 --- a/src/registrar/forms/application_wizard.py +++ b/src/registrar/forms/application_wizard.py @@ -2,6 +2,7 @@ from __future__ import annotations # allows forward references in annotations from itertools import zip_longest import logging from typing import Callable +from api.views import DOMAIN_API_MESSAGES from phonenumber_field.formfields import PhoneNumberField # type: ignore from django import forms @@ -384,8 +385,8 @@ class AlternativeDomainForm(RegistrarForm): """Validation code for domain names.""" requested = self.cleaned_data.get("alternative_domain", None) validated, _ = DraftDomain.validate_and_handle_errors( - domain=requested, - return_type=ValidationReturnType.FORM_VALIDATION_ERROR, + domain=requested, + return_type=ValidationReturnType.FORM_VALIDATION_ERROR, blank_ok=True, ) return validated @@ -464,12 +465,17 @@ class DotGovDomainForm(RegistrarForm): """Validation code for domain names.""" requested = self.cleaned_data.get("requested_domain", None) validated, _ = DraftDomain.validate_and_handle_errors( - domain=requested, + domain=requested, return_type=ValidationReturnType.FORM_VALIDATION_ERROR, ) return validated - requested_domain = forms.CharField(label="What .gov domain do you want?") + requested_domain = forms.CharField( + label="What .gov domain do you want?", + error_messages={ + "required": DOMAIN_API_MESSAGES["required"], + }, + ) class PurposeForm(RegistrarForm): diff --git a/src/registrar/models/utility/domain_helper.py b/src/registrar/models/utility/domain_helper.py index 5647fe49e..818473f2f 100644 --- a/src/registrar/models/utility/domain_helper.py +++ b/src/registrar/models/utility/domain_helper.py @@ -94,15 +94,11 @@ class DomainHelper: error_type = type(error) # Generate the response based on the error code and return type - response = DomainHelper._return_form_error_or_json_response( - return_type, code=error_map.get(error_type) - ) + response = DomainHelper._return_form_error_or_json_response(return_type, code=error_map.get(error_type)) else: # For form validation, we do not need to display the success message if return_type != ValidationReturnType.FORM_VALIDATION_ERROR: - response = DomainHelper._return_form_error_or_json_response( - return_type, code="success", available=True - ) + response = DomainHelper._return_form_error_or_json_response(return_type, code="success", available=True) # Return the validated domain and the response (either error or success) return (validated, response) diff --git a/src/registrar/tests/test_forms.py b/src/registrar/tests/test_forms.py index 0187a7636..a49257b75 100644 --- a/src/registrar/tests/test_forms.py +++ b/src/registrar/tests/test_forms.py @@ -2,7 +2,6 @@ import json from django.test import TestCase, RequestFactory -from django.urls import reverse from api.views import available from registrar.forms.application_wizard import ( @@ -92,7 +91,12 @@ class TestFormValidation(MockEppLib): "Enter a domain using only letters, numbers, " "or hyphens (though we don't recommend using hyphens).", ), # required - ("", "Enter the .gov domain you want. Don’t include “www” or “.gov.”"), + ( + "", + "Enter the .gov domain you want. Don’t include “www” or “.gov.”" + " For example, if you want www.city.gov, you would enter “city”" + " (without the quotes).", + ), # unavailable ( "whitehouse.gov", @@ -143,8 +147,6 @@ class TestFormValidation(MockEppLib): "underscores_forever", "Enter a domain using only letters, numbers, " "or hyphens (though we don't recommend using hyphens).", ), - # required - ("", "Enter the .gov domain you want. Don’t include “www” or “.gov.”"), # unavailable ( "whitehouse.gov", From cca3c5a23ce74e8d1f10010a4218fab8d16b55a4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 10 Jan 2024 12:25:56 -0700 Subject: [PATCH 16/33] Update test_forms.py --- src/registrar/tests/test_forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_forms.py b/src/registrar/tests/test_forms.py index a49257b75..e6913a48d 100644 --- a/src/registrar/tests/test_forms.py +++ b/src/registrar/tests/test_forms.py @@ -152,7 +152,7 @@ class TestFormValidation(MockEppLib): "whitehouse.gov", "That domain isn’t available. Read more about " - "choosing your .gov domain.", + "choosing your .gov domain..", ), ] From be8fb61c43b45149e4f000fe1637bcf1a3306c31 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 10 Jan 2024 12:32:53 -0700 Subject: [PATCH 17/33] Add period --- src/registrar/tests/test_forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_forms.py b/src/registrar/tests/test_forms.py index e6913a48d..2f464046f 100644 --- a/src/registrar/tests/test_forms.py +++ b/src/registrar/tests/test_forms.py @@ -102,7 +102,7 @@ class TestFormValidation(MockEppLib): "whitehouse.gov", "That domain isn’t available. Read more about " - "choosing your .gov domain.", + "choosing your .gov domain.", ), ] From f0027629168fe2e1dec5a88e7f8bed957acbe65c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 10 Jan 2024 12:36:29 -0700 Subject: [PATCH 18/33] Fix periods --- src/registrar/tests/test_forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_forms.py b/src/registrar/tests/test_forms.py index 2f464046f..7b42311d4 100644 --- a/src/registrar/tests/test_forms.py +++ b/src/registrar/tests/test_forms.py @@ -152,7 +152,7 @@ class TestFormValidation(MockEppLib): "whitehouse.gov", "That domain isn’t available. Read more about " - "choosing your .gov domain..", + "choosing your .gov domain.", ), ] From af656eb5a3eaeadb2b9328293a9af10879dd263f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 17 Jan 2024 11:58:49 -0700 Subject: [PATCH 19/33] Remove duplicate form errors --- src/registrar/assets/js/get-gov.js | 52 +++++++++++++++++++ .../templates/application_dotgov_domain.html | 3 +- 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/src/registrar/assets/js/get-gov.js b/src/registrar/assets/js/get-gov.js index 92b6a1e46..e016165ae 100644 --- a/src/registrar/assets/js/get-gov.js +++ b/src/registrar/assets/js/get-gov.js @@ -236,8 +236,60 @@ function handleValidationClick(e) { for(const button of activatesValidation) { button.addEventListener('click', handleValidationClick); } + + + // Add event listener to the "Check availability" button + const checkAvailabilityButton = document.getElementById('check-availability-button'); + if (checkAvailabilityButton) { + const targetId = checkAvailabilityButton.getAttribute('validate-for'); + const checkAvailabilityInput = document.getElementById(targetId); + checkAvailabilityButton.addEventListener('click', function() { + removeFormErrors(checkAvailabilityInput); + }); + } + + // Add event listener to the alternate domains input + const alternateDomainsInputs = document.querySelectorAll('[auto-validate]'); + if (alternateDomainsInputs) { + for (const domainInput of alternateDomainsInputs){ + // Only apply this logic to alternate domains input + if (domainInput.classList.contains('alternate-domain-input')){ + domainInput.addEventListener('input', function() { + removeFormErrors(domainInput); + }); + } + } + } })(); +/** + * Removes form errors surrounding a form input + */ +function removeFormErrors(input){ + // Remove error message + const errorMessage = document.getElementById(`${input.id}__error-message`); + if (errorMessage) { + errorMessage.remove(); + } + + // Remove error classes + if (input.classList.contains('usa-input--error')) { + input.classList.remove('usa-input--error'); + } + + const label = document.querySelector(`label[for="${input.id}"]`); + if (label) { + label.classList.remove('usa-label--error'); + + // Remove error classes from parent div + const parentDiv = label.parentElement; + if (parentDiv) { + parentDiv.classList.remove('usa-form-group--error'); + } + } + + +} /** * Prepare the namerservers and DS data forms delete buttons diff --git a/src/registrar/templates/application_dotgov_domain.html b/src/registrar/templates/application_dotgov_domain.html index ab5504264..25724c010 100644 --- a/src/registrar/templates/application_dotgov_domain.html +++ b/src/registrar/templates/application_dotgov_domain.html @@ -53,6 +53,7 @@ {% endwith %} {% endwith %}