diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 1476cee7e..52e214bb9 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -2003,6 +2003,9 @@ class DomainRequestAdmin(ListHeaderAdmin, ImportExportModelAdmin): custom_requested_domain.admin_order_field = "requested_domain__name" # type: ignore + # ------ Converted fields ------ + # These fields map to @Property methods and + # require these custom definitions to work properly @admin.display(description=_("Generic Org Type")) def converted_generic_org_type(self, obj): return obj.converted_generic_org_type_display diff --git a/src/registrar/assets/src/js/getgov-admin/domain-request-form.js b/src/registrar/assets/src/js/getgov-admin/domain-request-form.js index d82b2de8b..b3d14839e 100644 --- a/src/registrar/assets/src/js/getgov-admin/domain-request-form.js +++ b/src/registrar/assets/src/js/getgov-admin/domain-request-form.js @@ -645,7 +645,6 @@ function handleSuborgFieldsAndButtons() { const suborganizationCity = document.getElementById("id_suborganization_city"); const suborganizationStateTerritory = document.getElementById("id_suborganization_state_territory"); const rejectButton = document.querySelector("#clear-requested-suborganization"); - console.log("test12345678") // Ensure that every variable is present before proceeding if (!requestedSuborganizationField || !suborganizationCity || !suborganizationStateTerritory || !rejectButton) { diff --git a/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js b/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js index 55b42d10d..9a60e1684 100644 --- a/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js +++ b/src/registrar/assets/src/js/getgov-admin/helpers-portfolio-dynamic-fields.js @@ -495,6 +495,7 @@ export function handlePortfolioSelection( if (requestedSuborganizationField) hideElement(requestedSuborganizationField); if (suborganizationCity) hideElement(suborganizationCity); if (suborganizationStateTerritory) hideElement(suborganizationStateTerritory); + // == LOGIC FOR THE DOMAIN REQUEST PAGE == // if (rejectSuborganizationButton) hideElement(rejectSuborganizationButton); } diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index 003714dfb..6eb2fac07 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -244,7 +244,6 @@ class Domain(TimeStampedModel, DomainHelper): is called in the validate function on the request/domain page throws- RegistryError or InvalidDomainError""" - return True if not cls.string_could_be_domain(domain): logger.warning("Not a valid domain: %s" % str(domain)) # throw invalid domain error so that it can be caught in diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index f4e2a35a1..533a22ced 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -697,10 +697,18 @@ class DomainRequest(TimeStampedModel): portfolio__isnull=False, ).exists() ): - raise ValidationError( + # Add a field-level error to requested_suborganization. + # To pass in field-specific errors, we need to embed a dict of + # field: validationerror then pass that into a validation error itself. + # This is slightly confusing, but it just adds it at that level. + msg = ( "This suborganization already exists. " "Choose a new name, or select it directly if you would like to use it." ) + errors = { + "requested_suborganization": ValidationError(msg) + } + raise ValidationError(errors) elif self.portfolio and not self.sub_organization: # You cannot create a new suborganization without these fields required_suborg_fields = { @@ -708,16 +716,16 @@ class DomainRequest(TimeStampedModel): "suborganization_city": self.suborganization_city, "suborganization_state_territory": self.suborganization_state_territory, } - + # If at least one value is populated, enforce a all-or-nothing rule if any(bool(value) for value in required_suborg_fields.values()): - # Find which fields are empty - errors_dict = { - field_name: [f"This field is required when creating a new suborganization."] - for field_name, value in required_suborg_fields.items() - if not value - } - # Adds a validation error to each missing field - raise ValidationError({k: ValidationError(v, code="required") for k, v in errors_dict.items()}) + # Find which fields are empty and throw an error on the field + errors = {} + for field_name, value in required_suborg_fields.items(): + if not value: + errors[field_name] = ValidationError( + "This field is required when creating a new suborganization.", + ) + raise ValidationError(errors) def save(self, *args, **kwargs): """Save override for custom properties""" diff --git a/src/registrar/tests/test_admin_request.py b/src/registrar/tests/test_admin_request.py index 439f4fab0..a294c127f 100644 --- a/src/registrar/tests/test_admin_request.py +++ b/src/registrar/tests/test_admin_request.py @@ -1,5 +1,7 @@ from datetime import datetime +from django.forms import ValidationError from django.utils import timezone +from waffle.testutils import override_flag import re from django.test import RequestFactory, Client, TestCase, override_settings from django.contrib.admin.sites import AdminSite @@ -25,6 +27,7 @@ from registrar.models import ( Portfolio, AllowedEmail, ) +from registrar.models.suborganization import Suborganization from .common import ( MockSESClient, completed_domain_request, @@ -82,6 +85,7 @@ class TestDomainRequestAdmin(MockEppLib): Contact.objects.all().delete() Website.objects.all().delete() SeniorOfficial.objects.all().delete() + Suborganization.objects.all().delete() Portfolio.objects.all().delete() self.mock_client.EMAILS_SENT.clear() @@ -91,6 +95,99 @@ class TestDomainRequestAdmin(MockEppLib): User.objects.all().delete() AllowedEmail.objects.all().delete() + @override_flag("organization_feature", active=True) + @less_console_noise_decorator + def test_clean_validates_duplicate_suborganization(self): + """Tests that clean() prevents duplicate suborganization names within the same portfolio""" + # Create a portfolio and existing suborganization + portfolio = Portfolio.objects.create( + organization_name="Test Portfolio", + creator=self.superuser + ) + + # Create an existing suborganization + Suborganization.objects.create( + name="Existing Suborg", + portfolio=portfolio + ) + + # Create a domain request trying to use the same suborganization name + # (intentionally lowercase) + domain_request = completed_domain_request( + name="test1234.gov", + portfolio=portfolio, + requested_suborganization="existing suborg", + suborganization_city="Rome", + suborganization_state_territory=DomainRequest.StateTerritoryChoices.OHIO, + ) + + # Assert that the validation error is raised + with self.assertRaises(ValidationError) as err: + domain_request.clean() + + self.assertIn( + "This suborganization already exists", + str(err.exception) + ) + + # Test that a different name is allowed. Should not raise a error. + domain_request.requested_suborganization = "New Suborg" + domain_request.clean() + + @less_console_noise_decorator + @override_flag("organization_feature", active=True) + def test_clean_validates_partial_suborganization_fields(self): + """Tests that clean() enforces all-or-nothing rule for suborganization fields""" + portfolio = Portfolio.objects.create( + organization_name="Test Portfolio", + creator=self.superuser + ) + + # Create domain request with only city filled out + domain_request = completed_domain_request( + name="test1234.gov", + portfolio=portfolio, + suborganization_city="Test City", + ) + + # Assert validation error is raised with correct missing fields + with self.assertRaises(ValidationError) as err: + domain_request.clean() + + error_dict = err.exception.error_dict + expected_missing = ["requested_suborganization", "suborganization_state_territory"] + + # Verify correct fields are flagged as required + self.assertEqual( + sorted(error_dict.keys()), + sorted(expected_missing) + ) + + # Verify error message + for field in expected_missing: + self.assertEqual( + str(error_dict[field][0].message), + "This field is required when creating a new suborganization." + ) + + # When all data is passed in, this should validate correctly + domain_request.requested_suborganization = "Complete Suborg" + domain_request.suborganization_state_territory = DomainRequest.StateTerritoryChoices.OHIO + # Assert that no ValidationError is raised + try: + domain_request.clean() + except ValidationError as e: + self.fail(f"ValidationError was raised unexpectedly: {e}") + + # Also ensure that no validation error is raised if nothing is passed in at all + domain_request.suborganization_city = None + domain_request.requested_suborganization = None + domain_request.suborganization_state_territory = None + try: + domain_request.clean() + except ValidationError as e: + self.fail(f"ValidationError was raised unexpectedly: {e}") + @less_console_noise_decorator def test_domain_request_senior_official_is_alphabetically_sorted(self): """Tests if the senior offical dropdown is alphanetically sorted in the django admin display"""