From 35a0ed751e5b52b3e1c929a21515f9abbb77f371 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 16 Nov 2023 13:16:01 -0700 Subject: [PATCH 01/46] Parsing org data --- .../commands/load_organization_data.py | 47 +++ .../utility/extra_transition_domain_helper.py | 338 +++++++++++++++--- .../utility/transition_domain_arguments.py | 9 +- ...ess_line_transitiondomain_city_and_more.py | 37 ++ src/registrar/models/transition_domain.py | 35 ++ 5 files changed, 418 insertions(+), 48 deletions(-) create mode 100644 src/registrar/management/commands/load_organization_data.py create mode 100644 src/registrar/migrations/0047_transitiondomain_address_line_transitiondomain_city_and_more.py diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py new file mode 100644 index 000000000..d5ae51a46 --- /dev/null +++ b/src/registrar/management/commands/load_organization_data.py @@ -0,0 +1,47 @@ +"""Data migration: Send domain invitations once to existing customers.""" + +import argparse +import logging +import copy +import time + +from django.core.management import BaseCommand +from registrar.management.commands.utility.extra_transition_domain_helper import OrganizationDataLoader +from registrar.management.commands.utility.transition_domain_arguments import TransitionDomainArguments +from registrar.models import TransitionDomain +from ...utility.email import send_templated_email, EmailSendingError +from typing import List + +logger = logging.getLogger(__name__) + + +class Command(BaseCommand): + help = "Send domain invitations once to existing customers." + + def add_arguments(self, parser): + """Add command line arguments.""" + + parser.add_argument("--sep", default="|", help="Delimiter character") + + parser.add_argument("--debug", action=argparse.BooleanOptionalAction) + + parser.add_argument("--directory", default="migrationdata", help="Desired directory") + + parser.add_argument( + "--domain_additional_filename", + help="Defines the filename for additional domain data", + required=True, + ) + + parser.add_argument( + "--organization_adhoc_filename", + help="Defines the filename for domain type adhocs", + required=True, + ) + + def handle(self, **options): + """Process the objects in TransitionDomain.""" + args = TransitionDomainArguments(**options) + + load = OrganizationDataLoader(args) + load.update_organization_data_for_all() diff --git a/src/registrar/management/commands/utility/extra_transition_domain_helper.py b/src/registrar/management/commands/utility/extra_transition_domain_helper.py index 7961b47c1..640fe9875 100644 --- a/src/registrar/management/commands/utility/extra_transition_domain_helper.py +++ b/src/registrar/management/commands/utility/extra_transition_domain_helper.py @@ -751,6 +751,257 @@ class FileDataHolder: full_filename = date + "." + filename_without_date return (full_filename, can_infer) +class OrganizationDataLoader: + """Saves organization data onto Transition Domains. Handles file parsing.""" + def __init__(self, options: TransitionDomainArguments): + # Globally stores event logs and organizes them + self.parse_logs = FileTransitionLog() + self.debug = options.debug + + options.pattern_map_params = [ + ( + EnumFilenames.DOMAIN_ADDITIONAL, + options.domain_additional_filename, + DomainAdditionalData, + "domainname", + ), + ( + EnumFilenames.ORGANIZATION_ADHOC, + options.organization_adhoc_filename, + OrganizationAdhoc, + "orgid", + ), + ] + # Reads and parses organization data + self.parsed_data = ExtraTransitionDomain(options) + # options.infer_filenames will always be false when not SETTING.DEBUG + self.parsed_data.parse_all_files(options.infer_filenames) + + self.tds_to_update = [] + self.tds_failed_to_update = [] + + def update_organization_data_for_all(self): + """Updates org data for all TransitionDomains""" + all_transition_domains = TransitionDomain.objects.all() + if len(all_transition_domains) < 1: + logger.error( + f"{TerminalColors.FAIL}" + "No TransitionDomains exist. Cannot update." + f"{TerminalColors.ENDC}" + ) + return None + + # Store all actions we want to perform in tds_to_update + self.prepare_transition_domains(all_transition_domains) + # Then if we don't run into any exceptions, bulk_update it + self.bulk_update_transition_domains(self.tds_to_update) + + def prepare_transition_domains(self, transition_domains): + for item in transition_domains: + try: + updated = self.parse_org_data(item.domain_name, item) + self.tds_to_update.append(updated) + if self.debug: + logger.info(item.display_transition_domain()) + logger.info( + f"{TerminalColors.OKCYAN}" + f"{item.display_transition_domain()}" + f"{TerminalColors.ENDC}" + ) + except Exception as err: + logger.error(err) + self.tds_failed_to_update.append(item) + if self.debug: + logger.error( + f"{TerminalColors.YELLOW}" + f"{item.display_transition_domain()}" + f"{TerminalColors.ENDC}" + ) + + if len(self.tds_failed_to_update) > 0: + logger.error( + "Failed to update. An exception was encountered " + f"on the following TransitionDomains: {[item for item in self.tds_failed_to_update]}" + ) + raise Exception("Failed to update TransitionDomains") + + if not self.debug: + logger.info( + f"Ready to update {len(self.tds_to_update)} TransitionDomains." + ) + else: + logger.info( + f"Ready to update {len(self.tds_to_update)} TransitionDomains: {[item for item in self.tds_failed_to_update]}" + ) + + def bulk_update_transition_domains(self, update_list): + logger.info( + f"{TerminalColors.MAGENTA}" + "Beginning mass TransitionDomain update..." + f"{TerminalColors.ENDC}" + ) + + changed_fields = [ + "address_line", + "city", + "state_territory", + "zipcode", + "country_code", + ] + + TransitionDomain.objects.bulk_update(update_list, changed_fields) + + if not self.debug: + logger.info( + f"{TerminalColors.OKGREEN}" + f"Updated {len(self.tds_to_update)} TransitionDomains." + f"{TerminalColors.ENDC}" + ) + else: + logger.info( + f"{TerminalColors.OKGREEN}" + f"Updated {len(self.tds_to_update)} TransitionDomains: {[item for item in self.tds_failed_to_update]}" + f"{TerminalColors.ENDC}" + ) + + def parse_org_data(self, domain_name, transition_domain: TransitionDomain) -> TransitionDomain: + """Grabs organization_name from the parsed files and associates it + with a transition_domain object, then returns that object.""" + if not isinstance(transition_domain, TransitionDomain): + raise ValueError("Not a valid object, must be TransitionDomain") + + org_info = self.get_org_info(domain_name) + if org_info is None: + self.parse_logs.create_log_item( + EnumFilenames.ORGANIZATION_ADHOC, + LogCode.ERROR, + f"Could not add organization_name on {domain_name}, no data exists.", + domain_name, + not self.debug, + ) + return transition_domain + + # Add street info + transition_domain.address_line = org_info.orgstreet + transition_domain.city = org_info.orgcity + transition_domain.state_territory = org_info.orgstate + transition_domain.zipcode = org_info.orgzip + transition_domain.country_code = org_info.orgcountrycode + + # Log what happened to each field + changed_fields = [ + ("address_line", transition_domain.address_line), + ("city", transition_domain.city), + ("state_territory", transition_domain.state_territory), + ("zipcode", transition_domain.zipcode), + ("country_code", transition_domain.country_code), + ] + self.log_add_or_changed_values(EnumFilenames.AUTHORITY_ADHOC, changed_fields, domain_name) + + return transition_domain + + def get_org_info(self, domain_name) -> OrganizationAdhoc: + """Maps an id given in get_domain_data to a organization_adhoc + record which has its corresponding definition""" + domain_info = self.get_domain_data(domain_name) + if domain_info is None: + return None + org_id = domain_info.orgid + return self.get_organization_adhoc(org_id) + + def get_organization_adhoc(self, desired_id) -> OrganizationAdhoc: + """Grabs a corresponding row within the ORGANIZATION_ADHOC file, + based off a desired_id""" + return self.get_object_by_id(EnumFilenames.ORGANIZATION_ADHOC, desired_id) + + def get_domain_data(self, desired_id) -> DomainAdditionalData: + """Grabs a corresponding row within the DOMAIN_ADDITIONAL file, + based off a desired_id""" + return self.get_object_by_id(EnumFilenames.DOMAIN_ADDITIONAL, desired_id) + + def get_object_by_id(self, file_type: EnumFilenames, desired_id): + """Returns a field in a dictionary based off the type and id. + + vars: + file_type: (constant) EnumFilenames -> Which data file to target. + An example would be `EnumFilenames.DOMAIN_ADHOC`. + + desired_id: str -> Which id you want to search on. + An example would be `"12"` or `"igorville.gov"` + + Explanation: + Each data file has an associated type (file_type) for tracking purposes. + + Each file_type is a dictionary which + contains a dictionary of row[id_field]: object. + + In practice, this would look like: + + EnumFilenames.AUTHORITY_ADHOC: { + "1": AuthorityAdhoc(...), + "2": AuthorityAdhoc(...), + ... + } + + desired_id will then specify which id to grab. If we wanted "1", + then this function will return the value of id "1". + So, `AuthorityAdhoc(...)` + """ + # Grabs a dict associated with the file_type. + # For example, EnumFilenames.DOMAIN_ADDITIONAL. + desired_type = self.parsed_data.file_data.get(file_type) + if desired_type is None: + self.parse_logs.create_log_item( + file_type, + LogCode.ERROR, + f"Type {file_type} does not exist", + ) + return None + + # Grab the value given an Id within that file_type dict. + # For example, "igorville.gov". + obj = desired_type.data.get(desired_id) + if obj is None: + self.parse_logs.create_log_item( + file_type, + LogCode.ERROR, + f"Id {desired_id} does not exist for {file_type.value[0]}", + ) + return obj + + def log_add_or_changed_values(self, file_type, values_to_check, domain_name): + for field_name, value in values_to_check: + str_exists = value is not None and value.strip() != "" + # Logs if we either added to this property, + # or modified it. + self._add_or_change_message( + file_type, + field_name, + value, + domain_name, + str_exists, + ) + + def _add_or_change_message(self, file_type, var_name, changed_value, domain_name, is_update=False): + """Creates a log instance when a property + is successfully changed on a given TransitionDomain.""" + if not is_update: + self.parse_logs.create_log_item( + file_type, + LogCode.INFO, + f"Added {var_name} as '{changed_value}' on {domain_name}", + domain_name, + not self.debug, + ) + else: + self.parse_logs.create_log_item( + file_type, + LogCode.WARNING, + f"Updated existing {var_name} to '{changed_value}' on {domain_name}", + domain_name, + not self.debug, + ) + class ExtraTransitionDomain: """Helper class to aid in storing TransitionDomain data spread across @@ -775,52 +1026,49 @@ class ExtraTransitionDomain: # metadata about each file and associate it with an enum. # That way if we want the data located at the agency_adhoc file, # we can just call EnumFilenames.AGENCY_ADHOC. - pattern_map_params = [ - ( - EnumFilenames.AGENCY_ADHOC, - options.agency_adhoc_filename, - AgencyAdhoc, - "agencyid", - ), - ( - EnumFilenames.DOMAIN_ADDITIONAL, - options.domain_additional_filename, - DomainAdditionalData, - "domainname", - ), - ( - EnumFilenames.DOMAIN_ESCROW, - options.domain_escrow_filename, - DomainEscrow, - "domainname", - ), - ( - EnumFilenames.DOMAIN_ADHOC, - options.domain_adhoc_filename, - DomainTypeAdhoc, - "domaintypeid", - ), - ( - EnumFilenames.ORGANIZATION_ADHOC, - options.organization_adhoc_filename, - OrganizationAdhoc, - "orgid", - ), - ( - EnumFilenames.AUTHORITY_ADHOC, - options.authority_adhoc_filename, - AuthorityAdhoc, - "authorityid", - ), - ( - EnumFilenames.AUTHORITY_ADHOC, - options.authority_adhoc_filename, - AuthorityAdhoc, - "authorityid", - ), - ] + if ( + options.pattern_map_params is None or options.pattern_map_params == [] + ): + options.pattern_map_params = [ + ( + EnumFilenames.AGENCY_ADHOC, + options.agency_adhoc_filename, + AgencyAdhoc, + "agencyid", + ), + ( + EnumFilenames.DOMAIN_ADDITIONAL, + options.domain_additional_filename, + DomainAdditionalData, + "domainname", + ), + ( + EnumFilenames.DOMAIN_ESCROW, + options.domain_escrow_filename, + DomainEscrow, + "domainname", + ), + ( + EnumFilenames.DOMAIN_ADHOC, + options.domain_adhoc_filename, + DomainTypeAdhoc, + "domaintypeid", + ), + ( + EnumFilenames.ORGANIZATION_ADHOC, + options.organization_adhoc_filename, + OrganizationAdhoc, + "orgid", + ), + ( + EnumFilenames.AUTHORITY_ADHOC, + options.authority_adhoc_filename, + AuthorityAdhoc, + "authorityid", + ), + ] - self.file_data = self.populate_file_data(pattern_map_params) + self.file_data = self.populate_file_data(options.pattern_map_params) # TODO - revise comment def populate_file_data(self, pattern_map_params): diff --git a/src/registrar/management/commands/utility/transition_domain_arguments.py b/src/registrar/management/commands/utility/transition_domain_arguments.py index 56425a7b7..bfe1dd84e 100644 --- a/src/registrar/management/commands/utility/transition_domain_arguments.py +++ b/src/registrar/management/commands/utility/transition_domain_arguments.py @@ -1,5 +1,5 @@ from dataclasses import dataclass, field -from typing import Optional +from typing import List, Optional from registrar.management.commands.utility.epp_data_containers import EnumFilenames @@ -18,6 +18,9 @@ class TransitionDomainArguments: # Maintains an internal kwargs list and sets values # that match the class definition. def __init__(self, **kwargs): + self.pattern_map_params = [] + if "self.pattern_map_params" in kwargs: + self.pattern_map_params = kwargs["pattern_map_params"] self.kwargs = kwargs for k, v in kwargs.items(): if hasattr(self, k): @@ -36,13 +39,13 @@ class TransitionDomainArguments: limitParse: Optional[int] = field(default=None, repr=True) # Filenames # - # = Adhocs =# + # = Adhocs = # agency_adhoc_filename: Optional[str] = field(default=EnumFilenames.AGENCY_ADHOC.value[1], repr=True) domain_adhoc_filename: Optional[str] = field(default=EnumFilenames.DOMAIN_ADHOC.value[1], repr=True) organization_adhoc_filename: Optional[str] = field(default=EnumFilenames.ORGANIZATION_ADHOC.value[1], repr=True) authority_adhoc_filename: Optional[str] = field(default=EnumFilenames.AUTHORITY_ADHOC.value[1], repr=True) - # = Data files =# + # = Data files = # domain_escrow_filename: Optional[str] = field(default=EnumFilenames.DOMAIN_ESCROW.value[1], repr=True) domain_additional_filename: Optional[str] = field(default=EnumFilenames.DOMAIN_ADDITIONAL.value[1], repr=True) domain_contacts_filename: Optional[str] = field(default=None, repr=True) diff --git a/src/registrar/migrations/0047_transitiondomain_address_line_transitiondomain_city_and_more.py b/src/registrar/migrations/0047_transitiondomain_address_line_transitiondomain_city_and_more.py new file mode 100644 index 000000000..51ce8d6a2 --- /dev/null +++ b/src/registrar/migrations/0047_transitiondomain_address_line_transitiondomain_city_and_more.py @@ -0,0 +1,37 @@ +# Generated by Django 4.2.7 on 2023-11-16 19:56 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("registrar", "0046_transitiondomain_email_transitiondomain_first_name_and_more"), + ] + + operations = [ + migrations.AddField( + model_name="transitiondomain", + name="address_line", + field=models.TextField(blank=True, help_text="Street address", null=True), + ), + migrations.AddField( + model_name="transitiondomain", + name="city", + field=models.TextField(blank=True, help_text="City", null=True), + ), + migrations.AddField( + model_name="transitiondomain", + name="country_code", + field=models.CharField(blank=True, help_text="Country code", max_length=2, null=True), + ), + migrations.AddField( + model_name="transitiondomain", + name="state_territory", + field=models.CharField(blank=True, help_text="State, territory, or military post", max_length=2, null=True), + ), + migrations.AddField( + model_name="transitiondomain", + name="zipcode", + field=models.CharField(blank=True, db_index=True, help_text="Zip code", max_length=10, null=True), + ), + ] diff --git a/src/registrar/models/transition_domain.py b/src/registrar/models/transition_domain.py index 3f1c8d641..0f1e0a7bf 100644 --- a/src/registrar/models/transition_domain.py +++ b/src/registrar/models/transition_domain.py @@ -105,6 +105,36 @@ class TransitionDomain(TimeStampedModel): blank=True, help_text="Phone", ) + address_line = models.TextField( + null=True, + blank=True, + help_text="Street address", + ) + city = models.TextField( + null=True, + blank=True, + help_text="City", + ) + state_territory = models.CharField( + max_length=2, + null=True, + blank=True, + help_text="State, territory, or military post", + ) + zipcode = models.CharField( + max_length=10, + null=True, + blank=True, + help_text="Zip code", + db_index=True, + ) + country_code = models.CharField( + max_length=2, + null=True, + blank=True, + help_text="Country code", + ) + # TODO - Country code? def __str__(self): return f"{self.username}, {self.domain_name}" @@ -128,4 +158,9 @@ class TransitionDomain(TimeStampedModel): f"last_name: {self.last_name}, \n" f"email: {self.email}, \n" f"phone: {self.phone}, \n" + f"address_line: {self.address_line}, \n" + f"city: {self.city}, \n" + f"state_territory: {self.state_territory}, \n" + f"zipcode: {self.zipcode}, \n" + f"country_code: {self.country_code}, \n" ) From 44cd47e51d170814f48473d69f6d28d847a9e680 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 16 Nov 2023 14:42:20 -0700 Subject: [PATCH 02/46] Add DomainInformation data --- .../commands/load_organization_data.py | 152 +++++++++++++++++- .../utility/extra_transition_domain_helper.py | 4 +- src/registrar/models/transition_domain.py | 1 - 3 files changed, 153 insertions(+), 4 deletions(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index d5ae51a46..550f4def9 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -7,8 +7,11 @@ import time from django.core.management import BaseCommand from registrar.management.commands.utility.extra_transition_domain_helper import OrganizationDataLoader +from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper from registrar.management.commands.utility.transition_domain_arguments import TransitionDomainArguments from registrar.models import TransitionDomain +from registrar.models.domain import Domain +from registrar.models.domain_information import DomainInformation from ...utility.email import send_templated_email, EmailSendingError from typing import List @@ -43,5 +46,152 @@ class Command(BaseCommand): """Process the objects in TransitionDomain.""" args = TransitionDomainArguments(**options) + changed_fields = [ + "address_line", + "city", + "state_territory", + "zipcode", + "country_code", + ] + proceed = TerminalHelper.prompt_for_execution( + system_exit_on_terminate=True, + info_to_inspect=f""" + ==Master data file== + domain_additional_filename: {args.domain_additional_filename} + + ==Organization name information== + organization_adhoc_filename: {args.organization_adhoc_filename} + + ==Containing directory== + directory: {args.directory} + + ==Proposed Changes== + For each TransitionDomain, modify the following fields: {changed_fields} + """, + prompt_title="Do you wish to load organization data for TransitionDomains?", + ) + + if not proceed: + return None + + logger.info( + f"{TerminalColors.MAGENTA}" + "Loading organization data onto TransitionDomain tables..." + ) load = OrganizationDataLoader(args) - load.update_organization_data_for_all() + domain_information_to_update = load.update_organization_data_for_all() + + # Reprompt the user to reinspect before updating DomainInformation + proceed = TerminalHelper.prompt_for_execution( + system_exit_on_terminate=True, + info_to_inspect=f""" + ==Master data file== + domain_additional_filename: {args.domain_additional_filename} + + ==Organization name information== + organization_adhoc_filename: {args.organization_adhoc_filename} + + ==Containing directory== + directory: {args.directory} + + ==Proposed Changes== + Number of DomainInformation objects to change: {len(domain_information_to_update)} + """, + prompt_title="Do you wish to load organization data for DomainInformation?", + ) + + if not proceed: + return None + + logger.info( + f"{TerminalColors.MAGENTA}" + "Loading organization data onto DomainInformation tables..." + ) + self.update_domain_information(domain_information_to_update, args.debug) + + def update_domain_information(self, desired_objects: List[TransitionDomain], debug): + di_to_update = [] + di_failed_to_update = [] + for item in desired_objects: + # TODO - this can probably be done in fewer steps + transition_domains = TransitionDomain.objects.filter(username=item.username, domain_name=item.domain_name) + current_transition_domain = self.retrieve_and_assert_single_item(transition_domains, "TransitionDomain", "test") + + domains = Domain.objects.filter(name=current_transition_domain.domain_name) + current_domain = self.retrieve_and_assert_single_item(domains, "Domain", "test") + + domain_informations = DomainInformation.objects.filter(domain=current_domain) + current_domain_information = self.retrieve_and_assert_single_item(domain_informations, "DomainInformation", "test") + + try: + # TODO - add verification to each, for instance check address_line length + current_domain_information.address_line1 = current_transition_domain.address_line + current_domain_information.city = current_transition_domain.city + current_domain_information.state_territory = current_transition_domain.state_territory + current_domain_information.zipcode = current_transition_domain.zipcode + + # TODO - Country Code + #current_domain_information.country_code = current_transition_domain.country_code + except Exception as err: + logger.error(err) + di_failed_to_update.append(current_domain_information) + else: + di_to_update.append(current_domain_information) + + if len(di_failed_to_update) > 0: + logger.error( + "Failed to update. An exception was encountered " + f"on the following TransitionDomains: {[item for item in di_failed_to_update]}" + ) + raise Exception("Failed to update DomainInformations") + + if not debug: + logger.info( + f"Ready to update {len(di_to_update)} TransitionDomains." + ) + else: + logger.info( + f"Ready to update {len(di_to_update)} TransitionDomains: {[item for item in di_to_update]}" + ) + + logger.info( + f"{TerminalColors.MAGENTA}" + "Beginning mass DomainInformation update..." + f"{TerminalColors.ENDC}" + ) + + changed_fields = [ + "address_line1", + "city", + "state_territory", + "zipcode", + #"country_code", + ] + + DomainInformation.objects.bulk_update(di_to_update, changed_fields) + + if not debug: + logger.info( + f"{TerminalColors.OKGREEN}" + f"Updated {len(di_to_update)} DomainInformations." + f"{TerminalColors.ENDC}" + ) + else: + logger.info( + f"{TerminalColors.OKGREEN}" + f"Updated {len(di_to_update)} DomainInformations: {[item for item in di_to_update]}" + f"{TerminalColors.ENDC}" + ) + + # TODO - rename function + update so item_name_for_log works + def retrieve_and_assert_single_item(self, item_queryset, class_name_for_log, item_name_for_log): + """Checks if .filter returns one, and only one, item""" + if item_queryset.count() == 0: + # TODO - custom exception class + raise Exception(f"Could not update. {class_name_for_log} for {item_name_for_log} was not found") + + if item_queryset.count() > 1: + raise Exception(f"Could not update. Duplicate {class_name_for_log} for {item_name_for_log} was found") + + desired_item = item_queryset.get() + return desired_item diff --git a/src/registrar/management/commands/utility/extra_transition_domain_helper.py b/src/registrar/management/commands/utility/extra_transition_domain_helper.py index 640fe9875..7643f2d12 100644 --- a/src/registrar/management/commands/utility/extra_transition_domain_helper.py +++ b/src/registrar/management/commands/utility/extra_transition_domain_helper.py @@ -784,17 +784,17 @@ class OrganizationDataLoader: """Updates org data for all TransitionDomains""" all_transition_domains = TransitionDomain.objects.all() if len(all_transition_domains) < 1: - logger.error( + raise Exception( f"{TerminalColors.FAIL}" "No TransitionDomains exist. Cannot update." f"{TerminalColors.ENDC}" ) - return None # Store all actions we want to perform in tds_to_update self.prepare_transition_domains(all_transition_domains) # Then if we don't run into any exceptions, bulk_update it self.bulk_update_transition_domains(self.tds_to_update) + return self.tds_to_update def prepare_transition_domains(self, transition_domains): for item in transition_domains: diff --git a/src/registrar/models/transition_domain.py b/src/registrar/models/transition_domain.py index 0f1e0a7bf..e8001252d 100644 --- a/src/registrar/models/transition_domain.py +++ b/src/registrar/models/transition_domain.py @@ -134,7 +134,6 @@ class TransitionDomain(TimeStampedModel): blank=True, help_text="Country code", ) - # TODO - Country code? def __str__(self): return f"{self.username}, {self.domain_name}" From c86a9d5fffe2d1c313e8d0c09dd8254275ad1684 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 16 Nov 2023 15:16:35 -0700 Subject: [PATCH 03/46] Increase performance --- .../commands/load_organization_data.py | 64 +++++++++++-------- 1 file changed, 39 insertions(+), 25 deletions(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index 550f4def9..9d0c112a5 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -103,35 +103,62 @@ class Command(BaseCommand): if not proceed: return None + if len(domain_information_to_update) == 0: + logger.error( + f"{TerminalColors.MAGENTA}" + "No DomainInformation objects exist" + f"{TerminalColors.ENDC}" + ) + return None + logger.info( f"{TerminalColors.MAGENTA}" - "Loading organization data onto DomainInformation tables..." + "Preparing to load organization data onto DomainInformation tables..." + f"{TerminalColors.ENDC}" ) self.update_domain_information(domain_information_to_update, args.debug) def update_domain_information(self, desired_objects: List[TransitionDomain], debug): di_to_update = [] di_failed_to_update = [] + + # Fetch all TransitionDomains in one query + transition_domains = TransitionDomain.objects.filter( + username__in=[item.username for item in desired_objects], + domain_name__in=[item.domain_name for item in desired_objects] + ) + + # Fetch all Domains in one query + domains = Domain.objects.filter( + name__in=[td.domain_name for td in transition_domains] + ) + + # Fetch all DomainInformations in one query + domain_informations = DomainInformation.objects.filter( + domain__in=domains + ) + + # Create dictionaries for faster lookup + transition_domains_dict = {td.domain_name: td for td in transition_domains} + domains_dict = {d.name: d for d in domains} + domain_informations_dict = {di.domain.name: di for di in domain_informations} + for item in desired_objects: - # TODO - this can probably be done in fewer steps - transition_domains = TransitionDomain.objects.filter(username=item.username, domain_name=item.domain_name) - current_transition_domain = self.retrieve_and_assert_single_item(transition_domains, "TransitionDomain", "test") - - domains = Domain.objects.filter(name=current_transition_domain.domain_name) - current_domain = self.retrieve_and_assert_single_item(domains, "Domain", "test") - - domain_informations = DomainInformation.objects.filter(domain=current_domain) - current_domain_information = self.retrieve_and_assert_single_item(domain_informations, "DomainInformation", "test") - try: + current_transition_domain = transition_domains_dict[item.domain_name] + current_domain = domains_dict[current_transition_domain.domain_name] + current_domain_information = domain_informations_dict[current_domain.name] + # TODO - add verification to each, for instance check address_line length current_domain_information.address_line1 = current_transition_domain.address_line current_domain_information.city = current_transition_domain.city current_domain_information.state_territory = current_transition_domain.state_territory current_domain_information.zipcode = current_transition_domain.zipcode - # TODO - Country Code #current_domain_information.country_code = current_transition_domain.country_code + + if debug: + logger.info(f"Updating {current_domain.name}...") except Exception as err: logger.error(err) di_failed_to_update.append(current_domain_information) @@ -182,16 +209,3 @@ class Command(BaseCommand): f"Updated {len(di_to_update)} DomainInformations: {[item for item in di_to_update]}" f"{TerminalColors.ENDC}" ) - - # TODO - rename function + update so item_name_for_log works - def retrieve_and_assert_single_item(self, item_queryset, class_name_for_log, item_name_for_log): - """Checks if .filter returns one, and only one, item""" - if item_queryset.count() == 0: - # TODO - custom exception class - raise Exception(f"Could not update. {class_name_for_log} for {item_name_for_log} was not found") - - if item_queryset.count() > 1: - raise Exception(f"Could not update. Duplicate {class_name_for_log} for {item_name_for_log} was found") - - desired_item = item_queryset.get() - return desired_item From 13b1ca02387ec09fc27782b94ebb7c55d6f0d9a0 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 17 Nov 2023 10:04:09 -0700 Subject: [PATCH 04/46] Remove country code --- .../commands/load_organization_data.py | 35 ++++++++++--------- .../utility/extra_transition_domain_helper.py | 8 ++--- ...ess_line_transitiondomain_city_and_more.py | 5 --- src/registrar/models/transition_domain.py | 7 ---- 4 files changed, 23 insertions(+), 32 deletions(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index 9d0c112a5..937286a07 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -51,7 +51,6 @@ class Command(BaseCommand): "city", "state_territory", "zipcode", - "country_code", ] proceed = TerminalHelper.prompt_for_execution( system_exit_on_terminate=True, @@ -122,43 +121,48 @@ class Command(BaseCommand): di_to_update = [] di_failed_to_update = [] - # Fetch all TransitionDomains in one query + # Grab each TransitionDomain we want to change. Store it. + # Fetches all TransitionDomains in one query. transition_domains = TransitionDomain.objects.filter( username__in=[item.username for item in desired_objects], domain_name__in=[item.domain_name for item in desired_objects] ) - # Fetch all Domains in one query + if len(desired_objects) != len(transition_domains): + raise Exception("Could not find all desired TransitionDomains") + + # Then, for each domain_name grab the associated domain object. + # Fetches all Domains in one query. domains = Domain.objects.filter( name__in=[td.domain_name for td in transition_domains] ) - # Fetch all DomainInformations in one query + # Then, use each domain object to map domain <--> DomainInformation + # Fetches all DomainInformations in one query. domain_informations = DomainInformation.objects.filter( domain__in=domains ) # Create dictionaries for faster lookup - transition_domains_dict = {td.domain_name: td for td in transition_domains} domains_dict = {d.name: d for d in domains} domain_informations_dict = {di.domain.name: di for di in domain_informations} - for item in desired_objects: + for item in transition_domains: try: - current_transition_domain = transition_domains_dict[item.domain_name] - current_domain = domains_dict[current_transition_domain.domain_name] + # Grab the current Domain. This ensures we are pointing towards the right place. + current_domain = domains_dict[item.domain_name] + + # Based on the current domain, grab the right DomainInformation object. current_domain_information = domain_informations_dict[current_domain.name] - # TODO - add verification to each, for instance check address_line length - current_domain_information.address_line1 = current_transition_domain.address_line - current_domain_information.city = current_transition_domain.city - current_domain_information.state_territory = current_transition_domain.state_territory - current_domain_information.zipcode = current_transition_domain.zipcode - # TODO - Country Code - #current_domain_information.country_code = current_transition_domain.country_code + current_domain_information.address_line1 = item.address_line + current_domain_information.city = item.city + current_domain_information.state_territory = item.state_territory + current_domain_information.zipcode = item.zipcode if debug: logger.info(f"Updating {current_domain.name}...") + except Exception as err: logger.error(err) di_failed_to_update.append(current_domain_information) @@ -192,7 +196,6 @@ class Command(BaseCommand): "city", "state_territory", "zipcode", - #"country_code", ] DomainInformation.objects.bulk_update(di_to_update, changed_fields) diff --git a/src/registrar/management/commands/utility/extra_transition_domain_helper.py b/src/registrar/management/commands/utility/extra_transition_domain_helper.py index 7643f2d12..96cc550b3 100644 --- a/src/registrar/management/commands/utility/extra_transition_domain_helper.py +++ b/src/registrar/management/commands/utility/extra_transition_domain_helper.py @@ -804,6 +804,7 @@ class OrganizationDataLoader: if self.debug: logger.info(item.display_transition_domain()) logger.info( + f"Successfully updated TransitionDomain: \n" f"{TerminalColors.OKCYAN}" f"{item.display_transition_domain()}" f"{TerminalColors.ENDC}" @@ -813,6 +814,7 @@ class OrganizationDataLoader: self.tds_failed_to_update.append(item) if self.debug: logger.error( + f"Failed to update TransitionDomain: \n" f"{TerminalColors.YELLOW}" f"{item.display_transition_domain()}" f"{TerminalColors.ENDC}" @@ -846,7 +848,6 @@ class OrganizationDataLoader: "city", "state_territory", "zipcode", - "country_code", ] TransitionDomain.objects.bulk_update(update_list, changed_fields) @@ -886,15 +887,14 @@ class OrganizationDataLoader: transition_domain.city = org_info.orgcity transition_domain.state_territory = org_info.orgstate transition_domain.zipcode = org_info.orgzip - transition_domain.country_code = org_info.orgcountrycode - # Log what happened to each field + # Log what happened to each field. The first value + # is the field name that was updated, second is the value changed_fields = [ ("address_line", transition_domain.address_line), ("city", transition_domain.city), ("state_territory", transition_domain.state_territory), ("zipcode", transition_domain.zipcode), - ("country_code", transition_domain.country_code), ] self.log_add_or_changed_values(EnumFilenames.AUTHORITY_ADHOC, changed_fields, domain_name) diff --git a/src/registrar/migrations/0047_transitiondomain_address_line_transitiondomain_city_and_more.py b/src/registrar/migrations/0047_transitiondomain_address_line_transitiondomain_city_and_more.py index 51ce8d6a2..a312f62f2 100644 --- a/src/registrar/migrations/0047_transitiondomain_address_line_transitiondomain_city_and_more.py +++ b/src/registrar/migrations/0047_transitiondomain_address_line_transitiondomain_city_and_more.py @@ -19,11 +19,6 @@ class Migration(migrations.Migration): name="city", field=models.TextField(blank=True, help_text="City", null=True), ), - migrations.AddField( - model_name="transitiondomain", - name="country_code", - field=models.CharField(blank=True, help_text="Country code", max_length=2, null=True), - ), migrations.AddField( model_name="transitiondomain", name="state_territory", diff --git a/src/registrar/models/transition_domain.py b/src/registrar/models/transition_domain.py index e8001252d..c5b9b125c 100644 --- a/src/registrar/models/transition_domain.py +++ b/src/registrar/models/transition_domain.py @@ -128,12 +128,6 @@ class TransitionDomain(TimeStampedModel): help_text="Zip code", db_index=True, ) - country_code = models.CharField( - max_length=2, - null=True, - blank=True, - help_text="Country code", - ) def __str__(self): return f"{self.username}, {self.domain_name}" @@ -161,5 +155,4 @@ class TransitionDomain(TimeStampedModel): f"city: {self.city}, \n" f"state_territory: {self.state_territory}, \n" f"zipcode: {self.zipcode}, \n" - f"country_code: {self.country_code}, \n" ) From a6db4b7145de14616a2dfcd8e6eb44c9c2bcf821 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 17 Nov 2023 11:26:46 -0700 Subject: [PATCH 05/46] Data chunking / don't update existing data --- .../commands/load_organization_data.py | 75 +++++++++++++++---- .../utility/extra_transition_domain_helper.py | 10 ++- 2 files changed, 68 insertions(+), 17 deletions(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index 937286a07..2c07bbb0a 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -2,8 +2,6 @@ import argparse import logging -import copy -import time from django.core.management import BaseCommand from registrar.management.commands.utility.extra_transition_domain_helper import OrganizationDataLoader @@ -12,7 +10,7 @@ from registrar.management.commands.utility.transition_domain_arguments import Tr from registrar.models import TransitionDomain from registrar.models.domain import Domain from registrar.models.domain_information import DomainInformation -from ...utility.email import send_templated_email, EmailSendingError +from django.core.paginator import Paginator from typing import List logger = logging.getLogger(__name__) @@ -120,6 +118,9 @@ class Command(BaseCommand): def update_domain_information(self, desired_objects: List[TransitionDomain], debug): di_to_update = [] di_failed_to_update = [] + # These are fields that we COULD update, but fields we choose not to update. + # For instance, if the user already entered data - lets not corrupt that. + di_skipped = [] # Grab each TransitionDomain we want to change. Store it. # Fetches all TransitionDomains in one query. @@ -137,9 +138,27 @@ class Command(BaseCommand): name__in=[td.domain_name for td in transition_domains] ) + + # Start with all DomainInformation objects + filtered_domain_informations = DomainInformation.objects.all() + + changed_fields = [ + "address_line1", + "city", + "state_territory", + "zipcode", + ] + + # Chain filter calls for each field. This checks to see if the end user + # made a change to ANY field in changed_fields. If they did, don't update their information. + # We assume that if they made a change, we don't want to interfere with that. + for field in changed_fields: + # For each changed_field, check if no data exists + filtered_domain_informations = filtered_domain_informations.filter(**{f'{field}__isnull': True}) + # Then, use each domain object to map domain <--> DomainInformation # Fetches all DomainInformations in one query. - domain_informations = DomainInformation.objects.filter( + domain_informations = filtered_domain_informations.filter( domain__in=domains ) @@ -149,32 +168,52 @@ class Command(BaseCommand): for item in transition_domains: try: + should_update = True # Grab the current Domain. This ensures we are pointing towards the right place. current_domain = domains_dict[item.domain_name] # Based on the current domain, grab the right DomainInformation object. - current_domain_information = domain_informations_dict[current_domain.name] + if current_domain.name in domain_informations_dict: + current_domain_information = domain_informations_dict[current_domain.name] + current_domain_information.address_line1 = item.address_line + current_domain_information.city = item.city + current_domain_information.state_territory = item.state_territory + current_domain_information.zipcode = item.zipcode + + if debug: + logger.info(f"Updating {current_domain.name}...") - current_domain_information.address_line1 = item.address_line - current_domain_information.city = item.city - current_domain_information.state_territory = item.state_territory - current_domain_information.zipcode = item.zipcode - - if debug: - logger.info(f"Updating {current_domain.name}...") + else: + logger.info( + f"{TerminalColors.YELLOW}" + f"Domain {current_domain.name} was updated by a user. Cannot update." + f"{TerminalColors.ENDC}" + ) + should_update = False except Exception as err: logger.error(err) - di_failed_to_update.append(current_domain_information) + di_failed_to_update.append(item) else: - di_to_update.append(current_domain_information) + if should_update: + di_to_update.append(current_domain_information) + else: + # TODO either update to name for all, + # or have this filter to the right field + di_skipped.append(item) if len(di_failed_to_update) > 0: logger.error( + f"{TerminalColors.FAIL}" "Failed to update. An exception was encountered " f"on the following TransitionDomains: {[item for item in di_failed_to_update]}" + f"{TerminalColors.ENDC}" ) raise Exception("Failed to update DomainInformations") + + skipped_count = len(di_skipped) + if skipped_count > 0: + logger.info(f"Skipped updating {skipped_count} fields. User-supplied data exists") if not debug: logger.info( @@ -198,7 +237,13 @@ class Command(BaseCommand): "zipcode", ] - DomainInformation.objects.bulk_update(di_to_update, changed_fields) + batch_size = 1000 + # 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(di_to_update, batch_size) + for page_num in paginator.page_range: + page = paginator.page(page_num) + DomainInformation.objects.bulk_update(page.object_list, changed_fields) if not debug: logger.info( diff --git a/src/registrar/management/commands/utility/extra_transition_domain_helper.py b/src/registrar/management/commands/utility/extra_transition_domain_helper.py index 96cc550b3..be84e7681 100644 --- a/src/registrar/management/commands/utility/extra_transition_domain_helper.py +++ b/src/registrar/management/commands/utility/extra_transition_domain_helper.py @@ -10,7 +10,7 @@ import logging import os import sys from typing import Dict - +from django.core.paginator import Paginator from registrar.models.transition_domain import TransitionDomain from .epp_data_containers import ( @@ -850,7 +850,13 @@ class OrganizationDataLoader: "zipcode", ] - TransitionDomain.objects.bulk_update(update_list, changed_fields) + batch_size = 1000 + # 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) + TransitionDomain.objects.bulk_update(page.object_list, changed_fields) if not self.debug: logger.info( From 9e7f111206ad1e5510fc010517965c2354abeceb Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 17 Nov 2023 11:43:40 -0700 Subject: [PATCH 06/46] Update load_organization_data.py --- src/registrar/management/commands/load_organization_data.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index 2c07bbb0a..a4666d461 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -127,7 +127,7 @@ class Command(BaseCommand): transition_domains = TransitionDomain.objects.filter( username__in=[item.username for item in desired_objects], domain_name__in=[item.domain_name for item in desired_objects] - ) + ).distinct() if len(desired_objects) != len(transition_domains): raise Exception("Could not find all desired TransitionDomains") From 85ee97d615057ec31c646f785845a6810e142c50 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 17 Nov 2023 13:02:29 -0700 Subject: [PATCH 07/46] Add test cases One is still breaking, not mine but a related test. Test interference issue? --- .../commands/load_organization_data.py | 40 +++++- .../test_transition_domain_migrations.py | 125 ++++++++++++++++++ 2 files changed, 162 insertions(+), 3 deletions(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index a4666d461..5d9d70716 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -1,6 +1,7 @@ """Data migration: Send domain invitations once to existing customers.""" import argparse +import json import logging from django.core.management import BaseCommand @@ -22,6 +23,11 @@ class Command(BaseCommand): def add_arguments(self, parser): """Add command line arguments.""" + parser.add_argument( + "migration_json_filename", + help=("A JSON file that holds the location and filenames" "of all the data files used for migrations"), + ) + parser.add_argument("--sep", default="|", help="Delimiter character") parser.add_argument("--debug", action=argparse.BooleanOptionalAction) @@ -31,17 +37,45 @@ class Command(BaseCommand): parser.add_argument( "--domain_additional_filename", help="Defines the filename for additional domain data", - required=True, ) parser.add_argument( "--organization_adhoc_filename", help="Defines the filename for domain type adhocs", - required=True, ) - def handle(self, **options): + def handle(self, migration_json_filename, **options): """Process the objects in TransitionDomain.""" + + # === Parse JSON file === # + # Desired directory for additional TransitionDomain data + # (In the event they are stored seperately) + directory = options["directory"] + # Add a slash if the last character isn't one + if directory and directory[-1] != "/": + directory += "/" + + json_filepath = directory + migration_json_filename + + # If a JSON was provided, use its values instead of defaults. + with open(json_filepath, "r") as jsonFile: + # load JSON object as a dictionary + try: + data = json.load(jsonFile) + # Create an instance of TransitionDomainArguments + # Iterate over the data from the JSON file + for key, value in data.items(): + if value is not None and value.strip() != "": + options[key] = value + except Exception as err: + logger.error( + f"{TerminalColors.FAIL}" + "There was an error loading " + "the JSON responsible for providing filepaths." + f"{TerminalColors.ENDC}" + ) + raise err + # === End parse JSON file === # args = TransitionDomainArguments(**options) changed_fields = [ diff --git a/src/registrar/tests/test_transition_domain_migrations.py b/src/registrar/tests/test_transition_domain_migrations.py index c6418f013..0c959673d 100644 --- a/src/registrar/tests/test_transition_domain_migrations.py +++ b/src/registrar/tests/test_transition_domain_migrations.py @@ -81,6 +81,19 @@ class TestMigrations(TestCase): migrationJSON=self.migration_json_filename, disablePrompts=True, ) + + def run_load_organization_data(self): + # noqa here (E501) because splitting this up makes it + # confusing to read. + with patch( + "registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit", # noqa + return_value=True, + ): + call_command( + "load_organization_data", + self.migration_json_filename, + directory=self.test_data_file_location, + ) def compare_tables( self, @@ -157,6 +170,118 @@ class TestMigrations(TestCase): self.assertEqual(total_domain_informations, expected_total_domain_informations) self.assertEqual(total_domain_invitations, expected_total_domain_invitations) + def test_load_organization_data_transition_domain(self): + self.maxDiff = None + # == First, parse all existing data == # + self.run_master_script() + + # == Second, try adding org data to it == # + self.run_load_organization_data() + + # == Third, test that we've loaded data as we expect == # + transition_domains = TransitionDomain.objects.filter(domain_name="fakewebsite2.gov") + + # Should return three objects (three unique emails) + self.assertEqual(transition_domains.count(), 3) + + # Lets test the first one + transition = transition_domains.first() + expected_transition_domain = TransitionDomain( + id=6, + username='alexandra.bobbitt5@test.com', + domain_name='fakewebsite2.gov', + status='on hold', + email_sent=True, + organization_type='Federal', + organization_name='Fanoodle', + federal_type='Executive', + federal_agency='Department of Commerce', + epp_creation_date=datetime.date(2004, 5, 7), + epp_expiration_date=datetime.date(2023, 9, 30), + first_name='Seline', + middle_name='testmiddle2', + last_name='Tower', + title=None, + email='stower3@answers.com', + phone='151-539-6028', + address_line='93001 Arizona Drive', + city='Columbus', + state_territory='Oh', + zipcode='43268' + ) + + self.assertEqual(transition, expected_transition_domain) + + def test_load_organization_data_domain_information(self): + self.maxDiff = None + # == First, parse all existing data == # + self.run_master_script() + + # == Second, try adding org data to it == # + self.run_load_organization_data() + + # == Third, test that we've loaded data as we expect == # + _domain = Domain.objects.filter(name="fakewebsite2.gov").get() + domain_information = DomainInformation.objects.filter(domain=_domain).get() + expected_domain_information = DomainInformation( + id=4, + creator_id=1, + domain_application_id=None, + organization_type='federal', + federally_recognized_tribe=None, + state_recognized_tribe=None, + tribe_name=None, + federal_agency='Department of Commerce', + federal_type='executive', + is_election_board=None, + organization_name='Fanoodle', + address_line1='93001 Arizona Drive', + address_line2=None, + city='Columbus', + state_territory='Oh', + zipcode='43268', + urbanization=None, + about_your_organization=None, + authorizing_official_id=5, + domain_id=4, + submitter_id=None, + purpose=None, + no_other_contacts_rationale=None, + anything_else=None, + is_policy_acknowledged=None + ) + self.assertEqual(domain_information, expected_domain_information) + + def test_load_organization_data_integrity(self): + """Validates data integrity with the load_org_data command""" + # First, parse all existing data + self.run_master_script() + + # Second, try adding org data to it + self.run_load_organization_data() + + # Third, test that we didn't corrupt any data + expected_total_transition_domains = 9 + expected_total_domains = 5 + expected_total_domain_informations = 5 + expected_total_domain_invitations = 8 + + expected_missing_domains = 0 + expected_duplicate_domains = 0 + expected_missing_domain_informations = 0 + # we expect 1 missing invite from anomaly.gov (an injected error) + expected_missing_domain_invitations = 1 + self.compare_tables( + expected_total_transition_domains, + expected_total_domains, + expected_total_domain_informations, + expected_total_domain_invitations, + expected_missing_domains, + expected_duplicate_domains, + expected_missing_domain_informations, + expected_missing_domain_invitations, + ) + def test_master_migration_functions(self): """Run the full master migration script using local test data. NOTE: This is more of an integration test and so far does not From f1acd46588ee81227de69b8021b42de512522c88 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 17 Nov 2023 14:04:06 -0700 Subject: [PATCH 08/46] Fix test cases --- .../commands/load_organization_data.py | 4 + .../commands/send_domain_invitations.py | 12 +- .../test_transition_domain_migrations.py | 363 ++++++++++++------ 3 files changed, 263 insertions(+), 116 deletions(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index 5d9d70716..95743c6b8 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -204,6 +204,10 @@ class Command(BaseCommand): try: should_update = True # Grab the current Domain. This ensures we are pointing towards the right place. + if item.domain_name not in domains_dict: + logger.error(f"Could not add {item.domain_name}. Domain does not exist.") + di_failed_to_update.append(item) + continue current_domain = domains_dict[item.domain_name] # Based on the current domain, grab the right DomainInformation object. diff --git a/src/registrar/management/commands/send_domain_invitations.py b/src/registrar/management/commands/send_domain_invitations.py index 603fbce3a..0f8ca1c46 100644 --- a/src/registrar/management/commands/send_domain_invitations.py +++ b/src/registrar/management/commands/send_domain_invitations.py @@ -152,6 +152,12 @@ class Command(BaseCommand): for domain_name in email_data["domains"]: # self.transition_domains is a queryset so we can sub-select # from it and use the objects to mark them as sent - this_transition_domain = self.transition_domains.get(username=this_email, domain_name=domain_name) - this_transition_domain.email_sent = True - this_transition_domain.save() + transition_domains = self.transition_domains.filter(username=this_email, domain_name=domain_name) + if len(transition_domains) == 1: + this_transition_domain = transition_domains.get() + this_transition_domain.email_sent = True + this_transition_domain.save() + elif len(transition_domains) > 1: + logger.error(f"Multiple TransitionDomains exist for {this_email}") + else: + logger.error(f"No TransitionDomain exists for {this_email}") diff --git a/src/registrar/tests/test_transition_domain_migrations.py b/src/registrar/tests/test_transition_domain_migrations.py index 0c959673d..d5183837a 100644 --- a/src/registrar/tests/test_transition_domain_migrations.py +++ b/src/registrar/tests/test_transition_domain_migrations.py @@ -18,6 +18,254 @@ from unittest.mock import patch from .common import less_console_noise +class TestOrganizationMigration(TestCase): + def setUp(self): + """ """ + # self.load_transition_domain_script = "load_transition_domain", + # self.transfer_script = "transfer_transition_domains_to_domains", + # self.master_script = "load_transition_domain", + + self.test_data_file_location = "registrar/tests/data" + self.test_domain_contact_filename = "test_domain_contacts.txt" + self.test_contact_filename = "test_contacts.txt" + self.test_domain_status_filename = "test_domain_statuses.txt" + + # Files for parsing additional TransitionDomain data + self.test_agency_adhoc_filename = "test_agency_adhoc.txt" + self.test_authority_adhoc_filename = "test_authority_adhoc.txt" + self.test_domain_additional = "test_domain_additional.txt" + self.test_domain_types_adhoc = "test_domain_types_adhoc.txt" + self.test_escrow_domains_daily = "test_escrow_domains_daily" + self.test_organization_adhoc = "test_organization_adhoc.txt" + self.migration_json_filename = "test_migrationFilepaths.json" + + def tearDown(self): + # Delete domain information + Domain.objects.all().delete() + DomainInformation.objects.all().delete() + DomainInvitation.objects.all().delete() + TransitionDomain.objects.all().delete() + + # Delete users + User.objects.all().delete() + UserDomainRole.objects.all().delete() + + def run_load_domains(self): + # noqa here because splitting this up makes it confusing. + # ES501 + with patch( + "registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit", # noqa + return_value=True, + ): + call_command( + "load_transition_domain", + self.migration_json_filename, + directory=self.test_data_file_location, + ) + + def run_transfer_domains(self): + call_command("transfer_transition_domains_to_domains") + + def run_load_organization_data(self): + # noqa here (E501) because splitting this up makes it + # confusing to read. + with patch( + "registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit", # noqa + return_value=True, + ): + call_command( + "load_organization_data", + self.migration_json_filename, + directory=self.test_data_file_location, + ) + + def compare_tables( + self, + expected_total_transition_domains, + expected_total_domains, + expected_total_domain_informations, + expected_total_domain_invitations, + expected_missing_domains, + expected_duplicate_domains, + expected_missing_domain_informations, + expected_missing_domain_invitations, + ): + """Does a diff between the transition_domain and the following tables: + domain, domain_information and the domain_invitation. + Verifies that the data loaded correctly.""" + + missing_domains = [] + duplicate_domains = [] + missing_domain_informations = [] + missing_domain_invites = [] + for transition_domain in TransitionDomain.objects.all(): # DEBUG: + transition_domain_name = transition_domain.domain_name + transition_domain_email = transition_domain.username + + # Check Domain table + matching_domains = Domain.objects.filter(name=transition_domain_name) + # Check Domain Information table + matching_domain_informations = DomainInformation.objects.filter(domain__name=transition_domain_name) + # Check Domain Invitation table + matching_domain_invitations = DomainInvitation.objects.filter( + email=transition_domain_email.lower(), + domain__name=transition_domain_name, + ) + + if len(matching_domains) == 0: + missing_domains.append(transition_domain_name) + elif len(matching_domains) > 1: + duplicate_domains.append(transition_domain_name) + if len(matching_domain_informations) == 0: + missing_domain_informations.append(transition_domain_name) + if len(matching_domain_invitations) == 0: + missing_domain_invites.append(transition_domain_name) + + total_missing_domains = len(missing_domains) + total_duplicate_domains = len(duplicate_domains) + total_missing_domain_informations = len(missing_domain_informations) + total_missing_domain_invitations = len(missing_domain_invites) + + total_transition_domains = len(TransitionDomain.objects.all()) + total_domains = len(Domain.objects.all()) + total_domain_informations = len(DomainInformation.objects.all()) + total_domain_invitations = len(DomainInvitation.objects.all()) + + print( + f""" + total_missing_domains = {len(missing_domains)} + total_duplicate_domains = {len(duplicate_domains)} + total_missing_domain_informations = {len(missing_domain_informations)} + total_missing_domain_invitations = {total_missing_domain_invitations} + + total_transition_domains = {len(TransitionDomain.objects.all())} + total_domains = {len(Domain.objects.all())} + total_domain_informations = {len(DomainInformation.objects.all())} + total_domain_invitations = {len(DomainInvitation.objects.all())} + """ + ) + self.assertEqual(total_missing_domains, expected_missing_domains) + self.assertEqual(total_duplicate_domains, expected_duplicate_domains) + self.assertEqual(total_missing_domain_informations, expected_missing_domain_informations) + self.assertEqual(total_missing_domain_invitations, expected_missing_domain_invitations) + + self.assertEqual(total_transition_domains, expected_total_transition_domains) + self.assertEqual(total_domains, expected_total_domains) + self.assertEqual(total_domain_informations, expected_total_domain_informations) + self.assertEqual(total_domain_invitations, expected_total_domain_invitations) + + def test_load_organization_data_transition_domain(self): + # == First, parse all existing data == # + self.run_load_domains() + self.run_transfer_domains() + + # == Second, try adding org data to it == # + self.run_load_organization_data() + + # == Third, test that we've loaded data as we expect == # + transition_domains = TransitionDomain.objects.filter(domain_name="fakewebsite2.gov") + + # Should return three objects (three unique emails) + self.assertEqual(transition_domains.count(), 3) + + # Lets test the first one + transition = transition_domains.first() + expected_transition_domain = TransitionDomain( + id=24, + username='alexandra.bobbitt5@test.com', + domain_name='fakewebsite2.gov', + status='on hold', + email_sent=True, + organization_type='Federal', + organization_name='Fanoodle', + federal_type='Executive', + federal_agency='Department of Commerce', + epp_creation_date=datetime.date(2004, 5, 7), + epp_expiration_date=datetime.date(2023, 9, 30), + first_name='Seline', + middle_name='testmiddle2', + last_name='Tower', + title=None, + email='stower3@answers.com', + phone='151-539-6028', + address_line='93001 Arizona Drive', + city='Columbus', + state_territory='Oh', + zipcode='43268' + ) + + self.assertEqual(transition, expected_transition_domain) + + def test_load_organization_data_domain_information(self): + # == First, parse all existing data == # + self.run_load_domains() + self.run_transfer_domains() + + # == Second, try adding org data to it == # + self.run_load_organization_data() + + # == Third, test that we've loaded data as we expect == # + _domain = Domain.objects.filter(name="fakewebsite2.gov").get() + domain_information = DomainInformation.objects.filter(domain=_domain).get() + expected_domain_information = DomainInformation( + id=4, + creator_id=1, + domain_application_id=None, + organization_type='federal', + federally_recognized_tribe=None, + state_recognized_tribe=None, + tribe_name=None, + federal_agency='Department of Commerce', + federal_type='executive', + is_election_board=None, + organization_name='Fanoodle', + address_line1='93001 Arizona Drive', + address_line2=None, + city='Columbus', + state_territory='Oh', + zipcode='43268', + urbanization=None, + about_your_organization=None, + authorizing_official_id=5, + domain_id=4, + submitter_id=None, + purpose=None, + no_other_contacts_rationale=None, + anything_else=None, + is_policy_acknowledged=None + ) + self.assertEqual(domain_information, expected_domain_information) + + def test_load_organization_data_integrity(self): + """Validates data integrity with the load_org_data command""" + # First, parse all existing data + self.run_load_domains() + self.run_transfer_domains() + + # Second, try adding org data to it + self.run_load_organization_data() + + # Third, test that we didn't corrupt any data + expected_total_transition_domains = 9 + expected_total_domains = 5 + expected_total_domain_informations = 5 + expected_total_domain_invitations = 8 + + expected_missing_domains = 0 + expected_duplicate_domains = 0 + expected_missing_domain_informations = 0 + # we expect 1 missing invite from anomaly.gov (an injected error) + expected_missing_domain_invitations = 1 + self.compare_tables( + expected_total_transition_domains, + expected_total_domains, + expected_total_domain_informations, + expected_total_domain_invitations, + expected_missing_domains, + expected_duplicate_domains, + expected_missing_domain_informations, + expected_missing_domain_invitations, + ) class TestMigrations(TestCase): def setUp(self): @@ -41,11 +289,12 @@ class TestMigrations(TestCase): self.migration_json_filename = "test_migrationFilepaths.json" def tearDown(self): + super().tearDown() # Delete domain information TransitionDomain.objects.all().delete() Domain.objects.all().delete() - DomainInvitation.objects.all().delete() DomainInformation.objects.all().delete() + DomainInvitation.objects.all().delete() # Delete users User.objects.all().delete() @@ -170,118 +419,6 @@ class TestMigrations(TestCase): self.assertEqual(total_domain_informations, expected_total_domain_informations) self.assertEqual(total_domain_invitations, expected_total_domain_invitations) - def test_load_organization_data_transition_domain(self): - self.maxDiff = None - # == First, parse all existing data == # - self.run_master_script() - - # == Second, try adding org data to it == # - self.run_load_organization_data() - - # == Third, test that we've loaded data as we expect == # - transition_domains = TransitionDomain.objects.filter(domain_name="fakewebsite2.gov") - - # Should return three objects (three unique emails) - self.assertEqual(transition_domains.count(), 3) - - # Lets test the first one - transition = transition_domains.first() - expected_transition_domain = TransitionDomain( - id=6, - username='alexandra.bobbitt5@test.com', - domain_name='fakewebsite2.gov', - status='on hold', - email_sent=True, - organization_type='Federal', - organization_name='Fanoodle', - federal_type='Executive', - federal_agency='Department of Commerce', - epp_creation_date=datetime.date(2004, 5, 7), - epp_expiration_date=datetime.date(2023, 9, 30), - first_name='Seline', - middle_name='testmiddle2', - last_name='Tower', - title=None, - email='stower3@answers.com', - phone='151-539-6028', - address_line='93001 Arizona Drive', - city='Columbus', - state_territory='Oh', - zipcode='43268' - ) - - self.assertEqual(transition, expected_transition_domain) - - def test_load_organization_data_domain_information(self): - self.maxDiff = None - # == First, parse all existing data == # - self.run_master_script() - - # == Second, try adding org data to it == # - self.run_load_organization_data() - - # == Third, test that we've loaded data as we expect == # - _domain = Domain.objects.filter(name="fakewebsite2.gov").get() - domain_information = DomainInformation.objects.filter(domain=_domain).get() - expected_domain_information = DomainInformation( - id=4, - creator_id=1, - domain_application_id=None, - organization_type='federal', - federally_recognized_tribe=None, - state_recognized_tribe=None, - tribe_name=None, - federal_agency='Department of Commerce', - federal_type='executive', - is_election_board=None, - organization_name='Fanoodle', - address_line1='93001 Arizona Drive', - address_line2=None, - city='Columbus', - state_territory='Oh', - zipcode='43268', - urbanization=None, - about_your_organization=None, - authorizing_official_id=5, - domain_id=4, - submitter_id=None, - purpose=None, - no_other_contacts_rationale=None, - anything_else=None, - is_policy_acknowledged=None - ) - self.assertEqual(domain_information, expected_domain_information) - - def test_load_organization_data_integrity(self): - """Validates data integrity with the load_org_data command""" - # First, parse all existing data - self.run_master_script() - - # Second, try adding org data to it - self.run_load_organization_data() - - # Third, test that we didn't corrupt any data - expected_total_transition_domains = 9 - expected_total_domains = 5 - expected_total_domain_informations = 5 - expected_total_domain_invitations = 8 - - expected_missing_domains = 0 - expected_duplicate_domains = 0 - expected_missing_domain_informations = 0 - # we expect 1 missing invite from anomaly.gov (an injected error) - expected_missing_domain_invitations = 1 - self.compare_tables( - expected_total_transition_domains, - expected_total_domains, - expected_total_domain_informations, - expected_total_domain_invitations, - expected_missing_domains, - expected_duplicate_domains, - expected_missing_domain_informations, - expected_missing_domain_invitations, - ) - def test_master_migration_functions(self): """Run the full master migration script using local test data. NOTE: This is more of an integration test and so far does not From 8ec24bfb0d4049316221a6129fae38422cbbcf1b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 17 Nov 2023 14:09:47 -0700 Subject: [PATCH 09/46] Test --- src/registrar/tests/test_transition_domain_migrations.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/registrar/tests/test_transition_domain_migrations.py b/src/registrar/tests/test_transition_domain_migrations.py index d5183837a..9ae9a66be 100644 --- a/src/registrar/tests/test_transition_domain_migrations.py +++ b/src/registrar/tests/test_transition_domain_migrations.py @@ -171,7 +171,6 @@ class TestOrganizationMigration(TestCase): # Lets test the first one transition = transition_domains.first() expected_transition_domain = TransitionDomain( - id=24, username='alexandra.bobbitt5@test.com', domain_name='fakewebsite2.gov', status='on hold', @@ -193,10 +192,12 @@ class TestOrganizationMigration(TestCase): state_territory='Oh', zipcode='43268' ) + expected_transition_domain.id = transition.id self.assertEqual(transition, expected_transition_domain) def test_load_organization_data_domain_information(self): + self.maxDiff = None # == First, parse all existing data == # self.run_load_domains() self.run_transfer_domains() @@ -234,7 +235,7 @@ class TestOrganizationMigration(TestCase): anything_else=None, is_policy_acknowledged=None ) - self.assertEqual(domain_information, expected_domain_information) + self.assertEqual(domain_information.__dict__, expected_domain_information.__dict__) def test_load_organization_data_integrity(self): """Validates data integrity with the load_org_data command""" From 2f9896686bd304998c5358ff2c5f400a56537fa1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 17 Nov 2023 14:20:31 -0700 Subject: [PATCH 10/46] Update test_transition_domain_migrations.py --- .../test_transition_domain_migrations.py | 33 +++---------------- 1 file changed, 5 insertions(+), 28 deletions(-) diff --git a/src/registrar/tests/test_transition_domain_migrations.py b/src/registrar/tests/test_transition_domain_migrations.py index 9ae9a66be..3e75c6b4e 100644 --- a/src/registrar/tests/test_transition_domain_migrations.py +++ b/src/registrar/tests/test_transition_domain_migrations.py @@ -208,34 +208,11 @@ class TestOrganizationMigration(TestCase): # == Third, test that we've loaded data as we expect == # _domain = Domain.objects.filter(name="fakewebsite2.gov").get() domain_information = DomainInformation.objects.filter(domain=_domain).get() - expected_domain_information = DomainInformation( - id=4, - creator_id=1, - domain_application_id=None, - organization_type='federal', - federally_recognized_tribe=None, - state_recognized_tribe=None, - tribe_name=None, - federal_agency='Department of Commerce', - federal_type='executive', - is_election_board=None, - organization_name='Fanoodle', - address_line1='93001 Arizona Drive', - address_line2=None, - city='Columbus', - state_territory='Oh', - zipcode='43268', - urbanization=None, - about_your_organization=None, - authorizing_official_id=5, - domain_id=4, - submitter_id=None, - purpose=None, - no_other_contacts_rationale=None, - anything_else=None, - is_policy_acknowledged=None - ) - self.assertEqual(domain_information.__dict__, expected_domain_information.__dict__) + + self.assertEqual(domain_information.address_line1, '93001 Arizona Drive') + self.assertEqual(domain_information.city, 'Columbus') + self.assertEqual(domain_information.state_territory, 'Oh') + self.assertEqual(domain_information.zipcode, '43268') def test_load_organization_data_integrity(self): """Validates data integrity with the load_org_data command""" From b875a4583d0e15edad0b8330b32ed6ff64bde79c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 17 Nov 2023 15:14:02 -0700 Subject: [PATCH 11/46] Simplify Load_Organization_data --- .../commands/load_organization_data.py | 175 +++++++----------- .../utility/extra_transition_domain_helper.py | 56 +++--- .../utility/transition_domain_arguments.py | 2 +- .../test_transition_domain_migrations.py | 59 +++--- 4 files changed, 120 insertions(+), 172 deletions(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index 95743c6b8..ae9f7a29b 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -105,10 +105,7 @@ class Command(BaseCommand): if not proceed: return None - logger.info( - f"{TerminalColors.MAGENTA}" - "Loading organization data onto TransitionDomain tables..." - ) + logger.info(f"{TerminalColors.MAGENTA}" "Loading organization data onto TransitionDomain tables...") load = OrganizationDataLoader(args) domain_information_to_update = load.update_organization_data_for_all() @@ -135,11 +132,7 @@ class Command(BaseCommand): return None if len(domain_information_to_update) == 0: - logger.error( - f"{TerminalColors.MAGENTA}" - "No DomainInformation objects exist" - f"{TerminalColors.ENDC}" - ) + logger.error(f"{TerminalColors.MAGENTA}" "No DomainInformation objects exist" f"{TerminalColors.ENDC}") return None logger.info( @@ -148,125 +141,93 @@ class Command(BaseCommand): f"{TerminalColors.ENDC}" ) self.update_domain_information(domain_information_to_update, args.debug) - + def update_domain_information(self, desired_objects: List[TransitionDomain], debug): di_to_update = [] di_failed_to_update = [] - # These are fields that we COULD update, but fields we choose not to update. - # For instance, if the user already entered data - lets not corrupt that. di_skipped = [] - # Grab each TransitionDomain we want to change. Store it. - # Fetches all TransitionDomains in one query. + # Grab each TransitionDomain we want to change. transition_domains = TransitionDomain.objects.filter( username__in=[item.username for item in desired_objects], - domain_name__in=[item.domain_name for item in desired_objects] + domain_name__in=[item.domain_name for item in desired_objects], ).distinct() if len(desired_objects) != len(transition_domains): raise Exception("Could not find all desired TransitionDomains") # Then, for each domain_name grab the associated domain object. - # Fetches all Domains in one query. - domains = Domain.objects.filter( - name__in=[td.domain_name for td in transition_domains] - ) - + domains = Domain.objects.filter(name__in=[td.domain_name for td in transition_domains]) + # Create dictionary for faster lookup + domains_dict = {d.name: d for d in domains} # Start with all DomainInformation objects filtered_domain_informations = DomainInformation.objects.all() - - changed_fields = [ - "address_line1", - "city", - "state_territory", - "zipcode", - ] - - # Chain filter calls for each field. This checks to see if the end user - # made a change to ANY field in changed_fields. If they did, don't update their information. - # We assume that if they made a change, we don't want to interfere with that. - for field in changed_fields: - # For each changed_field, check if no data exists - filtered_domain_informations = filtered_domain_informations.filter(**{f'{field}__isnull': True}) # Then, use each domain object to map domain <--> DomainInformation # Fetches all DomainInformations in one query. + # If any related organization fields have been updated, + # we can assume that they modified this information themselves - thus we should not update it. domain_informations = filtered_domain_informations.filter( - domain__in=domains + domain__in=domains, + address_line1__isnull=True, + city__isnull=True, + state_territory__isnull=True, + zipcode__isnull=True, ) - # Create dictionaries for faster lookup - domains_dict = {d.name: d for d in domains} domain_informations_dict = {di.domain.name: di for di in domain_informations} for item in transition_domains: - try: - should_update = True - # Grab the current Domain. This ensures we are pointing towards the right place. - if item.domain_name not in domains_dict: - logger.error(f"Could not add {item.domain_name}. Domain does not exist.") - di_failed_to_update.append(item) - continue - current_domain = domains_dict[item.domain_name] - - # Based on the current domain, grab the right DomainInformation object. - if current_domain.name in domain_informations_dict: - current_domain_information = domain_informations_dict[current_domain.name] - current_domain_information.address_line1 = item.address_line - current_domain_information.city = item.city - current_domain_information.state_territory = item.state_territory - current_domain_information.zipcode = item.zipcode - - if debug: - logger.info(f"Updating {current_domain.name}...") - - else: - logger.info( - f"{TerminalColors.YELLOW}" - f"Domain {current_domain.name} was updated by a user. Cannot update." - f"{TerminalColors.ENDC}" - ) - should_update = False - - except Exception as err: - logger.error(err) + if item.domain_name not in domains_dict: + logger.error(f"Could not add {item.domain_name}. Domain does not exist.") di_failed_to_update.append(item) - else: - if should_update: - di_to_update.append(current_domain_information) - else: - # TODO either update to name for all, - # or have this filter to the right field - di_skipped.append(item) - - if len(di_failed_to_update) > 0: + continue + + current_domain = domains_dict[item.domain_name] + if current_domain.name not in domain_informations_dict: + logger.info( + f"{TerminalColors.YELLOW}" + f"Domain {current_domain.name} was updated by a user. Cannot update." + f"{TerminalColors.ENDC}" + ) + di_skipped.append(item) + continue + + # Based on the current domain, grab the right DomainInformation object. + current_domain_information = domain_informations_dict[current_domain.name] + + # Update fields + current_domain_information.address_line1 = item.address_line + current_domain_information.city = item.city + current_domain_information.state_territory = item.state_territory + current_domain_information.zipcode = item.zipcode + + di_to_update.append(current_domain_information) + if debug: + logger.info(f"Updated {current_domain.name}...") + + if di_failed_to_update: + failed = [item.domain_name for item in di_failed_to_update] logger.error( - f"{TerminalColors.FAIL}" - "Failed to update. An exception was encountered " - f"on the following TransitionDomains: {[item for item in di_failed_to_update]}" - f"{TerminalColors.ENDC}" + f"""{TerminalColors.FAIL} + Failed to update. An exception was encountered on the following TransitionDomains: {failed} + {TerminalColors.ENDC}""" ) raise Exception("Failed to update DomainInformations") - - skipped_count = len(di_skipped) - if skipped_count > 0: - logger.info(f"Skipped updating {skipped_count} fields. User-supplied data exists") - if not debug: - logger.info( - f"Ready to update {len(di_to_update)} TransitionDomains." - ) - else: - logger.info( - f"Ready to update {len(di_to_update)} TransitionDomains: {[item for item in di_to_update]}" - ) - - logger.info( - f"{TerminalColors.MAGENTA}" - "Beginning mass DomainInformation update..." - f"{TerminalColors.ENDC}" - ) + if di_skipped: + logger.info(f"Skipped updating {len(di_skipped)} fields. User-supplied data exists") + + self.bulk_update_domain_information(di_to_update, debug) + + def bulk_update_domain_information(self, di_to_update, debug): + if debug: + logger.info(f"Updating these TransitionDomains: {[item for item in di_to_update]}") + + logger.info(f"Ready to update {len(di_to_update)} TransitionDomains.") + + logger.info(f"{TerminalColors.MAGENTA}" "Beginning mass DomainInformation update..." f"{TerminalColors.ENDC}") changed_fields = [ "address_line1", @@ -283,15 +244,9 @@ class Command(BaseCommand): page = paginator.page(page_num) DomainInformation.objects.bulk_update(page.object_list, changed_fields) - if not debug: - logger.info( - f"{TerminalColors.OKGREEN}" - f"Updated {len(di_to_update)} DomainInformations." - f"{TerminalColors.ENDC}" - ) - else: - logger.info( - f"{TerminalColors.OKGREEN}" - f"Updated {len(di_to_update)} DomainInformations: {[item for item in di_to_update]}" - f"{TerminalColors.ENDC}" - ) + if debug: + logger.info(f"Updated these DomainInformations: {[item for item in di_to_update]}") + + logger.info( + f"{TerminalColors.OKGREEN}" f"Updated {len(di_to_update)} DomainInformations." f"{TerminalColors.ENDC}" + ) diff --git a/src/registrar/management/commands/utility/extra_transition_domain_helper.py b/src/registrar/management/commands/utility/extra_transition_domain_helper.py index be84e7681..5fcab8f82 100644 --- a/src/registrar/management/commands/utility/extra_transition_domain_helper.py +++ b/src/registrar/management/commands/utility/extra_transition_domain_helper.py @@ -751,26 +751,28 @@ class FileDataHolder: full_filename = date + "." + filename_without_date return (full_filename, can_infer) + class OrganizationDataLoader: """Saves organization data onto Transition Domains. Handles file parsing.""" + def __init__(self, options: TransitionDomainArguments): # Globally stores event logs and organizes them self.parse_logs = FileTransitionLog() self.debug = options.debug options.pattern_map_params = [ - ( - EnumFilenames.DOMAIN_ADDITIONAL, - options.domain_additional_filename, - DomainAdditionalData, - "domainname", - ), - ( - EnumFilenames.ORGANIZATION_ADHOC, - options.organization_adhoc_filename, - OrganizationAdhoc, - "orgid", - ), + ( + EnumFilenames.DOMAIN_ADDITIONAL, + options.domain_additional_filename, + DomainAdditionalData, + "domainname", + ), + ( + EnumFilenames.ORGANIZATION_ADHOC, + options.organization_adhoc_filename, + OrganizationAdhoc, + "orgid", + ), ] # Reads and parses organization data self.parsed_data = ExtraTransitionDomain(options) @@ -779,15 +781,13 @@ class OrganizationDataLoader: self.tds_to_update = [] self.tds_failed_to_update = [] - + def update_organization_data_for_all(self): """Updates org data for all TransitionDomains""" all_transition_domains = TransitionDomain.objects.all() if len(all_transition_domains) < 1: raise Exception( - f"{TerminalColors.FAIL}" - "No TransitionDomains exist. Cannot update." - f"{TerminalColors.ENDC}" + f"{TerminalColors.FAIL}" "No TransitionDomains exist. Cannot update." f"{TerminalColors.ENDC}" ) # Store all actions we want to perform in tds_to_update @@ -822,26 +822,20 @@ class OrganizationDataLoader: if len(self.tds_failed_to_update) > 0: logger.error( - "Failed to update. An exception was encountered " + "Failed to update. An exception was encountered " f"on the following TransitionDomains: {[item for item in self.tds_failed_to_update]}" ) raise Exception("Failed to update TransitionDomains") if not self.debug: - logger.info( - f"Ready to update {len(self.tds_to_update)} TransitionDomains." - ) + logger.info(f"Ready to update {len(self.tds_to_update)} TransitionDomains.") else: logger.info( f"Ready to update {len(self.tds_to_update)} TransitionDomains: {[item for item in self.tds_failed_to_update]}" ) def bulk_update_transition_domains(self, update_list): - logger.info( - f"{TerminalColors.MAGENTA}" - "Beginning mass TransitionDomain update..." - f"{TerminalColors.ENDC}" - ) + logger.info(f"{TerminalColors.MAGENTA}" "Beginning mass TransitionDomain update..." f"{TerminalColors.ENDC}") changed_fields = [ "address_line", @@ -905,7 +899,7 @@ class OrganizationDataLoader: self.log_add_or_changed_values(EnumFilenames.AUTHORITY_ADHOC, changed_fields, domain_name) return transition_domain - + def get_org_info(self, domain_name) -> OrganizationAdhoc: """Maps an id given in get_domain_data to a organization_adhoc record which has its corresponding definition""" @@ -914,17 +908,17 @@ class OrganizationDataLoader: return None org_id = domain_info.orgid return self.get_organization_adhoc(org_id) - + def get_organization_adhoc(self, desired_id) -> OrganizationAdhoc: """Grabs a corresponding row within the ORGANIZATION_ADHOC file, based off a desired_id""" return self.get_object_by_id(EnumFilenames.ORGANIZATION_ADHOC, desired_id) - + def get_domain_data(self, desired_id) -> DomainAdditionalData: """Grabs a corresponding row within the DOMAIN_ADDITIONAL file, based off a desired_id""" return self.get_object_by_id(EnumFilenames.DOMAIN_ADDITIONAL, desired_id) - + def get_object_by_id(self, file_type: EnumFilenames, desired_id): """Returns a field in a dictionary based off the type and id. @@ -1032,9 +1026,7 @@ class ExtraTransitionDomain: # metadata about each file and associate it with an enum. # That way if we want the data located at the agency_adhoc file, # we can just call EnumFilenames.AGENCY_ADHOC. - if ( - options.pattern_map_params is None or options.pattern_map_params == [] - ): + if options.pattern_map_params is None or options.pattern_map_params == []: options.pattern_map_params = [ ( EnumFilenames.AGENCY_ADHOC, diff --git a/src/registrar/management/commands/utility/transition_domain_arguments.py b/src/registrar/management/commands/utility/transition_domain_arguments.py index bfe1dd84e..bd6d8a970 100644 --- a/src/registrar/management/commands/utility/transition_domain_arguments.py +++ b/src/registrar/management/commands/utility/transition_domain_arguments.py @@ -1,5 +1,5 @@ from dataclasses import dataclass, field -from typing import List, Optional +from typing import Optional from registrar.management.commands.utility.epp_data_containers import EnumFilenames diff --git a/src/registrar/tests/test_transition_domain_migrations.py b/src/registrar/tests/test_transition_domain_migrations.py index 3e75c6b4e..3b6a04a89 100644 --- a/src/registrar/tests/test_transition_domain_migrations.py +++ b/src/registrar/tests/test_transition_domain_migrations.py @@ -18,6 +18,7 @@ from unittest.mock import patch from .common import less_console_noise + class TestOrganizationMigration(TestCase): def setUp(self): """ """ @@ -65,7 +66,7 @@ class TestOrganizationMigration(TestCase): def run_transfer_domains(self): call_command("transfer_transition_domains_to_domains") - + def run_load_organization_data(self): # noqa here (E501) because splitting this up makes it # confusing to read. @@ -162,7 +163,7 @@ class TestOrganizationMigration(TestCase): # == Second, try adding org data to it == # self.run_load_organization_data() - # == Third, test that we've loaded data as we expect == # + # == Third, test that we've loaded data as we expect == # transition_domains = TransitionDomain.objects.filter(domain_name="fakewebsite2.gov") # Should return three objects (three unique emails) @@ -171,33 +172,32 @@ class TestOrganizationMigration(TestCase): # Lets test the first one transition = transition_domains.first() expected_transition_domain = TransitionDomain( - username='alexandra.bobbitt5@test.com', - domain_name='fakewebsite2.gov', - status='on hold', + username="alexandra.bobbitt5@test.com", + domain_name="fakewebsite2.gov", + status="on hold", email_sent=True, - organization_type='Federal', - organization_name='Fanoodle', - federal_type='Executive', - federal_agency='Department of Commerce', + organization_type="Federal", + organization_name="Fanoodle", + federal_type="Executive", + federal_agency="Department of Commerce", epp_creation_date=datetime.date(2004, 5, 7), epp_expiration_date=datetime.date(2023, 9, 30), - first_name='Seline', - middle_name='testmiddle2', - last_name='Tower', + first_name="Seline", + middle_name="testmiddle2", + last_name="Tower", title=None, - email='stower3@answers.com', - phone='151-539-6028', - address_line='93001 Arizona Drive', - city='Columbus', - state_territory='Oh', - zipcode='43268' + email="stower3@answers.com", + phone="151-539-6028", + address_line="93001 Arizona Drive", + city="Columbus", + state_territory="Oh", + zipcode="43268", ) expected_transition_domain.id = transition.id self.assertEqual(transition, expected_transition_domain) - + def test_load_organization_data_domain_information(self): - self.maxDiff = None # == First, parse all existing data == # self.run_load_domains() self.run_transfer_domains() @@ -205,14 +205,14 @@ class TestOrganizationMigration(TestCase): # == Second, try adding org data to it == # self.run_load_organization_data() - # == Third, test that we've loaded data as we expect == # - _domain = Domain.objects.filter(name="fakewebsite2.gov").get() + # == Third, test that we've loaded data as we expect == # + _domain = Domain.objects.filter(name="fakewebsite2.gov").get() domain_information = DomainInformation.objects.filter(domain=_domain).get() - - self.assertEqual(domain_information.address_line1, '93001 Arizona Drive') - self.assertEqual(domain_information.city, 'Columbus') - self.assertEqual(domain_information.state_territory, 'Oh') - self.assertEqual(domain_information.zipcode, '43268') + + self.assertEqual(domain_information.address_line1, "93001 Arizona Drive") + self.assertEqual(domain_information.city, "Columbus") + self.assertEqual(domain_information.state_territory, "Oh") + self.assertEqual(domain_information.zipcode, "43268") def test_load_organization_data_integrity(self): """Validates data integrity with the load_org_data command""" @@ -222,7 +222,7 @@ class TestOrganizationMigration(TestCase): # Second, try adding org data to it self.run_load_organization_data() - + # Third, test that we didn't corrupt any data expected_total_transition_domains = 9 expected_total_domains = 5 @@ -245,6 +245,7 @@ class TestOrganizationMigration(TestCase): expected_missing_domain_invitations, ) + class TestMigrations(TestCase): def setUp(self): """ """ @@ -308,7 +309,7 @@ class TestMigrations(TestCase): migrationJSON=self.migration_json_filename, disablePrompts=True, ) - + def run_load_organization_data(self): # noqa here (E501) because splitting this up makes it # confusing to read. From 70360cd871ddf4a6ab01e53be1344fa9289f7368 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 17 Nov 2023 16:03:16 -0700 Subject: [PATCH 12/46] Simplified logic --- .../utility/extra_transition_domain_helper.py | 186 +++++++----------- 1 file changed, 74 insertions(+), 112 deletions(-) diff --git a/src/registrar/management/commands/utility/extra_transition_domain_helper.py b/src/registrar/management/commands/utility/extra_transition_domain_helper.py index 5fcab8f82..b15a93658 100644 --- a/src/registrar/management/commands/utility/extra_transition_domain_helper.py +++ b/src/registrar/management/commands/utility/extra_transition_domain_helper.py @@ -778,65 +778,45 @@ class OrganizationDataLoader: self.parsed_data = ExtraTransitionDomain(options) # options.infer_filenames will always be false when not SETTING.DEBUG self.parsed_data.parse_all_files(options.infer_filenames) - self.tds_to_update = [] - self.tds_failed_to_update = [] def update_organization_data_for_all(self): """Updates org data for all TransitionDomains""" all_transition_domains = TransitionDomain.objects.all() - if len(all_transition_domains) < 1: + if len(all_transition_domains) == 0: raise Exception( - f"{TerminalColors.FAIL}" "No TransitionDomains exist. Cannot update." f"{TerminalColors.ENDC}" + f"{TerminalColors.FAIL}No TransitionDomains exist. Cannot update.{TerminalColors.ENDC}" ) - # Store all actions we want to perform in tds_to_update self.prepare_transition_domains(all_transition_domains) - # Then if we don't run into any exceptions, bulk_update it + + logger.info(f"{TerminalColors.MAGENTA}" "Beginning mass TransitionDomain update..." f"{TerminalColors.ENDC}") self.bulk_update_transition_domains(self.tds_to_update) + return self.tds_to_update def prepare_transition_domains(self, transition_domains): for item in transition_domains: - try: - updated = self.parse_org_data(item.domain_name, item) - self.tds_to_update.append(updated) - if self.debug: - logger.info(item.display_transition_domain()) - logger.info( - f"Successfully updated TransitionDomain: \n" - f"{TerminalColors.OKCYAN}" - f"{item.display_transition_domain()}" - f"{TerminalColors.ENDC}" - ) - except Exception as err: - logger.error(err) - self.tds_failed_to_update.append(item) - if self.debug: - logger.error( - f"Failed to update TransitionDomain: \n" - f"{TerminalColors.YELLOW}" - f"{item.display_transition_domain()}" - f"{TerminalColors.ENDC}" - ) + updated = self.parse_org_data(item.domain_name, item) + self.tds_to_update.append(updated) + if self.debug: + logger.info( + f"""{TerminalColors.OKCYAN} + Successfully updated: + {item.display_transition_domain()} + {TerminalColors.ENDC}""" + ) - if len(self.tds_failed_to_update) > 0: - logger.error( - "Failed to update. An exception was encountered " - f"on the following TransitionDomains: {[item for item in self.tds_failed_to_update]}" - ) - raise Exception("Failed to update TransitionDomains") + if self.debug: + logger.info(f"Updating the following: {[item for item in self.tds_to_update]}") - if not self.debug: - logger.info(f"Ready to update {len(self.tds_to_update)} TransitionDomains.") - else: - logger.info( - f"Ready to update {len(self.tds_to_update)} TransitionDomains: {[item for item in self.tds_failed_to_update]}" - ) + logger.info( + f"""{TerminalColors.MAGENTA} + Ready to update {len(self.tds_to_update)} TransitionDomains. + {TerminalColors.ENDC}""" + ) def bulk_update_transition_domains(self, update_list): - logger.info(f"{TerminalColors.MAGENTA}" "Beginning mass TransitionDomain update..." f"{TerminalColors.ENDC}") - changed_fields = [ "address_line", "city", @@ -851,19 +831,15 @@ class OrganizationDataLoader: for page_num in paginator.page_range: page = paginator.page(page_num) TransitionDomain.objects.bulk_update(page.object_list, changed_fields) + + if self.debug: + logger.info(f"Updated the following: {[item for item in self.tds_to_update]}") - if not self.debug: - logger.info( - f"{TerminalColors.OKGREEN}" - f"Updated {len(self.tds_to_update)} TransitionDomains." - f"{TerminalColors.ENDC}" - ) - else: - logger.info( - f"{TerminalColors.OKGREEN}" - f"Updated {len(self.tds_to_update)} TransitionDomains: {[item for item in self.tds_failed_to_update]}" - f"{TerminalColors.ENDC}" - ) + logger.info( + f"{TerminalColors.OKGREEN}" + f"Updated {len(self.tds_to_update)} TransitionDomains." + f"{TerminalColors.ENDC}" + ) def parse_org_data(self, domain_name, transition_domain: TransitionDomain) -> TransitionDomain: """Grabs organization_name from the parsed files and associates it @@ -876,7 +852,7 @@ class OrganizationDataLoader: self.parse_logs.create_log_item( EnumFilenames.ORGANIZATION_ADHOC, LogCode.ERROR, - f"Could not add organization_name on {domain_name}, no data exists.", + f"Could not add organization data on {domain_name}, no data exists.", domain_name, not self.debug, ) @@ -888,38 +864,32 @@ class OrganizationDataLoader: transition_domain.state_territory = org_info.orgstate transition_domain.zipcode = org_info.orgzip - # Log what happened to each field. The first value - # is the field name that was updated, second is the value - changed_fields = [ - ("address_line", transition_domain.address_line), - ("city", transition_domain.city), - ("state_territory", transition_domain.state_territory), - ("zipcode", transition_domain.zipcode), - ] - self.log_add_or_changed_values(EnumFilenames.AUTHORITY_ADHOC, changed_fields, domain_name) + if self.debug: + # Log what happened to each field. The first value + # is the field name that was updated, second is the value + changed_fields = [ + ("address_line", transition_domain.address_line), + ("city", transition_domain.city), + ("state_territory", transition_domain.state_territory), + ("zipcode", transition_domain.zipcode), + ] + self.log_add_or_changed_values(changed_fields, domain_name) return transition_domain def get_org_info(self, domain_name) -> OrganizationAdhoc: """Maps an id given in get_domain_data to a organization_adhoc record which has its corresponding definition""" - domain_info = self.get_domain_data(domain_name) - if domain_info is None: + # Get a row in the domain_additional file. The id is the domain_name. + domain_additional_row = self.retrieve_file_data_by_id(EnumFilenames.DOMAIN_ADDITIONAL, domain_name) + if domain_additional_row is None: return None - org_id = domain_info.orgid - return self.get_organization_adhoc(org_id) - def get_organization_adhoc(self, desired_id) -> OrganizationAdhoc: - """Grabs a corresponding row within the ORGANIZATION_ADHOC file, - based off a desired_id""" - return self.get_object_by_id(EnumFilenames.ORGANIZATION_ADHOC, desired_id) + # Get a row in the organization_adhoc file. The id is the orgid in domain_info. + org_row = self.retrieve_file_data_by_id(EnumFilenames.ORGANIZATION_ADHOC, domain_additional_row.orgid) + return org_row - def get_domain_data(self, desired_id) -> DomainAdditionalData: - """Grabs a corresponding row within the DOMAIN_ADDITIONAL file, - based off a desired_id""" - return self.get_object_by_id(EnumFilenames.DOMAIN_ADDITIONAL, desired_id) - - def get_object_by_id(self, file_type: EnumFilenames, desired_id): + def retrieve_file_data_by_id(self, file_type: EnumFilenames, desired_id): """Returns a field in a dictionary based off the type and id. vars: @@ -948,59 +918,51 @@ class OrganizationDataLoader: So, `AuthorityAdhoc(...)` """ # Grabs a dict associated with the file_type. - # For example, EnumFilenames.DOMAIN_ADDITIONAL. - desired_type = self.parsed_data.file_data.get(file_type) - if desired_type is None: - self.parse_logs.create_log_item( - file_type, - LogCode.ERROR, - f"Type {file_type} does not exist", - ) + # For example, EnumFilenames.DOMAIN_ADDITIONAL would map to + # whatever data exists on the domain_additional file. + desired_file = self.parsed_data.file_data.get(file_type) + if desired_file is None: + logger.error(f"Type {file_type} does not exist") return None - # Grab the value given an Id within that file_type dict. - # For example, "igorville.gov". - obj = desired_type.data.get(desired_id) - if obj is None: - self.parse_logs.create_log_item( - file_type, - LogCode.ERROR, - f"Id {desired_id} does not exist for {file_type.value[0]}", - ) - return obj + # This is essentially a dictionary of rows. + data_in_file = desired_file.data - def log_add_or_changed_values(self, file_type, values_to_check, domain_name): + # Get a row in the given file, based on an id. + # For instance, "igorville.gov" in domain_additional. + row_in_file = data_in_file.get(desired_id) + if row_in_file is None: + logger.error(f"Id {desired_id} does not exist for {file_type.value[0]}") + + return row_in_file + + def log_add_or_changed_values(self, values_to_check, domain_name): + """Iterates through a list of fields, and determines if we should log + if the field was added or if the field was updated. + + A field is "updated" when it is not None or not "". + A field is "created" when it is either of those things. + + + """ for field_name, value in values_to_check: str_exists = value is not None and value.strip() != "" # Logs if we either added to this property, # or modified it. self._add_or_change_message( - file_type, field_name, value, domain_name, str_exists, ) - def _add_or_change_message(self, file_type, var_name, changed_value, domain_name, is_update=False): + def _add_or_change_message(self, field_name, changed_value, domain_name, is_update=False): """Creates a log instance when a property is successfully changed on a given TransitionDomain.""" if not is_update: - self.parse_logs.create_log_item( - file_type, - LogCode.INFO, - f"Added {var_name} as '{changed_value}' on {domain_name}", - domain_name, - not self.debug, - ) + logger.info(f"Added {field_name} as '{changed_value}' on {domain_name}") else: - self.parse_logs.create_log_item( - file_type, - LogCode.WARNING, - f"Updated existing {var_name} to '{changed_value}' on {domain_name}", - domain_name, - not self.debug, - ) + logger.warning(f"Updated existing {field_name} to '{changed_value}' on {domain_name}") class ExtraTransitionDomain: From 0552a77c649b3f1bd933d00bc3dbd7510de89646 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 17 Nov 2023 16:07:57 -0700 Subject: [PATCH 13/46] Linting --- .../utility/extra_transition_domain_helper.py | 25 ++++++------------- 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/src/registrar/management/commands/utility/extra_transition_domain_helper.py b/src/registrar/management/commands/utility/extra_transition_domain_helper.py index b15a93658..15932df04 100644 --- a/src/registrar/management/commands/utility/extra_transition_domain_helper.py +++ b/src/registrar/management/commands/utility/extra_transition_domain_helper.py @@ -757,7 +757,6 @@ class OrganizationDataLoader: def __init__(self, options: TransitionDomainArguments): # Globally stores event logs and organizes them - self.parse_logs = FileTransitionLog() self.debug = options.debug options.pattern_map_params = [ @@ -784,9 +783,7 @@ class OrganizationDataLoader: """Updates org data for all TransitionDomains""" all_transition_domains = TransitionDomain.objects.all() if len(all_transition_domains) == 0: - raise Exception( - f"{TerminalColors.FAIL}No TransitionDomains exist. Cannot update.{TerminalColors.ENDC}" - ) + raise Exception(f"{TerminalColors.FAIL}No TransitionDomains exist. Cannot update.{TerminalColors.ENDC}") self.prepare_transition_domains(all_transition_domains) @@ -831,14 +828,12 @@ class OrganizationDataLoader: for page_num in paginator.page_range: page = paginator.page(page_num) TransitionDomain.objects.bulk_update(page.object_list, changed_fields) - + if self.debug: logger.info(f"Updated the following: {[item for item in self.tds_to_update]}") logger.info( - f"{TerminalColors.OKGREEN}" - f"Updated {len(self.tds_to_update)} TransitionDomains." - f"{TerminalColors.ENDC}" + f"{TerminalColors.OKGREEN}" f"Updated {len(self.tds_to_update)} TransitionDomains." f"{TerminalColors.ENDC}" ) def parse_org_data(self, domain_name, transition_domain: TransitionDomain) -> TransitionDomain: @@ -849,13 +844,7 @@ class OrganizationDataLoader: org_info = self.get_org_info(domain_name) if org_info is None: - self.parse_logs.create_log_item( - EnumFilenames.ORGANIZATION_ADHOC, - LogCode.ERROR, - f"Could not add organization data on {domain_name}, no data exists.", - domain_name, - not self.debug, - ) + logger.error(f"Could not add organization data on {domain_name}, no data exists.") return transition_domain # Add street info @@ -938,12 +927,12 @@ class OrganizationDataLoader: def log_add_or_changed_values(self, values_to_check, domain_name): """Iterates through a list of fields, and determines if we should log - if the field was added or if the field was updated. - + if the field was added or if the field was updated. + A field is "updated" when it is not None or not "". A field is "created" when it is either of those things. - + """ for field_name, value in values_to_check: str_exists = value is not None and value.strip() != "" From 2659cd816bfdda83e87b03672c0ec25f365fe169 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 17 Nov 2023 16:10:00 -0700 Subject: [PATCH 14/46] Update extra_transition_domain_helper.py --- .../commands/utility/extra_transition_domain_helper.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/management/commands/utility/extra_transition_domain_helper.py b/src/registrar/management/commands/utility/extra_transition_domain_helper.py index 15932df04..68054f27e 100644 --- a/src/registrar/management/commands/utility/extra_transition_domain_helper.py +++ b/src/registrar/management/commands/utility/extra_transition_domain_helper.py @@ -870,15 +870,15 @@ class OrganizationDataLoader: """Maps an id given in get_domain_data to a organization_adhoc record which has its corresponding definition""" # Get a row in the domain_additional file. The id is the domain_name. - domain_additional_row = self.retrieve_file_data_by_id(EnumFilenames.DOMAIN_ADDITIONAL, domain_name) + domain_additional_row = self.retrieve_row_by_id(EnumFilenames.DOMAIN_ADDITIONAL, domain_name) if domain_additional_row is None: return None # Get a row in the organization_adhoc file. The id is the orgid in domain_info. - org_row = self.retrieve_file_data_by_id(EnumFilenames.ORGANIZATION_ADHOC, domain_additional_row.orgid) + org_row = self.retrieve_row_by_id(EnumFilenames.ORGANIZATION_ADHOC, domain_additional_row.orgid) return org_row - def retrieve_file_data_by_id(self, file_type: EnumFilenames, desired_id): + def retrieve_row_by_id(self, file_type: EnumFilenames, desired_id): """Returns a field in a dictionary based off the type and id. vars: From e853d4ef165ae573c6d3238335133b782b198918 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 20 Nov 2023 08:58:27 -0700 Subject: [PATCH 15/46] Code cleanup --- .../commands/load_organization_data.py | 69 ++++++++----------- src/registrar/utility/errors.py | 27 ++++++++ 2 files changed, 55 insertions(+), 41 deletions(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index ae9f7a29b..9a15f646e 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -3,17 +3,18 @@ import argparse import json import logging +import os from django.core.management import BaseCommand from registrar.management.commands.utility.extra_transition_domain_helper import OrganizationDataLoader from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper from registrar.management.commands.utility.transition_domain_arguments import TransitionDomainArguments -from registrar.models import TransitionDomain -from registrar.models.domain import Domain -from registrar.models.domain_information import DomainInformation +from registrar.models import TransitionDomain, DomainInformation from django.core.paginator import Paginator from typing import List +from registrar.utility.errors import LoadOrganizationError, LoadOrganizationErrorCodes + logger = logging.getLogger(__name__) @@ -48,25 +49,15 @@ class Command(BaseCommand): """Process the objects in TransitionDomain.""" # === Parse JSON file === # - # Desired directory for additional TransitionDomain data - # (In the event they are stored seperately) - directory = options["directory"] - # Add a slash if the last character isn't one - if directory and directory[-1] != "/": - directory += "/" - - json_filepath = directory + migration_json_filename + json_filepath = os.path.join(options["directory"], migration_json_filename) # If a JSON was provided, use its values instead of defaults. with open(json_filepath, "r") as jsonFile: # load JSON object as a dictionary try: data = json.load(jsonFile) - # Create an instance of TransitionDomainArguments - # Iterate over the data from the JSON file - for key, value in data.items(): - if value is not None and value.strip() != "": - options[key] = value + # Iterate over the data from the JSON file. Skip any unused values. + options.update({key: value for key, value in data.items() if value is not None and value.strip() != ""}) except Exception as err: logger.error( f"{TerminalColors.FAIL}" @@ -76,6 +67,7 @@ class Command(BaseCommand): ) raise err # === End parse JSON file === # + args = TransitionDomainArguments(**options) changed_fields = [ @@ -90,7 +82,7 @@ class Command(BaseCommand): ==Master data file== domain_additional_filename: {args.domain_additional_filename} - ==Organization name information== + ==Organization data== organization_adhoc_filename: {args.organization_adhoc_filename} ==Containing directory== @@ -151,51 +143,46 @@ class Command(BaseCommand): transition_domains = TransitionDomain.objects.filter( username__in=[item.username for item in desired_objects], domain_name__in=[item.domain_name for item in desired_objects], - ).distinct() + ) + # This indicates that some form of data corruption happened. if len(desired_objects) != len(transition_domains): - raise Exception("Could not find all desired TransitionDomains") - - # Then, for each domain_name grab the associated domain object. - domains = Domain.objects.filter(name__in=[td.domain_name for td in transition_domains]) - # Create dictionary for faster lookup - domains_dict = {d.name: d for d in domains} + raise LoadOrganizationError(code=LoadOrganizationErrorCodes.TRANSITION_DOMAINS_NOT_FOUND) # Start with all DomainInformation objects - filtered_domain_informations = DomainInformation.objects.all() + domain_informations = DomainInformation.objects.all() + domain_informations_dict = {di.domain.name: di for di in domain_informations} - # Then, use each domain object to map domain <--> DomainInformation + # Then, use each domain object to map TransitionDomain <--> DomainInformation # Fetches all DomainInformations in one query. # If any related organization fields have been updated, # we can assume that they modified this information themselves - thus we should not update it. - domain_informations = filtered_domain_informations.filter( - domain__in=domains, + domain_informations = domain_informations.filter( + domain__name__in=[td.domain_name for td in transition_domains], address_line1__isnull=True, city__isnull=True, state_territory__isnull=True, zipcode__isnull=True, ) - domain_informations_dict = {di.domain.name: di for di in domain_informations} - + filtered_domain_informations_dict = {di.domain.name: di for di in domain_informations} for item in transition_domains: - if item.domain_name not in domains_dict: + if item.domain_name not in domain_informations_dict: logger.error(f"Could not add {item.domain_name}. Domain does not exist.") di_failed_to_update.append(item) continue - current_domain = domains_dict[item.domain_name] - if current_domain.name not in domain_informations_dict: + if item.domain_name not in filtered_domain_informations_dict: logger.info( f"{TerminalColors.YELLOW}" - f"Domain {current_domain.name} was updated by a user. Cannot update." + f"Domain {item.domain_name} was updated by a user. Cannot update." f"{TerminalColors.ENDC}" ) di_skipped.append(item) continue # Based on the current domain, grab the right DomainInformation object. - current_domain_information = domain_informations_dict[current_domain.name] + current_domain_information = filtered_domain_informations_dict[item.domain_name] # Update fields current_domain_information.address_line1 = item.address_line @@ -205,27 +192,27 @@ class Command(BaseCommand): di_to_update.append(current_domain_information) if debug: - logger.info(f"Updated {current_domain.name}...") + logger.info(f"Updated {current_domain_information.domain.name}...") if di_failed_to_update: failed = [item.domain_name for item in di_failed_to_update] logger.error( f"""{TerminalColors.FAIL} - Failed to update. An exception was encountered on the following TransitionDomains: {failed} + Failed to update. An exception was encountered on the following DomainInformations: {failed} {TerminalColors.ENDC}""" ) - raise Exception("Failed to update DomainInformations") + raise LoadOrganizationError(code=LoadOrganizationErrorCodes.UPDATE_DOMAIN_INFO_FAILED) if di_skipped: - logger.info(f"Skipped updating {len(di_skipped)} fields. User-supplied data exists") + logger.info(f"Skipped updating {len(di_skipped)} fields. User-supplied data exists.") self.bulk_update_domain_information(di_to_update, debug) def bulk_update_domain_information(self, di_to_update, debug): if debug: - logger.info(f"Updating these TransitionDomains: {[item for item in di_to_update]}") + logger.info(f"Updating these DomainInformations: {[item for item in di_to_update]}") - logger.info(f"Ready to update {len(di_to_update)} TransitionDomains.") + logger.info(f"Ready to update {len(di_to_update)} DomainInformations.") logger.info(f"{TerminalColors.MAGENTA}" "Beginning mass DomainInformation update..." f"{TerminalColors.ENDC}") diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index 4ca3a9a12..b9f24eba3 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -57,6 +57,33 @@ contact help@get.gov return f"{self.message}" +class LoadOrganizationErrorCodes(IntEnum): + TRANSITION_DOMAINS_NOT_FOUND = 1 + UPDATE_DOMAIN_INFO_FAILED = 2 + + +class LoadOrganizationError(Exception): + """ + Error class used in the load_organization_data script + """ + + _error_mapping = { + LoadOrganizationErrorCodes.TRANSITION_DOMAINS_NOT_FOUND: ( + "Could not find all desired TransitionDomains. " "(Possible data corruption?)" + ), + LoadOrganizationErrorCodes.UPDATE_DOMAIN_INFO_FAILED: "Failed to update DomainInformation", + } + + def __init__(self, *args, code=None, **kwargs): + super().__init__(*args, **kwargs) + self.code = code + if self.code in self._error_mapping: + self.message = self._error_mapping.get(self.code) + + def __str__(self): + return f"{self.message}" + + class NameserverErrorCodes(IntEnum): """Used in the NameserverError class for error mapping. From 0ce4a04b4bf78f957e3f96e882bf186409e52616 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 20 Nov 2023 10:14:10 -0700 Subject: [PATCH 16/46] Add documentation --- docs/operations/data_migration.md | 52 +++++++++++++++++++ .../commands/load_organization_data.py | 14 ++++- 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index 7290349ad..fe3d5f45e 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -421,3 +421,55 @@ purposes Used by the migration scripts to trigger a prompt for deleting all table entries. Useful for testing purposes, but *use with caution* + +## Import organization data +During MVP, our import scripts did not populate the following fields: `address_line, city, state_territory, and zipcode`. This was primarily due to time constraints. Because of this, we need to run a follow-on script to load this remaining data on each `DomainInformation` object. + +This script is intended to run under the assumption that the [load_transition_domain](#step-1-load-transition-domains) and the [transfer_transition_domains_to_domains](#step-2-transfer-transition-domain-data-into-main-domain-tables) scripts have already been ran. + +##### LOCAL COMMAND +to run this command locally, enter the following: +```shell +docker compose run -T app ./manage.py load_organization_data {filename_of_migration_json} --debug +``` +* filename_of_migration_filepath_json - This is a [JSON containing a list of filenames](#step-2-obtain-json-file-for-file-locations). This same file was used in the preceeding steps, `load_transition_domain` and `transfer_transition_domains_to_domains`, however, this script only needs two fields: +``` +{ + "domain_additional_filename": "example.domainadditionaldatalink.adhoc.dotgov.txt", + "organization_adhoc_filename": "example.organization.adhoc.dotgov.txt" +} +``` +If you already possess the old JSON, you do not need to modify it. This script can run even if you specify multiple filepaths. It will just skip over unused ones. + +**Example** +```shell +docker compose run -T app ./manage.py load_organization_data migrationFilepaths.json --debug +``` + +##### SANDBOX COMMAND +```shell +./manage.py load_organization_data {filename_of_migration_json} --debug +``` +* **filename_of_migration_filepath_json** - This is a [JSON containing a list of filenames](#step-2-obtain-json-file-for-file-locations). This same file was used in the preceeding steps, `load_transition_domain` and `transfer_transition_domains_to_domains`, however, this script only needs two fields: +``` +{ + "domain_additional_filename": "example.domainadditionaldatalink.adhoc.dotgov.txt", + "organization_adhoc_filename": "example.organization.adhoc.dotgov.txt" +} +``` +If you already possess the old JSON, you do not need to modify it. This script can run even if you specify multiple filepaths. It will just skip over unused ones. + +**Example** +```shell +./manage.py load_organization_data migrationFilepaths.json --debug +``` + +##### Optional parameters +The `load_organization_data` script has five optional parameters. These are as follows: +| | Parameter | Description | +|:-:|:---------------------------------|:----------------------------------------------------------------------------| +| 1 | **sep** | Determines the file separator. Defaults to "\|" | +| 2 | **debug** | Increases logging detail. Defaults to False | +| 3 | **directory** | Specifies the containing directory of the data. Defaults to "migrationdata" | +| 4 | **domain_additional_filename** | Specifies the filename of domain_additional. Used as an override for the JSON. Has no default. | +| 5 | **organization_adhoc_filename** | Specifies the filename of organization_adhoc. Used as an override for the JSON. Has no default. | diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index 9a15f646e..3cfba0ca8 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -35,11 +35,13 @@ class Command(BaseCommand): parser.add_argument("--directory", default="migrationdata", help="Desired directory") + # Serves as a domain_additional_filename override parser.add_argument( "--domain_additional_filename", help="Defines the filename for additional domain data", ) + # Serves as a organization_adhoc_filename override parser.add_argument( "--organization_adhoc_filename", help="Defines the filename for domain type adhocs", @@ -56,8 +58,18 @@ class Command(BaseCommand): # load JSON object as a dictionary try: data = json.load(jsonFile) + + skipped_fields = ["domain_additional_filename", "organization_adhoc_filename"] # Iterate over the data from the JSON file. Skip any unused values. - options.update({key: value for key, value in data.items() if value is not None and value.strip() != ""}) + for key, value in data.items(): + if value is not None or value.strip() != "": + continue + + # If any key in skipped_fields has a value, then + # we override what is specified in the JSON. + if key not in skipped_fields: + options[key] = value + except Exception as err: logger.error( f"{TerminalColors.FAIL}" From 04c7bdd3b6b0a5720e851084d0f2511b97e1bf56 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 20 Nov 2023 10:38:53 -0700 Subject: [PATCH 17/46] Code cleanup --- .../utility/extra_transition_domain_helper.py | 26 +++++-------------- src/registrar/utility/errors.py | 2 ++ 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/src/registrar/management/commands/utility/extra_transition_domain_helper.py b/src/registrar/management/commands/utility/extra_transition_domain_helper.py index 68054f27e..0ad6fa2ab 100644 --- a/src/registrar/management/commands/utility/extra_transition_domain_helper.py +++ b/src/registrar/management/commands/utility/extra_transition_domain_helper.py @@ -12,6 +12,7 @@ import sys from typing import Dict from django.core.paginator import Paginator from registrar.models.transition_domain import TransitionDomain +from registrar.utility.errors import LoadOrganizationError, LoadOrganizationErrorCodes from .epp_data_containers import ( AgencyAdhoc, @@ -756,9 +757,9 @@ class OrganizationDataLoader: """Saves organization data onto Transition Domains. Handles file parsing.""" def __init__(self, options: TransitionDomainArguments): - # Globally stores event logs and organizes them self.debug = options.debug + # We want to data from the domain_additional file and the organization_adhoc file options.pattern_map_params = [ ( EnumFilenames.DOMAIN_ADDITIONAL, @@ -773,17 +774,20 @@ class OrganizationDataLoader: "orgid", ), ] + # Reads and parses organization data self.parsed_data = ExtraTransitionDomain(options) + # options.infer_filenames will always be false when not SETTING.DEBUG self.parsed_data.parse_all_files(options.infer_filenames) + self.tds_to_update = [] def update_organization_data_for_all(self): """Updates org data for all TransitionDomains""" all_transition_domains = TransitionDomain.objects.all() if len(all_transition_domains) == 0: - raise Exception(f"{TerminalColors.FAIL}No TransitionDomains exist. Cannot update.{TerminalColors.ENDC}") + raise LoadOrganizationError(code=LoadOrganizationErrorCodes.EMPTY_TRANSITION_DOMAIN_TABLE) self.prepare_transition_domains(all_transition_domains) @@ -887,24 +891,6 @@ class OrganizationDataLoader: desired_id: str -> Which id you want to search on. An example would be `"12"` or `"igorville.gov"` - - Explanation: - Each data file has an associated type (file_type) for tracking purposes. - - Each file_type is a dictionary which - contains a dictionary of row[id_field]: object. - - In practice, this would look like: - - EnumFilenames.AUTHORITY_ADHOC: { - "1": AuthorityAdhoc(...), - "2": AuthorityAdhoc(...), - ... - } - - desired_id will then specify which id to grab. If we wanted "1", - then this function will return the value of id "1". - So, `AuthorityAdhoc(...)` """ # Grabs a dict associated with the file_type. # For example, EnumFilenames.DOMAIN_ADDITIONAL would map to diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index b9f24eba3..71ffdb7ed 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -60,6 +60,7 @@ contact help@get.gov class LoadOrganizationErrorCodes(IntEnum): TRANSITION_DOMAINS_NOT_FOUND = 1 UPDATE_DOMAIN_INFO_FAILED = 2 + EMPTY_TRANSITION_DOMAIN_TABLE = 3 class LoadOrganizationError(Exception): @@ -72,6 +73,7 @@ class LoadOrganizationError(Exception): "Could not find all desired TransitionDomains. " "(Possible data corruption?)" ), LoadOrganizationErrorCodes.UPDATE_DOMAIN_INFO_FAILED: "Failed to update DomainInformation", + LoadOrganizationErrorCodes.EMPTY_TRANSITION_DOMAIN_TABLE: "No TransitionDomains exist. Cannot update." } def __init__(self, *args, code=None, **kwargs): From f97dba7ca4c71278d2f4694c43206a4a250996cd Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 20 Nov 2023 10:50:09 -0700 Subject: [PATCH 18/46] Fix typo --- src/registrar/management/commands/load_organization_data.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index 3cfba0ca8..c9203521d 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -62,12 +62,12 @@ class Command(BaseCommand): skipped_fields = ["domain_additional_filename", "organization_adhoc_filename"] # Iterate over the data from the JSON file. Skip any unused values. for key, value in data.items(): - if value is not None or value.strip() != "": + if value is None or value.strip() == "": continue # If any key in skipped_fields has a value, then # we override what is specified in the JSON. - if key not in skipped_fields: + if key not in skipped_fields or : options[key] = value except Exception as err: From 8544c2948f216e1c70feb17f6611637b7448ae2d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 20 Nov 2023 11:03:25 -0700 Subject: [PATCH 19/46] Fix code typo, v2 --- src/registrar/management/commands/load_organization_data.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index c9203521d..283ef6f0c 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -67,7 +67,7 @@ class Command(BaseCommand): # If any key in skipped_fields has a value, then # we override what is specified in the JSON. - if key not in skipped_fields or : + if key not in skipped_fields: options[key] = value except Exception as err: From 42ef0262545938f9a9e8ba0abe7532057959216d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 20 Nov 2023 11:19:55 -0700 Subject: [PATCH 20/46] Update load_organization_data.py --- src/registrar/management/commands/load_organization_data.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index 283ef6f0c..57290a41e 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -67,7 +67,7 @@ class Command(BaseCommand): # If any key in skipped_fields has a value, then # we override what is specified in the JSON. - if key not in skipped_fields: + if options not in skipped_fields or options[key] is not None: options[key] = value except Exception as err: From 2e44a9099a695b3fd5c4401384f7b06125ef1092 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 20 Nov 2023 11:28:07 -0700 Subject: [PATCH 21/46] Bug fix Accidentally left some stray code from a minor refactor --- src/registrar/management/commands/load_organization_data.py | 2 +- src/registrar/utility/errors.py | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index 57290a41e..2d6d7adff 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -67,7 +67,7 @@ class Command(BaseCommand): # If any key in skipped_fields has a value, then # we override what is specified in the JSON. - if options not in skipped_fields or options[key] is not None: + if options not in skipped_fields: options[key] = value except Exception as err: diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index 71ffdb7ed..ba9e9aaa6 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -58,6 +58,12 @@ contact help@get.gov class LoadOrganizationErrorCodes(IntEnum): + """Used when running the load_organization_data script + Overview of error codes: + - 1 TRANSITION_DOMAINS_NOT_FOUND + - 2 UPDATE_DOMAIN_INFO_FAILED + - 3 EMPTY_TRANSITION_DOMAIN_TABLE + """ TRANSITION_DOMAINS_NOT_FOUND = 1 UPDATE_DOMAIN_INFO_FAILED = 2 EMPTY_TRANSITION_DOMAIN_TABLE = 3 From 493c03e7d49c4d9dd15fae416f889dde262eafd9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 20 Nov 2023 11:53:57 -0700 Subject: [PATCH 22/46] Linting --- .../management/commands/load_organization_data.py | 9 ++++++--- .../commands/utility/extra_transition_domain_helper.py | 6 +++--- src/registrar/utility/errors.py | 5 ++++- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index 2d6d7adff..d98365a9b 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -64,7 +64,7 @@ class Command(BaseCommand): for key, value in data.items(): if value is None or value.strip() == "": continue - + # If any key in skipped_fields has a value, then # we override what is specified in the JSON. if options not in skipped_fields: @@ -163,7 +163,7 @@ class Command(BaseCommand): # Start with all DomainInformation objects domain_informations = DomainInformation.objects.all() - domain_informations_dict = {di.domain.name: di for di in domain_informations} + domain_informations_dict = {di.domain.name: di for di in domain_informations if di.domain is not None} # Then, use each domain object to map TransitionDomain <--> DomainInformation # Fetches all DomainInformations in one query. @@ -177,7 +177,7 @@ class Command(BaseCommand): zipcode__isnull=True, ) - filtered_domain_informations_dict = {di.domain.name: di for di in domain_informations} + filtered_domain_informations_dict = {di.domain.name: di for di in domain_informations if di.domain is not None} for item in transition_domains: if item.domain_name not in domain_informations_dict: logger.error(f"Could not add {item.domain_name}. Domain does not exist.") @@ -196,6 +196,9 @@ class Command(BaseCommand): # Based on the current domain, grab the right DomainInformation object. current_domain_information = filtered_domain_informations_dict[item.domain_name] + if current_domain_information.domain is None or current_domain_information.domain.name is None: + raise LoadOrganizationError(code=LoadOrganizationErrorCodes.DOMAIN_NAME_WAS_NONE) + # Update fields current_domain_information.address_line1 = item.address_line current_domain_information.city = item.city diff --git a/src/registrar/management/commands/utility/extra_transition_domain_helper.py b/src/registrar/management/commands/utility/extra_transition_domain_helper.py index 0ad6fa2ab..06b210950 100644 --- a/src/registrar/management/commands/utility/extra_transition_domain_helper.py +++ b/src/registrar/management/commands/utility/extra_transition_domain_helper.py @@ -9,7 +9,7 @@ import logging import os import sys -from typing import Dict +from typing import Dict, List from django.core.paginator import Paginator from registrar.models.transition_domain import TransitionDomain from registrar.utility.errors import LoadOrganizationError, LoadOrganizationErrorCodes @@ -781,7 +781,7 @@ class OrganizationDataLoader: # options.infer_filenames will always be false when not SETTING.DEBUG self.parsed_data.parse_all_files(options.infer_filenames) - self.tds_to_update = [] + self.tds_to_update: List[TransitionDomain] = [] def update_organization_data_for_all(self): """Updates org data for all TransitionDomains""" @@ -870,7 +870,7 @@ class OrganizationDataLoader: return transition_domain - def get_org_info(self, domain_name) -> OrganizationAdhoc: + def get_org_info(self, domain_name) -> OrganizationAdhoc | None: """Maps an id given in get_domain_data to a organization_adhoc record which has its corresponding definition""" # Get a row in the domain_additional file. The id is the domain_name. diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index ba9e9aaa6..4e0745a2d 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -64,9 +64,11 @@ class LoadOrganizationErrorCodes(IntEnum): - 2 UPDATE_DOMAIN_INFO_FAILED - 3 EMPTY_TRANSITION_DOMAIN_TABLE """ + TRANSITION_DOMAINS_NOT_FOUND = 1 UPDATE_DOMAIN_INFO_FAILED = 2 EMPTY_TRANSITION_DOMAIN_TABLE = 3 + DOMAIN_NAME_WAS_NONE = 4 class LoadOrganizationError(Exception): @@ -79,7 +81,8 @@ class LoadOrganizationError(Exception): "Could not find all desired TransitionDomains. " "(Possible data corruption?)" ), LoadOrganizationErrorCodes.UPDATE_DOMAIN_INFO_FAILED: "Failed to update DomainInformation", - LoadOrganizationErrorCodes.EMPTY_TRANSITION_DOMAIN_TABLE: "No TransitionDomains exist. Cannot update." + LoadOrganizationErrorCodes.EMPTY_TRANSITION_DOMAIN_TABLE: "No TransitionDomains exist. Cannot update.", + LoadOrganizationErrorCodes.DOMAIN_NAME_WAS_NONE: "DomainInformation was updated, but domain was None", } def __init__(self, *args, code=None, **kwargs): From 6f3a27a888ec3c02cf44b0280ec9a68ccc788a81 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 20 Nov 2023 12:16:06 -0700 Subject: [PATCH 23/46] Add more detailed logging --- .../management/commands/load_organization_data.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index d98365a9b..69dafdd4d 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -199,6 +199,15 @@ class Command(BaseCommand): if current_domain_information.domain is None or current_domain_information.domain.name is None: raise LoadOrganizationError(code=LoadOrganizationErrorCodes.DOMAIN_NAME_WAS_NONE) + if item.address_line is None and item.city is None and item.state_territory and item.zipcode is None: + logger.info( + f"{TerminalColors.YELLOW}" + f"Domain {item.domain_name} has no Organization Data. Cannot update." + f"{TerminalColors.ENDC}" + ) + di_skipped.append(item) + continue + # Update fields current_domain_information.address_line1 = item.address_line current_domain_information.city = item.city @@ -219,7 +228,7 @@ class Command(BaseCommand): raise LoadOrganizationError(code=LoadOrganizationErrorCodes.UPDATE_DOMAIN_INFO_FAILED) if di_skipped: - logger.info(f"Skipped updating {len(di_skipped)} fields. User-supplied data exists.") + logger.info(f"Skipped updating {len(di_skipped)} fields. User-supplied data exists, or there is no data.") self.bulk_update_domain_information(di_to_update, debug) From e7db6b8842701dc36c405ffc6cf9ea56825ba7e2 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 20 Nov 2023 15:27:44 -0700 Subject: [PATCH 24/46] Rename variable and move if statement --- .../commands/load_organization_data.py | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index 69dafdd4d..ee193f115 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -111,7 +111,7 @@ class Command(BaseCommand): logger.info(f"{TerminalColors.MAGENTA}" "Loading organization data onto TransitionDomain tables...") load = OrganizationDataLoader(args) - domain_information_to_update = load.update_organization_data_for_all() + transition_domains = load.update_organization_data_for_all() # Reprompt the user to reinspect before updating DomainInformation proceed = TerminalHelper.prompt_for_execution( @@ -127,7 +127,7 @@ class Command(BaseCommand): directory: {args.directory} ==Proposed Changes== - Number of DomainInformation objects to change: {len(domain_information_to_update)} + Number of DomainInformation objects to (potentially) change: {len(transition_domains)} """, prompt_title="Do you wish to load organization data for DomainInformation?", ) @@ -135,8 +135,8 @@ class Command(BaseCommand): if not proceed: return None - if len(domain_information_to_update) == 0: - logger.error(f"{TerminalColors.MAGENTA}" "No DomainInformation objects exist" f"{TerminalColors.ENDC}") + if len(transition_domains) == 0: + logger.error(f"{TerminalColors.MAGENTA}" "No TransitionDomain objects exist" f"{TerminalColors.ENDC}") return None logger.info( @@ -144,7 +144,7 @@ class Command(BaseCommand): "Preparing to load organization data onto DomainInformation tables..." f"{TerminalColors.ENDC}" ) - self.update_domain_information(domain_information_to_update, args.debug) + self.update_domain_information(transition_domains, args.debug) def update_domain_information(self, desired_objects: List[TransitionDomain], debug): di_to_update = [] @@ -183,6 +183,15 @@ class Command(BaseCommand): logger.error(f"Could not add {item.domain_name}. Domain does not exist.") di_failed_to_update.append(item) continue + + if item.address_line is None and item.city is None and item.state_territory and item.zipcode is None: + logger.info( + f"{TerminalColors.YELLOW}" + f"Domain {item.domain_name} has no Organization Data. Cannot update." + f"{TerminalColors.ENDC}" + ) + di_skipped.append(item) + continue if item.domain_name not in filtered_domain_informations_dict: logger.info( @@ -199,15 +208,6 @@ class Command(BaseCommand): if current_domain_information.domain is None or current_domain_information.domain.name is None: raise LoadOrganizationError(code=LoadOrganizationErrorCodes.DOMAIN_NAME_WAS_NONE) - if item.address_line is None and item.city is None and item.state_territory and item.zipcode is None: - logger.info( - f"{TerminalColors.YELLOW}" - f"Domain {item.domain_name} has no Organization Data. Cannot update." - f"{TerminalColors.ENDC}" - ) - di_skipped.append(item) - continue - # Update fields current_domain_information.address_line1 = item.address_line current_domain_information.city = item.city From 221534e19dd04419f3a377601f62aff955193ab5 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 20 Nov 2023 15:49:34 -0700 Subject: [PATCH 25/46] Update comment --- .../commands/utility/extra_transition_domain_helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/management/commands/utility/extra_transition_domain_helper.py b/src/registrar/management/commands/utility/extra_transition_domain_helper.py index 06b210950..781972077 100644 --- a/src/registrar/management/commands/utility/extra_transition_domain_helper.py +++ b/src/registrar/management/commands/utility/extra_transition_domain_helper.py @@ -878,7 +878,7 @@ class OrganizationDataLoader: if domain_additional_row is None: return None - # Get a row in the organization_adhoc file. The id is the orgid in domain_info. + # Get a row in the organization_adhoc file. The id is the orgid in domain_additional_row. org_row = self.retrieve_row_by_id(EnumFilenames.ORGANIZATION_ADHOC, domain_additional_row.orgid) return org_row From 0969a76610dd3d538abf8163efc56271c4a15a6f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 20 Nov 2023 15:53:04 -0700 Subject: [PATCH 26/46] Linting --- src/registrar/management/commands/load_organization_data.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index ee193f115..2cd2e6514 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -183,7 +183,7 @@ class Command(BaseCommand): logger.error(f"Could not add {item.domain_name}. Domain does not exist.") di_failed_to_update.append(item) continue - + if item.address_line is None and item.city is None and item.state_territory and item.zipcode is None: logger.info( f"{TerminalColors.YELLOW}" From ba2994812febc6af6de524bf53fd1852652efd36 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 20 Nov 2023 16:19:33 -0700 Subject: [PATCH 27/46] Split JSON parsing into its own function --- .../commands/load_organization_data.py | 60 ++++++++++--------- 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index 2cd2e6514..71be71b39 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -51,35 +51,7 @@ class Command(BaseCommand): """Process the objects in TransitionDomain.""" # === Parse JSON file === # - json_filepath = os.path.join(options["directory"], migration_json_filename) - - # If a JSON was provided, use its values instead of defaults. - with open(json_filepath, "r") as jsonFile: - # load JSON object as a dictionary - try: - data = json.load(jsonFile) - - skipped_fields = ["domain_additional_filename", "organization_adhoc_filename"] - # Iterate over the data from the JSON file. Skip any unused values. - for key, value in data.items(): - if value is None or value.strip() == "": - continue - - # If any key in skipped_fields has a value, then - # we override what is specified in the JSON. - if options not in skipped_fields: - options[key] = value - - except Exception as err: - logger.error( - f"{TerminalColors.FAIL}" - "There was an error loading " - "the JSON responsible for providing filepaths." - f"{TerminalColors.ENDC}" - ) - raise err - # === End parse JSON file === # - + options = self.load_json_settings(options, migration_json_filename) args = TransitionDomainArguments(**options) changed_fields = [ @@ -146,6 +118,36 @@ class Command(BaseCommand): ) self.update_domain_information(transition_domains, args.debug) + def load_json_settings(self, options, migration_json_filename): + """Parses options from the given JSON file.""" + json_filepath = os.path.join(options["directory"], migration_json_filename) + + # If a JSON was provided, use its values instead of defaults. + with open(json_filepath, "r") as jsonFile: + # load JSON object as a dictionary + try: + data = json.load(jsonFile) + + skipped_fields = ["domain_additional_filename", "organization_adhoc_filename"] + # Iterate over the data from the JSON file. Skip any unused values. + for key, value in data.items(): + if value is not None and value.strip() != "": + # If any key in skipped_fields has a value, then + # we override what is specified in the JSON. + if options not in skipped_fields: + options[key] = value + + except Exception as err: + logger.error( + f"{TerminalColors.FAIL}" + "There was an error loading " + "the JSON responsible for providing filepaths." + f"{TerminalColors.ENDC}" + ) + raise err + + return options + def update_domain_information(self, desired_objects: List[TransitionDomain], debug): di_to_update = [] di_failed_to_update = [] From d5cdc624b2f4e3bde9ebb0abc1815f7969ba5add Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 21 Nov 2023 08:01:28 -0700 Subject: [PATCH 28/46] Bug fix --- .../utility/extra_transition_domain_helper.py | 77 +++++++++---------- .../utility/transition_domain_arguments.py | 3 - 2 files changed, 38 insertions(+), 42 deletions(-) diff --git a/src/registrar/management/commands/utility/extra_transition_domain_helper.py b/src/registrar/management/commands/utility/extra_transition_domain_helper.py index 781972077..ed90196ca 100644 --- a/src/registrar/management/commands/utility/extra_transition_domain_helper.py +++ b/src/registrar/management/commands/utility/extra_transition_domain_helper.py @@ -963,45 +963,44 @@ class ExtraTransitionDomain: # metadata about each file and associate it with an enum. # That way if we want the data located at the agency_adhoc file, # we can just call EnumFilenames.AGENCY_ADHOC. - if options.pattern_map_params is None or options.pattern_map_params == []: - options.pattern_map_params = [ - ( - EnumFilenames.AGENCY_ADHOC, - options.agency_adhoc_filename, - AgencyAdhoc, - "agencyid", - ), - ( - EnumFilenames.DOMAIN_ADDITIONAL, - options.domain_additional_filename, - DomainAdditionalData, - "domainname", - ), - ( - EnumFilenames.DOMAIN_ESCROW, - options.domain_escrow_filename, - DomainEscrow, - "domainname", - ), - ( - EnumFilenames.DOMAIN_ADHOC, - options.domain_adhoc_filename, - DomainTypeAdhoc, - "domaintypeid", - ), - ( - EnumFilenames.ORGANIZATION_ADHOC, - options.organization_adhoc_filename, - OrganizationAdhoc, - "orgid", - ), - ( - EnumFilenames.AUTHORITY_ADHOC, - options.authority_adhoc_filename, - AuthorityAdhoc, - "authorityid", - ), - ] + options.pattern_map_params = [ + ( + EnumFilenames.AGENCY_ADHOC, + options.agency_adhoc_filename, + AgencyAdhoc, + "agencyid", + ), + ( + EnumFilenames.DOMAIN_ADDITIONAL, + options.domain_additional_filename, + DomainAdditionalData, + "domainname", + ), + ( + EnumFilenames.DOMAIN_ESCROW, + options.domain_escrow_filename, + DomainEscrow, + "domainname", + ), + ( + EnumFilenames.DOMAIN_ADHOC, + options.domain_adhoc_filename, + DomainTypeAdhoc, + "domaintypeid", + ), + ( + EnumFilenames.ORGANIZATION_ADHOC, + options.organization_adhoc_filename, + OrganizationAdhoc, + "orgid", + ), + ( + EnumFilenames.AUTHORITY_ADHOC, + options.authority_adhoc_filename, + AuthorityAdhoc, + "authorityid", + ), + ] self.file_data = self.populate_file_data(options.pattern_map_params) diff --git a/src/registrar/management/commands/utility/transition_domain_arguments.py b/src/registrar/management/commands/utility/transition_domain_arguments.py index bd6d8a970..f941d4edb 100644 --- a/src/registrar/management/commands/utility/transition_domain_arguments.py +++ b/src/registrar/management/commands/utility/transition_domain_arguments.py @@ -18,9 +18,6 @@ class TransitionDomainArguments: # Maintains an internal kwargs list and sets values # that match the class definition. def __init__(self, **kwargs): - self.pattern_map_params = [] - if "self.pattern_map_params" in kwargs: - self.pattern_map_params = kwargs["pattern_map_params"] self.kwargs = kwargs for k, v in kwargs.items(): if hasattr(self, k): From 8ef2a8ec026bb424315c648d88fb157725789d54 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 21 Nov 2023 10:28:14 -0700 Subject: [PATCH 29/46] Code clarity refactor --- .../commands/load_organization_data.py | 210 ++++++++++-------- .../utility/extra_transition_domain_helper.py | 77 +++---- .../utility/transition_domain_arguments.py | 2 +- .../test_transition_domain_migrations.py | 11 - 4 files changed, 152 insertions(+), 148 deletions(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index 71be71b39..0b6fe1f19 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -19,7 +19,22 @@ logger = logging.getLogger(__name__) class Command(BaseCommand): - help = "Send domain invitations once to existing customers." + help = "Load organization data on TransitionDomain and DomainInformation objects" + + def __init__(self): + super().__init__() + self.domain_information_to_update: List[DomainInformation] = [] + + # Stores the domain_name for logging purposes + self.domains_failed_to_update: List[str] = [] + self.domains_skipped: List[str] = [] + + self.changed_fields = [ + "address_line1", + "city", + "state_territory", + "zipcode", + ] def add_arguments(self, parser): """Add command line arguments.""" @@ -49,18 +64,12 @@ class Command(BaseCommand): def handle(self, migration_json_filename, **options): """Process the objects in TransitionDomain.""" - - # === Parse JSON file === # + # Parse JSON file options = self.load_json_settings(options, migration_json_filename) args = TransitionDomainArguments(**options) - changed_fields = [ - "address_line", - "city", - "state_territory", - "zipcode", - ] - proceed = TerminalHelper.prompt_for_execution( + # Will sys.exit() when prompt is "n" + TerminalHelper.prompt_for_execution( system_exit_on_terminate=True, info_to_inspect=f""" ==Master data file== @@ -71,22 +80,16 @@ class Command(BaseCommand): ==Containing directory== directory: {args.directory} - - ==Proposed Changes== - For each TransitionDomain, modify the following fields: {changed_fields} """, prompt_title="Do you wish to load organization data for TransitionDomains?", ) - if not proceed: - return None - - logger.info(f"{TerminalColors.MAGENTA}" "Loading organization data onto TransitionDomain tables...") load = OrganizationDataLoader(args) transition_domains = load.update_organization_data_for_all() # Reprompt the user to reinspect before updating DomainInformation - proceed = TerminalHelper.prompt_for_execution( + # Will sys.exit() when prompt is "n" + TerminalHelper.prompt_for_execution( system_exit_on_terminate=True, info_to_inspect=f""" ==Master data file== @@ -100,23 +103,20 @@ class Command(BaseCommand): ==Proposed Changes== Number of DomainInformation objects to (potentially) change: {len(transition_domains)} + For each DomainInformation, modify the following fields: {self.changed_fields} """, prompt_title="Do you wish to load organization data for DomainInformation?", ) - if not proceed: - return None - - if len(transition_domains) == 0: - logger.error(f"{TerminalColors.MAGENTA}" "No TransitionDomain objects exist" f"{TerminalColors.ENDC}") - return None - logger.info( f"{TerminalColors.MAGENTA}" "Preparing to load organization data onto DomainInformation tables..." f"{TerminalColors.ENDC}" ) - self.update_domain_information(transition_domains, args.debug) + self.prepare_update_domain_information(transition_domains, args.debug) + + logger.info(f"{TerminalColors.MAGENTA}" "Beginning mass DomainInformation update..." f"{TerminalColors.ENDC}") + self.bulk_update_domain_information(args.debug) def load_json_settings(self, options, migration_json_filename): """Parses options from the given JSON file.""" @@ -148,19 +148,19 @@ class Command(BaseCommand): return options - def update_domain_information(self, desired_objects: List[TransitionDomain], debug): - di_to_update = [] - di_failed_to_update = [] - di_skipped = [] + def prepare_update_domain_information(self, target_transition_domains: List[TransitionDomain], debug): + """Returns an array of DomainInformation objects with updated organization data.""" + if len(target_transition_domains) == 0: + raise LoadOrganizationError(code=LoadOrganizationErrorCodes.EMPTY_TRANSITION_DOMAIN_TABLE) # Grab each TransitionDomain we want to change. transition_domains = TransitionDomain.objects.filter( - username__in=[item.username for item in desired_objects], - domain_name__in=[item.domain_name for item in desired_objects], + username__in=[item.username for item in target_transition_domains], + domain_name__in=[item.domain_name for item in target_transition_domains], ) # This indicates that some form of data corruption happened. - if len(desired_objects) != len(transition_domains): + if len(target_transition_domains) != len(transition_domains): raise LoadOrganizationError(code=LoadOrganizationErrorCodes.TRANSITION_DOMAINS_NOT_FOUND) # Start with all DomainInformation objects @@ -178,35 +178,84 @@ class Command(BaseCommand): state_territory__isnull=True, zipcode__isnull=True, ) - filtered_domain_informations_dict = {di.domain.name: di for di in domain_informations if di.domain is not None} + + # === Create DomainInformation objects === # for item in transition_domains: - if item.domain_name not in domain_informations_dict: - logger.error(f"Could not add {item.domain_name}. Domain does not exist.") - di_failed_to_update.append(item) - continue + self.map_transition_domain_to_domain_information( + item, domain_informations_dict, filtered_domain_informations_dict, debug + ) - if item.address_line is None and item.city is None and item.state_territory and item.zipcode is None: - logger.info( - f"{TerminalColors.YELLOW}" - f"Domain {item.domain_name} has no Organization Data. Cannot update." - f"{TerminalColors.ENDC}" - ) - di_skipped.append(item) - continue + # === Log results and return data === # + if len(self.domains_failed_to_update) > 0: + failed = [item.domain_name for item in self.domains_failed_to_update] + logger.error( + f"""{TerminalColors.FAIL} + Failed to update. An exception was encountered on the following Domains: {failed} + {TerminalColors.ENDC}""" + ) + raise LoadOrganizationError(code=LoadOrganizationErrorCodes.UPDATE_DOMAIN_INFO_FAILED) - if item.domain_name not in filtered_domain_informations_dict: - logger.info( - f"{TerminalColors.YELLOW}" - f"Domain {item.domain_name} was updated by a user. Cannot update." - f"{TerminalColors.ENDC}" - ) - di_skipped.append(item) - continue + if debug: + logger.info(f"Updating these DomainInformations: {[item for item in self.domain_information_to_update]}") + if len(self.domains_skipped) > 0: + logger.info( + f"Skipped updating {len(self.domains_skipped)} fields. User-supplied data exists, or there is no data." + ) + + logger.info(f"Ready to update {len(self.domain_information_to_update)} DomainInformations.") + + return self.domain_information_to_update + + def bulk_update_domain_information(self, debug): + """Performs a bulk_update operation on a list of DomainInformation objects""" + # 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. + batch_size = 1000 + paginator = Paginator(self.domain_information_to_update, batch_size) + for page_num in paginator.page_range: + page = paginator.page(page_num) + DomainInformation.objects.bulk_update(page.object_list, self.changed_fields) + + if debug: + logger.info(f"Updated these DomainInformations: {[item for item in self.domain_information_to_update]}") + + logger.info( + f"{TerminalColors.OKGREEN}" + f"Updated {len(self.domain_information_to_update)} DomainInformations." + f"{TerminalColors.ENDC}" + ) + + def map_transition_domain_to_domain_information( + self, item, domain_informations_dict, filtered_domain_informations_dict, debug + ): + """Attempts to return a DomainInformation object based on values from TransitionDomain. + Any domains which cannot be updated will be stored in an array. + """ + does_not_exist: bool = self.is_domain_name_missing(item, domain_informations_dict) + all_fields_are_none: bool = self.is_organization_data_missing(item) + user_updated_field: bool = self.is_domain_name_missing(item, filtered_domain_informations_dict) + if does_not_exist: + logger.error(f"Could not add {item.domain_name}. Domain does not exist.") + self.domains_failed_to_update.append(item) + elif all_fields_are_none: + logger.info( + f"{TerminalColors.YELLOW}" + f"Domain {item.domain_name} has no Organization Data. Cannot update." + f"{TerminalColors.ENDC}" + ) + self.domains_skipped.append(item) + elif user_updated_field: + logger.info( + f"{TerminalColors.YELLOW}" + f"Domain {item.domain_name} was updated by a user. Cannot update." + f"{TerminalColors.ENDC}" + ) + self.domains_skipped.append(item) + else: # Based on the current domain, grab the right DomainInformation object. current_domain_information = filtered_domain_informations_dict[item.domain_name] - if current_domain_information.domain is None or current_domain_information.domain.name is None: raise LoadOrganizationError(code=LoadOrganizationErrorCodes.DOMAIN_NAME_WAS_NONE) @@ -215,51 +264,16 @@ class Command(BaseCommand): current_domain_information.city = item.city current_domain_information.state_territory = item.state_territory current_domain_information.zipcode = item.zipcode + self.domain_information_to_update.append(current_domain_information) - di_to_update.append(current_domain_information) if debug: logger.info(f"Updated {current_domain_information.domain.name}...") - if di_failed_to_update: - failed = [item.domain_name for item in di_failed_to_update] - logger.error( - f"""{TerminalColors.FAIL} - Failed to update. An exception was encountered on the following DomainInformations: {failed} - {TerminalColors.ENDC}""" - ) - raise LoadOrganizationError(code=LoadOrganizationErrorCodes.UPDATE_DOMAIN_INFO_FAILED) + def is_domain_name_missing(self, item: TransitionDomain, domain_informations_dict): + """Checks if domain_name is in the supplied dictionary""" + return item.domain_name not in domain_informations_dict - if di_skipped: - logger.info(f"Skipped updating {len(di_skipped)} fields. User-supplied data exists, or there is no data.") - - self.bulk_update_domain_information(di_to_update, debug) - - def bulk_update_domain_information(self, di_to_update, debug): - if debug: - logger.info(f"Updating these DomainInformations: {[item for item in di_to_update]}") - - logger.info(f"Ready to update {len(di_to_update)} DomainInformations.") - - logger.info(f"{TerminalColors.MAGENTA}" "Beginning mass DomainInformation update..." f"{TerminalColors.ENDC}") - - changed_fields = [ - "address_line1", - "city", - "state_territory", - "zipcode", - ] - - batch_size = 1000 - # 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(di_to_update, batch_size) - for page_num in paginator.page_range: - page = paginator.page(page_num) - DomainInformation.objects.bulk_update(page.object_list, changed_fields) - - if debug: - logger.info(f"Updated these DomainInformations: {[item for item in di_to_update]}") - - logger.info( - f"{TerminalColors.OKGREEN}" f"Updated {len(di_to_update)} DomainInformations." f"{TerminalColors.ENDC}" - ) + def is_organization_data_missing(self, item: TransitionDomain): + """Checks if all desired Organization fields to update are none""" + fields = [item.address_line, item.city, item.state_territory, item.zipcode] + return all(field is None for field in fields) diff --git a/src/registrar/management/commands/utility/extra_transition_domain_helper.py b/src/registrar/management/commands/utility/extra_transition_domain_helper.py index ed90196ca..781972077 100644 --- a/src/registrar/management/commands/utility/extra_transition_domain_helper.py +++ b/src/registrar/management/commands/utility/extra_transition_domain_helper.py @@ -963,44 +963,45 @@ class ExtraTransitionDomain: # metadata about each file and associate it with an enum. # That way if we want the data located at the agency_adhoc file, # we can just call EnumFilenames.AGENCY_ADHOC. - options.pattern_map_params = [ - ( - EnumFilenames.AGENCY_ADHOC, - options.agency_adhoc_filename, - AgencyAdhoc, - "agencyid", - ), - ( - EnumFilenames.DOMAIN_ADDITIONAL, - options.domain_additional_filename, - DomainAdditionalData, - "domainname", - ), - ( - EnumFilenames.DOMAIN_ESCROW, - options.domain_escrow_filename, - DomainEscrow, - "domainname", - ), - ( - EnumFilenames.DOMAIN_ADHOC, - options.domain_adhoc_filename, - DomainTypeAdhoc, - "domaintypeid", - ), - ( - EnumFilenames.ORGANIZATION_ADHOC, - options.organization_adhoc_filename, - OrganizationAdhoc, - "orgid", - ), - ( - EnumFilenames.AUTHORITY_ADHOC, - options.authority_adhoc_filename, - AuthorityAdhoc, - "authorityid", - ), - ] + if options.pattern_map_params is None or options.pattern_map_params == []: + options.pattern_map_params = [ + ( + EnumFilenames.AGENCY_ADHOC, + options.agency_adhoc_filename, + AgencyAdhoc, + "agencyid", + ), + ( + EnumFilenames.DOMAIN_ADDITIONAL, + options.domain_additional_filename, + DomainAdditionalData, + "domainname", + ), + ( + EnumFilenames.DOMAIN_ESCROW, + options.domain_escrow_filename, + DomainEscrow, + "domainname", + ), + ( + EnumFilenames.DOMAIN_ADHOC, + options.domain_adhoc_filename, + DomainTypeAdhoc, + "domaintypeid", + ), + ( + EnumFilenames.ORGANIZATION_ADHOC, + options.organization_adhoc_filename, + OrganizationAdhoc, + "orgid", + ), + ( + EnumFilenames.AUTHORITY_ADHOC, + options.authority_adhoc_filename, + AuthorityAdhoc, + "authorityid", + ), + ] self.file_data = self.populate_file_data(options.pattern_map_params) diff --git a/src/registrar/management/commands/utility/transition_domain_arguments.py b/src/registrar/management/commands/utility/transition_domain_arguments.py index f941d4edb..1a57d0d2d 100644 --- a/src/registrar/management/commands/utility/transition_domain_arguments.py +++ b/src/registrar/management/commands/utility/transition_domain_arguments.py @@ -18,7 +18,7 @@ class TransitionDomainArguments: # Maintains an internal kwargs list and sets values # that match the class definition. def __init__(self, **kwargs): - self.kwargs = kwargs + self.pattern_map_params = kwargs.get("pattern_map_params", []) for k, v in kwargs.items(): if hasattr(self, k): setattr(self, k, v) diff --git a/src/registrar/tests/test_transition_domain_migrations.py b/src/registrar/tests/test_transition_domain_migrations.py index 3b6a04a89..f2107a941 100644 --- a/src/registrar/tests/test_transition_domain_migrations.py +++ b/src/registrar/tests/test_transition_domain_migrations.py @@ -27,17 +27,6 @@ class TestOrganizationMigration(TestCase): # self.master_script = "load_transition_domain", self.test_data_file_location = "registrar/tests/data" - self.test_domain_contact_filename = "test_domain_contacts.txt" - self.test_contact_filename = "test_contacts.txt" - self.test_domain_status_filename = "test_domain_statuses.txt" - - # Files for parsing additional TransitionDomain data - self.test_agency_adhoc_filename = "test_agency_adhoc.txt" - self.test_authority_adhoc_filename = "test_authority_adhoc.txt" - self.test_domain_additional = "test_domain_additional.txt" - self.test_domain_types_adhoc = "test_domain_types_adhoc.txt" - self.test_escrow_domains_daily = "test_escrow_domains_daily" - self.test_organization_adhoc = "test_organization_adhoc.txt" self.migration_json_filename = "test_migrationFilepaths.json" def tearDown(self): From bd89eea21f142b817e7bb00a5d8230871c1818d8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 21 Nov 2023 10:52:59 -0700 Subject: [PATCH 30/46] Minor refactor --- .../commands/load_organization_data.py | 58 +++++++++++-------- 1 file changed, 34 insertions(+), 24 deletions(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index 0b6fe1f19..27daf468b 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -12,6 +12,7 @@ from registrar.management.commands.utility.transition_domain_arguments import Tr from registrar.models import TransitionDomain, DomainInformation from django.core.paginator import Paginator from typing import List +from registrar.models.domain import Domain from registrar.utility.errors import LoadOrganizationError, LoadOrganizationErrorCodes @@ -163,15 +164,10 @@ class Command(BaseCommand): if len(target_transition_domains) != len(transition_domains): raise LoadOrganizationError(code=LoadOrganizationErrorCodes.TRANSITION_DOMAINS_NOT_FOUND) - # Start with all DomainInformation objects - domain_informations = DomainInformation.objects.all() - domain_informations_dict = {di.domain.name: di for di in domain_informations if di.domain is not None} - - # Then, use each domain object to map TransitionDomain <--> DomainInformation - # Fetches all DomainInformations in one query. + # Maps TransitionDomain <--> DomainInformation. # If any related organization fields have been updated, # we can assume that they modified this information themselves - thus we should not update it. - domain_informations = domain_informations.filter( + domain_informations = DomainInformation.objects.filter( domain__name__in=[td.domain_name for td in transition_domains], address_line1__isnull=True, city__isnull=True, @@ -182,9 +178,7 @@ class Command(BaseCommand): # === Create DomainInformation objects === # for item in transition_domains: - self.map_transition_domain_to_domain_information( - item, domain_informations_dict, filtered_domain_informations_dict, debug - ) + self.map_transition_domain_to_domain_information(item, filtered_domain_informations_dict, debug) # === Log results and return data === # if len(self.domains_failed_to_update) > 0: @@ -227,18 +221,14 @@ class Command(BaseCommand): f"{TerminalColors.ENDC}" ) - def map_transition_domain_to_domain_information( - self, item, domain_informations_dict, filtered_domain_informations_dict, debug - ): + def map_transition_domain_to_domain_information(self, item, domain_informations_dict, debug): """Attempts to return a DomainInformation object based on values from TransitionDomain. Any domains which cannot be updated will be stored in an array. """ does_not_exist: bool = self.is_domain_name_missing(item, domain_informations_dict) all_fields_are_none: bool = self.is_organization_data_missing(item) - user_updated_field: bool = self.is_domain_name_missing(item, filtered_domain_informations_dict) if does_not_exist: - logger.error(f"Could not add {item.domain_name}. Domain does not exist.") - self.domains_failed_to_update.append(item) + self.handle_if_domain_name_missing(item.domain_name) elif all_fields_are_none: logger.info( f"{TerminalColors.YELLOW}" @@ -246,16 +236,9 @@ class Command(BaseCommand): f"{TerminalColors.ENDC}" ) self.domains_skipped.append(item) - elif user_updated_field: - logger.info( - f"{TerminalColors.YELLOW}" - f"Domain {item.domain_name} was updated by a user. Cannot update." - f"{TerminalColors.ENDC}" - ) - self.domains_skipped.append(item) else: # Based on the current domain, grab the right DomainInformation object. - current_domain_information = filtered_domain_informations_dict[item.domain_name] + current_domain_information = domain_informations_dict[item.domain_name] if current_domain_information.domain is None or current_domain_information.domain.name is None: raise LoadOrganizationError(code=LoadOrganizationErrorCodes.DOMAIN_NAME_WAS_NONE) @@ -277,3 +260,30 @@ class Command(BaseCommand): """Checks if all desired Organization fields to update are none""" fields = [item.address_line, item.city, item.state_territory, item.zipcode] return all(field is None for field in fields) + + def handle_if_domain_name_missing(self, domain_name): + """ + Infers what to log if we can't find a domain_name and updates the relevant lists. + + This function performs the following checks: + 1. If the domain does not exist, it logs an error and adds the domain name to the `domains_failed_to_update` list. + 2. If the domain was updated by a user, it logs an info message and adds the domain name to the `domains_skipped` list. + 3. If there are duplicate domains, it logs an error and adds the domain name to the `domains_failed_to_update` list. + + Args: + domain_name (str): The name of the domain to check. + """ + domains = Domain.objects.filter(name=domain_name) + if domains.count() == 0: + logger.error(f"Could not add {domain_name}. Domain does not exist.") + self.domains_failed_to_update.append(domain_name) + elif domains.count() == 1: + logger.info( + f"{TerminalColors.YELLOW}" + f"Domain {domain_name} was updated by a user. Cannot update." + f"{TerminalColors.ENDC}" + ) + self.domains_skipped.append(domain_name) + else: + logger.error(f"Could not add {domain_name}. Duplicate domains exist.") + self.domains_failed_to_update.append(domain_name) From 79112522beb48212b011874acae1cd69b3458284 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 21 Nov 2023 10:55:18 -0700 Subject: [PATCH 31/46] Update load_organization_data.py --- .../management/commands/load_organization_data.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index 27daf468b..3a4bdc664 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -110,13 +110,17 @@ class Command(BaseCommand): ) logger.info( - f"{TerminalColors.MAGENTA}" - "Preparing to load organization data onto DomainInformation tables..." - f"{TerminalColors.ENDC}" + f"""{TerminalColors.MAGENTA} + Preparing to load organization data onto DomainInformation tables... + {TerminalColors.ENDC}""" ) self.prepare_update_domain_information(transition_domains, args.debug) - logger.info(f"{TerminalColors.MAGENTA}" "Beginning mass DomainInformation update..." f"{TerminalColors.ENDC}") + logger.info( + f"""{TerminalColors.MAGENTA} + Beginning mass DomainInformation update... + {TerminalColors.ENDC}""" + ) self.bulk_update_domain_information(args.debug) def load_json_settings(self, options, migration_json_filename): From 0d47a02bfb5e2cd59655a7d54beac288c1e36a7d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 21 Nov 2023 11:01:23 -0700 Subject: [PATCH 32/46] Undo domain invitation bug fix Not needed in this PR --- .../management/commands/send_domain_invitations.py | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/registrar/management/commands/send_domain_invitations.py b/src/registrar/management/commands/send_domain_invitations.py index 0f8ca1c46..603fbce3a 100644 --- a/src/registrar/management/commands/send_domain_invitations.py +++ b/src/registrar/management/commands/send_domain_invitations.py @@ -152,12 +152,6 @@ class Command(BaseCommand): for domain_name in email_data["domains"]: # self.transition_domains is a queryset so we can sub-select # from it and use the objects to mark them as sent - transition_domains = self.transition_domains.filter(username=this_email, domain_name=domain_name) - if len(transition_domains) == 1: - this_transition_domain = transition_domains.get() - this_transition_domain.email_sent = True - this_transition_domain.save() - elif len(transition_domains) > 1: - logger.error(f"Multiple TransitionDomains exist for {this_email}") - else: - logger.error(f"No TransitionDomain exists for {this_email}") + this_transition_domain = self.transition_domains.get(username=this_email, domain_name=domain_name) + this_transition_domain.email_sent = True + this_transition_domain.save() From fc8847a36fe9e1e88a002088e7af7de0eb305428 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 21 Nov 2023 13:09:41 -0700 Subject: [PATCH 33/46] PR suggestions - move errors / test cases --- .../commands/load_organization_data.py | 7 +- .../utility/extra_transition_domain_helper.py | 5 +- .../utility/load_organization_error.py | 39 +++++ .../test_transition_domain_migrations.py | 161 +++++++++++++++--- src/registrar/utility/errors.py | 38 ----- 5 files changed, 184 insertions(+), 66 deletions(-) create mode 100644 src/registrar/management/commands/utility/load_organization_error.py diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index 3a4bdc664..4726bcab1 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -14,7 +14,10 @@ from django.core.paginator import Paginator from typing import List from registrar.models.domain import Domain -from registrar.utility.errors import LoadOrganizationError, LoadOrganizationErrorCodes +from registrar.management.commands.utility.load_organization_error import ( + LoadOrganizationError, + LoadOrganizationErrorCodes, +) logger = logging.getLogger(__name__) @@ -276,7 +279,7 @@ class Command(BaseCommand): Args: domain_name (str): The name of the domain to check. - """ + """ # noqa - E501 (harder to read) domains = Domain.objects.filter(name=domain_name) if domains.count() == 0: logger.error(f"Could not add {domain_name}. Domain does not exist.") diff --git a/src/registrar/management/commands/utility/extra_transition_domain_helper.py b/src/registrar/management/commands/utility/extra_transition_domain_helper.py index 781972077..75cde995a 100644 --- a/src/registrar/management/commands/utility/extra_transition_domain_helper.py +++ b/src/registrar/management/commands/utility/extra_transition_domain_helper.py @@ -12,7 +12,10 @@ import sys from typing import Dict, List from django.core.paginator import Paginator from registrar.models.transition_domain import TransitionDomain -from registrar.utility.errors import LoadOrganizationError, LoadOrganizationErrorCodes +from registrar.management.commands.utility.load_organization_error import ( + LoadOrganizationError, + LoadOrganizationErrorCodes, +) from .epp_data_containers import ( AgencyAdhoc, diff --git a/src/registrar/management/commands/utility/load_organization_error.py b/src/registrar/management/commands/utility/load_organization_error.py new file mode 100644 index 000000000..1849558c4 --- /dev/null +++ b/src/registrar/management/commands/utility/load_organization_error.py @@ -0,0 +1,39 @@ +from enum import IntEnum + + +class LoadOrganizationErrorCodes(IntEnum): + """Used when running the load_organization_data script + Overview of error codes: + - 1 TRANSITION_DOMAINS_NOT_FOUND + - 2 UPDATE_DOMAIN_INFO_FAILED + - 3 EMPTY_TRANSITION_DOMAIN_TABLE + """ + + TRANSITION_DOMAINS_NOT_FOUND = 1 + UPDATE_DOMAIN_INFO_FAILED = 2 + EMPTY_TRANSITION_DOMAIN_TABLE = 3 + DOMAIN_NAME_WAS_NONE = 4 + + +class LoadOrganizationError(Exception): + """ + Error class used in the load_organization_data script + """ + + _error_mapping = { + LoadOrganizationErrorCodes.TRANSITION_DOMAINS_NOT_FOUND: ( + "Could not find all desired TransitionDomains. " "(Possible data corruption?)" + ), + LoadOrganizationErrorCodes.UPDATE_DOMAIN_INFO_FAILED: "Failed to update DomainInformation", + LoadOrganizationErrorCodes.EMPTY_TRANSITION_DOMAIN_TABLE: "No TransitionDomains exist. Cannot update.", + LoadOrganizationErrorCodes.DOMAIN_NAME_WAS_NONE: "DomainInformation was updated, but domain was None", + } + + def __init__(self, *args, code=None, **kwargs): + super().__init__(*args, **kwargs) + self.code = code + if self.code in self._error_mapping: + self.message = self._error_mapping.get(self.code) + + def __str__(self): + return f"{self.message}" diff --git a/src/registrar/tests/test_transition_domain_migrations.py b/src/registrar/tests/test_transition_domain_migrations.py index f2107a941..1f21115ff 100644 --- a/src/registrar/tests/test_transition_domain_migrations.py +++ b/src/registrar/tests/test_transition_domain_migrations.py @@ -16,20 +16,19 @@ from registrar.models import ( from django.core.management import call_command from unittest.mock import patch +from registrar.models.contact import Contact + from .common import less_console_noise class TestOrganizationMigration(TestCase): def setUp(self): - """ """ - # self.load_transition_domain_script = "load_transition_domain", - # self.transfer_script = "transfer_transition_domains_to_domains", - # self.master_script = "load_transition_domain", - + """Defines the file name of migration_json and the folder its contained in""" self.test_data_file_location = "registrar/tests/data" self.migration_json_filename = "test_migrationFilepaths.json" def tearDown(self): + """Deletes all DB objects related to migrations""" # Delete domain information Domain.objects.all().delete() DomainInformation.objects.all().delete() @@ -41,6 +40,16 @@ class TestOrganizationMigration(TestCase): UserDomainRole.objects.all().delete() def run_load_domains(self): + """ + This method executes the load_transition_domain command. + + It uses 'unittest.mock.patch' to mock the TerminalHelper.query_yes_no_exit method, + which is a user prompt in the terminal. The mock function always returns True, + allowing the test to proceed without manual user input. + + The 'call_command' function from Django's management framework is then used to + execute the load_transition_domain command with the specified arguments. + """ # noqa here because splitting this up makes it confusing. # ES501 with patch( @@ -54,9 +63,25 @@ class TestOrganizationMigration(TestCase): ) def run_transfer_domains(self): + """ + This method executes the transfer_transition_domains_to_domains command. + + The 'call_command' function from Django's management framework is then used to + execute the load_transition_domain command with the specified arguments. + """ call_command("transfer_transition_domains_to_domains") def run_load_organization_data(self): + """ + This method executes the load_organization_data command. + + It uses 'unittest.mock.patch' to mock the TerminalHelper.query_yes_no_exit method, + which is a user prompt in the terminal. The mock function always returns True, + allowing the test to proceed without manual user input. + + The 'call_command' function from Django's management framework is then used to + execute the load_organization_data command with the specified arguments. + """ # noqa here (E501) because splitting this up makes it # confusing to read. with patch( @@ -88,7 +113,7 @@ class TestOrganizationMigration(TestCase): duplicate_domains = [] missing_domain_informations = [] missing_domain_invites = [] - for transition_domain in TransitionDomain.objects.all(): # DEBUG: + for transition_domain in TransitionDomain.objects.all(): transition_domain_name = transition_domain.domain_name transition_domain_email = transition_domain.username @@ -121,19 +146,6 @@ class TestOrganizationMigration(TestCase): total_domain_informations = len(DomainInformation.objects.all()) total_domain_invitations = len(DomainInvitation.objects.all()) - print( - f""" - total_missing_domains = {len(missing_domains)} - total_duplicate_domains = {len(duplicate_domains)} - total_missing_domain_informations = {len(missing_domain_informations)} - total_missing_domain_invitations = {total_missing_domain_invitations} - - total_transition_domains = {len(TransitionDomain.objects.all())} - total_domains = {len(Domain.objects.all())} - total_domain_informations = {len(DomainInformation.objects.all())} - total_domain_invitations = {len(DomainInvitation.objects.all())} - """ - ) self.assertEqual(total_missing_domains, expected_missing_domains) self.assertEqual(total_duplicate_domains, expected_duplicate_domains) self.assertEqual(total_missing_domain_informations, expected_missing_domain_informations) @@ -145,6 +157,17 @@ class TestOrganizationMigration(TestCase): self.assertEqual(total_domain_invitations, expected_total_domain_invitations) def test_load_organization_data_transition_domain(self): + """ + This test verifies the functionality of the load_organization_data method for TransitionDomain objects. + + The test follows these steps: + 1. Parses all existing data by running the load_domains and transfer_domains methods. + 2. Attempts to add organization data to the parsed data by running the load_organization_data method. + 3. Checks that the data has been loaded as expected. + + The expected result is a set of TransitionDomain objects with specific attributes. + The test fetches the actual TransitionDomain objects from the database and compares them with the expected objects. + """ # noqa - E501 (harder to read) # == First, parse all existing data == # self.run_load_domains() self.run_transfer_domains() @@ -187,6 +210,18 @@ class TestOrganizationMigration(TestCase): self.assertEqual(transition, expected_transition_domain) def test_load_organization_data_domain_information(self): + """ + This test verifies the functionality of the load_organization_data method. + + The test follows these steps: + 1. Parses all existing data by running the load_domains and transfer_domains methods. + 2. Attempts to add organization data to the parsed data by running the load_organization_data method. + 3. Checks that the data has been loaded as expected. + + The expected result is a DomainInformation object with specific attributes. + The test fetches the actual DomainInformation object from the database + and compares it with the expected object. + """ # == First, parse all existing data == # self.run_load_domains() self.run_transfer_domains() @@ -198,13 +233,90 @@ class TestOrganizationMigration(TestCase): _domain = Domain.objects.filter(name="fakewebsite2.gov").get() domain_information = DomainInformation.objects.filter(domain=_domain).get() - self.assertEqual(domain_information.address_line1, "93001 Arizona Drive") - self.assertEqual(domain_information.city, "Columbus") - self.assertEqual(domain_information.state_territory, "Oh") - self.assertEqual(domain_information.zipcode, "43268") + expected_creator = User.objects.filter(username="System").get() + expected_ao = Contact.objects.filter(first_name="Seline", middle_name="testmiddle2", last_name="Tower").get() + expected_domain_information = DomainInformation( + creator=expected_creator, + organization_type="federal", + federal_agency="Department of Commerce", + federal_type="executive", + organization_name="Fanoodle", + address_line1="93001 Arizona Drive", + city="Columbus", + state_territory="Oh", + zipcode="43268", + authorizing_official=expected_ao, + domain=_domain, + ) + # Given that these are different objects, this needs to be set + expected_domain_information.id = domain_information.id + self.assertEqual(domain_information, expected_domain_information) + + def test_load_organization_data_preserves_existing_data(self): + """ + This test verifies that the load_organization_data method does not overwrite existing data. + + The test follows these steps: + 1. Parses all existing data by running the load_domains and transfer_domains methods. + 2. Adds pre-existing fake data to a DomainInformation object and saves it to the database. + 3. Runs the load_organization_data method. + 4. Checks that the pre-existing data in the DomainInformation object has not been overwritten. + + The expected result is that the DomainInformation object retains its pre-existing data + after the load_organization_data method is run. + """ + # == First, parse all existing data == # + self.run_load_domains() + self.run_transfer_domains() + + # == Second, try add prexisting fake data == # + _domain_old = Domain.objects.filter(name="fakewebsite2.gov").get() + domain_information_old = DomainInformation.objects.filter(domain=_domain_old).get() + domain_information_old.address_line1 = "93001 Galactic Way" + domain_information_old.city = "Olympus" + domain_information_old.state_territory = "MA" + domain_information_old.zipcode = "12345" + domain_information_old.save() + + # == Third, try running the script == # + self.run_load_organization_data() + + # == Fourth, test that no data is overwritten as we expect == # + _domain = Domain.objects.filter(name="fakewebsite2.gov").get() + domain_information = DomainInformation.objects.filter(domain=_domain).get() + + expected_creator = User.objects.filter(username="System").get() + expected_ao = Contact.objects.filter(first_name="Seline", middle_name="testmiddle2", last_name="Tower").get() + expected_domain_information = DomainInformation( + creator=expected_creator, + organization_type="federal", + federal_agency="Department of Commerce", + federal_type="executive", + organization_name="Fanoodle", + address_line1="93001 Galactic Way", + city="Olympus", + state_territory="MA", + zipcode="12345", + authorizing_official=expected_ao, + domain=_domain, + ) + # Given that these are different objects, this needs to be set + expected_domain_information.id = domain_information.id + self.assertEqual(domain_information, expected_domain_information) def test_load_organization_data_integrity(self): - """Validates data integrity with the load_org_data command""" + """ + This test verifies the data integrity after running the load_organization_data method. + + The test follows these steps: + 1. Parses all existing data by running the load_domains and transfer_domains methods. + 2. Attempts to add organization data to the parsed data by running the load_organization_data method. + 3. Checks that the data has not been corrupted by comparing the actual counts of objects in the database + with the expected counts. + + The expected result is that the counts of objects in the database + match the expected counts, indicating that the data has not been corrupted. + """ # First, parse all existing data self.run_load_domains() self.run_transfer_domains() @@ -221,7 +333,6 @@ class TestOrganizationMigration(TestCase): expected_missing_domains = 0 expected_duplicate_domains = 0 expected_missing_domain_informations = 0 - # we expect 1 missing invite from anomaly.gov (an injected error) expected_missing_domain_invitations = 1 self.compare_tables( expected_total_transition_domains, diff --git a/src/registrar/utility/errors.py b/src/registrar/utility/errors.py index 4e0745a2d..4ca3a9a12 100644 --- a/src/registrar/utility/errors.py +++ b/src/registrar/utility/errors.py @@ -57,44 +57,6 @@ contact help@get.gov return f"{self.message}" -class LoadOrganizationErrorCodes(IntEnum): - """Used when running the load_organization_data script - Overview of error codes: - - 1 TRANSITION_DOMAINS_NOT_FOUND - - 2 UPDATE_DOMAIN_INFO_FAILED - - 3 EMPTY_TRANSITION_DOMAIN_TABLE - """ - - TRANSITION_DOMAINS_NOT_FOUND = 1 - UPDATE_DOMAIN_INFO_FAILED = 2 - EMPTY_TRANSITION_DOMAIN_TABLE = 3 - DOMAIN_NAME_WAS_NONE = 4 - - -class LoadOrganizationError(Exception): - """ - Error class used in the load_organization_data script - """ - - _error_mapping = { - LoadOrganizationErrorCodes.TRANSITION_DOMAINS_NOT_FOUND: ( - "Could not find all desired TransitionDomains. " "(Possible data corruption?)" - ), - LoadOrganizationErrorCodes.UPDATE_DOMAIN_INFO_FAILED: "Failed to update DomainInformation", - LoadOrganizationErrorCodes.EMPTY_TRANSITION_DOMAIN_TABLE: "No TransitionDomains exist. Cannot update.", - LoadOrganizationErrorCodes.DOMAIN_NAME_WAS_NONE: "DomainInformation was updated, but domain was None", - } - - def __init__(self, *args, code=None, **kwargs): - super().__init__(*args, **kwargs) - self.code = code - if self.code in self._error_mapping: - self.message = self._error_mapping.get(self.code) - - def __str__(self): - return f"{self.message}" - - class NameserverErrorCodes(IntEnum): """Used in the NameserverError class for error mapping. From 9c92c998685f30d31af5714509d81b475f31c755 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 21 Nov 2023 13:13:20 -0700 Subject: [PATCH 34/46] Removed unused domain invite test --- .../test_transition_domain_migrations.py | 20 ------------------- 1 file changed, 20 deletions(-) diff --git a/src/registrar/tests/test_transition_domain_migrations.py b/src/registrar/tests/test_transition_domain_migrations.py index 1f21115ff..7b04897f3 100644 --- a/src/registrar/tests/test_transition_domain_migrations.py +++ b/src/registrar/tests/test_transition_domain_migrations.py @@ -99,11 +99,9 @@ class TestOrganizationMigration(TestCase): expected_total_transition_domains, expected_total_domains, expected_total_domain_informations, - expected_total_domain_invitations, expected_missing_domains, expected_duplicate_domains, expected_missing_domain_informations, - expected_missing_domain_invitations, ): """Does a diff between the transition_domain and the following tables: domain, domain_information and the domain_invitation. @@ -112,20 +110,12 @@ class TestOrganizationMigration(TestCase): missing_domains = [] duplicate_domains = [] missing_domain_informations = [] - missing_domain_invites = [] for transition_domain in TransitionDomain.objects.all(): transition_domain_name = transition_domain.domain_name - transition_domain_email = transition_domain.username - # Check Domain table matching_domains = Domain.objects.filter(name=transition_domain_name) # Check Domain Information table matching_domain_informations = DomainInformation.objects.filter(domain__name=transition_domain_name) - # Check Domain Invitation table - matching_domain_invitations = DomainInvitation.objects.filter( - email=transition_domain_email.lower(), - domain__name=transition_domain_name, - ) if len(matching_domains) == 0: missing_domains.append(transition_domain_name) @@ -133,28 +123,22 @@ class TestOrganizationMigration(TestCase): duplicate_domains.append(transition_domain_name) if len(matching_domain_informations) == 0: missing_domain_informations.append(transition_domain_name) - if len(matching_domain_invitations) == 0: - missing_domain_invites.append(transition_domain_name) total_missing_domains = len(missing_domains) total_duplicate_domains = len(duplicate_domains) total_missing_domain_informations = len(missing_domain_informations) - total_missing_domain_invitations = len(missing_domain_invites) total_transition_domains = len(TransitionDomain.objects.all()) total_domains = len(Domain.objects.all()) total_domain_informations = len(DomainInformation.objects.all()) - total_domain_invitations = len(DomainInvitation.objects.all()) self.assertEqual(total_missing_domains, expected_missing_domains) self.assertEqual(total_duplicate_domains, expected_duplicate_domains) self.assertEqual(total_missing_domain_informations, expected_missing_domain_informations) - self.assertEqual(total_missing_domain_invitations, expected_missing_domain_invitations) self.assertEqual(total_transition_domains, expected_total_transition_domains) self.assertEqual(total_domains, expected_total_domains) self.assertEqual(total_domain_informations, expected_total_domain_informations) - self.assertEqual(total_domain_invitations, expected_total_domain_invitations) def test_load_organization_data_transition_domain(self): """ @@ -328,21 +312,17 @@ class TestOrganizationMigration(TestCase): expected_total_transition_domains = 9 expected_total_domains = 5 expected_total_domain_informations = 5 - expected_total_domain_invitations = 8 expected_missing_domains = 0 expected_duplicate_domains = 0 expected_missing_domain_informations = 0 - expected_missing_domain_invitations = 1 self.compare_tables( expected_total_transition_domains, expected_total_domains, expected_total_domain_informations, - expected_total_domain_invitations, expected_missing_domains, expected_duplicate_domains, expected_missing_domain_informations, - expected_missing_domain_invitations, ) From c6d285d73bb092a86da0bc2cb11fd236bb8138be Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 21 Nov 2023 13:14:41 -0700 Subject: [PATCH 35/46] Update test_transition_domain_migrations.py --- .../tests/test_transition_domain_migrations.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/registrar/tests/test_transition_domain_migrations.py b/src/registrar/tests/test_transition_domain_migrations.py index 7b04897f3..91625207d 100644 --- a/src/registrar/tests/test_transition_domain_migrations.py +++ b/src/registrar/tests/test_transition_domain_migrations.py @@ -390,19 +390,6 @@ class TestMigrations(TestCase): disablePrompts=True, ) - def run_load_organization_data(self): - # noqa here (E501) because splitting this up makes it - # confusing to read. - with patch( - "registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit", # noqa - return_value=True, - ): - call_command( - "load_organization_data", - self.migration_json_filename, - directory=self.test_data_file_location, - ) - def compare_tables( self, expected_total_transition_domains, From 598d0f643098d108cb2a7ea82b45b4befd150895 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 21 Nov 2023 13:25:58 -0700 Subject: [PATCH 36/46] Fix logging bug --- .../commands/load_organization_data.py | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index 4726bcab1..87bc62512 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -113,17 +113,13 @@ class Command(BaseCommand): ) logger.info( - f"""{TerminalColors.MAGENTA} - Preparing to load organization data onto DomainInformation tables... - {TerminalColors.ENDC}""" + f"{TerminalColors.MAGENTA}" + "Preparing to load organization data onto DomainInformation tables..." + f"{TerminalColors.ENDC}" ) self.prepare_update_domain_information(transition_domains, args.debug) - logger.info( - f"""{TerminalColors.MAGENTA} - Beginning mass DomainInformation update... - {TerminalColors.ENDC}""" - ) + logger.info(f"{TerminalColors.MAGENTA}" f"Beginning mass DomainInformation update..." f"{TerminalColors.ENDC}") self.bulk_update_domain_information(args.debug) def load_json_settings(self, options, migration_json_filename): @@ -189,10 +185,9 @@ class Command(BaseCommand): # === Log results and return data === # if len(self.domains_failed_to_update) > 0: - failed = [item.domain_name for item in self.domains_failed_to_update] logger.error( f"""{TerminalColors.FAIL} - Failed to update. An exception was encountered on the following Domains: {failed} + Failed to update. An exception was encountered on the following Domains: {self.domains_failed_to_update} {TerminalColors.ENDC}""" ) raise LoadOrganizationError(code=LoadOrganizationErrorCodes.UPDATE_DOMAIN_INFO_FAILED) @@ -201,8 +196,11 @@ class Command(BaseCommand): logger.info(f"Updating these DomainInformations: {[item for item in self.domain_information_to_update]}") if len(self.domains_skipped) > 0: + logger.info(f"Skipped these fields: {self.domains_skipped}") logger.info( + f"{TerminalColors.YELLOW}" f"Skipped updating {len(self.domains_skipped)} fields. User-supplied data exists, or there is no data." + f"{TerminalColors.ENDC}" ) logger.info(f"Ready to update {len(self.domain_information_to_update)} DomainInformations.") @@ -242,7 +240,7 @@ class Command(BaseCommand): f"Domain {item.domain_name} has no Organization Data. Cannot update." f"{TerminalColors.ENDC}" ) - self.domains_skipped.append(item) + self.domains_skipped.append(item.domain_name) else: # Based on the current domain, grab the right DomainInformation object. current_domain_information = domain_informations_dict[item.domain_name] From a3a623fc98cb9f9d4ec872461aa828207936a733 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 22 Nov 2023 08:13:01 -0700 Subject: [PATCH 37/46] Update docs/operations/data_migration.md Co-authored-by: Alysia Broddrick <109625347+abroddrick@users.noreply.github.com> --- docs/operations/data_migration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index fe3d5f45e..aa74d6260 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -423,7 +423,7 @@ Used by the migration scripts to trigger a prompt for deleting all table entries Useful for testing purposes, but *use with caution* ## Import organization data -During MVP, our import scripts did not populate the following fields: `address_line, city, state_territory, and zipcode`. This was primarily due to time constraints. Because of this, we need to run a follow-on script to load this remaining data on each `DomainInformation` object. +During MVP, our import scripts did not populate the following fields: `address_line, city, state_territory, and zipcode` for organization address in Domain Information. This was primarily due to time constraints. Because of this, we need to run a follow-on script to load this remaining data on each `DomainInformation` object. This script is intended to run under the assumption that the [load_transition_domain](#step-1-load-transition-domains) and the [transfer_transition_domains_to_domains](#step-2-transfer-transition-domain-data-into-main-domain-tables) scripts have already been ran. From 4afb230a750c78d426293b66ad8b860ba1657813 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 22 Nov 2023 08:14:06 -0700 Subject: [PATCH 38/46] Update docs/operations/data_migration.md Co-authored-by: Alysia Broddrick <109625347+abroddrick@users.noreply.github.com> --- docs/operations/data_migration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index aa74d6260..058536f85 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -470,6 +470,6 @@ The `load_organization_data` script has five optional parameters. These are as f |:-:|:---------------------------------|:----------------------------------------------------------------------------| | 1 | **sep** | Determines the file separator. Defaults to "\|" | | 2 | **debug** | Increases logging detail. Defaults to False | -| 3 | **directory** | Specifies the containing directory of the data. Defaults to "migrationdata" | +| 3 | **directory** | Specifies the directory containing the files that will be parsed. Defaults to "migrationdata" | | 4 | **domain_additional_filename** | Specifies the filename of domain_additional. Used as an override for the JSON. Has no default. | | 5 | **organization_adhoc_filename** | Specifies the filename of organization_adhoc. Used as an override for the JSON. Has no default. | From 854f0d44644e5125200c08a4920b0695798b4438 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 22 Nov 2023 08:29:42 -0700 Subject: [PATCH 39/46] Change load_organization_data comment and remove unused params --- .../management/commands/load_organization_data.py | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index 87bc62512..f480bf567 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -1,4 +1,4 @@ -"""Data migration: Send domain invitations once to existing customers.""" +"""Data migration: Load organization data for TransitionDomain and DomainInformation objects""" import argparse import json @@ -54,18 +54,6 @@ class Command(BaseCommand): parser.add_argument("--directory", default="migrationdata", help="Desired directory") - # Serves as a domain_additional_filename override - parser.add_argument( - "--domain_additional_filename", - help="Defines the filename for additional domain data", - ) - - # Serves as a organization_adhoc_filename override - parser.add_argument( - "--organization_adhoc_filename", - help="Defines the filename for domain type adhocs", - ) - def handle(self, migration_json_filename, **options): """Process the objects in TransitionDomain.""" # Parse JSON file From 86506f4e312a9baa2f0175ce8ba5ab70c07ae7c6 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 22 Nov 2023 08:33:51 -0700 Subject: [PATCH 40/46] Update src/registrar/management/commands/load_organization_data.py Co-authored-by: Alysia Broddrick <109625347+abroddrick@users.noreply.github.com> --- src/registrar/management/commands/load_organization_data.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index f480bf567..16426eb08 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -55,7 +55,7 @@ class Command(BaseCommand): parser.add_argument("--directory", default="migrationdata", help="Desired directory") def handle(self, migration_json_filename, **options): - """Process the objects in TransitionDomain.""" + """Load organization address data into the TransitionDomain and DomainInformation tables by using the organization adhoc file and domain_additional file""" # Parse JSON file options = self.load_json_settings(options, migration_json_filename) args = TransitionDomainArguments(**options) From 64bd9a38d2b1a63d30899be12d5a64cf86653d05 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 22 Nov 2023 08:34:16 -0700 Subject: [PATCH 41/46] Update src/registrar/management/commands/utility/extra_transition_domain_helper.py Co-authored-by: Alysia Broddrick <109625347+abroddrick@users.noreply.github.com> --- .../commands/utility/extra_transition_domain_helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/management/commands/utility/extra_transition_domain_helper.py b/src/registrar/management/commands/utility/extra_transition_domain_helper.py index 75cde995a..a198386fb 100644 --- a/src/registrar/management/commands/utility/extra_transition_domain_helper.py +++ b/src/registrar/management/commands/utility/extra_transition_domain_helper.py @@ -787,7 +787,7 @@ class OrganizationDataLoader: self.tds_to_update: List[TransitionDomain] = [] def update_organization_data_for_all(self): - """Updates org data for all TransitionDomains""" + """Updates org address data for all TransitionDomains""" all_transition_domains = TransitionDomain.objects.all() if len(all_transition_domains) == 0: raise LoadOrganizationError(code=LoadOrganizationErrorCodes.EMPTY_TRANSITION_DOMAIN_TABLE) From a7afe798dd239cfd6df110ba3513c2eb903b32b5 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 22 Nov 2023 08:37:01 -0700 Subject: [PATCH 42/46] Update src/registrar/management/commands/utility/extra_transition_domain_helper.py Co-authored-by: Alysia Broddrick <109625347+abroddrick@users.noreply.github.com> --- .../commands/utility/extra_transition_domain_helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/management/commands/utility/extra_transition_domain_helper.py b/src/registrar/management/commands/utility/extra_transition_domain_helper.py index a198386fb..3f6b2423f 100644 --- a/src/registrar/management/commands/utility/extra_transition_domain_helper.py +++ b/src/registrar/management/commands/utility/extra_transition_domain_helper.py @@ -845,7 +845,7 @@ class OrganizationDataLoader: def parse_org_data(self, domain_name, transition_domain: TransitionDomain) -> TransitionDomain: """Grabs organization_name from the parsed files and associates it - with a transition_domain object, then returns that object.""" + with a transition_domain object, then updates that transition domain object and returns it""" if not isinstance(transition_domain, TransitionDomain): raise ValueError("Not a valid object, must be TransitionDomain") From 99d449679cc0c709f709f380886559c596a45954 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 22 Nov 2023 08:37:20 -0700 Subject: [PATCH 43/46] Update src/registrar/management/commands/load_organization_data.py Co-authored-by: Alysia Broddrick <109625347+abroddrick@users.noreply.github.com> --- src/registrar/management/commands/load_organization_data.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index 16426eb08..bc4d6698b 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -97,7 +97,7 @@ class Command(BaseCommand): Number of DomainInformation objects to (potentially) change: {len(transition_domains)} For each DomainInformation, modify the following fields: {self.changed_fields} """, - prompt_title="Do you wish to load organization data for DomainInformation?", + prompt_title="Do you wish to update organization address data for DomainInformation as well?", ) logger.info( From 8535d783e82094bb787b2c771a7a109c0ae95ba9 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 22 Nov 2023 08:37:48 -0700 Subject: [PATCH 44/46] PR suggestions Change variable names and coments --- .../commands/load_organization_data.py | 22 +++++++++---------- .../utility/extra_transition_domain_helper.py | 2 ++ 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index f480bf567..087baaf17 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -58,26 +58,26 @@ class Command(BaseCommand): """Process the objects in TransitionDomain.""" # Parse JSON file options = self.load_json_settings(options, migration_json_filename) - args = TransitionDomainArguments(**options) + org_args = TransitionDomainArguments(**options) # Will sys.exit() when prompt is "n" TerminalHelper.prompt_for_execution( system_exit_on_terminate=True, info_to_inspect=f""" ==Master data file== - domain_additional_filename: {args.domain_additional_filename} + domain_additional_filename: {org_args.domain_additional_filename} ==Organization data== - organization_adhoc_filename: {args.organization_adhoc_filename} + organization_adhoc_filename: {org_args.organization_adhoc_filename} ==Containing directory== - directory: {args.directory} + directory: {org_args.directory} """, prompt_title="Do you wish to load organization data for TransitionDomains?", ) - load = OrganizationDataLoader(args) - transition_domains = load.update_organization_data_for_all() + org_load_helper = OrganizationDataLoader(org_args) + transition_domains = org_load_helper.update_organization_data_for_all() # Reprompt the user to reinspect before updating DomainInformation # Will sys.exit() when prompt is "n" @@ -85,13 +85,13 @@ class Command(BaseCommand): system_exit_on_terminate=True, info_to_inspect=f""" ==Master data file== - domain_additional_filename: {args.domain_additional_filename} + domain_additional_filename: {org_args.domain_additional_filename} ==Organization name information== - organization_adhoc_filename: {args.organization_adhoc_filename} + organization_adhoc_filename: {org_args.organization_adhoc_filename} ==Containing directory== - directory: {args.directory} + directory: {org_args.directory} ==Proposed Changes== Number of DomainInformation objects to (potentially) change: {len(transition_domains)} @@ -105,10 +105,10 @@ class Command(BaseCommand): "Preparing to load organization data onto DomainInformation tables..." f"{TerminalColors.ENDC}" ) - self.prepare_update_domain_information(transition_domains, args.debug) + self.prepare_update_domain_information(transition_domains, org_args.debug) logger.info(f"{TerminalColors.MAGENTA}" f"Beginning mass DomainInformation update..." f"{TerminalColors.ENDC}") - self.bulk_update_domain_information(args.debug) + self.bulk_update_domain_information(org_args.debug) def load_json_settings(self, options, migration_json_filename): """Parses options from the given JSON file.""" diff --git a/src/registrar/management/commands/utility/extra_transition_domain_helper.py b/src/registrar/management/commands/utility/extra_transition_domain_helper.py index 75cde995a..677885839 100644 --- a/src/registrar/management/commands/utility/extra_transition_domain_helper.py +++ b/src/registrar/management/commands/utility/extra_transition_domain_helper.py @@ -800,6 +800,8 @@ class OrganizationDataLoader: return self.tds_to_update def prepare_transition_domains(self, transition_domains): + """Pares org data for each transition domain, + then appends it to the tds_to_update list""" for item in transition_domains: updated = self.parse_org_data(item.domain_name, item) self.tds_to_update.append(updated) From db964498e1c8edc231b4e8267df0f618df64537f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 22 Nov 2023 08:49:48 -0700 Subject: [PATCH 45/46] Linting --- src/registrar/management/commands/load_organization_data.py | 3 ++- .../commands/utility/extra_transition_domain_helper.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index 0036faf45..618e700b0 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -55,7 +55,8 @@ class Command(BaseCommand): parser.add_argument("--directory", default="migrationdata", help="Desired directory") def handle(self, migration_json_filename, **options): - """Load organization address data into the TransitionDomain and DomainInformation tables by using the organization adhoc file and domain_additional file""" + """Load organization address data into the TransitionDomain + and DomainInformation tables by using the organization adhoc file and domain_additional file""" # Parse JSON file options = self.load_json_settings(options, migration_json_filename) org_args = TransitionDomainArguments(**options) diff --git a/src/registrar/management/commands/utility/extra_transition_domain_helper.py b/src/registrar/management/commands/utility/extra_transition_domain_helper.py index ea3f3911f..04170811f 100644 --- a/src/registrar/management/commands/utility/extra_transition_domain_helper.py +++ b/src/registrar/management/commands/utility/extra_transition_domain_helper.py @@ -800,7 +800,7 @@ class OrganizationDataLoader: return self.tds_to_update def prepare_transition_domains(self, transition_domains): - """Pares org data for each transition domain, + """Parses org data for each transition domain, then appends it to the tds_to_update list""" for item in transition_domains: updated = self.parse_org_data(item.domain_name, item) From 91d33f4c2a4da291aaee45049f006b59bcf8010c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 22 Nov 2023 09:08:49 -0700 Subject: [PATCH 46/46] Linting --- src/registrar/management/commands/load_organization_data.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index 618e700b0..122795400 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -55,7 +55,7 @@ class Command(BaseCommand): parser.add_argument("--directory", default="migrationdata", help="Desired directory") def handle(self, migration_json_filename, **options): - """Load organization address data into the TransitionDomain + """Load organization address data into the TransitionDomain and DomainInformation tables by using the organization adhoc file and domain_additional file""" # Parse JSON file options = self.load_json_settings(options, migration_json_filename)