From 725441da70e4528a5b446fabe53de9beb244e2c1 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Mon, 25 Nov 2024 14:59:53 -0700
Subject: [PATCH 1/9] Allow create portfolio script to run for multiple

---
 .../commands/create_federal_portfolio.py      | 139 ++++++++----------
 .../commands/utility/terminal_helper.py       |   7 +-
 2 files changed, 66 insertions(+), 80 deletions(-)

diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py
index d05a2911b..e5f4be61c 100644
--- a/src/registrar/management/commands/create_federal_portfolio.py
+++ b/src/registrar/management/commands/create_federal_portfolio.py
@@ -13,16 +13,28 @@ logger = logging.getLogger(__name__)
 class Command(BaseCommand):
     help = "Creates a federal portfolio given a FederalAgency name"
 
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.created_portfolios = []
+        self.skipped_portfolios = []
+        self.failed_portfolios = []
+
     def add_arguments(self, parser):
         """Add three arguments:
         1. agency_name => the value of FederalAgency.agency
         2. --parse_requests => if true, adds the given portfolio to each related DomainRequest
         3. --parse_domains => if true, adds the given portfolio to each related DomainInformation
         """
-        parser.add_argument(
-            "agency_name",
+        group = parser.add_mutually_exclusive_group(required=True)
+        group.add_argument(
+            "--agency_name",
             help="The name of the FederalAgency to add",
         )
+        group.add_argument(
+            "--branch",
+            choices=["executive", "legislative", "judicial"],
+            help="The federal branch to process. Creates a portfolio for each FederalAgency in this branch.",
+        )
         parser.add_argument(
             "--parse_requests",
             action=argparse.BooleanOptionalAction,
@@ -39,7 +51,9 @@ class Command(BaseCommand):
             help="Adds portfolio to both requests and domains",
         )
 
-    def handle(self, agency_name, **options):
+    def handle(self, **options):
+        agency_name = options.get("agency_name")
+        branch = options.get("branch")
         parse_requests = options.get("parse_requests")
         parse_domains = options.get("parse_domains")
         both = options.get("both")
@@ -51,25 +65,40 @@ class Command(BaseCommand):
             if parse_requests or parse_domains:
                 raise CommandError("You cannot pass --parse_requests or --parse_domains when passing --both.")
 
-        federal_agency = FederalAgency.objects.filter(agency__iexact=agency_name).first()
-        if not federal_agency:
-            raise ValueError(
-                f"Cannot find the federal agency '{agency_name}' in our database. "
-                "The value you enter for `agency_name` must be "
-                "prepopulated in the FederalAgency table before proceeding."
-            )
+        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:
+            if agency_name:
+                raise ValueError(
+                    f"Cannot find the federal agency '{agency_name}' in our database. "
+                    "The value you enter for `agency_name` must be "
+                    "prepopulated in the FederalAgency table before proceeding."
+                )
+            else:
+                raise ValueError(f"Cannot find '{branch}' federal agencies in our database.")
 
-        portfolio = self.create_or_modify_portfolio(federal_agency)
-        self.create_suborganizations(portfolio, federal_agency)
+        for federal_agency in agencies:
+            message = f"Processing federal agency '{federal_agency.agency}'..."
+            TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message)
+            try:
+                portfolio, created = self.create_portfolio(federal_agency)
+                if created:
+                    self.create_suborganizations(portfolio, federal_agency)
+                    if parse_domains or both:
+                        self.handle_portfolio_domains(portfolio, federal_agency)
 
-        if parse_requests or both:
-            self.handle_portfolio_requests(portfolio, federal_agency)
+                if parse_requests or both:
+                    self.handle_portfolio_requests(portfolio, federal_agency)
 
-        if parse_domains or both:
-            self.handle_portfolio_domains(portfolio, federal_agency)
+            except Exception as exec:
+                self.failed_portfolios.append(federal_agency)
+                logger.error(exec)
+        TerminalHelper.log_script_run_summary(
+            self.created_portfolios, self.failed_portfolios, self.skipped_portfolios, debug=False,
+            skipped_header="----- SOME PORTFOLIOS WERE SKIPPED -----"
+        )
 
-    def create_or_modify_portfolio(self, federal_agency):
-        """Creates or modifies a portfolio record based on a federal agency."""
+    def create_portfolio(self, federal_agency):
         portfolio_args = {
             "federal_agency": federal_agency,
             "organization_name": federal_agency.agency,
@@ -84,8 +113,8 @@ class Command(BaseCommand):
         portfolio, created = Portfolio.objects.get_or_create(
             organization_name=portfolio_args.get("organization_name"), defaults=portfolio_args
         )
-
         if created:
+            self.created_portfolios.append(portfolio)
             message = f"Created portfolio '{portfolio}'"
             TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message)
 
@@ -99,36 +128,12 @@ class Command(BaseCommand):
                 )
                 TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message)
         else:
-            proceed = TerminalHelper.prompt_for_execution(
-                system_exit_on_terminate=False,
-                prompt_message=f"""
-                The given portfolio '{federal_agency.agency}' already exists in our DB.
-                If you cancel, the rest of the script will still execute but this record will not update.
-                """,
-                prompt_title="Do you wish to modify this record?",
+            self.skipped_portfolios.append(portfolio)
+            message = (
+                f"The given portfolio '{portfolio}' already exists in our DB. Skipping create."
             )
-            if proceed:
-
-                # Don't override the creator and notes fields
-                if portfolio.creator:
-                    portfolio_args.pop("creator")
-
-                if portfolio.notes:
-                    portfolio_args.pop("notes")
-
-                # Update everything else
-                for key, value in portfolio_args.items():
-                    setattr(portfolio, key, value)
-
-                portfolio.save()
-                message = f"Modified portfolio '{portfolio}'"
-                TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message)
-
-                if portfolio_args.get("senior_official"):
-                    message = f"Added/modified senior official '{portfolio_args['senior_official']}'"
-                    TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message)
-
-        return portfolio
+            TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message)
+        return portfolio, created
 
     def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency):
         """Create Suborganizations tied to the given portfolio based on DomainInformation objects"""
@@ -149,7 +154,8 @@ class Command(BaseCommand):
         # Check if we need to update any existing suborgs first. This step is optional.
         existing_suborgs = Suborganization.objects.filter(name__in=org_names)
         if existing_suborgs.exists():
-            self._update_existing_suborganizations(portfolio, existing_suborgs)
+            message = f"Some suborganizations already exist for portfolio '{portfolio}'. Skipping create."
+            TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message)
 
         # Create new suborgs, as long as they don't exist in the db already
         new_suborgs = []
@@ -175,29 +181,6 @@ class Command(BaseCommand):
         else:
             TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, "No suborganizations added")
 
-    def _update_existing_suborganizations(self, portfolio, orgs_to_update):
-        """
-        Update existing suborganizations with new portfolio.
-        Prompts for user confirmation before proceeding.
-        """
-        proceed = TerminalHelper.prompt_for_execution(
-            system_exit_on_terminate=False,
-            prompt_message=f"""Some suborganizations already exist in our DB.
-            If you cancel, the rest of the script will still execute but these records will not update.
-
-            ==Proposed Changes==
-            The following suborgs will be updated: {[org.name for org in orgs_to_update]}
-            """,
-            prompt_title="Do you wish to modify existing suborganizations?",
-        )
-        if proceed:
-            for org in orgs_to_update:
-                org.portfolio = portfolio
-
-            Suborganization.objects.bulk_update(orgs_to_update, ["portfolio"])
-            message = f"Updated {len(orgs_to_update)} suborganizations."
-            TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message)
-
     def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency):
         """
         Associate portfolio with domain requests for a federal agency.
@@ -208,11 +191,11 @@ class Command(BaseCommand):
             DomainRequest.DomainRequestStatus.INELIGIBLE,
             DomainRequest.DomainRequestStatus.REJECTED,
         ]
-        domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency).exclude(status__in=invalid_states)
+        domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency, portfolio__isnull=True).exclude(status__in=invalid_states)
         if not domain_requests.exists():
             message = f"""
