From 3dff2760793fdd161d00107d469b23eb567cb5ff Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 24 Jan 2025 15:15:46 -0700 Subject: [PATCH 1/7] Fix bug --- .../management/commands/create_federal_portfolio.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index c56b4ff6b..170b85ed4 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -178,6 +178,7 @@ class Command(BaseCommand): also create new suborganizations""" portfolio, _ = self.create_portfolio(federal_agency) self.create_suborganizations(portfolio, federal_agency) + if parse_domains or both: self.handle_portfolio_domains(portfolio, federal_agency) @@ -283,7 +284,7 @@ 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 ) if not domain_requests.exists(): @@ -291,7 +292,7 @@ class Command(BaseCommand): 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 +336,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 From 0fc136cf54eafbfb5d319d1c900043a270042f52 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 27 Jan 2025 08:17:54 -0700 Subject: [PATCH 2/7] Cleanup --- docs/operations/data_migration.md | 15 ++++--- .../commands/create_federal_portfolio.py | 43 ++++++++++++++----- 2 files changed, 41 insertions(+), 17 deletions(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index 499c0840f..96ff88666 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 iterate through DomainRequest / DomainInfo records with portfolio__isnull=True | - 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 170b85ed4..5930539fb 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) @@ -173,17 +181,17 @@ 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) self.create_suborganizations(portfolio, federal_agency) if parse_domains or both: - self.handle_portfolio_domains(portfolio, federal_agency) + self.handle_portfolio_domains(portfolio, federal_agency, skip_existing_portfolios) if parse_requests or both: - self.handle_portfolio_requests(portfolio, federal_agency) + self.handle_portfolio_requests(portfolio, federal_agency, skip_existing_portfolios) return portfolio @@ -274,7 +282,9 @@ class Command(BaseCommand): else: TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, "No suborganizations added") - def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency): + def handle_portfolio_requests( + self, portfolio: Portfolio, federal_agency: FederalAgency, skip_existing_portfolios: bool + ): """ Associate portfolio with domain requests for a federal agency. Updates all relevant domain request records. @@ -284,9 +294,16 @@ class Command(BaseCommand): DomainRequest.DomainRequestStatus.INELIGIBLE, DomainRequest.DomainRequestStatus.REJECTED, ] - domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency).exclude( - status__in=invalid_states - ) + + if skip_existing_portfolios: + domain_requests = DomainRequest.objects.filter( + federal_agency=federal_agency, portfolio__isnull=True + ).exclude(status__in=invalid_states) + else: + 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. @@ -329,14 +346,20 @@ class Command(BaseCommand): message = f"Added portfolio '{portfolio}' to {len(domain_requests)} domain requests." TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) - def handle_portfolio_domains(self, portfolio: Portfolio, federal_agency: FederalAgency): + def handle_portfolio_domains( + self, portfolio: Portfolio, federal_agency: FederalAgency, skip_existing_portfolios: bool + ): """ Associate portfolio with domains for a federal agency. Updates all relevant domain information records. Returns a queryset of DomainInformation objects, or None if nothing changed. """ - domain_infos = DomainInformation.objects.filter(federal_agency=federal_agency) + if skip_existing_portfolios: + domain_infos = DomainInformation.objects.filter(federal_agency=federal_agency, portfolio__isnull=True) + else: + 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. From 41cca72bd6e1d5ac5f8b0ef736c47268ea6dcdc4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 27 Jan 2025 10:28:35 -0700 Subject: [PATCH 3/7] Cleanup --- docs/operations/data_migration.md | 2 +- .../commands/create_federal_portfolio.py | 41 ++++++++----------- 2 files changed, 17 insertions(+), 26 deletions(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index 96ff88666..f3aa70239 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -914,7 +914,7 @@ Example (only requests): `./manage.py create_federal_portfolio --branch "executi | 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 iterate through DomainRequest / DomainInfo records with portfolio__isnull=True | +| 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 5930539fb..b5e7ab071 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -184,14 +184,22 @@ class Command(BaseCommand): 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) - self.create_suborganizations(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, skip_existing_portfolios) + self.handle_portfolio_domains(portfolio, federal_agency) if parse_requests or both: - self.handle_portfolio_requests(portfolio, federal_agency, skip_existing_portfolios) + self.handle_portfolio_requests(portfolio, federal_agency) return portfolio @@ -282,9 +290,7 @@ class Command(BaseCommand): else: TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, "No suborganizations added") - def handle_portfolio_requests( - self, portfolio: Portfolio, federal_agency: FederalAgency, skip_existing_portfolios: bool - ): + def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency): """ Associate portfolio with domain requests for a federal agency. Updates all relevant domain request records. @@ -294,16 +300,7 @@ class Command(BaseCommand): DomainRequest.DomainRequestStatus.INELIGIBLE, DomainRequest.DomainRequestStatus.REJECTED, ] - - if skip_existing_portfolios: - domain_requests = DomainRequest.objects.filter( - federal_agency=federal_agency, portfolio__isnull=True - ).exclude(status__in=invalid_states) - else: - domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency).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. @@ -346,20 +343,14 @@ class Command(BaseCommand): message = f"Added portfolio '{portfolio}' to {len(domain_requests)} domain requests." TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) - def handle_portfolio_domains( - self, portfolio: Portfolio, federal_agency: FederalAgency, skip_existing_portfolios: bool - ): + def handle_portfolio_domains(self, portfolio: Portfolio, federal_agency: FederalAgency): """ Associate portfolio with domains for a federal agency. Updates all relevant domain information records. Returns a queryset of DomainInformation objects, or None if nothing changed. """ - if skip_existing_portfolios: - domain_infos = DomainInformation.objects.filter(federal_agency=federal_agency, portfolio__isnull=True) - else: - domain_infos = DomainInformation.objects.filter(federal_agency=federal_agency) - + 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. From e2d0f808b9bcf2d67233509877d514cb66b4db3e Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 27 Jan 2025 13:15:07 -0700 Subject: [PATCH 4/7] Update test_management_scripts.py --- .../tests/test_management_scripts.py | 76 ++++++++++++++++++- 1 file changed, 74 insertions(+), 2 deletions(-) diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 536d1e760..443bafa71 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -32,7 +32,14 @@ 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 ( + MockDb, + MockEppLib, + less_console_noise, + completed_domain_request, + MockSESClient, + MockDbForIndividualTests, +) from api.tests.common import less_console_noise_decorator @@ -1822,7 +1829,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, @@ -1845,6 +1852,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. From ebfaee319f8131309ce0a850a76eb93783d89e43 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 28 Jan 2025 09:53:42 -0700 Subject: [PATCH 5/7] Updated verbiage on script run summary --- .../management/commands/create_federal_portfolio.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index b5e7ab071..aa26e3bcd 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -117,22 +117,29 @@ 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?" ), From 7b2db4b850d1a4ce74968e1889660d2f2ad1464b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 28 Jan 2025 11:43:10 -0700 Subject: [PATCH 6/7] Update create_federal_portfolio.py --- .../management/commands/create_federal_portfolio.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index aa26e3bcd..4bc8f6715 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -143,7 +143,7 @@ class Command(BaseCommand): 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) @@ -166,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 From 6c5da74f6ee08bf8ffa1e10e634556ecb6eaac83 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 28 Jan 2025 14:47:53 -0700 Subject: [PATCH 7/7] Update test_management_scripts.py --- src/registrar/tests/test_management_scripts.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 443bafa71..a1c9a6c26 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -33,7 +33,6 @@ from unittest.mock import patch, call, MagicMock, mock_open from epplibwrapper import commands, common from .common import ( - MockDb, MockEppLib, less_console_noise, completed_domain_request,