diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 4f74b4163..df9a969df 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -4,7 +4,6 @@ import time import logging from urllib.parse import urlparse, urlunparse, urlencode - logger = logging.getLogger(__name__) @@ -82,9 +81,13 @@ class CreateOrUpdateOrganizationTypeHelper: def _handle_new_instance(self): # == Check for invalid conditions before proceeding == # - should_proceed = self._validate_new_instance() - if not should_proceed: + try: + should_proceed = self._validate_new_instance() + if not should_proceed: + return None + except ValueError: return None + # == Program flow will halt here if there is no reason to update == # # == Update the linked values == # @@ -104,6 +107,7 @@ class CreateOrUpdateOrganizationTypeHelper: def _handle_existing_instance(self, force_update_when_no_changes_are_found=False): # == Init variables == # try: + # Instance is already in the database, fetch its current state current_instance = self.sender.objects.get(id=self.instance.id) @@ -174,8 +178,8 @@ class CreateOrUpdateOrganizationTypeHelper: self.instance.organization_type = generic_org_type else: # This can only happen with manual data tinkering, which causes these to be out of sync. - if self.instance.is_election_board is None: - self.instance.is_election_board = False + # if self.instance.is_election_board is None: + # self.instance.is_election_board = False if self.instance.is_election_board: self.instance.organization_type = self.generic_org_to_org_map[generic_org_type] @@ -219,12 +223,15 @@ class CreateOrUpdateOrganizationTypeHelper: self.instance.is_election_board = None self.instance.generic_org_type = None - def _validate_new_instance(self): + def _validate_new_instance(self) -> bool: """ Validates whether a new instance of DomainRequest or DomainInformation can proceed with the update based on the consistency between organization_type, generic_org_type, and is_election_board. Returns a boolean determining if execution should proceed or not. + + Raises: + ValueError if there is a mismatch between organization_type, generic_org_type, and is_election_board """ # We conditionally accept both of these values to exist simultaneously, as long as @@ -249,6 +256,7 @@ class CreateOrUpdateOrganizationTypeHelper: "Cannot add organization_type and generic_org_type simultaneously " "when generic_org_type, is_election_board, and organization_type values do not match." ) + logger.error("_validate_new_instance: %s", message) raise ValueError(message) return True diff --git a/src/registrar/tests/test_models.py b/src/registrar/tests/test_models.py index b50525e27..9b80e556d 100644 --- a/src/registrar/tests/test_models.py +++ b/src/registrar/tests/test_models.py @@ -1495,13 +1495,6 @@ class TestDomainRequestCustomSave(TestCase): self.assertEqual(domain_request.is_election_board, False) self.assertEqual(domain_request.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY) - # Try reverting setting an invalid value for election board (should revert to False) - domain_request.is_election_board = None - domain_request.save() - - self.assertEqual(domain_request.is_election_board, False) - self.assertEqual(domain_request.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY) - @less_console_noise_decorator def test_create_or_update_organization_type_existing_instance_updates_generic_org_type(self): """Test create_or_update_organization_type when modifying generic_org_type on an existing instance.""" @@ -1654,13 +1647,6 @@ class TestDomainInformationCustomSave(TestCase): self.assertEqual(domain_information.is_election_board, False) self.assertEqual(domain_information.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY) - # Try reverting setting an invalid value for election board (should revert to False) - domain_information.is_election_board = None - domain_information.save() - - self.assertEqual(domain_information.is_election_board, False) - self.assertEqual(domain_information.organization_type, DomainRequest.OrgChoicesElectionOffice.CITY) - @less_console_noise_decorator def test_create_or_update_organization_type_existing_instance_updates_generic_org_type(self): """Test create_or_update_organization_type when modifying generic_org_type on an existing instance.""" @@ -1858,8 +1844,7 @@ class TestDomainRequestIncomplete(TestCase): self.assertTrue(self.domain_request._is_state_or_territory_complete()) self.domain_request.is_election_board = None self.domain_request.save() - # is_election_board will overwrite to False bc of _update_org_type_from_generic_org_and_election - self.assertTrue(self.domain_request._is_state_or_territory_complete()) + self.assertFalse(self.domain_request._is_state_or_territory_complete()) @less_console_noise_decorator def test_is_tribal_complete(self): @@ -1882,8 +1867,7 @@ class TestDomainRequestIncomplete(TestCase): self.assertTrue(self.domain_request._is_county_complete()) self.domain_request.is_election_board = None self.domain_request.save() - # is_election_board will overwrite to False bc of _update_org_type_from_generic_org_and_election - self.assertTrue(self.domain_request._is_county_complete()) + self.assertFalse(self.domain_request._is_county_complete()) @less_console_noise_decorator def test_is_city_complete(self): @@ -1893,8 +1877,7 @@ class TestDomainRequestIncomplete(TestCase): self.assertTrue(self.domain_request._is_city_complete()) self.domain_request.is_election_board = None self.domain_request.save() - # is_election_board will overwrite to False bc of _update_org_type_from_generic_org_and_election - self.assertTrue(self.domain_request._is_city_complete()) + self.assertFalse(self.domain_request._is_city_complete()) @less_console_noise_decorator def test_is_special_district_complete(self): diff --git a/src/registrar/tests/test_views_request.py b/src/registrar/tests/test_views_request.py index 0cee9d563..efad41d8f 100644 --- a/src/registrar/tests/test_views_request.py +++ b/src/registrar/tests/test_views_request.py @@ -2979,7 +2979,7 @@ class TestWizardUnlockingSteps(TestWithUser, WebTest): expected_dict = [] self.assertEqual(unlocked_steps, expected_dict) - @less_console_noise_decorator + # @less_console_noise_decorator def test_unlocked_steps_full_domain_request(self): """Test when all fields in the domain request are filled."""