-            Portfolios 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.
+            Portfolio '{portfolio}' not added to domain requests: no valid records found.
+            This means that a filter on DomainInformation for the federal_agency '{federal_agency}' and portfolio__isnull=True returned no results.
             Excluded statuses: STARTED, INELIGIBLE, REJECTED.
             """
             TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message)
@@ -234,11 +217,11 @@ class Command(BaseCommand):
         Associate portfolio with domains for a federal agency.
         Updates all relevant domain information records.
         """
-        domain_infos = DomainInformation.objects.filter(federal_agency=federal_agency)
+        domain_infos = DomainInformation.objects.filter(federal_agency=federal_agency, portfolio__isnull=True)
         if not domain_infos.exists():
             message = f"""
-            Portfolios not added to domains: no valid records found.
-            This means that a filter on DomainInformation for the federal_agency '{federal_agency}' returned no results.
+            Portfolio '{portfolio}' not added to domains: no valid records found.
+            The filter on DomainInformation for the federal_agency '{federal_agency}' and portfolio__isnull=True returned no results.
             """
             TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message)
             return None
diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py
index fa7cde683..22b5a7cc3 100644
--- a/src/registrar/management/commands/utility/terminal_helper.py
+++ b/src/registrar/management/commands/utility/terminal_helper.py
@@ -192,7 +192,7 @@ class PopulateScriptTemplate(ABC):
 class TerminalHelper:
     @staticmethod
     def log_script_run_summary(
-        to_update, failed_to_update, skipped, debug: bool, log_header=None, display_as_str=False
+        to_update, failed_to_update, skipped, debug: bool, log_header=None, skipped_header=None, display_as_str=False
     ):
         """Prints success, failed, and skipped counts, as well as
         all affected objects."""
@@ -203,6 +203,9 @@ class TerminalHelper:
         if log_header is None:
             log_header = "============= FINISHED ==============="
 
+        if skipped_header is None:
+            skipped_header = "----- SOME DATA WAS INVALID (NEEDS MANUAL PATCHING) -----"
+
         # Prepare debug messages
         if debug:
             updated_display = [str(u) for u in to_update] if display_as_str else to_update
@@ -236,7 +239,7 @@ class TerminalHelper:
                 f"""{TerminalColors.YELLOW}
                 {log_header}
                 Updated {update_success_count} entries
-                ----- SOME DATA WAS INVALID (NEEDS MANUAL PATCHING) -----
+                {skipped_header}
                 Skipped updating {update_skipped_count} entries
                 {TerminalColors.ENDC}
                 """

From 7c1e232d6b4fd6a39313eda1136a123851af52db Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Tue, 26 Nov 2024 10:14:44 -0700
Subject: [PATCH 2/9] cleanup

---
 .../commands/create_federal_portfolio.py      | 85 +++++++++++--------
 .../commands/utility/terminal_helper.py       | 23 ++++-
 2 files changed, 69 insertions(+), 39 deletions(-)

diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py
index e5f4be61c..4c0856fb4 100644
--- a/src/registrar/management/commands/create_federal_portfolio.py
+++ b/src/registrar/management/commands/create_federal_portfolio.py
@@ -15,9 +15,9 @@ class Command(BaseCommand):
 
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        self.created_portfolios = []
-        self.skipped_portfolios = []
-        self.failed_portfolios = []
+        self.updated_portfolios = set()
+        self.skipped_portfolios = set()
+        self.failed_portfolios = set()
 
     def add_arguments(self, parser):
         """Add three arguments:
@@ -91,49 +91,61 @@ class Command(BaseCommand):
                     self.handle_portfolio_requests(portfolio, federal_agency)
 
             except Exception as exec:
-                self.failed_portfolios.append(federal_agency)
+                self.failed_portfolios.add(federal_agency)
                 logger.error(exec)
+                message = f"Failed to create portfolio '{portfolio}'"
+                TerminalHelper.colorful_logger(logger.info, TerminalColors.FAIL, message)
+
         TerminalHelper.log_script_run_summary(
-            self.created_portfolios, self.failed_portfolios, self.skipped_portfolios, debug=False,
-            skipped_header="----- SOME PORTFOLIOS WERE SKIPPED -----"
+            self.updated_portfolios,
+            self.failed_portfolios,
+            self.skipped_portfolios,
+            debug=False,
+            skipped_header="----- SOME PORTFOLIOS WERE SKIPPED -----",
+            display_as_str=True,
         )
 
     def create_portfolio(self, federal_agency):
-        portfolio_args = {
-            "federal_agency": federal_agency,
-            "organization_name": federal_agency.agency,
-            "organization_type": DomainRequest.OrganizationChoices.FEDERAL,
-            "creator": User.get_default_user(),
-            "notes": "Auto-generated record",
-        }
+        # Get the org name / senior official
+        org_name = federal_agency.agency
+        so = federal_agency.so_federal_agency.first() if federal_agency.so_federal_agency.exists() else None
 
-        if federal_agency.so_federal_agency.exists():
-            portfolio_args["senior_official"] = federal_agency.so_federal_agency.first()
+        # First just try to get an existing portfolio
+        portfolio = Portfolio.objects.filter(organization_name=org_name).first()
+        if portfolio:
+            self.skipped_portfolios.add(portfolio)
+            TerminalHelper.colorful_logger(
+                logger.info,
+                TerminalColors.YELLOW,
+                f"Portfolio with organization name '{org_name}' already exists. Skipping create.",
+            )
+            return portfolio, False
 
-        portfolio, created = Portfolio.objects.get_or_create(
-            organization_name=portfolio_args.get("organization_name"), defaults=portfolio_args
+        # Create new portfolio if it doesn't exist
+        portfolio = Portfolio.objects.create(
+            organization_name=org_name,
+            federal_agency=federal_agency,
+            organization_type=DomainRequest.OrganizationChoices.FEDERAL,
+            creator=User.get_default_user(),
+            notes="Auto-generated record",
+            senior_official=so,
         )
-        if created:
-            self.created_portfolios.append(portfolio)
-            message = f"Created portfolio '{portfolio}'"
-            TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message)
 
-            if portfolio_args.get("senior_official"):
-                message = f"Added senior official '{portfolio_args['senior_official']}'"
-                TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message)
-            else:
-                message = (
-                    f"No senior official added to portfolio '{portfolio}'. "
-                    "None was returned for the reverse relation `FederalAgency.so_federal_agency.first()`"
-                )
-                TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message)
+        self.updated_portfolios.add(portfolio)
+        TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, f"Created portfolio '{portfolio}'")
+
+        # Log if the senior official was added or not.
+        if portfolio.senior_official:
+            message = f"Added senior official '{portfolio.senior_official}'"
+            TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message)
         else:
-            self.skipped_portfolios.append(portfolio)
             message = (
-                f"The given portfolio '{portfolio}' already exists in our DB. Skipping create."
+                f"No senior official added to portfolio '{org_name}'. "
+                "None was returned for the reverse relation `FederalAgency.so_federal_agency.first()`"
             )
             TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message)
-        return portfolio, created
+
+        return portfolio, True
 
     def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency):
         """Create Suborganizations tied to the given portfolio based on DomainInformation objects"""
@@ -155,7 +167,7 @@ class Command(BaseCommand):
         existing_suborgs = Suborganization.objects.filter(name__in=org_names)
         if existing_suborgs.exists():
             message = f"Some suborganizations already exist for portfolio '{portfolio}'. Skipping create."
-            TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message)
+            TerminalHelper.colorful_logger(logger.warning, TerminalColors.OKBLUE, message)
 
         # Create new suborgs, as long as they don't exist in the db already
         new_suborgs = []
@@ -191,7 +203,9 @@ class Command(BaseCommand):
             DomainRequest.DomainRequestStatus.INELIGIBLE,
             DomainRequest.DomainRequestStatus.REJECTED,
         ]
-        domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency, portfolio__isnull=True).exclude(status__in=invalid_states)
+        domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency, portfolio__isnull=True).exclude(
+            status__in=invalid_states
+        )
         if not domain_requests.exists():
             message = f"""
             Portfolio '{portfolio}' not added to domain requests: no valid records found.
