From 0118e1f00dd2abe550d851c5fb0ba77f81b0149e Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 11:36:44 -0700 Subject: [PATCH 01/36] Add suborg data and clean duplicate script --- .../commands/clean_duplicate_suborgs.py | 123 ++++++++++++++++++ .../commands/create_federal_portfolio.py | 23 +++- 2 files changed, 139 insertions(+), 7 deletions(-) create mode 100644 src/registrar/management/commands/clean_duplicate_suborgs.py diff --git a/src/registrar/management/commands/clean_duplicate_suborgs.py b/src/registrar/management/commands/clean_duplicate_suborgs.py new file mode 100644 index 000000000..a5c7a87f0 --- /dev/null +++ b/src/registrar/management/commands/clean_duplicate_suborgs.py @@ -0,0 +1,123 @@ +import logging +from django.core.management import BaseCommand +from registrar.models import Suborganization, DomainRequest, DomainInformation +from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper + + +logger = logging.getLogger(__name__) + + +class Command(BaseCommand): + help = "Clean up duplicate suborganizations that differ only by spaces and capitalization" + + def handle(self, **kwargs): + # Find duplicates + duplicates = {} + all_suborgs = Suborganization.objects.all() + + for suborg in all_suborgs: + # Normalize name by removing extra spaces and converting to lowercase + normalized_name = " ".join(suborg.name.split()).lower() + + # First occurrence of this name + if normalized_name not in duplicates: + duplicates[normalized_name] = { + "keep": suborg, + "delete": [] + } + continue + + # Compare with our current best + current_best = duplicates[normalized_name]["keep"] + + # Check if all other fields match. + # If they don't, we should inspect this record manually. + fields_to_compare = ["portfolio", "city", "state_territory"] + fields_match = all( + getattr(suborg, field) == getattr(current_best, field) + for field in fields_to_compare + ) + if not fields_match: + logger.warning( + f"{TerminalColors.YELLOW}" + f"\nSkipping potential duplicate: {suborg.name} (id: {suborg.id})" + f"\nData mismatch with {current_best.name} (id: {current_best.id})" + f"{TerminalColors.ENDC}" + ) + continue + + # Determine if new suborg is better than current best. + # The fewest spaces and most capitals wins. + new_has_fewer_spaces = suborg.name.count(" ") < current_best.name.count(" ") + new_has_more_capitals = sum(1 for c in suborg.name if c.isupper()) > sum(1 for c in current_best.name if c.isupper()) + # TODO + # Split into words and count properly capitalized first letters + # new_proper_caps = sum( + # 1 for word in suborg.name.split() + # if word and word[0].isupper() + # ) + # current_proper_caps = sum( + # 1 for word in current_best.name.split() + # if word and word[0].isupper() + # ) + # new_has_better_caps = new_proper_caps > current_proper_caps + + if new_has_fewer_spaces or new_has_more_capitals: + # New suborg is better - demote the old one to the delete list + duplicates[normalized_name]["delete"].append(current_best) + duplicates[normalized_name]["keep"] = suborg + else: + # If it is not better, just delete the old one + duplicates[normalized_name]["delete"].append(suborg) + + # Filter out entries without duplicates + duplicates = {k: v for k, v in duplicates.items() if v.get("delete")} + if not duplicates: + logger.info(f"No duplicate suborganizations found.") + return + + # Show preview of changes + preview = "The following duplicates will be removed:\n" + for data in duplicates.values(): + best = data.get("keep") + preview += f"\nKeeping: '{best.name}' (id: {best.id})" + + for duplicate in data.get("delete"): + preview += f"\nRemoving: '{duplicate.name}' (id: {duplicate.id})" + preview += "\n" + + # Get confirmation and execute deletions + if TerminalHelper.prompt_for_execution( + system_exit_on_terminate=True, + prompt_message=preview, + prompt_title="Clean up duplicate suborganizations?", + verify_message="*** WARNING: This will delete suborganizations! ***" + ): + try: + # Update all references to point to the right suborg before deletion + for record in duplicates.values(): + best_record = record.get("keep") + delete_ids = [dupe.id for dupe in record.get("delete")] + + # Update domain requests + DomainRequest.objects.filter( + sub_organization_id__in=delete_ids + ).update(sub_organization=best_record) + + # Update domain information + DomainInformation.objects.filter( + sub_organization_id__in=delete_ids + ).update(sub_organization=best_record) + + ids_to_delete = [ + dupe.id + for data in duplicates.values() + for dupe in data["delete"] + ] + + # Bulk delete all duplicates + delete_count, _ = Suborganization.objects.filter(id__in=ids_to_delete).delete() + logger.info(f"{TerminalColors.OKGREEN}Successfully deleted {delete_count} suborganizations{TerminalColors.ENDC}") + + except Exception as e: + logger.error(f"{TerminalColors.FAIL}Failed to clean up suborganizations: {str(e)}{TerminalColors.ENDC}") diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 9cf4d36ea..b8a0ed091 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -104,7 +104,11 @@ def handle_populate_portfolio(self, federal_agency, parse_domains, parse_request also create new suborganizations""" portfolio, created = self.create_portfolio(federal_agency) if created: - self.create_suborganizations(portfolio, federal_agency) + valid_agencies = DomainInformation.objects.filter( + federal_agency=federal_agency, organization_name__isnull=False + ) + org_names = set(valid_agencies.values_list("organization_name", flat=True)) + self.create_suborganizations(portfolio, federal_agency, org_names) if parse_domains or both: self.handle_portfolio_domains(portfolio, federal_agency) @@ -155,13 +159,8 @@ def create_portfolio(self, federal_agency): return portfolio, True - def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency): + def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency, org_names: set): """Create Suborganizations tied to the given portfolio based on DomainInformation objects""" - valid_agencies = DomainInformation.objects.filter( - federal_agency=federal_agency, organization_name__isnull=False - ) - org_names = set(valid_agencies.values_list("organization_name", flat=True)) - if not org_names: message = ( "Could not add any suborganizations." @@ -232,6 +231,16 @@ def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: Federa domain_request.portfolio = portfolio if domain_request.organization_name in suborgs: domain_request.sub_organization = suborgs.get(domain_request.organization_name) + else: + # Fill in the requesting suborg fields if we have the data to do so + if domain_request.organization_name and domain_request.city and domain_request.state_territory: + domain_request.requested_suborganization = domain_request.organization_name + domain_request.suborganization_city = domain_request.city + domain_request.suborganization_state_territory = domain_request.state_territory + else: + message = f"No suborganization data found whatsoever for {domain_request}." + TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) + self.updated_portfolios.add(portfolio) DomainRequest.objects.bulk_update(domain_requests, ["portfolio", "sub_organization"]) From d0183d4d149c643d638c3fd9856ef90ef715ae67 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 12:42:49 -0700 Subject: [PATCH 02/36] cleanup --- .../commands/clean_duplicate_suborgs.py | 23 +++++-------------- .../commands/create_federal_portfolio.py | 2 +- .../models/utility/generic_helper.py | 6 +++++ 3 files changed, 13 insertions(+), 18 deletions(-) diff --git a/src/registrar/management/commands/clean_duplicate_suborgs.py b/src/registrar/management/commands/clean_duplicate_suborgs.py index a5c7a87f0..2e44746ad 100644 --- a/src/registrar/management/commands/clean_duplicate_suborgs.py +++ b/src/registrar/management/commands/clean_duplicate_suborgs.py @@ -17,7 +17,7 @@ def handle(self, **kwargs): for suborg in all_suborgs: # Normalize name by removing extra spaces and converting to lowercase - normalized_name = " ".join(suborg.name.split()).lower() + normalized_name = " ".join(suborg.name.trim().split()).lower() # First occurrence of this name if normalized_name not in duplicates: @@ -28,7 +28,8 @@ def handle(self, **kwargs): continue # Compare with our current best - current_best = duplicates[normalized_name]["keep"] + duplicate_record = duplicates.get(normalized_name) + current_best = duplicate_record.get("keep") # Check if all other fields match. # If they don't, we should inspect this record manually. @@ -50,25 +51,13 @@ def handle(self, **kwargs): # The fewest spaces and most capitals wins. new_has_fewer_spaces = suborg.name.count(" ") < current_best.name.count(" ") new_has_more_capitals = sum(1 for c in suborg.name if c.isupper()) > sum(1 for c in current_best.name if c.isupper()) - # TODO - # Split into words and count properly capitalized first letters - # new_proper_caps = sum( - # 1 for word in suborg.name.split() - # if word and word[0].isupper() - # ) - # current_proper_caps = sum( - # 1 for word in current_best.name.split() - # if word and word[0].isupper() - # ) - # new_has_better_caps = new_proper_caps > current_proper_caps - if new_has_fewer_spaces or new_has_more_capitals: # New suborg is better - demote the old one to the delete list - duplicates[normalized_name]["delete"].append(current_best) - duplicates[normalized_name]["keep"] = suborg + duplicate_record["delete"].append(current_best) + duplicate_record["keep"] = suborg else: # If it is not better, just delete the old one - duplicates[normalized_name]["delete"].append(suborg) + duplicate_record["delete"].append(suborg) # Filter out entries without duplicates duplicates = {k: v for k, v in duplicates.items() if v.get("delete")} diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index b8a0ed091..15740cca9 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -107,7 +107,7 @@ def handle_populate_portfolio(self, federal_agency, parse_domains, parse_request valid_agencies = DomainInformation.objects.filter( federal_agency=federal_agency, organization_name__isnull=False ) - org_names = set(valid_agencies.values_list("organization_name", flat=True)) + org_names = set([agency.trim() for agency in valid_agencies.values_list("organization_name", flat=True)]) self.create_suborganizations(portfolio, federal_agency, org_names) if parse_domains or both: self.handle_portfolio_domains(portfolio, federal_agency) diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 5e425f5a3..d1812d476 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -343,3 +343,9 @@ def value_of_attribute(obj, attribute_name: str): if callable(value): value = value() return value + + +def normalize_string(string_to_normalize: str) -> str: + """Normalizes a given string. Returns a string without extra spaces, in all lowercase.""" + new_string = " ".join(string_to_normalize.trim().split()) + return new_string.lower() From 6086b9587f10ab44aa0274a2289fa511663138ca Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 12:43:48 -0700 Subject: [PATCH 03/36] Remove unrelated content --- .../commands/clean_duplicate_suborgs.py | 112 ------------------ .../models/utility/generic_helper.py | 6 - 2 files changed, 118 deletions(-) delete mode 100644 src/registrar/management/commands/clean_duplicate_suborgs.py diff --git a/src/registrar/management/commands/clean_duplicate_suborgs.py b/src/registrar/management/commands/clean_duplicate_suborgs.py deleted file mode 100644 index 2e44746ad..000000000 --- a/src/registrar/management/commands/clean_duplicate_suborgs.py +++ /dev/null @@ -1,112 +0,0 @@ -import logging -from django.core.management import BaseCommand -from registrar.models import Suborganization, DomainRequest, DomainInformation -from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper - - -logger = logging.getLogger(__name__) - - -class Command(BaseCommand): - help = "Clean up duplicate suborganizations that differ only by spaces and capitalization" - - def handle(self, **kwargs): - # Find duplicates - duplicates = {} - all_suborgs = Suborganization.objects.all() - - for suborg in all_suborgs: - # Normalize name by removing extra spaces and converting to lowercase - normalized_name = " ".join(suborg.name.trim().split()).lower() - - # First occurrence of this name - if normalized_name not in duplicates: - duplicates[normalized_name] = { - "keep": suborg, - "delete": [] - } - continue - - # Compare with our current best - duplicate_record = duplicates.get(normalized_name) - current_best = duplicate_record.get("keep") - - # Check if all other fields match. - # If they don't, we should inspect this record manually. - fields_to_compare = ["portfolio", "city", "state_territory"] - fields_match = all( - getattr(suborg, field) == getattr(current_best, field) - for field in fields_to_compare - ) - if not fields_match: - logger.warning( - f"{TerminalColors.YELLOW}" - f"\nSkipping potential duplicate: {suborg.name} (id: {suborg.id})" - f"\nData mismatch with {current_best.name} (id: {current_best.id})" - f"{TerminalColors.ENDC}" - ) - continue - - # Determine if new suborg is better than current best. - # The fewest spaces and most capitals wins. - new_has_fewer_spaces = suborg.name.count(" ") < current_best.name.count(" ") - new_has_more_capitals = sum(1 for c in suborg.name if c.isupper()) > sum(1 for c in current_best.name if c.isupper()) - if new_has_fewer_spaces or new_has_more_capitals: - # New suborg is better - demote the old one to the delete list - duplicate_record["delete"].append(current_best) - duplicate_record["keep"] = suborg - else: - # If it is not better, just delete the old one - duplicate_record["delete"].append(suborg) - - # Filter out entries without duplicates - duplicates = {k: v for k, v in duplicates.items() if v.get("delete")} - if not duplicates: - logger.info(f"No duplicate suborganizations found.") - return - - # Show preview of changes - preview = "The following duplicates will be removed:\n" - for data in duplicates.values(): - best = data.get("keep") - preview += f"\nKeeping: '{best.name}' (id: {best.id})" - - for duplicate in data.get("delete"): - preview += f"\nRemoving: '{duplicate.name}' (id: {duplicate.id})" - preview += "\n" - - # Get confirmation and execute deletions - if TerminalHelper.prompt_for_execution( - system_exit_on_terminate=True, - prompt_message=preview, - prompt_title="Clean up duplicate suborganizations?", - verify_message="*** WARNING: This will delete suborganizations! ***" - ): - try: - # Update all references to point to the right suborg before deletion - for record in duplicates.values(): - best_record = record.get("keep") - delete_ids = [dupe.id for dupe in record.get("delete")] - - # Update domain requests - DomainRequest.objects.filter( - sub_organization_id__in=delete_ids - ).update(sub_organization=best_record) - - # Update domain information - DomainInformation.objects.filter( - sub_organization_id__in=delete_ids - ).update(sub_organization=best_record) - - ids_to_delete = [ - dupe.id - for data in duplicates.values() - for dupe in data["delete"] - ] - - # Bulk delete all duplicates - delete_count, _ = Suborganization.objects.filter(id__in=ids_to_delete).delete() - logger.info(f"{TerminalColors.OKGREEN}Successfully deleted {delete_count} suborganizations{TerminalColors.ENDC}") - - except Exception as e: - logger.error(f"{TerminalColors.FAIL}Failed to clean up suborganizations: {str(e)}{TerminalColors.ENDC}") diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index d1812d476..5e425f5a3 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -343,9 +343,3 @@ def value_of_attribute(obj, attribute_name: str): if callable(value): value = value() return value - - -def normalize_string(string_to_normalize: str) -> str: - """Normalizes a given string. Returns a string without extra spaces, in all lowercase.""" - new_string = " ".join(string_to_normalize.trim().split()) - return new_string.lower() From 345ca0307e161fe76e982b941b18064edd704f43 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 12:46:36 -0700 Subject: [PATCH 04/36] Update create_federal_portfolio.py --- .../management/commands/create_federal_portfolio.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 15740cca9..f73f1ec66 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -104,11 +104,7 @@ def handle_populate_portfolio(self, federal_agency, parse_domains, parse_request also create new suborganizations""" portfolio, created = self.create_portfolio(federal_agency) if created: - valid_agencies = DomainInformation.objects.filter( - federal_agency=federal_agency, organization_name__isnull=False - ) - org_names = set([agency.trim() for agency in valid_agencies.values_list("organization_name", flat=True)]) - self.create_suborganizations(portfolio, federal_agency, org_names) + self.create_suborganizations(portfolio, federal_agency) if parse_domains or both: self.handle_portfolio_domains(portfolio, federal_agency) @@ -159,8 +155,13 @@ def create_portfolio(self, federal_agency): return portfolio, True - def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency, org_names: set): + def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency): """Create Suborganizations tied to the given portfolio based on DomainInformation objects""" + valid_agencies = DomainInformation.objects.filter( + federal_agency=federal_agency, organization_name__isnull=False + ) + org_names = set(valid_agencies.values_list("organization_name", flat=True)) + if not org_names: message = ( "Could not add any suborganizations." From 5e6a7987dfbd6ef9222c9adc66ead235f9433b8b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 12:47:57 -0700 Subject: [PATCH 05/36] Update create_federal_portfolio.py --- .../management/commands/create_federal_portfolio.py | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index f73f1ec66..5fac24f53 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -233,14 +233,9 @@ def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: Federa if domain_request.organization_name in suborgs: domain_request.sub_organization = suborgs.get(domain_request.organization_name) else: - # Fill in the requesting suborg fields if we have the data to do so - if domain_request.organization_name and domain_request.city and domain_request.state_territory: - domain_request.requested_suborganization = domain_request.organization_name - domain_request.suborganization_city = domain_request.city - domain_request.suborganization_state_territory = domain_request.state_territory - else: - message = f"No suborganization data found whatsoever for {domain_request}." - TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) + domain_request.requested_suborganization = domain_request.organization_name + domain_request.suborganization_city = domain_request.city + domain_request.suborganization_state_territory = domain_request.state_territory self.updated_portfolios.add(portfolio) From 9052d9ed3f3924293675879c9e6955ffb4cf06a4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 13:21:50 -0700 Subject: [PATCH 06/36] Use dedicated script --- .../commands/create_federal_portfolio.py | 5 --- .../commands/populate_requested_suborg.py | 37 +++++++++++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) create mode 100644 src/registrar/management/commands/populate_requested_suborg.py diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 5fac24f53..9cf4d36ea 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -232,11 +232,6 @@ def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: Federa domain_request.portfolio = portfolio if domain_request.organization_name in suborgs: domain_request.sub_organization = suborgs.get(domain_request.organization_name) - else: - domain_request.requested_suborganization = domain_request.organization_name - domain_request.suborganization_city = domain_request.city - domain_request.suborganization_state_territory = domain_request.state_territory - self.updated_portfolios.add(portfolio) DomainRequest.objects.bulk_update(domain_requests, ["portfolio", "sub_organization"]) diff --git a/src/registrar/management/commands/populate_requested_suborg.py b/src/registrar/management/commands/populate_requested_suborg.py new file mode 100644 index 000000000..54f811413 --- /dev/null +++ b/src/registrar/management/commands/populate_requested_suborg.py @@ -0,0 +1,37 @@ +import logging +from django.core.management import BaseCommand +from registrar.management.commands.utility.terminal_helper import PopulateScriptTemplate, TerminalColors, TerminalHelper +from registrar.models import DomainRequest + +logger = logging.getLogger(__name__) + + +class Command(BaseCommand, PopulateScriptTemplate): + help = "Loops through each domain request object and populates the last_status_update and first_submitted_date" + + def handle(self, **kwargs): + """Loops through each DomainRequest object and populates + its last_status_update and first_submitted_date values""" + filter_conditions = {"portfolio__isnull": False, "sub_organization__isnull": True} + fields_to_update = ["requested_suborganization", "suborganization_city", "suborganization_state_territory"] + self.mass_update_records(DomainRequest, filter_conditions, fields_to_update) + + def update_record(self, record: DomainRequest): + """Adds data to requested_suborganization, suborganization_city, and suborganization_state_territory.""" + record.requested_suborganization = record.organization_name + record.suborganization_city = record.city + record.suborganization_state_territory = record.state_territory + message = ( + f"Updating {record} => requested_suborg: {record.organization_name}, " + f"sub_city: {record.city}, suborg_state_territory: {record.state_territory}." + ) + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, message) + + def should_skip_record(self, record: DomainRequest) -> bool: + """Skips record update if we're missing org name, city, and state territory.""" + required_fields = [ + record.organization_name, + record.city, + record.state_territory + ] + return not all(bool(field) for field in required_fields) From 188c99a16971320c1a9b54d84103539f51c3a9bd Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 13:39:56 -0700 Subject: [PATCH 07/36] Unit tests and documentation --- docs/operations/data_migration.md | 33 ++++++++ .../commands/populate_requested_suborg.py | 11 +-- src/registrar/tests/common.py | 8 +- .../tests/test_management_scripts.py | 77 ++++++++++++++++++- 4 files changed, 117 insertions(+), 12 deletions(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index 0863aa0b7..93a5a4241 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -918,3 +918,36 @@ Example (only requests): `./manage.py create_federal_portfolio --branch "executi - 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. + + +## Populate requested suborg +This script populates the "requested suborganization" fields on DomainRequest. +These fields are: requested_suborganization, suborganization_city, suborganization_state_territory. + +This is done by pulling from organization_name, city, and state_territory. +The script will only parse records with a portfolio but no suborganization attached to it. + +### Running on sandboxes + +#### Step 1: Login to CloudFoundry +```cf login -a api.fr.cloud.gov --sso``` + +#### Step 2: SSH into your environment +```cf ssh getgov-{space}``` + +Example: `cf ssh getgov-za` + +#### Step 3: Create a shell instance +```/tmp/lifecycle/shell``` + +#### Step 4: Upload your csv to the desired sandbox +[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 +To create a specific portfolio: +```./manage.py populate_requested_suborg``` + +### Running locally + +#### Step 1: Running the script +```docker-compose exec app ./manage.py populate_requested_suborg``` diff --git a/src/registrar/management/commands/populate_requested_suborg.py b/src/registrar/management/commands/populate_requested_suborg.py index 54f811413..8bb0a77b9 100644 --- a/src/registrar/management/commands/populate_requested_suborg.py +++ b/src/registrar/management/commands/populate_requested_suborg.py @@ -7,7 +7,7 @@ class Command(BaseCommand, PopulateScriptTemplate): - help = "Loops through each domain request object and populates the last_status_update and first_submitted_date" + help = "Loops through each domain request object and populates requested suborg info" def handle(self, **kwargs): """Loops through each DomainRequest object and populates @@ -26,12 +26,3 @@ def update_record(self, record: DomainRequest): f"sub_city: {record.city}, suborg_state_territory: {record.state_territory}." ) TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, message) - - def should_skip_record(self, record: DomainRequest) -> bool: - """Skips record update if we're missing org name, city, and state territory.""" - required_fields = [ - record.organization_name, - record.city, - record.state_territory - ] - return not all(bool(field) for field in required_fields) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 8eca0108e..b79cbe0c2 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -1032,6 +1032,8 @@ def completed_domain_request( # noqa federal_agency=None, federal_type=None, action_needed_reason=None, + city=None, + state_territory=None, portfolio=None, organization_name=None, sub_organization=None, @@ -1076,7 +1078,7 @@ def completed_domain_request( # noqa organization_name=organization_name if organization_name else "Testorg", address_line1="address 1", address_line2="address 2", - state_territory="NY", + state_territory="NY" if not state_territory else state_territory, zipcode="10002", senior_official=so, requested_domain=domain, @@ -1085,6 +1087,10 @@ def completed_domain_request( # noqa investigator=investigator, federal_agency=federal_agency, ) + + if city: + domain_request_kwargs["city"] = city + if has_about_your_organization: domain_request_kwargs["about_your_organization"] = "e-Government" if has_anything_else: diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 7cce0d2b2..dfe0904c5 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -32,7 +32,7 @@ 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 +from .common import MockEppLib, create_user, less_console_noise, completed_domain_request, MockSESClient from api.tests.common import less_console_noise_decorator @@ -1731,3 +1731,78 @@ def test_does_not_update_existing_portfolio(self): self.assertEqual(existing_portfolio.organization_name, self.federal_agency.agency) self.assertEqual(existing_portfolio.notes, "Old notes") self.assertEqual(existing_portfolio.creator, self.user) + + +class TestPopulateRequestedSuborg(MockEppLib): + """Tests for the populate_requested_suborg script""" + + @less_console_noise_decorator + def setUp(self): + """Creates test domain requests with various states""" + super().setUp() + + self.user = create_user() + # Create a portfolio + self.portfolio = Portfolio.objects.create(organization_name="Test Portfolio", creator=self.user) + + # Create a domain request with all required fields + self.complete_request = completed_domain_request( + name="complete.gov", + organization_name="Complete Org", + city="Washington", + state_territory="DC", + portfolio=self.portfolio, + ) + + # Create a request that already has a suborganization + self.suborg = Suborganization.objects.create(name="Existing Suborg", portfolio=self.portfolio) + self.existing_suborg_request = completed_domain_request( + name="existing.gov", + organization_name="Existing Org", + city="New York", + state_territory="NY", + portfolio=self.portfolio, + sub_organization=self.suborg, + ) + + def tearDown(self): + """Cleans up test data""" + super().tearDown() + DomainRequest.objects.all().delete() + Suborganization.objects.all().delete() + Portfolio.objects.all().delete() + User.objects.all().delete() + + @less_console_noise_decorator + def run_populate_requested_suborg(self): + """Executes the populate_requested_suborg command""" + with patch( + "registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit", + return_value=True, + ): + call_command("populate_requested_suborg") + + @less_console_noise_decorator + def test_populate_requested_suborg_complete_request(self): + """Tests that complete requests are updated correctly""" + self.run_populate_requested_suborg() + + self.complete_request.refresh_from_db() + + # Verify fields were populated correctly + self.assertEqual(self.complete_request.requested_suborganization, "Complete Org") + self.assertEqual(self.complete_request.suborganization_city, "Washington") + self.assertEqual(self.complete_request.suborganization_state_territory, "DC") + + @less_console_noise_decorator + def test_populate_requested_suborg_existing_suborg(self): + """Tests that requests with existing suborgs are skipped""" + self.run_populate_requested_suborg() + + self.existing_suborg_request.refresh_from_db() + + # Verify the request wasn't modified + self.assertEqual(self.existing_suborg_request.sub_organization, self.suborg) + self.assertIsNone(self.existing_suborg_request.requested_suborganization) + self.assertIsNone(self.existing_suborg_request.suborganization_city) + self.assertIsNone(self.existing_suborg_request.suborganization_state_territory) From 822c8dde0f1273368cfab986144e96344c016574 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 13:40:51 -0700 Subject: [PATCH 08/36] Update test_management_scripts.py --- src/registrar/tests/test_management_scripts.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index dfe0904c5..de35fb02b 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1738,7 +1738,6 @@ class TestPopulateRequestedSuborg(MockEppLib): @less_console_noise_decorator def setUp(self): - """Creates test domain requests with various states""" super().setUp() self.user = create_user() @@ -1766,7 +1765,6 @@ def setUp(self): ) def tearDown(self): - """Cleans up test data""" super().tearDown() DomainRequest.objects.all().delete() Suborganization.objects.all().delete() From 6d13f26ffb18e972d5ebcaf582a2efb6cf5b33fd Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 13:51:40 -0700 Subject: [PATCH 09/36] Add rule not to parse cases where the org name matches the portfolio name --- docs/operations/data_migration.md | 1 + .../commands/populate_requested_suborg.py | 5 +++++ .../tests/test_management_scripts.py | 22 +++++++++++++++++++ 3 files changed, 28 insertions(+) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index 93a5a4241..32de4d31a 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -926,6 +926,7 @@ These fields are: requested_suborganization, suborganization_city, suborganizati This is done by pulling from organization_name, city, and state_territory. The script will only parse records with a portfolio but no suborganization attached to it. +Additionally, it will not parse records wherein the organization name matches the portfolio org name. ### Running on sandboxes diff --git a/src/registrar/management/commands/populate_requested_suborg.py b/src/registrar/management/commands/populate_requested_suborg.py index 8bb0a77b9..e33a93e42 100644 --- a/src/registrar/management/commands/populate_requested_suborg.py +++ b/src/registrar/management/commands/populate_requested_suborg.py @@ -2,6 +2,7 @@ from django.core.management import BaseCommand from registrar.management.commands.utility.terminal_helper import PopulateScriptTemplate, TerminalColors, TerminalHelper from registrar.models import DomainRequest +from django.db.models import F logger = logging.getLogger(__name__) @@ -16,6 +17,10 @@ def handle(self, **kwargs): fields_to_update = ["requested_suborganization", "suborganization_city", "suborganization_state_territory"] self.mass_update_records(DomainRequest, filter_conditions, fields_to_update) + def custom_filter(self, records): + """Exclude domain requests that have the same org name as the portfolio""" + return records.exclude(organization_name=F("portfolio__organization_name")) + def update_record(self, record: DomainRequest): """Adds data to requested_suborganization, suborganization_city, and suborganization_state_territory.""" record.requested_suborganization = record.organization_name diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index de35fb02b..29b310fd7 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1764,6 +1764,16 @@ def setUp(self): sub_organization=self.suborg, ) + # Create a request where org name matches portfolio name + self.matching_name_request = completed_domain_request( + name="matching.gov", + organization_name="Test Portfolio", + city="Boston", + state_territory="MA", + portfolio=self.portfolio, + user=self.user, + ) + def tearDown(self): super().tearDown() DomainRequest.objects.all().delete() @@ -1804,3 +1814,15 @@ def test_populate_requested_suborg_existing_suborg(self): self.assertIsNone(self.existing_suborg_request.requested_suborganization) self.assertIsNone(self.existing_suborg_request.suborganization_city) self.assertIsNone(self.existing_suborg_request.suborganization_state_territory) + + @less_console_noise_decorator + def test_populate_requested_suborg_matching_name(self): + """Tests that requests with matching org/portfolio names are skipped""" + self.run_populate_requested_suborg() + + self.matching_name_request.refresh_from_db() + + # Verify the request wasn't modified since org name matches portfolio name + self.assertIsNone(self.matching_name_request.requested_suborganization) + self.assertIsNone(self.matching_name_request.suborganization_city) + self.assertIsNone(self.matching_name_request.suborganization_state_territory) From 87b08e3fa0111bfa7521819b89f7c34200e8eeca Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 14:00:52 -0700 Subject: [PATCH 10/36] Update populate_requested_suborg.py --- src/registrar/management/commands/populate_requested_suborg.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/management/commands/populate_requested_suborg.py b/src/registrar/management/commands/populate_requested_suborg.py index e33a93e42..e65ed3f53 100644 --- a/src/registrar/management/commands/populate_requested_suborg.py +++ b/src/registrar/management/commands/populate_requested_suborg.py @@ -19,7 +19,7 @@ def handle(self, **kwargs): def custom_filter(self, records): """Exclude domain requests that have the same org name as the portfolio""" - return records.exclude(organization_name=F("portfolio__organization_name")) + return records.exclude(organization_name__iexact=F("portfolio__organization_name")) def update_record(self, record: DomainRequest): """Adds data to requested_suborganization, suborganization_city, and suborganization_state_territory.""" From 8e7ffab8b2a34b3812c46fd6a254c1233d40bdd9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 14:10:40 -0700 Subject: [PATCH 11/36] Exclude approved --- .../management/commands/populate_requested_suborg.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/registrar/management/commands/populate_requested_suborg.py b/src/registrar/management/commands/populate_requested_suborg.py index e65ed3f53..2fdb14ac2 100644 --- a/src/registrar/management/commands/populate_requested_suborg.py +++ b/src/registrar/management/commands/populate_requested_suborg.py @@ -18,8 +18,12 @@ def handle(self, **kwargs): self.mass_update_records(DomainRequest, filter_conditions, fields_to_update) def custom_filter(self, records): - """Exclude domain requests that have the same org name as the portfolio""" - return records.exclude(organization_name__iexact=F("portfolio__organization_name")) + """Exclude domain requests that have the same org name as the portfolio. + Also excludes approved.""" + return records.exclude( + organization_name__iexact=F("portfolio__organization_name"), + status=DomainRequest.DomainRequestStatus.APPROVED, + ) def update_record(self, record: DomainRequest): """Adds data to requested_suborganization, suborganization_city, and suborganization_state_territory.""" From 4fceb62c434b41da8bee79c8f55560d3423bb471 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 14:17:33 -0700 Subject: [PATCH 12/36] Update populate_requested_suborg.py --- .../management/commands/populate_requested_suborg.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/registrar/management/commands/populate_requested_suborg.py b/src/registrar/management/commands/populate_requested_suborg.py index 2fdb14ac2..580d1cdbc 100644 --- a/src/registrar/management/commands/populate_requested_suborg.py +++ b/src/registrar/management/commands/populate_requested_suborg.py @@ -22,7 +22,12 @@ def custom_filter(self, records): Also excludes approved.""" return records.exclude( organization_name__iexact=F("portfolio__organization_name"), - status=DomainRequest.DomainRequestStatus.APPROVED, + status__in=[ + DomainRequest.DomainRequestStatus.APPROVED, + DomainRequest.DomainRequestStatus.REJECTED, + DomainRequest.DomainRequestStatus.INELIGIBLE, + DomainRequest.DomainRequestStatus.STARTED, + ], ) def update_record(self, record: DomainRequest): From e91c160a67301963140132b381dc4fdd6d0695c2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 14:51:43 -0700 Subject: [PATCH 13/36] bug fix --- .../commands/populate_requested_suborg.py | 34 ++++++++++++------- .../models/utility/generic_helper.py | 6 ++++ .../tests/test_management_scripts.py | 3 ++ 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/src/registrar/management/commands/populate_requested_suborg.py b/src/registrar/management/commands/populate_requested_suborg.py index 580d1cdbc..121eb497c 100644 --- a/src/registrar/management/commands/populate_requested_suborg.py +++ b/src/registrar/management/commands/populate_requested_suborg.py @@ -4,6 +4,8 @@ from registrar.models import DomainRequest from django.db.models import F +from registrar.models.utility.generic_helper import normalize_string + logger = logging.getLogger(__name__) @@ -17,19 +19,6 @@ def handle(self, **kwargs): fields_to_update = ["requested_suborganization", "suborganization_city", "suborganization_state_territory"] self.mass_update_records(DomainRequest, filter_conditions, fields_to_update) - def custom_filter(self, records): - """Exclude domain requests that have the same org name as the portfolio. - Also excludes approved.""" - return records.exclude( - organization_name__iexact=F("portfolio__organization_name"), - status__in=[ - DomainRequest.DomainRequestStatus.APPROVED, - DomainRequest.DomainRequestStatus.REJECTED, - DomainRequest.DomainRequestStatus.INELIGIBLE, - DomainRequest.DomainRequestStatus.STARTED, - ], - ) - def update_record(self, record: DomainRequest): """Adds data to requested_suborganization, suborganization_city, and suborganization_state_territory.""" record.requested_suborganization = record.organization_name @@ -40,3 +29,22 @@ def update_record(self, record: DomainRequest): f"sub_city: {record.city}, suborg_state_territory: {record.state_territory}." ) TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, message) + + def should_skip_record(self, record) -> bool: + """Skips updating the record if the portfolio name is the same as the org name, + or if we are trying to update a record in an invalid status.""" + portfolio_normalized = normalize_string(record.portfolio.organization_name) + org_normalized = normalize_string(record.organization_name) + if portfolio_normalized == org_normalized: + return True + + invalid_statuses = [ + DomainRequest.DomainRequestStatus.APPROVED, + DomainRequest.DomainRequestStatus.REJECTED, + DomainRequest.DomainRequestStatus.INELIGIBLE, + DomainRequest.DomainRequestStatus.STARTED, + ] + if record.status in invalid_statuses: + return True + + return False diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 5e425f5a3..9ee39911d 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -343,3 +343,9 @@ def value_of_attribute(obj, attribute_name: str): if callable(value): value = value() return value + + +def normalize_string(string_to_normalize: str) -> str: + """Normalizes a given string. Returns a string without extra spaces, in all lowercase.""" + new_string = " ".join(string_to_normalize.split()) + return new_string.lower() diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 29b310fd7..1c92728d2 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1751,6 +1751,7 @@ def setUp(self): city="Washington", state_territory="DC", portfolio=self.portfolio, + status=DomainRequest.DomainRequestStatus.IN_REVIEW, ) # Create a request that already has a suborganization @@ -1762,6 +1763,7 @@ def setUp(self): state_territory="NY", portfolio=self.portfolio, sub_organization=self.suborg, + status=DomainRequest.DomainRequestStatus.IN_REVIEW, ) # Create a request where org name matches portfolio name @@ -1772,6 +1774,7 @@ def setUp(self): state_territory="MA", portfolio=self.portfolio, user=self.user, + status=DomainRequest.DomainRequestStatus.IN_REVIEW, ) def tearDown(self): From d68ccdc1b1aa2104a97128f22af0aad2e5d89a83 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 15:02:33 -0700 Subject: [PATCH 14/36] Finish script --- .../commands/populate_requested_suborg.py | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/registrar/management/commands/populate_requested_suborg.py b/src/registrar/management/commands/populate_requested_suborg.py index 121eb497c..0273aba28 100644 --- a/src/registrar/management/commands/populate_requested_suborg.py +++ b/src/registrar/management/commands/populate_requested_suborg.py @@ -2,8 +2,6 @@ from django.core.management import BaseCommand from registrar.management.commands.utility.terminal_helper import PopulateScriptTemplate, TerminalColors, TerminalHelper from registrar.models import DomainRequest -from django.db.models import F - from registrar.models.utility.generic_helper import normalize_string logger = logging.getLogger(__name__) @@ -15,7 +13,12 @@ class Command(BaseCommand, PopulateScriptTemplate): def handle(self, **kwargs): """Loops through each DomainRequest object and populates its last_status_update and first_submitted_date values""" - filter_conditions = {"portfolio__isnull": False, "sub_organization__isnull": True} + filter_conditions = { + "portfolio__isnull": False, + "organization_name__isnull": False, + "sub_organization__isnull": True, + "portfolio__organization_name__isnull": False, + } fields_to_update = ["requested_suborganization", "suborganization_city", "suborganization_state_territory"] self.mass_update_records(DomainRequest, filter_conditions, fields_to_update) @@ -29,15 +32,10 @@ def update_record(self, record: DomainRequest): f"sub_city: {record.city}, suborg_state_territory: {record.state_territory}." ) TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, message) - + def should_skip_record(self, record) -> bool: """Skips updating the record if the portfolio name is the same as the org name, or if we are trying to update a record in an invalid status.""" - portfolio_normalized = normalize_string(record.portfolio.organization_name) - org_normalized = normalize_string(record.organization_name) - if portfolio_normalized == org_normalized: - return True - invalid_statuses = [ DomainRequest.DomainRequestStatus.APPROVED, DomainRequest.DomainRequestStatus.REJECTED, @@ -47,4 +45,9 @@ def should_skip_record(self, record) -> bool: if record.status in invalid_statuses: return True + portfolio_normalized = normalize_string(record.portfolio.organization_name) + org_normalized = normalize_string(record.organization_name) + if portfolio_normalized == org_normalized: + return True + return False From feb319f3c2839c3d9a444e5324987aff618c0b0a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 6 Jan 2025 15:13:53 -0700 Subject: [PATCH 15/36] Update populate_requested_suborg.py --- src/registrar/management/commands/populate_requested_suborg.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/management/commands/populate_requested_suborg.py b/src/registrar/management/commands/populate_requested_suborg.py index 0273aba28..cf76c1fdf 100644 --- a/src/registrar/management/commands/populate_requested_suborg.py +++ b/src/registrar/management/commands/populate_requested_suborg.py @@ -37,7 +37,6 @@ def should_skip_record(self, record) -> bool: """Skips updating the record if the portfolio name is the same as the org name, or if we are trying to update a record in an invalid status.""" invalid_statuses = [ - DomainRequest.DomainRequestStatus.APPROVED, DomainRequest.DomainRequestStatus.REJECTED, DomainRequest.DomainRequestStatus.INELIGIBLE, DomainRequest.DomainRequestStatus.STARTED, From ca9a40caaf89bb050990ab4d696a62a7ace7cbca Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 Jan 2025 08:31:17 -0700 Subject: [PATCH 16/36] Add logic for requested suborgs --- .../management/commands/create_federal_portfolio.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 9cf4d36ea..f417154c7 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -232,6 +232,18 @@ def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: Federa domain_request.portfolio = portfolio if domain_request.organization_name in suborgs: domain_request.sub_organization = suborgs.get(domain_request.organization_name) + else: + clean_organization_name = None + if isinstance(domain_request.organization_name, str): + clean_organization_name = domain_request.organization_name.strip() + + clean_city = None + if isinstance(domain_request.city, str): + clean_city = domain_request.city.strip() + + domain_request.requested_suborganization = clean_organization_name + domain_request.suborganization_city = clean_city + domain_request.suborganization_state_territory = domain_request.state_territory self.updated_portfolios.add(portfolio) DomainRequest.objects.bulk_update(domain_requests, ["portfolio", "sub_organization"]) From 2c5c9de6dca75356db2d9eb3904bb97e06438818 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 Jan 2025 08:42:56 -0700 Subject: [PATCH 17/36] Remove second script, modify first --- docs/operations/data_migration.md | 33 ----- .../commands/populate_requested_suborg.py | 52 -------- .../models/utility/generic_helper.py | 6 - .../tests/test_management_scripts.py | 121 ++++-------------- 4 files changed, 24 insertions(+), 188 deletions(-) delete mode 100644 src/registrar/management/commands/populate_requested_suborg.py diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index 32de4d31a..ada3efa55 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -919,36 +919,3 @@ Example (only requests): `./manage.py create_federal_portfolio --branch "executi - 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. - -## Populate requested suborg -This script populates the "requested suborganization" fields on DomainRequest. -These fields are: requested_suborganization, suborganization_city, suborganization_state_territory. - -This is done by pulling from organization_name, city, and state_territory. -The script will only parse records with a portfolio but no suborganization attached to it. -Additionally, it will not parse records wherein the organization name matches the portfolio org name. - -### Running on sandboxes - -#### Step 1: Login to CloudFoundry -```cf login -a api.fr.cloud.gov --sso``` - -#### Step 2: SSH into your environment -```cf ssh getgov-{space}``` - -Example: `cf ssh getgov-za` - -#### Step 3: Create a shell instance -```/tmp/lifecycle/shell``` - -#### Step 4: Upload your csv to the desired sandbox -[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 -To create a specific portfolio: -```./manage.py populate_requested_suborg``` - -### Running locally - -#### Step 1: Running the script -```docker-compose exec app ./manage.py populate_requested_suborg``` diff --git a/src/registrar/management/commands/populate_requested_suborg.py b/src/registrar/management/commands/populate_requested_suborg.py deleted file mode 100644 index cf76c1fdf..000000000 --- a/src/registrar/management/commands/populate_requested_suborg.py +++ /dev/null @@ -1,52 +0,0 @@ -import logging -from django.core.management import BaseCommand -from registrar.management.commands.utility.terminal_helper import PopulateScriptTemplate, TerminalColors, TerminalHelper -from registrar.models import DomainRequest -from registrar.models.utility.generic_helper import normalize_string - -logger = logging.getLogger(__name__) - - -class Command(BaseCommand, PopulateScriptTemplate): - help = "Loops through each domain request object and populates requested suborg info" - - def handle(self, **kwargs): - """Loops through each DomainRequest object and populates - its last_status_update and first_submitted_date values""" - filter_conditions = { - "portfolio__isnull": False, - "organization_name__isnull": False, - "sub_organization__isnull": True, - "portfolio__organization_name__isnull": False, - } - fields_to_update = ["requested_suborganization", "suborganization_city", "suborganization_state_territory"] - self.mass_update_records(DomainRequest, filter_conditions, fields_to_update) - - def update_record(self, record: DomainRequest): - """Adds data to requested_suborganization, suborganization_city, and suborganization_state_territory.""" - record.requested_suborganization = record.organization_name - record.suborganization_city = record.city - record.suborganization_state_territory = record.state_territory - message = ( - f"Updating {record} => requested_suborg: {record.organization_name}, " - f"sub_city: {record.city}, suborg_state_territory: {record.state_territory}." - ) - TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, message) - - def should_skip_record(self, record) -> bool: - """Skips updating the record if the portfolio name is the same as the org name, - or if we are trying to update a record in an invalid status.""" - invalid_statuses = [ - DomainRequest.DomainRequestStatus.REJECTED, - DomainRequest.DomainRequestStatus.INELIGIBLE, - DomainRequest.DomainRequestStatus.STARTED, - ] - if record.status in invalid_statuses: - return True - - portfolio_normalized = normalize_string(record.portfolio.organization_name) - org_normalized = normalize_string(record.organization_name) - if portfolio_normalized == org_normalized: - return True - - return False diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 9ee39911d..5e425f5a3 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -343,9 +343,3 @@ def value_of_attribute(obj, attribute_name: str): if callable(value): value = value() return value - - -def normalize_string(string_to_normalize: str) -> str: - """Normalizes a given string. Returns a string without extra spaces, in all lowercase.""" - new_string = " ".join(string_to_normalize.split()) - return new_string.lower() diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 1c92728d2..d391cb34d 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1588,6 +1588,30 @@ def test_create_multiple_portfolios_for_branch_legislative(self): 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_script_adds_requested_suborganization_information(self): + """Tests that the script adds the requested suborg fields for domain requests""" + custom_suborg_request = completed_domain_request( + name="custom_org.gov", + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=self.executive_agency_2, + user=self.user, + organization_name="requested org name", + city="Austin", + state_territory=DomainRequest.StateTerritoryChoices.TEXAS + ) + + self.assertIsNone(custom_suborg_request.requested_suborganization) + self.assertIsNone(custom_suborg_request.suborganization_city) + self.assertIsNone(custom_suborg_request.suborganization_state_territory) + + # Run the script and test it + self.run_create_federal_portfolio(branch="executive", parse_requests=True) + + self.assertEqual(custom_suborg_request.requested_suborganization, "requested org name") + self.assertEqual(custom_suborg_request.suborganization_city, "Austin") + self.assertEqual(custom_suborg_request.suborganization_state_territory, DomainRequest.StateTerritoryChoices.TEXAS) + def test_create_multiple_portfolios_for_branch_executive(self): """Tests creating all portfolios under a given branch""" federal_choice = DomainRequest.OrganizationChoices.FEDERAL @@ -1732,100 +1756,3 @@ def test_does_not_update_existing_portfolio(self): self.assertEqual(existing_portfolio.notes, "Old notes") self.assertEqual(existing_portfolio.creator, self.user) - -class TestPopulateRequestedSuborg(MockEppLib): - """Tests for the populate_requested_suborg script""" - - @less_console_noise_decorator - def setUp(self): - super().setUp() - - self.user = create_user() - # Create a portfolio - self.portfolio = Portfolio.objects.create(organization_name="Test Portfolio", creator=self.user) - - # Create a domain request with all required fields - self.complete_request = completed_domain_request( - name="complete.gov", - organization_name="Complete Org", - city="Washington", - state_territory="DC", - portfolio=self.portfolio, - status=DomainRequest.DomainRequestStatus.IN_REVIEW, - ) - - # Create a request that already has a suborganization - self.suborg = Suborganization.objects.create(name="Existing Suborg", portfolio=self.portfolio) - self.existing_suborg_request = completed_domain_request( - name="existing.gov", - organization_name="Existing Org", - city="New York", - state_territory="NY", - portfolio=self.portfolio, - sub_organization=self.suborg, - status=DomainRequest.DomainRequestStatus.IN_REVIEW, - ) - - # Create a request where org name matches portfolio name - self.matching_name_request = completed_domain_request( - name="matching.gov", - organization_name="Test Portfolio", - city="Boston", - state_territory="MA", - portfolio=self.portfolio, - user=self.user, - status=DomainRequest.DomainRequestStatus.IN_REVIEW, - ) - - def tearDown(self): - super().tearDown() - DomainRequest.objects.all().delete() - Suborganization.objects.all().delete() - Portfolio.objects.all().delete() - User.objects.all().delete() - - @less_console_noise_decorator - def run_populate_requested_suborg(self): - """Executes the populate_requested_suborg command""" - with patch( - "registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit", - return_value=True, - ): - call_command("populate_requested_suborg") - - @less_console_noise_decorator - def test_populate_requested_suborg_complete_request(self): - """Tests that complete requests are updated correctly""" - self.run_populate_requested_suborg() - - self.complete_request.refresh_from_db() - - # Verify fields were populated correctly - self.assertEqual(self.complete_request.requested_suborganization, "Complete Org") - self.assertEqual(self.complete_request.suborganization_city, "Washington") - self.assertEqual(self.complete_request.suborganization_state_territory, "DC") - - @less_console_noise_decorator - def test_populate_requested_suborg_existing_suborg(self): - """Tests that requests with existing suborgs are skipped""" - self.run_populate_requested_suborg() - - self.existing_suborg_request.refresh_from_db() - - # Verify the request wasn't modified - self.assertEqual(self.existing_suborg_request.sub_organization, self.suborg) - self.assertIsNone(self.existing_suborg_request.requested_suborganization) - self.assertIsNone(self.existing_suborg_request.suborganization_city) - self.assertIsNone(self.existing_suborg_request.suborganization_state_territory) - - @less_console_noise_decorator - def test_populate_requested_suborg_matching_name(self): - """Tests that requests with matching org/portfolio names are skipped""" - self.run_populate_requested_suborg() - - self.matching_name_request.refresh_from_db() - - # Verify the request wasn't modified since org name matches portfolio name - self.assertIsNone(self.matching_name_request.requested_suborganization) - self.assertIsNone(self.matching_name_request.suborganization_city) - self.assertIsNone(self.matching_name_request.suborganization_state_territory) From 7e274920fe6481610f44b2cdd056dce1680c0588 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 Jan 2025 08:45:05 -0700 Subject: [PATCH 18/36] Remove errant spaces --- docs/operations/data_migration.md | 1 - src/registrar/tests/test_management_scripts.py | 1 - 2 files changed, 2 deletions(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index ada3efa55..0863aa0b7 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -918,4 +918,3 @@ Example (only requests): `./manage.py create_federal_portfolio --branch "executi - 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/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index d391cb34d..d4b9589cd 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1755,4 +1755,3 @@ def test_does_not_update_existing_portfolio(self): 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 d2804a64f5e5a3db469b280601d0a3f965e5ceed Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 Jan 2025 08:59:44 -0700 Subject: [PATCH 19/36] Add bulk update --- .../management/commands/create_federal_portfolio.py | 11 ++++++++++- src/registrar/tests/test_management_scripts.py | 12 ++++++++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index f417154c7..42f4f5687 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -246,7 +246,16 @@ def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: Federa domain_request.suborganization_state_territory = domain_request.state_territory self.updated_portfolios.add(portfolio) - DomainRequest.objects.bulk_update(domain_requests, ["portfolio", "sub_organization"]) + DomainRequest.objects.bulk_update( + domain_requests, + [ + "portfolio", + "sub_organization", + "requested_suborganization", + "suborganization_city", + "suborganization_state_territory", + ], + ) message = f"Added portfolio '{portfolio}' to {len(domain_requests)} domain requests." TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index d4b9589cd..4ca2a2d72 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1590,15 +1590,16 @@ def test_create_multiple_portfolios_for_branch_legislative(self): def test_script_adds_requested_suborganization_information(self): """Tests that the script adds the requested suborg fields for domain requests""" + # Create a new domain request with some errant spacing custom_suborg_request = completed_domain_request( name="custom_org.gov", status=DomainRequest.DomainRequestStatus.IN_REVIEW, generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, federal_agency=self.executive_agency_2, user=self.user, - organization_name="requested org name", - city="Austin", - state_territory=DomainRequest.StateTerritoryChoices.TEXAS + organization_name=" requested org name ", + city="Austin ", + state_territory=DomainRequest.StateTerritoryChoices.TEXAS, ) self.assertIsNone(custom_suborg_request.requested_suborganization) @@ -1607,10 +1608,13 @@ def test_script_adds_requested_suborganization_information(self): # Run the script and test it self.run_create_federal_portfolio(branch="executive", parse_requests=True) + custom_suborg_request.refresh_from_db() self.assertEqual(custom_suborg_request.requested_suborganization, "requested org name") self.assertEqual(custom_suborg_request.suborganization_city, "Austin") - self.assertEqual(custom_suborg_request.suborganization_state_territory, DomainRequest.StateTerritoryChoices.TEXAS) + self.assertEqual( + custom_suborg_request.suborganization_state_territory, DomainRequest.StateTerritoryChoices.TEXAS + ) def test_create_multiple_portfolios_for_branch_executive(self): """Tests creating all portfolios under a given branch""" From d06df851e6ff0253b2b6db15aa28cca2f3905e97 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 Jan 2025 09:11:35 -0700 Subject: [PATCH 20/36] Update test_management_scripts.py --- src/registrar/tests/test_management_scripts.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 4ca2a2d72..5c3ab4ba3 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -32,7 +32,7 @@ from unittest.mock import patch, call, MagicMock, mock_open from epplibwrapper import commands, common -from .common import MockEppLib, create_user, less_console_noise, completed_domain_request, MockSESClient +from .common import MockEppLib, less_console_noise, completed_domain_request, MockSESClient from api.tests.common import less_console_noise_decorator From 3a356c6055f8c6eef1fca306e6572ada0f90a195 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 Jan 2025 12:32:56 -0700 Subject: [PATCH 21/36] Normalize name and clear fed agency in some cases --- .../commands/create_federal_portfolio.py | 36 +++++++++++++--- src/registrar/models/domain_request.py | 19 +++++++- .../models/utility/generic_helper.py | 5 +++ .../tests/test_management_scripts.py | 43 +++++++++++++++++++ src/registrar/tests/test_models_requests.py | 32 ++++++++++++++ 5 files changed, 127 insertions(+), 8 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 42f4f5687..83bb3dd0f 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -5,6 +5,7 @@ from django.core.management import BaseCommand, CommandError from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper from registrar.models import DomainInformation, DomainRequest, FederalAgency, Suborganization, Portfolio, User +from registrar.models.utility.generic_helper import normalize_string logger = logging.getLogger(__name__) @@ -51,6 +52,11 @@ def add_arguments(self, parser): action=argparse.BooleanOptionalAction, help="Adds portfolio to both requests and domains", ) + parser.add_argument( + "--include_started_requests", + action=argparse.BooleanOptionalAction, + help="If parse_requests is enabled, we parse started", + ) def handle(self, **options): agency_name = options.get("agency_name") @@ -58,6 +64,7 @@ def handle(self, **options): parse_requests = options.get("parse_requests") parse_domains = options.get("parse_domains") both = options.get("both") + include_started_requests = options.get("include_started_requests") if not both: if not parse_requests and not parse_domains: @@ -66,6 +73,9 @@ def handle(self, **options): if parse_requests or parse_domains: raise CommandError("You cannot pass --parse_requests or --parse_domains when passing --both.") + if include_started_requests and not parse_requests: + raise CommandError("You must pass --parse_requests when using --include_started_requests") + 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: @@ -83,7 +93,9 @@ def handle(self, **options): TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) try: # C901 'Command.handle' is too complex (12) - self.handle_populate_portfolio(federal_agency, parse_domains, parse_requests, both) + self.handle_populate_portfolio( + federal_agency, parse_domains, parse_requests, both, include_started_requests + ) except Exception as exec: self.failed_portfolios.add(federal_agency) logger.error(exec) @@ -99,7 +111,7 @@ def handle(self, **options): display_as_str=True, ) - def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, both): + def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, both, include_started_requests): """Attempts to create a portfolio. If successful, this function will also create new suborganizations""" portfolio, created = self.create_portfolio(federal_agency) @@ -109,7 +121,7 @@ def handle_populate_portfolio(self, federal_agency, parse_domains, parse_request self.handle_portfolio_domains(portfolio, federal_agency) if parse_requests or both: - self.handle_portfolio_requests(portfolio, federal_agency) + self.handle_portfolio_requests(portfolio, federal_agency, include_started_requests) def create_portfolio(self, federal_agency): """Creates a portfolio if it doesn't presently exist. @@ -182,7 +194,7 @@ def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalA for name in org_names - set(existing_suborgs.values_list("name", flat=True)): # Stored in variables due to linter wanting type information here. portfolio_name: str = portfolio.organization_name if portfolio.organization_name is not None else "" - if name is not None and name.lower() == portfolio_name.lower(): + if name is not None and normalize_string(name) == normalize_string(portfolio_name): # You can use this to populate location information, when this occurs. # However, this isn't needed for now so we can skip it. message = ( @@ -191,7 +203,7 @@ def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalA ) TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) else: - new_suborgs.append(Suborganization(name=name, portfolio=portfolio)) # type: ignore + new_suborgs.append(Suborganization(name=normalize_string(name, lowercase=False), portfolio=portfolio)) # type: ignore if new_suborgs: Suborganization.objects.bulk_create(new_suborgs) @@ -201,16 +213,18 @@ def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalA 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, include_started_requests): """ Associate portfolio with domain requests for a federal agency. Updates all relevant domain request records. """ invalid_states = [ - DomainRequest.DomainRequestStatus.STARTED, DomainRequest.DomainRequestStatus.INELIGIBLE, DomainRequest.DomainRequestStatus.REJECTED, ] + if not include_started_requests: + invalid_states.append(DomainRequest.DomainRequestStatus.STARTED) + domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency, portfolio__isnull=True).exclude( status__in=invalid_states ) @@ -229,7 +243,14 @@ def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: Federa # Get all suborg information and store it in a dict to avoid doing a db call suborgs = Suborganization.objects.filter(portfolio=portfolio).in_bulk(field_name="name") for domain_request in domain_requests: + # Set the portfolio domain_request.portfolio = portfolio + + # Conditionally clear federal agency if the org name is the same as the portfolio name. + if include_started_requests: + domain_request.sync_portfolio_and_federal_agency_for_started_requests() + + # Set suborg info if domain_request.organization_name in suborgs: domain_request.sub_organization = suborgs.get(domain_request.organization_name) else: @@ -254,6 +275,7 @@ def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: Federa "requested_suborganization", "suborganization_city", "suborganization_state_territory", + "federal_agency", ], ) message = f"Added portfolio '{portfolio}' to {len(domain_requests)} domain requests." diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 3d3aac769..617c96737 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -8,7 +8,7 @@ from django.utils import timezone from registrar.models.domain import Domain from registrar.models.federal_agency import FederalAgency -from registrar.models.utility.generic_helper import CreateOrUpdateOrganizationTypeHelper +from registrar.models.utility.generic_helper import CreateOrUpdateOrganizationTypeHelper, normalize_string from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes from registrar.utility.constants import BranchChoices from auditlog.models import LogEntry @@ -729,6 +729,7 @@ def save(self, *args, **kwargs): """Save override for custom properties""" self.sync_organization_type() self.sync_yes_no_form_fields() + self.sync_portfolio_and_federal_agency_for_started_requests() if self._cached_status != self.status: self.last_status_update = timezone.now().date() @@ -744,6 +745,22 @@ def save(self, *args, **kwargs): # Update the cached values after saving self._cache_status_and_status_reasons() + def sync_portfolio_and_federal_agency_for_started_requests(self) -> bool: + """ + Prevents duplicate organization data by clearing the federal_agency field if it matches the portfolio. + Only runs on STARTED requests. + + Returns a bool indicating if the federal agency was changed or not. + """ + agency_name = getattr(self.federal_agency, "agency", None) + portfolio_name = getattr(self.portfolio, "organization_name", None) + if portfolio_name and agency_name and self.status == DomainRequest.DomainRequestStatus.STARTED: + if normalize_string(agency_name) == normalize_string(portfolio_name): + self.federal_agency = None + return True + + return False + def create_requested_suborganization(self): """Creates the requested suborganization. Adds the name, portfolio, city, and state_territory fields. diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 5e425f5a3..7aa4e62bc 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -343,3 +343,8 @@ def value_of_attribute(obj, attribute_name: str): if callable(value): value = value() return value + +def normalize_string(string_to_normalize: str, lowercase=True) -> str: + """Normalizes a given string. Returns a string without extra spaces, in all lowercase.""" + new_string = " ".join(string_to_normalize.split()) + return new_string.lower() if lowercase else new_string diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 5c3ab4ba3..418a22411 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1424,6 +1424,7 @@ def setUp(self): # Create an agency wih no federal type (can only be created via specifiying it manually) self.federal_agency = FederalAgency.objects.create(agency="Test Federal Agency") + self.federal_agency_2 = FederalAgency.objects.create(agency="Sugarcane") # And create some with federal_type ones with creative names self.executive_agency_1 = FederalAgency.objects.create( @@ -1516,6 +1517,48 @@ def run_create_federal_portfolio(self, **kwargs): ): call_command("create_federal_portfolio", **kwargs) + @less_console_noise_decorator + def test_handle_portfolio_requests_sync_federal_agency(self): + """Test that federal agency is cleared when org name matches portfolio name""" + + # Create a domain request with matching org name + matching_request = completed_domain_request( + name="matching.gov", + status=DomainRequest.DomainRequestStatus.STARTED, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=self.federal_agency_2, + user=self.user, + ) + + # Create a request not in started (no change should occur) + matching_request_in_wrong_status = completed_domain_request( + name="kinda-matching.gov", + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=self.federal_agency, + user=self.user, + ) + + self.run_create_federal_portfolio(agency_name="Sugarcane", parse_requests=True, include_started_requests=True) + self.run_create_federal_portfolio( + agency_name="Test Federal Agency", parse_requests=True, include_started_requests=True + ) + + # Refresh from db + matching_request.refresh_from_db() + matching_request_in_wrong_status.refresh_from_db() + + # Request with matching name should have federal_agency cleared + self.assertIsNone(matching_request.federal_agency) + self.assertIsNotNone(matching_request.portfolio) + self.assertEqual(matching_request.portfolio.organization_name, "Sugarcane") + + # Request with matching name but wrong state should keep its federal agency + self.assertEqual(matching_request_in_wrong_status.federal_agency, self.federal_agency) + self.assertIsNotNone(matching_request_in_wrong_status.portfolio) + self.assertEqual(matching_request_in_wrong_status.portfolio.organization_name, "Test Federal Agency") + + @less_console_noise_decorator def test_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) diff --git a/src/registrar/tests/test_models_requests.py b/src/registrar/tests/test_models_requests.py index 983a12b3c..0c674d417 100644 --- a/src/registrar/tests/test_models_requests.py +++ b/src/registrar/tests/test_models_requests.py @@ -1074,6 +1074,38 @@ def test_converted_type(self): self.assertEqual(domain_request2.generic_org_type, domain_request2.converted_generic_org_type) self.assertEqual(domain_request2.federal_agency, domain_request2.converted_federal_agency) + def test_sync_portfolio_and_federal_agency_for_started_requests(self): + """Tests the sync_portfolio_and_federal_agency_for_started_requests function""" + # Create a federal agency with a "bad" name + user = create_user() + federal_agency_2 = FederalAgency.objects.create(agency="Sugarcane ") + request = completed_domain_request( + name="matching.gov", + status=DomainRequest.DomainRequestStatus.STARTED, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=federal_agency_2, + user=user, + ) + self.assertIsNone(request.portfolio) + + # Nothing should happen on normal save + request.notes = "test change" + request.save() + self.assertEqual(request.federal_agency, federal_agency_2) + + # But when a portfolio exists, the federal agency should be cleared if its a duplicate + portfolio = Portfolio.objects.create(organization_name="sugarcane", creator=user) + request.portfolio = portfolio + request.save() + self.assertIsNone(request.federal_agency) + + # However -- this change should only occur if the names match (when normalized) + portfolio = Portfolio.objects.create(organization_name="some other name", creator=user) + request.portfolio = portfolio + request.federal_agency = federal_agency_2 + request.save() + self.assertEqual(request.federal_agency, federal_agency_2) + class TestDomainRequestSuborganization(TestCase): """Tests for the suborganization fields on domain requests""" From 2d163c1c1cac8f545b40c63c77760663fe40399a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 Jan 2025 12:41:52 -0700 Subject: [PATCH 22/36] Lint --- .../commands/create_federal_portfolio.py | 17 ++++++++++------- src/registrar/models/utility/generic_helper.py | 1 + src/registrar/tests/test_management_scripts.py | 5 +++++ 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 83bb3dd0f..1674d9057 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -203,7 +203,9 @@ def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalA ) TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) else: - new_suborgs.append(Suborganization(name=normalize_string(name, lowercase=False), portfolio=portfolio)) # type: ignore + new_suborgs.append( + Suborganization(name=normalize_string(name, lowercase=False), portfolio=portfolio) + ) # type: ignore if new_suborgs: Suborganization.objects.bulk_create(new_suborgs) @@ -246,25 +248,26 @@ def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: Federa # Set the portfolio domain_request.portfolio = portfolio - # Conditionally clear federal agency if the org name is the same as the portfolio name. - if include_started_requests: - domain_request.sync_portfolio_and_federal_agency_for_started_requests() - # Set suborg info if domain_request.organization_name in suborgs: domain_request.sub_organization = suborgs.get(domain_request.organization_name) else: clean_organization_name = None if isinstance(domain_request.organization_name, str): - clean_organization_name = domain_request.organization_name.strip() + clean_organization_name = normalize_string(domain_request.organization_name, lowercase=False) clean_city = None if isinstance(domain_request.city, str): - clean_city = domain_request.city.strip() + clean_city = normalize_string(domain_request.city, lowercase=False) domain_request.requested_suborganization = clean_organization_name domain_request.suborganization_city = clean_city domain_request.suborganization_state_territory = domain_request.state_territory + + # Conditionally clear federal agency if the org name is the same as the portfolio name. + if include_started_requests: + domain_request.sync_portfolio_and_federal_agency_for_started_requests() + self.updated_portfolios.add(portfolio) DomainRequest.objects.bulk_update( diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 7aa4e62bc..7d384d577 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -344,6 +344,7 @@ def value_of_attribute(obj, attribute_name: str): value = value() return value + def normalize_string(string_to_normalize: str, lowercase=True) -> str: """Normalizes a given string. Returns a string without extra spaces, in all lowercase.""" new_string = " ".join(string_to_normalize.split()) diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 418a22411..1c6cdd123 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1770,6 +1770,11 @@ def test_command_error_parse_options(self): ): self.run_create_federal_portfolio(agency_name="test") + with self.assertRaisesRegex( + CommandError, "You must pass --parse_requests when using --include_started_requests" + ): + self.run_create_federal_portfolio(agency_name="test", include_started_requests=True) + def test_command_error_agency_not_found(self): """Check error handling for non-existent agency.""" expected_message = ( From 2f5e0ec873f087e59a0329604a1b0f356f4bfb47 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 Jan 2025 13:01:15 -0700 Subject: [PATCH 23/36] Code cleanup --- .../management/commands/create_federal_portfolio.py | 11 ++++------- src/registrar/tests/test_management_scripts.py | 2 +- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 1674d9057..5ad350d02 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -184,7 +184,7 @@ def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalA return # Check for existing suborgs on the current portfolio - existing_suborgs = Suborganization.objects.filter(name__in=org_names) + existing_suborgs = Suborganization.objects.filter(name__in=org_names, name__isnull=False) if existing_suborgs.exists(): message = f"Some suborganizations already exist for portfolio '{portfolio}'." TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, message) @@ -192,9 +192,7 @@ def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalA # Create new suborgs, as long as they don't exist in the db already new_suborgs = [] for name in org_names - set(existing_suborgs.values_list("name", flat=True)): - # Stored in variables due to linter wanting type information here. - portfolio_name: str = portfolio.organization_name if portfolio.organization_name is not None else "" - if name is not None and normalize_string(name) == normalize_string(portfolio_name): + if normalize_string(name) == normalize_string(portfolio.organization_name): # You can use this to populate location information, when this occurs. # However, this isn't needed for now so we can skip it. message = ( @@ -249,9 +247,8 @@ def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: Federa domain_request.portfolio = portfolio # Set suborg info - if domain_request.organization_name in suborgs: - domain_request.sub_organization = suborgs.get(domain_request.organization_name) - else: + domain_request.sub_organization = suborgs.get(domain_request.organization_name, None) + if domain_request.sub_organization is None: clean_organization_name = None if isinstance(domain_request.organization_name, str): clean_organization_name = normalize_string(domain_request.organization_name, lowercase=False) diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 1c6cdd123..13746b624 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1773,7 +1773,7 @@ def test_command_error_parse_options(self): with self.assertRaisesRegex( CommandError, "You must pass --parse_requests when using --include_started_requests" ): - self.run_create_federal_portfolio(agency_name="test", include_started_requests=True) + self.run_create_federal_portfolio(agency_name="test", branch="executive", include_started_requests=True) def test_command_error_agency_not_found(self): """Check error handling for non-existent agency.""" From 9e49ca4eacc94fd77d6b689dcca693994034d5ca Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 Jan 2025 14:57:26 -0700 Subject: [PATCH 24/36] Simplify logic --- .../commands/create_federal_portfolio.py | 70 ++++++++++++------- src/registrar/models/domain_request.py | 17 ----- .../tests/test_management_scripts.py | 16 ++--- src/registrar/tests/test_models_requests.py | 32 --------- 4 files changed, 54 insertions(+), 81 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 5ad350d02..c3d3bdd42 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -52,11 +52,6 @@ def add_arguments(self, parser): action=argparse.BooleanOptionalAction, help="Adds portfolio to both requests and domains", ) - parser.add_argument( - "--include_started_requests", - action=argparse.BooleanOptionalAction, - help="If parse_requests is enabled, we parse started", - ) def handle(self, **options): agency_name = options.get("agency_name") @@ -64,7 +59,6 @@ def handle(self, **options): parse_requests = options.get("parse_requests") parse_domains = options.get("parse_domains") both = options.get("both") - include_started_requests = options.get("include_started_requests") if not both: if not parse_requests and not parse_domains: @@ -73,9 +67,6 @@ def handle(self, **options): if parse_requests or parse_domains: raise CommandError("You cannot pass --parse_requests or --parse_domains when passing --both.") - if include_started_requests and not parse_requests: - raise CommandError("You must pass --parse_requests when using --include_started_requests") - 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: @@ -88,14 +79,15 @@ def handle(self, **options): else: raise CommandError(f"Cannot find '{branch}' federal agencies in our database.") + portfolio_set = set() for federal_agency in agencies: message = f"Processing federal agency '{federal_agency.agency}'..." TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) try: # C901 'Command.handle' is too complex (12) - self.handle_populate_portfolio( - federal_agency, parse_domains, parse_requests, both, include_started_requests - ) + # We currently only grab the list of changed domain requests, but we may want to grab the domains too + portfolio = self.handle_populate_portfolio(federal_agency, parse_domains, parse_requests, both) + portfolio_set.add(portfolio) except Exception as exec: self.failed_portfolios.add(federal_agency) logger.error(exec) @@ -111,9 +103,39 @@ def handle(self, **options): display_as_str=True, ) - def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, both, include_started_requests): + # POST PROCESSING STEP: Remove the federal agency if it matches the portfolio name. + # We only do this for started domain requests. + if parse_requests: + message = "Removing duplicate portfolio and federal_agency values from domain requests..." + TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) + + domain_requests_to_update = DomainRequest.objects.filter( + portfolio__in=portfolio_set, + status=DomainRequest.DomainRequestStatus.STARTED, + federal_agency__agency__isnull=False, + portfolio__organization_name__isnull=False, + ) + updated_requests = [] + for req in domain_requests_to_update: + if normalize_string(req.federal_agency.agency) == normalize_string(req.portfolio.organization_name): + req.federal_agency = None + updated_requests.append(req) + DomainRequest.objects.bulk_update(updated_requests, ["federal_agency"]) + + # Log the results + if TerminalHelper.prompt_for_execution( + system_exit_on_terminate=False, + prompt_message=f"Updated {updated_requests} domain requests successfully.", + prompt_title="Do you want to see a list of all changed domain requests?", + ): + logger.info(f"Federal agency set to none on: {[str(request) for request in updated_requests]}") + + 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""" + also create new suborganizations. + + Returns the processed portfolio + """ portfolio, created = self.create_portfolio(federal_agency) if created: self.create_suborganizations(portfolio, federal_agency) @@ -121,7 +143,9 @@ def handle_populate_portfolio(self, federal_agency, parse_domains, parse_request self.handle_portfolio_domains(portfolio, federal_agency) if parse_requests or both: - self.handle_portfolio_requests(portfolio, federal_agency, include_started_requests) + self.handle_portfolio_requests(portfolio, federal_agency) + + return portfolio def create_portfolio(self, federal_agency): """Creates a portfolio if it doesn't presently exist. @@ -213,17 +237,19 @@ def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalA else: TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, "No suborganizations added") - def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency, include_started_requests): + 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. + Returns a list of updated records. + + Returns a queryset of DomainRequest objects, or None if nothing changed. """ invalid_states = [ + DomainRequest.DomainRequestStatus.STARTED, DomainRequest.DomainRequestStatus.INELIGIBLE, DomainRequest.DomainRequestStatus.REJECTED, ] - if not include_started_requests: - invalid_states.append(DomainRequest.DomainRequestStatus.STARTED) domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency, portfolio__isnull=True).exclude( status__in=invalid_states @@ -260,11 +286,6 @@ def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: Federa domain_request.requested_suborganization = clean_organization_name domain_request.suborganization_city = clean_city domain_request.suborganization_state_territory = domain_request.state_territory - - # Conditionally clear federal agency if the org name is the same as the portfolio name. - if include_started_requests: - domain_request.sync_portfolio_and_federal_agency_for_started_requests() - self.updated_portfolios.add(portfolio) DomainRequest.objects.bulk_update( @@ -275,7 +296,6 @@ def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: Federa "requested_suborganization", "suborganization_city", "suborganization_state_territory", - "federal_agency", ], ) message = f"Added portfolio '{portfolio}' to {len(domain_requests)} domain requests." @@ -285,6 +305,8 @@ def handle_portfolio_domains(self, portfolio: Portfolio, federal_agency: Federal """ 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, portfolio__isnull=True) if not domain_infos.exists(): diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 617c96737..d1a2597f4 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -729,7 +729,6 @@ def save(self, *args, **kwargs): """Save override for custom properties""" self.sync_organization_type() self.sync_yes_no_form_fields() - self.sync_portfolio_and_federal_agency_for_started_requests() if self._cached_status != self.status: self.last_status_update = timezone.now().date() @@ -745,22 +744,6 @@ def save(self, *args, **kwargs): # Update the cached values after saving self._cache_status_and_status_reasons() - def sync_portfolio_and_federal_agency_for_started_requests(self) -> bool: - """ - Prevents duplicate organization data by clearing the federal_agency field if it matches the portfolio. - Only runs on STARTED requests. - - Returns a bool indicating if the federal agency was changed or not. - """ - agency_name = getattr(self.federal_agency, "agency", None) - portfolio_name = getattr(self.portfolio, "organization_name", None) - if portfolio_name and agency_name and self.status == DomainRequest.DomainRequestStatus.STARTED: - if normalize_string(agency_name) == normalize_string(portfolio_name): - self.federal_agency = None - return True - - return False - def create_requested_suborganization(self): """Creates the requested suborganization. Adds the name, portfolio, city, and state_territory fields. diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 13746b624..06ecf7a5f 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1520,7 +1520,11 @@ def run_create_federal_portfolio(self, **kwargs): @less_console_noise_decorator def test_handle_portfolio_requests_sync_federal_agency(self): """Test that federal agency is cleared when org name matches portfolio name""" - + # Create a portfolio. This script skips over "started" + portfolio = Portfolio.objects.create( + organization_name="Sugarcane", + creator=self.user + ) # Create a domain request with matching org name matching_request = completed_domain_request( name="matching.gov", @@ -1528,6 +1532,7 @@ def test_handle_portfolio_requests_sync_federal_agency(self): generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, federal_agency=self.federal_agency_2, user=self.user, + portfolio=portfolio ) # Create a request not in started (no change should occur) @@ -1539,9 +1544,9 @@ def test_handle_portfolio_requests_sync_federal_agency(self): user=self.user, ) - self.run_create_federal_portfolio(agency_name="Sugarcane", parse_requests=True, include_started_requests=True) + self.run_create_federal_portfolio(agency_name="Sugarcane", parse_requests=True) self.run_create_federal_portfolio( - agency_name="Test Federal Agency", parse_requests=True, include_started_requests=True + agency_name="Test Federal Agency", parse_requests=True ) # Refresh from db @@ -1770,11 +1775,6 @@ def test_command_error_parse_options(self): ): self.run_create_federal_portfolio(agency_name="test") - with self.assertRaisesRegex( - CommandError, "You must pass --parse_requests when using --include_started_requests" - ): - self.run_create_federal_portfolio(agency_name="test", branch="executive", include_started_requests=True) - def test_command_error_agency_not_found(self): """Check error handling for non-existent agency.""" expected_message = ( diff --git a/src/registrar/tests/test_models_requests.py b/src/registrar/tests/test_models_requests.py index 0c674d417..983a12b3c 100644 --- a/src/registrar/tests/test_models_requests.py +++ b/src/registrar/tests/test_models_requests.py @@ -1074,38 +1074,6 @@ def test_converted_type(self): self.assertEqual(domain_request2.generic_org_type, domain_request2.converted_generic_org_type) self.assertEqual(domain_request2.federal_agency, domain_request2.converted_federal_agency) - def test_sync_portfolio_and_federal_agency_for_started_requests(self): - """Tests the sync_portfolio_and_federal_agency_for_started_requests function""" - # Create a federal agency with a "bad" name - user = create_user() - federal_agency_2 = FederalAgency.objects.create(agency="Sugarcane ") - request = completed_domain_request( - name="matching.gov", - status=DomainRequest.DomainRequestStatus.STARTED, - generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, - federal_agency=federal_agency_2, - user=user, - ) - self.assertIsNone(request.portfolio) - - # Nothing should happen on normal save - request.notes = "test change" - request.save() - self.assertEqual(request.federal_agency, federal_agency_2) - - # But when a portfolio exists, the federal agency should be cleared if its a duplicate - portfolio = Portfolio.objects.create(organization_name="sugarcane", creator=user) - request.portfolio = portfolio - request.save() - self.assertIsNone(request.federal_agency) - - # However -- this change should only occur if the names match (when normalized) - portfolio = Portfolio.objects.create(organization_name="some other name", creator=user) - request.portfolio = portfolio - request.federal_agency = federal_agency_2 - request.save() - self.assertEqual(request.federal_agency, federal_agency_2) - class TestDomainRequestSuborganization(TestCase): """Tests for the suborganization fields on domain requests""" From 8f3f7f14f2916511bd6ce531f855ef4153e0d858 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 Jan 2025 15:12:25 -0700 Subject: [PATCH 25/36] Cleanup part 2 --- .../commands/create_federal_portfolio.py | 18 ++++++------------ src/registrar/models/domain_request.py | 2 +- src/registrar/models/utility/generic_helper.py | 4 ++++ src/registrar/tests/test_management_scripts.py | 11 +++-------- 4 files changed, 14 insertions(+), 21 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index c3d3bdd42..1bf4a3fca 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -275,17 +275,12 @@ def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: Federa # Set suborg info domain_request.sub_organization = suborgs.get(domain_request.organization_name, None) if domain_request.sub_organization is None: - clean_organization_name = None - if isinstance(domain_request.organization_name, str): - clean_organization_name = normalize_string(domain_request.organization_name, lowercase=False) - - clean_city = None - if isinstance(domain_request.city, str): - clean_city = normalize_string(domain_request.city, lowercase=False) - - domain_request.requested_suborganization = clean_organization_name - domain_request.suborganization_city = clean_city + domain_request.requested_suborganization = normalize_string( + domain_request.organization_name, lowercase=False + ) + domain_request.suborganization_city = normalize_string(domain_request.city, lowercase=False) domain_request.suborganization_state_territory = domain_request.state_territory + self.updated_portfolios.add(portfolio) DomainRequest.objects.bulk_update( @@ -322,8 +317,7 @@ def handle_portfolio_domains(self, portfolio: Portfolio, federal_agency: Federal suborgs = Suborganization.objects.filter(portfolio=portfolio).in_bulk(field_name="name") for domain_info in domain_infos: domain_info.portfolio = portfolio - if domain_info.organization_name in suborgs: - domain_info.sub_organization = suborgs.get(domain_info.organization_name) + domain_info.sub_organization = suborgs.get(domain_info.organization_name, None) DomainInformation.objects.bulk_update(domain_infos, ["portfolio", "sub_organization"]) message = f"Added portfolio '{portfolio}' to {len(domain_infos)} domains." diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index d1a2597f4..3d3aac769 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -8,7 +8,7 @@ from django.utils import timezone from registrar.models.domain import Domain from registrar.models.federal_agency import FederalAgency -from registrar.models.utility.generic_helper import CreateOrUpdateOrganizationTypeHelper, normalize_string +from registrar.models.utility.generic_helper import CreateOrUpdateOrganizationTypeHelper from registrar.utility.errors import FSMDomainRequestError, FSMErrorCodes from registrar.utility.constants import BranchChoices from auditlog.models import LogEntry diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 7d384d577..3a8da508e 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -347,5 +347,9 @@ def value_of_attribute(obj, attribute_name: str): def normalize_string(string_to_normalize: str, lowercase=True) -> str: """Normalizes a given string. Returns a string without extra spaces, in all lowercase.""" + if not isinstance(string_to_normalize, str): + logger.error(f"normalize_string => {string_to_normalize} is not type str.") + return string_to_normalize + new_string = " ".join(string_to_normalize.split()) return new_string.lower() if lowercase else new_string diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 06ecf7a5f..882396f1e 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1521,10 +1521,7 @@ def run_create_federal_portfolio(self, **kwargs): def test_handle_portfolio_requests_sync_federal_agency(self): """Test that federal agency is cleared when org name matches portfolio name""" # Create a portfolio. This script skips over "started" - portfolio = Portfolio.objects.create( - organization_name="Sugarcane", - creator=self.user - ) + portfolio = Portfolio.objects.create(organization_name="Sugarcane", creator=self.user) # Create a domain request with matching org name matching_request = completed_domain_request( name="matching.gov", @@ -1532,7 +1529,7 @@ def test_handle_portfolio_requests_sync_federal_agency(self): generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, federal_agency=self.federal_agency_2, user=self.user, - portfolio=portfolio + portfolio=portfolio, ) # Create a request not in started (no change should occur) @@ -1545,9 +1542,7 @@ def test_handle_portfolio_requests_sync_federal_agency(self): ) self.run_create_federal_portfolio(agency_name="Sugarcane", parse_requests=True) - self.run_create_federal_portfolio( - agency_name="Test Federal Agency", parse_requests=True - ) + self.run_create_federal_portfolio(agency_name="Test Federal Agency", parse_requests=True) # Refresh from db matching_request.refresh_from_db() From d9cad13f953b581d525c7f2204ec9245ee3a0f2d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 7 Jan 2025 15:55:27 -0700 Subject: [PATCH 26/36] Update create_federal_portfolio.py --- .../commands/create_federal_portfolio.py | 51 +++++++++++-------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 1bf4a3fca..8ff32824b 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -106,29 +106,36 @@ def handle(self, **options): # POST PROCESSING STEP: Remove the federal agency if it matches the portfolio name. # We only do this for started domain requests. if parse_requests: - message = "Removing duplicate portfolio and federal_agency values from domain requests..." - TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) + self.post_process_started_domain_requests(portfolio_set) - domain_requests_to_update = DomainRequest.objects.filter( - portfolio__in=portfolio_set, - status=DomainRequest.DomainRequestStatus.STARTED, - federal_agency__agency__isnull=False, - portfolio__organization_name__isnull=False, - ) - updated_requests = [] - for req in domain_requests_to_update: - if normalize_string(req.federal_agency.agency) == normalize_string(req.portfolio.organization_name): - req.federal_agency = None - updated_requests.append(req) - DomainRequest.objects.bulk_update(updated_requests, ["federal_agency"]) - - # Log the results - if TerminalHelper.prompt_for_execution( - system_exit_on_terminate=False, - prompt_message=f"Updated {updated_requests} domain requests successfully.", - prompt_title="Do you want to see a list of all changed domain requests?", - ): - logger.info(f"Federal agency set to none on: {[str(request) for request in updated_requests]}") + def post_process_started_domain_requests(self, portfolio_set): + """ + Removes duplicate organization data by clearing federal_agency when it matches the portfolio name. + Only processes domain requests in STARTED status. + """ + message = "Removing duplicate portfolio and federal_agency values from domain requests..." + TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) + + domain_requests_to_update = DomainRequest.objects.filter( + portfolio__in=portfolio_set, + status=DomainRequest.DomainRequestStatus.STARTED, + federal_agency__agency__isnull=False, + portfolio__organization_name__isnull=False, + ) + updated_requests = [] + for req in domain_requests_to_update: + if normalize_string(req.federal_agency.agency) == normalize_string(req.portfolio.organization_name): + req.federal_agency = None + updated_requests.append(req) + DomainRequest.objects.bulk_update(updated_requests, ["federal_agency"]) + + # Log the results + if TerminalHelper.prompt_for_execution( + system_exit_on_terminate=False, + prompt_message=f"Updated {len(updated_requests)} domain requests successfully.", + prompt_title="Do you want to see a list of all changed domain requests?", + ): + logger.info(f"Federal agency set to none on: {[str(request) for request in updated_requests]}") def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, both): """Attempts to create a portfolio. If successful, this function will From 5ca74fdc5374b864db3aa6e32b6d2c5ebed3882c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 Jan 2025 10:38:50 -0700 Subject: [PATCH 27/36] Finish unit tests --- .../commands/create_federal_portfolio.py | 32 ++++++++---- .../tests/test_management_scripts.py | 50 +++++++++++++++++-- 2 files changed, 68 insertions(+), 14 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 8ff32824b..ddcb5a4a9 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -79,7 +79,7 @@ def handle(self, **options): else: raise CommandError(f"Cannot find '{branch}' federal agencies in our database.") - portfolio_set = set() + portfolios = [] for federal_agency in agencies: message = f"Processing federal agency '{federal_agency.agency}'..." TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) @@ -87,7 +87,7 @@ def handle(self, **options): # C901 'Command.handle' is too complex (12) # We currently only grab the list of changed domain requests, but we may want to grab the domains too portfolio = self.handle_populate_portfolio(federal_agency, parse_domains, parse_requests, both) - portfolio_set.add(portfolio) + portfolios.append(portfolio) except Exception as exec: self.failed_portfolios.add(federal_agency) logger.error(exec) @@ -106,9 +106,14 @@ def handle(self, **options): # POST PROCESSING STEP: Remove the federal agency if it matches the portfolio name. # We only do this for started domain requests. if parse_requests: - self.post_process_started_domain_requests(portfolio_set) + TerminalHelper.prompt_for_execution( + system_exit_on_terminate=True, + prompt_message="This action will update domain requests even if they aren't on a portfolio.", + prompt_title="Do you want to clear federal agency on started domain requests?", + ) + self.post_process_started_domain_requests(agencies, portfolios) - def post_process_started_domain_requests(self, portfolio_set): + def post_process_started_domain_requests(self, agencies, portfolios): """ Removes duplicate organization data by clearing federal_agency when it matches the portfolio name. Only processes domain requests in STARTED status. @@ -116,15 +121,24 @@ def post_process_started_domain_requests(self, portfolio_set): message = "Removing duplicate portfolio and federal_agency values from domain requests..." TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) + # For each request, clear the federal agency under these conditions: + # 1. A portfolio *already exists* with the same name as the federal agency. + # 2. Said portfolio (or portfolios) are only the ones specified at the start of the script. + # 3. The domain request is in status "started". + # Note: Both names are normalized so excess spaces are stripped and the string is lowercased. domain_requests_to_update = DomainRequest.objects.filter( - portfolio__in=portfolio_set, - status=DomainRequest.DomainRequestStatus.STARTED, + federal_agency__in=agencies, federal_agency__agency__isnull=False, - portfolio__organization_name__isnull=False, + status=DomainRequest.DomainRequestStatus.STARTED, + organization_name__isnull=False, ) + portfolio_set = {normalize_string(portfolio.organization_name) for portfolio in portfolios if portfolio} + + # Update the request, assuming the given agency name matches the portfolio name updated_requests = [] for req in domain_requests_to_update: - if normalize_string(req.federal_agency.agency) == normalize_string(req.portfolio.organization_name): + agency_name = normalize_string(req.federal_agency.agency) + if agency_name in portfolio_set: req.federal_agency = None updated_requests.append(req) DomainRequest.objects.bulk_update(updated_requests, ["federal_agency"]) @@ -140,8 +154,6 @@ def post_process_started_domain_requests(self, portfolio_set): def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, both): """Attempts to create a portfolio. If successful, this function will also create new suborganizations. - - Returns the processed portfolio """ portfolio, created = self.create_portfolio(federal_agency) if created: diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 882396f1e..8ecb7cbea 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1424,7 +1424,6 @@ def setUp(self): # Create an agency wih no federal type (can only be created via specifiying it manually) self.federal_agency = FederalAgency.objects.create(agency="Test Federal Agency") - self.federal_agency_2 = FederalAgency.objects.create(agency="Sugarcane") # And create some with federal_type ones with creative names self.executive_agency_1 = FederalAgency.objects.create( @@ -1518,8 +1517,13 @@ def run_create_federal_portfolio(self, **kwargs): call_command("create_federal_portfolio", **kwargs) @less_console_noise_decorator - def test_handle_portfolio_requests_sync_federal_agency(self): - """Test that federal agency is cleared when org name matches portfolio name""" + def test_post_process_started_domain_requests_existing_portfolio(self): + """Ensures that federal agency is cleared when agency name matches portfolio name. + As the name implies, this implicitly tests the "post_process_started_domain_requests" function. + """ + federal_agency_2 = FederalAgency.objects.create(agency="Sugarcane", federal_type=BranchChoices.EXECUTIVE) + + # Test records with portfolios and no org names # Create a portfolio. This script skips over "started" portfolio = Portfolio.objects.create(organization_name="Sugarcane", creator=self.user) # Create a domain request with matching org name @@ -1527,7 +1531,7 @@ def test_handle_portfolio_requests_sync_federal_agency(self): name="matching.gov", status=DomainRequest.DomainRequestStatus.STARTED, generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, - federal_agency=self.federal_agency_2, + federal_agency=federal_agency_2, user=self.user, portfolio=portfolio, ) @@ -1558,6 +1562,44 @@ def test_handle_portfolio_requests_sync_federal_agency(self): self.assertIsNotNone(matching_request_in_wrong_status.portfolio) self.assertEqual(matching_request_in_wrong_status.portfolio.organization_name, "Test Federal Agency") + @less_console_noise_decorator + def test_post_process_started_domain_requests(self): + """Tests that federal agency is cleared when agency name + matches an existing portfolio's name, even if the domain request isn't + directly on that portfolio.""" + + federal_agency_2 = FederalAgency.objects.create(agency="Sugarcane", federal_type=BranchChoices.EXECUTIVE) + + # Create a request with matching federal_agency name but no direct portfolio association + matching_agency_request = completed_domain_request( + name="agency-match.gov", + status=DomainRequest.DomainRequestStatus.STARTED, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=federal_agency_2, + user=self.user, + ) + + # Create a control request that shouldn't match + non_matching_request = completed_domain_request( + name="no-match.gov", + status=DomainRequest.DomainRequestStatus.STARTED, + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + federal_agency=self.federal_agency, + user=self.user, + ) + + # We expect the matching agency to have its fed agency cleared. + self.run_create_federal_portfolio(agency_name="Sugarcane", parse_requests=True) + matching_agency_request.refresh_from_db() + non_matching_request.refresh_from_db() + + # Request with matching agency name should have federal_agency cleared + self.assertIsNone(matching_agency_request.federal_agency) + + # Non-matching request should keep its federal_agency + self.assertIsNotNone(non_matching_request.federal_agency) + self.assertEqual(non_matching_request.federal_agency, self.federal_agency) + @less_console_noise_decorator def test_create_single_portfolio(self): """Test portfolio creation with suborg and senior official.""" From bc92270897cfc8967894e874f3cdddb39c8e78c6 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 Jan 2025 10:49:04 -0700 Subject: [PATCH 28/36] Update create_federal_portfolio.py --- .../commands/create_federal_portfolio.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index ddcb5a4a9..72548aa08 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -85,7 +85,6 @@ def handle(self, **options): TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) try: # C901 'Command.handle' is too complex (12) - # We currently only grab the list of changed domain requests, but we may want to grab the domains too portfolio = self.handle_populate_portfolio(federal_agency, parse_domains, parse_requests, both) portfolios.append(portfolio) except Exception as exec: @@ -105,11 +104,12 @@ def handle(self, **options): # POST PROCESSING STEP: Remove the federal agency if it matches the portfolio name. # We only do this for started domain requests. - if parse_requests: + if parse_requests or both: TerminalHelper.prompt_for_execution( system_exit_on_terminate=True, prompt_message="This action will update domain requests even if they aren't on a portfolio.", - prompt_title="Do you want to clear federal agency on started domain requests?", + prompt_title="Do you want to clear federal agency on (related) started domain requests?", + verify_message=None ) self.post_process_started_domain_requests(agencies, portfolios) @@ -141,15 +141,18 @@ def post_process_started_domain_requests(self, agencies, portfolios): if agency_name in portfolio_set: req.federal_agency = None updated_requests.append(req) - DomainRequest.objects.bulk_update(updated_requests, ["federal_agency"]) - # Log the results + # Execute the update and Log the results if TerminalHelper.prompt_for_execution( system_exit_on_terminate=False, - prompt_message=f"Updated {len(updated_requests)} domain requests successfully.", - prompt_title="Do you want to see a list of all changed domain requests?", + prompt_message=( + f"{len(domain_requests_to_update)} domain requests will be updated. " + f"These records will be changed: {[str(req) for req in updated_requests]}" + ), + prompt_title="Do wish to commit this update to the database?", ): - logger.info(f"Federal agency set to none on: {[str(request) for request in updated_requests]}") + 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): """Attempts to create a portfolio. If successful, this function will From b9dc92808440a5b124e6d146193302156920ee8c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 Jan 2025 10:56:06 -0700 Subject: [PATCH 29/36] lint --- src/registrar/management/commands/create_federal_portfolio.py | 4 ++-- src/registrar/models/utility/generic_helper.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 72548aa08..d0fb14a9f 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -109,7 +109,7 @@ def handle(self, **options): system_exit_on_terminate=True, prompt_message="This action will update domain requests even if they aren't on a portfolio.", prompt_title="Do you want to clear federal agency on (related) started domain requests?", - verify_message=None + verify_message=None, ) self.post_process_started_domain_requests(agencies, portfolios) @@ -248,7 +248,7 @@ def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalA TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) else: new_suborgs.append( - Suborganization(name=normalize_string(name, lowercase=False), portfolio=portfolio) + Suborganization(name=name, portfolio=portfolio) ) # type: ignore if new_suborgs: diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 3a8da508e..e8992acc2 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -345,7 +345,7 @@ def value_of_attribute(obj, attribute_name: str): return value -def normalize_string(string_to_normalize: str, lowercase=True) -> str: +def normalize_string(string_to_normalize, lowercase=True): """Normalizes a given string. Returns a string without extra spaces, in all lowercase.""" if not isinstance(string_to_normalize, str): logger.error(f"normalize_string => {string_to_normalize} is not type str.") From 6da172c0285ad0c568216311ecbe397b9103a81f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 8 Jan 2025 12:25:17 -0700 Subject: [PATCH 30/36] Update create_federal_portfolio.py --- .../management/commands/create_federal_portfolio.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index d0fb14a9f..4f4002da4 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -108,7 +108,9 @@ def handle(self, **options): TerminalHelper.prompt_for_execution( system_exit_on_terminate=True, prompt_message="This action will update domain requests even if they aren't on a portfolio.", - prompt_title="Do you want to clear federal agency on (related) started domain requests?", + prompt_title=( + "POST PROCESS STEP: Do you want to clear federal agency on (related) started domain requests?" + ), verify_message=None, ) self.post_process_started_domain_requests(agencies, portfolios) @@ -247,9 +249,7 @@ def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalA ) TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) else: - new_suborgs.append( - Suborganization(name=name, portfolio=portfolio) - ) # type: ignore + new_suborgs.append(Suborganization(name=name, portfolio=portfolio)) # type: ignore if new_suborgs: Suborganization.objects.bulk_create(new_suborgs) From 983fdddc9c741da6b37d872f9d16d7b44f895382 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 9 Jan 2025 08:18:53 -0700 Subject: [PATCH 31/36] Update src/registrar/management/commands/create_federal_portfolio.py --- src/registrar/management/commands/create_federal_portfolio.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 4f4002da4..c6316d33e 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -263,9 +263,6 @@ def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: Federa """ Associate portfolio with domain requests for a federal agency. Updates all relevant domain request records. - Returns a list of updated records. - - Returns a queryset of DomainRequest objects, or None if nothing changed. """ invalid_states = [ DomainRequest.DomainRequestStatus.STARTED, From 26c3e89b41f0c1455f632e4a049fd9960cbb689f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 9 Jan 2025 08:19:11 -0700 Subject: [PATCH 32/36] Update src/registrar/management/commands/create_federal_portfolio.py --- src/registrar/management/commands/create_federal_portfolio.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index c6316d33e..d96b0241d 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -269,7 +269,6 @@ def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: Federa DomainRequest.DomainRequestStatus.INELIGIBLE, DomainRequest.DomainRequestStatus.REJECTED, ] - domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency, portfolio__isnull=True).exclude( status__in=invalid_states ) From f6b4d177857415e12e680d0433889d307771d09b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 9 Jan 2025 08:20:17 -0700 Subject: [PATCH 33/36] Update src/registrar/management/commands/create_federal_portfolio.py --- src/registrar/management/commands/create_federal_portfolio.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index d96b0241d..dc21c6f64 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -159,6 +159,7 @@ def post_process_started_domain_requests(self, agencies, portfolios): def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, both): """Attempts to create a portfolio. If successful, this function will also create new suborganizations. + Returns the portfolio for the given federal_agency. """ portfolio, created = self.create_portfolio(federal_agency) if created: From 5308217d225b7750ea83305c99020314206e514d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 13 Jan 2025 10:08:08 -0700 Subject: [PATCH 34/36] Added better function comment for add_arugments --- .../commands/create_federal_portfolio.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index dc21c6f64..096580609 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -22,10 +22,21 @@ def __init__(self, *args, **kwargs): self.failed_portfolios = set() 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 + """Add command line arguments to create federal portfolios. + + Required (mutually exclusive) arguments: + --agency_name: Name of a specific FederalAgency to create a portfolio for + --branch: Federal branch to process ("executive", "legislative", or "judicial"). + Creates portfolios for all FederalAgencies in that branch. + + Required (at least one): + --parse_requests: Add the created portfolio(s) to related DomainRequest records + --parse_domains: Add the created portfolio(s) to related DomainInformation records + Note: You can use both --parse_requests and --parse_domains together + + Optional (mutually exclusive with parse options): + --both: Shorthand for using both --parse_requests and --parse_domains + Cannot be used with --parse_requests or --parse_domains """ group = parser.add_mutually_exclusive_group(required=True) group.add_argument( From f6e94da96868cf2d915b4a5e8d0f74052793773b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 13 Jan 2025 10:12:43 -0700 Subject: [PATCH 35/36] Update src/registrar/management/commands/create_federal_portfolio.py --- src/registrar/management/commands/create_federal_portfolio.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 096580609..a3c8d3e21 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -162,7 +162,7 @@ def post_process_started_domain_requests(self, agencies, portfolios): f"{len(domain_requests_to_update)} domain requests will be updated. " f"These records will be changed: {[str(req) for req in updated_requests]}" ), - prompt_title="Do wish to commit this update to the database?", + prompt_title="Do you wish to commit this update to the database?", ): DomainRequest.objects.bulk_update(updated_requests, ["federal_agency"]) TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, "Action completed successfully.") From 0f59a631d294dddbf11b44104c83f60662d43890 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 13 Jan 2025 10:30:23 -0700 Subject: [PATCH 36/36] Lint --- src/registrar/management/commands/create_federal_portfolio.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 096580609..5731154a2 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -26,12 +26,12 @@ def add_arguments(self, parser): Required (mutually exclusive) arguments: --agency_name: Name of a specific FederalAgency to create a portfolio for - --branch: Federal branch to process ("executive", "legislative", or "judicial"). + --branch: Federal branch to process ("executive", "legislative", or "judicial"). Creates portfolios for all FederalAgencies in that branch. Required (at least one): --parse_requests: Add the created portfolio(s) to related DomainRequest records - --parse_domains: Add the created portfolio(s) to related DomainInformation records + --parse_domains: Add the created portfolio(s) to related DomainInformation records Note: You can use both --parse_requests and --parse_domains together Optional (mutually exclusive with parse options):