Merge pull request #1615 from cisagov/za/1523-inconsistent-error-messages

(On getgov-backup) Ticket #1523: Fix inconsistent error messages

src/api/views.py

@@ -1,10 +1,11 @@
 """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
+from registrar.utility.enums import ValidationReturnType
 from registrar.utility.errors import GenericError, GenericErrorCodes
 
 import requests
@@ -71,6 +72,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,22 +88,14 @@ 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")
+
-    # validate that the given domain could be a domain name and fail early if
+    _, json_response = Domain.validate_and_handle_errors(
-    # not.
+        domain=domain,
-    if not (DraftDomain.string_could_be_domain(domain) or DraftDomain.string_could_be_domain(domain + ".gov")):
+        return_type=ValidationReturnType.JSON_RESPONSE,
-        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
+    return json_response
-    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"]})
 
 
 @require_http_methods(["GET"])

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);
     }

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
@@ -9,11 +10,9 @@ from django.core.validators import RegexValidator, MaxLengthValidator
 from django.utils.safestring import mark_safe
 from django.db.models.fields.related import ForeignObjectRel
 
-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 ValidationReturnType
 
 logger = logging.getLogger(__name__)
 
@@ -411,17 +410,12 @@ 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)
-            requested = self.cleaned_data.get("alternative_domain", None)
+        validated, _ = DraftDomain.validate_and_handle_errors(
-            validated = DraftDomain.validate(requested, blank_ok=True)
+            domain=requested,
-        except errors.ExtraDotsError:
+            return_type=ValidationReturnType.FORM_VALIDATION_ERROR,
-            raise forms.ValidationError(DOMAIN_API_MESSAGES["extra_dots"], code="extra_dots")
+            blank_ok=True,
-        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")
         return validated
 
     alternative_domain = forms.CharField(
@@ -498,22 +492,19 @@ class DotGovDomainForm(RegistrarForm):
 
     def clean_requested_domain(self):
         """Validation code for domain names."""
-        try:
+        requested = self.cleaned_data.get("requested_domain", None)
-            requested = self.cleaned_data.get("requested_domain", None)
+        validated, _ = DraftDomain.validate_and_handle_errors(
-            validated = DraftDomain.validate(requested)
+            domain=requested,
-        except errors.BlankValueError:
+            return_type=ValidationReturnType.FORM_VALIDATION_ERROR,
-            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")
         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):

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__)
 

src/registrar/management/commands/utility/terminal_helper.py

@@ -1,30 +1,12 @@
-from enum import Enum
 import logging
 import sys
 from django.core.paginator import Paginator
 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)"""

src/registrar/models/utility/domain_helper.py

@@ -1,8 +1,12 @@
 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
+from registrar.utility.enums import ValidationReturnType
 
 
 class DomainHelper:
@@ -23,21 +27,12 @@ class DomainHelper:
         return bool(cls.DOMAIN_REGEX.match(domain))
 
     @classmethod
-    def validate(cls, domain: str | None, blank_ok=False) -> str:
+    def validate(cls, domain: str, blank_ok=False) -> str:
         """Attempt to determine if a domain name could be requested."""
-        if domain is None:
+
-            raise errors.BlankValueError()
+        # Split into pieces for the linter
-        if not isinstance(domain, str):
+        domain = cls._validate_domain_string(domain, blank_ok)
-            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()
         try:
             if not check_domain_available(domain):
                 raise errors.DomainUnavailableError()
@@ -45,6 +40,110 @@ class DomainHelper:
             raise errors.RegistrySystemError() from err
         return domain
 
+    @staticmethod
+    def _validate_domain_string(domain, blank_ok):
+        """Normalize the domain string, and check its content"""
+        if domain is None:
+            raise errors.BlankValueError()
+
+        if not isinstance(domain, str):
+            raise errors.InvalidDomainError()
+
+        domain = domain.lower().strip()
+
+        if domain == "" and not blank_ok:
+            raise errors.BlankValueError()
+        elif domain == "":
+            # If blank ok is true, just return the domain
+            return domain
+
+        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()
+
+        return domain
+
+    @classmethod
+    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 `return_type` parameter.
+
+        Args:
+            domain (str): The domain to validate.
+            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:
+            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",
+            errors.DomainUnavailableError: "unavailable",
+            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:
+            # 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(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)
+
+        # Return the validated domain and the response (either error or success)
+        return (validated, response)
+
+    @staticmethod
+    def _return_form_error_or_json_response(return_type: ValidationReturnType, 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 (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.
+
+        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 ValidationReturnType.FORM_VALIDATION_ERROR:
+                raise forms.ValidationError(DOMAIN_API_MESSAGES[code], code=code)
+            case ValidationReturnType.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):
         """

src/registrar/templates/application_dotgov_domain.html

@@ -48,6 +48,7 @@
       {% endwith %}
     {% endwith %}
     <button
+      id="check-availability-button"
       type="button"
       class="usa-button"
       validate-for="{{ forms.0.requested_domain.auto_id }}"
@@ -68,9 +69,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 %}
+        {% with add_class="blank-ok alternate-domain-input" %}
-          {% input_with_errors form.alternative_domain %}
+          {% for form in forms.1 %}
-        {% endfor %}
+            {% input_with_errors form.alternative_domain %}
+          {% endfor %}
+        {% endwith %}
       {% endwith %}
     {% endwith %}
 

src/registrar/tests/test_forms.py

@@ -1,8 +1,11 @@
 """Test form validation requirements."""
 
+import json
 from django.test import TestCase, RequestFactory
+from api.views import available
 
 from registrar.forms.application_wizard import (
+    AlternativeDomainForm,
     CurrentSitesForm,
     DotGovDomainForm,
     AuthorizingOfficialForm,
@@ -23,6 +26,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 +78,113 @@ 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.”"
+                " For example, if you want www.city.gov, you would enter “city”"
+                " (without the quotes).",
+            ),
+            # unavailable
+            (
+                "whitehouse.gov",
+                "That domain isn’t available. <a class='usa-link' "
+                "href='https://beta.get.gov/domains/choosing' target='_blank'>Read more about "
+                "choosing your .gov domain</a>.",
+            ),
+        ]
+
+        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).",
+            ),
+            # unavailable
+            (
+                "whitehouse.gov",
+                "That domain isn’t available. <a class='usa-link' "
+                "href='https://beta.get.gov/domains/choosing' target='_blank'>Read more about "
+                "choosing your .gov domain</a>.",
+            ),
+        ]
+
+        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"})

src/registrar/utility/enums.py

@@ -0,0 +1,28 @@
+"""Used for holding various enums"""
+
+from enum import Enum
+
+
+class ValidationReturnType(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

src/registrar/utility/errors.py

@@ -17,6 +17,12 @@ class RegistrySystemError(ValueError):
     pass
 
 
+class InvalidDomainError(ValueError):
+    """Error class for situations where an invalid domain is supplied"""
+
+    pass
+
+
 class ActionNotAllowed(Exception):
     """User accessed an action that is not
     allowed by the current state"""