@@ -207,6 +221,7 @@ class Command(BaseCommand):
             domain_request.portfolio = portfolio
             if domain_request.organization_name in suborgs:
                 domain_request.sub_organization = suborgs.get(domain_request.organization_name)
+            self.updated_portfolios.add(portfolio)
 
         DomainRequest.objects.bulk_update(domain_requests, ["portfolio", "sub_organization"])
         message = f"Added portfolio '{portfolio}' to {len(domain_requests)} domain requests."
diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py
index 22b5a7cc3..0352fd3bc 100644
--- a/src/registrar/management/commands/utility/terminal_helper.py
+++ b/src/registrar/management/commands/utility/terminal_helper.py
@@ -206,8 +206,18 @@ class TerminalHelper:
         if skipped_header is None:
             skipped_header = "----- SOME DATA WAS INVALID (NEEDS MANUAL PATCHING) -----"
 
+        # Give the user the option to see failed / skipped records if any exist.
+        debug_anyway = False
+        if not debug and update_failed_count > 0 or update_skipped_count > 0:
+            debug_anyway = TerminalHelper.prompt_for_execution(
+                system_exit_on_terminate=False,
+                prompt_message=f"You will see {update_failed_count} failed and {update_skipped_count} skipped records.",
+                verify_message="** Some records were skipped, or some failed to update. **",
+                prompt_title="Do you wish to see the full list of failed, skipped and updated records?",
+            )
+
         # Prepare debug messages
-        if debug:
+        if debug or debug_anyway:
             updated_display = [str(u) for u in to_update] if display_as_str else to_update
             skipped_display = [str(s) for s in skipped] if display_as_str else skipped
             failed_display = [str(f) for f in failed_to_update] if display_as_str else failed_to_update
@@ -220,7 +230,7 @@ class TerminalHelper:
             # Print out a list of everything that was changed, if we have any changes to log.
             # Otherwise, don't print anything.
             TerminalHelper.print_conditional(
-                debug,
+                True,
                 f"{debug_messages.get('success') if update_success_count > 0 else ''}"
                 f"{debug_messages.get('skipped') if update_skipped_count > 0 else ''}"
                 f"{debug_messages.get('failed') if update_failed_count > 0 else ''}",
@@ -371,7 +381,9 @@ class TerminalHelper:
                     logger.info(print_statement)
 
     @staticmethod
-    def prompt_for_execution(system_exit_on_terminate: bool, prompt_message: str, prompt_title: str) -> bool:
+    def prompt_for_execution(
+        system_exit_on_terminate: bool, prompt_message: str, prompt_title: str, verify_message=None
+    ) -> bool:
         """Create to reduce code complexity.
         Prompts the user to inspect the given string
         and asks if they wish to proceed.
@@ -383,6 +395,9 @@ class TerminalHelper:
         if system_exit_on_terminate:
             action_description_for_selecting_no = "exit"
 
+        if verify_message is None:
+            verify_message = "*** IMPORTANT:  VERIFY THE FOLLOWING LOOKS CORRECT ***"
+
         # Allow the user to inspect the command string
         # and ask if they wish to proceed
         proceed_execution = TerminalHelper.query_yes_no_exit(
@@ -390,7 +405,7 @@ class TerminalHelper:
             =====================================================
             {prompt_title}
             =====================================================
-            *** IMPORTANT:  VERIFY THE FOLLOWING LOOKS CORRECT ***
+            {verify_message}
 
             {prompt_message}
             {TerminalColors.FAIL}

From 7f8c0b9196f2e12b0cea34c9230247f94950e604 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Tue, 26 Nov 2024 14:46:22 -0700
Subject: [PATCH 3/9] fix some tests

---
 src/registrar/tests/test_management_scripts.py | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py
index 9fcd261f7..faa2313de 100644
--- a/src/registrar/tests/test_management_scripts.py
+++ b/src/registrar/tests/test_management_scripts.py
@@ -1456,18 +1456,18 @@ class TestCreateFederalPortfolio(TestCase):
         User.objects.all().delete()
 
     @less_console_noise_decorator
-    def run_create_federal_portfolio(self, agency_name, parse_requests=False, parse_domains=False):
+    def run_create_federal_portfolio(self, **kwargs):
         with patch(
             "registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit",
             return_value=True,
         ):
             call_command(
-                "create_federal_portfolio", agency_name, parse_requests=parse_requests, parse_domains=parse_domains
+                "create_federal_portfolio", **kwargs
             )
 
     def test_create_or_modify_portfolio(self):
         """Test portfolio creation and modification with suborg and senior official."""
-        self.run_create_federal_portfolio("Test Federal Agency", parse_requests=True)
+        self.run_create_federal_portfolio(agency_name="Test Federal Agency", parse_requests=True)
 
         portfolio = Portfolio.objects.get(federal_agency=self.federal_agency)
         self.assertEqual(portfolio.organization_name, self.federal_agency.agency)
@@ -1485,7 +1485,7 @@ class TestCreateFederalPortfolio(TestCase):
 
     def test_handle_portfolio_requests(self):
         """Verify portfolio association with domain requests."""
-        self.run_create_federal_portfolio("Test Federal Agency", parse_requests=True)
+        self.run_create_federal_portfolio(agency_name="Test Federal Agency", parse_requests=True)
 
         self.domain_request.refresh_from_db()
         self.assertIsNotNone(self.domain_request.portfolio)
@@ -1494,7 +1494,7 @@ class TestCreateFederalPortfolio(TestCase):
 
     def test_handle_portfolio_domains(self):
         """Check portfolio association with domain information."""
-        self.run_create_federal_portfolio("Test Federal Agency", parse_domains=True)
+        self.run_create_federal_portfolio(agency_name="Test Federal Agency", parse_domains=True)
 
         self.domain_info.refresh_from_db()
         self.assertIsNotNone(self.domain_info.portfolio)
@@ -1503,7 +1503,7 @@ class TestCreateFederalPortfolio(TestCase):
 
     def test_handle_parse_both(self):
         """Ensure correct parsing of both requests and domains."""
-        self.run_create_federal_portfolio("Test Federal Agency", parse_requests=True, parse_domains=True)
+        self.run_create_federal_portfolio(agency_name="Test Federal Agency", parse_requests=True, parse_domains=True)
 
         self.domain_request.refresh_from_db()
         self.domain_info.refresh_from_db()
@@ -1516,7 +1516,7 @@ class TestCreateFederalPortfolio(TestCase):
         with self.assertRaisesRegex(
             CommandError, "You must specify at least one of --parse_requests or --parse_domains."
         ):
-            self.run_create_federal_portfolio("Test Federal Agency")
+            self.run_create_federal_portfolio(agency_name="Test Federal Agency")
 
     def test_command_error_agency_not_found(self):
         """Check error handling for non-existent agency."""
@@ -1525,7 +1525,7 @@ class TestCreateFederalPortfolio(TestCase):
             "The value you enter for `agency_name` must be prepopulated in the FederalAgency table before proceeding."
         )
         with self.assertRaisesRegex(ValueError, expected_message):
-            self.run_create_federal_portfolio("Non-existent Agency", parse_requests=True)
+            self.run_create_federal_portfolio(agency_name="Non-existent Agency", parse_requests=True)
 
     def test_update_existing_portfolio(self):
         """Test updating an existing portfolio."""
@@ -1538,7 +1538,7 @@ class TestCreateFederalPortfolio(TestCase):
             notes="Old notes",
         )
 
-        self.run_create_federal_portfolio("Test Federal Agency", parse_requests=True)
+        self.run_create_federal_portfolio(agency_name="Test Federal Agency", parse_requests=True)
 
         existing_portfolio.refresh_from_db()
         self.assertEqual(existing_portfolio.organization_name, self.federal_agency.agency)

From aaa16842df9bab5c10dff3f6985511d8f61a29cf Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 27 Nov 2024 09:44:52 -0700
Subject: [PATCH 4/9] Doc update

---
 docs/operations/data_migration.md             | 22 ++++++++++++-------
 .../commands/create_federal_portfolio.py      | 11 +++++++---
 2 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md
index a234d882b..0863aa0b7 100644
--- a/docs/operations/data_migration.md
+++ b/docs/operations/data_migration.md
@@ -893,22 +893,28 @@ Example: `cf ssh getgov-za`
 [Follow these steps](#use-scp-to-transfer-data-to-sandboxes) to upload the federal_cio csv to a sandbox of your choice.
 
 #### Step 5: Running the script
-```./manage.py create_federal_portfolio "{federal_agency_name}" --both```
-
+To create a specific portfolio: 
+```./manage.py create_federal_portfolio --agency_name "{federal_agency_name}" --both```
 Example (only requests): `./manage.py create_federal_portfolio "AMTRAK" --parse_requests`
 
+To create a portfolios for all federal agencies in a branch: 
+```./manage.py create_federal_portfolio --branch "{executive|legislative|judicial}" --both```
+Example (only requests): `./manage.py create_federal_portfolio --branch "executive" --parse_requests`
+
 ### Running locally
 
 #### Step 1: Running the script
-```docker-compose exec app ./manage.py create_federal_portfolio "{federal_agency_name}" --both```
+```docker-compose exec app ./manage.py create_federal_portfolio --agency_name "{federal_agency_name}" --both```
 
 ##### Parameters
 |   | Parameter                  | Description                                                                                |
 |:-:|:-------------------------- |:-------------------------------------------------------------------------------------------|
-| 1 | **federal_agency_name**    | Name of the FederalAgency record surrounded by quotes. For instance,"AMTRAK".              |
-| 2 | **both**                   | If True, runs parse_requests and parse_domains.                                            |
-| 3 | **parse_requests**         | If True, then the created portfolio is added to all related DomainRequests.                |
-| 4 | **parse_domains**          | If True, then the created portfolio is added to all related Domains.                       |
+| 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.                       |
 
-Note: Regarding 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,
+- 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,
 you must specify at least one to run this script.
diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py
index 4c0856fb4..48202a93b 100644
--- a/src/registrar/management/commands/create_federal_portfolio.py
+++ b/src/registrar/management/commands/create_federal_portfolio.py
@@ -14,6 +14,7 @@ class Command(BaseCommand):
     help = "Creates a federal portfolio given a FederalAgency name"
 
     def __init__(self, *args, **kwargs):
+        """Defines fields to track what portfolios were updated, skipped, or just outright failed."""
         super().__init__(*args, **kwargs)
         self.updated_portfolios = set()
         self.skipped_portfolios = set()
@@ -209,8 +210,11 @@ class Command(BaseCommand):
         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}' and portfolio__isnull=True returned no results.
+            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(
+                status__in=invalid_states
+            )
             """
             TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message)
             return None
