diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 42f4f5687..83bb3dd0f 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -5,6 +5,7 @@ import logging from django.core.management import BaseCommand, CommandError from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper from registrar.models import DomainInformation, DomainRequest, FederalAgency, Suborganization, Portfolio, User +from registrar.models.utility.generic_helper import normalize_string logger = logging.getLogger(__name__) @@ -51,6 +52,11 @@ class Command(BaseCommand): action=argparse.BooleanOptionalAction, help="Adds portfolio to both requests and domains", ) + parser.add_argument( + "--include_started_requests", + action=argparse.BooleanOptionalAction, + help="If parse_requests is enabled, we parse started", + ) def handle(self, **options): agency_name = options.get("agency_name") @@ -58,6 +64,7 @@ class Command(BaseCommand): parse_requests = options.get("parse_requests") parse_domains = options.get("parse_domains") both = options.get("both") + include_started_requests = options.get("include_started_requests") if not both: if not parse_requests and not parse_domains: @@ -66,6 +73,9 @@ class Command(BaseCommand): if parse_requests or parse_domains: raise CommandError("You cannot pass --parse_requests or --parse_domains when passing --both.") + if include_started_requests and not parse_requests: + raise CommandError("You must pass --parse_requests when using --include_started_requests") + federal_agency_filter = {"agency__iexact": agency_name} if agency_name else {"federal_type": branch} agencies = FederalAgency.objects.filter(**federal_agency_filter) if not agencies or agencies.count() < 1: @@ -83,7 +93,9 @@ class Command(BaseCommand): TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) try: # C901 'Command.handle' is too complex (12) - self.handle_populate_portfolio(federal_agency, parse_domains, parse_requests, both) + self.handle_populate_portfolio( + federal_agency, parse_domains, parse_requests, both, include_started_requests + ) except Exception as exec: self.failed_portfolios.add(federal_agency) logger.error(exec) @@ -99,7 +111,7 @@ class Command(BaseCommand): display_as_str=True, ) - def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, both): + def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, both, include_started_requests): """Attempts to create a portfolio. If successful, this function will also create new suborganizations""" portfolio, created = self.create_portfolio(federal_agency) @@ -109,7 +121,7 @@ class Command(BaseCommand): self.handle_portfolio_domains(portfolio, federal_agency) if parse_requests or both: - self.handle_portfolio_requests(portfolio, federal_agency) + self.handle_portfolio_requests(portfolio, federal_agency, include_started_requests) def create_portfolio(self, federal_agency): """Creates a portfolio if it doesn't presently exist. @@ -182,7 +194,7 @@ class Command(BaseCommand): for name in org_names - set(existing_suborgs.values_list("name", flat=True)): # Stored in variables due to linter wanting type information here. portfolio_name: str = portfolio.organization_name if portfolio.organization_name is not None else "" - if name is not None and name.lower() == portfolio_name.lower(): + if name is not None and normalize_string(name) == normalize_string(portfolio_name): # You can use this to populate location information, when this occurs. # However, this isn't needed for now so we can skip it. message = ( @@ -191,7 +203,7 @@ class Command(BaseCommand): ) TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) else: - new_suborgs.append(Suborganization(name=name, portfolio=portfolio)) # type: ignore + new_suborgs.append(Suborganization(name=normalize_string(name, lowercase=False), portfolio=portfolio)) # type: ignore if new_suborgs: Suborganization.objects.bulk_create(new_suborgs) @@ -201,16 +213,18 @@ class Command(BaseCommand): else: TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, "No suborganizations added") - def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency): + def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency, include_started_requests): """ Associate portfolio with domain requests for a federal agency. Updates all relevant domain request records. """ invalid_states = [ - DomainRequest.DomainRequestStatus.STARTED, DomainRequest.DomainRequestStatus.INELIGIBLE, DomainRequest.DomainRequestStatus.REJECTED, ] + if not include_started_requests: + invalid_states.append(DomainRequest.DomainRequestStatus.STARTED) + domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency, portfolio__isnull=True).exclude( status__in=invalid_states ) @@ -229,7 +243,14 @@ class Command(BaseCommand): # Get all suborg information and store it in a dict to avoid doing a db call suborgs = Suborganization.objects.filter(portfolio=portfolio).in_bulk(field_name="name") for domain_request in domain_requests: + # Set the portfolio domain_request.portfolio = portfolio + + # Conditionally clear federal agency if the org name is the same as the portfolio name. + if include_started_requests: + domain_request.sync_portfolio_and_federal_agency_for_started_requests() + + # Set suborg info if domain_request.organization_name in suborgs: domain_request.sub_organization = suborgs.get(domain_request.organization_name) else: @@ -254,6 +275,7 @@ class Command(BaseCommand): "requested_suborganization", "suborganization_city", "suborganization_state_territory", + "federal_agency", ], ) message = f"Added portfolio '{portfolio}' to {len(domain_requests)} domain requests." diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 3d3aac769..617c96737 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -8,7 +8,7 @@ from django_fsm import FSMField, transition # type: ignore from django.utils import timezone from registrar.models.domain import Domain from registrar.models.federal_agency import FederalAgency -from registrar.models.utility.generic_helper import CreateOrUpdateOrganizationTypeHelper +from registrar.models.utility.generic_helper import CreateOrUpdateOrganizationTypeHelper, normalize_string from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes from registrar.utility.constants import BranchChoices from auditlog.models import LogEntry @@ -729,6 +729,7 @@ class DomainRequest(TimeStampedModel): """Save override for custom properties""" self.sync_organization_type() self.sync_yes_no_form_fields() + self.sync_portfolio_and_federal_agency_for_started_requests() if self._cached_status != self.status: self.last_status_update = timezone.now().date() @@ -744,6 +745,22 @@ class DomainRequest(TimeStampedModel): # Update the cached values after saving self._cache_status_and_status_reasons() + def sync_portfolio_and_federal_agency_for_started_requests(self) -> bool: + """ + Prevents duplicate organization data by clearing the federal_agency field if it matches the portfolio. + Only runs on STARTED requests. + + Returns a bool indicating if the federal agency was changed or not. + """ + agency_name = getattr(self.federal_agency, "agency", None) + portfolio_name = getattr(self.portfolio, "organization_name", None) + if portfolio_name and agency_name and self.status == DomainRequest.DomainRequestStatus.STARTED: + if normalize_string(agency_name) == normalize_string(portfolio_name): + self.federal_agency = None + return True + + return False + def create_requested_suborganization(self): """Creates the requested suborganization. Adds the name, portfolio, city, and state_territory fields. diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 5e425f5a3..7aa4e62bc 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -343,3 +343,8 @@ def value_of_attribute(obj, attribute_name: str): if callable(value): value = value() return value + +def normalize_string(string_to_normalize: str, lowercase=True) -> str: + """Normalizes a given string. Returns a string without extra spaces, in all lowercase.""" + new_string = " ".join(string_to_normalize.split()) + return new_string.lower() if lowercase else new_string diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 5c3ab4ba3..418a22411 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1424,6 +1424,7 @@ class TestCreateFederalPortfolio(TestCase): # Create an agency wih no federal type (can only be created via specifiying it manually) self.federal_agency = FederalAgency.objects.create(agency="Test Federal Agency") + self.federal_agency_2 = FederalAgency.objects.create(agency="Sugarcane") # And create some with federal_type ones with creative names self.executive_agency_1 = FederalAgency.objects.create( @@ -1516,6 +1517,48 @@ class TestCreateFederalPortfolio(TestCase): ): call_command("create_federal_portfolio", **kwargs) + @less_console_noise_decorator + def test_handle_portfolio_requests_sync_federal_agency(self): + """Test that federal agency is cleared when org name matches portfolio name""" + + # Create a domain request with matching org name + matching_request = completed_domain_request( + name="matching.gov", + status=DomainRequest.DomainRequestStatus.STARTED, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=self.federal_agency_2, + user=self.user, + ) + + # Create a request not in started (no change should occur) + matching_request_in_wrong_status = completed_domain_request( + name="kinda-matching.gov", + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=self.federal_agency, + user=self.user, + ) + + self.run_create_federal_portfolio(agency_name="Sugarcane", parse_requests=True, include_started_requests=True) + self.run_create_federal_portfolio( + agency_name="Test Federal Agency", parse_requests=True, include_started_requests=True + ) + + # Refresh from db + matching_request.refresh_from_db() + matching_request_in_wrong_status.refresh_from_db() + + # Request with matching name should have federal_agency cleared + self.assertIsNone(matching_request.federal_agency) + self.assertIsNotNone(matching_request.portfolio) + self.assertEqual(matching_request.portfolio.organization_name, "Sugarcane") + + # Request with matching name but wrong state should keep its federal agency + self.assertEqual(matching_request_in_wrong_status.federal_agency, self.federal_agency) + self.assertIsNotNone(matching_request_in_wrong_status.portfolio) + self.assertEqual(matching_request_in_wrong_status.portfolio.organization_name, "Test Federal Agency") + + @less_console_noise_decorator def test_create_single_portfolio(self): """Test portfolio creation with suborg and senior official.""" self.run_create_federal_portfolio(agency_name="Test Federal Agency", parse_requests=True) diff --git a/src/registrar/tests/test_models_requests.py b/src/registrar/tests/test_models_requests.py index 983a12b3c..0c674d417 100644 --- a/src/registrar/tests/test_models_requests.py +++ b/src/registrar/tests/test_models_requests.py @@ -1074,6 +1074,38 @@ class TestDomainRequest(TestCase): self.assertEqual(domain_request2.generic_org_type, domain_request2.converted_generic_org_type) self.assertEqual(domain_request2.federal_agency, domain_request2.converted_federal_agency) + def test_sync_portfolio_and_federal_agency_for_started_requests(self): + """Tests the sync_portfolio_and_federal_agency_for_started_requests function""" + # Create a federal agency with a "bad" name + user = create_user() + federal_agency_2 = FederalAgency.objects.create(agency="Sugarcane ") + request = completed_domain_request( + name="matching.gov", + status=DomainRequest.DomainRequestStatus.STARTED, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=federal_agency_2, + user=user, + ) + self.assertIsNone(request.portfolio) + + # Nothing should happen on normal save + request.notes = "test change" + request.save() + self.assertEqual(request.federal_agency, federal_agency_2) + + # But when a portfolio exists, the federal agency should be cleared if its a duplicate + portfolio = Portfolio.objects.create(organization_name="sugarcane", creator=user) + request.portfolio = portfolio + request.save() + self.assertIsNone(request.federal_agency) + + # However -- this change should only occur if the names match (when normalized) + portfolio = Portfolio.objects.create(organization_name="some other name", creator=user) + request.portfolio = portfolio + request.federal_agency = federal_agency_2 + request.save() + self.assertEqual(request.federal_agency, federal_agency_2) + class TestDomainRequestSuborganization(TestCase): """Tests for the suborganization fields on domain requests"""