diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index b64b5ea76..cdef3dba7 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -907,13 +907,14 @@ Example (only requests): `./manage.py create_federal_portfolio --branch "executi ```docker-compose exec app ./manage.py create_federal_portfolio --agency_name "{federal_agency_name}" --both``` ##### 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. | +| | 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, diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index c56b4ff6b..4bc8f6715 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/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( - status__in=invalid_states - ) + domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency).exclude(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 diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 965f98f51..fd53c21f8 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/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.