@@ -236,7 +240,8 @@ class Command(BaseCommand):
         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}' and portfolio__isnull=True returned no results.
+            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)
             """
             TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message)
             return None
@@ -249,5 +254,5 @@ class Command(BaseCommand):
                 domain_info.sub_organization = suborgs.get(domain_info.organization_name)
 
         DomainInformation.objects.bulk_update(domain_infos, ["portfolio", "sub_organization"])
-        message = f"Added portfolio '{portfolio}' to {len(domain_infos)} domains"
+        message = f"Added portfolio '{portfolio}' to {len(domain_infos)} domains."
         TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message)

From 4beec038c7a39d4b9f3f7bb8b76883dcbe86f4dc Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 27 Nov 2024 13:25:15 -0700
Subject: [PATCH 5/9] Unit tests

---
 .../tests/test_management_scripts.py          | 189 +++++++++++++++++-
 1 file changed, 179 insertions(+), 10 deletions(-)

diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py
index faa2313de..47cf61d6a 100644
--- a/src/registrar/tests/test_management_scripts.py
+++ b/src/registrar/tests/test_management_scripts.py
@@ -1421,10 +1421,27 @@ class TestCreateFederalPortfolio(TestCase):
     def setUp(self):
         self.mock_client = MockSESClient()
         self.user = User.objects.create(username="testuser")
+
+        # 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")
+
+        # And create some with federal_type ones with creative names
+        self.executive_agency_1 = FederalAgency.objects.create(agency="Executive Agency 1", federal_type=BranchChoices.EXECUTIVE)
+        self.executive_agency_2 = FederalAgency.objects.create(agency="Executive Agency 2", federal_type=BranchChoices.EXECUTIVE)
+        self.executive_agency_3 = FederalAgency.objects.create(agency="Executive Agency 3", federal_type=BranchChoices.EXECUTIVE)
+        self.legislative_agency_1 = FederalAgency.objects.create(agency="Legislative Agency 1", federal_type=BranchChoices.LEGISLATIVE)
+        self.legislative_agency_2 = FederalAgency.objects.create(agency="Legislative Agency 2", federal_type=BranchChoices.LEGISLATIVE)
+        self.judicial_agency_1 = FederalAgency.objects.create(agency="Judicial Agency 1", federal_type=BranchChoices.JUDICIAL)
+        self.judicial_agency_2 = FederalAgency.objects.create(agency="Judicial Agency 2", federal_type=BranchChoices.JUDICIAL)
         self.senior_official = SeniorOfficial.objects.create(
             first_name="first", last_name="last", email="testuser@igorville.gov", federal_agency=self.federal_agency
         )
+        self.executive_so_1 = SeniorOfficial.objects.create(
+            first_name="first", last_name="last", email="apple@igorville.gov", federal_agency=self.executive_agency_1
+        )
+        self.executive_so_2 = SeniorOfficial.objects.create(
+            first_name="first", last_name="last", email="mango@igorville.gov", federal_agency=self.executive_agency_2
+        )
         with boto3_mocking.clients.handler_for("sesv2", self.mock_client):
             self.domain_request = completed_domain_request(
                 status=DomainRequest.DomainRequestStatus.IN_REVIEW,
@@ -1436,7 +1453,7 @@ class TestCreateFederalPortfolio(TestCase):
             self.domain_info = DomainInformation.objects.filter(domain_request=self.domain_request).get()
 
             self.domain_request_2 = completed_domain_request(
-                name="sock@igorville.org",
+                name="icecreamforigorville.gov",
                 status=DomainRequest.DomainRequestStatus.IN_REVIEW,
                 generic_org_type=DomainRequest.OrganizationChoices.CITY,
                 federal_agency=self.federal_agency,
@@ -1446,6 +1463,28 @@ class TestCreateFederalPortfolio(TestCase):
             self.domain_request_2.approve()
             self.domain_info_2 = DomainInformation.objects.filter(domain_request=self.domain_request_2).get()
 
+            self.domain_request_3 = completed_domain_request(
+                name="exec_1.gov",
+                status=DomainRequest.DomainRequestStatus.IN_REVIEW,
+                generic_org_type=DomainRequest.OrganizationChoices.FEDERAL,
+                federal_agency=self.executive_agency_1,
+                user=self.user,
+                organization_name="Executive Agency 1",
+            )
+            self.domain_request_3.approve()
+            self.domain_info_3 = self.domain_request_3.DomainRequest_info
+
+            self.domain_request_4 = completed_domain_request(
+                name="exec_2.gov",
+                status=DomainRequest.DomainRequestStatus.IN_REVIEW,
+                generic_org_type=DomainRequest.OrganizationChoices.FEDERAL,
+                federal_agency=self.executive_agency_2,
+                user=self.user,
+                organization_name="Executive Agency 2",
+            )
+            self.domain_request_4.approve()
+            self.domain_info_4 = self.domain_request_4.DomainRequest_info
+
     def tearDown(self):
         DomainInformation.objects.all().delete()
         DomainRequest.objects.all().delete()
@@ -1465,8 +1504,8 @@ class TestCreateFederalPortfolio(TestCase):
                 "create_federal_portfolio", **kwargs
             )
 
-    def test_create_or_modify_portfolio(self):
-        """Test portfolio creation and modification with suborg and senior official."""
+    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)
 
         portfolio = Portfolio.objects.get(federal_agency=self.federal_agency)
@@ -1483,6 +1522,115 @@ class TestCreateFederalPortfolio(TestCase):
         # Test the senior official
         self.assertEqual(portfolio.senior_official, self.senior_official)
 
+    def test_create_multiple_portfolios_for_branch(self):
+        """Tests creating all portfolios under a given branch"""
+        federal_choice = DomainRequest.OrganizationChoices.FEDERAL
+
+        # == Test creating executive portfolios == #
+        expected_portfolio_names = {
+            self.executive_agency_1.agency,
+            self.executive_agency_2.agency,
+            self.executive_agency_3.agency
+        }
+        self.run_create_federal_portfolio(branch="executive", parse_requests=True, parse_domains=True)
+
+        # Ensure that all the portfolios we expect to get created were created
+        portfolios = Portfolio.objects.all()
+        self.assertEqual(portfolios.count(), 3)
+
+        # Test that all created portfolios have the correct values
+        org_names, org_types, creators, notes, senior_officials = [], [], [], [], []
+        for portfolio in portfolios:
+            org_names.append(portfolio.organization_name)
+            org_types.append(portfolio.organization_type)
+            creators.append(portfolio.creator)
+            notes.append(portfolio.notes)
+            senior_officials.append(portfolio.senior_official)
+
+        # Test organization_name, organization_type, creator, and notes (in that order)
+        self.assertTrue(all([org_name in expected_portfolio_names for org_name in org_names]))
+        self.assertTrue(all([org_type == federal_choice for org_type in org_types]))
+        self.assertTrue(all([creator == User.get_default_user() for creator in creators]))
+        self.assertTrue(all([note == "Auto-generated record" for note in notes]))
+
+        # Test senior officials were assigned correctly
+        expected_senior_officials = {
+            self.executive_so_1,
+            self.executive_so_2
+        }
+        self.assertTrue(all([senior_official in expected_senior_officials for senior_official in senior_officials]))
+
+        # Test that domain requests / domains were assigned correctly
+        expected_requests = DomainRequest.objects.filter(portfolio__id__in=[
+            self.domain_request_3.id,
+            self.domain_request_4.id
+        ])
+        expected_domain_infos = DomainInformation.objects.filter(
+            portfolio__id__in=[
+                self.domain_info_3.id,
+                self.domain_info_4.id
+            ]
+        )
+        self.assertEqual(expected_requests.count(), 2)
+        self.assertEqual(expected_domain_infos.count(), 2)
+        # == Test creating legislative portfolios == #
+
+        # Clear old portfolios
+        Portfolio.objects.all().delete()
+
+        expected_portfolio_names = {
+            self.legislative_agency_1.agency,
+            self.legislative_agency_2.agency,
+        }
+        self.run_create_federal_portfolio(branch="legislative", parse_requests=True, parse_domains=True)
+
+        # Ensure that all the portfolios we expect to get created were created
+        portfolios = Portfolio.objects.all()
+        self.assertEqual(portfolios.count(), 2)
+
+        # Test that all created portfolios have the correct values
+        org_names, org_types, creators, notes = [], [], [], []
+        for portfolio in portfolios:
+            org_names.append(portfolio.organization_name)
+            org_types.append(portfolio.organization_type)
+            creators.append(portfolio.creator)
+            notes.append(portfolio.notes)
+
+        # Test organization_name, organization_type, creator, and notes (in that order)
+        self.assertTrue(all([org_name in expected_portfolio_names for org_name in org_names]))
+        self.assertTrue(all([org_type == federal_choice for org_type in org_types]))
+        self.assertTrue(all([creator == User.get_default_user() for creator in creators]))
+        self.assertTrue(all([note == "Auto-generated record" for note in notes]))
+
+        # == Test creating judicial portfolios == #
+
+        # Clear old portfolios
+        Portfolio.objects.all().delete()
+
+        expected_portfolio_names = {
+            self.judicial_agency_1.agency,
+            self.judicial_agency_2.agency,
+        }
+        self.run_create_federal_portfolio(branch="judicial", parse_requests=True, parse_domains=True)
+
+        # Ensure that all the portfolios we expect to get created were created
+        portfolios = Portfolio.objects.all()
+        self.assertEqual(portfolios.count(), 2)
+
+        # Test that all created portfolios have the correct values
+        org_names, org_types, creators, notes = [], [], [], []
+        for portfolio in portfolios:
+            org_names.append(portfolio.organization_name)
+            org_types.append(portfolio.organization_type)
+            creators.append(portfolio.creator)
+            notes.append(portfolio.notes)
+
+        # Test organization_name, organization_type, creator, and notes (in that order)
+        self.assertTrue(all([org_name in expected_portfolio_names for org_name in org_names]))
+        self.assertTrue(all([org_type == federal_choice for org_type in org_types]))
+        self.assertTrue(all([creator == User.get_default_user() for creator in creators]))
+        self.assertTrue(all([note == "Auto-generated record" for note in notes]))
+
     def test_handle_portfolio_requests(self):
         """Verify portfolio association with domain requests."""
         self.run_create_federal_portfolio(agency_name="Test Federal Agency", parse_requests=True)
@@ -1511,12 +1659,30 @@ class TestCreateFederalPortfolio(TestCase):
         self.assertIsNotNone(self.domain_info.portfolio)
         self.assertEqual(self.domain_request.portfolio, self.domain_info.portfolio)
 
-    def test_command_error_no_parse_options(self):
-        """Verify error when no parse options are provided."""
+    def test_command_error_parse_options(self):
+        """Verify error when bad parse options are provided."""
+        # The command should enforce either --branch or --agency_name
+        with self.assertRaisesRegex(
+            CommandError, "Error: one of the arguments --agency_name --branch is required"
+        ):
+            self.run_create_federal_portfolio()
+        
+        # We should forbid both at the same time
+        with self.assertRaisesRegex(
+            CommandError, "Error: argument --branch: not allowed with argument --agency_name"
+        ):
+            self.run_create_federal_portfolio(agency_name="test", branch="executive")
+        
+        # We expect a error to be thrown when we dont pass parse requests or domains
         with self.assertRaisesRegex(
             CommandError, "You must specify at least one of --parse_requests or --parse_domains."
         ):
-            self.run_create_federal_portfolio(agency_name="Test Federal Agency")
+            self.run_create_federal_portfolio(branch="executive")
+        
+        with self.assertRaisesRegex(
+            CommandError, "You must specify at least one of --parse_requests or --parse_domains."
+        ):
+            self.run_create_federal_portfolio(agency_name="test")
 
     def test_command_error_agency_not_found(self):
         """Check error handling for non-existent agency."""
@@ -1527,8 +1693,8 @@ class TestCreateFederalPortfolio(TestCase):
         with self.assertRaisesRegex(ValueError, expected_message):
             self.run_create_federal_portfolio(agency_name="Non-existent Agency", parse_requests=True)
 
-    def test_update_existing_portfolio(self):
-        """Test updating an existing portfolio."""
+    def test_does_not_update_existing_portfolio(self):
+        """Tests that an existing portfolio is not updated"""
         # Create an existing portfolio
         existing_portfolio = Portfolio.objects.create(
             federal_agency=self.federal_agency,
@@ -1541,9 +1707,12 @@ class TestCreateFederalPortfolio(TestCase):
         self.run_create_federal_portfolio(agency_name="Test Federal Agency", parse_requests=True)
 
         existing_portfolio.refresh_from_db()
-        self.assertEqual(existing_portfolio.organization_name, self.federal_agency.agency)
-        self.assertEqual(existing_portfolio.organization_type, DomainRequest.OrganizationChoices.FEDERAL)
+        # 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)

From 022a63dc30e10366536e24b9d96bc9f515ad6a3d Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 27 Nov 2024 13:48:33 -0700
Subject: [PATCH 6/9] Cleanup

---
 .../commands/create_federal_portfolio.py      |   6 +-
 .../tests/test_management_scripts.py          | 185 ++++++++++--------
 2 files changed, 103 insertions(+), 88 deletions(-)

diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py
index 48202a93b..9e2598595 100644
--- a/src/registrar/management/commands/create_federal_portfolio.py
+++ b/src/registrar/management/commands/create_federal_portfolio.py
@@ -70,13 +70,13 @@ class Command(BaseCommand):
         agencies = FederalAgency.objects.filter(**federal_agency_filter)
         if not agencies or agencies.count() < 1:
             if agency_name:
-                raise ValueError(
+                raise CommandError(
                     f"Cannot find the federal agency '{agency_name}' in our database. "
                     "The value you enter for `agency_name` must be "
                     "prepopulated in the FederalAgency table before proceeding."
                 )
             else:
-                raise ValueError(f"Cannot find '{branch}' federal agencies in our database.")
+                raise CommandError(f"Cannot find '{branch}' federal agencies in our database.")
 
         for federal_agency in agencies:
             message = f"Processing federal agency '{federal_agency.agency}'..."
@@ -167,7 +167,7 @@ class Command(BaseCommand):
         # Check if we need to update any existing suborgs first. This step is optional.
         existing_suborgs = Suborganization.objects.filter(name__in=org_names)
         if existing_suborgs.exists():
-            message = f"Some suborganizations already exist for portfolio '{portfolio}'. Skipping create."
+            message = f"Some suborganizations already exist for portfolio '{portfolio}'."
             TerminalHelper.colorful_logger(logger.warning, TerminalColors.OKBLUE, message)
 
         # Create new suborgs, as long as they don't exist in the db already
diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py
index 47cf61d6a..429aed0d9 100644
--- a/src/registrar/tests/test_management_scripts.py
+++ b/src/registrar/tests/test_management_scripts.py
@@ -1426,13 +1426,27 @@ class TestCreateFederalPortfolio(TestCase):
         self.federal_agency = FederalAgency.objects.create(agency="Test Federal Agency")
 
         # And create some with federal_type ones with creative names
-        self.executive_agency_1 = FederalAgency.objects.create(agency="Executive Agency 1", federal_type=BranchChoices.EXECUTIVE)
-        self.executive_agency_2 = FederalAgency.objects.create(agency="Executive Agency 2", federal_type=BranchChoices.EXECUTIVE)
-        self.executive_agency_3 = FederalAgency.objects.create(agency="Executive Agency 3", federal_type=BranchChoices.EXECUTIVE)
-        self.legislative_agency_1 = FederalAgency.objects.create(agency="Legislative Agency 1", federal_type=BranchChoices.LEGISLATIVE)
-        self.legislative_agency_2 = FederalAgency.objects.create(agency="Legislative Agency 2", federal_type=BranchChoices.LEGISLATIVE)
-        self.judicial_agency_1 = FederalAgency.objects.create(agency="Judicial Agency 1", federal_type=BranchChoices.JUDICIAL)
-        self.judicial_agency_2 = FederalAgency.objects.create(agency="Judicial Agency 2", federal_type=BranchChoices.JUDICIAL)
+        self.executive_agency_1 = FederalAgency.objects.create(
+            agency="Executive Agency 1", federal_type=BranchChoices.EXECUTIVE
+        )
+        self.executive_agency_2 = FederalAgency.objects.create(
+            agency="Executive Agency 2", federal_type=BranchChoices.EXECUTIVE
+        )
+        self.executive_agency_3 = FederalAgency.objects.create(
+            agency="Executive Agency 3", federal_type=BranchChoices.EXECUTIVE
+        )
+        self.legislative_agency_1 = FederalAgency.objects.create(
+            agency="Legislative Agency 1", federal_type=BranchChoices.LEGISLATIVE
+        )
+        self.legislative_agency_2 = FederalAgency.objects.create(
+            agency="Legislative Agency 2", federal_type=BranchChoices.LEGISLATIVE
+        )
+        self.judicial_agency_1 = FederalAgency.objects.create(
+            agency="Judicial Agency 1", federal_type=BranchChoices.JUDICIAL
+        )
+        self.judicial_agency_2 = FederalAgency.objects.create(
+            agency="Judicial Agency 2", federal_type=BranchChoices.JUDICIAL
+        )
         self.senior_official = SeniorOfficial.objects.create(
             first_name="first", last_name="last", email="testuser@igorville.gov", federal_agency=self.federal_agency
         )
@@ -1500,9 +1514,7 @@ class TestCreateFederalPortfolio(TestCase):
             "registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit",
             return_value=True,
         ):
-            call_command(
-                "create_federal_portfolio", **kwargs
-            )
+            call_command("create_federal_portfolio", **kwargs)
 
     def test_create_single_portfolio(self):
         """Test portfolio creation with suborg and senior official."""
@@ -1522,7 +1534,61 @@ class TestCreateFederalPortfolio(TestCase):
         # Test the senior official
         self.assertEqual(portfolio.senior_official, self.senior_official)
 
-    def test_create_multiple_portfolios_for_branch(self):
+    def test_create_multiple_portfolios_for_branch_judicial(self):
+        """Tests creating all portfolios under a given branch"""
+        federal_choice = DomainRequest.OrganizationChoices.FEDERAL
+        expected_portfolio_names = {
+            self.judicial_agency_1.agency,
+            self.judicial_agency_2.agency,
+        }
+        self.run_create_federal_portfolio(branch="judicial", parse_requests=True, parse_domains=True)
+
+        # Ensure that all the portfolios we expect to get created were created
+        portfolios = Portfolio.objects.all()
+        self.assertEqual(portfolios.count(), 2)
+
+        # Test that all created portfolios have the correct values
+        org_names, org_types, creators, notes = [], [], [], []
+        for portfolio in portfolios:
+            org_names.append(portfolio.organization_name)
+            org_types.append(portfolio.organization_type)
+            creators.append(portfolio.creator)
+            notes.append(portfolio.notes)
+
+        # Test organization_name, organization_type, creator, and notes (in that order)
+        self.assertTrue(all([org_name in expected_portfolio_names for org_name in org_names]))
+        self.assertTrue(all([org_type == federal_choice for org_type in org_types]))
+        self.assertTrue(all([creator == User.get_default_user() for creator in creators]))
+        self.assertTrue(all([note == "Auto-generated record" for note in notes]))
+
+    def test_create_multiple_portfolios_for_branch_legislative(self):
+        """Tests creating all portfolios under a given branch"""
+        federal_choice = DomainRequest.OrganizationChoices.FEDERAL
+        expected_portfolio_names = {
+            self.legislative_agency_1.agency,
+            self.legislative_agency_2.agency,
+        }
+        self.run_create_federal_portfolio(branch="legislative", parse_requests=True, parse_domains=True)
+
+        # Ensure that all the portfolios we expect to get created were created
+        portfolios = Portfolio.objects.all()
+        self.assertEqual(portfolios.count(), 2)
+
+        # Test that all created portfolios have the correct values
+        org_names, org_types, creators, notes = [], [], [], []
+        for portfolio in portfolios:
+            org_names.append(portfolio.organization_name)
+            org_types.append(portfolio.organization_type)
+            creators.append(portfolio.creator)
+            notes.append(portfolio.notes)
+
+        # Test organization_name, organization_type, creator, and notes (in that order)
+        self.assertTrue(all([org_name in expected_portfolio_names for org_name in org_names]))
+        self.assertTrue(all([org_type == federal_choice for org_type in org_types]))
+        self.assertTrue(all([creator == User.get_default_user() for creator in creators]))
+        self.assertTrue(all([note == "Auto-generated record" for note in notes]))
+
+    def test_create_multiple_portfolios_for_branch_executive(self):
         """Tests creating all portfolios under a given branch"""
         federal_choice = DomainRequest.OrganizationChoices.FEDERAL
 
@@ -1530,7 +1596,7 @@ class TestCreateFederalPortfolio(TestCase):
         expected_portfolio_names = {
             self.executive_agency_1.agency,
             self.executive_agency_2.agency,
-            self.executive_agency_3.agency
+            self.executive_agency_3.agency,
         }
         self.run_create_federal_portfolio(branch="executive", parse_requests=True, parse_domains=True)
 
@@ -1556,80 +1622,33 @@ class TestCreateFederalPortfolio(TestCase):
         # Test senior officials were assigned correctly
         expected_senior_officials = {
             self.executive_so_1,
-            self.executive_so_2
+            self.executive_so_2,
+            # We expect one record to skip
+            None,
         }
         self.assertTrue(all([senior_official in expected_senior_officials for senior_official in senior_officials]))
 
         # Test that domain requests / domains were assigned correctly
-        expected_requests = DomainRequest.objects.filter(portfolio__id__in=[
-            self.domain_request_3.id,
-            self.domain_request_4.id
-        ])
+        self.domain_request_3.refresh_from_db()
+        self.domain_request_4.refresh_from_db()
+        self.domain_info_3.refresh_from_db()
+        self.domain_info_4.refresh_from_db()
+        expected_requests = DomainRequest.objects.filter(
+            portfolio__id__in=[
+                # Implicity tests for existence
+                self.domain_request_3.portfolio.id,
+                self.domain_request_4.portfolio.id,
+            ]
+        )
         expected_domain_infos = DomainInformation.objects.filter(
             portfolio__id__in=[
-                self.domain_info_3.id,
-                self.domain_info_4.id
+                # Implicity tests for existence
+                self.domain_info_3.portfolio.id,
+                self.domain_info_4.portfolio.id,
             ]
         )
         self.assertEqual(expected_requests.count(), 2)
         self.assertEqual(expected_domain_infos.count(), 2)
-        # == Test creating legislative portfolios == #
-
-        # Clear old portfolios
-        Portfolio.objects.all().delete()
-
-        expected_portfolio_names = {
-            self.legislative_agency_1.agency,
-            self.legislative_agency_2.agency,
-        }
-        self.run_create_federal_portfolio(branch="legislative", parse_requests=True, parse_domains=True)
-
-        # Ensure that all the portfolios we expect to get created were created
-        portfolios = Portfolio.objects.all()
-        self.assertEqual(portfolios.count(), 2)
-
-        # Test that all created portfolios have the correct values
-        org_names, org_types, creators, notes = [], [], [], []
-        for portfolio in portfolios:
-            org_names.append(portfolio.organization_name)
-            org_types.append(portfolio.organization_type)
-            creators.append(portfolio.creator)
-            notes.append(portfolio.notes)
-
-        # Test organization_name, organization_type, creator, and notes (in that order)
-        self.assertTrue(all([org_name in expected_portfolio_names for org_name in org_names]))
-        self.assertTrue(all([org_type == federal_choice for org_type in org_types]))
-        self.assertTrue(all([creator == User.get_default_user() for creator in creators]))
-        self.assertTrue(all([note == "Auto-generated record" for note in notes]))
-
-        # == Test creating judicial portfolios == #
-
-        # Clear old portfolios
-        Portfolio.objects.all().delete()
-
-        expected_portfolio_names = {
-            self.judicial_agency_1.agency,
-            self.judicial_agency_2.agency,
-        }
-        self.run_create_federal_portfolio(branch="judicial", parse_requests=True, parse_domains=True)
-
-        # Ensure that all the portfolios we expect to get created were created
-        portfolios = Portfolio.objects.all()
-        self.assertEqual(portfolios.count(), 2)
-
-        # Test that all created portfolios have the correct values
-        org_names, org_types, creators, notes = [], [], [], []
-        for portfolio in portfolios:
-            org_names.append(portfolio.organization_name)
-            org_types.append(portfolio.organization_type)
-            creators.append(portfolio.creator)
-            notes.append(portfolio.notes)
-
-        # Test organization_name, organization_type, creator, and notes (in that order)
-        self.assertTrue(all([org_name in expected_portfolio_names for org_name in org_names]))
-        self.assertTrue(all([org_type == federal_choice for org_type in org_types]))
-        self.assertTrue(all([creator == User.get_default_user() for creator in creators]))
-        self.assertTrue(all([note == "Auto-generated record" for note in notes]))
 
     def test_handle_portfolio_requests(self):
         """Verify portfolio association with domain requests."""
@@ -1662,23 +1681,19 @@ class TestCreateFederalPortfolio(TestCase):
     def test_command_error_parse_options(self):
         """Verify error when bad parse options are provided."""
         # The command should enforce either --branch or --agency_name
-        with self.assertRaisesRegex(
-            CommandError, "Error: one of the arguments --agency_name --branch is required"
-        ):
+        with self.assertRaisesRegex(CommandError, "Error: one of the arguments --agency_name --branch is required"):
             self.run_create_federal_portfolio()
-        
+
         # We should forbid both at the same time
-        with self.assertRaisesRegex(
-            CommandError, "Error: argument --branch: not allowed with argument --agency_name"
-        ):
+        with self.assertRaisesRegex(CommandError, "Error: argument --branch: not allowed with argument --agency_name"):
             self.run_create_federal_portfolio(agency_name="test", branch="executive")
-        
+
         # We expect a error to be thrown when we dont pass parse requests or domains
         with self.assertRaisesRegex(
             CommandError, "You must specify at least one of --parse_requests or --parse_domains."
         ):
             self.run_create_federal_portfolio(branch="executive")
-        
+
         with self.assertRaisesRegex(
             CommandError, "You must specify at least one of --parse_requests or --parse_domains."
         ):

From 0c3fb834ff38928b41cdb9a77c72534f78286418 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 27 Nov 2024 14:20:48 -0700
Subject: [PATCH 7/9] linting

---
 .../commands/create_federal_portfolio.py      | 29 ++++++++++++-------
 .../tests/test_management_scripts.py          |  2 +-
 2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py
index 9e2598595..9b8541b75 100644
--- a/src/registrar/management/commands/create_federal_portfolio.py
+++ b/src/registrar/management/commands/create_federal_portfolio.py
@@ -82,15 +82,8 @@ class Command(BaseCommand):
             message = f"Processing federal agency '{federal_agency.agency}'..."
             TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message)
             try:
-                portfolio, created = self.create_portfolio(federal_agency)
-                if created:
-                    self.create_suborganizations(portfolio, federal_agency)
-                    if parse_domains or both:
-                        self.handle_portfolio_domains(portfolio, federal_agency)
-
-                if parse_requests or both:
-                    self.handle_portfolio_requests(portfolio, federal_agency)
-
+                # C901 'Command.handle' is too complex (12)
+                self.handle_populate_portfolio(federal_agency, parse_domains, parse_requests, both)
             except Exception as exec:
                 self.failed_portfolios.add(federal_agency)
                 logger.error(exec)
@@ -106,7 +99,21 @@ class Command(BaseCommand):
             display_as_str=True,
         )
 
+    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"""
+        portfolio, created = self.create_portfolio(federal_agency)
+        if created:
+            self.create_suborganizations(portfolio, federal_agency)
+            if parse_domains or both:
+                self.handle_portfolio_domains(portfolio, federal_agency)
+
+        if parse_requests or both:
+            self.handle_portfolio_requests(portfolio, federal_agency)
+
     def create_portfolio(self, federal_agency):
