From 5ca74fdc5374b864db3aa6e32b6d2c5ebed3882c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 Jan 2025 10:38:50 -0700 Subject: [PATCH] Finish unit tests --- .../commands/create_federal_portfolio.py | 32 ++++++++---- .../tests/test_management_scripts.py | 50 +++++++++++++++++-- 2 files changed, 68 insertions(+), 14 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 8ff32824b..ddcb5a4a9 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -79,7 +79,7 @@ class Command(BaseCommand): else: raise CommandError(f"Cannot find '{branch}' federal agencies in our database.") - portfolio_set = set() + portfolios = [] for federal_agency in agencies: message = f"Processing federal agency '{federal_agency.agency}'..." TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) @@ -87,7 +87,7 @@ class Command(BaseCommand): # C901 'Command.handle' is too complex (12) # We currently only grab the list of changed domain requests, but we may want to grab the domains too portfolio = self.handle_populate_portfolio(federal_agency, parse_domains, parse_requests, both) - portfolio_set.add(portfolio) + portfolios.append(portfolio) except Exception as exec: self.failed_portfolios.add(federal_agency) logger.error(exec) @@ -106,9 +106,14 @@ class Command(BaseCommand): # POST PROCESSING STEP: Remove the federal agency if it matches the portfolio name. # We only do this for started domain requests. if parse_requests: - self.post_process_started_domain_requests(portfolio_set) + TerminalHelper.prompt_for_execution( + system_exit_on_terminate=True, + prompt_message="This action will update domain requests even if they aren't on a portfolio.", + prompt_title="Do you want to clear federal agency on started domain requests?", + ) + self.post_process_started_domain_requests(agencies, portfolios) - def post_process_started_domain_requests(self, portfolio_set): + def post_process_started_domain_requests(self, agencies, portfolios): """ Removes duplicate organization data by clearing federal_agency when it matches the portfolio name. Only processes domain requests in STARTED status. @@ -116,15 +121,24 @@ class Command(BaseCommand): message = "Removing duplicate portfolio and federal_agency values from domain requests..." TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) + # For each request, clear the federal agency under these conditions: + # 1. A portfolio *already exists* with the same name as the federal agency. + # 2. Said portfolio (or portfolios) are only the ones specified at the start of the script. + # 3. The domain request is in status "started". + # Note: Both names are normalized so excess spaces are stripped and the string is lowercased. domain_requests_to_update = DomainRequest.objects.filter( - portfolio__in=portfolio_set, - status=DomainRequest.DomainRequestStatus.STARTED, + federal_agency__in=agencies, federal_agency__agency__isnull=False, - portfolio__organization_name__isnull=False, + status=DomainRequest.DomainRequestStatus.STARTED, + organization_name__isnull=False, ) + portfolio_set = {normalize_string(portfolio.organization_name) for portfolio in portfolios if portfolio} + + # Update the request, assuming the given agency name matches the portfolio name updated_requests = [] for req in domain_requests_to_update: - if normalize_string(req.federal_agency.agency) == normalize_string(req.portfolio.organization_name): + agency_name = normalize_string(req.federal_agency.agency) + if agency_name in portfolio_set: req.federal_agency = None updated_requests.append(req) DomainRequest.objects.bulk_update(updated_requests, ["federal_agency"]) @@ -140,8 +154,6 @@ class Command(BaseCommand): def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, both): """Attempts to create a portfolio. If successful, this function will also create new suborganizations. - - Returns the processed portfolio """ portfolio, created = self.create_portfolio(federal_agency) if created: diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 882396f1e..8ecb7cbea 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1424,7 +1424,6 @@ 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( @@ -1518,8 +1517,13 @@ 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""" + def test_post_process_started_domain_requests_existing_portfolio(self): + """Ensures that federal agency is cleared when agency name matches portfolio name. + As the name implies, this implicitly tests the "post_process_started_domain_requests" function. + """ + federal_agency_2 = FederalAgency.objects.create(agency="Sugarcane", federal_type=BranchChoices.EXECUTIVE) + + # Test records with portfolios and no org names # Create a portfolio. This script skips over "started" portfolio = Portfolio.objects.create(organization_name="Sugarcane", creator=self.user) # Create a domain request with matching org name @@ -1527,7 +1531,7 @@ class TestCreateFederalPortfolio(TestCase): name="matching.gov", status=DomainRequest.DomainRequestStatus.STARTED, generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, - federal_agency=self.federal_agency_2, + federal_agency=federal_agency_2, user=self.user, portfolio=portfolio, ) @@ -1558,6 +1562,44 @@ class TestCreateFederalPortfolio(TestCase): 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_post_process_started_domain_requests(self): + """Tests that federal agency is cleared when agency name + matches an existing portfolio's name, even if the domain request isn't + directly on that portfolio.""" + + federal_agency_2 = FederalAgency.objects.create(agency="Sugarcane", federal_type=BranchChoices.EXECUTIVE) + + # Create a request with matching federal_agency name but no direct portfolio association + matching_agency_request = completed_domain_request( + name="agency-match.gov", + status=DomainRequest.DomainRequestStatus.STARTED, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=federal_agency_2, + user=self.user, + ) + + # Create a control request that shouldn't match + non_matching_request = completed_domain_request( + name="no-match.gov", + status=DomainRequest.DomainRequestStatus.STARTED, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=self.federal_agency, + user=self.user, + ) + + # We expect the matching agency to have its fed agency cleared. + self.run_create_federal_portfolio(agency_name="Sugarcane", parse_requests=True) + matching_agency_request.refresh_from_db() + non_matching_request.refresh_from_db() + + # Request with matching agency name should have federal_agency cleared + self.assertIsNone(matching_agency_request.federal_agency) + + # Non-matching request should keep its federal_agency + self.assertIsNotNone(non_matching_request.federal_agency) + self.assertEqual(non_matching_request.federal_agency, self.federal_agency) + @less_console_noise_decorator def test_create_single_portfolio(self): """Test portfolio creation with suborg and senior official."""