Merge branch 'main' into litterbox/3280-design-review

docs/operations/data_migration.md

@@ -908,12 +908,13 @@ Example (only requests): `./manage.py create_federal_portfolio --branch "executi
 
 ##### Parameters
 |   | Parameter                    | Description                                                                                |
-|:-:|:-------------------------- |:-------------------------------------------------------------------------------------------|
+|:-:|:---------------------------- |:-------------------------------------------------------------------------------------------|
 | 1 | **agency_name**              | Name of the FederalAgency record surrounded by quotes. For instance,"AMTRAK".              |
 | 2 | **branch**                   | Creates a portfolio for each federal agency in a branch: executive, legislative, judicial  |
 | 3 | **both**                     | If True, runs parse_requests and parse_domains.                                            |
 | 4 | **parse_requests**           | If True, then the created portfolio is added to all related DomainRequests.                |
 | 5 | **parse_domains**            | If True, then the created portfolio is added to all related Domains.                       |
+| 6 | **skip_existing_portfolios** | If True, then the script will only create suborganizations, modify DomainRequest, and modify DomainInformation records only when creating a new portfolio. Use this flag when you do not want to modify existing records.       |
 
 - Parameters #1-#2: Either `--agency_name` or `--branch` must be specified. Not both.
 - Parameters #2-#3, you cannot use `--both` while using these. You must specify either `--parse_requests` or `--parse_domains` seperately. While all of these parameters are optional in that you do not need to specify all of them,

src/registrar/management/commands/create_federal_portfolio.py

@@ -64,6 +64,11 @@ class Command(BaseCommand):
             action=argparse.BooleanOptionalAction,
             help="Adds portfolio to both requests and domains",
         )
+        parser.add_argument(
+            "--skip_existing_portfolios",
+            action=argparse.BooleanOptionalAction,
+            help="Only add suborganizations to newly created portfolios, skip existing ones.",
+        )
 
     def handle(self, **options):
         agency_name = options.get("agency_name")
@@ -71,6 +76,7 @@ class Command(BaseCommand):
         parse_requests = options.get("parse_requests")
         parse_domains = options.get("parse_domains")
         both = options.get("both")
+        skip_existing_portfolios = options.get("skip_existing_portfolios")
 
         if not both:
             if not parse_requests and not parse_domains:
@@ -97,7 +103,9 @@ class Command(BaseCommand):
             TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message)
             try:
                 # C901 'Command.handle' is too complex (12)
-                portfolio = self.handle_populate_portfolio(federal_agency, parse_domains, parse_requests, both)
+                portfolio = self.handle_populate_portfolio(
+                    federal_agency, parse_domains, parse_requests, both, skip_existing_portfolios
+                )
                 portfolios.append(portfolio)
             except Exception as exec:
                 self.failed_portfolios.add(federal_agency)
@@ -109,26 +117,33 @@ class Command(BaseCommand):
         updated_suborg_count = self.post_process_all_suborganization_fields(agencies)
         message = f"Added city and state_territory information to {updated_suborg_count} suborgs."
         TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message)
-
         TerminalHelper.log_script_run_summary(
             self.updated_portfolios,
             self.failed_portfolios,
             self.skipped_portfolios,
             debug=False,
-            skipped_header="----- SOME PORTFOLIOS WERENT CREATED -----",
+            log_header="============= FINISHED HANDLE PORTFOLIO STEP ===============",
+            skipped_header="----- SOME PORTFOLIOS WERENT CREATED (BUT OTHER RECORDS ARE STILL PROCESSED) -----",
             display_as_str=True,
         )
 
         # POST PROCESSING STEP: Remove the federal agency if it matches the portfolio name.
         # We only do this for started domain requests.
         if parse_requests or both:
+            prompt_message = (
+                "This action will update domain requests even if they aren't on a portfolio."
+                "\nNOTE: This will modify domain requests, even if no portfolios were created."
+                "\nIn the event no portfolios *are* created, then this step will target "
+                "the existing portfolios with your given params."
+                "\nThis step is entirely optional, and is just for extra data cleanup."
+            )
             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_message=prompt_message,
                 prompt_title=(
                     "POST PROCESS STEP: Do you want to clear federal agency on (related) started domain requests?"
                 ),
-                verify_message=None,
+                verify_message="*** THIS STEP IS OPTIONAL ***",
             )
             self.post_process_started_domain_requests(agencies, portfolios)
 
@@ -151,6 +166,11 @@ class Command(BaseCommand):
             status=DomainRequest.DomainRequestStatus.STARTED,
             organization_name__isnull=False,
         )
+
+        if domain_requests_to_update.count() == 0:
+            TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, "No domain requests to update.")
+            return
+
         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
@@ -173,10 +193,19 @@ class Command(BaseCommand):
             DomainRequest.objects.bulk_update(updated_requests, ["federal_agency"])
             TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, "Action completed successfully.")
 
-    def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, both):
+    def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, both, skip_existing_portfolios):
         """Attempts to create a portfolio. If successful, this function will
         also create new suborganizations"""
-        portfolio, _ = self.create_portfolio(federal_agency)
+        portfolio, created = self.create_portfolio(federal_agency)
+        if skip_existing_portfolios and not created:
+            TerminalHelper.colorful_logger(
+                logger.warning,
+                TerminalColors.YELLOW,
+                "Skipping modifications to suborgs, domain requests, and "
+                "domains due to the --skip_existing_portfolios flag. Portfolio already exists.",
+            )
+            return portfolio
+
         self.create_suborganizations(portfolio, federal_agency)
         if parse_domains or both:
             self.handle_portfolio_domains(portfolio, federal_agency)
@@ -283,15 +312,13 @@ class Command(BaseCommand):
             DomainRequest.DomainRequestStatus.INELIGIBLE,
             DomainRequest.DomainRequestStatus.REJECTED,
         ]
-        domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency, portfolio__isnull=True).exclude(
+        domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency).exclude(status__in=invalid_states)
-            status__in=invalid_states
-        )
         if not domain_requests.exists():
             message = f"""
             Portfolio '{portfolio}' not added to domain requests: no valid records found.
             This means that a filter on DomainInformation for the federal_agency '{federal_agency}' returned no results.
             Excluded statuses: STARTED, INELIGIBLE, REJECTED.
-            Filter info: DomainRequest.objects.filter(federal_agency=federal_agency, portfolio__isnull=True).exclude(
+            Filter info: DomainRequest.objects.filter(federal_agency=federal_agency).exclude(
                 status__in=invalid_states
             )
             """
@@ -335,12 +362,12 @@ class Command(BaseCommand):
 
         Returns a queryset of DomainInformation objects, or None if nothing changed.
         """
-        domain_infos = DomainInformation.objects.filter(federal_agency=federal_agency, portfolio__isnull=True)
+        domain_infos = DomainInformation.objects.filter(federal_agency=federal_agency)
         if not domain_infos.exists():
             message = f"""
             Portfolio '{portfolio}' not added to domains: no valid records found.
             The filter on DomainInformation for the federal_agency '{federal_agency}' returned no results.
-            Filter info: DomainInformation.objects.filter(federal_agency=federal_agency, portfolio__isnull=True)
+            Filter info: DomainInformation.objects.filter(federal_agency=federal_agency)
             """
             TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message)
             return None

src/registrar/tests/test_management_scripts.py

@@ -35,7 +35,13 @@ import tablib
 from unittest.mock import patch, call, MagicMock, mock_open
 from epplibwrapper import commands, common
 
-from .common import MockEppLib, less_console_noise, completed_domain_request, MockSESClient, MockDbForIndividualTests
+from .common import (
+    MockEppLib,
+    less_console_noise,
+    completed_domain_request,
+    MockSESClient,
+    MockDbForIndividualTests,
+)
 from api.tests.common import less_console_noise_decorator
 
 
@@ -1825,7 +1831,7 @@ class TestCreateFederalPortfolio(TestCase):
             self.run_create_federal_portfolio(agency_name="Non-existent Agency", parse_requests=True)
 
     def test_does_not_update_existing_portfolio(self):
-        """Tests that an existing portfolio is not updated"""
+        """Tests that an existing portfolio is not updated when"""
         # Create an existing portfolio
         existing_portfolio = Portfolio.objects.create(
             federal_agency=self.federal_agency,
@@ -1848,6 +1854,71 @@ class TestCreateFederalPortfolio(TestCase):
         self.assertEqual(existing_portfolio.notes, "Old notes")
         self.assertEqual(existing_portfolio.creator, self.user)
 
+    def test_skip_existing_portfolios(self):
+        """Tests the skip_existing_portfolios to ensure that it doesn't add
+        suborgs, domain requests, and domain info."""
+        # Create an existing portfolio with a suborganization
+        existing_portfolio = Portfolio.objects.create(
+            federal_agency=self.federal_agency,
+            organization_name="Test Federal Agency",
+            organization_type=DomainRequest.OrganizationChoices.CITY,
+            creator=self.user,
+            notes="Old notes",
+        )
+
+        existing_suborg = Suborganization.objects.create(
+            portfolio=existing_portfolio, name="Existing Suborg", city="Old City", state_territory="CA"
+        )
+
+        # Create a domain request that would normally be associated
+        domain_request = completed_domain_request(
+            name="wackytaco.gov",
+            status=DomainRequest.DomainRequestStatus.IN_REVIEW,
+            generic_org_type=DomainRequest.OrganizationChoices.FEDERAL,
+            federal_agency=self.federal_agency,
+            user=self.user,
+            organization_name="would_create_suborg",
+        )
+        domain_request.approve()
+        domain = Domain.objects.get(name="wackytaco.gov").domain_info
+
+        # Run the command with skip_existing_portfolios=True
+        self.run_create_federal_portfolio(
+            agency_name="Test Federal Agency", parse_requests=True, skip_existing_portfolios=True
+        )
+
+        # Refresh objects from database
+        existing_portfolio.refresh_from_db()
+        existing_suborg.refresh_from_db()
+        domain_request.refresh_from_db()
+        domain.refresh_from_db()
+
+        # Verify nothing was changed on the portfolio itself
+        # SANITY CHECK: if the portfolio updates, it will change to FEDERAL.
+        # if this case fails, it means we are overriding data (and not simply just other weirdness)
+        self.assertNotEqual(existing_portfolio.organization_type, DomainRequest.OrganizationChoices.FEDERAL)
+
+        # Notes and creator should be untouched
+        self.assertEqual(existing_portfolio.organization_type, DomainRequest.OrganizationChoices.CITY)
+        self.assertEqual(existing_portfolio.organization_name, self.federal_agency.agency)
+        self.assertEqual(existing_portfolio.notes, "Old notes")
+        self.assertEqual(existing_portfolio.creator, self.user)
+
+        # Verify suborganization wasn't modified
+        self.assertEqual(existing_suborg.city, "Old City")
+        self.assertEqual(existing_suborg.state_territory, "CA")
+
+        # Verify that the domain request wasn't modified
+        self.assertIsNone(domain_request.portfolio)
+        self.assertIsNone(domain_request.sub_organization)
+
+        # Verify that the domain wasn't modified
+        self.assertIsNone(domain.portfolio)
+        self.assertIsNone(domain.sub_organization)
+
+        # Verify that a new suborg wasn't created
+        self.assertFalse(Suborganization.objects.filter(name="would_create_suborg").exists())
+
     @less_console_noise_decorator
     def test_post_process_suborganization_fields(self):
         """Test suborganization field updates from domain and request data.