+        """Creates a portfolio if it doesn't presently exist.
+        Returns portfolio, created."""
         # Get the org name / senior official
         org_name = federal_agency.agency
         so = federal_agency.so_federal_agency.first() if federal_agency.so_federal_agency.exists() else None
@@ -164,11 +171,11 @@ class Command(BaseCommand):
             TerminalHelper.colorful_logger(logger.warning, TerminalColors.FAIL, message)
             return
 
-        # Check if we need to update any existing suborgs first. This step is optional.
+        # Check for existing suborgs on the current portfolio
         existing_suborgs = Suborganization.objects.filter(name__in=org_names)
         if existing_suborgs.exists():
             message = f"Some suborganizations already exist for portfolio '{portfolio}'."
-            TerminalHelper.colorful_logger(logger.warning, TerminalColors.OKBLUE, message)
+            TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, message)
 
         # Create new suborgs, as long as they don't exist in the db already
         new_suborgs = []
diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py
index 429aed0d9..7cce0d2b2 100644
--- a/src/registrar/tests/test_management_scripts.py
+++ b/src/registrar/tests/test_management_scripts.py
@@ -1705,7 +1705,7 @@ class TestCreateFederalPortfolio(TestCase):
             "Cannot find the federal agency 'Non-existent Agency' in our database. "
             "The value you enter for `agency_name` must be prepopulated in the FederalAgency table before proceeding."
         )
