From 590df23812ee4c83de403b6c51473fd662870b49 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 13 Mar 2025 10:52:22 -0600 Subject: [PATCH 01/27] Refactor log_script_run_summary --- .../commands/create_federal_portfolio.py | 58 +++++---- .../commands/populate_first_ready.py | 2 +- .../commands/populate_organization_type.py | 9 +- .../commands/utility/terminal_helper.py | 116 ++++++++++-------- 4 files changed, 100 insertions(+), 85 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index d753d0ce8..8ce8e624b 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -32,6 +32,14 @@ class Command(BaseCommand): self.added_invitations = set() self.skipped_invitations = set() self.failed_managers = set() + self.portfolio_changes = {"add": [], "update": [], "skip": [], "fail": []} + self.suborganization_changes = {"add": [], "update": [], "skip": [], "fail": []} + self.domain_info_changes = {"add": [], "update": [], "skip": [], "fail": []} + self.domain_request_changes = {"add": [], "update": [], "skip": [], "fail": []} + + self.user_domain_role_changes = {"add": [], "update": [], "skip": [], "fail": []} + + self.portfolio_invitation_changes = {"add": [], "update": [], "skip": [], "fail": []} def add_arguments(self, parser): """Add command line arguments to create federal portfolios. @@ -44,11 +52,6 @@ class Command(BaseCommand): 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 Optional: --add_managers: Add all domain managers of the portfolio's domains to the organization. @@ -73,11 +76,6 @@ class Command(BaseCommand): action=argparse.BooleanOptionalAction, help="Adds portfolio to DomainInformation", ) - parser.add_argument( - "--both", - action=argparse.BooleanOptionalAction, - help="Adds portfolio to both requests and domains", - ) parser.add_argument( "--add_managers", action=argparse.BooleanOptionalAction, @@ -94,19 +92,14 @@ class Command(BaseCommand): branch = options.get("branch") parse_requests = options.get("parse_requests") parse_domains = options.get("parse_domains") - both = options.get("both") add_managers = options.get("add_managers") skip_existing_portfolios = options.get("skip_existing_portfolios") - if not both: - if not (parse_requests or parse_domains or add_managers): - raise CommandError( - "You must specify at least one of --parse_requests, --parse_domains, or --add_managers." - ) - else: - if parse_requests or parse_domains: - raise CommandError("You cannot pass --parse_requests or --parse_domains when passing --both.") + # Parse script params + if not (parse_requests or parse_domains or add_managers): + raise CommandError("You must specify at least one of --parse_requests, --parse_domains, or --add_managers.") + # Get agencies 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: @@ -118,6 +111,8 @@ class Command(BaseCommand): ) else: raise CommandError(f"Cannot find '{branch}' federal agencies in our database.") + + # Parse portfolios portfolios = [] for federal_agency in agencies: message = f"Processing federal agency '{federal_agency.agency}'..." @@ -125,7 +120,7 @@ class Command(BaseCommand): try: # C901 'Command.handle' is too complex (12) portfolio = self.handle_populate_portfolio( - federal_agency, parse_domains, parse_requests, both, skip_existing_portfolios + federal_agency, parse_domains, parse_requests, skip_existing_portfolios ) portfolios.append(portfolio) if add_managers: @@ -141,9 +136,10 @@ class Command(BaseCommand): message = f"Added city and state_territory information to {updated_suborg_count} suborgs." TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) TerminalHelper.log_script_run_summary( - self.updated_portfolios, - self.failed_portfolios, - self.skipped_portfolios, + self.portfolio_changes.get("add"), + self.portfolio_changes.get("fail"), + self.portfolio_changes.get("skip"), + [], debug=False, log_header="============= FINISHED HANDLE PORTFOLIO STEP ===============", skipped_header="----- SOME PORTFOLIOS WERENT CREATED (BUT OTHER RECORDS ARE STILL PROCESSED) -----", @@ -152,9 +148,10 @@ class Command(BaseCommand): if add_managers: TerminalHelper.log_script_run_summary( - self.added_managers, - self.failed_managers, - [], # can't skip managers, can only add or fail + self.user_domain_role_changes.get("add"), + self.user_domain_role_changes.get("fail"), + self.user_domain_role_changes.get("skip"), + [], log_header="----- MANAGERS ADDED -----", debug=False, display_as_str=True, @@ -164,6 +161,7 @@ class Command(BaseCommand): self.added_invitations, [], self.skipped_invitations, + [], log_header="----- INVITATIONS ADDED -----", debug=False, skipped_header="----- INVITATIONS SKIPPED (ALREADY EXISTED) -----", @@ -172,7 +170,7 @@ class Command(BaseCommand): # POST PROCESSING STEP: Remove the federal agency if it matches the portfolio name. # We only do this for started domain requests. - if parse_requests or both: + if parse_requests: prompt_message = ( "This action will update domain requests even if they aren't on a portfolio." "\nNOTE: This will modify domain requests, even if no portfolios were created." @@ -304,7 +302,7 @@ class Command(BaseCommand): DomainRequest.objects.bulk_update(updated_requests, ["federal_agency"]) TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, "Action completed successfully.") - def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, both, skip_existing_portfolios): + def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, skip_existing_portfolios): """Attempts to create a portfolio. If successful, this function will also create new suborganizations""" portfolio, created = self.create_portfolio(federal_agency) @@ -318,10 +316,10 @@ class Command(BaseCommand): return portfolio self.create_suborganizations(portfolio, federal_agency) - if parse_domains or both: + if parse_domains: self.handle_portfolio_domains(portfolio, federal_agency) - if parse_requests or both: + if parse_requests: self.handle_portfolio_requests(portfolio, federal_agency) return portfolio diff --git a/src/registrar/management/commands/populate_first_ready.py b/src/registrar/management/commands/populate_first_ready.py index 04468029a..1cef2245d 100644 --- a/src/registrar/management/commands/populate_first_ready.py +++ b/src/registrar/management/commands/populate_first_ready.py @@ -51,7 +51,7 @@ class Command(BaseCommand): ScriptDataHelper.bulk_update_fields(Domain, self.to_update, ["first_ready"]) # Log what happened - TerminalHelper.log_script_run_summary(self.to_update, self.failed_to_update, self.skipped, debug) + TerminalHelper.log_script_run_summary(self.to_update, self.failed_to_update, self.skipped, [], debug=debug) def update_first_ready_for_domain(self, domain: Domain, debug: bool): """Grabs the created_at field and associates it with the first_ready column. diff --git a/src/registrar/management/commands/populate_organization_type.py b/src/registrar/management/commands/populate_organization_type.py index 60d179cb8..20cd6d130 100644 --- a/src/registrar/management/commands/populate_organization_type.py +++ b/src/registrar/management/commands/populate_organization_type.py @@ -144,7 +144,12 @@ class Command(BaseCommand): # Log what happened log_header = "============= FINISHED UPDATE FOR DOMAINREQUEST ===============" TerminalHelper.log_script_run_summary( - self.request_to_update, self.request_failed_to_update, self.request_skipped, True, log_header + self.request_to_update, + self.request_failed_to_update, + self.request_skipped, + [], + debug=True, + log_header=log_header, ) update_skipped_count = len(self.request_to_update) @@ -195,7 +200,7 @@ class Command(BaseCommand): # Log what happened log_header = "============= FINISHED UPDATE FOR DOMAININFORMATION ===============" TerminalHelper.log_script_run_summary( - self.di_to_update, self.di_failed_to_update, self.di_skipped, True, log_header + self.di_to_update, self.di_failed_to_update, self.di_skipped, [], debug=True, log_header=log_header ) update_skipped_count = len(self.di_skipped) diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index 87d9f12e5..f4319e587 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -161,6 +161,7 @@ class PopulateScriptTemplate(ABC): to_update, failed_to_update, to_skip, + [], debug=debug, log_header=self.run_summary_header, display_as_str=True, @@ -190,15 +191,31 @@ class PopulateScriptTemplate(ABC): class TerminalHelper: + @staticmethod def log_script_run_summary( - to_update, failed_to_update, skipped, debug: bool, log_header=None, skipped_header=None, display_as_str=False + to_update, + failed_to_update, + skipped, + to_add, + debug: bool, + log_header=None, + skipped_header=None, + failed_header=None, + display_as_str=False, ): """Prints success, failed, and skipped counts, as well as all affected objects.""" - update_success_count = len(to_update) - update_failed_count = len(failed_to_update) - update_skipped_count = len(skipped) + add_count = len(to_add) + update_count = len(to_update) + skipped_count = len(skipped) + failed_count = len(failed_to_update) + count_msgs = { + "added": ("Added", add_count, to_add), + "updated": ("Updated", update_count, to_update), + "skipped": ("Skipped updating", skipped_count, skipped), + "failed": ("Failed to update", failed_count, failed_to_update), + } if log_header is None: log_header = "============= FINISHED ===============" @@ -206,65 +223,60 @@ class TerminalHelper: if skipped_header is None: skipped_header = "----- SOME DATA WAS INVALID (NEEDS MANUAL PATCHING) -----" + if failed_header is None: + failed_header = "----- UPDATE FAILED -----" + # Give the user the option to see failed / skipped records if any exist. display_detailed_logs = False - if not debug and update_failed_count > 0 or update_skipped_count > 0: + if not debug and failed_count > 0 or skipped_count > 0: display_detailed_logs = TerminalHelper.prompt_for_execution( system_exit_on_terminate=False, - prompt_message=f"You will see {update_failed_count} failed and {update_skipped_count} skipped records.", + prompt_message=f"You will see {failed_count} failed and {skipped_count} skipped records.", verify_message="** Some records were skipped, or some failed to update. **", prompt_title="Do you wish to see the full list of failed, skipped and updated records?", ) - # Prepare debug messages + # Prepare debug messages (prints the internal add, update, skip, fail lists) if debug or display_detailed_logs: - updated_display = [str(u) for u in to_update] if display_as_str else to_update - skipped_display = [str(s) for s in skipped] if display_as_str else skipped - failed_display = [str(f) for f in failed_to_update] if display_as_str else failed_to_update - debug_messages = { - "success": (f"{TerminalColors.OKCYAN}Updated: {updated_display}{TerminalColors.ENDC}\n"), - "skipped": (f"{TerminalColors.YELLOW}Skipped: {skipped_display}{TerminalColors.ENDC}\n"), - "failed": (f"{TerminalColors.FAIL}Failed: {failed_display}{TerminalColors.ENDC}\n"), - } + debug_colors = [ + ("added", TerminalColors.OKBLUE), + ("updated", TerminalColors.OKCYAN), + ("skipped", TerminalColors.YELLOW), + ("failed", TerminalColors.FAIL), + ] + debug_messages = [] + for change_type, debug_log_color in debug_colors: + label, count, values = count_msgs.get(change_type) + display_values = [str(v) for v in values] if display_as_str else values + if count > 0: + debug_messages.append(f"{debug_log_color}{label}: {display_values}{TerminalColors.ENDC}") + if len(debug_messages) > 0: + logger.info("\n".join(debug_messages)) + else: + logger.info("Nothing to show: no changes occured.") - # Print out a list of everything that was changed, if we have any changes to log. - # Otherwise, don't print anything. - TerminalHelper.print_conditional( - True, - f"{debug_messages.get('success') if update_success_count > 0 else ''}" - f"{debug_messages.get('skipped') if update_skipped_count > 0 else ''}" - f"{debug_messages.get('failed') if update_failed_count > 0 else ''}", - ) + # Construct the header, log color, and level + color = TerminalColors.OKGREEN + log_level = logger.info + messages = [f"\n{log_header}"] + if failed_count > 0: + color = TerminalColors.FAIL + log_level = logger.error + messages.append(failed_header) + elif skipped_count > 0: + color = TerminalColors.YELLOW + log_level = logger.warning + messages.append(skipped_header) - if update_failed_count == 0 and update_skipped_count == 0: - logger.info( - f"""{TerminalColors.OKGREEN} - {log_header} - Updated {update_success_count} entries - {TerminalColors.ENDC} - """ - ) - elif update_failed_count == 0: - logger.warning( - f"""{TerminalColors.YELLOW} - {log_header} - Updated {update_success_count} entries - {skipped_header} - Skipped updating {update_skipped_count} entries - {TerminalColors.ENDC} - """ - ) - else: - logger.error( - f"""{TerminalColors.FAIL} - {log_header} - Updated {update_success_count} entries - ----- UPDATE FAILED ----- - Failed to update {update_failed_count} entries, - Skipped updating {update_skipped_count} entries - {TerminalColors.ENDC} - """ - ) + # Construct message (headers + only the counts that are > 0) + change_occured = False + for label, count, _ in count_msgs.values(): + if count > 0: + messages.append(f"{label} {count} entries") + change_occured = True + if not change_occured: + messages.append("No changes occured.") + TerminalHelper.colorful_logger(log_level, color, "\n".join(messages)) @staticmethod def query_yes_no(question: str, default="yes"): From e3a7713bba877f423b81d1f613e1afa35cb048d1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 13 Mar 2025 11:06:45 -0600 Subject: [PATCH 02/27] Update terminal_helper.py --- .../commands/utility/terminal_helper.py | 52 ++++++++----------- 1 file changed, 21 insertions(+), 31 deletions(-) diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index f4319e587..0f90d2a11 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -210,12 +210,6 @@ class TerminalHelper: update_count = len(to_update) skipped_count = len(skipped) failed_count = len(failed_to_update) - count_msgs = { - "added": ("Added", add_count, to_add), - "updated": ("Updated", update_count, to_update), - "skipped": ("Skipped updating", skipped_count, skipped), - "failed": ("Failed to update", failed_count, failed_to_update), - } if log_header is None: log_header = "============= FINISHED ===============" @@ -236,25 +230,6 @@ class TerminalHelper: prompt_title="Do you wish to see the full list of failed, skipped and updated records?", ) - # Prepare debug messages (prints the internal add, update, skip, fail lists) - if debug or display_detailed_logs: - debug_colors = [ - ("added", TerminalColors.OKBLUE), - ("updated", TerminalColors.OKCYAN), - ("skipped", TerminalColors.YELLOW), - ("failed", TerminalColors.FAIL), - ] - debug_messages = [] - for change_type, debug_log_color in debug_colors: - label, count, values = count_msgs.get(change_type) - display_values = [str(v) for v in values] if display_as_str else values - if count > 0: - debug_messages.append(f"{debug_log_color}{label}: {display_values}{TerminalColors.ENDC}") - if len(debug_messages) > 0: - logger.info("\n".join(debug_messages)) - else: - logger.info("Nothing to show: no changes occured.") - # Construct the header, log color, and level color = TerminalColors.OKGREEN log_level = logger.info @@ -268,14 +243,29 @@ class TerminalHelper: log_level = logger.warning messages.append(skipped_header) - # Construct message (headers + only the counts that are > 0) - change_occured = False - for label, count, _ in count_msgs.values(): + # Label, count, values, and debug log color + count_msgs = { + "added": ("Added", add_count, to_add, TerminalColors.OKBLUE), + "updated": ("Updated", update_count, to_update, TerminalColors.OKCYAN), + "skipped": ("Skipped updating", skipped_count, skipped, TerminalColors.YELLOW), + "failed": ("Failed to update", failed_count, failed_to_update, TerminalColors.FAIL), + } + change_occurred = False + for label, count, values, debug_log_color in count_msgs.values(): + # Print debug messages (prints the internal add, update, skip, fail lists) + if debug or display_detailed_logs: + display_values = [str(v) for v in values] if display_as_str else values + if count > 0: + message = f"{label}: {display_values}" + TerminalHelper.colorful_logger(logger.info, debug_log_color, message) + + # Assemble the final summary message (headers + only the counts that are > 0) if count > 0: messages.append(f"{label} {count} entries") - change_occured = True - if not change_occured: - messages.append("No changes occured.") + change_occurred = True + + if not change_occurred: + messages.append("No changes occurred.") TerminalHelper.colorful_logger(log_level, color, "\n".join(messages)) @staticmethod From 5d8bc1c638d271db8434b178dccbada244973091 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 13 Mar 2025 11:40:10 -0600 Subject: [PATCH 03/27] Extra cleanup --- .../commands/utility/terminal_helper.py | 66 +++++++++---------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index 0f90d2a11..c36c2eeef 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -210,6 +210,13 @@ class TerminalHelper: update_count = len(to_update) skipped_count = len(skipped) failed_count = len(failed_to_update) + # Label, count, values, and debug-specific log color + count_msgs = { + "added": ("Added", add_count, to_add, TerminalColors.OKBLUE), + "updated": ("Updated", update_count, to_update, TerminalColors.OKCYAN), + "skipped": ("Skipped updating", skipped_count, skipped, TerminalColors.YELLOW), + "failed": ("Failed to update", failed_count, failed_to_update, TerminalColors.FAIL), + } if log_header is None: log_header = "============= FINISHED ===============" @@ -230,43 +237,36 @@ class TerminalHelper: prompt_title="Do you wish to see the full list of failed, skipped and updated records?", ) - # Construct the header, log color, and level - color = TerminalColors.OKGREEN - log_level = logger.info - messages = [f"\n{log_header}"] - if failed_count > 0: - color = TerminalColors.FAIL - log_level = logger.error - messages.append(failed_header) - elif skipped_count > 0: - color = TerminalColors.YELLOW - log_level = logger.warning - messages.append(skipped_header) - - # Label, count, values, and debug log color - count_msgs = { - "added": ("Added", add_count, to_add, TerminalColors.OKBLUE), - "updated": ("Updated", update_count, to_update, TerminalColors.OKCYAN), - "skipped": ("Skipped updating", skipped_count, skipped, TerminalColors.YELLOW), - "failed": ("Failed to update", failed_count, failed_to_update, TerminalColors.FAIL), - } change_occurred = False - for label, count, values, debug_log_color in count_msgs.values(): - # Print debug messages (prints the internal add, update, skip, fail lists) - if debug or display_detailed_logs: - display_values = [str(v) for v in values] if display_as_str else values - if count > 0: + messages = [f"\n{log_header}"] + for change_type, dict_tuple in count_msgs.items(): + label, count, values, debug_log_color = dict_tuple + if count > 0: + # Print debug messages (prints the internal add, update, skip, fail lists) + if debug or display_detailed_logs: + display_values = [str(v) for v in values] if display_as_str else values message = f"{label}: {display_values}" TerminalHelper.colorful_logger(logger.info, debug_log_color, message) - # Assemble the final summary message (headers + only the counts that are > 0) - if count > 0: + # Get the sub-header for fail conditions + if change_type == "failed": + messages.append(failed_header) + if change_type == "skipped": + messages.append(skipped_header) + + # Print the change count messages.append(f"{label} {count} entries") change_occurred = True if not change_occurred: messages.append("No changes occurred.") - TerminalHelper.colorful_logger(log_level, color, "\n".join(messages)) + + if failed_count > 0: + TerminalHelper.colorful_logger("ERROR", "FAIL", "\n".join(messages)) + elif skipped_count > 0: + TerminalHelper.colorful_logger("WARNING", "YELLOW", "\n".join(messages)) + else: + TerminalHelper.colorful_logger("INFO", "OKGREEN", "\n".join(messages)) @staticmethod def query_yes_no(question: str, default="yes"): @@ -404,11 +404,11 @@ class TerminalHelper: # and ask if they wish to proceed proceed_execution = TerminalHelper.query_yes_no_exit( f"\n{TerminalColors.OKCYAN}" - "=====================================================" - f"\n{prompt_title}\n" - "=====================================================" - f"\n{verify_message}\n" - f"\n{prompt_message}\n" + "=====================================================\n" + f"{prompt_title}\n" + "=====================================================\n" + f"{verify_message}\n" + f"{prompt_message}\n" f"{TerminalColors.FAIL}" f"Proceed? (Y = proceed, N = {action_description_for_selecting_no})" f"{TerminalColors.ENDC}" From d894ee2e70f2f2d12e70ba2e3be9eae19452b7d0 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 13 Mar 2025 11:44:26 -0600 Subject: [PATCH 04/27] Update terminal_helper.py --- .../commands/utility/terminal_helper.py | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index c36c2eeef..a336489b3 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -204,8 +204,34 @@ class TerminalHelper: failed_header=None, display_as_str=False, ): - """Prints success, failed, and skipped counts, as well as - all affected objects.""" + """Generates a formatted summary of script execution results with colored output. + + Displays counts and details of successful, failed, and skipped operations. + In debug mode or when prompted, shows full record details. + Uses color coding: green for success, yellow for skipped, red for failures. + + Args: + to_update: Records that were successfully updated + failed_to_update: Records that failed to update + skipped: Records that were intentionally skipped + to_add: Records that were newly added + debug: If True, shows detailed record information + log_header: Custom header for the summary (default: "FINISHED") + skipped_header: Custom header for skipped records section + failed_header: Custom header for failed records section + display_as_str: If True, converts records to strings for display + + Output Format: + [Header] + Added: X entries + Updated: Y entries + Skipped: Z entries + Failed: W entries + + Debug output (if enabled): + - Full record details for each category + - Color coded by operation type + """ add_count = len(to_add) update_count = len(to_update) skipped_count = len(skipped) From 404178ef6f651f3bb911ac249651e9e0323c3a5b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 13 Mar 2025 15:26:37 -0600 Subject: [PATCH 05/27] Refactor (WIP) --- .../commands/create_federal_portfolio.py | 200 +++++++----------- .../commands/utility/terminal_helper.py | 45 ++-- 2 files changed, 99 insertions(+), 146 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 8ce8e624b..93bf1764b 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -3,7 +3,7 @@ import argparse import logging from django.core.management import BaseCommand, CommandError -from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper +from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper, ScriptChangeTracker from registrar.models import DomainInformation, DomainRequest, FederalAgency, Suborganization, Portfolio, User from registrar.models.domain import Domain from registrar.models.domain_invitation import DomainInvitation @@ -22,6 +22,13 @@ logger = logging.getLogger(__name__) class Command(BaseCommand): help = "Creates a federal portfolio given a FederalAgency name" + class ChangeTracker: + def __init__(self): + self.add = [] + self.update = [] + self.skip = [] + self.fail = [] + def __init__(self, *args, **kwargs): """Defines fields to track what portfolios were updated, skipped, or just outright failed.""" super().__init__(*args, **kwargs) @@ -32,14 +39,12 @@ class Command(BaseCommand): self.added_invitations = set() self.skipped_invitations = set() self.failed_managers = set() - self.portfolio_changes = {"add": [], "update": [], "skip": [], "fail": []} - self.suborganization_changes = {"add": [], "update": [], "skip": [], "fail": []} - self.domain_info_changes = {"add": [], "update": [], "skip": [], "fail": []} - self.domain_request_changes = {"add": [], "update": [], "skip": [], "fail": []} - - self.user_domain_role_changes = {"add": [], "update": [], "skip": [], "fail": []} - - self.portfolio_invitation_changes = {"add": [], "update": [], "skip": [], "fail": []} + self.portfolio_changes = self.ChangeTracker() + self.suborganization_changes = self.ChangeTracker() + self.domain_info_changes = self.ChangeTracker() + self.domain_request_changes = self.ChangeTracker() + self.user_domain_role_changes = self.ChangeTracker() + self.portfolio_invitation_changes = self.ChangeTracker() def add_arguments(self, parser): """Add command line arguments to create federal portfolios. @@ -102,7 +107,7 @@ class Command(BaseCommand): # Get agencies federal_agency_filter = {"agency__iexact": agency_name} if agency_name else {"federal_type": branch} agencies = FederalAgency.objects.filter(**federal_agency_filter) - if not agencies or agencies.count() < 1: + if not agencies.exists(): if agency_name: raise CommandError( f"Cannot find the federal agency '{agency_name}' in our database. " @@ -117,29 +122,32 @@ class Command(BaseCommand): 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) - portfolio = self.handle_populate_portfolio( - federal_agency, parse_domains, parse_requests, skip_existing_portfolios + portfolio, created = self.get_or_create_portfolio(federal_agency) + if skip_existing_portfolios and not created: + TerminalHelper.colorful_logger( + logger.warning, + TerminalColors.YELLOW, + "Skipping modifications to suborgs, domain requests, and " + "domains due to the --skip_existing_portfolios flag. Portfolio already exists.", ) + else: + self.create_suborganizations(portfolio, federal_agency) + if parse_domains: + self.handle_portfolio_domains(portfolio, federal_agency) + + if parse_requests: + self.handle_portfolio_requests(portfolio, federal_agency) + portfolios.append(portfolio) if add_managers: self.add_managers_to_portfolio(portfolio) - except Exception as exec: - self.failed_portfolios.add(federal_agency) - logger.error(exec) - message = f"Failed to create portfolio '{federal_agency.agency}'" - TerminalHelper.colorful_logger(logger.info, TerminalColors.FAIL, message) # POST PROCESS STEP: Add additional suborg info where applicable. updated_suborg_count = self.post_process_all_suborganization_fields(agencies) message = f"Added city and state_territory information to {updated_suborg_count} suborgs." TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) TerminalHelper.log_script_run_summary( - self.portfolio_changes.get("add"), - self.portfolio_changes.get("fail"), - self.portfolio_changes.get("skip"), - [], + **vars(self.portfolio_changes), debug=False, log_header="============= FINISHED HANDLE PORTFOLIO STEP ===============", skipped_header="----- SOME PORTFOLIOS WERENT CREATED (BUT OTHER RECORDS ARE STILL PROCESSED) -----", @@ -148,20 +156,14 @@ class Command(BaseCommand): if add_managers: TerminalHelper.log_script_run_summary( - self.user_domain_role_changes.get("add"), - self.user_domain_role_changes.get("fail"), - self.user_domain_role_changes.get("skip"), - [], + **vars(self.user_domain_role_changes), log_header="----- MANAGERS ADDED -----", debug=False, display_as_str=True, ) TerminalHelper.log_script_run_summary( - self.added_invitations, - [], - self.skipped_invitations, - [], + **vars(self.portfolio_invitation_changes), log_header="----- INVITATIONS ADDED -----", debug=False, skipped_header="----- INVITATIONS SKIPPED (ALREADY EXISTED) -----", @@ -188,6 +190,29 @@ class Command(BaseCommand): ) self.post_process_started_domain_requests(agencies, portfolios) + def get_or_create_portfolio(self, federal_agency): + portfolio, created = Portfolio.objects.get_or_create( + organization_name=federal_agency.agency, + federal_agency=federal_agency, + organization_type=DomainRequest.OrganizationChoices.FEDERAL, + creator=User.get_default_user(), + notes="Auto-generated record", + senior_official=federal_agency.so_federal_agency.first(), + ) + + if created: + self.portfolio_changes.add.append(portfolio) + TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, f"Created portfolio '{portfolio}'") + else: + self.skipped_portfolios.add(portfolio) + TerminalHelper.colorful_logger( + logger.info, + TerminalColors.YELLOW, + f"Portfolio with organization name '{portfolio}' already exists. Skipping create.", + ) + + return portfolio, created + def add_managers_to_portfolio(self, portfolio: Portfolio): """ Add all domain managers of the portfolio's domains to the organization. @@ -302,114 +327,39 @@ class Command(BaseCommand): DomainRequest.objects.bulk_update(updated_requests, ["federal_agency"]) TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, "Action completed successfully.") - def handle_populate_portfolio(self, federal_agency, parse_domains, parse_requests, skip_existing_portfolios): - """Attempts to create a portfolio. If successful, this function will - also create new suborganizations""" - portfolio, created = self.create_portfolio(federal_agency) - if skip_existing_portfolios and not created: - TerminalHelper.colorful_logger( - logger.warning, - TerminalColors.YELLOW, - "Skipping modifications to suborgs, domain requests, and " - "domains due to the --skip_existing_portfolios flag. Portfolio already exists.", - ) - return portfolio - - self.create_suborganizations(portfolio, federal_agency) - if parse_domains: - self.handle_portfolio_domains(portfolio, federal_agency) - - if parse_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. - Returns portfolio, created.""" - # Get the org name / senior official - org_name = federal_agency.agency - so = federal_agency.so_federal_agency.first() if federal_agency.so_federal_agency.exists() else None - - # First just try to get an existing portfolio - portfolio = Portfolio.objects.filter(organization_name=org_name).first() - if portfolio: - self.skipped_portfolios.add(portfolio) - TerminalHelper.colorful_logger( - logger.info, - TerminalColors.YELLOW, - f"Portfolio with organization name '{org_name}' already exists. Skipping create.", - ) - return portfolio, False - - # Create new portfolio if it doesn't exist - portfolio = Portfolio.objects.create( - organization_name=org_name, - federal_agency=federal_agency, - organization_type=DomainRequest.OrganizationChoices.FEDERAL, - creator=User.get_default_user(), - notes="Auto-generated record", - senior_official=so, - ) - - self.updated_portfolios.add(portfolio) - TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, f"Created portfolio '{portfolio}'") - - # Log if the senior official was added or not. - if portfolio.senior_official: - message = f"Added senior official '{portfolio.senior_official}'" - TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) - else: - message = ( - f"No senior official added to portfolio '{org_name}'. " - "None was returned for the reverse relation `FederalAgency.so_federal_agency.first()`" - ) - TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) - - return portfolio, True - 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 - ) + valid_agencies = federal_agency.domaininformation_set.filter(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." - f"\nNo suborganizations were found for '{federal_agency}' when filtering on this name, " - "and excluding null organization_name records." - ) - TerminalHelper.colorful_logger(logger.warning, TerminalColors.FAIL, message) - return - - # Check for existing suborgs on the current portfolio - 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) - - # 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)): + existing_org_names = set( + Suborganization.objects + .filter(name__in=org_names) + .values_list("name", flat=True) + ) + for name in org_names - existing_org_names: 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. + self.suborganization_changes.skip.append(suborg) message = ( f"Skipping suborganization create on record '{name}'. " "The federal agency name is the same as the portfolio name." ) TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) else: - new_suborgs.append(Suborganization(name=name, portfolio=portfolio)) # type: ignore + suborg = Suborganization(name=name, portfolio=portfolio) + self.suborganization_changes.add.append(suborg) - if new_suborgs: - Suborganization.objects.bulk_create(new_suborgs) + suborg_add_count = len(self.suborganization_changes.add) + if suborg_add_count > 0: + Suborganization.objects.bulk_create(self.suborganization_changes.add) TerminalHelper.colorful_logger( - logger.info, TerminalColors.OKGREEN, f"Added {len(new_suborgs)} suborganizations" + logger.info, TerminalColors.OKGREEN, f"Added {len(suborg_add_count)} suborganizations to '{federal_agency}'." ) else: - TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, "No suborganizations added") + TerminalHelper.colorful_logger( + logger.warning, + TerminalColors.YELLOW, + f"No suborganizations added for '{federal_agency}'." + ) def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency): """ diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index a336489b3..12402b1ff 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -192,12 +192,13 @@ class PopulateScriptTemplate(ABC): class TerminalHelper: + @staticmethod def log_script_run_summary( - to_update, - failed_to_update, - skipped, - to_add, + add, + update, + skip, + fail, debug: bool, log_header=None, skipped_header=None, @@ -221,27 +222,29 @@ class TerminalHelper: failed_header: Custom header for failed records section display_as_str: If True, converts records to strings for display - Output Format: - [Header] - Added: X entries - Updated: Y entries - Skipped: Z entries - Failed: W entries + Output Format (if count > 0 for each category): + [log_header] + Added W entries + Updated X entries + [skipped_header] + Skipped updating Y entries + [failed_header] + Failed to update Z entries - Debug output (if enabled): - - Full record details for each category - - Color coded by operation type + Debug output (if enabled): + - Directly prints each list for each category (add, update, etc) + - Converts each item to string if display_as_str is True """ - add_count = len(to_add) - update_count = len(to_update) - skipped_count = len(skipped) - failed_count = len(failed_to_update) + add_count = len(add) + update_count = len(update) + skipped_count = len(skip) + failed_count = len(fail) # Label, count, values, and debug-specific log color count_msgs = { - "added": ("Added", add_count, to_add, TerminalColors.OKBLUE), - "updated": ("Updated", update_count, to_update, TerminalColors.OKCYAN), - "skipped": ("Skipped updating", skipped_count, skipped, TerminalColors.YELLOW), - "failed": ("Failed to update", failed_count, failed_to_update, TerminalColors.FAIL), + "added": ("Added", add_count, add, TerminalColors.OKBLUE), + "updated": ("Updated", update_count, update, TerminalColors.OKCYAN), + "skipped": ("Skipped updating", skipped_count, skip, TerminalColors.YELLOW), + "failed": ("Failed to update", failed_count, fail, TerminalColors.FAIL), } if log_header is None: From b813dac57e41896a9ce99d44d0ace49d3a845204 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 14 Mar 2025 08:29:52 -0600 Subject: [PATCH 06/27] Update create_federal_portfolio.py --- .../commands/create_federal_portfolio.py | 82 ++++++++++--------- 1 file changed, 45 insertions(+), 37 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 93bf1764b..80a80dddb 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -131,6 +131,7 @@ class Command(BaseCommand): "domains due to the --skip_existing_portfolios flag. Portfolio already exists.", ) else: + # if parse_suborganizations self.create_suborganizations(portfolio, federal_agency) if parse_domains: self.handle_portfolio_domains(portfolio, federal_agency) @@ -213,6 +214,41 @@ class Command(BaseCommand): return portfolio, created + def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency): + """Create Suborganizations tied to the given portfolio based on DomainInformation objects""" + valid_agencies = federal_agency.domaininformation_set.filter(organization_name__isnull=False) + org_names = set(valid_agencies.values_list("organization_name", flat=True)) + # TODO - this needs to do a iexact / normalize step + existing_org_names = set( + Suborganization.objects + .filter(name__in=org_names) + .values_list("name", flat=True) + ) + for name in org_names - existing_org_names: + if normalize_string(name) == normalize_string(portfolio.organization_name): + self.suborganization_changes.skip.append(suborg) + message = ( + f"Skipping suborganization create on record '{name}'. " + "The federal agency name is the same as the portfolio name." + ) + TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) + else: + suborg = Suborganization(name=name, portfolio=portfolio) + self.suborganization_changes.add.append(suborg) + + suborg_add_count = len(self.suborganization_changes.add) + if suborg_add_count > 0: + Suborganization.objects.bulk_create(self.suborganization_changes.add) + TerminalHelper.colorful_logger( + logger.info, TerminalColors.OKGREEN, f"Added {len(suborg_add_count)} suborganizations to '{federal_agency}'." + ) + else: + TerminalHelper.colorful_logger( + logger.warning, + TerminalColors.YELLOW, + f"No suborganizations added for '{federal_agency}'." + ) + def add_managers_to_portfolio(self, portfolio: Portfolio): """ Add all domain managers of the portfolio's domains to the organization. @@ -327,40 +363,6 @@ class Command(BaseCommand): DomainRequest.objects.bulk_update(updated_requests, ["federal_agency"]) TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, "Action completed successfully.") - def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency): - """Create Suborganizations tied to the given portfolio based on DomainInformation objects""" - valid_agencies = federal_agency.domaininformation_set.filter(organization_name__isnull=False) - org_names = set(valid_agencies.values_list("organization_name", flat=True)) - existing_org_names = set( - Suborganization.objects - .filter(name__in=org_names) - .values_list("name", flat=True) - ) - for name in org_names - existing_org_names: - if normalize_string(name) == normalize_string(portfolio.organization_name): - self.suborganization_changes.skip.append(suborg) - message = ( - f"Skipping suborganization create on record '{name}'. " - "The federal agency name is the same as the portfolio name." - ) - TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) - else: - suborg = Suborganization(name=name, portfolio=portfolio) - self.suborganization_changes.add.append(suborg) - - suborg_add_count = len(self.suborganization_changes.add) - if suborg_add_count > 0: - Suborganization.objects.bulk_create(self.suborganization_changes.add) - TerminalHelper.colorful_logger( - logger.info, TerminalColors.OKGREEN, f"Added {len(suborg_add_count)} suborganizations to '{federal_agency}'." - ) - else: - TerminalHelper.colorful_logger( - logger.warning, - TerminalColors.YELLOW, - f"No suborganizations added for '{federal_agency}'." - ) - def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency): """ Associate portfolio with domain requests for a federal agency. @@ -421,7 +423,7 @@ class Command(BaseCommand): Returns a queryset of DomainInformation objects, or None if nothing changed. """ - domain_infos = DomainInformation.objects.filter(federal_agency=federal_agency) + domain_infos = federal_agency.domaininformation_set.all() if not domain_infos.exists(): message = f""" Portfolio '{portfolio}' not added to domains: no valid records found. @@ -434,8 +436,14 @@ class Command(BaseCommand): # 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_info in domain_infos: - domain_info.portfolio = portfolio - domain_info.sub_organization = suborgs.get(domain_info.organization_name, None) + # Does this need a log? + suborg = suborgs.get(domain_info.organization_name, None) + if suborg: + domain_info.portfolio = portfolio + domain_info.sub_organization = suborgs.get(domain_info.organization_name, None) + self.domain_info_changes.update.append(domain_info) + else: + self.domain_info_changes.skip.append(domain_info) DomainInformation.objects.bulk_update(domain_infos, ["portfolio", "sub_organization"]) message = f"Added portfolio '{portfolio}' to {len(domain_infos)} domains." From 5fa6e0dbcb02c1a8036f77995f87b3d4d7ed6719 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Sun, 16 Mar 2025 20:51:04 -0600 Subject: [PATCH 07/27] Further simplification --- .../commands/utility/terminal_helper.py | 86 ++++++++----------- 1 file changed, 35 insertions(+), 51 deletions(-) diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index 12402b1ff..d1f725974 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -200,9 +200,9 @@ class TerminalHelper: skip, fail, debug: bool, - log_header=None, - skipped_header=None, - failed_header=None, + log_header="============= FINISHED ===============", + skipped_header="----- SOME DATA WAS INVALID (NEEDS MANUAL PATCHING) -----", + failed_header="----- UPDATE FAILED -----", display_as_str=False, ): """Generates a formatted summary of script execution results with colored output. @@ -235,67 +235,51 @@ class TerminalHelper: - Directly prints each list for each category (add, update, etc) - Converts each item to string if display_as_str is True """ - add_count = len(add) - update_count = len(update) - skipped_count = len(skip) - failed_count = len(fail) - # Label, count, values, and debug-specific log color - count_msgs = { - "added": ("Added", add_count, add, TerminalColors.OKBLUE), - "updated": ("Updated", update_count, update, TerminalColors.OKCYAN), - "skipped": ("Skipped updating", skipped_count, skip, TerminalColors.YELLOW), - "failed": ("Failed to update", failed_count, fail, TerminalColors.FAIL), + counts = { + "added": len(add), + "updated": len(update), + "skipped": len(skip), + "failed": len(fail), } - if log_header is None: - log_header = "============= FINISHED ===============" - - if skipped_header is None: - skipped_header = "----- SOME DATA WAS INVALID (NEEDS MANUAL PATCHING) -----" - - if failed_header is None: - failed_header = "----- UPDATE FAILED -----" - # Give the user the option to see failed / skipped records if any exist. display_detailed_logs = False - if not debug and failed_count > 0 or skipped_count > 0: + if not debug and counts["failed"] > 0 or counts["skipped"] > 0: display_detailed_logs = TerminalHelper.prompt_for_execution( system_exit_on_terminate=False, - prompt_message=f"You will see {failed_count} failed and {skipped_count} skipped records.", + prompt_message=f'You will see {counts["failed"]} failed and {counts["updated"]} skipped records.', verify_message="** Some records were skipped, or some failed to update. **", prompt_title="Do you wish to see the full list of failed, skipped and updated records?", ) - - change_occurred = False - messages = [f"\n{log_header}"] - for change_type, dict_tuple in count_msgs.items(): - label, count, values, debug_log_color = dict_tuple - if count > 0: - # Print debug messages (prints the internal add, update, skip, fail lists) - if debug or display_detailed_logs: - display_values = [str(v) for v in values] if display_as_str else values - message = f"{label}: {display_values}" - TerminalHelper.colorful_logger(logger.info, debug_log_color, message) - - # Get the sub-header for fail conditions - if change_type == "failed": - messages.append(failed_header) - if change_type == "skipped": + + messages = [] + for category, count in counts.items(): + match category: + case "added": + label, values, debug_color = "Added", add, TerminalColors.OKBLUE + case "updated": + label, values, debug_color = "Updated", update, TerminalColors.OKCYAN + case "skipped": + label, values, debug_color = "Skipped updating", skip, TerminalColors.YELLOW messages.append(skipped_header) + case "failed": + label, values, debug_color = "Failed to update", fail, TerminalColors.FAIL + messages.append(failed_header) + messages.append(f"{label} {count} entries") - # Print the change count - messages.append(f"{label} {count} entries") - change_occurred = True + # Print debug messages (prints the internal add, update, skip, fail lists) + if debug or display_detailed_logs: + display_values = [str(v) for v in values] if display_as_str else values + debug_message = f"{label}: {display_values}" + TerminalHelper.colorful_logger(logger.info, debug_color, debug_message) - if not change_occurred: - messages.append("No changes occurred.") - - if failed_count > 0: - TerminalHelper.colorful_logger("ERROR", "FAIL", "\n".join(messages)) - elif skipped_count > 0: - TerminalHelper.colorful_logger("WARNING", "YELLOW", "\n".join(messages)) + final_message = f"\n{log_header}" + "\n".join(messages) + if counts["failed"] > 0: + TerminalHelper.colorful_logger("ERROR", "FAIL", final_message) + elif counts["skipped"] > 0: + TerminalHelper.colorful_logger("WARNING", "YELLOW", final_message) else: - TerminalHelper.colorful_logger("INFO", "OKGREEN", "\n".join(messages)) + TerminalHelper.colorful_logger("INFO", "OKGREEN", final_message) @staticmethod def query_yes_no(question: str, default="yes"): From 40c5aa470450c45c7ea7e0118effdee9a4960bdd Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Sun, 16 Mar 2025 20:52:29 -0600 Subject: [PATCH 08/27] Update terminal_helper.py --- src/registrar/management/commands/utility/terminal_helper.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index d1f725974..5c9ddade2 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -251,9 +251,10 @@ class TerminalHelper: verify_message="** Some records were skipped, or some failed to update. **", prompt_title="Do you wish to see the full list of failed, skipped and updated records?", ) - + + non_zero_counts = {category: count for category, count in counts.items() if count > 0} messages = [] - for category, count in counts.items(): + for category, count in non_zero_counts.items(): match category: case "added": label, values, debug_color = "Added", add, TerminalColors.OKBLUE From 7f277eaa0de659117e705408e90b1a53b9f66ac8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 18 Mar 2025 09:19:06 -0600 Subject: [PATCH 09/27] Update create_federal_portfolio.py --- .../commands/create_federal_portfolio.py | 196 ++++++++---------- 1 file changed, 88 insertions(+), 108 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 80a80dddb..495184b60 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -121,15 +121,14 @@ class Command(BaseCommand): portfolios = [] for federal_agency in agencies: message = f"Processing federal agency '{federal_agency.agency}'..." - TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) + logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") portfolio, created = self.get_or_create_portfolio(federal_agency) if skip_existing_portfolios and not created: - TerminalHelper.colorful_logger( - logger.warning, - TerminalColors.YELLOW, + message = ( "Skipping modifications to suborgs, domain requests, and " - "domains due to the --skip_existing_portfolios flag. Portfolio already exists.", + "domains due to the --skip_existing_portfolios flag. Portfolio already exists." ) + logger.warning(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") else: # if parse_suborganizations self.create_suborganizations(portfolio, federal_agency) @@ -203,22 +202,18 @@ class Command(BaseCommand): if created: self.portfolio_changes.add.append(portfolio) - TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, f"Created portfolio '{portfolio}'") + logger.info(f"{TerminalColors.OKGREEN}Created portfolio '{portfolio}'.{TerminalColors.ENDC}") else: self.skipped_portfolios.add(portfolio) - TerminalHelper.colorful_logger( - logger.info, - TerminalColors.YELLOW, - f"Portfolio with organization name '{portfolio}' already exists. Skipping create.", - ) - + message = f"Portfolio with organization name '{portfolio}' already exists. Skipping create." + logger.info(f"{TerminalColors.OKGREEN}{message}{TerminalColors.ENDC}") + return portfolio, created def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency): """Create Suborganizations tied to the given portfolio based on DomainInformation objects""" valid_agencies = federal_agency.domaininformation_set.filter(organization_name__isnull=False) org_names = set(valid_agencies.values_list("organization_name", flat=True)) - # TODO - this needs to do a iexact / normalize step existing_org_names = set( Suborganization.objects .filter(name__in=org_names) @@ -231,7 +226,7 @@ class Command(BaseCommand): f"Skipping suborganization create on record '{name}'. " "The federal agency name is the same as the portfolio name." ) - TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) + logger.warning(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") else: suborg = Suborganization(name=name, portfolio=portfolio) self.suborganization_changes.add.append(suborg) @@ -239,15 +234,86 @@ class Command(BaseCommand): suborg_add_count = len(self.suborganization_changes.add) if suborg_add_count > 0: Suborganization.objects.bulk_create(self.suborganization_changes.add) - TerminalHelper.colorful_logger( - logger.info, TerminalColors.OKGREEN, f"Added {len(suborg_add_count)} suborganizations to '{federal_agency}'." - ) + message = f"Added {suborg_add_count} suborganizations to '{federal_agency}'." + logger.info(f"{TerminalColors.OKGREEN}{message}{TerminalColors.ENDC}") else: - TerminalHelper.colorful_logger( - logger.warning, - TerminalColors.YELLOW, - f"No suborganizations added for '{federal_agency}'." - ) + message = f"No suborganizations added for '{federal_agency}'." + logger.warning(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") + + def handle_portfolio_domains(self, portfolio: Portfolio, federal_agency: FederalAgency): + """ + Associate portfolio with domains for a federal agency. + Updates all relevant domain information records. + + Returns a queryset of DomainInformation objects, or None if nothing changed. + """ + domain_infos = federal_agency.domaininformation_set.all() + if not domain_infos.exists(): + message = f""" + Portfolio '{portfolio}' not added to domains: no valid records found. + The filter on DomainInformation for the federal_agency '{federal_agency}' returned no results. + """ + logger.info(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") + return None + + # TODO - this needs a normalize step on suborg! + # 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_info in domain_infos: + domain_info.portfolio = portfolio + domain_info.sub_organization = suborgs.get(domain_info.organization_name, None) + self.domain_info_changes.update.append(domain_info) + + DomainInformation.objects.bulk_update(domain_infos, ["portfolio", "sub_organization"]) + message = f"Added portfolio '{portfolio}' to {len(domain_infos)} domains." + logger.info(f"{TerminalColors.OKGREEN}{message}{TerminalColors.ENDC}") + + 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. + """ + invalid_states = [ + DomainRequest.DomainRequestStatus.STARTED, + DomainRequest.DomainRequestStatus.INELIGIBLE, + DomainRequest.DomainRequestStatus.REJECTED, + ] + domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency).exclude(status__in=invalid_states) + if not domain_requests.exists(): + message = f""" + Portfolio '{portfolio}' not added to domain requests: nothing to add found. + Excluded statuses: STARTED, INELIGIBLE, REJECTED. + """ + logger.warning(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") + TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) + return None + + # Get all suborg information and store it in a dict to avoid doing a db call + # TODO - this needs a normalize step on suborg! + suborgs = Suborganization.objects.filter(portfolio=portfolio).in_bulk(field_name="name") + for domain_request in domain_requests: + domain_request.portfolio = portfolio + domain_request.sub_organization = suborgs.get(domain_request.organization_name, None) + if domain_request.sub_organization is None: + 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.domain_request_changes.add.append(portfolio) + + 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." + logger.info(f"{TerminalColors.OKGREEN}{message}{TerminalColors.ENDC}") def add_managers_to_portfolio(self, portfolio: Portfolio): """ @@ -363,92 +429,6 @@ class Command(BaseCommand): DomainRequest.objects.bulk_update(updated_requests, ["federal_agency"]) TerminalHelper.colorful_logger(logger.info, TerminalColors.OKBLUE, "Action completed successfully.") - 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. - """ - invalid_states = [ - DomainRequest.DomainRequestStatus.STARTED, - DomainRequest.DomainRequestStatus.INELIGIBLE, - DomainRequest.DomainRequestStatus.REJECTED, - ] - domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency).exclude(status__in=invalid_states) - if not domain_requests.exists(): - message = f""" - Portfolio '{portfolio}' not added to domain requests: no valid records found. - This means that a filter on DomainInformation for the federal_agency '{federal_agency}' returned no results. - Excluded statuses: STARTED, INELIGIBLE, REJECTED. - Filter info: DomainRequest.objects.filter(federal_agency=federal_agency).exclude( - status__in=invalid_states - ) - """ - TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) - return None - - # 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 - - # Set suborg info - domain_request.sub_organization = suborgs.get(domain_request.organization_name, None) - if domain_request.sub_organization is None: - 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( - 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) - - def handle_portfolio_domains(self, portfolio: Portfolio, federal_agency: FederalAgency): - """ - Associate portfolio with domains for a federal agency. - Updates all relevant domain information records. - - Returns a queryset of DomainInformation objects, or None if nothing changed. - """ - domain_infos = federal_agency.domaininformation_set.all() - if not domain_infos.exists(): - message = f""" - Portfolio '{portfolio}' not added to domains: no valid records found. - The filter on DomainInformation for the federal_agency '{federal_agency}' returned no results. - Filter info: DomainInformation.objects.filter(federal_agency=federal_agency) - """ - TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) - return None - - # 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_info in domain_infos: - # Does this need a log? - suborg = suborgs.get(domain_info.organization_name, None) - if suborg: - domain_info.portfolio = portfolio - domain_info.sub_organization = suborgs.get(domain_info.organization_name, None) - self.domain_info_changes.update.append(domain_info) - else: - self.domain_info_changes.skip.append(domain_info) - - DomainInformation.objects.bulk_update(domain_infos, ["portfolio", "sub_organization"]) - message = f"Added portfolio '{portfolio}' to {len(domain_infos)} domains." - TerminalHelper.colorful_logger(logger.info, TerminalColors.OKGREEN, message) - def post_process_all_suborganization_fields(self, agencies): """Batch updates suborganization locations from domain and request data. From 7f7b42e384c01458b4d463007291f9b5ebec622c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 18 Mar 2025 09:25:17 -0600 Subject: [PATCH 10/27] Normalize --- .../commands/create_federal_portfolio.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 495184b60..251fefddf 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -191,8 +191,9 @@ class Command(BaseCommand): self.post_process_started_domain_requests(agencies, portfolios) def get_or_create_portfolio(self, federal_agency): + portfolio_name = normalize_string(federal_agency.agency, lowercase=False) portfolio, created = Portfolio.objects.get_or_create( - organization_name=federal_agency.agency, + organization_name=portfolio_name, federal_agency=federal_agency, organization_type=DomainRequest.OrganizationChoices.FEDERAL, creator=User.get_default_user(), @@ -228,7 +229,8 @@ class Command(BaseCommand): ) logger.warning(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") else: - suborg = Suborganization(name=name, portfolio=portfolio) + suborg_name = normalize_string(name) + suborg = Suborganization(name=suborg_name, portfolio=portfolio) self.suborganization_changes.add.append(suborg) suborg_add_count = len(self.suborganization_changes.add) @@ -256,12 +258,12 @@ class Command(BaseCommand): logger.info(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") return None - # TODO - this needs a normalize step on suborg! # 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_info in domain_infos: + org_name = normalize_string(domain_info.organization_name, lowercase=False) domain_info.portfolio = portfolio - domain_info.sub_organization = suborgs.get(domain_info.organization_name, None) + domain_info.sub_organization = suborgs.get(org_name, None) self.domain_info_changes.update.append(domain_info) DomainInformation.objects.bulk_update(domain_infos, ["portfolio", "sub_organization"]) @@ -289,11 +291,11 @@ class Command(BaseCommand): return None # Get all suborg information and store it in a dict to avoid doing a db call - # TODO - this needs a normalize step on suborg! suborgs = Suborganization.objects.filter(portfolio=portfolio).in_bulk(field_name="name") for domain_request in domain_requests: + org_name = normalize_string(domain_request.organization_name, lowercase=False) domain_request.portfolio = portfolio - domain_request.sub_organization = suborgs.get(domain_request.organization_name, None) + domain_request.sub_organization = suborgs.get(org_name, None) if domain_request.sub_organization is None: domain_request.requested_suborganization = normalize_string( domain_request.organization_name, lowercase=False From 0355edf7db7d401e86ec064e858a8dd9454a3a92 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 18 Mar 2025 10:00:11 -0600 Subject: [PATCH 11/27] simplify handle manager logic --- .../commands/create_federal_portfolio.py | 119 +++++++++--------- 1 file changed, 62 insertions(+), 57 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 251fefddf..5f6019b46 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -82,7 +82,7 @@ class Command(BaseCommand): help="Adds portfolio to DomainInformation", ) parser.add_argument( - "--add_managers", + "--parse_managers", action=argparse.BooleanOptionalAction, help="Add all domain managers of the portfolio's domains to the organization.", ) @@ -97,12 +97,12 @@ class Command(BaseCommand): branch = options.get("branch") parse_requests = options.get("parse_requests") parse_domains = options.get("parse_domains") - add_managers = options.get("add_managers") + parse_managers = options.get("parse_managers") skip_existing_portfolios = options.get("skip_existing_portfolios") # Parse script params - if not (parse_requests or parse_domains or add_managers): - raise CommandError("You must specify at least one of --parse_requests, --parse_domains, or --add_managers.") + if not (parse_requests or parse_domains or parse_managers): + raise CommandError("You must specify at least one of --parse_requests, --parse_domains, or --parse_managers.") # Get agencies federal_agency_filter = {"agency__iexact": agency_name} if agency_name else {"federal_type": branch} @@ -139,8 +139,8 @@ class Command(BaseCommand): self.handle_portfolio_requests(portfolio, federal_agency) portfolios.append(portfolio) - if add_managers: - self.add_managers_to_portfolio(portfolio) + if parse_managers: + self.handle_portfolio_managers(portfolio) # POST PROCESS STEP: Add additional suborg info where applicable. updated_suborg_count = self.post_process_all_suborganization_fields(agencies) @@ -154,7 +154,7 @@ class Command(BaseCommand): display_as_str=True, ) - if add_managers: + if parse_managers: TerminalHelper.log_script_run_summary( **vars(self.user_domain_role_changes), log_header="----- MANAGERS ADDED -----", @@ -213,6 +213,9 @@ class Command(BaseCommand): def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency): """Create Suborganizations tied to the given portfolio based on DomainInformation objects""" + message = f"Creating suborgs for portfolio {portfolio}." + logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") + valid_agencies = federal_agency.domaininformation_set.filter(organization_name__isnull=False) org_names = set(valid_agencies.values_list("organization_name", flat=True)) existing_org_names = set( @@ -249,6 +252,9 @@ class Command(BaseCommand): Returns a queryset of DomainInformation objects, or None if nothing changed. """ + message = f"Adding domains to portfolio {portfolio}." + logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") + domain_infos = federal_agency.domaininformation_set.all() if not domain_infos.exists(): message = f""" @@ -275,6 +281,9 @@ class Command(BaseCommand): Associate portfolio with domain requests for a federal agency. Updates all relevant domain request records. """ + message = f"Adding domain requests to portfolio {portfolio}." + logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") + invalid_states = [ DomainRequest.DomainRequestStatus.STARTED, DomainRequest.DomainRequestStatus.INELIGIBLE, @@ -317,72 +326,68 @@ class Command(BaseCommand): message = f"Added portfolio '{portfolio}' to {len(domain_requests)} domain requests." logger.info(f"{TerminalColors.OKGREEN}{message}{TerminalColors.ENDC}") - def add_managers_to_portfolio(self, portfolio: Portfolio): + # TODO - this doesn't send an email out, should it? + def handle_portfolio_managers(self, portfolio: Portfolio): """ Add all domain managers of the portfolio's domains to the organization. This includes adding them to the correct group and creating portfolio invitations. """ - logger.info(f"Adding managers for portfolio {portfolio}") + message = f"Adding managers to portfolio {portfolio}." + logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") - # Fetch all domains associated with the portfolio - domains = Domain.objects.filter(domain_info__portfolio=portfolio) - domain_managers: set[UserDomainRole] = set() + domains = portfolio.information_portfolio.all().values_list("domain", flat=True) # Fetch all users with manager roles for the domains - # select_related means that a db query will not be occur when you do user_domain_role.user - # Its similar to a set or dict in that it costs slightly more upfront in exchange for perf later user_domain_roles = UserDomainRole.objects.select_related("user").filter( domain__in=domains, role=UserDomainRole.Roles.MANAGER ) - domain_managers.update(user_domain_roles) + existing_permissions = UserPortfolioPermission.objects.filter( + user__in=user_domain_roles.values_list("user") + ).in_bulk(field_name="user") + for user_domain_role in user_domain_roles: + user = user_domain_role.user + if user not in existing_permissions: + permission = UserPortfolioPermission( + portfolio=portfolio, + user=user, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + ) + self.user_domain_role_changes.add.append(permission) + logger.info(f"Added manager '{permission.user}' to portfolio '{portfolio}'.") + else: + existing_permission = existing_permissions.get(user) + self.user_domain_role_changes.skip.append(existing_permission) + logger.info(f"Manager '{permission.user}' already exists in portfolio '{portfolio}'.") - invited_managers: set[str] = set() + # Bulk create user portfolio permissions + UserPortfolioPermission.objects.bulk_create(self.user_domain_role_changes.add) + # TODO - needs normalize step # Get the emails of invited managers domain_invitations = DomainInvitation.objects.filter( domain__in=domains, status=DomainInvitation.DomainInvitationStatus.INVITED - ).values_list("email", flat=True) - invited_managers.update(domain_invitations) - - for user_domain_role in domain_managers: - try: - # manager is a user id - user = user_domain_role.user - _, created = UserPortfolioPermission.objects.get_or_create( - portfolio=portfolio, - user=user, - defaults={"roles": [UserPortfolioRoleChoices.ORGANIZATION_MEMBER]}, - ) - self.added_managers.add(user) - if created: - logger.info(f"Added manager '{user}' to portfolio '{portfolio}'") - else: - logger.info(f"Manager '{user}' already exists in portfolio '{portfolio}'") - except User.DoesNotExist: - self.failed_managers.add(user) - logger.debug(f"User '{user}' does not exist") - - for email in invited_managers: - self.create_portfolio_invitation(portfolio, email) - - def create_portfolio_invitation(self, portfolio: Portfolio, email: str): - """ - Create a portfolio invitation for the given email. - """ - _, created = PortfolioInvitation.objects.get_or_create( - portfolio=portfolio, - email=email, - defaults={ - "status": PortfolioInvitation.PortfolioInvitationStatus.INVITED, - "roles": [UserPortfolioRoleChoices.ORGANIZATION_MEMBER], - }, ) - if created: - self.added_invitations.add(email) - logger.info(f"Created portfolio invitation for '{email}' to portfolio '{portfolio}'") - else: - self.skipped_invitations.add(email) - logger.info(f"Found existing portfolio invitation for '{email}' to portfolio '{portfolio}'") + existing_invitations = PortfolioInvitation.objects.filter( + email__in=domain_invitations.values_list("email") + ).in_bulk(field_name="user") + for domain_invitation in domain_invitations: + email = domain_invitation.email + if email not in existing_invitations: + invitation = PortfolioInvitation( + portfolio=portfolio, + email=email, + status = PortfolioInvitation.PortfolioInvitationStatus.INVITED, + roles = [UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + ) + self.portfolio_invitation_changes.add.append(invitation) + logger.info(f"Added invitation '{email}' to portfolio '{portfolio}'.") + else: + existing_invitation = existing_invitations.get(email) + self.portfolio_invitation_changes.skip.append(existing_invitation) + logger.info(f"Invitation '{email}' already exists in portfolio '{portfolio}'.") + + # Bulk create portfolio invitations + PortfolioInvitation.objects.bulk_create(self.user_domain_role_changes.add) def post_process_started_domain_requests(self, agencies, portfolios): """ From ceff1b4a48056c64255f4b981c3e9ebef353c2d5 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 18 Mar 2025 10:32:32 -0600 Subject: [PATCH 12/27] Update create_federal_portfolio.py --- .../commands/create_federal_portfolio.py | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 5f6019b46..56046d3eb 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -125,11 +125,14 @@ class Command(BaseCommand): portfolio, created = self.get_or_create_portfolio(federal_agency) if skip_existing_portfolios and not created: message = ( - "Skipping modifications to suborgs, domain requests, and " - "domains due to the --skip_existing_portfolios flag. Portfolio already exists." + f"Portfolio '{portfolio}' already exists." + "Skipping modifications to suborgs, domain requests, " + "domains, and mangers due to the --skip_existing_portfolios flag. " ) logger.warning(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") else: + # TODO - Split this into two for loops? Could include post process + # Maybe post process steps should be options for the script? # if parse_suborganizations self.create_suborganizations(portfolio, federal_agency) if parse_domains: @@ -141,11 +144,11 @@ class Command(BaseCommand): portfolios.append(portfolio) if parse_managers: self.handle_portfolio_managers(portfolio) + # for portfolio in portfolios: - # POST PROCESS STEP: Add additional suborg info where applicable. - updated_suborg_count = self.post_process_all_suborganization_fields(agencies) - message = f"Added city and state_territory information to {updated_suborg_count} suborgs." - TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) + # == POST PROCESS STEPS == # + # Post process step: Add additional suborg info where applicable. + self.post_process_all_suborganization_fields(agencies) TerminalHelper.log_script_run_summary( **vars(self.portfolio_changes), debug=False, @@ -551,7 +554,7 @@ class Command(BaseCommand): if not domain and not request: message = f"Skipping adding city / state_territory information to suborg: {suborg}. Bad data." - TerminalHelper.colorful_logger(logger.warning, TerminalColors.YELLOW, message) + logger.warning(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") return # PRIORITY: @@ -567,9 +570,3 @@ class Command(BaseCommand): elif request and use_location_for_request: suborg.city = normalize_string(request.city, lowercase=False) suborg.state_territory = request.state_territory - - message = ( - f"Added city/state_territory to suborg: {suborg}. " - f"city - {suborg.city}, state - {suborg.state_territory}" - ) - TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, message) From bd7fc0265596239ba9e8fd7633bb01f930d8c6c3 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 18 Mar 2025 13:27:03 -0600 Subject: [PATCH 13/27] cleanup --- .../commands/create_federal_portfolio.py | 197 +++++++++++------- .../commands/utility/terminal_helper.py | 13 +- 2 files changed, 130 insertions(+), 80 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 56046d3eb..6d2c95e03 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -3,9 +3,8 @@ import argparse import logging from django.core.management import BaseCommand, CommandError -from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper, ScriptChangeTracker +from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper from registrar.models import DomainInformation, DomainRequest, FederalAgency, Suborganization, Portfolio, User -from registrar.models.domain import Domain from registrar.models.domain_invitation import DomainInvitation from registrar.models.portfolio_invitation import PortfolioInvitation from registrar.models.user_domain_role import UserDomainRole @@ -28,6 +27,25 @@ class Command(BaseCommand): self.update = [] self.skip = [] self.fail = [] + + def print_script_run_summary(self, no_changes_message, **kwargs): + """Helper function that runs TerminalHelper.log_script_run_summary on this object.""" + if self.has_changes(): + TerminalHelper.log_script_run_summary( + **vars(self), + **kwargs + ) + else: + logger.info(f"{TerminalColors.BOLD}{no_changes_message}{TerminalColors.ENDC}") + + def has_changes(self) -> bool: + num_changes = [ + len(self.add), + len(self.update), + len(self.skip), + len(self.fail) + ] + return any([num_change > 0 for num_change in num_changes]) def __init__(self, *args, **kwargs): """Defines fields to track what portfolios were updated, skipped, or just outright failed.""" @@ -92,6 +110,12 @@ class Command(BaseCommand): help="Only add suborganizations to newly created portfolios, skip existing ones.", ) + parser.add_argument( + "--debug", + action=argparse.BooleanOptionalAction, + help="Shows additional log info.", + ) + def handle(self, **options): # noqa: C901 agency_name = options.get("agency_name") branch = options.get("branch") @@ -99,6 +123,7 @@ class Command(BaseCommand): parse_domains = options.get("parse_domains") parse_managers = options.get("parse_managers") skip_existing_portfolios = options.get("skip_existing_portfolios") + debug = options.get("debug") # Parse script params if not (parse_requests or parse_domains or parse_managers): @@ -120,8 +145,6 @@ class Command(BaseCommand): # Parse portfolios portfolios = [] for federal_agency in agencies: - message = f"Processing federal agency '{federal_agency.agency}'..." - logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") portfolio, created = self.get_or_create_portfolio(federal_agency) if skip_existing_portfolios and not created: message = ( @@ -131,9 +154,6 @@ class Command(BaseCommand): ) logger.warning(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") else: - # TODO - Split this into two for loops? Could include post process - # Maybe post process steps should be options for the script? - # if parse_suborganizations self.create_suborganizations(portfolio, federal_agency) if parse_domains: self.handle_portfolio_domains(portfolio, federal_agency) @@ -141,40 +161,40 @@ class Command(BaseCommand): if parse_requests: self.handle_portfolio_requests(portfolio, federal_agency) - portfolios.append(portfolio) if parse_managers: self.handle_portfolio_managers(portfolio) - # for portfolio in portfolios: + portfolios.append(portfolio) + # == Mass create/update records == # + # Create suborganizations + try: + suborg_add_count = len(self.suborganization_changes.add) + if suborg_add_count > 0: + Suborganization.objects.bulk_create(self.suborganization_changes.add) + message = f"Added {suborg_add_count} suborganizations to portfolio." + logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") + else: + message = f"No suborganizations added for '{portfolio}'." + logger.warning(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") + except Exception as err: + # In this case, just swap the fail and add lists + self.suborganization_changes.fail = self.suborganization_changes.add.copy() + self.suborganization_changes.add = [] + message = f"Error when adding suborganizations to '{portfolio}': {err}" + logger.error(f"{TerminalColors.FAIL}{message}{TerminalColors.ENDC}") + # Update DomainInformation + DomainInformation.objects.bulk_update(self.domain_info_changes.add, ["portfolio", "sub_organization"]) + message = f"Added {len(domain_infos)} domains to portfolio." + logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") + + # Update DomainRequest + # Create UserPortfolioPermission + # Create PortfolioInvitation # == POST PROCESS STEPS == # # Post process step: Add additional suborg info where applicable. self.post_process_all_suborganization_fields(agencies) - TerminalHelper.log_script_run_summary( - **vars(self.portfolio_changes), - debug=False, - log_header="============= FINISHED HANDLE PORTFOLIO STEP ===============", - skipped_header="----- SOME PORTFOLIOS WERENT CREATED (BUT OTHER RECORDS ARE STILL PROCESSED) -----", - display_as_str=True, - ) - if parse_managers: - TerminalHelper.log_script_run_summary( - **vars(self.user_domain_role_changes), - log_header="----- MANAGERS ADDED -----", - debug=False, - display_as_str=True, - ) - - TerminalHelper.log_script_run_summary( - **vars(self.portfolio_invitation_changes), - log_header="----- INVITATIONS ADDED -----", - debug=False, - skipped_header="----- INVITATIONS SKIPPED (ALREADY EXISTED) -----", - display_as_str=True, - ) - - # POST PROCESSING STEP: Remove the federal agency if it matches the portfolio name. - # We only do this for started domain requests. + # Post processing step: Remove the federal agency if it matches the portfolio name. if parse_requests: prompt_message = ( "This action will update domain requests even if they aren't on a portfolio." @@ -193,6 +213,54 @@ class Command(BaseCommand): ) self.post_process_started_domain_requests(agencies, portfolios) + # == PRINT RUN SUMMARY == # + self.portfolio_changes.print_script_run_summary( + no_changes_message="\n============= No portfolios changed. =============", + debug=debug, + log_header="||============= PORTFOLIOS =============||", + skipped_header="----- SOME PORTFOLIOS WERENT CREATED (BUT OTHER RECORDS ARE STILL PROCESSED) -----", + display_as_str=True, + ) + self.suborganization_changes.print_script_run_summary( + no_changes_message="\n============= No suborganizations changed. =============", + debug=debug, + log_header="============= SUBORGANIZATIONS =============", + skipped_header="----- SUBORGANIZATIONS SKIPPED (SAME NAME AS PORTFOLIO NAME) -----", + display_as_str=True, + ) + + if parse_domains: + self.domain_info_changes.print_script_run_summary( + no_changes_message="\n============= No domains changed. =============", + debug=debug, + log_header="============= DOMAINS =============", + display_as_str=True, + ) + + if parse_requests: + self.domain_request_changes.print_script_run_summary( + no_changes_message="\n============= No domain requests changed. =============", + debug=debug, + log_header="============= DOMAIN REQUESTS =============", + display_as_str=True, + ) + + if parse_managers: + self.user_domain_role_changes.print_script_run_summary( + no_changes_message="\n============= No managers changed. =============", + log_header="============= MANAGERS =============", + skipped_header="----- MANAGERS SKIPPED (ALREADY EXISTED) -----", + debug=debug, + display_as_str=True, + ) + self.portfolio_invitation_changes.print_script_run_summary( + no_changes_message="\n============= No manager invitations changed. =============", + log_header="============= MANAGER INVITATIONS =============", + debug=debug, + skipped_header="----- INVITATIONS SKIPPED (ALREADY EXISTED) -----", + display_as_str=True, + ) + def get_or_create_portfolio(self, federal_agency): portfolio_name = normalize_string(federal_agency.agency, lowercase=False) portfolio, created = Portfolio.objects.get_or_create( @@ -209,16 +277,13 @@ class Command(BaseCommand): logger.info(f"{TerminalColors.OKGREEN}Created portfolio '{portfolio}'.{TerminalColors.ENDC}") else: self.skipped_portfolios.add(portfolio) - message = f"Portfolio with organization name '{portfolio}' already exists. Skipping create." - logger.info(f"{TerminalColors.OKGREEN}{message}{TerminalColors.ENDC}") + message = f"Portfolio '{portfolio}' already exists. Skipping create." + logger.info(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") return portfolio, created def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency): """Create Suborganizations tied to the given portfolio based on DomainInformation objects""" - message = f"Creating suborgs for portfolio {portfolio}." - logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") - valid_agencies = federal_agency.domaininformation_set.filter(organization_name__isnull=False) org_names = set(valid_agencies.values_list("organization_name", flat=True)) existing_org_names = set( @@ -239,15 +304,6 @@ class Command(BaseCommand): suborg = Suborganization(name=suborg_name, portfolio=portfolio) self.suborganization_changes.add.append(suborg) - suborg_add_count = len(self.suborganization_changes.add) - if suborg_add_count > 0: - Suborganization.objects.bulk_create(self.suborganization_changes.add) - message = f"Added {suborg_add_count} suborganizations to '{federal_agency}'." - logger.info(f"{TerminalColors.OKGREEN}{message}{TerminalColors.ENDC}") - else: - message = f"No suborganizations added for '{federal_agency}'." - logger.warning(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") - def handle_portfolio_domains(self, portfolio: Portfolio, federal_agency: FederalAgency): """ Associate portfolio with domains for a federal agency. @@ -255,9 +311,6 @@ class Command(BaseCommand): Returns a queryset of DomainInformation objects, or None if nothing changed. """ - message = f"Adding domains to portfolio {portfolio}." - logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") - domain_infos = federal_agency.domaininformation_set.all() if not domain_infos.exists(): message = f""" @@ -275,18 +328,11 @@ class Command(BaseCommand): domain_info.sub_organization = suborgs.get(org_name, None) self.domain_info_changes.update.append(domain_info) - DomainInformation.objects.bulk_update(domain_infos, ["portfolio", "sub_organization"]) - message = f"Added portfolio '{portfolio}' to {len(domain_infos)} domains." - logger.info(f"{TerminalColors.OKGREEN}{message}{TerminalColors.ENDC}") - 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. """ - message = f"Adding domain requests to portfolio {portfolio}." - logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") - invalid_states = [ DomainRequest.DomainRequestStatus.STARTED, DomainRequest.DomainRequestStatus.INELIGIBLE, @@ -327,29 +373,31 @@ class Command(BaseCommand): ], ) message = f"Added portfolio '{portfolio}' to {len(domain_requests)} domain requests." - logger.info(f"{TerminalColors.OKGREEN}{message}{TerminalColors.ENDC}") + logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") - # TODO - this doesn't send an email out, should it? def handle_portfolio_managers(self, portfolio: Portfolio): """ Add all domain managers of the portfolio's domains to the organization. This includes adding them to the correct group and creating portfolio invitations. """ - message = f"Adding managers to portfolio {portfolio}." - logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") - domains = portfolio.information_portfolio.all().values_list("domain", flat=True) + self.user_domain_role_changes.add = [] + self.user_domain_role_changes.skip = [] + self.portfolio_invitation_changes.add = [] + self.portfolio_invitation_changes.skip = [] # Fetch all users with manager roles for the domains user_domain_roles = UserDomainRole.objects.select_related("user").filter( domain__in=domains, role=UserDomainRole.Roles.MANAGER ) existing_permissions = UserPortfolioPermission.objects.filter( - user__in=user_domain_roles.values_list("user") - ).in_bulk(field_name="user") + user__in=user_domain_roles.values_list("user"), + portfolio=portfolio + ) + existing_permissions_dict = {permission.user: permission for permission in existing_permissions} for user_domain_role in user_domain_roles: user = user_domain_role.user - if user not in existing_permissions: + if user not in existing_permissions_dict: permission = UserPortfolioPermission( portfolio=portfolio, user=user, @@ -358,24 +406,25 @@ class Command(BaseCommand): self.user_domain_role_changes.add.append(permission) logger.info(f"Added manager '{permission.user}' to portfolio '{portfolio}'.") else: - existing_permission = existing_permissions.get(user) + existing_permission = existing_permissions_dict.get(user) self.user_domain_role_changes.skip.append(existing_permission) - logger.info(f"Manager '{permission.user}' already exists in portfolio '{portfolio}'.") + logger.info(f"Manager '{user}' already exists on portfolio '{portfolio}'.") # Bulk create user portfolio permissions UserPortfolioPermission.objects.bulk_create(self.user_domain_role_changes.add) - # TODO - needs normalize step # Get the emails of invited managers domain_invitations = DomainInvitation.objects.filter( domain__in=domains, status=DomainInvitation.DomainInvitationStatus.INVITED ) existing_invitations = PortfolioInvitation.objects.filter( - email__in=domain_invitations.values_list("email") - ).in_bulk(field_name="user") + email__in=domain_invitations.values_list("email"), + portfolio=portfolio + ) + existing_invitation_dict = {normalize_string(invite.email): invite for invite in existing_invitations} for domain_invitation in domain_invitations: - email = domain_invitation.email - if email not in existing_invitations: + email = normalize_string(domain_invitation.email) + if email not in existing_invitation_dict: invitation = PortfolioInvitation( portfolio=portfolio, email=email, @@ -390,7 +439,7 @@ class Command(BaseCommand): logger.info(f"Invitation '{email}' already exists in portfolio '{portfolio}'.") # Bulk create portfolio invitations - PortfolioInvitation.objects.bulk_create(self.user_domain_role_changes.add) + PortfolioInvitation.objects.bulk_create(self.portfolio_invitation_changes.add) def post_process_started_domain_requests(self, agencies, portfolios): """ diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index 5c9ddade2..32385e214 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -200,10 +200,11 @@ class TerminalHelper: skip, fail, debug: bool, - log_header="============= FINISHED ===============", + log_header="============= FINISHED =============", skipped_header="----- SOME DATA WAS INVALID (NEEDS MANUAL PATCHING) -----", failed_header="----- UPDATE FAILED -----", display_as_str=False, + detailed_prompt_title="Do you wish to see the full list of failed, skipped and updated records?" ): """Generates a formatted summary of script execution results with colored output. @@ -247,9 +248,9 @@ class TerminalHelper: if not debug and counts["failed"] > 0 or counts["skipped"] > 0: display_detailed_logs = TerminalHelper.prompt_for_execution( system_exit_on_terminate=False, - prompt_message=f'You will see {counts["failed"]} failed and {counts["updated"]} skipped records.', + prompt_message=f'You will see {counts["failed"]} failed and {counts["skipped"]} skipped records.', verify_message="** Some records were skipped, or some failed to update. **", - prompt_title="Do you wish to see the full list of failed, skipped and updated records?", + prompt_title=detailed_prompt_title, ) non_zero_counts = {category: count for category, count in counts.items() if count > 0} @@ -272,11 +273,11 @@ class TerminalHelper: if debug or display_detailed_logs: display_values = [str(v) for v in values] if display_as_str else values debug_message = f"{label}: {display_values}" - TerminalHelper.colorful_logger(logger.info, debug_color, debug_message) + logger.info(f"{debug_color}{debug_message}{TerminalColors.ENDC}") - final_message = f"\n{log_header}" + "\n".join(messages) + final_message = f"\n{log_header}\n" + "\n".join(messages) if counts["failed"] > 0: - TerminalHelper.colorful_logger("ERROR", "FAIL", final_message) + logger.error(f"{TerminalColors.FAIL}{final_message}{TerminalColors.ENDC}") elif counts["skipped"] > 0: TerminalHelper.colorful_logger("WARNING", "YELLOW", final_message) else: From 5bcf255c36de405d586fa79a3ac9ce093575072a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 19 Mar 2025 13:45:58 -0600 Subject: [PATCH 14/27] Further refactor --- .../commands/create_federal_portfolio.py | 369 ++++++++++-------- .../commands/utility/terminal_helper.py | 33 +- 2 files changed, 247 insertions(+), 155 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 6d2c95e03..983368f56 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -3,7 +3,7 @@ import argparse import logging from django.core.management import BaseCommand, CommandError -from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper +from registrar.management.commands.utility.terminal_helper import ScriptDataHelper, TerminalColors, TerminalHelper from registrar.models import DomainInformation, DomainRequest, FederalAgency, Suborganization, Portfolio, User from registrar.models.domain_invitation import DomainInvitation from registrar.models.portfolio_invitation import PortfolioInvitation @@ -22,7 +22,8 @@ class Command(BaseCommand): help = "Creates a federal portfolio given a FederalAgency name" class ChangeTracker: - def __init__(self): + def __init__(self, model_class): + self.model_class = model_class self.add = [] self.update = [] self.skip = [] @@ -32,7 +33,10 @@ class Command(BaseCommand): """Helper function that runs TerminalHelper.log_script_run_summary on this object.""" if self.has_changes(): TerminalHelper.log_script_run_summary( - **vars(self), + self.add, + self.update, + self.skip, + self.fail, **kwargs ) else: @@ -47,6 +51,33 @@ class Command(BaseCommand): ] return any([num_change > 0 for num_change in num_changes]) + def bulk_create(self): + try: + ScriptDataHelper.bulk_create_fields( + self.model_class, + self.add, + quiet=True + ) + except Exception as err: + # In this case, just swap the fail and add lists + self.fail = self.add.copy() + self.add = [] + raise err + + def bulk_update(self, fields_to_update): + try: + ScriptDataHelper.bulk_update_fields( + self.model_class, + self.update, + fields_to_update, + quiet=True + ) + except Exception as err: + # In this case, just swap the fail and update lists + self.fail = self.update.copy() + self.update = [] + raise err + def __init__(self, *args, **kwargs): """Defines fields to track what portfolios were updated, skipped, or just outright failed.""" super().__init__(*args, **kwargs) @@ -57,12 +88,12 @@ class Command(BaseCommand): self.added_invitations = set() self.skipped_invitations = set() self.failed_managers = set() - self.portfolio_changes = self.ChangeTracker() - self.suborganization_changes = self.ChangeTracker() - self.domain_info_changes = self.ChangeTracker() - self.domain_request_changes = self.ChangeTracker() - self.user_domain_role_changes = self.ChangeTracker() - self.portfolio_invitation_changes = self.ChangeTracker() + self.portfolio_changes = self.ChangeTracker(model_class=Portfolio) + self.suborganization_changes = self.ChangeTracker(model_class=Suborganization) + self.domain_info_changes = self.ChangeTracker(model_class=DomainInformation) + self.domain_request_changes = self.ChangeTracker(model_class=DomainRequest) + self.user_portfolio_perm_changes = self.ChangeTracker(model_class=UserPortfolioPermission) + self.portfolio_invitation_changes = self.ChangeTracker(model_class=PortfolioInvitation) def add_arguments(self, parser): """Add command line arguments to create federal portfolios. @@ -131,7 +162,7 @@ class Command(BaseCommand): # Get agencies federal_agency_filter = {"agency__iexact": agency_name} if agency_name else {"federal_type": branch} - agencies = FederalAgency.objects.filter(**federal_agency_filter) + agencies = FederalAgency.objects.filter(**federal_agency_filter).distinct() if not agencies.exists(): if agency_name: raise CommandError( @@ -143,8 +174,10 @@ class Command(BaseCommand): raise CommandError(f"Cannot find '{branch}' federal agencies in our database.") # Parse portfolios - portfolios = [] - for federal_agency in agencies: + # TODO - some kind of duplicate check + agencies_set = {normalize_string(agency.agency): agency for agency in agencies} + portfolios = set() + for federal_agency in agencies_set.values(): portfolio, created = self.get_or_create_portfolio(federal_agency) if skip_existing_portfolios and not created: message = ( @@ -154,46 +187,26 @@ class Command(BaseCommand): ) logger.warning(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") else: - self.create_suborganizations(portfolio, federal_agency) - if parse_domains: - self.handle_portfolio_domains(portfolio, federal_agency) + portfolios.add(portfolio) - if parse_requests: - self.handle_portfolio_requests(portfolio, federal_agency) + # Add additional fields to portfolio + for portfolio in portfolios: + org_name = normalize_string(portfolio.organization_name) + federal_agency = agencies_set.get(org_name) + self.create_suborganizations(portfolio, federal_agency) + if parse_domains: + self.handle_portfolio_domains(portfolio, federal_agency) - if parse_managers: - self.handle_portfolio_managers(portfolio) - portfolios.append(portfolio) + if parse_requests: + self.handle_portfolio_requests(portfolio, federal_agency) + + if parse_managers: + self.handle_portfolio_managers(portfolio) # == Mass create/update records == # - # Create suborganizations - try: - suborg_add_count = len(self.suborganization_changes.add) - if suborg_add_count > 0: - Suborganization.objects.bulk_create(self.suborganization_changes.add) - message = f"Added {suborg_add_count} suborganizations to portfolio." - logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") - else: - message = f"No suborganizations added for '{portfolio}'." - logger.warning(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") - except Exception as err: - # In this case, just swap the fail and add lists - self.suborganization_changes.fail = self.suborganization_changes.add.copy() - self.suborganization_changes.add = [] - message = f"Error when adding suborganizations to '{portfolio}': {err}" - logger.error(f"{TerminalColors.FAIL}{message}{TerminalColors.ENDC}") - # Update DomainInformation - DomainInformation.objects.bulk_update(self.domain_info_changes.add, ["portfolio", "sub_organization"]) - message = f"Added {len(domain_infos)} domains to portfolio." - logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") + self.commit_changes_to_db() - # Update DomainRequest - # Create UserPortfolioPermission - # Create PortfolioInvitation # == POST PROCESS STEPS == # - # Post process step: Add additional suborg info where applicable. - self.post_process_all_suborganization_fields(agencies) - # Post processing step: Remove the federal agency if it matches the portfolio name. if parse_requests: prompt_message = ( @@ -203,17 +216,52 @@ class Command(BaseCommand): "the existing portfolios with your given params." "\nThis step is entirely optional, and is just for extra data cleanup." ) - TerminalHelper.prompt_for_execution( - system_exit_on_terminate=True, + if TerminalHelper.prompt_for_execution( + system_exit_on_terminate=False, prompt_message=prompt_message, prompt_title=( "POST PROCESS STEP: Do you want to clear federal agency on (related) started domain requests?" ), verify_message="*** THIS STEP IS OPTIONAL ***", - ) - self.post_process_started_domain_requests(agencies, portfolios) + ): + self.post_process_started_domain_requests(agencies, portfolios) # == PRINT RUN SUMMARY == # + self.print_final_run_summary(parse_domains, parse_requests, parse_managers, debug) + + def commit_changes_to_db(self): + # Create suborganizations + self.suborganization_changes.bulk_create() + message = f"Added {len(self.suborganization_changes.add)} suborganizations to portfolios." + logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") + + # Update DomainInformation + self.domain_info_changes.bulk_update(["portfolio", "sub_organization"]) + message = f"Added {len(self.suborganization_changes.update)} domains to portfolios." + logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") + + # Update DomainRequest + self.domain_request_changes.bulk_update([ + "portfolio", + "sub_organization", + "requested_suborganization", + "suborganization_city", + "suborganization_state_territory", + ]) + message = f"Added {len(self.domain_request_changes.update)} domain requests to portfolios." + logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") + + # Create UserPortfolioPermission + self.user_portfolio_perm_changes.bulk_create() + message = f"Added {len(self.user_portfolio_perm_changes.add)} managers to portfolios." + logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") + + # Create PortfolioInvitation + self.portfolio_invitation_changes.bulk_create() + message = f"Added {len(self.portfolio_invitation_changes.add)} manager invitations to portfolios." + logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") + + def print_final_run_summary(self, parse_domains, parse_requests, parse_managers, debug): self.portfolio_changes.print_script_run_summary( no_changes_message="\n============= No portfolios changed. =============", debug=debug, @@ -246,7 +294,7 @@ class Command(BaseCommand): ) if parse_managers: - self.user_domain_role_changes.print_script_run_summary( + self.user_portfolio_perm_changes.print_script_run_summary( no_changes_message="\n============= No managers changed. =============", log_header="============= MANAGERS =============", skipped_header="----- MANAGERS SKIPPED (ALREADY EXISTED) -----", @@ -284,8 +332,13 @@ class Command(BaseCommand): def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency): """Create Suborganizations tied to the given portfolio based on DomainInformation objects""" - valid_agencies = federal_agency.domaininformation_set.filter(organization_name__isnull=False) - org_names = set(valid_agencies.values_list("organization_name", flat=True)) + base_filter = Q( + organization_name__isnull=False, + ) & ~Q(organization_name__iexact=F("portfolio__organization_name")) + domains = federal_agency.domaininformation_set.filter(base_filter) + requests = federal_agency.domainrequest_set.filter(base_filter) + + org_names = set(domains.values_list("organization_name", flat=True)) existing_org_names = set( Suborganization.objects .filter(name__in=org_names) @@ -300,9 +353,113 @@ class Command(BaseCommand): ) logger.warning(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") else: - suborg_name = normalize_string(name) - suborg = Suborganization(name=suborg_name, portfolio=portfolio) - self.suborganization_changes.add.append(suborg) + suborg = Suborganization(name=name, portfolio=portfolio) + # TODO - change this portion + if suborg.name not in [org.name for org in self.suborganization_changes.add]: + self.suborganization_changes.add.append(suborg) + + # Add location information to suborgs. + # This can vary per domain and request, so this is a seperate step. + # First: Filter domains and requests by those that have data + valid_domains = domains.filter( + city__isnull=False, + state_territory__isnull=False, + portfolio__isnull=False, + sub_organization__isnull=False, + ) + valid_requests = requests.filter( + ( + Q(city__isnull=False, state_territory__isnull=False) + | Q(suborganization_city__isnull=False, suborganization_state_territory__isnull=False) + ), + portfolio__isnull=False, + sub_organization__isnull=False, + ) + + # Second: Group domains and requests by normalized organization name. + # This means that later down the line we can account for "duplicate" org names. + domains_dict = {} + requests_dict = {} + for domain in valid_domains: + normalized_name = normalize_string(domain.organization_name) + domains_dict.setdefault(normalized_name, []).append(domain) + + for request in valid_requests: + normalized_name = normalize_string(request.organization_name) + requests_dict.setdefault(normalized_name, []).append(request) + + # Fourth: Process each suborg to add city / state territory info + for suborg in self.suborganization_changes.add: + self.set_suborganization_location(suborg, domains_dict, requests_dict) + + def set_suborganization_location(self, suborg, domains_dict, requests_dict): + """Updates a single suborganization's location data if valid. + + Args: + suborg: Suborganization to update + domains_dict: Dict of domain info records grouped by org name + requests_dict: Dict of domain requests grouped by org name + + Priority matches parent method. Updates are skipped if location data conflicts + between multiple records of the same type. + """ + normalized_suborg_name = normalize_string(suborg.name) + domains = domains_dict.get(normalized_suborg_name, []) + requests = requests_dict.get(normalized_suborg_name, []) + + # Try to get matching domain info + domain = None + if domains: + reference = domains[0] + use_location_for_domain = all( + d.city == reference.city and d.state_territory == reference.state_territory for d in domains + ) + if use_location_for_domain: + domain = reference + + # Try to get matching request info + # Uses consensus: if all city / state_territory info matches, then we can assume the data is "good". + # If not, take the safe route and just skip updating this particular record. + request = None + use_suborg_location_for_request = True + use_location_for_request = True + if requests: + reference = requests[0] + use_suborg_location_for_request = all( + r.suborganization_city + and r.suborganization_state_territory + and r.suborganization_city == reference.suborganization_city + and r.suborganization_state_territory == reference.suborganization_state_territory + for r in requests + ) + use_location_for_request = all( + r.city + and r.state_territory + and r.city == reference.city + and r.state_territory == reference.state_territory + for r in requests + ) + if use_suborg_location_for_request or use_location_for_request: + request = reference + + if not domain and not request: + message = f"Skipping adding city / state_territory information to suborg: {suborg}. Bad data." + logger.warning(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") + return + + # PRIORITY: + # 1. Domain info + # 2. Domain request requested suborg fields + # 3. Domain request normal fields + if domain: + suborg.city = normalize_string(domain.city, lowercase=False) + suborg.state_territory = domain.state_territory + elif request and use_suborg_location_for_request: + suborg.city = normalize_string(request.suborganization_city, lowercase=False) + suborg.state_territory = request.suborganization_state_territory + elif request and use_location_for_request: + suborg.city = normalize_string(request.city, lowercase=False) + suborg.state_territory = request.state_territory def handle_portfolio_domains(self, portfolio: Portfolio, federal_agency: FederalAgency): """ @@ -360,20 +517,7 @@ class Command(BaseCommand): ) domain_request.suborganization_city = normalize_string(domain_request.city, lowercase=False) domain_request.suborganization_state_territory = domain_request.state_territory - self.domain_request_changes.add.append(portfolio) - - 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." - logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") + self.domain_request_changes.update.append(domain_request) def handle_portfolio_managers(self, portfolio: Portfolio): """ @@ -381,10 +525,6 @@ class Command(BaseCommand): This includes adding them to the correct group and creating portfolio invitations. """ domains = portfolio.information_portfolio.all().values_list("domain", flat=True) - self.user_domain_role_changes.add = [] - self.user_domain_role_changes.skip = [] - self.portfolio_invitation_changes.add = [] - self.portfolio_invitation_changes.skip = [] # Fetch all users with manager roles for the domains user_domain_roles = UserDomainRole.objects.select_related("user").filter( @@ -403,16 +543,13 @@ class Command(BaseCommand): user=user, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], ) - self.user_domain_role_changes.add.append(permission) + self.user_portfolio_perm_changes.add.append(permission) logger.info(f"Added manager '{permission.user}' to portfolio '{portfolio}'.") else: existing_permission = existing_permissions_dict.get(user) - self.user_domain_role_changes.skip.append(existing_permission) + self.user_portfolio_perm_changes.skip.append(existing_permission) logger.info(f"Manager '{user}' already exists on portfolio '{portfolio}'.") - # Bulk create user portfolio permissions - UserPortfolioPermission.objects.bulk_create(self.user_domain_role_changes.add) - # Get the emails of invited managers domain_invitations = DomainInvitation.objects.filter( domain__in=domains, status=DomainInvitation.DomainInvitationStatus.INVITED @@ -438,9 +575,6 @@ class Command(BaseCommand): self.portfolio_invitation_changes.skip.append(existing_invitation) logger.info(f"Invitation '{email}' already exists in portfolio '{portfolio}'.") - # Bulk create portfolio invitations - PortfolioInvitation.objects.bulk_create(self.portfolio_invitation_changes.add) - def post_process_started_domain_requests(self, agencies, portfolios): """ Removes duplicate organization data by clearing federal_agency when it matches the portfolio name. @@ -546,76 +680,7 @@ class Command(BaseCommand): # Fourth: Process each suborg to add city / state territory info for suborg in suborgs_to_edit: - self.post_process_suborganization_fields(suborg, domains_dict, requests_dict) + self.set_suborganization_location(suborg, domains_dict, requests_dict) # Fifth: Perform a bulk update return Suborganization.objects.bulk_update(suborgs_to_edit, ["city", "state_territory"]) - - def post_process_suborganization_fields(self, suborg, domains_dict, requests_dict): - """Updates a single suborganization's location data if valid. - - Args: - suborg: Suborganization to update - domains_dict: Dict of domain info records grouped by org name - requests_dict: Dict of domain requests grouped by org name - - Priority matches parent method. Updates are skipped if location data conflicts - between multiple records of the same type. - """ - normalized_suborg_name = normalize_string(suborg.name) - domains = domains_dict.get(normalized_suborg_name, []) - requests = requests_dict.get(normalized_suborg_name, []) - - # Try to get matching domain info - domain = None - if domains: - reference = domains[0] - use_location_for_domain = all( - d.city == reference.city and d.state_territory == reference.state_territory for d in domains - ) - if use_location_for_domain: - domain = reference - - # Try to get matching request info - # Uses consensus: if all city / state_territory info matches, then we can assume the data is "good". - # If not, take the safe route and just skip updating this particular record. - request = None - use_suborg_location_for_request = True - use_location_for_request = True - if requests: - reference = requests[0] - use_suborg_location_for_request = all( - r.suborganization_city - and r.suborganization_state_territory - and r.suborganization_city == reference.suborganization_city - and r.suborganization_state_territory == reference.suborganization_state_territory - for r in requests - ) - use_location_for_request = all( - r.city - and r.state_territory - and r.city == reference.city - and r.state_territory == reference.state_territory - for r in requests - ) - if use_suborg_location_for_request or use_location_for_request: - request = reference - - if not domain and not request: - message = f"Skipping adding city / state_territory information to suborg: {suborg}. Bad data." - logger.warning(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") - return - - # PRIORITY: - # 1. Domain info - # 2. Domain request requested suborg fields - # 3. Domain request normal fields - if domain: - suborg.city = normalize_string(domain.city, lowercase=False) - suborg.state_territory = domain.state_territory - elif request and use_suborg_location_for_request: - suborg.city = normalize_string(request.suborganization_city, lowercase=False) - suborg.state_territory = request.suborganization_state_territory - elif request and use_location_for_request: - suborg.city = normalize_string(request.city, lowercase=False) - suborg.state_territory = request.state_territory diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index 32385e214..63fc9e681 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -32,7 +32,7 @@ class ScriptDataHelper: """Helper method with utilities to speed up development of scripts that do DB operations""" @staticmethod - def bulk_update_fields(model_class, update_list, fields_to_update, batch_size=1000): + def bulk_update_fields(model_class, update_list, fields_to_update, batch_size=1000, quiet=False): """ This function performs a bulk update operation on a specified Django model class in batches. It uses Django's Paginator to handle large datasets in a memory-efficient manner. @@ -51,15 +51,42 @@ class ScriptDataHelper: fields_to_update: Specifies which fields to update. Usage: - bulk_update_fields(Domain, page.object_list, ["first_ready"]) + ScriptDataHelper.bulk_update_fields(Domain, page.object_list, ["first_ready"]) """ - logger.info(f"{TerminalColors.YELLOW} Bulk updating fields... {TerminalColors.ENDC}") + if not quiet: + logger.info(f"{TerminalColors.YELLOW} Bulk updating fields... {TerminalColors.ENDC}") # Create a Paginator object. Bulk_update on the full dataset # is too memory intensive for our current app config, so we can chunk this data instead. paginator = Paginator(update_list, batch_size) for page_num in paginator.page_range: page = paginator.page(page_num) model_class.objects.bulk_update(page.object_list, fields_to_update) + + @staticmethod + def bulk_create_fields(model_class, update_list, batch_size=1000, quiet=False): + """ + This function performs a bulk create operation on a specified Django model class in batches. + It uses Django's Paginator to handle large datasets in a memory-efficient manner. + + Parameters: + model_class: The Django model class that you want to perform the bulk update on. + This should be the actual class, not a string of the class name. + + update_list: A list of model instances that you want to update. Each instance in the list + should already have the updated values set on the instance. + + batch_size: The maximum number of model instances to update in a single database query. + Defaults to 1000. If you're dealing with models that have a large number of fields, + or large field values, you may need to decrease this value to prevent out-of-memory errors. + Usage: + ScriptDataHelper.bulk_add_fields(Domain, page.object_list) + """ + if not quiet: + logger.info(f"{TerminalColors.YELLOW} Bulk adding fields... {TerminalColors.ENDC}") + paginator = Paginator(update_list, batch_size) + for page_num in paginator.page_range: + page = paginator.page(page_num) + model_class.objects.bulk_create(page.object_list) class PopulateScriptTemplate(ABC): From d7de1f6093a81b882e174e5ab124791e1a0e96f9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 19 Mar 2025 14:05:23 -0600 Subject: [PATCH 15/27] Consolidate post process logic --- .../commands/create_federal_portfolio.py | 186 ++++-------------- 1 file changed, 37 insertions(+), 149 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 983368f56..51c181c11 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -173,7 +173,7 @@ class Command(BaseCommand): else: raise CommandError(f"Cannot find '{branch}' federal agencies in our database.") - # Parse portfolios + # == Handle portfolios == # # TODO - some kind of duplicate check agencies_set = {normalize_string(agency.agency): agency for agency in agencies} portfolios = set() @@ -189,11 +189,22 @@ class Command(BaseCommand): else: portfolios.add(portfolio) - # Add additional fields to portfolio + # == Handle suborganizations == # for portfolio in portfolios: org_name = normalize_string(portfolio.organization_name) federal_agency = agencies_set.get(org_name) self.create_suborganizations(portfolio, federal_agency) + + # Create suborganizations + self.suborganization_changes.bulk_create() + message = f"Added {len(self.suborganization_changes.add)} suborganizations to portfolios." + logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") + + # == Handle domains, requests, and managers == # + for portfolio in portfolios: + org_name = normalize_string(portfolio.organization_name) + federal_agency = agencies_set.get(org_name) + if parse_domains: self.handle_portfolio_domains(portfolio, federal_agency) @@ -203,38 +214,6 @@ class Command(BaseCommand): if parse_managers: self.handle_portfolio_managers(portfolio) - # == Mass create/update records == # - self.commit_changes_to_db() - - # == POST PROCESS STEPS == # - # Post processing step: Remove the federal agency if it matches the portfolio name. - if parse_requests: - prompt_message = ( - "This action will update domain requests even if they aren't on a portfolio." - "\nNOTE: This will modify domain requests, even if no portfolios were created." - "\nIn the event no portfolios *are* created, then this step will target " - "the existing portfolios with your given params." - "\nThis step is entirely optional, and is just for extra data cleanup." - ) - if TerminalHelper.prompt_for_execution( - system_exit_on_terminate=False, - prompt_message=prompt_message, - prompt_title=( - "POST PROCESS STEP: Do you want to clear federal agency on (related) started domain requests?" - ), - verify_message="*** THIS STEP IS OPTIONAL ***", - ): - self.post_process_started_domain_requests(agencies, portfolios) - - # == PRINT RUN SUMMARY == # - self.print_final_run_summary(parse_domains, parse_requests, parse_managers, debug) - - def commit_changes_to_db(self): - # Create suborganizations - self.suborganization_changes.bulk_create() - message = f"Added {len(self.suborganization_changes.add)} suborganizations to portfolios." - logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") - # Update DomainInformation self.domain_info_changes.bulk_update(["portfolio", "sub_organization"]) message = f"Added {len(self.suborganization_changes.update)} domains to portfolios." @@ -247,6 +226,7 @@ class Command(BaseCommand): "requested_suborganization", "suborganization_city", "suborganization_state_territory", + "federal_agency", ]) message = f"Added {len(self.domain_request_changes.update)} domain requests to portfolios." logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") @@ -261,6 +241,9 @@ class Command(BaseCommand): message = f"Added {len(self.portfolio_invitation_changes.add)} manager invitations to portfolios." logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") + # == PRINT RUN SUMMARY == # + self.print_final_run_summary(parse_domains, parse_requests, parse_managers, debug) + def print_final_run_summary(self, parse_domains, parse_requests, parse_managers, debug): self.portfolio_changes.print_script_run_summary( no_changes_message="\n============= No portfolios changed. =============", @@ -470,11 +453,6 @@ class Command(BaseCommand): """ domain_infos = federal_agency.domaininformation_set.all() if not domain_infos.exists(): - message = f""" - Portfolio '{portfolio}' not added to domains: no valid records found. - The filter on DomainInformation for the federal_agency '{federal_agency}' returned no results. - """ - logger.info(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") return None # Get all suborg information and store it in a dict to avoid doing a db call @@ -519,6 +497,26 @@ class Command(BaseCommand): domain_request.suborganization_state_territory = domain_request.state_territory self.domain_request_changes.update.append(domain_request) + # TODO - add this option as a FLAG to pass into the script directly + # For each STARTED 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. + started_domain_requests = federal_agency.domainrequest_set.filter( + status=DomainRequest.DomainRequestStatus.STARTED, + organization_name__isnull=False, + ) + + portfolio_name = normalize_string(portfolio.organization_name) + + # Update the request, assuming the given agency name matches the portfolio name + for domain_request in started_domain_requests: + agency_name = normalize_string(domain_request.federal_agency.agency) + if agency_name == portfolio_name: + domain_request.federal_agency = None + self.domain_request_changes.update.append(domain_request) + def handle_portfolio_managers(self, portfolio: Portfolio): """ Add all domain managers of the portfolio's domains to the organization. @@ -574,113 +572,3 @@ class Command(BaseCommand): existing_invitation = existing_invitations.get(email) self.portfolio_invitation_changes.skip.append(existing_invitation) logger.info(f"Invitation '{email}' already exists in portfolio '{portfolio}'.") - - 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. - """ - 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( - federal_agency__in=agencies, - federal_agency__agency__isnull=False, - status=DomainRequest.DomainRequestStatus.STARTED, - organization_name__isnull=False, - ) - - if domain_requests_to_update.count() == 0: - TerminalHelper.colorful_logger(logger.info, TerminalColors.MAGENTA, "No domain requests to update.") - return - - portfolio_set = {normalize_string(portfolio.organization_name) for portfolio in portfolios if portfolio} - - # Update the request, assuming the given agency name matches the portfolio name - updated_requests = [] - for req in domain_requests_to_update: - agency_name = normalize_string(req.federal_agency.agency) - if agency_name in portfolio_set: - req.federal_agency = None - updated_requests.append(req) - - # Execute the update and Log the results - if TerminalHelper.prompt_for_execution( - system_exit_on_terminate=False, - 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 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.") - - def post_process_all_suborganization_fields(self, agencies): - """Batch updates suborganization locations from domain and request data. - - Args: - agencies: List of FederalAgency objects to process - - Returns: - int: Number of suborganizations updated - - Priority for location data: - 1. Domain information - 2. Domain request suborganization fields - 3. Domain request standard fields - """ - # Common filter between domaininformation / domain request. - # Filter by only the agencies we've updated thus far. - # Then, only process records without null portfolio, org name, or suborg name. - base_filter = Q( - federal_agency__in=agencies, - portfolio__isnull=False, - organization_name__isnull=False, - sub_organization__isnull=False, - ) & ~Q(organization_name__iexact=F("portfolio__organization_name")) - - # First: Remove null city / state_territory values on domain info / domain requests. - # We want to add city data if there is data to add to begin with! - domains = DomainInformation.objects.filter( - base_filter, - Q(city__isnull=False, state_territory__isnull=False), - ) - requests = DomainRequest.objects.filter( - base_filter, - ( - Q(city__isnull=False, state_territory__isnull=False) - | Q(suborganization_city__isnull=False, suborganization_state_territory__isnull=False) - ), - ) - - # Second: Group domains and requests by normalized organization name. - # This means that later down the line we have to account for "duplicate" org names. - domains_dict = {} - requests_dict = {} - for domain in domains: - normalized_name = normalize_string(domain.organization_name) - domains_dict.setdefault(normalized_name, []).append(domain) - - for request in requests: - normalized_name = normalize_string(request.organization_name) - requests_dict.setdefault(normalized_name, []).append(request) - - # Third: Get suborganizations to update - suborgs_to_edit = Suborganization.objects.filter( - Q(id__in=domains.values_list("sub_organization", flat=True)) - | Q(id__in=requests.values_list("sub_organization", flat=True)) - ) - - # Fourth: Process each suborg to add city / state territory info - for suborg in suborgs_to_edit: - self.set_suborganization_location(suborg, domains_dict, requests_dict) - - # Fifth: Perform a bulk update - return Suborganization.objects.bulk_update(suborgs_to_edit, ["city", "state_territory"]) From 0dbc2d59c328e5631684f77458ae0f8c52193e10 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 19 Mar 2025 14:42:06 -0600 Subject: [PATCH 16/27] Update create_federal_portfolio.py --- .../management/commands/create_federal_portfolio.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 51c181c11..1affbe252 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -328,14 +328,7 @@ class Command(BaseCommand): .values_list("name", flat=True) ) for name in org_names - existing_org_names: - if normalize_string(name) == normalize_string(portfolio.organization_name): - self.suborganization_changes.skip.append(suborg) - message = ( - f"Skipping suborganization create on record '{name}'. " - "The federal agency name is the same as the portfolio name." - ) - logger.warning(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") - else: + if normalize_string(name) != normalize_string(portfolio.organization_name): suborg = Suborganization(name=name, portfolio=portfolio) # TODO - change this portion if suborg.name not in [org.name for org in self.suborganization_changes.add]: From b927f7f267172a7fb3f1981868244eafdea4f9c7 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 20 Mar 2025 11:21:11 -0600 Subject: [PATCH 17/27] tighten up logs --- .../commands/create_federal_portfolio.py | 222 ++++++++---------- .../commands/utility/terminal_helper.py | 27 ++- 2 files changed, 112 insertions(+), 137 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 1affbe252..7fa9c12e8 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -28,36 +28,21 @@ class Command(BaseCommand): self.update = [] self.skip = [] self.fail = [] - + def print_script_run_summary(self, no_changes_message, **kwargs): """Helper function that runs TerminalHelper.log_script_run_summary on this object.""" if self.has_changes(): - TerminalHelper.log_script_run_summary( - self.add, - self.update, - self.skip, - self.fail, - **kwargs - ) + TerminalHelper.log_script_run_summary(self.add, self.update, self.skip, self.fail, **kwargs) else: logger.info(f"{TerminalColors.BOLD}{no_changes_message}{TerminalColors.ENDC}") - + def has_changes(self) -> bool: - num_changes = [ - len(self.add), - len(self.update), - len(self.skip), - len(self.fail) - ] + num_changes = [len(self.add), len(self.update), len(self.skip), len(self.fail)] return any([num_change > 0 for num_change in num_changes]) def bulk_create(self): try: - ScriptDataHelper.bulk_create_fields( - self.model_class, - self.add, - quiet=True - ) + ScriptDataHelper.bulk_create_fields(self.model_class, self.add, quiet=True) except Exception as err: # In this case, just swap the fail and add lists self.fail = self.add.copy() @@ -66,12 +51,7 @@ class Command(BaseCommand): def bulk_update(self, fields_to_update): try: - ScriptDataHelper.bulk_update_fields( - self.model_class, - self.update, - fields_to_update, - quiet=True - ) + ScriptDataHelper.bulk_update_fields(self.model_class, self.update, fields_to_update, quiet=True) except Exception as err: # In this case, just swap the fail and update lists self.fail = self.update.copy() @@ -81,13 +61,6 @@ class Command(BaseCommand): def __init__(self, *args, **kwargs): """Defines fields to track what portfolios were updated, skipped, or just outright failed.""" super().__init__(*args, **kwargs) - self.updated_portfolios = set() - self.skipped_portfolios = set() - self.failed_portfolios = set() - self.added_managers = set() - self.added_invitations = set() - self.skipped_invitations = set() - self.failed_managers = set() self.portfolio_changes = self.ChangeTracker(model_class=Portfolio) self.suborganization_changes = self.ChangeTracker(model_class=Suborganization) self.domain_info_changes = self.ChangeTracker(model_class=DomainInformation) @@ -158,11 +131,13 @@ class Command(BaseCommand): # Parse script params if not (parse_requests or parse_domains or parse_managers): - raise CommandError("You must specify at least one of --parse_requests, --parse_domains, or --parse_managers.") + raise CommandError( + "You must specify at least one of --parse_requests, --parse_domains, or --parse_managers." + ) # Get agencies federal_agency_filter = {"agency__iexact": agency_name} if agency_name else {"federal_type": branch} - agencies = FederalAgency.objects.filter(**federal_agency_filter).distinct() + agencies = FederalAgency.objects.filter(agency__isnull=False, **federal_agency_filter).distinct() if not agencies.exists(): if agency_name: raise CommandError( @@ -173,146 +148,149 @@ class Command(BaseCommand): else: raise CommandError(f"Cannot find '{branch}' federal agencies in our database.") - # == Handle portfolios == # - # TODO - some kind of duplicate check + # == Handle portfolios and suborganizations == # + # TODO - some kind of duplicate check on agencies and existing portfolios + existing_portfolios = Portfolio.objects.filter( + organization_name__in=agencies.values_list("agency", flat=True), + organization_name__isnull=False + ) + existing_portfolios_set = {normalize_string(p.organization_name): p for p in existing_portfolios} agencies_set = {normalize_string(agency.agency): agency for agency in agencies} - portfolios = set() for federal_agency in agencies_set.values(): - portfolio, created = self.get_or_create_portfolio(federal_agency) - if skip_existing_portfolios and not created: - message = ( - f"Portfolio '{portfolio}' already exists." - "Skipping modifications to suborgs, domain requests, " - "domains, and mangers due to the --skip_existing_portfolios flag. " + portfolio_name = normalize_string(federal_agency.agency, lowercase=False) + portfolio = existing_portfolios_set.get(portfolio_name, None) + new_portfolio = portfolio is None + if new_portfolio: + portfolio = Portfolio( + organization_name=portfolio_name, + federal_agency=federal_agency, + organization_type=DomainRequest.OrganizationChoices.FEDERAL, + creator=User.get_default_user(), + notes="Auto-generated record", + senior_official=federal_agency.so_federal_agency.first(), ) - logger.warning(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") + self.portfolio_changes.add.append(portfolio) + logger.info(f"{TerminalColors.OKGREEN}Created portfolio '{portfolio}'.{TerminalColors.ENDC}") else: - portfolios.add(portfolio) + self.portfolio_changes.skip.append(portfolio) + message = f"Portfolio '{portfolio}' already exists. Skipping create." + logger.info(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") - # == Handle suborganizations == # - for portfolio in portfolios: - org_name = normalize_string(portfolio.organization_name) - federal_agency = agencies_set.get(org_name) - self.create_suborganizations(portfolio, federal_agency) + if not (skip_existing_portfolios and not new_portfolio): + self.create_suborganizations(portfolio, federal_agency) + + # NOTE - exceptions to portfolio and suborg are intentionally uncaught. + # parse domains, requests, and managers all rely on these fields to function. + # An error here means everything down the line is compromised. + # The individual parse steps, however, are independent from eachother. + + # Create portfolios + self.portfolio_changes.bulk_create() # Create suborganizations self.suborganization_changes.bulk_create() - message = f"Added {len(self.suborganization_changes.add)} suborganizations to portfolios." - logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") # == Handle domains, requests, and managers == # - for portfolio in portfolios: + portfolios_to_modify = set(list(existing_portfolios) + self.portfolio_changes.add) + for portfolio in portfolios_to_modify: org_name = normalize_string(portfolio.organization_name) federal_agency = agencies_set.get(org_name) - + if parse_domains: self.handle_portfolio_domains(portfolio, federal_agency) if parse_requests: - self.handle_portfolio_requests(portfolio, federal_agency) + self.handle_portfolio_requests(portfolio, federal_agency, debug) if parse_managers: - self.handle_portfolio_managers(portfolio) + self.handle_portfolio_managers(portfolio, debug) # Update DomainInformation - self.domain_info_changes.bulk_update(["portfolio", "sub_organization"]) - message = f"Added {len(self.suborganization_changes.update)} domains to portfolios." - logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") + try: + self.domain_info_changes.bulk_update(["portfolio", "sub_organization"]) + except Exception as err: + logger.error(f"{TerminalColors.FAIL}Could not bulk update domain infos.{TerminalColors.ENDC}") + logger.error(err, exc_info=True) # Update DomainRequest - self.domain_request_changes.bulk_update([ - "portfolio", - "sub_organization", - "requested_suborganization", - "suborganization_city", - "suborganization_state_territory", - "federal_agency", - ]) - message = f"Added {len(self.domain_request_changes.update)} domain requests to portfolios." - logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") + try: + self.domain_request_changes.bulk_update( + [ + "portfolio", + "sub_organization", + "requested_suborganization", + "suborganization_city", + "suborganization_state_territory", + "federal_agency", + ] + ) + except Exception as err: + logger.error(f"{TerminalColors.FAIL}Could not bulk update domain requests.{TerminalColors.ENDC}") + logger.error(err, exc_info=True) # Create UserPortfolioPermission self.user_portfolio_perm_changes.bulk_create() - message = f"Added {len(self.user_portfolio_perm_changes.add)} managers to portfolios." - logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") - # Create PortfolioInvitation + # Create PortfolioInvitation self.portfolio_invitation_changes.bulk_create() - message = f"Added {len(self.portfolio_invitation_changes.add)} manager invitations to portfolios." - logger.info(f"{TerminalColors.MAGENTA}{message}{TerminalColors.ENDC}") # == PRINT RUN SUMMARY == # self.print_final_run_summary(parse_domains, parse_requests, parse_managers, debug) def print_final_run_summary(self, parse_domains, parse_requests, parse_managers, debug): self.portfolio_changes.print_script_run_summary( - no_changes_message="\n============= No portfolios changed. =============", + no_changes_message="\n||============= No portfolios changed. =============||", debug=debug, - log_header="||============= PORTFOLIOS =============||", + log_header="============= PORTFOLIOS =============", skipped_header="----- SOME PORTFOLIOS WERENT CREATED (BUT OTHER RECORDS ARE STILL PROCESSED) -----", + detailed_prompt_title="PORTFOLIOS: Do you wish to see the full list of failed, skipped and updated records?", display_as_str=True, ) self.suborganization_changes.print_script_run_summary( - no_changes_message="\n============= No suborganizations changed. =============", + no_changes_message="\n||============= No suborganizations changed. =============||", debug=debug, log_header="============= SUBORGANIZATIONS =============", skipped_header="----- SUBORGANIZATIONS SKIPPED (SAME NAME AS PORTFOLIO NAME) -----", + detailed_prompt_title="SUBORGANIZATIONS: Do you wish to see the full list of failed, skipped and updated records?", display_as_str=True, ) if parse_domains: self.domain_info_changes.print_script_run_summary( - no_changes_message="\n============= No domains changed. =============", + no_changes_message="||============= No domains changed. =============||", debug=debug, log_header="============= DOMAINS =============", + detailed_prompt_title="DOMAINS: Do you wish to see the full list of failed, skipped and updated records?", display_as_str=True, ) if parse_requests: self.domain_request_changes.print_script_run_summary( - no_changes_message="\n============= No domain requests changed. =============", + no_changes_message="||============= No domain requests changed. =============||", debug=debug, log_header="============= DOMAIN REQUESTS =============", + detailed_prompt_title="DOMAIN REQUESTS: Do you wish to see the full list of failed, skipped and updated records?", display_as_str=True, ) if parse_managers: self.user_portfolio_perm_changes.print_script_run_summary( - no_changes_message="\n============= No managers changed. =============", + no_changes_message="||============= No managers changed. =============||", log_header="============= MANAGERS =============", skipped_header="----- MANAGERS SKIPPED (ALREADY EXISTED) -----", debug=debug, + detailed_prompt_title="MANAGERS: Do you wish to see the full list of failed, skipped and updated records?", display_as_str=True, ) self.portfolio_invitation_changes.print_script_run_summary( - no_changes_message="\n============= No manager invitations changed. =============", + no_changes_message="||============= No manager invitations changed. =============||", log_header="============= MANAGER INVITATIONS =============", debug=debug, skipped_header="----- INVITATIONS SKIPPED (ALREADY EXISTED) -----", + detailed_prompt_title="MANAGER INVITATIONS: Do you wish to see the full list of failed, skipped and updated records?", display_as_str=True, ) - def get_or_create_portfolio(self, federal_agency): - portfolio_name = normalize_string(federal_agency.agency, lowercase=False) - portfolio, created = Portfolio.objects.get_or_create( - organization_name=portfolio_name, - federal_agency=federal_agency, - organization_type=DomainRequest.OrganizationChoices.FEDERAL, - creator=User.get_default_user(), - notes="Auto-generated record", - senior_official=federal_agency.so_federal_agency.first(), - ) - - if created: - self.portfolio_changes.add.append(portfolio) - logger.info(f"{TerminalColors.OKGREEN}Created portfolio '{portfolio}'.{TerminalColors.ENDC}") - else: - self.skipped_portfolios.add(portfolio) - message = f"Portfolio '{portfolio}' already exists. Skipping create." - logger.info(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") - - return portfolio, created - def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency): """Create Suborganizations tied to the given portfolio based on DomainInformation objects""" base_filter = Q( @@ -322,11 +300,7 @@ class Command(BaseCommand): requests = federal_agency.domainrequest_set.filter(base_filter) org_names = set(domains.values_list("organization_name", flat=True)) - existing_org_names = set( - Suborganization.objects - .filter(name__in=org_names) - .values_list("name", flat=True) - ) + existing_org_names = set(Suborganization.objects.filter(name__in=org_names).values_list("name", flat=True)) for name in org_names - existing_org_names: if normalize_string(name) != normalize_string(portfolio.organization_name): suborg = Suborganization(name=name, portfolio=portfolio) @@ -338,7 +312,7 @@ class Command(BaseCommand): # This can vary per domain and request, so this is a seperate step. # First: Filter domains and requests by those that have data valid_domains = domains.filter( - city__isnull=False, + city__isnull=False, state_territory__isnull=False, portfolio__isnull=False, sub_organization__isnull=False, @@ -456,7 +430,7 @@ class Command(BaseCommand): domain_info.sub_organization = suborgs.get(org_name, None) self.domain_info_changes.update.append(domain_info) - def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency): + def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency, debug): """ Associate portfolio with domain requests for a federal agency. Updates all relevant domain request records. @@ -468,12 +442,12 @@ class Command(BaseCommand): ] domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency).exclude(status__in=invalid_states) if not domain_requests.exists(): - message = f""" - Portfolio '{portfolio}' not added to domain requests: nothing to add found. - Excluded statuses: STARTED, INELIGIBLE, REJECTED. - """ - logger.warning(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") - TerminalHelper.colorful_logger(logger.info, TerminalColors.YELLOW, message) + if debug: + message = ( + f"Portfolio '{portfolio}' not added to domain requests: nothing to add found." + "Excluded statuses: STARTED, INELIGIBLE, REJECTED." + ) + logger.warning(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") return None # Get all suborg information and store it in a dict to avoid doing a db call @@ -522,8 +496,7 @@ class Command(BaseCommand): domain__in=domains, role=UserDomainRole.Roles.MANAGER ) existing_permissions = UserPortfolioPermission.objects.filter( - user__in=user_domain_roles.values_list("user"), - portfolio=portfolio + user__in=user_domain_roles.values_list("user"), portfolio=portfolio ) existing_permissions_dict = {permission.user: permission for permission in existing_permissions} for user_domain_role in user_domain_roles: @@ -535,19 +508,20 @@ class Command(BaseCommand): roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], ) self.user_portfolio_perm_changes.add.append(permission) - logger.info(f"Added manager '{permission.user}' to portfolio '{portfolio}'.") + if debug: + logger.info(f"Added manager '{permission.user}' to portfolio '{portfolio}'.") else: existing_permission = existing_permissions_dict.get(user) self.user_portfolio_perm_changes.skip.append(existing_permission) - logger.info(f"Manager '{user}' already exists on portfolio '{portfolio}'.") + if debug: + logger.info(f"Manager '{user}' already exists on portfolio '{portfolio}'.") # Get the emails of invited managers domain_invitations = DomainInvitation.objects.filter( domain__in=domains, status=DomainInvitation.DomainInvitationStatus.INVITED ) existing_invitations = PortfolioInvitation.objects.filter( - email__in=domain_invitations.values_list("email"), - portfolio=portfolio + email__in=domain_invitations.values_list("email"), portfolio=portfolio ) existing_invitation_dict = {normalize_string(invite.email): invite for invite in existing_invitations} for domain_invitation in domain_invitations: @@ -556,8 +530,8 @@ class Command(BaseCommand): invitation = PortfolioInvitation( portfolio=portfolio, email=email, - status = PortfolioInvitation.PortfolioInvitationStatus.INVITED, - roles = [UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + status=PortfolioInvitation.PortfolioInvitationStatus.INVITED, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], ) self.portfolio_invitation_changes.add.append(invitation) logger.info(f"Added invitation '{email}' to portfolio '{portfolio}'.") diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index 63fc9e681..c3b2222d2 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -61,7 +61,7 @@ class ScriptDataHelper: for page_num in paginator.page_range: page = paginator.page(page_num) model_class.objects.bulk_update(page.object_list, fields_to_update) - + @staticmethod def bulk_create_fields(model_class, update_list, batch_size=1000, quiet=False): """ @@ -146,15 +146,17 @@ class PopulateScriptTemplate(ABC): readable_class_name = self.get_class_name(object_class) # for use in the execution prompt. - proposed_changes = f"""==Proposed Changes== - Number of {readable_class_name} objects to change: {len(records)} - These fields will be updated on each record: {fields_to_update} - """ + proposed_changes = ( + "==Proposed Changes==\n" + f"Number of {readable_class_name} objects to change: {len(records)}\n" + f"These fields will be updated on each record: {fields_to_update}" + ) if verbose: - proposed_changes = f"""{proposed_changes} - These records will be updated: {list(records.all())} - """ + proposed_changes = ( + f"{proposed_changes}\n" + f"These records will be updated: {list(records.all())}" + ) # Code execution will stop here if the user prompts "N" TerminalHelper.prompt_for_execution( @@ -219,7 +221,6 @@ class PopulateScriptTemplate(ABC): class TerminalHelper: - @staticmethod def log_script_run_summary( add, @@ -231,10 +232,10 @@ class TerminalHelper: skipped_header="----- SOME DATA WAS INVALID (NEEDS MANUAL PATCHING) -----", failed_header="----- UPDATE FAILED -----", display_as_str=False, - detailed_prompt_title="Do you wish to see the full list of failed, skipped and updated records?" + detailed_prompt_title="Do you wish to see the full list of failed, skipped and updated records?", ): """Generates a formatted summary of script execution results with colored output. - + Displays counts and details of successful, failed, and skipped operations. In debug mode or when prompted, shows full record details. Uses color coding: green for success, yellow for skipped, red for failures. @@ -306,9 +307,9 @@ class TerminalHelper: if counts["failed"] > 0: logger.error(f"{TerminalColors.FAIL}{final_message}{TerminalColors.ENDC}") elif counts["skipped"] > 0: - TerminalHelper.colorful_logger("WARNING", "YELLOW", final_message) + logger.warning(f"{TerminalColors.YELLOW}{final_message}{TerminalColors.ENDC}") else: - TerminalHelper.colorful_logger("INFO", "OKGREEN", final_message) + logger.info(f"{TerminalColors.OKGREEN}{final_message}{TerminalColors.ENDC}") @staticmethod def query_yes_no(question: str, default="yes"): From 65e70704eb9c6ed0fb016c736d0cfc5b35cf2aa3 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 20 Mar 2025 14:52:33 -0600 Subject: [PATCH 18/27] fixing unit tests --- .../commands/create_federal_portfolio.py | 186 ++++++++++++------ .../commands/utility/terminal_helper.py | 15 +- .../tests/test_management_scripts.py | 15 +- 3 files changed, 139 insertions(+), 77 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 7fa9c12e8..2b4f64d37 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -24,7 +24,7 @@ class Command(BaseCommand): class ChangeTracker: def __init__(self, model_class): self.model_class = model_class - self.add = [] + self.create = [] self.update = [] self.skip = [] self.fail = [] @@ -32,21 +32,21 @@ class Command(BaseCommand): def print_script_run_summary(self, no_changes_message, **kwargs): """Helper function that runs TerminalHelper.log_script_run_summary on this object.""" if self.has_changes(): - TerminalHelper.log_script_run_summary(self.add, self.update, self.skip, self.fail, **kwargs) + TerminalHelper.log_script_run_summary(self.create, self.update, self.skip, self.fail, **kwargs) else: logger.info(f"{TerminalColors.BOLD}{no_changes_message}{TerminalColors.ENDC}") def has_changes(self) -> bool: - num_changes = [len(self.add), len(self.update), len(self.skip), len(self.fail)] + num_changes = [len(self.create), len(self.update), len(self.skip), len(self.fail)] return any([num_change > 0 for num_change in num_changes]) def bulk_create(self): try: - ScriptDataHelper.bulk_create_fields(self.model_class, self.add, quiet=True) + ScriptDataHelper.bulk_create_fields(self.model_class, self.create, quiet=True) except Exception as err: # In this case, just swap the fail and add lists - self.fail = self.add.copy() - self.add = [] + self.fail = self.create.copy() + self.create.clear() raise err def bulk_update(self, fields_to_update): @@ -55,7 +55,7 @@ class Command(BaseCommand): except Exception as err: # In this case, just swap the fail and update lists self.fail = self.update.copy() - self.update = [] + self.update.clear() raise err def __init__(self, *args, **kwargs): @@ -79,9 +79,12 @@ class Command(BaseCommand): 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_managers: Add all domain managers of the portfolio's domains to the organization. Optional: - --add_managers: Add all domain managers of the portfolio's domains to the organization. + --skip_existing_portfolios: Does not perform substeps on a portfolio if it already exists. + -- clear_federal_agency_on_started_domain_requests: Parses started domain requests + --debug: Increases log verbosity """ group = parser.add_mutually_exclusive_group(required=True) group.add_argument( @@ -111,9 +114,13 @@ class Command(BaseCommand): parser.add_argument( "--skip_existing_portfolios", action=argparse.BooleanOptionalAction, - help="Only add suborganizations to newly created portfolios, skip existing ones.", + help="Only parses newly created portfolios, skippubg existing ones.", + ) + parser.add_argument( + "--clear_federal_agency_on_started_domain_requests", + action=argparse.BooleanOptionalAction, + help="Clears the federal agency field on started domain requests under the given portfolio", ) - parser.add_argument( "--debug", action=argparse.BooleanOptionalAction, @@ -127,6 +134,7 @@ class Command(BaseCommand): parse_domains = options.get("parse_domains") parse_managers = options.get("parse_managers") skip_existing_portfolios = options.get("skip_existing_portfolios") + clear_federal_agency_on_started_domain_requests = options.get("clear_federal_agency_on_started_domain_requests") debug = options.get("debug") # Parse script params @@ -148,11 +156,15 @@ class Command(BaseCommand): else: raise CommandError(f"Cannot find '{branch}' federal agencies in our database.") - # == Handle portfolios and suborganizations == # + # NOTE: exceptions to portfolio and suborg are intentionally uncaught. + # parse domains, requests, and managers all rely on these fields to function. + # An error here means everything down the line is compromised. + # The individual parse steps, however, are independent from eachother. + + # == Handle portfolios == # # TODO - some kind of duplicate check on agencies and existing portfolios existing_portfolios = Portfolio.objects.filter( - organization_name__in=agencies.values_list("agency", flat=True), - organization_name__isnull=False + organization_name__in=agencies.values_list("agency", flat=True), organization_name__isnull=False ) existing_portfolios_set = {normalize_string(p.organization_name): p for p in existing_portfolios} agencies_set = {normalize_string(agency.agency): agency for agency in agencies} @@ -169,30 +181,35 @@ class Command(BaseCommand): notes="Auto-generated record", senior_official=federal_agency.so_federal_agency.first(), ) - self.portfolio_changes.add.append(portfolio) + self.portfolio_changes.create.append(portfolio) logger.info(f"{TerminalColors.OKGREEN}Created portfolio '{portfolio}'.{TerminalColors.ENDC}") - else: - self.portfolio_changes.skip.append(portfolio) - message = f"Portfolio '{portfolio}' already exists. Skipping create." + + if skip_existing_portfolios and not new_portfolio: + message = f"Portfolio '{portfolio}' already exists. Skipped." logger.info(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") - - if not (skip_existing_portfolios and not new_portfolio): - self.create_suborganizations(portfolio, federal_agency) - - # NOTE - exceptions to portfolio and suborg are intentionally uncaught. - # parse domains, requests, and managers all rely on these fields to function. - # An error here means everything down the line is compromised. - # The individual parse steps, however, are independent from eachother. + if portfolio: + self.portfolio_changes.skip.append(portfolio) # Create portfolios self.portfolio_changes.bulk_create() - # Create suborganizations - self.suborganization_changes.bulk_create() + # After create, get the list of all portfolios to use + portfolios_to_use = set(self.portfolio_changes.create) + if not skip_existing_portfolios: + portfolios_to_use.update(set(existing_portfolios)) + + # == Handle suborganizations == # + for portfolio in portfolios_to_use: + created_suborgs = [] + org_name = normalize_string(portfolio.organization_name) + federal_agency = agencies_set.get(org_name) + if portfolio: + created_suborgs = self.create_suborganizations(portfolio, federal_agency) + Suborganization.objects.bulk_create(created_suborgs) + self.suborganization_changes.create.extend(created_suborgs) # == Handle domains, requests, and managers == # - portfolios_to_modify = set(list(existing_portfolios) + self.portfolio_changes.add) - for portfolio in portfolios_to_modify: + for portfolio in portfolios_to_use: org_name = normalize_string(portfolio.organization_name) federal_agency = agencies_set.get(org_name) @@ -200,7 +217,9 @@ class Command(BaseCommand): self.handle_portfolio_domains(portfolio, federal_agency) if parse_requests: - self.handle_portfolio_requests(portfolio, federal_agency, debug) + self.handle_portfolio_requests( + portfolio, federal_agency, clear_federal_agency_on_started_domain_requests, debug + ) if parse_managers: self.handle_portfolio_managers(portfolio, debug) @@ -229,10 +248,18 @@ class Command(BaseCommand): logger.error(err, exc_info=True) # Create UserPortfolioPermission - self.user_portfolio_perm_changes.bulk_create() + try: + self.user_portfolio_perm_changes.bulk_create() + except Exception as err: + logger.error(f"{TerminalColors.FAIL}Could not bulk create user portfolio permissions.{TerminalColors.ENDC}") + logger.error(err, exc_info=True) # Create PortfolioInvitation - self.portfolio_invitation_changes.bulk_create() + try: + self.portfolio_invitation_changes.bulk_create() + except Exception as err: + logger.error(f"{TerminalColors.FAIL}Could not bulk create portfolio invitations.{TerminalColors.ENDC}") + logger.error(err, exc_info=True) # == PRINT RUN SUMMARY == # self.print_final_run_summary(parse_domains, parse_requests, parse_managers, debug) @@ -291,22 +318,45 @@ class Command(BaseCommand): display_as_str=True, ) - def create_suborganizations(self, portfolio: Portfolio, federal_agency: FederalAgency): + def create_suborganizations(self, portfolio, federal_agency): """Create Suborganizations tied to the given portfolio based on DomainInformation objects""" base_filter = Q( organization_name__isnull=False, ) & ~Q(organization_name__iexact=F("portfolio__organization_name")) + domains = federal_agency.domaininformation_set.filter(base_filter) requests = federal_agency.domainrequest_set.filter(base_filter) + existing_orgs = Suborganization.objects.all() - org_names = set(domains.values_list("organization_name", flat=True)) - existing_org_names = set(Suborganization.objects.filter(name__in=org_names).values_list("name", flat=True)) - for name in org_names - existing_org_names: - if normalize_string(name) != normalize_string(portfolio.organization_name): + # Normalize all suborg names so we don't add duplicate data unintentionally. + # Get all suborg names that we COULD add + org_names_normalized = {} + for domain in domains: + org_name = normalize_string(domain.organization_name) + if org_name not in org_names_normalized: + org_names_normalized[org_name] = domain.organization_name + + # Get all suborg names that presently exist + existing_org_names_normalized = {} + for org in existing_orgs: + org_name = normalize_string(org.name) + if org_name not in existing_org_names_normalized: + existing_org_names_normalized[org_name] = org.name + + # Subtract existing names from ones we COULD add. + # We don't want to add existing names. + new_org_names = {} + for norm_name, name in org_names_normalized.items(): + if norm_name not in existing_org_names_normalized: + new_org_names[norm_name] = name + + # Add new suborgs assuming they aren't duplicates and don't already exist in the db. + created_suborgs = [] + for norm_name, name in new_org_names.items(): + norm_portfolio_name = normalize_string(portfolio.organization_name) + if norm_name != norm_portfolio_name: suborg = Suborganization(name=name, portfolio=portfolio) - # TODO - change this portion - if suborg.name not in [org.name for org in self.suborganization_changes.add]: - self.suborganization_changes.add.append(suborg) + created_suborgs.append(suborg) # Add location information to suborgs. # This can vary per domain and request, so this is a seperate step. @@ -331,16 +381,20 @@ class Command(BaseCommand): domains_dict = {} requests_dict = {} for domain in valid_domains: + print(f"what is the org name? {domain.organization_name}") normalized_name = normalize_string(domain.organization_name) domains_dict.setdefault(normalized_name, []).append(domain) for request in valid_requests: + print(f"what is the org name for requests? {request.organization_name}") normalized_name = normalize_string(request.organization_name) requests_dict.setdefault(normalized_name, []).append(request) # Fourth: Process each suborg to add city / state territory info - for suborg in self.suborganization_changes.add: + for suborg in created_suborgs: self.set_suborganization_location(suborg, domains_dict, requests_dict) + + return created_suborgs def set_suborganization_location(self, suborg, domains_dict, requests_dict): """Updates a single suborganization's location data if valid. @@ -356,6 +410,8 @@ class Command(BaseCommand): normalized_suborg_name = normalize_string(suborg.name) domains = domains_dict.get(normalized_suborg_name, []) requests = requests_dict.get(normalized_suborg_name, []) + print(f"domains: {domains}") + print(f"requests: {requests}") # Try to get matching domain info domain = None @@ -430,7 +486,13 @@ class Command(BaseCommand): domain_info.sub_organization = suborgs.get(org_name, None) self.domain_info_changes.update.append(domain_info) - def handle_portfolio_requests(self, portfolio: Portfolio, federal_agency: FederalAgency, debug): + def handle_portfolio_requests( + self, + portfolio: Portfolio, + federal_agency: FederalAgency, + clear_federal_agency_on_started_domain_requests: bool, + debug: bool, + ): """ Associate portfolio with domain requests for a federal agency. Updates all relevant domain request records. @@ -444,8 +506,8 @@ class Command(BaseCommand): if not domain_requests.exists(): if debug: message = ( - f"Portfolio '{portfolio}' not added to domain requests: nothing to add found." - "Excluded statuses: STARTED, INELIGIBLE, REJECTED." + f"Portfolio '{portfolio}' not added to domain requests: nothing to add found." + "Excluded statuses: STARTED, INELIGIBLE, REJECTED." ) logger.warning(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") return None @@ -464,27 +526,27 @@ class Command(BaseCommand): domain_request.suborganization_state_territory = domain_request.state_territory self.domain_request_changes.update.append(domain_request) - # TODO - add this option as a FLAG to pass into the script directly # For each STARTED 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. - started_domain_requests = federal_agency.domainrequest_set.filter( - status=DomainRequest.DomainRequestStatus.STARTED, - organization_name__isnull=False, - ) + if clear_federal_agency_on_started_domain_requests: + started_domain_requests = federal_agency.domainrequest_set.filter( + status=DomainRequest.DomainRequestStatus.STARTED, + organization_name__isnull=False, + ) - portfolio_name = normalize_string(portfolio.organization_name) + portfolio_name = normalize_string(portfolio.organization_name) - # Update the request, assuming the given agency name matches the portfolio name - for domain_request in started_domain_requests: - agency_name = normalize_string(domain_request.federal_agency.agency) - if agency_name == portfolio_name: - domain_request.federal_agency = None - self.domain_request_changes.update.append(domain_request) + # Update the request, assuming the given agency name matches the portfolio name + for domain_request in started_domain_requests: + agency_name = normalize_string(domain_request.federal_agency.agency) + if agency_name == portfolio_name: + domain_request.federal_agency = None + self.domain_request_changes.update.append(domain_request) - def handle_portfolio_managers(self, portfolio: Portfolio): + def handle_portfolio_managers(self, portfolio: Portfolio, debug): """ Add all domain managers of the portfolio's domains to the organization. This includes adding them to the correct group and creating portfolio invitations. @@ -507,7 +569,7 @@ class Command(BaseCommand): user=user, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], ) - self.user_portfolio_perm_changes.add.append(permission) + self.user_portfolio_perm_changes.create.append(permission) if debug: logger.info(f"Added manager '{permission.user}' to portfolio '{portfolio}'.") else: @@ -533,9 +595,11 @@ class Command(BaseCommand): status=PortfolioInvitation.PortfolioInvitationStatus.INVITED, roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], ) - self.portfolio_invitation_changes.add.append(invitation) - logger.info(f"Added invitation '{email}' to portfolio '{portfolio}'.") + self.portfolio_invitation_changes.create.append(invitation) + if debug: + logger.info(f"Added invitation '{email}' to portfolio '{portfolio}'.") else: existing_invitation = existing_invitations.get(email) self.portfolio_invitation_changes.skip.append(existing_invitation) - logger.info(f"Invitation '{email}' already exists in portfolio '{portfolio}'.") + if debug: + logger.info(f"Invitation '{email}' already exists in portfolio '{portfolio}'.") diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index c3b2222d2..85af1f9e5 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -153,10 +153,7 @@ class PopulateScriptTemplate(ABC): ) if verbose: - proposed_changes = ( - f"{proposed_changes}\n" - f"These records will be updated: {list(records.all())}" - ) + proposed_changes = f"{proposed_changes}\n" f"These records will be updated: {list(records.all())}" # Code execution will stop here if the user prompts "N" TerminalHelper.prompt_for_execution( @@ -223,7 +220,7 @@ class TerminalHelper: @staticmethod def log_script_run_summary( - add, + create, update, skip, fail, @@ -253,7 +250,7 @@ class TerminalHelper: Output Format (if count > 0 for each category): [log_header] - Added W entries + Created W entries Updated X entries [skipped_header] Skipped updating Y entries @@ -265,7 +262,7 @@ class TerminalHelper: - Converts each item to string if display_as_str is True """ counts = { - "added": len(add), + "created": len(create), "updated": len(update), "skipped": len(skip), "failed": len(fail), @@ -285,8 +282,8 @@ class TerminalHelper: messages = [] for category, count in non_zero_counts.items(): match category: - case "added": - label, values, debug_color = "Added", add, TerminalColors.OKBLUE + case "created": + label, values, debug_color = "Created", create, TerminalColors.OKBLUE case "updated": label, values, debug_color = "Updated", update, TerminalColors.OKCYAN case "skipped": diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 110feea85..4a17c7d61 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1473,6 +1473,7 @@ class TestCreateFederalPortfolio(TestCase): generic_org_type=DomainRequest.OrganizationChoices.CITY, federal_agency=self.federal_agency, user=self.user, + organization_name="Testorg" ) self.domain_request.approve() self.domain_info = DomainInformation.objects.filter(domain_request=self.domain_request).get() @@ -1822,12 +1823,12 @@ class TestCreateFederalPortfolio(TestCase): # We expect a error to be thrown when we dont pass parse requests or domains with self.assertRaisesRegex( - CommandError, "You must specify at least one of --parse_requests, --parse_domains, or --add_managers." + CommandError, "You must specify at least one of --parse_requests, --parse_domains, or --parse_managers." ): self.run_create_federal_portfolio(branch="executive") with self.assertRaisesRegex( - CommandError, "You must specify at least one of --parse_requests, --parse_domains, or --add_managers." + CommandError, "You must specify at least one of --parse_requests, --parse_domains, or --parse_managers." ): self.run_create_federal_portfolio(agency_name="test") @@ -1877,7 +1878,7 @@ class TestCreateFederalPortfolio(TestCase): UserDomainRole.objects.create(user=manager2, domain=self.domain, role=UserDomainRole.Roles.MANAGER) # Run the management command - self.run_create_federal_portfolio(agency_name=self.federal_agency.agency, parse_domains=True, add_managers=True) + self.run_create_federal_portfolio(agency_name=self.federal_agency.agency, parse_domains=True, parse_managers=True) # Check that the portfolio was created self.portfolio = Portfolio.objects.get(federal_agency=self.federal_agency) @@ -1900,7 +1901,7 @@ class TestCreateFederalPortfolio(TestCase): ) # Run the management command - self.run_create_federal_portfolio(agency_name=self.federal_agency.agency, parse_domains=True, add_managers=True) + self.run_create_federal_portfolio(agency_name=self.federal_agency.agency, parse_domains=True, parse_managers=True) # Check that the portfolio was created self.portfolio = Portfolio.objects.get(federal_agency=self.federal_agency) @@ -1917,7 +1918,7 @@ class TestCreateFederalPortfolio(TestCase): # Verify that no duplicate invitations are created self.run_create_federal_portfolio( - agency_name=self.federal_agency.agency, parse_requests=True, add_managers=True + agency_name=self.federal_agency.agency, parse_requests=True, parse_managers=True ) invitations = PortfolioInvitation.objects.filter(email="manager1@example.com", portfolio=self.portfolio) self.assertEqual( @@ -1945,7 +1946,7 @@ class TestCreateFederalPortfolio(TestCase): # Run the management command self.run_create_federal_portfolio( - agency_name=self.federal_agency.agency, parse_requests=True, add_managers=True + agency_name=self.federal_agency.agency, parse_requests=True, parse_managers=True ) # Ensure that the manager is not duplicated @@ -1993,7 +1994,7 @@ class TestCreateFederalPortfolio(TestCase): self.run_create_federal_portfolio( agency_name=self.federal_agency.agency, parse_requests=True, - add_managers=True, + parse_managers=True, skip_existing_portfolios=True, ) From 46a720abb3858a4911cb672135d761101a6556fb Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 21 Mar 2025 10:54:54 -0600 Subject: [PATCH 19/27] fix create_suborg step and consolidate --- .../commands/create_federal_portfolio.py | 157 ++++++++---------- .../commands/utility/terminal_helper.py | 15 +- 2 files changed, 81 insertions(+), 91 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 2b4f64d37..9fb8fee02 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -37,12 +37,14 @@ class Command(BaseCommand): logger.info(f"{TerminalColors.BOLD}{no_changes_message}{TerminalColors.ENDC}") def has_changes(self) -> bool: - num_changes = [len(self.create), len(self.update), len(self.skip), len(self.fail)] - return any([num_change > 0 for num_change in num_changes]) + changes = [self.create, self.update, self.skip, self.fail] + return any([change for change in changes if change]) def bulk_create(self): try: - ScriptDataHelper.bulk_create_fields(self.model_class, self.create, quiet=True) + res = ScriptDataHelper.bulk_create_fields(self.model_class, self.create, return_created=True, quiet=True) + self.create = res + return res except Exception as err: # In this case, just swap the fail and add lists self.fail = self.create.copy() @@ -51,7 +53,9 @@ class Command(BaseCommand): def bulk_update(self, fields_to_update): try: - ScriptDataHelper.bulk_update_fields(self.model_class, self.update, fields_to_update, quiet=True) + res = ScriptDataHelper.bulk_update_fields(self.model_class, self.update, fields_to_update, quiet=True) + self.update = res + return res except Exception as err: # In this case, just swap the fail and update lists self.fail = self.update.copy() @@ -167,12 +171,11 @@ class Command(BaseCommand): organization_name__in=agencies.values_list("agency", flat=True), organization_name__isnull=False ) existing_portfolios_set = {normalize_string(p.organization_name): p for p in existing_portfolios} - agencies_set = {normalize_string(agency.agency): agency for agency in agencies} - for federal_agency in agencies_set.values(): + agencies_dict = {normalize_string(agency.agency): agency for agency in agencies} + for federal_agency in agencies_dict.values(): portfolio_name = normalize_string(federal_agency.agency, lowercase=False) portfolio = existing_portfolios_set.get(portfolio_name, None) - new_portfolio = portfolio is None - if new_portfolio: + if portfolio is None: portfolio = Portfolio( organization_name=portfolio_name, federal_agency=federal_agency, @@ -183,36 +186,30 @@ class Command(BaseCommand): ) self.portfolio_changes.create.append(portfolio) logger.info(f"{TerminalColors.OKGREEN}Created portfolio '{portfolio}'.{TerminalColors.ENDC}") - - if skip_existing_portfolios and not new_portfolio: + elif skip_existing_portfolios: message = f"Portfolio '{portfolio}' already exists. Skipped." logger.info(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") - if portfolio: - self.portfolio_changes.skip.append(portfolio) + self.portfolio_changes.skip.append(portfolio) # Create portfolios - self.portfolio_changes.bulk_create() + portfolios_to_use = self.portfolio_changes.bulk_create() # After create, get the list of all portfolios to use portfolios_to_use = set(self.portfolio_changes.create) if not skip_existing_portfolios: portfolios_to_use.update(set(existing_portfolios)) + + portfolios_to_use_dict = {normalize_string(p.organization_name): p for p in portfolios_to_use} # == Handle suborganizations == # - for portfolio in portfolios_to_use: - created_suborgs = [] - org_name = normalize_string(portfolio.organization_name) - federal_agency = agencies_set.get(org_name) - if portfolio: - created_suborgs = self.create_suborganizations(portfolio, federal_agency) - Suborganization.objects.bulk_create(created_suborgs) - self.suborganization_changes.create.extend(created_suborgs) + created_suborgs = self.create_suborganizations(portfolios_to_use_dict, agencies_dict) + if created_suborgs: + self.suborganization_changes.create.extend(created_suborgs.values()) + self.suborganization_changes.bulk_create() # == Handle domains, requests, and managers == # - for portfolio in portfolios_to_use: - org_name = normalize_string(portfolio.organization_name) - federal_agency = agencies_set.get(org_name) - + for portfolio_org_name, portfolio in portfolios_to_use_dict.items(): + federal_agency = agencies_dict.get(portfolio_org_name) if parse_domains: self.handle_portfolio_domains(portfolio, federal_agency) @@ -318,85 +315,66 @@ class Command(BaseCommand): display_as_str=True, ) - def create_suborganizations(self, portfolio, federal_agency): + def create_suborganizations(self, portfolio_dict, agency_dict): """Create Suborganizations tied to the given portfolio based on DomainInformation objects""" - base_filter = Q( - organization_name__isnull=False, - ) & ~Q(organization_name__iexact=F("portfolio__organization_name")) + created_suborgs = {} - domains = federal_agency.domaininformation_set.filter(base_filter) - requests = federal_agency.domainrequest_set.filter(base_filter) - existing_orgs = Suborganization.objects.all() + portfolios = portfolio_dict.values() + agencies = agency_dict.values() + existing_suborgs = Suborganization.objects.filter(portfolio__in=portfolios) + suborg_dict = {normalize_string(org.name): org for org in existing_suborgs} + + domains = DomainInformation.objects.filter( + # Org name must not be null, and must not be the portfolio name + Q( + organization_name__isnull=False, + ) & ~Q(organization_name__iexact=F("portfolio__organization_name")), + # Only get relevant data to the agency/portfolio we are targeting + Q(federal_agency__in=agencies) | Q(portfolio__in=portfolios), + ) + requests = DomainRequest.objects.filter( + # Org name must not be null, and must not be the portfolio name + Q( + organization_name__isnull=False, + ) & ~Q(organization_name__iexact=F("portfolio__organization_name")), + # Only get relevant data to the agency/portfolio we are targeting + Q(federal_agency__in=agencies) | Q(portfolio__in=portfolios), + ) # Normalize all suborg names so we don't add duplicate data unintentionally. - # Get all suborg names that we COULD add - org_names_normalized = {} - for domain in domains: - org_name = normalize_string(domain.organization_name) - if org_name not in org_names_normalized: - org_names_normalized[org_name] = domain.organization_name + for portfolio_name, portfolio in portfolio_dict.items(): + for domain in domains: + if normalize_string(domain.federal_agency.agency) != portfolio_name: + continue - # Get all suborg names that presently exist - existing_org_names_normalized = {} - for org in existing_orgs: - org_name = normalize_string(org.name) - if org_name not in existing_org_names_normalized: - existing_org_names_normalized[org_name] = org.name - - # Subtract existing names from ones we COULD add. - # We don't want to add existing names. - new_org_names = {} - for norm_name, name in org_names_normalized.items(): - if norm_name not in existing_org_names_normalized: - new_org_names[norm_name] = name - - # Add new suborgs assuming they aren't duplicates and don't already exist in the db. - created_suborgs = [] - for norm_name, name in new_org_names.items(): - norm_portfolio_name = normalize_string(portfolio.organization_name) - if norm_name != norm_portfolio_name: - suborg = Suborganization(name=name, portfolio=portfolio) - created_suborgs.append(suborg) + org_name = domain.organization_name + norm_org_name = normalize_string(domain.organization_name) + # If the suborg already exists or if we've already added it, don't add it again. + if norm_org_name not in suborg_dict and norm_org_name not in created_suborgs: + suborg = Suborganization(name=org_name, portfolio=portfolio) + created_suborgs[norm_org_name] = suborg # Add location information to suborgs. # This can vary per domain and request, so this is a seperate step. - # First: Filter domains and requests by those that have data - valid_domains = domains.filter( - city__isnull=False, - state_territory__isnull=False, - portfolio__isnull=False, - sub_organization__isnull=False, - ) - valid_requests = requests.filter( - ( - Q(city__isnull=False, state_territory__isnull=False) - | Q(suborganization_city__isnull=False, suborganization_state_territory__isnull=False) - ), - portfolio__isnull=False, - sub_organization__isnull=False, - ) - # Second: Group domains and requests by normalized organization name. - # This means that later down the line we can account for "duplicate" org names. + # First: Group domains and requests by normalized organization name. domains_dict = {} requests_dict = {} - for domain in valid_domains: - print(f"what is the org name? {domain.organization_name}") + for domain in domains: normalized_name = normalize_string(domain.organization_name) domains_dict.setdefault(normalized_name, []).append(domain) - for request in valid_requests: - print(f"what is the org name for requests? {request.organization_name}") + for request in requests: normalized_name = normalize_string(request.organization_name) requests_dict.setdefault(normalized_name, []).append(request) - # Fourth: Process each suborg to add city / state territory info - for suborg in created_suborgs: - self.set_suborganization_location(suborg, domains_dict, requests_dict) - + # Second: Process each suborg to add city / state territory info + for norm_name, suborg in created_suborgs.items(): + self.set_suborganization_location(norm_name, suborg, domains_dict, requests_dict) + return created_suborgs - def set_suborganization_location(self, suborg, domains_dict, requests_dict): + def set_suborganization_location(self, normalized_suborg_name, suborg, domains_dict, requests_dict): """Updates a single suborganization's location data if valid. Args: @@ -407,18 +385,19 @@ class Command(BaseCommand): Priority matches parent method. Updates are skipped if location data conflicts between multiple records of the same type. """ - normalized_suborg_name = normalize_string(suborg.name) domains = domains_dict.get(normalized_suborg_name, []) requests = requests_dict.get(normalized_suborg_name, []) - print(f"domains: {domains}") - print(f"requests: {requests}") # Try to get matching domain info domain = None if domains: reference = domains[0] use_location_for_domain = all( - d.city == reference.city and d.state_territory == reference.state_territory for d in domains + d.city + and d.state_territory + and d.city == reference.city + and d.state_territory == reference.state_territory + for d in domains ) if use_location_for_domain: domain = reference diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index 85af1f9e5..e8ec5e41f 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -52,6 +52,8 @@ class ScriptDataHelper: Usage: ScriptDataHelper.bulk_update_fields(Domain, page.object_list, ["first_ready"]) + + Returns: A queryset of the updated objets """ if not quiet: logger.info(f"{TerminalColors.YELLOW} Bulk updating fields... {TerminalColors.ENDC}") @@ -63,7 +65,7 @@ class ScriptDataHelper: model_class.objects.bulk_update(page.object_list, fields_to_update) @staticmethod - def bulk_create_fields(model_class, update_list, batch_size=1000, quiet=False): + def bulk_create_fields(model_class, update_list, batch_size=1000, return_created=False, quiet=False): """ This function performs a bulk create operation on a specified Django model class in batches. It uses Django's Paginator to handle large datasets in a memory-efficient manner. @@ -80,13 +82,22 @@ class ScriptDataHelper: or large field values, you may need to decrease this value to prevent out-of-memory errors. Usage: ScriptDataHelper.bulk_add_fields(Domain, page.object_list) + + Returns: A queryset of the added objects """ if not quiet: logger.info(f"{TerminalColors.YELLOW} Bulk adding fields... {TerminalColors.ENDC}") + + created_objs = [] paginator = Paginator(update_list, batch_size) for page_num in paginator.page_range: page = paginator.page(page_num) - model_class.objects.bulk_create(page.object_list) + all_created = model_class.objects.bulk_create(page.object_list) + if return_created: + created_objs.extend([created.id for created in all_created]) + if return_created: + return model_class.objects.filter(id__in=created_objs) + return None class PopulateScriptTemplate(ABC): From 8f7959a023651fedf8cf2e6e8e0bc2a21a377c1e Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 21 Mar 2025 11:30:08 -0600 Subject: [PATCH 20/27] Consolidate location logic inside of create suborgs This was *tricky* --- .../commands/create_federal_portfolio.py | 64 +++++++++++-------- 1 file changed, 36 insertions(+), 28 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 9fb8fee02..2c7848126 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -9,7 +9,7 @@ from registrar.models.domain_invitation import DomainInvitation from registrar.models.portfolio_invitation import PortfolioInvitation from registrar.models.user_domain_role import UserDomainRole from registrar.models.user_portfolio_permission import UserPortfolioPermission -from registrar.models.utility.generic_helper import normalize_string +from registrar.models.utility.generic_helper import count_capitals, normalize_string from django.db.models import F, Q from registrar.models.utility.portfolio_helper import UserPortfolioRoleChoices @@ -321,8 +321,6 @@ class Command(BaseCommand): portfolios = portfolio_dict.values() agencies = agency_dict.values() - existing_suborgs = Suborganization.objects.filter(portfolio__in=portfolios) - suborg_dict = {normalize_string(org.name): org for org in existing_suborgs} domains = DomainInformation.objects.filter( # Org name must not be null, and must not be the portfolio name @@ -341,23 +339,11 @@ class Command(BaseCommand): Q(federal_agency__in=agencies) | Q(portfolio__in=portfolios), ) - # Normalize all suborg names so we don't add duplicate data unintentionally. - for portfolio_name, portfolio in portfolio_dict.items(): - for domain in domains: - if normalize_string(domain.federal_agency.agency) != portfolio_name: - continue + # First: get all existing suborgs + existing_suborgs = Suborganization.objects.filter(portfolio__in=portfolios) + suborg_dict = {normalize_string(org.name): org for org in existing_suborgs} - org_name = domain.organization_name - norm_org_name = normalize_string(domain.organization_name) - # If the suborg already exists or if we've already added it, don't add it again. - if norm_org_name not in suborg_dict and norm_org_name not in created_suborgs: - suborg = Suborganization(name=org_name, portfolio=portfolio) - created_suborgs[norm_org_name] = suborg - - # Add location information to suborgs. - # This can vary per domain and request, so this is a seperate step. - - # First: Group domains and requests by normalized organization name. + # Second: Group domains and requests by normalized organization name. domains_dict = {} requests_dict = {} for domain in domains: @@ -368,25 +354,47 @@ class Command(BaseCommand): normalized_name = normalize_string(request.organization_name) requests_dict.setdefault(normalized_name, []).append(request) - # Second: Process each suborg to add city / state territory info - for norm_name, suborg in created_suborgs.items(): - self.set_suborganization_location(norm_name, suborg, domains_dict, requests_dict) + # Third: Parse through each group of domains that have the same organization names, + # then create *one* suborg record from it. + # Normalize all suborg names so we don't add duplicate data unintentionally. + for portfolio_name, portfolio in portfolio_dict.items(): + for norm_org_name, domains in domains_dict.items(): + if norm_org_name == portfolio_name: + continue + + new_suborg_name = None + if len(domains) == 1: + new_suborg_name = domains[0].organization_name + elif len(domains) > 1: + # Pick the best record for a suborg name (fewest spaces, most leading capitals) + best_record = max( + domains, + key=lambda rank: ( + -domain.organization_name.count(" "), count_capitals(domain.organization_name, leading_only=True) + ), + ) + new_suborg_name = best_record.organization_name + + # If the suborg already exists, don't add it again. + if norm_org_name not in suborg_dict and norm_org_name not in created_suborgs: + requests = requests_dict.get(norm_org_name) + suborg = Suborganization(name=new_suborg_name, portfolio=portfolio) + self.set_suborganization_location(suborg, domains, requests) + created_suborgs[norm_org_name] = suborg return created_suborgs - def set_suborganization_location(self, normalized_suborg_name, suborg, domains_dict, requests_dict): + def set_suborganization_location(self, suborg, domains, requests): """Updates a single suborganization's location data if valid. Args: suborg: Suborganization to update - domains_dict: Dict of domain info records grouped by org name - requests_dict: Dict of domain requests grouped by org name + domains: omain info records grouped by org name + requests: domain requests grouped by org name - Priority matches parent method. Updates are skipped if location data conflicts + Updates are skipped if location data conflicts between multiple records of the same type. """ - domains = domains_dict.get(normalized_suborg_name, []) - requests = requests_dict.get(normalized_suborg_name, []) # Try to get matching domain info domain = None From 2e119c260036fed11a63763be63e591b4fabf0e4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 21 Mar 2025 13:17:47 -0600 Subject: [PATCH 21/27] Fix managers portion --- .../commands/create_federal_portfolio.py | 248 ++++++++---------- .../tests/test_management_scripts.py | 5 +- 2 files changed, 113 insertions(+), 140 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 2c7848126..89ccfb1da 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -5,6 +5,7 @@ import logging from django.core.management import BaseCommand, CommandError from registrar.management.commands.utility.terminal_helper import ScriptDataHelper, TerminalColors, TerminalHelper from registrar.models import DomainInformation, DomainRequest, FederalAgency, Suborganization, Portfolio, User +from registrar.models.domain import Domain from registrar.models.domain_invitation import DomainInvitation from registrar.models.portfolio_invitation import PortfolioInvitation from registrar.models.user_domain_role import UserDomainRole @@ -87,7 +88,6 @@ class Command(BaseCommand): Optional: --skip_existing_portfolios: Does not perform substeps on a portfolio if it already exists. - -- clear_federal_agency_on_started_domain_requests: Parses started domain requests --debug: Increases log verbosity """ group = parser.add_mutually_exclusive_group(required=True) @@ -120,11 +120,6 @@ class Command(BaseCommand): action=argparse.BooleanOptionalAction, help="Only parses newly created portfolios, skippubg existing ones.", ) - parser.add_argument( - "--clear_federal_agency_on_started_domain_requests", - action=argparse.BooleanOptionalAction, - help="Clears the federal agency field on started domain requests under the given portfolio", - ) parser.add_argument( "--debug", action=argparse.BooleanOptionalAction, @@ -138,7 +133,6 @@ class Command(BaseCommand): parse_domains = options.get("parse_domains") parse_managers = options.get("parse_managers") skip_existing_portfolios = options.get("skip_existing_portfolios") - clear_federal_agency_on_started_domain_requests = options.get("clear_federal_agency_on_started_domain_requests") debug = options.get("debug") # Parse script params @@ -160,24 +154,26 @@ class Command(BaseCommand): else: raise CommandError(f"Cannot find '{branch}' federal agencies in our database.") + # Store all portfolios and agencies in a dict to avoid extra db calls + existing_portfolios = Portfolio.objects.filter( + organization_name__in=agencies.values_list("agency", flat=True), organization_name__isnull=False + ) + existing_portfolios_dict = {normalize_string(p.organization_name): p for p in existing_portfolios} + agencies_dict = {normalize_string(agency.agency): agency for agency in agencies} + # NOTE: exceptions to portfolio and suborg are intentionally uncaught. # parse domains, requests, and managers all rely on these fields to function. # An error here means everything down the line is compromised. # The individual parse steps, however, are independent from eachother. # == Handle portfolios == # - # TODO - some kind of duplicate check on agencies and existing portfolios - existing_portfolios = Portfolio.objects.filter( - organization_name__in=agencies.values_list("agency", flat=True), organization_name__isnull=False - ) - existing_portfolios_set = {normalize_string(p.organization_name): p for p in existing_portfolios} - agencies_dict = {normalize_string(agency.agency): agency for agency in agencies} + # Loop through every agency we want to add and create a portfolio if the record is new. for federal_agency in agencies_dict.values(): - portfolio_name = normalize_string(federal_agency.agency, lowercase=False) - portfolio = existing_portfolios_set.get(portfolio_name, None) + norm_agency_name = normalize_string(federal_agency.agency) + portfolio = existing_portfolios_dict.get(norm_agency_name, None) if portfolio is None: portfolio = Portfolio( - organization_name=portfolio_name, + organization_name=federal_agency.agency, federal_agency=federal_agency, organization_type=DomainRequest.OrganizationChoices.FEDERAL, creator=User.get_default_user(), @@ -192,13 +188,13 @@ class Command(BaseCommand): self.portfolio_changes.skip.append(portfolio) # Create portfolios - portfolios_to_use = self.portfolio_changes.bulk_create() + self.portfolio_changes.bulk_create() # After create, get the list of all portfolios to use portfolios_to_use = set(self.portfolio_changes.create) if not skip_existing_portfolios: portfolios_to_use.update(set(existing_portfolios)) - + portfolios_to_use_dict = {normalize_string(p.organization_name): p for p in portfolios_to_use} # == Handle suborganizations == # @@ -207,19 +203,18 @@ class Command(BaseCommand): self.suborganization_changes.create.extend(created_suborgs.values()) self.suborganization_changes.bulk_create() - # == Handle domains, requests, and managers == # + # == Handle domains and requests == # for portfolio_org_name, portfolio in portfolios_to_use_dict.items(): federal_agency = agencies_dict.get(portfolio_org_name) + suborgs = portfolio.portfolio_suborganizations.in_bulk(field_name="name") + if parse_domains: - self.handle_portfolio_domains(portfolio, federal_agency) + updated_domains = self.update_domains(portfolio, federal_agency, suborgs, debug) + self.domain_info_changes.update.extend(updated_domains) if parse_requests: - self.handle_portfolio_requests( - portfolio, federal_agency, clear_federal_agency_on_started_domain_requests, debug - ) - - if parse_managers: - self.handle_portfolio_managers(portfolio, debug) + updated_domain_requests = self.update_requests(portfolio, federal_agency, suborgs, debug) + self.domain_request_changes.update.extend(updated_domain_requests) # Update DomainInformation try: @@ -244,19 +239,18 @@ class Command(BaseCommand): logger.error(f"{TerminalColors.FAIL}Could not bulk update domain requests.{TerminalColors.ENDC}") logger.error(err, exc_info=True) - # Create UserPortfolioPermission - try: - self.user_portfolio_perm_changes.bulk_create() - except Exception as err: - logger.error(f"{TerminalColors.FAIL}Could not bulk create user portfolio permissions.{TerminalColors.ENDC}") - logger.error(err, exc_info=True) + # == Handle managers (no bulk_create) == # + if parse_managers: + domain_infos = DomainInformation.objects.filter( + portfolio__in=portfolios_to_use + ) + domains = Domain.objects.filter(domain_info__in=domain_infos) - # Create PortfolioInvitation - try: - self.portfolio_invitation_changes.bulk_create() - except Exception as err: - logger.error(f"{TerminalColors.FAIL}Could not bulk create portfolio invitations.{TerminalColors.ENDC}") - logger.error(err, exc_info=True) + # Create UserPortfolioPermission + self.create_user_portfolio_permissions(domains) + + # Create PortfolioInvitation + self.create_portfolio_invitations(domains) # == PRINT RUN SUMMARY == # self.print_final_run_summary(parse_domains, parse_requests, parse_managers, debug) @@ -359,6 +353,7 @@ class Command(BaseCommand): # Normalize all suborg names so we don't add duplicate data unintentionally. for portfolio_name, portfolio in portfolio_dict.items(): for norm_org_name, domains in domains_dict.items(): + # Don't add the record if the suborg name would equal the portfolio name if norm_org_name == portfolio_name: continue @@ -381,7 +376,6 @@ class Command(BaseCommand): suborg = Suborganization(name=new_suborg_name, portfolio=portfolio) self.set_suborganization_location(suborg, domains, requests) created_suborgs[norm_org_name] = suborg - return created_suborgs def set_suborganization_location(self, suborg, domains, requests): @@ -454,139 +448,121 @@ class Command(BaseCommand): suborg.city = normalize_string(request.city, lowercase=False) suborg.state_territory = request.state_territory - def handle_portfolio_domains(self, portfolio: Portfolio, federal_agency: FederalAgency): + def update_domains(self, portfolio, federal_agency, suborgs, debug): """ 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. """ + updated_domains = set() domain_infos = federal_agency.domaininformation_set.all() - if not domain_infos.exists(): - return None - - # 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_info in domain_infos: org_name = normalize_string(domain_info.organization_name, lowercase=False) domain_info.portfolio = portfolio domain_info.sub_organization = suborgs.get(org_name, None) - self.domain_info_changes.update.append(domain_info) + updated_domains.add(domain_info) - def handle_portfolio_requests( + if not updated_domains and debug: + message = ( + f"Portfolio '{portfolio}' not added to domains: nothing to add found." + ) + logger.warning(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") + + return updated_domains + + def update_requests( self, - portfolio: Portfolio, - federal_agency: FederalAgency, - clear_federal_agency_on_started_domain_requests: bool, - debug: bool, + portfolio, + federal_agency, + suborgs, + debug, ): """ Associate portfolio with domain requests for a federal agency. Updates all relevant domain request records. """ + updated_domain_requests = set() invalid_states = [ - DomainRequest.DomainRequestStatus.STARTED, DomainRequest.DomainRequestStatus.INELIGIBLE, DomainRequest.DomainRequestStatus.REJECTED, ] - domain_requests = DomainRequest.objects.filter(federal_agency=federal_agency).exclude(status__in=invalid_states) - if not domain_requests.exists(): - if debug: - message = ( - f"Portfolio '{portfolio}' not added to domain requests: nothing to add found." - "Excluded statuses: STARTED, INELIGIBLE, REJECTED." - ) - logger.warning(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") - return None + domain_requests = federal_agency.domainrequest_set.exclude(status__in=invalid_states) - # 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") + # Add portfolio, sub_org, requested_suborg, suborg_city, and suborg_state_territory. + # For started domain requests, set the federal agency to None if not on a portfolio. for domain_request in domain_requests: - org_name = normalize_string(domain_request.organization_name, lowercase=False) - domain_request.portfolio = portfolio - domain_request.sub_organization = suborgs.get(org_name, None) - if domain_request.sub_organization is None: - 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.domain_request_changes.update.append(domain_request) - - # For each STARTED 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. - if clear_federal_agency_on_started_domain_requests: - started_domain_requests = federal_agency.domainrequest_set.filter( - status=DomainRequest.DomainRequestStatus.STARTED, - organization_name__isnull=False, - ) - - portfolio_name = normalize_string(portfolio.organization_name) - - # Update the request, assuming the given agency name matches the portfolio name - for domain_request in started_domain_requests: + if domain_request.status != DomainRequest.DomainRequestStatus.STARTED: + org_name = normalize_string(domain_request.organization_name, lowercase=False) + domain_request.portfolio = portfolio + domain_request.sub_organization = suborgs.get(org_name, None) + if domain_request.sub_organization is None: + 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 + else: + # Clear the federal agency for started domain requests agency_name = normalize_string(domain_request.federal_agency.agency) - if agency_name == portfolio_name: + portfolio_name = normalize_string(portfolio.organization_name) + if not domain_request.portfolio and agency_name == portfolio_name: domain_request.federal_agency = None - self.domain_request_changes.update.append(domain_request) + logger.info(f"Set federal agency on started domain request '{domain_request}' to None.") + updated_domain_requests.add(domain_request) - def handle_portfolio_managers(self, portfolio: Portfolio, debug): - """ - Add all domain managers of the portfolio's domains to the organization. - This includes adding them to the correct group and creating portfolio invitations. - """ - domains = portfolio.information_portfolio.all().values_list("domain", flat=True) + if not updated_domain_requests and debug: + message = ( + f"Portfolio '{portfolio}' not added to domain requests: nothing to add found." + ) + logger.warning(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") - # Fetch all users with manager roles for the domains - user_domain_roles = UserDomainRole.objects.select_related("user").filter( - domain__in=domains, role=UserDomainRole.Roles.MANAGER + return updated_domain_requests + + def create_user_portfolio_permissions(self, domains): + user_domain_roles = UserDomainRole.objects.select_related( + "user", + "domain", + "domain__domain_info", + "domain__domain_info__portfolio" + ).filter( + domain__in=domains, + domain__domain_info__portfolio__isnull=False, + role=UserDomainRole.Roles.MANAGER ) - existing_permissions = UserPortfolioPermission.objects.filter( - user__in=user_domain_roles.values_list("user"), portfolio=portfolio - ) - existing_permissions_dict = {permission.user: permission for permission in existing_permissions} for user_domain_role in user_domain_roles: user = user_domain_role.user - if user not in existing_permissions_dict: - permission = UserPortfolioPermission( - portfolio=portfolio, - user=user, - roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], - ) + permission, created = UserPortfolioPermission.objects.get_or_create( + portfolio=user_domain_role.domain.domain_info.portfolio, + user=user, + defaults={ + "roles": [UserPortfolioRoleChoices.ORGANIZATION_MEMBER] + } + ) + if created: self.user_portfolio_perm_changes.create.append(permission) - if debug: - logger.info(f"Added manager '{permission.user}' to portfolio '{portfolio}'.") else: - existing_permission = existing_permissions_dict.get(user) - self.user_portfolio_perm_changes.skip.append(existing_permission) - if debug: - logger.info(f"Manager '{user}' already exists on portfolio '{portfolio}'.") + self.user_portfolio_perm_changes.skip.append(permission) - # Get the emails of invited managers - domain_invitations = DomainInvitation.objects.filter( - domain__in=domains, status=DomainInvitation.DomainInvitationStatus.INVITED + def create_portfolio_invitations(self, domains): + domain_invitations = DomainInvitation.objects.select_related( + "domain", + "domain__domain_info", + "domain__domain_info__portfolio" + ).filter( + domain__in=domains, + domain__domain_info__portfolio__isnull=False, + status=DomainInvitation.DomainInvitationStatus.INVITED ) - existing_invitations = PortfolioInvitation.objects.filter( - email__in=domain_invitations.values_list("email"), portfolio=portfolio - ) - existing_invitation_dict = {normalize_string(invite.email): invite for invite in existing_invitations} for domain_invitation in domain_invitations: email = normalize_string(domain_invitation.email) - if email not in existing_invitation_dict: - invitation = PortfolioInvitation( - portfolio=portfolio, - email=email, - status=PortfolioInvitation.PortfolioInvitationStatus.INVITED, - roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], - ) + invitation, created = PortfolioInvitation.objects.get_or_create( + portfolio=domain_invitation.domain.domain_info.portfolio, + email=email, + status=PortfolioInvitation.PortfolioInvitationStatus.INVITED, + roles=[UserPortfolioRoleChoices.ORGANIZATION_MEMBER], + ) + if created: self.portfolio_invitation_changes.create.append(invitation) - if debug: - logger.info(f"Added invitation '{email}' to portfolio '{portfolio}'.") else: - existing_invitation = existing_invitations.get(email) - self.portfolio_invitation_changes.skip.append(existing_invitation) - if debug: - logger.info(f"Invitation '{email}' already exists in portfolio '{portfolio}'.") + self.portfolio_invitation_changes.skip.append(invitation) diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 4a17c7d61..9fdd5ba8e 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1530,13 +1530,10 @@ class TestCreateFederalPortfolio(TestCase): @less_console_noise_decorator 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. - """ + """Ensures that federal agency is cleared when agency name matches portfolio name.""" 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 matching_request = completed_domain_request( From 1ccaea502824cfa18277f88f912dc7a00798c88b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 21 Mar 2025 13:20:22 -0600 Subject: [PATCH 22/27] lintomatic --- .../commands/create_federal_portfolio.py | 52 +++++++------------ .../commands/utility/terminal_helper.py | 4 +- 2 files changed, 22 insertions(+), 34 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 89ccfb1da..2113ed97a 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -43,7 +43,9 @@ class Command(BaseCommand): def bulk_create(self): try: - res = ScriptDataHelper.bulk_create_fields(self.model_class, self.create, return_created=True, quiet=True) + res = ScriptDataHelper.bulk_create_fields( + self.model_class, self.create, return_created=True, quiet=True + ) self.create = res return res except Exception as err: @@ -241,9 +243,7 @@ class Command(BaseCommand): # == Handle managers (no bulk_create) == # if parse_managers: - domain_infos = DomainInformation.objects.filter( - portfolio__in=portfolios_to_use - ) + domain_infos = DomainInformation.objects.filter(portfolio__in=portfolios_to_use) domains = Domain.objects.filter(domain_info__in=domain_infos) # Create UserPortfolioPermission @@ -320,7 +320,8 @@ class Command(BaseCommand): # Org name must not be null, and must not be the portfolio name Q( organization_name__isnull=False, - ) & ~Q(organization_name__iexact=F("portfolio__organization_name")), + ) + & ~Q(organization_name__iexact=F("portfolio__organization_name")), # Only get relevant data to the agency/portfolio we are targeting Q(federal_agency__in=agencies) | Q(portfolio__in=portfolios), ) @@ -328,7 +329,8 @@ class Command(BaseCommand): # Org name must not be null, and must not be the portfolio name Q( organization_name__isnull=False, - ) & ~Q(organization_name__iexact=F("portfolio__organization_name")), + ) + & ~Q(organization_name__iexact=F("portfolio__organization_name")), # Only get relevant data to the agency/portfolio we are targeting Q(federal_agency__in=agencies) | Q(portfolio__in=portfolios), ) @@ -356,7 +358,7 @@ class Command(BaseCommand): # Don't add the record if the suborg name would equal the portfolio name if norm_org_name == portfolio_name: continue - + new_suborg_name = None if len(domains) == 1: new_suborg_name = domains[0].organization_name @@ -365,7 +367,8 @@ class Command(BaseCommand): best_record = max( domains, key=lambda rank: ( - -domain.organization_name.count(" "), count_capitals(domain.organization_name, leading_only=True) + -domain.organization_name.count(" "), + count_capitals(domain.organization_name, leading_only=True), ), ) new_suborg_name = best_record.organization_name @@ -397,8 +400,8 @@ class Command(BaseCommand): use_location_for_domain = all( d.city and d.state_territory - and d.city == reference.city - and d.state_territory == reference.state_territory + and d.city == reference.city + and d.state_territory == reference.state_territory for d in domains ) if use_location_for_domain: @@ -464,9 +467,7 @@ class Command(BaseCommand): updated_domains.add(domain_info) if not updated_domains and debug: - message = ( - f"Portfolio '{portfolio}' not added to domains: nothing to add found." - ) + message = f"Portfolio '{portfolio}' not added to domains: nothing to add found." logger.warning(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") return updated_domains @@ -512,32 +513,21 @@ class Command(BaseCommand): updated_domain_requests.add(domain_request) if not updated_domain_requests and debug: - message = ( - f"Portfolio '{portfolio}' not added to domain requests: nothing to add found." - ) + message = f"Portfolio '{portfolio}' not added to domain requests: nothing to add found." logger.warning(f"{TerminalColors.YELLOW}{message}{TerminalColors.ENDC}") return updated_domain_requests def create_user_portfolio_permissions(self, domains): user_domain_roles = UserDomainRole.objects.select_related( - "user", - "domain", - "domain__domain_info", - "domain__domain_info__portfolio" - ).filter( - domain__in=domains, - domain__domain_info__portfolio__isnull=False, - role=UserDomainRole.Roles.MANAGER - ) + "user", "domain", "domain__domain_info", "domain__domain_info__portfolio" + ).filter(domain__in=domains, domain__domain_info__portfolio__isnull=False, role=UserDomainRole.Roles.MANAGER) for user_domain_role in user_domain_roles: user = user_domain_role.user permission, created = UserPortfolioPermission.objects.get_or_create( portfolio=user_domain_role.domain.domain_info.portfolio, user=user, - defaults={ - "roles": [UserPortfolioRoleChoices.ORGANIZATION_MEMBER] - } + defaults={"roles": [UserPortfolioRoleChoices.ORGANIZATION_MEMBER]}, ) if created: self.user_portfolio_perm_changes.create.append(permission) @@ -546,13 +536,11 @@ class Command(BaseCommand): def create_portfolio_invitations(self, domains): domain_invitations = DomainInvitation.objects.select_related( - "domain", - "domain__domain_info", - "domain__domain_info__portfolio" + "domain", "domain__domain_info", "domain__domain_info__portfolio" ).filter( domain__in=domains, domain__domain_info__portfolio__isnull=False, - status=DomainInvitation.DomainInvitationStatus.INVITED + status=DomainInvitation.DomainInvitationStatus.INVITED, ) for domain_invitation in domain_invitations: email = normalize_string(domain_invitation.email) diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index e8ec5e41f..d57093505 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -52,7 +52,7 @@ class ScriptDataHelper: Usage: ScriptDataHelper.bulk_update_fields(Domain, page.object_list, ["first_ready"]) - + Returns: A queryset of the updated objets """ if not quiet: @@ -82,7 +82,7 @@ class ScriptDataHelper: or large field values, you may need to decrease this value to prevent out-of-memory errors. Usage: ScriptDataHelper.bulk_add_fields(Domain, page.object_list) - + Returns: A queryset of the added objects """ if not quiet: From 3cefc4c24aa4e6a65eda8de7a19edefaaa96f086 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 24 Mar 2025 08:20:55 -0600 Subject: [PATCH 23/27] fix tests and lint --- docs/operations/data_migration.md | 20 +++++++++---------- .../commands/create_federal_portfolio.py | 6 +++--- .../tests/test_management_scripts.py | 14 ++++++++----- 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index 8185922a4..402489a47 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -894,32 +894,32 @@ Example: `cf ssh getgov-za` #### Step 5: Running the script To create a specific portfolio: -```./manage.py create_federal_portfolio --agency_name "{federal_agency_name}" --both``` +```./manage.py create_federal_portfolio --agency_name "{federal_agency_name}" --parse_domains --parse_requests --parse_managers``` Example (only requests): `./manage.py create_federal_portfolio "AMTRAK" --parse_requests` To create a portfolios for all federal agencies in a branch: -```./manage.py create_federal_portfolio --branch "{executive|legislative|judicial}" --both``` +```./manage.py create_federal_portfolio --branch "{executive|legislative|judicial}" --parse_domains --parse_requests --parse_managers``` Example (only requests): `./manage.py create_federal_portfolio --branch "executive" --parse_requests` ### Running locally #### Step 1: Running the script -```docker-compose exec app ./manage.py create_federal_portfolio --agency_name "{federal_agency_name}" --both``` +```docker-compose exec app ./manage.py create_federal_portfolio --agency_name "{federal_agency_name}" --parse_domains``` ##### Parameters | | Parameter | Description | |:-:|:---------------------------- |:-------------------------------------------------------------------------------------------| | 1 | **agency_name** | Name of the FederalAgency record surrounded by quotes. For instance,"AMTRAK". | | 2 | **branch** | Creates a portfolio for each federal agency in a branch: executive, legislative, judicial | -| 3 | **both** | If True, runs parse_requests and parse_domains. | -| 4 | **parse_requests** | If True, then the created portfolio is added to all related DomainRequests. | -| 5 | **parse_domains** | If True, then the created portfolio is added to all related Domains. | -| 6 | **add_managers** | If True, then the created portfolio will add all managers of the portfolio domains as members of the portfolio, including invited managers. | -| 7 | **skip_existing_portfolios** | If True, then the script will only create suborganizations, modify DomainRequest, and modify DomainInformation records only when creating a new portfolio. Use this flag when you do not want to modify existing records. | +| 3 | **parse_requests** | If True, then the created portfolio is added to all related DomainRequests. | +| 4 | **parse_domains** | If True, then the created portfolio is added to all related Domains. | +| 5 | **parse_members** | If True, then the created portfolio will add all managers of the portfolio domains as members of the portfolio, including invited managers. | +| 6 | **skip_existing_portfolios** | If True, then the script will only create suborganizations, modify DomainRequest, and modify DomainInformation records only when creating a new portfolio. Use this flag when you do not want to modify existing records. | +| 7 | **Debug** | Increases log verbosity | - 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. +- Parameters #3-#5, 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. You can also chain all of them together. ## Patch suborganizations diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 2113ed97a..aaee6fbde 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -257,7 +257,7 @@ class Command(BaseCommand): def print_final_run_summary(self, parse_domains, parse_requests, parse_managers, debug): self.portfolio_changes.print_script_run_summary( - no_changes_message="\n||============= No portfolios changed. =============||", + no_changes_message="||============= No portfolios changed. =============||", debug=debug, log_header="============= PORTFOLIOS =============", skipped_header="----- SOME PORTFOLIOS WERENT CREATED (BUT OTHER RECORDS ARE STILL PROCESSED) -----", @@ -265,7 +265,7 @@ class Command(BaseCommand): display_as_str=True, ) self.suborganization_changes.print_script_run_summary( - no_changes_message="\n||============= No suborganizations changed. =============||", + no_changes_message="||============= No suborganizations changed. =============||", debug=debug, log_header="============= SUBORGANIZATIONS =============", skipped_header="----- SUBORGANIZATIONS SKIPPED (SAME NAME AS PORTFOLIO NAME) -----", @@ -507,7 +507,7 @@ class Command(BaseCommand): # Clear the federal agency for started domain requests agency_name = normalize_string(domain_request.federal_agency.agency) portfolio_name = normalize_string(portfolio.organization_name) - if not domain_request.portfolio and agency_name == portfolio_name: + if agency_name == portfolio_name: domain_request.federal_agency = None logger.info(f"Set federal agency on started domain request '{domain_request}' to None.") updated_domain_requests.add(domain_request) diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 9fdd5ba8e..5d9dc6759 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1473,7 +1473,7 @@ class TestCreateFederalPortfolio(TestCase): generic_org_type=DomainRequest.OrganizationChoices.CITY, federal_agency=self.federal_agency, user=self.user, - organization_name="Testorg" + organization_name="Testorg", ) self.domain_request.approve() self.domain_info = DomainInformation.objects.filter(domain_request=self.domain_request).get() @@ -1875,7 +1875,9 @@ class TestCreateFederalPortfolio(TestCase): UserDomainRole.objects.create(user=manager2, domain=self.domain, role=UserDomainRole.Roles.MANAGER) # Run the management command - self.run_create_federal_portfolio(agency_name=self.federal_agency.agency, parse_domains=True, parse_managers=True) + self.run_create_federal_portfolio( + agency_name=self.federal_agency.agency, parse_domains=True, parse_managers=True + ) # Check that the portfolio was created self.portfolio = Portfolio.objects.get(federal_agency=self.federal_agency) @@ -1898,7 +1900,9 @@ class TestCreateFederalPortfolio(TestCase): ) # Run the management command - self.run_create_federal_portfolio(agency_name=self.federal_agency.agency, parse_domains=True, parse_managers=True) + self.run_create_federal_portfolio( + agency_name=self.federal_agency.agency, parse_domains=True, parse_managers=True + ) # Check that the portfolio was created self.portfolio = Portfolio.objects.get(federal_agency=self.federal_agency) @@ -1995,9 +1999,9 @@ class TestCreateFederalPortfolio(TestCase): skip_existing_portfolios=True, ) - # Check that managers were added to the portfolio + # Check that managers weren't added to the portfolio permissions = UserPortfolioPermission.objects.filter(portfolio=self.portfolio, user__in=[manager1, manager2]) - self.assertEqual(permissions.count(), 2) + self.assertEqual(permissions.count(), 0) for perm in permissions: self.assertIn(UserPortfolioRoleChoices.ORGANIZATION_MEMBER, perm.roles) From 36880bc5146e7824088d45d5e93f09995b191a0e Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 24 Mar 2025 09:20:39 -0600 Subject: [PATCH 24/27] Update create_federal_portfolio.py --- .../management/commands/create_federal_portfolio.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index aaee6fbde..7df07bd5e 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -336,7 +336,7 @@ class Command(BaseCommand): ) # First: get all existing suborgs - existing_suborgs = Suborganization.objects.filter(portfolio__in=portfolios) + existing_suborgs = Suborganization.objects.all() suborg_dict = {normalize_string(org.name): org for org in existing_suborgs} # Second: Group domains and requests by normalized organization name. @@ -354,6 +354,7 @@ class Command(BaseCommand): # then create *one* suborg record from it. # Normalize all suborg names so we don't add duplicate data unintentionally. for portfolio_name, portfolio in portfolio_dict.items(): + # For a given agency, find all domains that list suborg info for it. for norm_org_name, domains in domains_dict.items(): # Don't add the record if the suborg name would equal the portfolio name if norm_org_name == portfolio_name: @@ -361,7 +362,7 @@ class Command(BaseCommand): new_suborg_name = None if len(domains) == 1: - new_suborg_name = domains[0].organization_name + new_suborg_name = normalize_string(domains[0].organization_name, lowercase=False) elif len(domains) > 1: # Pick the best record for a suborg name (fewest spaces, most leading capitals) best_record = max( @@ -371,7 +372,7 @@ class Command(BaseCommand): count_capitals(domain.organization_name, leading_only=True), ), ) - new_suborg_name = best_record.organization_name + new_suborg_name = normalize_string(best_record.organization_name, lowercase=False) # If the suborg already exists, don't add it again. if norm_org_name not in suborg_dict and norm_org_name not in created_suborgs: From ce22ddd62c2984470d09fb6f2caa78ce54b0ee4a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 24 Mar 2025 09:24:52 -0600 Subject: [PATCH 25/27] Add comment --- src/registrar/management/commands/create_federal_portfolio.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 7df07bd5e..3dbb836c1 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -336,6 +336,10 @@ class Command(BaseCommand): ) # First: get all existing suborgs + # NOTE: .all() is a heavy query, but unavoidable as we need to check for duplicate names. + # This is not quite as heavy as just using a for loop and .get_or_create, but worth noting. + # Change this if you can find a way to avoid doing this. + # This won't scale great for 10k+ records. existing_suborgs = Suborganization.objects.all() suborg_dict = {normalize_string(org.name): org for org in existing_suborgs} From 9cbd2242dc2429ec138261eea8132464f149fd57 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 24 Mar 2025 09:27:32 -0600 Subject: [PATCH 26/27] lint --- .../commands/create_federal_portfolio.py | 36 ++++++++++++------- 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index 3dbb836c1..faebb7006 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -258,37 +258,45 @@ class Command(BaseCommand): def print_final_run_summary(self, parse_domains, parse_requests, parse_managers, debug): self.portfolio_changes.print_script_run_summary( no_changes_message="||============= No portfolios changed. =============||", - debug=debug, log_header="============= PORTFOLIOS =============", skipped_header="----- SOME PORTFOLIOS WERENT CREATED (BUT OTHER RECORDS ARE STILL PROCESSED) -----", - detailed_prompt_title="PORTFOLIOS: Do you wish to see the full list of failed, skipped and updated records?", + detailed_prompt_title=( + "PORTFOLIOS: Do you wish to see the full list of failed, skipped and updated records?" + ), display_as_str=True, + debug=debug, ) self.suborganization_changes.print_script_run_summary( no_changes_message="||============= No suborganizations changed. =============||", - debug=debug, log_header="============= SUBORGANIZATIONS =============", skipped_header="----- SUBORGANIZATIONS SKIPPED (SAME NAME AS PORTFOLIO NAME) -----", - detailed_prompt_title="SUBORGANIZATIONS: Do you wish to see the full list of failed, skipped and updated records?", + detailed_prompt_title=( + "SUBORGANIZATIONS: Do you wish to see the full list of failed, skipped and updated records?" + ), display_as_str=True, + debug=debug, ) if parse_domains: self.domain_info_changes.print_script_run_summary( no_changes_message="||============= No domains changed. =============||", - debug=debug, log_header="============= DOMAINS =============", - detailed_prompt_title="DOMAINS: Do you wish to see the full list of failed, skipped and updated records?", + detailed_prompt_title=( + "DOMAINS: Do you wish to see the full list of failed, skipped and updated records?" + ), display_as_str=True, + debug=debug, ) if parse_requests: self.domain_request_changes.print_script_run_summary( no_changes_message="||============= No domain requests changed. =============||", - debug=debug, log_header="============= DOMAIN REQUESTS =============", - detailed_prompt_title="DOMAIN REQUESTS: Do you wish to see the full list of failed, skipped and updated records?", + detailed_prompt_title=( + "DOMAIN REQUESTS: Do you wish to see the full list of failed, skipped and updated records?" + ), display_as_str=True, + debug=debug, ) if parse_managers: @@ -296,17 +304,21 @@ class Command(BaseCommand): no_changes_message="||============= No managers changed. =============||", log_header="============= MANAGERS =============", skipped_header="----- MANAGERS SKIPPED (ALREADY EXISTED) -----", - debug=debug, - detailed_prompt_title="MANAGERS: Do you wish to see the full list of failed, skipped and updated records?", + detailed_prompt_title=( + "MANAGERS: Do you wish to see the full list of failed, skipped and updated records?" + ), display_as_str=True, + debug=debug, ) self.portfolio_invitation_changes.print_script_run_summary( no_changes_message="||============= No manager invitations changed. =============||", log_header="============= MANAGER INVITATIONS =============", - debug=debug, skipped_header="----- INVITATIONS SKIPPED (ALREADY EXISTED) -----", - detailed_prompt_title="MANAGER INVITATIONS: Do you wish to see the full list of failed, skipped and updated records?", + detailed_prompt_title=( + "MANAGER INVITATIONS: Do you wish to see the full list of failed, skipped and updated records?" + ), display_as_str=True, + debug=debug, ) def create_suborganizations(self, portfolio_dict, agency_dict): From 27871ee5e0fdd1f3e08e9e0d872a72f63a76d50f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 24 Mar 2025 10:06:40 -0600 Subject: [PATCH 27/27] Update create_federal_portfolio.py --- src/registrar/management/commands/create_federal_portfolio.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/registrar/management/commands/create_federal_portfolio.py b/src/registrar/management/commands/create_federal_portfolio.py index faebb7006..5e97bf44c 100644 --- a/src/registrar/management/commands/create_federal_portfolio.py +++ b/src/registrar/management/commands/create_federal_portfolio.py @@ -56,9 +56,7 @@ class Command(BaseCommand): def bulk_update(self, fields_to_update): try: - res = ScriptDataHelper.bulk_update_fields(self.model_class, self.update, fields_to_update, quiet=True) - self.update = res - return res + ScriptDataHelper.bulk_update_fields(self.model_class, self.update, fields_to_update, quiet=True) except Exception as err: # In this case, just swap the fail and update lists self.fail = self.update.copy()