From eff92bf06b355b669dc59c6e3f87ad552435fcde Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 28 Dec 2023 15:08:11 -0700 Subject: [PATCH 01/29] Add patch script --- .../commands/patch_federal_agency_info.py | 148 ++++++++++++++++++ 1 file changed, 148 insertions(+) create mode 100644 src/registrar/management/commands/patch_federal_agency_info.py diff --git a/src/registrar/management/commands/patch_federal_agency_info.py b/src/registrar/management/commands/patch_federal_agency_info.py new file mode 100644 index 000000000..87fa0972a --- /dev/null +++ b/src/registrar/management/commands/patch_federal_agency_info.py @@ -0,0 +1,148 @@ +"""""" +import argparse +import json +import logging + +import os +from typing import List + +from django.core.management import BaseCommand +from registrar.management.commands.utility.epp_data_containers import AgencyAdhoc, AuthorityAdhoc, DomainAdditionalData, EnumFilenames +from registrar.management.commands.utility.extra_transition_domain_helper import ExtraTransitionDomain, MigrationDataFileLoader +from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper +from registrar.management.commands.utility.transition_domain_arguments import TransitionDomainArguments +from registrar.models.domain_information import DomainInformation +from django.db.models import Q + +from registrar.models.transition_domain import TransitionDomain + +logger = logging.getLogger(__name__) + + +class Command(BaseCommand): + help = "Runs the cat command on files from /tmp into the getgov directory." + + def __init__(self): + super().__init__() + self.di_to_update: List[DomainInformation] = [] + + # Stores the domain_name for logging purposes + self.di_failed_to_update: List[str] = [] + self.di_skipped: List[str] = [] + + def add_arguments(self, parser): + """Adds command line arguments""" + parser.add_argument("--debug", action=argparse.BooleanOptionalAction) + + def handle(self, **options): + debug = options.get("debug") + domain_info_to_fix = DomainInformation.objects.filter(Q(federal_agency=None) | Q(federal_agency="")) + + domain_names = domain_info_to_fix.values_list('domain__name', flat=True) + transition_domains = TransitionDomain.objects.filter(domain_name__in=domain_names) + + # Get the domain names from TransitionDomain + td_agencies = transition_domains.values_list("domain_name", "federal_agency").distinct() + + TerminalHelper.prompt_for_execution( + system_exit_on_terminate=True, + info_to_inspect=f""" + ==Proposed Changes== + Number of DomainInformation objects to change: {len(td_agencies)} + The following DomainInformation objects will be modified: {td_agencies} + """, + prompt_title="Do you wish to update organization address data for DomainInformation as well?", + ) + logger.info("Updating...") + + # Create a dictionary mapping of domain_name to federal_agency + td_dict = dict(td_agencies) + + for di in domain_info_to_fix: + domain_name = di.domain.name + if domain_name in td_dict and td_dict.get(domain_name) is not None: + # Grab existing federal_agency data + di.federal_agency = td_dict.get(domain_name) + # Append it to our update list + self.di_to_update.append(di) + if debug: + logger.info( + f"{TerminalColors.OKCYAN}Updated {di}{TerminalColors.ENDC}" + ) + else: + self.di_skipped.append(di) + if debug: + logger.info( + f"{TerminalColors.YELLOW}Skipping update for {di}{TerminalColors.ENDC}" + ) + + DomainInformation.objects.bulk_update(self.di_to_update, ["federal_agency"]) + + # After the update has happened, do a sweep of what we get back. + # If the fields we expect to update are still None, then something is wrong. + for di in domain_info_to_fix: + if domain_name in td_dict and td_dict.get(domain_name) is not None: + logger.info( + f"{TerminalColors.FAIL}Failed to update {di}{TerminalColors.ENDC}" + ) + self.di_failed_to_update.append(di) + + # === Log results and return data === # + self.log_script_run_summary(debug) + + def log_script_run_summary(self, debug): + """Prints success, failed, and skipped counts, as well as + all affected objects.""" + update_success_count = len(self.di_to_update) + update_failed_count = len(self.di_failed_to_update) + update_skipped_count = len(self.di_skipped) + + # Prepare debug messages + debug_messages = { + "success": (f"{TerminalColors.OKCYAN}Updated: {self.di_to_update}{TerminalColors.ENDC}\n"), + "skipped": (f"{TerminalColors.YELLOW}Skipped: {self.di_skipped}{TerminalColors.ENDC}\n"), + "failed": ( + f"{TerminalColors.FAIL}Failed: {self.di_failed_to_update}{TerminalColors.ENDC}\n" + ), + } + + # Print out a list of everything that was changed, if we have any changes to log. + # Otherwise, don't print anything. + TerminalHelper.print_conditional( + debug, + f"{debug_messages.get('success') if update_success_count > 0 else ''}" + f"{debug_messages.get('skipped') if update_skipped_count > 0 else ''}" + f"{debug_messages.get('failed') if update_failed_count > 0 else ''}", + ) + + if update_failed_count == 0 and update_skipped_count == 0: + logger.info( + f"""{TerminalColors.OKGREEN} + ============= FINISHED =============== + Updated {update_success_count} DomainInformation entries + {TerminalColors.ENDC} + """ + ) + elif update_failed_count == 0: + logger.info( + f"""{TerminalColors.YELLOW} + ============= FINISHED =============== + Updated {update_success_count} DomainInformation entries + + ----- SOME AGENCY DATA WAS NONE ----- + Skipped updating {update_skipped_count} DomainInformation entries + {TerminalColors.ENDC} + """ + ) + else: + logger.info( + f"""{TerminalColors.FAIL} + ============= FINISHED =============== + Updated {update_success_count} DomainInformation entries + + ----- UPDATE FAILED ----- + Failed to update {update_failed_count} DomainInformation entries, + Skipped updating {update_skipped_count} DomainInformation entries + {TerminalColors.ENDC} + """ + ) \ No newline at end of file From 68f75ec8a881025b189189525a036ecc70ed175c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 29 Dec 2023 08:26:43 -0700 Subject: [PATCH 02/29] Update patch_federal_agency_info.py --- .../commands/patch_federal_agency_info.py | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/registrar/management/commands/patch_federal_agency_info.py b/src/registrar/management/commands/patch_federal_agency_info.py index 87fa0972a..18f357afe 100644 --- a/src/registrar/management/commands/patch_federal_agency_info.py +++ b/src/registrar/management/commands/patch_federal_agency_info.py @@ -1,16 +1,12 @@ """""" import argparse -import json import logging -import os from typing import List from django.core.management import BaseCommand -from registrar.management.commands.utility.epp_data_containers import AgencyAdhoc, AuthorityAdhoc, DomainAdditionalData, EnumFilenames -from registrar.management.commands.utility.extra_transition_domain_helper import ExtraTransitionDomain, MigrationDataFileLoader from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper -from registrar.management.commands.utility.transition_domain_arguments import TransitionDomainArguments +from registrar.models.domain import Domain from registrar.models.domain_information import DomainInformation from django.db.models import Q @@ -36,6 +32,11 @@ class Command(BaseCommand): def handle(self, **options): debug = options.get("debug") + + # Update the "federal_agency" field + self.patch_agency_info(debug) + + def patch_agency_info(self, debug): domain_info_to_fix = DomainInformation.objects.filter(Q(federal_agency=None) | Q(federal_agency="")) domain_names = domain_info_to_fix.values_list('domain__name', flat=True) @@ -51,7 +52,7 @@ class Command(BaseCommand): Number of DomainInformation objects to change: {len(td_agencies)} The following DomainInformation objects will be modified: {td_agencies} """, - prompt_title="Do you wish to update organization address data for DomainInformation as well?", + prompt_title="Do you wish to patch federal_agency data?", ) logger.info("Updating...") @@ -78,10 +79,11 @@ class Command(BaseCommand): DomainInformation.objects.bulk_update(self.di_to_update, ["federal_agency"]) + corrected_domains = DomainInformation.objects.filter(domain__name__in=domain_names) # After the update has happened, do a sweep of what we get back. # If the fields we expect to update are still None, then something is wrong. - for di in domain_info_to_fix: - if domain_name in td_dict and td_dict.get(domain_name) is not None: + for di in corrected_domains: + if domain_name in td_dict and td_dict.get(domain_name) is None: logger.info( f"{TerminalColors.FAIL}Failed to update {di}{TerminalColors.ENDC}" ) @@ -129,7 +131,7 @@ class Command(BaseCommand): ============= FINISHED =============== Updated {update_success_count} DomainInformation entries - ----- SOME AGENCY DATA WAS NONE ----- + ----- SOME AGENCY DATA WAS NONE (NEEDS MANUAL PATCHING) ----- Skipped updating {update_skipped_count} DomainInformation entries {TerminalColors.ENDC} """ From 57d4234544cb6796e59b3bdfbf822ec37ca79d1a Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 29 Dec 2023 11:26:02 -0700 Subject: [PATCH 03/29] Increase code clarity --- .../commands/patch_federal_agency_info.py | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/registrar/management/commands/patch_federal_agency_info.py b/src/registrar/management/commands/patch_federal_agency_info.py index 18f357afe..fc1ef4ac1 100644 --- a/src/registrar/management/commands/patch_federal_agency_info.py +++ b/src/registrar/management/commands/patch_federal_agency_info.py @@ -61,25 +61,27 @@ class Command(BaseCommand): for di in domain_info_to_fix: domain_name = di.domain.name - if domain_name in td_dict and td_dict.get(domain_name) is not None: - # Grab existing federal_agency data - di.federal_agency = td_dict.get(domain_name) - # Append it to our update list + federal_agency = td_dict.get(domain_name) + + # If agency exists on a TransitionDomain, update the related DomainInformation object + if domain_name in td_dict and federal_agency is not None: + di.federal_agency = federal_agency self.di_to_update.append(di) - if debug: - logger.info( - f"{TerminalColors.OKCYAN}Updated {di}{TerminalColors.ENDC}" - ) + log_message = f"{TerminalColors.OKCYAN}Updated {di}{TerminalColors.ENDC}" else: self.di_skipped.append(di) - if debug: - logger.info( - f"{TerminalColors.YELLOW}Skipping update for {di}{TerminalColors.ENDC}" - ) - + log_message = f"{TerminalColors.YELLOW}Skipping update for {di}{TerminalColors.ENDC}" + + # Log the action if debug mode is on + if debug: + logger.info(log_message) + + # Bulk update the federal agency field in DomainInformation objects DomainInformation.objects.bulk_update(self.di_to_update, ["federal_agency"]) + # Get a list of each domain we changed corrected_domains = DomainInformation.objects.filter(domain__name__in=domain_names) + # After the update has happened, do a sweep of what we get back. # If the fields we expect to update are still None, then something is wrong. for di in corrected_domains: From 21edb7b0adc733f7db757c600f5650c23422dd97 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 29 Dec 2023 11:28:36 -0700 Subject: [PATCH 04/29] Add comment --- src/registrar/management/commands/patch_federal_agency_info.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/registrar/management/commands/patch_federal_agency_info.py b/src/registrar/management/commands/patch_federal_agency_info.py index fc1ef4ac1..6ae679ed5 100644 --- a/src/registrar/management/commands/patch_federal_agency_info.py +++ b/src/registrar/management/commands/patch_federal_agency_info.py @@ -45,6 +45,7 @@ class Command(BaseCommand): # Get the domain names from TransitionDomain td_agencies = transition_domains.values_list("domain_name", "federal_agency").distinct() + # Code execution will stop here if the user prompts "N" TerminalHelper.prompt_for_execution( system_exit_on_terminate=True, info_to_inspect=f""" From 5716af32392d9b82b94ce80785bb7494420fb058 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 29 Dec 2023 13:19:45 -0700 Subject: [PATCH 05/29] Unit tests --- .../commands/patch_federal_agency_info.py | 48 ++++++++------- .../test_transition_domain_migrations.py | 59 +++++++++++++++++++ 2 files changed, 85 insertions(+), 22 deletions(-) diff --git a/src/registrar/management/commands/patch_federal_agency_info.py b/src/registrar/management/commands/patch_federal_agency_info.py index 6ae679ed5..0903858ee 100644 --- a/src/registrar/management/commands/patch_federal_agency_info.py +++ b/src/registrar/management/commands/patch_federal_agency_info.py @@ -1,4 +1,4 @@ -"""""" +"""Loops through each valid DomainInformation object and updates its agency value""" import argparse import logging @@ -6,7 +6,6 @@ from typing import List from django.core.management import BaseCommand from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper -from registrar.models.domain import Domain from registrar.models.domain_information import DomainInformation from django.db.models import Q @@ -16,7 +15,7 @@ logger = logging.getLogger(__name__) class Command(BaseCommand): - help = "Runs the cat command on files from /tmp into the getgov directory." + help = "Loops through each valid DomainInformation object and updates its agency value" def __init__(self): super().__init__() @@ -30,18 +29,26 @@ class Command(BaseCommand): """Adds command line arguments""" parser.add_argument("--debug", action=argparse.BooleanOptionalAction) - def handle(self, **options): - debug = options.get("debug") + def handle(self, **kwargs): + """Loops through each valid DomainInformation object and updates its agency value""" + debug = kwargs.get("debug") # Update the "federal_agency" field self.patch_agency_info(debug) def patch_agency_info(self, debug): - domain_info_to_fix = DomainInformation.objects.filter(Q(federal_agency=None) | Q(federal_agency="")) + """ + Updates the `federal_agency` field of each valid `DomainInformation` object based on the corresponding + `TransitionDomain` object. Skips the update if the `TransitionDomain` object does not exist or its + `federal_agency` field is `None`. Logs the update, skip, and failure actions if debug mode is on. + After all updates, logs a summary of the results. + """ + empty_agency_query = Q(federal_agency=None) | Q(federal_agency="") + domain_info_to_fix = DomainInformation.objects.filter(empty_agency_query) + + domain_names = domain_info_to_fix.values_list("domain__name", flat=True) + transition_domains = TransitionDomain.objects.filter(domain_name__in=domain_names).exclude(empty_agency_query) - domain_names = domain_info_to_fix.values_list('domain__name', flat=True) - transition_domains = TransitionDomain.objects.filter(domain_name__in=domain_names) - # Get the domain names from TransitionDomain td_agencies = transition_domains.values_list("domain_name", "federal_agency").distinct() @@ -63,18 +70,19 @@ class Command(BaseCommand): for di in domain_info_to_fix: domain_name = di.domain.name federal_agency = td_dict.get(domain_name) + log_message = None # If agency exists on a TransitionDomain, update the related DomainInformation object if domain_name in td_dict and federal_agency is not None: di.federal_agency = federal_agency self.di_to_update.append(di) log_message = f"{TerminalColors.OKCYAN}Updated {di}{TerminalColors.ENDC}" - else: + elif domain_name not in td_dict: self.di_skipped.append(di) log_message = f"{TerminalColors.YELLOW}Skipping update for {di}{TerminalColors.ENDC}" - + # Log the action if debug mode is on - if debug: + if debug and log_message is not None: logger.info(log_message) # Bulk update the federal agency field in DomainInformation objects @@ -87,11 +95,9 @@ class Command(BaseCommand): # If the fields we expect to update are still None, then something is wrong. for di in corrected_domains: if domain_name in td_dict and td_dict.get(domain_name) is None: - logger.info( - f"{TerminalColors.FAIL}Failed to update {di}{TerminalColors.ENDC}" - ) + logger.info(f"{TerminalColors.FAIL}Failed to update {di}{TerminalColors.ENDC}") self.di_failed_to_update.append(di) - + # === Log results and return data === # self.log_script_run_summary(debug) @@ -106,9 +112,7 @@ class Command(BaseCommand): debug_messages = { "success": (f"{TerminalColors.OKCYAN}Updated: {self.di_to_update}{TerminalColors.ENDC}\n"), "skipped": (f"{TerminalColors.YELLOW}Skipped: {self.di_skipped}{TerminalColors.ENDC}\n"), - "failed": ( - f"{TerminalColors.FAIL}Failed: {self.di_failed_to_update}{TerminalColors.ENDC}\n" - ), + "failed": (f"{TerminalColors.FAIL}Failed: {self.di_failed_to_update}{TerminalColors.ENDC}\n"), } # Print out a list of everything that was changed, if we have any changes to log. @@ -129,7 +133,7 @@ class Command(BaseCommand): """ ) elif update_failed_count == 0: - logger.info( + logger.warning( f"""{TerminalColors.YELLOW} ============= FINISHED =============== Updated {update_success_count} DomainInformation entries @@ -140,7 +144,7 @@ class Command(BaseCommand): """ ) else: - logger.info( + logger.error( f"""{TerminalColors.FAIL} ============= FINISHED =============== Updated {update_success_count} DomainInformation entries @@ -150,4 +154,4 @@ class Command(BaseCommand): Skipped updating {update_skipped_count} DomainInformation entries {TerminalColors.ENDC} """ - ) \ No newline at end of file + ) diff --git a/src/registrar/tests/test_transition_domain_migrations.py b/src/registrar/tests/test_transition_domain_migrations.py index f3fd76e88..472dbac86 100644 --- a/src/registrar/tests/test_transition_domain_migrations.py +++ b/src/registrar/tests/test_transition_domain_migrations.py @@ -21,6 +21,65 @@ from registrar.models.contact import Contact from .common import MockEppLib, less_console_noise +class TestPatchAgencyInfo(TestCase): + def setUp(self): + self.user, _ = User.objects.get_or_create(username="testuser") + self.domain, _ = Domain.objects.get_or_create(name="testdomain.gov") + self.domain_info, _ = DomainInformation.objects.get_or_create(domain=self.domain, creator=self.user) + self.transition_domain, _ = TransitionDomain.objects.get_or_create( + domain_name="testdomain.gov", federal_agency="test agency" + ) + + def tearDown(self): + Domain.objects.all().delete() + DomainInformation.objects.all().delete() + User.objects.all().delete() + TransitionDomain.objects.all().delete() + + @patch("registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit", return_value=True) + def call_patch_federal_agency_info(self, mock_prompt): + """Calls the patch_federal_agency_info command and mimics a keypress""" + call_command("patch_federal_agency_info", debug=True) + + def test_patch_agency_info(self): + """ + Tests that the `patch_federal_agency_info` command successfully + updates the `federal_agency` field + of a `DomainInformation` object when the corresponding + `TransitionDomain` object has a valid `federal_agency`. + """ + self.call_patch_federal_agency_info() + + # Reload the domain_info object from the database + self.domain_info.refresh_from_db() + + # Check that the federal_agency field was updated + self.assertEqual(self.domain_info.federal_agency, "test agency") + + def test_patch_agency_info_skip(self): + """ + Tests that the `patch_federal_agency_info` command logs a warning and + does not update the `federal_agency` field + of a `DomainInformation` object when the corresponding + `TransitionDomain` object does not exist. + """ + # Set federal_agency to None to simulate a skip + self.transition_domain.federal_agency = None + self.transition_domain.save() + + with self.assertLogs("registrar.management.commands.patch_federal_agency_info", level="WARNING") as context: + self.call_patch_federal_agency_info() + + # Check that the correct log message was output + self.assertIn("SOME AGENCY DATA WAS NONE", context.output[0]) + + # Reload the domain_info object from the database + self.domain_info.refresh_from_db() + + # Check that the federal_agency field was not updated + self.assertIsNone(self.domain_info.federal_agency) + + class TestExtendExpirationDates(MockEppLib): def setUp(self): """Defines the file name of migration_json and the folder its contained in""" From e747321bd78e456784464273562cbba8ac2ad423 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 29 Dec 2023 13:28:47 -0700 Subject: [PATCH 06/29] Add test case --- .../test_transition_domain_migrations.py | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/registrar/tests/test_transition_domain_migrations.py b/src/registrar/tests/test_transition_domain_migrations.py index 472dbac86..c27da7d78 100644 --- a/src/registrar/tests/test_transition_domain_migrations.py +++ b/src/registrar/tests/test_transition_domain_migrations.py @@ -79,6 +79,27 @@ class TestPatchAgencyInfo(TestCase): # Check that the federal_agency field was not updated self.assertIsNone(self.domain_info.federal_agency) + def test_patch_agency_info_skips_valid_domains(self): + """ + Tests that the `patch_federal_agency_info` command logs INFO and + does not update the `federal_agency` field + of a `DomainInformation` object + """ + self.domain_info.federal_agency = "unchanged" + self.domain_info.save() + + with self.assertLogs("registrar.management.commands.patch_federal_agency_info", level="INFO") as context: + self.call_patch_federal_agency_info() + + # Check that the correct log message was output + self.assertIn("FINISHED", context.output[1]) + + # Reload the domain_info object from the database + self.domain_info.refresh_from_db() + + # Check that the federal_agency field was not updated + self.assertEqual(self.domain_info.federal_agency, "unchanged") + class TestExtendExpirationDates(MockEppLib): def setUp(self): From fd847a2a40b36decc70d82aed8bb399907cdf457 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 2 Jan 2024 13:32:43 -0700 Subject: [PATCH 07/29] Read from current-full.csv --- .../commands/patch_federal_agency_info.py | 103 +++++++++++++++--- 1 file changed, 90 insertions(+), 13 deletions(-) diff --git a/src/registrar/management/commands/patch_federal_agency_info.py b/src/registrar/management/commands/patch_federal_agency_info.py index 0903858ee..76a1ed889 100644 --- a/src/registrar/management/commands/patch_federal_agency_info.py +++ b/src/registrar/management/commands/patch_federal_agency_info.py @@ -1,7 +1,8 @@ """Loops through each valid DomainInformation object and updates its agency value""" import argparse +import csv import logging - +import os from typing import List from django.core.management import BaseCommand @@ -20,27 +21,103 @@ class Command(BaseCommand): def __init__(self): super().__init__() self.di_to_update: List[DomainInformation] = [] - - # Stores the domain_name for logging purposes - self.di_failed_to_update: List[str] = [] - self.di_skipped: List[str] = [] + self.di_failed_to_update: List[DomainInformation] = [] + self.di_skipped: List[DomainInformation] = [] def add_arguments(self, parser): """Adds command line arguments""" + parser.add_argument( + "current_full_filepath", + help="TBD", + ) parser.add_argument("--debug", action=argparse.BooleanOptionalAction) + parser.add_argument("--sep", default=",", help="Delimiter character") - def handle(self, **kwargs): + def handle(self, current_full_filepath, **kwargs): """Loops through each valid DomainInformation object and updates its agency value""" debug = kwargs.get("debug") + seperator = kwargs.get("sep") - # Update the "federal_agency" field + # Check if the provided file path is valid + if not os.path.isfile(current_full_filepath): + raise argparse.ArgumentTypeError(f"Invalid file path '{current_full_filepath}'") + + # === Update the "federal_agency" field === # self.patch_agency_info(debug) + # === Try to process anything that was skipped === # + if len(self.di_skipped) > 0: + self.process_skipped_records(current_full_filepath, seperator, debug) + + # Clear the old skipped list, and log the run summary + self.di_skipped.clear() + self.log_script_run_summary(debug) + + def process_skipped_records(self, file_path, seperator, debug): + """If we encounter any DomainInformation records that do not have data in the associated + TransitionDomain record, then check the associated current-full.csv file for this + information.""" + self.di_to_update.clear() + self.di_failed_to_update.clear() + # Code execution will stop here if the user prompts "N" + TerminalHelper.prompt_for_execution( + system_exit_on_terminate=True, + info_to_inspect=f""" + ==File location== + current-full.csv filepath: {file_path} + + ==Proposed Changes== + Number of DomainInformation objects to change: {len(self.di_skipped)} + The following DomainInformation objects will be modified: {self.di_skipped} + """, + prompt_title="Do you wish to patch skipped records?", + ) + logger.info("Updating...") + + file_data = self.read_current_full(file_path, seperator) + for di in self.di_skipped: + domain_name = di.domain.name + row = file_data.get(domain_name) + fed_agency = None + if row is not None and "Agency" in row: + fed_agency = row.get("Agency") + + # Determine if we should update this record or not. + # If we don't get any data back, something went wrong. + if fed_agency is not None: + di.federal_agency = fed_agency + self.di_to_update.append(di) + if debug: + logger.info( + f"{TerminalColors.OKCYAN}" + f"Updating {di}" + f"{TerminalColors.ENDC}" + ) + else: + self.di_failed_to_update.append(di) + logger.error( + f"{TerminalColors.FAIL}" + f"Could not update {di}. No information found." + f"{TerminalColors.ENDC}" + ) + + # Bulk update the federal agency field in DomainInformation objects + DomainInformation.objects.bulk_update(self.di_to_update, ["federal_agency"]) + + def read_current_full(self, file_path, seperator): + """Reads the current-full.csv file and stores it in a dictionary""" + with open(file_path, "r") as requested_file: + reader = csv.DictReader(requested_file, delimiter=seperator) + # Return a dictionary with the domain name as the key, + # and the row information as the value + dict_data = {row.get("Domain Name").lower(): row for row in reader} + return dict_data + def patch_agency_info(self, debug): """ - Updates the `federal_agency` field of each valid `DomainInformation` object based on the corresponding - `TransitionDomain` object. Skips the update if the `TransitionDomain` object does not exist or its - `federal_agency` field is `None`. Logs the update, skip, and failure actions if debug mode is on. + Updates the federal_agency field of each valid DomainInformation object based on the corresponding + TransitionDomain object. Skips the update if the TransitionDomain object does not exist or its + federal_agency field is None. Logs the update, skip, and failure actions if debug mode is on. After all updates, logs a summary of the results. """ empty_agency_query = Q(federal_agency=None) | Q(federal_agency="") @@ -57,8 +134,8 @@ class Command(BaseCommand): system_exit_on_terminate=True, info_to_inspect=f""" ==Proposed Changes== - Number of DomainInformation objects to change: {len(td_agencies)} - The following DomainInformation objects will be modified: {td_agencies} + Number of DomainInformation objects to change: {len(domain_info_to_fix)} + The following DomainInformation objects will be modified: {domain_info_to_fix} """, prompt_title="Do you wish to patch federal_agency data?", ) @@ -138,7 +215,7 @@ class Command(BaseCommand): ============= FINISHED =============== Updated {update_success_count} DomainInformation entries - ----- SOME AGENCY DATA WAS NONE (NEEDS MANUAL PATCHING) ----- + ----- SOME AGENCY DATA WAS NONE (WILL BE PATCHED AUTOMATICALLY) ----- Skipped updating {update_skipped_count} DomainInformation entries {TerminalColors.ENDC} """ From 20f34d56bed2fdf6fcf2923ebe3634108051ac42 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 2 Jan 2024 13:46:26 -0700 Subject: [PATCH 08/29] Fix unit tests --- .../management/commands/patch_federal_agency_info.py | 8 +++++++- src/registrar/tests/test_transition_domain_migrations.py | 6 +++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/registrar/management/commands/patch_federal_agency_info.py b/src/registrar/management/commands/patch_federal_agency_info.py index 76a1ed889..8565ceeda 100644 --- a/src/registrar/management/commands/patch_federal_agency_info.py +++ b/src/registrar/management/commands/patch_federal_agency_info.py @@ -110,7 +110,13 @@ class Command(BaseCommand): reader = csv.DictReader(requested_file, delimiter=seperator) # Return a dictionary with the domain name as the key, # and the row information as the value - dict_data = {row.get("Domain Name").lower(): row for row in reader} + dict_data = {} + for row in reader: + domain_name = row.get("Domain Name") + if domain_name is not None: + domain_name = domain_name.lower() + row[domain_name] = row + return dict_data def patch_agency_info(self, debug): diff --git a/src/registrar/tests/test_transition_domain_migrations.py b/src/registrar/tests/test_transition_domain_migrations.py index c27da7d78..09638cfa6 100644 --- a/src/registrar/tests/test_transition_domain_migrations.py +++ b/src/registrar/tests/test_transition_domain_migrations.py @@ -39,7 +39,11 @@ class TestPatchAgencyInfo(TestCase): @patch("registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit", return_value=True) def call_patch_federal_agency_info(self, mock_prompt): """Calls the patch_federal_agency_info command and mimics a keypress""" - call_command("patch_federal_agency_info", debug=True) + call_command( + "patch_federal_agency_info", + "registrar/tests/data/fake_current_full.csv", + debug=True + ) def test_patch_agency_info(self): """ From 58bb2f0f8793de42bbe768fb82181a3d5b47c3fb Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 2 Jan 2024 14:26:35 -0700 Subject: [PATCH 09/29] Add logic to read current-full.csv --- .../commands/patch_federal_agency_info.py | 159 ++++++++++-------- .../test_transition_domain_migrations.py | 32 +++- 2 files changed, 118 insertions(+), 73 deletions(-) diff --git a/src/registrar/management/commands/patch_federal_agency_info.py b/src/registrar/management/commands/patch_federal_agency_info.py index 8565ceeda..c6585089d 100644 --- a/src/registrar/management/commands/patch_federal_agency_info.py +++ b/src/registrar/management/commands/patch_federal_agency_info.py @@ -43,81 +43,29 @@ class Command(BaseCommand): raise argparse.ArgumentTypeError(f"Invalid file path '{current_full_filepath}'") # === Update the "federal_agency" field === # - self.patch_agency_info(debug) + was_success = self.patch_agency_info(debug) # === Try to process anything that was skipped === # - if len(self.di_skipped) > 0: + # We should only correct skipped records if the previous step was successful. + # If something goes wrong, then we risk corrupting data, so skip this step. + if len(self.di_skipped) > 0 and was_success: + # Flush out the list of DomainInformations to update + self.di_to_update.clear() self.process_skipped_records(current_full_filepath, seperator, debug) # Clear the old skipped list, and log the run summary self.di_skipped.clear() self.log_script_run_summary(debug) - - def process_skipped_records(self, file_path, seperator, debug): - """If we encounter any DomainInformation records that do not have data in the associated - TransitionDomain record, then check the associated current-full.csv file for this - information.""" - self.di_to_update.clear() - self.di_failed_to_update.clear() - # Code execution will stop here if the user prompts "N" - TerminalHelper.prompt_for_execution( - system_exit_on_terminate=True, - info_to_inspect=f""" - ==File location== - current-full.csv filepath: {file_path} - - ==Proposed Changes== - Number of DomainInformation objects to change: {len(self.di_skipped)} - The following DomainInformation objects will be modified: {self.di_skipped} - """, - prompt_title="Do you wish to patch skipped records?", - ) - logger.info("Updating...") - - file_data = self.read_current_full(file_path, seperator) - for di in self.di_skipped: - domain_name = di.domain.name - row = file_data.get(domain_name) - fed_agency = None - if row is not None and "Agency" in row: - fed_agency = row.get("Agency") - - # Determine if we should update this record or not. - # If we don't get any data back, something went wrong. - if fed_agency is not None: - di.federal_agency = fed_agency - self.di_to_update.append(di) - if debug: - logger.info( - f"{TerminalColors.OKCYAN}" - f"Updating {di}" - f"{TerminalColors.ENDC}" - ) - else: - self.di_failed_to_update.append(di) - logger.error( - f"{TerminalColors.FAIL}" - f"Could not update {di}. No information found." - f"{TerminalColors.ENDC}" - ) - - # Bulk update the federal agency field in DomainInformation objects - DomainInformation.objects.bulk_update(self.di_to_update, ["federal_agency"]) - - def read_current_full(self, file_path, seperator): - """Reads the current-full.csv file and stores it in a dictionary""" - with open(file_path, "r") as requested_file: - reader = csv.DictReader(requested_file, delimiter=seperator) - # Return a dictionary with the domain name as the key, - # and the row information as the value - dict_data = {} - for row in reader: - domain_name = row.get("Domain Name") - if domain_name is not None: - domain_name = domain_name.lower() - row[domain_name] = row - - return dict_data + else: + # This code should never execute. This can only occur if bulk_update somehow fails, + # which may indicate some sort of data corruption. + logger.error( + f"{TerminalColors.FAIL}" + "Could not automatically patch skipped records. " + "An error was encountered when running this script, please inspect the following " + f"records for accuracy and completeness: {self.di_failed_to_update}" + f"{TerminalColors.ENDC}" + ) def patch_agency_info(self, debug): """ @@ -126,6 +74,9 @@ class Command(BaseCommand): federal_agency field is None. Logs the update, skip, and failure actions if debug mode is on. After all updates, logs a summary of the results. """ + + # Grab all DomainInformation objects (and their associated TransitionDomains) + # that need to be updated empty_agency_query = Q(federal_agency=None) | Q(federal_agency="") domain_info_to_fix = DomainInformation.objects.filter(empty_agency_query) @@ -184,6 +135,78 @@ class Command(BaseCommand): # === Log results and return data === # self.log_script_run_summary(debug) + # Tracks if this script was successful. If any errors are found, something went very wrong. + was_success = len(self.di_failed_to_update) != 0 + return was_success + + def process_skipped_records(self, file_path, seperator, debug): + """If we encounter any DomainInformation records that do not have data in the associated + TransitionDomain record, then check the associated current-full.csv file for this + information.""" + self.di_to_update.clear() + self.di_failed_to_update.clear() + # Code execution will stop here if the user prompts "N" + TerminalHelper.prompt_for_execution( + system_exit_on_terminate=True, + info_to_inspect=f""" + ==File location== + current-full.csv filepath: {file_path} + + ==Proposed Changes== + Number of DomainInformation objects to change: {len(self.di_skipped)} + The following DomainInformation objects will be modified: {self.di_skipped} + """, + prompt_title="Do you wish to patch skipped records?", + ) + logger.info("Updating...") + + file_data = self.read_current_full(file_path, seperator) + for di in self.di_skipped: + domain_name = di.domain.name + row = file_data.get(domain_name) + fed_agency = None + if row is not None and "agency" in row: + fed_agency = row.get("agency") + + # Determine if we should update this record or not. + # If we don't get any data back, something went wrong. + if fed_agency is not None: + di.federal_agency = fed_agency + self.di_to_update.append(di) + if debug: + logger.info(f"{TerminalColors.OKCYAN}" f"Updating {di}" f"{TerminalColors.ENDC}") + else: + self.di_failed_to_update.append(di) + logger.error( + f"{TerminalColors.FAIL}" f"Could not update {di}. No information found." f"{TerminalColors.ENDC}" + ) + + # Bulk update the federal agency field in DomainInformation objects + DomainInformation.objects.bulk_update(self.di_to_update, ["federal_agency"]) + + def read_current_full(self, file_path, seperator): + """Reads the current-full.csv file and stores it in a dictionary""" + with open(file_path, "r") as requested_file: + old_reader = csv.DictReader(requested_file, delimiter=seperator) + # Some variants of current-full.csv have key casing differences for fields + # such as "Domain name" or "Domain Name". This corrects that. + reader = self.lowercase_fieldnames(old_reader) + # Return a dictionary with the domain name as the key, + # and the row information as the value + dict_data = {} + for row in reader: + domain_name = row.get("domain name") + if domain_name is not None: + domain_name = domain_name.lower() + dict_data[domain_name] = row + + return dict_data + + def lowercase_fieldnames(self, reader): + """Lowercases all field keys in a dictreader to account for potential casing differences""" + for row in reader: + yield {k.lower(): v for k, v in row.items()} + def log_script_run_summary(self, debug): """Prints success, failed, and skipped counts, as well as all affected objects.""" diff --git a/src/registrar/tests/test_transition_domain_migrations.py b/src/registrar/tests/test_transition_domain_migrations.py index 09638cfa6..960ba0480 100644 --- a/src/registrar/tests/test_transition_domain_migrations.py +++ b/src/registrar/tests/test_transition_domain_migrations.py @@ -39,11 +39,7 @@ class TestPatchAgencyInfo(TestCase): @patch("registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit", return_value=True) def call_patch_federal_agency_info(self, mock_prompt): """Calls the patch_federal_agency_info command and mimics a keypress""" - call_command( - "patch_federal_agency_info", - "registrar/tests/data/fake_current_full.csv", - debug=True - ) + call_command("patch_federal_agency_info", "registrar/tests/data/fake_current_full.csv", debug=True) def test_patch_agency_info(self): """ @@ -83,6 +79,32 @@ class TestPatchAgencyInfo(TestCase): # Check that the federal_agency field was not updated self.assertIsNone(self.domain_info.federal_agency) + def test_patch_agency_info_skip_updates_data(self): + """ + Tests that the `patch_federal_agency_info` command logs a warning but + updates the DomainInformation object, because an record exists in the + provided current-full.csv file. + """ + # Set federal_agency to None to simulate a skip + self.transition_domain.federal_agency = None + self.transition_domain.save() + + # Change the domain name to something parsable in the .csv + self.domain.name = "cdomain1.gov" + self.domain.save() + + with self.assertLogs("registrar.management.commands.patch_federal_agency_info", level="WARNING") as context: + self.call_patch_federal_agency_info() + + # Check that the correct log message was output + self.assertIn("SOME AGENCY DATA WAS NONE", context.output[0]) + + # Reload the domain_info object from the database + self.domain_info.refresh_from_db() + + # Check that the federal_agency field was not updated + self.assertEqual(self.domain_info.federal_agency, "World War I Centennial Commission") + def test_patch_agency_info_skips_valid_domains(self): """ Tests that the `patch_federal_agency_info` command logs INFO and From 19f6e3ef8be41c784099dee1765512a691f06f42 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 2 Jan 2024 14:32:24 -0700 Subject: [PATCH 10/29] Fix minor logic error --- src/registrar/management/commands/patch_federal_agency_info.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/management/commands/patch_federal_agency_info.py b/src/registrar/management/commands/patch_federal_agency_info.py index c6585089d..e108cd14c 100644 --- a/src/registrar/management/commands/patch_federal_agency_info.py +++ b/src/registrar/management/commands/patch_federal_agency_info.py @@ -136,7 +136,7 @@ class Command(BaseCommand): self.log_script_run_summary(debug) # Tracks if this script was successful. If any errors are found, something went very wrong. - was_success = len(self.di_failed_to_update) != 0 + was_success = len(self.di_failed_to_update) == 0 return was_success def process_skipped_records(self, file_path, seperator, debug): From 3e91f9790cf9e225b8924aa5e53b4fd59fd63c44 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 2 Jan 2024 14:57:25 -0700 Subject: [PATCH 11/29] Add fix for security email --- src/registrar/utility/csv_export.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 64136c3a5..06d5d8186 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -19,6 +19,16 @@ def export_domains_to_writer(writer, columns, sort_fields, filter_condition): first_name = domainInfo.authorizing_official.first_name or "" last_name = domainInfo.authorizing_official.last_name or "" ao = first_name + " " + last_name + + security_email = " " + if security_contacts: + security_email = security_contacts[0].email + + # These are default emails that should not be displayed in the csv report + disallowed_emails = ["registrar@dotgov.gov"] + if security_email and security_email.lower() in disallowed_emails: + security_email = "(blank)" + # create a dictionary of fields which can be included in output FIELDS = { "Domain name": domainInfo.domain.name, @@ -31,7 +41,7 @@ def export_domains_to_writer(writer, columns, sort_fields, filter_condition): "State": domainInfo.state_territory, "AO": ao, "AO email": domainInfo.authorizing_official.email if domainInfo.authorizing_official else " ", - "Security Contact Email": security_contacts[0].email if security_contacts else " ", + "Security Contact Email": security_email, "Status": domainInfo.domain.state, "Expiration Date": domainInfo.domain.expiration_date, } From 3efd81130d073df8aa9cb89e4eeb84ff63211fd7 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 2 Jan 2024 15:26:44 -0700 Subject: [PATCH 12/29] Update csv_export.py --- src/registrar/utility/csv_export.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 06d5d8186..821a7e76b 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -25,8 +25,7 @@ def export_domains_to_writer(writer, columns, sort_fields, filter_condition): security_email = security_contacts[0].email # These are default emails that should not be displayed in the csv report - disallowed_emails = ["registrar@dotgov.gov"] - if security_email and security_email.lower() in disallowed_emails: + if security_email is not None and security_email.lower() == "registrar@dotgov.gov": security_email = "(blank)" # create a dictionary of fields which can be included in output From f8c33838403571d4cf6368e2c79ddc258d7b3b64 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Jan 2024 08:41:56 -0700 Subject: [PATCH 13/29] Simplify logic --- .../management/commands/patch_federal_agency_info.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/registrar/management/commands/patch_federal_agency_info.py b/src/registrar/management/commands/patch_federal_agency_info.py index e108cd14c..5148fd426 100644 --- a/src/registrar/management/commands/patch_federal_agency_info.py +++ b/src/registrar/management/commands/patch_federal_agency_info.py @@ -56,7 +56,7 @@ class Command(BaseCommand): # Clear the old skipped list, and log the run summary self.di_skipped.clear() self.log_script_run_summary(debug) - else: + elif not was_success: # This code should never execute. This can only occur if bulk_update somehow fails, # which may indicate some sort of data corruption. logger.error( @@ -107,11 +107,11 @@ class Command(BaseCommand): log_message = None # If agency exists on a TransitionDomain, update the related DomainInformation object - if domain_name in td_dict and federal_agency is not None: + if domain_name in td_dict: di.federal_agency = federal_agency self.di_to_update.append(di) log_message = f"{TerminalColors.OKCYAN}Updated {di}{TerminalColors.ENDC}" - elif domain_name not in td_dict: + else: self.di_skipped.append(di) log_message = f"{TerminalColors.YELLOW}Skipping update for {di}{TerminalColors.ENDC}" @@ -128,7 +128,7 @@ class Command(BaseCommand): # After the update has happened, do a sweep of what we get back. # If the fields we expect to update are still None, then something is wrong. for di in corrected_domains: - if domain_name in td_dict and td_dict.get(domain_name) is None: + if domain_name in td_dict and td_dict.get(domain_name) is not None and di.federal_agency is None: logger.info(f"{TerminalColors.FAIL}Failed to update {di}{TerminalColors.ENDC}") self.di_failed_to_update.append(di) From 22450ed96468aef431bf595e127e2d07744f0bd4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Jan 2024 08:51:22 -0700 Subject: [PATCH 14/29] Update patch_federal_agency_info.py --- src/registrar/management/commands/patch_federal_agency_info.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/management/commands/patch_federal_agency_info.py b/src/registrar/management/commands/patch_federal_agency_info.py index 5148fd426..2348b8759 100644 --- a/src/registrar/management/commands/patch_federal_agency_info.py +++ b/src/registrar/management/commands/patch_federal_agency_info.py @@ -128,7 +128,7 @@ class Command(BaseCommand): # After the update has happened, do a sweep of what we get back. # If the fields we expect to update are still None, then something is wrong. for di in corrected_domains: - if domain_name in td_dict and td_dict.get(domain_name) is not None and di.federal_agency is None: + if di not in self.di_skipped and di.federal_agency is None: logger.info(f"{TerminalColors.FAIL}Failed to update {di}{TerminalColors.ENDC}") self.di_failed_to_update.append(di) From ac4dbd1a64931067f9ddd99f5983fb3fda226fd8 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Jan 2024 09:43:59 -0700 Subject: [PATCH 15/29] Add some documentation --- docs/operations/data_migration.md | 34 +++++++++++++++++++ .../commands/patch_federal_agency_info.py | 4 +-- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index a45e27982..dd55fc1b6 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -524,3 +524,37 @@ Example: `cf ssh getgov-za` | 2 | **debug** | Increases logging detail. Defaults to False. | | 3 | **limitParse** | Determines how many domains to parse. Defaults to all. | | 4 | **disableIdempotentCheck** | Boolean that determines if we should check for idempotence or not. Compares the proposed extension date to the value in TransitionDomains. Defaults to False. | + + +## Patch Federal Agency Info +This section outlines how to use `patch_federal_agency_info.py` + +### Running on sandboxes + +#### Step 1: Grab the latest `current-full.csv` file from the dotgov-data repo +Download the csv from [here](https://github.com/cisagov/dotgov-data/blob/main/current-full.csv) and place this file under the `src/migrationdata/` directory. + +#### Step 2: Transfer the `current-full.csv` file to your sandbox +[Click here to go to the section about transferring data to sandboxes](#step-1-transfer-data-to-sandboxes) + +#### Step 3: Login to CloudFoundry +```cf login -a api.fr.cloud.gov --sso``` + +#### Step 4: SSH into your environment +```cf ssh getgov-{space}``` + +Example: `cf ssh getgov-za` + +#### Step 5: Create a shell instance +```/tmp/lifecycle/shell``` + +#### Step 6: Patch agency info +```./manage.py patch_federal_agency_info migrationdata/current-full.csv --debug``` + +### Running locally +```docker-compose exec app ./manage.py patch_federal_agency_info migrationdata/current-full.csv --debug``` + +##### Optional parameters +| | Parameter | Description | +|:-:|:-------------------------- |:----------------------------------------------------------------------------| +| 1 | **debug** | Increases logging detail. Defaults to False. | diff --git a/src/registrar/management/commands/patch_federal_agency_info.py b/src/registrar/management/commands/patch_federal_agency_info.py index 2348b8759..b7f4bc4d0 100644 --- a/src/registrar/management/commands/patch_federal_agency_info.py +++ b/src/registrar/management/commands/patch_federal_agency_info.py @@ -134,7 +134,6 @@ class Command(BaseCommand): # === Log results and return data === # self.log_script_run_summary(debug) - # Tracks if this script was successful. If any errors are found, something went very wrong. was_success = len(self.di_failed_to_update) == 0 return was_success @@ -143,8 +142,7 @@ class Command(BaseCommand): """If we encounter any DomainInformation records that do not have data in the associated TransitionDomain record, then check the associated current-full.csv file for this information.""" - self.di_to_update.clear() - self.di_failed_to_update.clear() + # Code execution will stop here if the user prompts "N" TerminalHelper.prompt_for_execution( system_exit_on_terminate=True, From bf2dfccac562c9b07c55b1d5c6b7ad136452fca3 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Jan 2024 10:30:06 -0700 Subject: [PATCH 16/29] Exclude default emails from current-csv, add unit test --- src/registrar/tests/common.py | 22 ++++++++ src/registrar/tests/test_reports.py | 80 ++++++++++++++++++++++++++++- src/registrar/utility/csv_export.py | 3 +- 3 files changed, 102 insertions(+), 3 deletions(-) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 5efabdb47..caae195ac 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -743,6 +743,25 @@ class MockEppLib(TestCase): ], ) + mockVerisignDataInfoContact = mockDataInfoDomain.dummyInfoContactResultData( + "defaultVeri", "registrar@dotgov.gov", datetime.datetime(2023, 5, 25, 19, 45, 35), "lastPw" + ) + InfoDomainWithVerisignSecurityContact = fakedEppObject( + "fakepw", + cr_date=datetime.datetime(2023, 5, 25, 19, 45, 35), + contacts=[ + common.DomainContact( + contact="defaultVeri", + type=PublicContact.ContactTypeChoices.SECURITY, + ) + ], + hosts=["fake.host.com"], + statuses=[ + common.Status(state="serverTransferProhibited", description="", lang="en"), + common.Status(state="inactive", description="", lang="en"), + ], + ) + InfoDomainWithDefaultTechnicalContact = fakedEppObject( "fakepw", cr_date=datetime.datetime(2023, 5, 25, 19, 45, 35), @@ -1058,6 +1077,7 @@ class MockEppLib(TestCase): "freeman.gov": (self.InfoDomainWithContacts, None), "threenameserversDomain.gov": (self.infoDomainThreeHosts, None), "defaultsecurity.gov": (self.InfoDomainWithDefaultSecurityContact, None), + "adomain2.gov": (self.InfoDomainWithVerisignSecurityContact, None), "defaulttechnical.gov": (self.InfoDomainWithDefaultTechnicalContact, None), "justnameserver.com": (self.justNameserver, None), } @@ -1087,6 +1107,8 @@ class MockEppLib(TestCase): mocked_result = self.mockDefaultSecurityContact case "defaultTech": mocked_result = self.mockDefaultTechnicalContact + case "defaultVeri": + mocked_result = self.mockVerisignDataInfoContact case _: # Default contact return mocked_result = self.mockDataInfoContact diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 4b854a0a0..938f4ed54 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -4,8 +4,10 @@ from django.test import Client, RequestFactory, TestCase from io import StringIO from registrar.models.domain_information import DomainInformation from registrar.models.domain import Domain +from registrar.models.public_contact import PublicContact from registrar.models.user import User from django.contrib.auth import get_user_model +from registrar.tests.common import MockEppLib from registrar.utility.csv_export import export_domains_to_writer from django.core.management import call_command from unittest.mock import MagicMock, call, mock_open, patch @@ -215,8 +217,9 @@ class CsvReportsTest(TestCase): self.assertEqual(expected_file_content, response.content) -class ExportDataTest(TestCase): +class ExportDataTest(MockEppLib): def setUp(self): + super().setUp() username = "test_user" first_name = "First" last_name = "Last" @@ -229,7 +232,6 @@ class ExportDataTest(TestCase): self.domain_2, _ = Domain.objects.get_or_create(name="adomain2.gov", state=Domain.State.DNS_NEEDED) self.domain_3, _ = Domain.objects.get_or_create(name="ddomain3.gov", state=Domain.State.ON_HOLD) self.domain_4, _ = Domain.objects.get_or_create(name="bdomain4.gov", state=Domain.State.UNKNOWN) - self.domain_4, _ = Domain.objects.get_or_create(name="bdomain4.gov", state=Domain.State.UNKNOWN) self.domain_information_1, _ = DomainInformation.objects.get_or_create( creator=self.user, @@ -257,6 +259,7 @@ class ExportDataTest(TestCase): ) def tearDown(self): + PublicContact.objects.all().delete() Domain.objects.all().delete() DomainInformation.objects.all().delete() User.objects.all().delete() @@ -323,6 +326,79 @@ class ExportDataTest(TestCase): self.assertEqual(csv_content, expected_content) + def test_export_domains_to_writer_security_emails(self): + """Test that export_domains_to_writer returns the + expected security email""" + # Create a CSV file in memory + csv_file = StringIO() + writer = csv.writer(csv_file) + + # Define columns, sort fields, and filter condition + columns = [ + "Domain name", + "Domain type", + "Agency", + "Organization name", + "City", + "State", + "AO", + "AO email", + "Submitter", + "Submitter title", + "Submitter email", + "Submitter phone", + "Security Contact Email", + "Status", + ] + sort_fields = ["domain__name"] + filter_condition = { + "domain__state__in": [ + Domain.State.READY, + Domain.State.DNS_NEEDED, + Domain.State.ON_HOLD, + ], + } + + # Add security email information + self.domain_1.name = "defaultsecurity.gov" + self.domain_1.save() + # Invoke setter + self.domain_1.security_contact + + # Invoke setter + self.domain_2.security_contact + + # Invoke setter + self.domain_3.security_contact + + # Call the export function + export_domains_to_writer(writer, columns, sort_fields, filter_condition) + + # Reset the CSV file's position to the beginning + csv_file.seek(0) + + # Read the content into a variable + csv_content = csv_file.read() + + # We expect READY domains, + # sorted alphabetially by domain name + expected_content = ( + "Domain name,Domain type,Agency,Organization name,City,State,AO," + "AO email,Submitter,Submitter title,Submitter email,Submitter phone," + "Security Contact Email,Status\n" + "adomain2.gov,Interstate,(blank),dnsneeded\n" + "ddomain3.gov,Federal,Armed Forces Retirement Home,123@mail.gov,onhold\n" + "defaultsecurity.gov,Federal - Executive,World War I Centennial Commission,(blank),ready\n" + ) + print(csv_content) + + # Normalize line endings and remove commas, + # spaces and leading/trailing whitespace + csv_content = csv_content.replace(",,", "").replace(",", "").replace(" ", "").replace("\r\n", "\n").strip() + expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip() + + self.assertEqual(csv_content, expected_content) + def test_export_domains_to_writer_additional(self): """An additional test for filters and multi-column sort""" # Create a CSV file in memory diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 821a7e76b..8e9e40b2a 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -24,8 +24,9 @@ def export_domains_to_writer(writer, columns, sort_fields, filter_condition): if security_contacts: security_email = security_contacts[0].email + invalid_emails = {"registrar@dotgov.gov", "dotgov@cisa.dhs.gov"} # These are default emails that should not be displayed in the csv report - if security_email is not None and security_email.lower() == "registrar@dotgov.gov": + if security_email is not None and security_email.lower() in invalid_emails: security_email = "(blank)" # create a dictionary of fields which can be included in output From 07690da7915970e5d24ea90c72f5c70df8151e69 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Jan 2024 10:52:15 -0700 Subject: [PATCH 17/29] Update test_reports.py --- src/registrar/tests/test_reports.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index e825ec108..90728058c 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -444,8 +444,9 @@ class ExportDataTest(MockEppLib): # Invoke setter self.domain_3.security_contact - # Call the export function - export_domains_to_writer(writer, columns, sort_fields, filter_condition) + # Call the export functions + write_header(writer, columns) + write_body(writer, columns, sort_fields, filter_condition) # Reset the CSV file's position to the beginning csv_file.seek(0) From e9af48593d8379c5790b29538a8d90f0d6058759 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Jan 2024 10:59:42 -0700 Subject: [PATCH 18/29] Remove bad unit test --- src/registrar/tests/test_reports.py | 74 ----------------------------- 1 file changed, 74 deletions(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 90728058c..8025c6403 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -399,80 +399,6 @@ class ExportDataTest(MockEppLib): self.assertEqual(csv_content, expected_content) - def test_export_domains_to_writer_security_emails(self): - """Test that export_domains_to_writer returns the - expected security email""" - # Create a CSV file in memory - csv_file = StringIO() - writer = csv.writer(csv_file) - - # Define columns, sort fields, and filter condition - columns = [ - "Domain name", - "Domain type", - "Agency", - "Organization name", - "City", - "State", - "AO", - "AO email", - "Submitter", - "Submitter title", - "Submitter email", - "Submitter phone", - "Security Contact Email", - "Status", - ] - sort_fields = ["domain__name"] - filter_condition = { - "domain__state__in": [ - Domain.State.READY, - Domain.State.DNS_NEEDED, - Domain.State.ON_HOLD, - ], - } - - # Add security email information - self.domain_1.name = "defaultsecurity.gov" - self.domain_1.save() - # Invoke setter - self.domain_1.security_contact - - # Invoke setter - self.domain_2.security_contact - - # Invoke setter - self.domain_3.security_contact - - # Call the export functions - write_header(writer, columns) - write_body(writer, columns, sort_fields, filter_condition) - - # Reset the CSV file's position to the beginning - csv_file.seek(0) - - # Read the content into a variable - csv_content = csv_file.read() - - # We expect READY domains, - # sorted alphabetially by domain name - expected_content = ( - "Domain name,Domain type,Agency,Organization name,City,State,AO," - "AO email,Submitter,Submitter title,Submitter email,Submitter phone," - "Security Contact Email,Status\n" - "adomain2.gov,Interstate,(blank),dnsneeded\n" - "ddomain3.gov,Federal,Armed Forces Retirement Home,123@mail.gov,onhold\n" - "defaultsecurity.gov,Federal - Executive,World War I Centennial Commission,(blank),ready\n" - ) - print(csv_content) - - # Normalize line endings and remove commas, - # spaces and leading/trailing whitespace - csv_content = csv_content.replace(",,", "").replace(",", "").replace(" ", "").replace("\r\n", "\n").strip() - expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip() - - self.assertEqual(csv_content, expected_content) - def test_write_body_additional(self): """An additional test for filters and multi-column sort""" # Create a CSV file in memory From 2e01eb78150ff134447dcd24d6d8a9725a637472 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 3 Jan 2024 12:45:23 -0700 Subject: [PATCH 19/29] Linting --- src/registrar/utility/csv_export.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index e3de75dec..af6800c4b 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -32,7 +32,7 @@ def write_row(writer, columns, domain_info): first_name = domain_info.authorizing_official.first_name or "" last_name = domain_info.authorizing_official.last_name or "" ao = first_name + " " + last_name - + security_email = " " if security_contacts: security_email = security_contacts[0].email From 4acfe307b7989213975ec34023919baf0e7cf534 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 10 Jan 2024 08:03:43 -0700 Subject: [PATCH 20/29] Update src/registrar/management/commands/patch_federal_agency_info.py Co-authored-by: rachidatecs <107004823+rachidatecs@users.noreply.github.com> --- src/registrar/management/commands/patch_federal_agency_info.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/management/commands/patch_federal_agency_info.py b/src/registrar/management/commands/patch_federal_agency_info.py index b7f4bc4d0..0d929efcc 100644 --- a/src/registrar/management/commands/patch_federal_agency_info.py +++ b/src/registrar/management/commands/patch_federal_agency_info.py @@ -152,7 +152,7 @@ class Command(BaseCommand): ==Proposed Changes== Number of DomainInformation objects to change: {len(self.di_skipped)} - The following DomainInformation objects will be modified: {self.di_skipped} + The following DomainInformation objects will be modified if agency data exists in file: {self.di_skipped} """, prompt_title="Do you wish to patch skipped records?", ) From 9193a91e713a3541d6c0d45cda9ef010613489ab Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 10 Jan 2024 08:03:55 -0700 Subject: [PATCH 21/29] Update src/registrar/management/commands/patch_federal_agency_info.py Co-authored-by: rachidatecs <107004823+rachidatecs@users.noreply.github.com> --- src/registrar/management/commands/patch_federal_agency_info.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/management/commands/patch_federal_agency_info.py b/src/registrar/management/commands/patch_federal_agency_info.py index 0d929efcc..72d127e28 100644 --- a/src/registrar/management/commands/patch_federal_agency_info.py +++ b/src/registrar/management/commands/patch_federal_agency_info.py @@ -51,7 +51,7 @@ class Command(BaseCommand): if len(self.di_skipped) > 0 and was_success: # Flush out the list of DomainInformations to update self.di_to_update.clear() - self.process_skipped_records(current_full_filepath, seperator, debug) + self.process_skipped_records(current_full_filepath, separator, debug) # Clear the old skipped list, and log the run summary self.di_skipped.clear() From 51a71caeaa7b092fa289882e987bc4525d1d460b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 10 Jan 2024 08:04:03 -0700 Subject: [PATCH 22/29] Update src/registrar/management/commands/patch_federal_agency_info.py Co-authored-by: rachidatecs <107004823+rachidatecs@users.noreply.github.com> --- src/registrar/management/commands/patch_federal_agency_info.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/management/commands/patch_federal_agency_info.py b/src/registrar/management/commands/patch_federal_agency_info.py index 72d127e28..d8a98ddff 100644 --- a/src/registrar/management/commands/patch_federal_agency_info.py +++ b/src/registrar/management/commands/patch_federal_agency_info.py @@ -36,7 +36,7 @@ class Command(BaseCommand): def handle(self, current_full_filepath, **kwargs): """Loops through each valid DomainInformation object and updates its agency value""" debug = kwargs.get("debug") - seperator = kwargs.get("sep") + separator = kwargs.get("sep") # Check if the provided file path is valid if not os.path.isfile(current_full_filepath): From 5835791606ae8dc79eef6ef97aabe3e0db060631 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 10 Jan 2024 08:05:33 -0700 Subject: [PATCH 23/29] Update src/registrar/management/commands/patch_federal_agency_info.py Co-authored-by: rachidatecs <107004823+rachidatecs@users.noreply.github.com> --- src/registrar/management/commands/patch_federal_agency_info.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/management/commands/patch_federal_agency_info.py b/src/registrar/management/commands/patch_federal_agency_info.py index d8a98ddff..9c2c731bc 100644 --- a/src/registrar/management/commands/patch_federal_agency_info.py +++ b/src/registrar/management/commands/patch_federal_agency_info.py @@ -138,7 +138,7 @@ class Command(BaseCommand): was_success = len(self.di_failed_to_update) == 0 return was_success - def process_skipped_records(self, file_path, seperator, debug): + def process_skipped_records(self, file_path, separator, debug): """If we encounter any DomainInformation records that do not have data in the associated TransitionDomain record, then check the associated current-full.csv file for this information.""" From d9348973238fc838e8e99e425225706913c43286 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 10 Jan 2024 08:26:23 -0700 Subject: [PATCH 24/29] PR suggestions --- .../management/commands/patch_federal_agency_info.py | 8 ++++---- src/registrar/tests/test_transition_domain_migrations.py | 4 ++++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/registrar/management/commands/patch_federal_agency_info.py b/src/registrar/management/commands/patch_federal_agency_info.py index 9c2c731bc..a04e59a55 100644 --- a/src/registrar/management/commands/patch_federal_agency_info.py +++ b/src/registrar/management/commands/patch_federal_agency_info.py @@ -61,7 +61,7 @@ class Command(BaseCommand): # which may indicate some sort of data corruption. logger.error( f"{TerminalColors.FAIL}" - "Could not automatically patch skipped records. " + "Could not automatically patch skipped records. The initial update failed." "An error was encountered when running this script, please inspect the following " f"records for accuracy and completeness: {self.di_failed_to_update}" f"{TerminalColors.ENDC}" @@ -158,7 +158,7 @@ class Command(BaseCommand): ) logger.info("Updating...") - file_data = self.read_current_full(file_path, seperator) + file_data = self.read_current_full(file_path, separator) for di in self.di_skipped: domain_name = di.domain.name row = file_data.get(domain_name) @@ -182,10 +182,10 @@ class Command(BaseCommand): # Bulk update the federal agency field in DomainInformation objects DomainInformation.objects.bulk_update(self.di_to_update, ["federal_agency"]) - def read_current_full(self, file_path, seperator): + def read_current_full(self, file_path, separator): """Reads the current-full.csv file and stores it in a dictionary""" with open(file_path, "r") as requested_file: - old_reader = csv.DictReader(requested_file, delimiter=seperator) + old_reader = csv.DictReader(requested_file, delimiter=separator) # Some variants of current-full.csv have key casing differences for fields # such as "Domain name" or "Domain Name". This corrects that. reader = self.lowercase_fieldnames(old_reader) diff --git a/src/registrar/tests/test_transition_domain_migrations.py b/src/registrar/tests/test_transition_domain_migrations.py index 960ba0480..c0d90bd5c 100644 --- a/src/registrar/tests/test_transition_domain_migrations.py +++ b/src/registrar/tests/test_transition_domain_migrations.py @@ -48,6 +48,10 @@ class TestPatchAgencyInfo(TestCase): of a `DomainInformation` object when the corresponding `TransitionDomain` object has a valid `federal_agency`. """ + + # Ensure that the federal_agency is None + self.assertEqual(self.domain_info.federal_agency, None) + self.call_patch_federal_agency_info() # Reload the domain_info object from the database From 2f5559a72f7845fd934e5b382ac95fe938ae3e72 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 10 Jan 2024 08:40:01 -0700 Subject: [PATCH 25/29] PR suggestions pt. 2 --- src/registrar/tests/test_reports.py | 2 +- src/registrar/tests/test_transition_domain_migrations.py | 2 +- src/registrar/utility/csv_export.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 8025c6403..0e9ef82ef 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -223,7 +223,7 @@ class CsvReportsTest(TestCase): self.assertEqual(expected_file_content, response.content) -class ExportDataTest(MockEppLib): +class ExportDataTest(TestCase): def setUp(self): super().setUp() username = "test_user" diff --git a/src/registrar/tests/test_transition_domain_migrations.py b/src/registrar/tests/test_transition_domain_migrations.py index c0d90bd5c..994f83789 100644 --- a/src/registrar/tests/test_transition_domain_migrations.py +++ b/src/registrar/tests/test_transition_domain_migrations.py @@ -86,7 +86,7 @@ class TestPatchAgencyInfo(TestCase): def test_patch_agency_info_skip_updates_data(self): """ Tests that the `patch_federal_agency_info` command logs a warning but - updates the DomainInformation object, because an record exists in the + updates the DomainInformation object, because a record exists in the provided current-full.csv file. """ # Set federal_agency to None to simulate a skip diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index af6800c4b..026fed4b9 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -37,7 +37,7 @@ def write_row(writer, columns, domain_info): if security_contacts: security_email = security_contacts[0].email - invalid_emails = {"registrar@dotgov.gov", "dotgov@cisa.dhs.gov"} + invalid_emails = {"registrar@dotgov.gov"} # These are default emails that should not be displayed in the csv report if security_email is not None and security_email.lower() in invalid_emails: security_email = "(blank)" From cb7eec6b028773ca3d18441377c7ed64693592c3 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 10 Jan 2024 08:49:20 -0700 Subject: [PATCH 26/29] Linter --- src/registrar/tests/test_reports.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 0e9ef82ef..079f77799 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -7,7 +7,6 @@ from registrar.models.domain import Domain from registrar.models.public_contact import PublicContact from registrar.models.user import User from django.contrib.auth import get_user_model -from registrar.tests.common import MockEppLib from registrar.utility.csv_export import ( write_header, write_body, From d49b42dacf8814caf2219280fb654fdd26a9511f Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 10 Jan 2024 14:24:35 -0700 Subject: [PATCH 27/29] Add back deleted unit test --- src/registrar/tests/test_reports.py | 77 ++++++++++++++++++++++++++++- src/registrar/utility/csv_export.py | 2 + 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 079f77799..e7da76f95 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -7,6 +7,7 @@ from registrar.models.domain import Domain from registrar.models.public_contact import PublicContact from registrar.models.user import User from django.contrib.auth import get_user_model +from registrar.tests.common import MockEppLib from registrar.utility.csv_export import ( write_header, write_body, @@ -222,7 +223,7 @@ class CsvReportsTest(TestCase): self.assertEqual(expected_file_content, response.content) -class ExportDataTest(TestCase): +class ExportDataTest(MockEppLib): def setUp(self): super().setUp() username = "test_user" @@ -335,6 +336,80 @@ class ExportDataTest(TestCase): User.objects.all().delete() super().tearDown() + def test_export_domains_to_writer_security_emails(self): + """Test that export_domains_to_writer returns the + expected security email""" + + # Add security email information + self.domain_1.name = "defaultsecurity.gov" + self.domain_1.save() + + # Invoke setter + self.domain_1.security_contact + + # Invoke setter + self.domain_2.security_contact + + # Invoke setter + self.domain_3.security_contact + + # Create a CSV file in memory + csv_file = StringIO() + writer = csv.writer(csv_file) + + # Define columns, sort fields, and filter condition + columns = [ + "Domain name", + "Domain type", + "Agency", + "Organization name", + "City", + "State", + "AO", + "AO email", + "Security contact email", + "Status", + "Expiration date", + ] + sort_fields = ["domain__name"] + filter_condition = { + "domain__state__in": [ + Domain.State.READY, + Domain.State.DNS_NEEDED, + Domain.State.ON_HOLD, + ], + } + + self.maxDiff = None + # Call the export functions + write_header(writer, columns) + write_body(writer, columns, sort_fields, filter_condition) + + # Reset the CSV file's position to the beginning + csv_file.seek(0) + + # Read the content into a variable + csv_content = csv_file.read() + + # We expect READY domains, + # sorted alphabetially by domain name + expected_content = ( + "Domain name,Domain type,Agency,Organization name,City,State,AO," + "AO email,Security contact email,Status,Expiration date\n" + "adomain10.gov,Federal,Armed Forces Retirement Home,Ready\n" + "adomain2.gov,Interstate,(blank),Dns needed\n" + "ddomain3.gov,Federal,Armed Forces Retirement Home,123@mail.gov,On hold,2023-05-25\n" + "defaultsecurity.gov,Federal - Executive,World War I Centennial Commission,dotgov@cisa.dhs.gov,Ready" + ) + + print(csv_content) + # Normalize line endings and remove commas, + # spaces and leading/trailing whitespace + csv_content = csv_content.replace(",,", "").replace(",", "").replace(" ", "").replace("\r\n", "\n").strip() + expected_content = expected_content.replace(",,", "").replace(",", "").replace(" ", "").strip() + + self.assertEqual(csv_content, expected_content) + def test_write_body(self): """Test that write_body returns the existing domain, test that sort by domain name works, diff --git a/src/registrar/utility/csv_export.py b/src/registrar/utility/csv_export.py index 026fed4b9..52afb218b 100644 --- a/src/registrar/utility/csv_export.py +++ b/src/registrar/utility/csv_export.py @@ -26,6 +26,7 @@ def get_domain_infos(filter_condition, sort_fields): def write_row(writer, columns, domain_info): security_contacts = domain_info.domain.contacts.filter(contact_type=PublicContact.ContactTypeChoices.SECURITY) + # For linter ao = " " if domain_info.authorizing_official: @@ -61,6 +62,7 @@ def write_row(writer, columns, domain_info): "First ready": domain_info.domain.first_ready, "Deleted": domain_info.domain.deleted, } + writer.writerow([FIELDS.get(column, "") for column in columns]) From 2ac2996e5d690ed113eee9574e4c43fa7e6fb0b1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 10 Jan 2024 14:29:13 -0700 Subject: [PATCH 28/29] Make terminal output more readable --- .../management/commands/patch_federal_agency_info.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/registrar/management/commands/patch_federal_agency_info.py b/src/registrar/management/commands/patch_federal_agency_info.py index a04e59a55..35642c1bf 100644 --- a/src/registrar/management/commands/patch_federal_agency_info.py +++ b/src/registrar/management/commands/patch_federal_agency_info.py @@ -86,13 +86,14 @@ class Command(BaseCommand): # Get the domain names from TransitionDomain td_agencies = transition_domains.values_list("domain_name", "federal_agency").distinct() + human_readable_domain_names = list(domain_names) # Code execution will stop here if the user prompts "N" TerminalHelper.prompt_for_execution( system_exit_on_terminate=True, info_to_inspect=f""" ==Proposed Changes== - Number of DomainInformation objects to change: {len(domain_info_to_fix)} - The following DomainInformation objects will be modified: {domain_info_to_fix} + Number of DomainInformation objects to change: {len(human_readable_domain_names)} + The following DomainInformation objects will be modified: {human_readable_domain_names} """, prompt_title="Do you wish to patch federal_agency data?", ) From e595349e758676639e7022016ccbafa9f0960ef3 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 11 Jan 2024 10:49:05 -0700 Subject: [PATCH 29/29] Update test_reports.py --- src/registrar/tests/test_reports.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index e7da76f95..b1c631b3d 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -402,7 +402,6 @@ class ExportDataTest(MockEppLib): "defaultsecurity.gov,Federal - Executive,World War I Centennial Commission,dotgov@cisa.dhs.gov,Ready" ) - print(csv_content) # Normalize line endings and remove commas, # spaces and leading/trailing whitespace csv_content = csv_content.replace(",,", "").replace(",", "").replace(" ", "").replace("\r\n", "\n").strip()