-        with self.assertRaisesRegex(ValueError, expected_message):
+        with self.assertRaisesRegex(CommandError, expected_message):
             self.run_create_federal_portfolio(agency_name="Non-existent Agency", parse_requests=True)
 
     def test_does_not_update_existing_portfolio(self):

From 8304caf026b1b5a7be75bbd748c0584c41af5c3d Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 27 Nov 2024 14:33:13 -0700
Subject: [PATCH 8/9] Update create_federal_portfolio.py

---
 .../management/commands/create_federal_portfolio.py         | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py
index 9b8541b75..64672366b 100644
--- a/src/registrar/management/commands/create_federal_portfolio.py
+++ b/src/registrar/management/commands/create_federal_portfolio.py
@@ -87,7 +87,7 @@ class Command(BaseCommand):
             except Exception as exec:
                 self.failed_portfolios.add(federal_agency)
                 logger.error(exec)
-                message = f"Failed to create portfolio '{portfolio}'"
+                message = f"Failed to create portfolio '{federal_agency.agency}'"
                 TerminalHelper.colorful_logger(logger.info, TerminalColors.FAIL, message)
 
         TerminalHelper.log_script_run_summary(
@@ -174,6 +174,10 @@ class Command(BaseCommand):
         # Check for existing suborgs on the current portfolio
         existing_suborgs = Suborganization.objects.filter(name__in=org_names)
         if existing_suborgs.exists():
+            # QUESTION FOR REVIEWERS: Should we be doing this too?
+            # existing_suborgs.filter(portfolio__isnull=True).update(
+            #     portfolio=portfolio
+            # )
             message = f"Some suborganizations already exist for portfolio '{portfolio}'."
             TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, message)
 

From 3cfea0619d2b6f7362bfc8274d47b13b9aec5088 Mon Sep 17 00:00:00 2001
From: zandercymatics <141044360+zandercymatics@users.noreply.github.com>
Date: Wed, 4 Dec 2024 08:47:17 -0700
Subject: [PATCH 9/9] PR feedback

---
 .../management/commands/create_federal_portfolio.py         | 4 ----
 .../management/commands/utility/terminal_helper.py          | 6 +++---
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py
index 64672366b..9cf4d36ea 100644
--- a/src/registrar/management/commands/create_federal_portfolio.py
+++ b/src/registrar/management/commands/create_federal_portfolio.py
@@ -174,10 +174,6 @@ class Command(BaseCommand):
         # Check for existing suborgs on the current portfolio
         existing_suborgs = Suborganization.objects.filter(name__in=org_names)
         if existing_suborgs.exists():
-            # QUESTION FOR REVIEWERS: Should we be doing this too?
-            # existing_suborgs.filter(portfolio__isnull=True).update(
-            #     portfolio=portfolio
-            # )
             message = f"Some suborganizations already exist for portfolio '{portfolio}'."
             TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, message)
 
diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py
index 0352fd3bc..eed1027f7 100644
--- a/src/registrar/management/commands/utility/terminal_helper.py
+++ b/src/registrar/management/commands/utility/terminal_helper.py
@@ -207,9 +207,9 @@ class TerminalHelper:
             skipped_header = "----- SOME DATA WAS INVALID (NEEDS MANUAL PATCHING) -----"
 
         # Give the user the option to see failed / skipped records if any exist.
-        debug_anyway = False
+        display_detailed_logs = False
         if not debug and update_failed_count > 0 or update_skipped_count > 0:
-            debug_anyway = TerminalHelper.prompt_for_execution(
+            display_detailed_logs = TerminalHelper.prompt_for_execution(
                 system_exit_on_terminate=False,
                 prompt_message=f"You will see {update_failed_count} failed and {update_skipped_count} skipped records.",
                 verify_message="** Some records were skipped, or some failed to update. **",
@@ -217,7 +217,7 @@ class TerminalHelper:
             )
 
         # Prepare debug messages
-        if debug or debug_anyway:
+        if debug or display_detailed_logs:
             updated_display = [str(u) for u in to_update] if display_as_str else to_update
             skipped_display = [str(s) for s in skipped] if display_as_str else skipped
             failed_display = [str(f) for f in failed_to_update] if display_as_str else failed_to_update