From c20f124a2ed1dca3ae115c43e197418c741aee7c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 2 Apr 2024 12:01:07 -0600 Subject: [PATCH 01/23] Basic script --- .../commands/populate_organization_type.py | 123 ++++++++++++++++++ .../commands/utility/terminal_helper.py | 11 +- 2 files changed, 130 insertions(+), 4 deletions(-) create mode 100644 src/registrar/management/commands/populate_organization_type.py diff --git a/src/registrar/management/commands/populate_organization_type.py b/src/registrar/management/commands/populate_organization_type.py new file mode 100644 index 000000000..a6d32358e --- /dev/null +++ b/src/registrar/management/commands/populate_organization_type.py @@ -0,0 +1,123 @@ +import argparse +import logging +from typing import List +from django.core.management import BaseCommand +from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper, ScriptDataHelper +from registrar.models import DomainInformation, DomainRequest, Domain + +logger = logging.getLogger(__name__) + + +class Command(BaseCommand): + help = "Loops through each valid DomainInformation and DomainRequest object and updates its organization_type value" + + def __init__(self): + super().__init__() + # Get lists for DomainRequest + self.request_to_update: List[DomainRequest] = [] + self.request_failed_to_update: List[DomainRequest] = [] + self.request_skipped: List[DomainRequest] = [] + + # Get lists for DomainInformation + self.di_to_update: List[DomainInformation] = [] + self.di_failed_to_update: List[DomainInformation] = [] + self.di_skipped: List[DomainInformation] = [] + + def add_arguments(self, parser): + """Adds command line arguments""" + parser.add_argument("--debug", action=argparse.BooleanOptionalAction) + parser.add_argument( + "election_office_filename", + help=("A JSON file that holds the location and filenames" "of all the data files used for migrations"), + ) + + def handle(self, **kwargs): + """Loops through each valid Domain object and updates its first_created value""" + debug = kwargs.get("debug") + domain_requests = DomainRequest.objects.filter(organization_type__isnull=True) + + # 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 DomainRequest objects to change: {len(domain_requests)} + + Organization_type data will be added for all of these fields. + """, + prompt_title="Do you wish to process DomainRequest?", + ) + logger.info("Updating DomainRequest(s)...") + + self.update_domain_requests(domain_requests, debug) + + domain_infos = DomainInformation.objects.filter(domain_request__isnull=False, organization_type__isnull=True) + # 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_infos)} + + Organization_type data will be added for all of these fields. + """, + prompt_title="Do you wish to process DomainInformation?", + ) + logger.info("Updating DomainInformation(s)...") + + self.update_domain_informations(domain_infos, debug) + + def update_domain_requests(self, domain_requests, debug): + for request in domain_requests: + try: + # TODO - parse data from hfile ere + if request.generic_org_type is not None: + request.is_election_board = True + self.request_to_update.append(request) + if debug: + logger.info(f"Updating {request}") + else: + self.request_skipped.append(request) + if debug: + logger.warning(f"Skipped updating {request}") + except Exception as err: + self.request_failed_to_update.append(request) + logger.error(err) + logger.error(f"{TerminalColors.FAIL}" f"Failed to update {request}" f"{TerminalColors.ENDC}") + + # Do a bulk update on the organization_type field + ScriptDataHelper.bulk_update_fields(DomainRequest, self.request_to_update, ["organization_type"]) + + # Log what happened + log_header = "============= FINISHED UPDATE FOR DOMAINREQUEST ===============" + TerminalHelper.log_script_run_summary( + self.request_to_update, self.request_failed_to_update, self.request_skipped, debug, log_header + ) + + def update_domain_informations(self, domain_informations, debug): + for info in domain_informations: + try: + # TODO - parse data from hfile ere + if info.generic_org_type is not None: + info.is_election_board = True + self.di_to_update.append(info) + if debug: + logger.info(f"Updating {info}") + else: + self.di_skipped.append(info) + if debug: + logger.warning(f"Skipped updating {info}") + except Exception as err: + self.di_failed_to_update.append(info) + logger.error(err) + logger.error(f"{TerminalColors.FAIL}" f"Failed to update {info}" f"{TerminalColors.ENDC}") + + # Do a bulk update on the organization_type field + ScriptDataHelper.bulk_update_fields(DomainInformation, self.di_to_update, ["organization_type"]) + + # Log what happened + log_header = "============= FINISHED UPDATE FOR DOMAININFORMATION ===============" + TerminalHelper.log_script_run_summary( + self.di_to_update, self.di_failed_to_update, self.di_skipped, debug, log_header + ) + diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index 49ab89b9a..2149f429a 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -59,13 +59,16 @@ def bulk_update_fields(model_class, update_list, fields_to_update, batch_size=10 class TerminalHelper: @staticmethod - def log_script_run_summary(to_update, failed_to_update, skipped, debug: bool): + def log_script_run_summary(to_update, failed_to_update, skipped, debug: bool, log_header=None): """Prints success, failed, and skipped counts, as well as all affected objects.""" update_success_count = len(to_update) update_failed_count = len(failed_to_update) update_skipped_count = len(skipped) + if log_header is None: + log_header = "============= FINISHED ===============" + # Prepare debug messages debug_messages = { "success": (f"{TerminalColors.OKCYAN}Updated: {to_update}{TerminalColors.ENDC}\n"), @@ -85,7 +88,7 @@ def log_script_run_summary(to_update, failed_to_update, skipped, debug: bool): if update_failed_count == 0 and update_skipped_count == 0: logger.info( f"""{TerminalColors.OKGREEN} - ============= FINISHED =============== + {log_header} Updated {update_success_count} entries {TerminalColors.ENDC} """ @@ -93,7 +96,7 @@ def log_script_run_summary(to_update, failed_to_update, skipped, debug: bool): elif update_failed_count == 0: logger.warning( f"""{TerminalColors.YELLOW} - ============= FINISHED =============== + {log_header} Updated {update_success_count} entries ----- SOME DATA WAS INVALID (NEEDS MANUAL PATCHING) ----- Skipped updating {update_skipped_count} entries @@ -103,7 +106,7 @@ def log_script_run_summary(to_update, failed_to_update, skipped, debug: bool): else: logger.error( f"""{TerminalColors.FAIL} - ============= FINISHED =============== + {log_header} Updated {update_success_count} entries ----- UPDATE FAILED ----- Failed to update {update_failed_count} entries, From 3b0fa34ee22c017b44d3616214fe6a9ca1e8edfc Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 4 Apr 2024 14:37:45 -0600 Subject: [PATCH 02/23] Update script --- src/registrar/admin.py | 2 + .../commands/populate_organization_type.py | 106 +++++++++++------- 2 files changed, 69 insertions(+), 39 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index e4c71f8d5..2bcc604ff 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -883,6 +883,8 @@ class DomainInformationAdmin(ListHeaderAdmin): "Type of organization", { "fields": [ + "is_election_board", + "generic_org_type", "organization_type", ] }, diff --git a/src/registrar/management/commands/populate_organization_type.py b/src/registrar/management/commands/populate_organization_type.py index a6d32358e..d79bf613f 100644 --- a/src/registrar/management/commands/populate_organization_type.py +++ b/src/registrar/management/commands/populate_organization_type.py @@ -1,10 +1,12 @@ import argparse import logging +import os +from registrar.signals import create_or_update_organization_type from typing import List from django.core.management import BaseCommand from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper, ScriptDataHelper from registrar.models import DomainInformation, DomainRequest, Domain - +from django.db import transaction logger = logging.getLogger(__name__) @@ -23,18 +25,37 @@ def __init__(self): self.di_failed_to_update: List[DomainInformation] = [] self.di_skipped: List[DomainInformation] = [] + # Define a global variable for all domains with election offices + self.domains_with_election_offices_set = set() + def add_arguments(self, parser): """Adds command line arguments""" parser.add_argument("--debug", action=argparse.BooleanOptionalAction) parser.add_argument( - "election_office_filename", + "domain_election_office_filename", help=("A JSON file that holds the location and filenames" "of all the data files used for migrations"), ) - def handle(self, **kwargs): + def handle(self, domain_election_office_filename, **kwargs): """Loops through each valid Domain object and updates its first_created value""" debug = kwargs.get("debug") - domain_requests = DomainRequest.objects.filter(organization_type__isnull=True) + + # Check if the provided file path is valid + if not os.path.isfile(domain_election_office_filename): + raise argparse.ArgumentTypeError(f"Invalid file path '{domain_election_office_filename}'") + + + with open(domain_election_office_filename, "r") as file: + for line in file: + # Remove any leading/trailing whitespace + domain = line.strip() + if domain not in self.domains_with_election_offices_set: + self.domains_with_election_offices_set.add(domain) + + domain_requests = DomainRequest.objects.filter( + organization_type__isnull=True, + requested_domain__name__isnull=False + ) # Code execution will stop here if the user prompts "N" TerminalHelper.prompt_for_execution( @@ -51,7 +72,9 @@ def handle(self, **kwargs): self.update_domain_requests(domain_requests, debug) - domain_infos = DomainInformation.objects.filter(domain_request__isnull=False, organization_type__isnull=True) + # We should actually be targeting all fields with no value for organization type, + # but do have a value for generic_org_type. This is because there is data that we can infer. + domain_infos = DomainInformation.objects.filter(organization_type__isnull=True) # Code execution will stop here if the user prompts "N" TerminalHelper.prompt_for_execution( system_exit_on_terminate=True, @@ -68,25 +91,28 @@ def handle(self, **kwargs): self.update_domain_informations(domain_infos, debug) def update_domain_requests(self, domain_requests, debug): - for request in domain_requests: - try: - # TODO - parse data from hfile ere - if request.generic_org_type is not None: - request.is_election_board = True - self.request_to_update.append(request) - if debug: - logger.info(f"Updating {request}") - else: - self.request_skipped.append(request) - if debug: - logger.warning(f"Skipped updating {request}") - except Exception as err: - self.request_failed_to_update.append(request) - logger.error(err) - logger.error(f"{TerminalColors.FAIL}" f"Failed to update {request}" f"{TerminalColors.ENDC}") + with transaction.atomic(): + for request in domain_requests: + try: + # TODO - parse data from hfile ere + if request.generic_org_type is not None: + domain_name = request.requested_domain.name + request.is_election_board = domain_name in self.domains_with_election_offices_set + request.save() + self.request_to_update.append(request) + if debug: + logger.info(f"Updated {request} => {request.organization_type}") + else: + self.request_skipped.append(request) + if debug: + logger.warning(f"Skipped updating {request}. No generic_org_type was found.") + except Exception as err: + self.request_failed_to_update.append(request) + logger.error(err) + logger.error(f"{TerminalColors.FAIL}" f"Failed to update {request}" f"{TerminalColors.ENDC}") # Do a bulk update on the organization_type field - ScriptDataHelper.bulk_update_fields(DomainRequest, self.request_to_update, ["organization_type"]) + # ScriptDataHelper.bulk_update_fields(DomainRequest, self.request_to_update, ["is_election_board"]) # Log what happened log_header = "============= FINISHED UPDATE FOR DOMAINREQUEST ===============" @@ -95,25 +121,27 @@ def update_domain_requests(self, domain_requests, debug): ) def update_domain_informations(self, domain_informations, debug): - for info in domain_informations: - try: - # TODO - parse data from hfile ere - if info.generic_org_type is not None: - info.is_election_board = True - self.di_to_update.append(info) - if debug: - logger.info(f"Updating {info}") - else: - self.di_skipped.append(info) - if debug: - logger.warning(f"Skipped updating {info}") - except Exception as err: - self.di_failed_to_update.append(info) - logger.error(err) - logger.error(f"{TerminalColors.FAIL}" f"Failed to update {info}" f"{TerminalColors.ENDC}") + with transaction.atomic(): + for info in domain_informations: + try: + if info.generic_org_type is not None: + domain_name = info.domain.name + info.is_election_board = domain_name in self.domains_with_election_offices_set + info.save() + self.di_to_update.append(info) + if debug: + logger.info(f"Updated {info} => {info.organization_type}") + else: + self.di_skipped.append(info) + if debug: + logger.warning(f"Skipped updating {info}. No generic_org_type was found.") + except Exception as err: + self.di_failed_to_update.append(info) + logger.error(err) + logger.error(f"{TerminalColors.FAIL}" f"Failed to update {info}" f"{TerminalColors.ENDC}") # Do a bulk update on the organization_type field - ScriptDataHelper.bulk_update_fields(DomainInformation, self.di_to_update, ["organization_type"]) + # ScriptDataHelper.bulk_update_fields(DomainInformation, self.di_to_update, ["organization_type", "is_election_board", "generic_org_type"]) # Log what happened log_header = "============= FINISHED UPDATE FOR DOMAININFORMATION ===============" From 755c7a9a561b0dcdf89644f2764da3b82db4eae4 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 4 Apr 2024 14:55:34 -0600 Subject: [PATCH 03/23] Finish script --- .../commands/populate_organization_type.py | 25 ++++++++++--------- src/registrar/signals.py | 15 ++++------- 2 files changed, 18 insertions(+), 22 deletions(-) diff --git a/src/registrar/management/commands/populate_organization_type.py b/src/registrar/management/commands/populate_organization_type.py index d79bf613f..f10b0ae3c 100644 --- a/src/registrar/management/commands/populate_organization_type.py +++ b/src/registrar/management/commands/populate_organization_type.py @@ -7,6 +7,7 @@ from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper, ScriptDataHelper from registrar.models import DomainInformation, DomainRequest, Domain from django.db import transaction + logger = logging.getLogger(__name__) @@ -44,7 +45,6 @@ def handle(self, domain_election_office_filename, **kwargs): if not os.path.isfile(domain_election_office_filename): raise argparse.ArgumentTypeError(f"Invalid file path '{domain_election_office_filename}'") - with open(domain_election_office_filename, "r") as file: for line in file: # Remove any leading/trailing whitespace @@ -53,8 +53,7 @@ def handle(self, domain_election_office_filename, **kwargs): self.domains_with_election_offices_set.add(domain) domain_requests = DomainRequest.objects.filter( - organization_type__isnull=True, - requested_domain__name__isnull=False + organization_type__isnull=True, requested_domain__name__isnull=False ) # Code execution will stop here if the user prompts "N" @@ -94,14 +93,13 @@ def update_domain_requests(self, domain_requests, debug): with transaction.atomic(): for request in domain_requests: try: - # TODO - parse data from hfile ere if request.generic_org_type is not None: domain_name = request.requested_domain.name request.is_election_board = domain_name in self.domains_with_election_offices_set - request.save() + request = create_or_update_organization_type(DomainRequest, request, return_instance=True) self.request_to_update.append(request) if debug: - logger.info(f"Updated {request} => {request.organization_type}") + logger.info(f"Updating {request} => {request.organization_type}") else: self.request_skipped.append(request) if debug: @@ -112,14 +110,16 @@ def update_domain_requests(self, domain_requests, debug): logger.error(f"{TerminalColors.FAIL}" f"Failed to update {request}" f"{TerminalColors.ENDC}") # Do a bulk update on the organization_type field - # ScriptDataHelper.bulk_update_fields(DomainRequest, self.request_to_update, ["is_election_board"]) + ScriptDataHelper.bulk_update_fields( + DomainRequest, self.request_to_update, ["organization_type", "is_election_board", "generic_org_type"] + ) # Log what happened log_header = "============= FINISHED UPDATE FOR DOMAINREQUEST ===============" TerminalHelper.log_script_run_summary( self.request_to_update, self.request_failed_to_update, self.request_skipped, debug, log_header ) - + def update_domain_informations(self, domain_informations, debug): with transaction.atomic(): for info in domain_informations: @@ -127,10 +127,10 @@ def update_domain_informations(self, domain_informations, debug): if info.generic_org_type is not None: domain_name = info.domain.name info.is_election_board = domain_name in self.domains_with_election_offices_set - info.save() + info = create_or_update_organization_type(DomainInformation, info, return_instance=True) self.di_to_update.append(info) if debug: - logger.info(f"Updated {info} => {info.organization_type}") + logger.info(f"Updating {info} => {info.organization_type}") else: self.di_skipped.append(info) if debug: @@ -141,11 +141,12 @@ def update_domain_informations(self, domain_informations, debug): logger.error(f"{TerminalColors.FAIL}" f"Failed to update {info}" f"{TerminalColors.ENDC}") # Do a bulk update on the organization_type field - # ScriptDataHelper.bulk_update_fields(DomainInformation, self.di_to_update, ["organization_type", "is_election_board", "generic_org_type"]) + ScriptDataHelper.bulk_update_fields( + DomainInformation, self.di_to_update, ["organization_type", "is_election_board", "generic_org_type"] + ) # Log what happened log_header = "============= FINISHED UPDATE FOR DOMAININFORMATION ===============" TerminalHelper.log_script_run_summary( self.di_to_update, self.di_failed_to_update, self.di_skipped, debug, log_header ) - diff --git a/src/registrar/signals.py b/src/registrar/signals.py index ad287219d..2e1e9ea1b 100644 --- a/src/registrar/signals.py +++ b/src/registrar/signals.py @@ -11,7 +11,7 @@ @receiver(pre_save, sender=DomainRequest) @receiver(pre_save, sender=DomainInformation) -def create_or_update_organization_type(sender, instance, **kwargs): +def create_or_update_organization_type(sender, instance, return_instance=False, **kwargs): """The organization_type field on DomainRequest and DomainInformation is consituted from the generic_org_type and is_election_board fields. To keep the organization_type field up to date, we need to update it before save based off of those field @@ -62,15 +62,7 @@ def create_or_update_organization_type(sender, instance, **kwargs): # == Init variables == # # Instance is already in the database, fetch its current state - if isinstance(instance, DomainRequest): - current_instance = DomainRequest.objects.get(id=instance.id) - elif isinstance(instance, DomainInformation): - current_instance = DomainInformation.objects.get(id=instance.id) - else: - # This should never occur. But it never hurts to have this check anyway. - raise ValueError( - "create_or_update_organization_type() -> instance was not DomainRequest or DomainInformation" - ) + current_instance = sender.objects.get(id=instance.id) # Check the new and old values generic_org_type_changed = instance.generic_org_type != current_instance.generic_org_type @@ -100,6 +92,9 @@ def create_or_update_organization_type(sender, instance, **kwargs): _update_generic_org_and_election_from_org_type( instance, election_org_to_generic_org_map, generic_org_to_org_map ) + + if return_instance: + return instance def _update_org_type_from_generic_org_and_election(instance, org_map): From ebee04e5012de5685be148cdab42460be771081d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 5 Apr 2024 08:59:27 -0600 Subject: [PATCH 04/23] Update test_reports.py --- src/registrar/tests/test_reports.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 6299349c5..5b2569d05 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -718,7 +718,7 @@ def test_get_sliced_domains(self): } # Test with distinct managed_domains_sliced_at_end_date = get_sliced_domains(filter_condition) - expected_content = [3, 2, 1, 0, 0, 0, 0, 0, 0, 0] + expected_content = [3, 4, 1, 0, 0, 0, 0, 0, 0, 0] self.assertEqual(managed_domains_sliced_at_end_date, expected_content) # Test without distinct From 17c231a22a12d83a448fea3bcb1de5fb6c99aa8d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 5 Apr 2024 10:30:55 -0600 Subject: [PATCH 05/23] Add tests --- .../tests/data/fake_election_domains.csv | 1 + .../tests/test_management_scripts.py | 223 +++++++++++++++++- 2 files changed, 223 insertions(+), 1 deletion(-) create mode 100644 src/registrar/tests/data/fake_election_domains.csv diff --git a/src/registrar/tests/data/fake_election_domains.csv b/src/registrar/tests/data/fake_election_domains.csv new file mode 100644 index 000000000..4ec005bb1 --- /dev/null +++ b/src/registrar/tests/data/fake_election_domains.csv @@ -0,0 +1 @@ +manualtransmission.gov \ No newline at end of file diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 34178e262..303c3dfd6 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1,5 +1,6 @@ import copy from datetime import date, datetime, time +from unittest import skip from django.utils import timezone from django.test import TestCase @@ -7,6 +8,9 @@ from registrar.models import ( User, Domain, + DomainRequest, + Contact, + Website, DomainInvitation, TransitionDomain, DomainInformation, @@ -18,7 +22,224 @@ from unittest.mock import patch, call from epplibwrapper import commands, common -from .common import MockEppLib, less_console_noise +from .common import MockEppLib, less_console_noise, completed_domain_request +from api.tests.common import less_console_noise_decorator + +class TestPopulateOrganizationType(MockEppLib): + """Tests for the populate_organization_type script""" + + def setUp(self): + """Creates a fake domain object""" + super().setUp() + + # Get the domain requests + self.domain_request_1 = completed_domain_request( + name="lasers.gov", + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + is_election_board=True, + ) + self.domain_request_2 = completed_domain_request( + name="readysetgo.gov", + generic_org_type=DomainRequest.OrganizationChoices.CITY, + ) + self.domain_request_3 = completed_domain_request( + name="manualtransmission.gov", + generic_org_type=DomainRequest.OrganizationChoices.TRIBAL, + ) + self.domain_request_4 = completed_domain_request( + name="saladandfries.gov", + generic_org_type=DomainRequest.OrganizationChoices.TRIBAL, + is_election_board=True, + ) + + # Approve all three requests + self.domain_request_1.approve() + self.domain_request_2.approve() + self.domain_request_3.approve() + self.domain_request_4.approve() + + # Get the domains + self.domain_1 = Domain.objects.get(name="lasers.gov") + self.domain_2 = Domain.objects.get(name="readysetgo.gov") + self.domain_3 = Domain.objects.get(name="manualtransmission.gov") + self.domain_4 = Domain.objects.get(name="saladandfries.gov") + + # Get the domain infos + self.domain_info_1 = DomainInformation.objects.get(domain=self.domain_1) + self.domain_info_2 = DomainInformation.objects.get(domain=self.domain_2) + self.domain_info_3 = DomainInformation.objects.get(domain=self.domain_3) + self.domain_info_4 = DomainInformation.objects.get(domain=self.domain_4) + + def tearDown(self): + """Deletes all DB objects related to migrations""" + super().tearDown() + + # Delete domains and related information + Domain.objects.all().delete() + DomainInformation.objects.all().delete() + DomainRequest.objects.all().delete() + User.objects.all().delete() + Contact.objects.all().delete() + Website.objects.all().delete() + + @less_console_noise_decorator + def run_populate_organization_type(self): + """ + This method executes the populate_organization_type command. + + The 'call_command' function from Django's management framework is then used to + execute the populate_organization_type command with the specified arguments. + """ + with patch( + "registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit", # noqa + return_value=True, + ): + call_command("populate_organization_type", "registrar/tests/data/fake_election_domains.csv", debug=True) + + def assert_expected_org_values_on_request_and_info( + self, + domain_request: DomainRequest, + domain_info: DomainInformation, + expected_values: dict, + ): + """ + This is a a helper function that ensures that: + 1. DomainRequest and DomainInformation (on given objects) are equivalent + 2. That generic_org_type, is_election_board, and organization_type are equal to passed in values + """ + + # Test domain request + self.assertEqual(domain_request.generic_org_type, expected_values['generic_org_type']) + self.assertEqual(domain_request.is_election_board, expected_values['is_election_board']) + self.assertEqual(domain_request.organization_type, expected_values['organization_type']) + + # Test domain info + self.assertEqual(domain_info.generic_org_type, expected_values['generic_org_type']) + self.assertEqual(domain_info.is_election_board, expected_values['is_election_board']) + self.assertEqual(domain_info.organization_type, expected_values['organization_type']) + + def test_request_and_info_city_not_in_csv(self): + """Tests what happens to a city domain that is not defined in the CSV""" + city_request = self.domain_request_2 + city_info = self.domain_request_2 + + # Make sure that all data is correct before proceeding. + # Since the presave fixture is in effect, we should expect that + # is_election_board is equal to none, even though we tried to define it as "True" + expected_values = { + 'is_election_board': False, + 'generic_org_type': DomainRequest.OrganizationChoices.CITY, + 'organization_type': DomainRequest.OrgChoicesElectionOffice.CITY, + } + self.assert_expected_org_values_on_request_and_info(city_request, city_info, expected_values) + + # Run the populate script + try: + self.run_populate_organization_type() + except Exception as e: + self.fail(f"Could not run populate_organization_type script. Failed with exception: {e}") + + # All values should be the same + self.assert_expected_org_values_on_request_and_info(city_request, city_info, expected_values) + + def test_request_and_info_federal(self): + """Tests what happens to a federal domain with is_election_board=True (which should be reverted to None)""" + federal_request = self.domain_request_1 + federal_info = self.domain_info_1 + + # Make sure that all data is correct before proceeding. + # Since the presave fixture is in effect, we should expect that + # is_election_board is equal to none, even though we tried to define it as "True" + # TODO - either add some logging if this happens or a ValueError in the original ticket. + # Or FSM? Probably FSM. + expected_values = { + 'is_election_board': None, + 'generic_org_type': DomainRequest.OrganizationChoices.FEDERAL, + 'organization_type': DomainRequest.OrgChoicesElectionOffice.FEDERAL, + } + self.assert_expected_org_values_on_request_and_info(federal_request, federal_info, expected_values) + + # Run the populate script + try: + self.run_populate_organization_type() + except Exception as e: + self.fail(f"Could not run populate_organization_type script. Failed with exception: {e}") + + # All values should be the same + self.assert_expected_org_values_on_request_and_info(federal_request, federal_info, expected_values) + + def test_request_and_info_tribal_add_election_office(self): + """ + Tests if a tribal domain in the election csv changes organization_type to TRIBAL - ELECTION + for the domain request and the domain info + """ + + tribal_request = self.domain_request_3 + tribal_info = self.domain_info_3 + + # Make sure that all data is correct before proceeding. + # Because the presave fixture is in place when creating this, we should expect that the + # organization_type variable is already pre-populated. We will test what happens when + # it is not in another test. + + expected_values = { + 'is_election_board': False, + 'generic_org_type': DomainRequest.OrganizationChoices.TRIBAL, + 'organization_type': DomainRequest.OrgChoicesElectionOffice.TRIBAL + } + self.assert_expected_org_values_on_request_and_info(tribal_request, tribal_info, expected_values) + + # Run the populate script + try: + self.run_populate_organization_type() + except Exception as e: + self.fail(f"Could not run populate_organization_type script. Failed with exception: {e}") + + # Because we define this in the "csv", we expect that is election board will switch to True, + # and organization_type will now be tribal_election + expected_values["is_election_board"] = True + expected_values["organization_type"] = DomainRequest.OrgChoicesElectionOffice.TRIBAL_ELECTION + + self.assert_expected_org_values_on_request_and_info(tribal_request, tribal_info, expected_values) + + def test_request_and_info_tribal_remove_election_office(self): + """ + Tests if a tribal domain in the election csv changes organization_type to TRIBAL + when it used to be TRIBAL - ELECTION + for the domain request and the domain info + """ + + tribal_election_request = self.domain_request_4 + tribal_election_info = self.domain_info_4 + + # Make sure that all data is correct before proceeding. + # Because the presave fixture is in place when creating this, we should expect that the + # organization_type variable is already pre-populated. We will test what happens when + # it is not in another test. + expected_values = { + 'is_election_board': True, + 'generic_org_type': DomainRequest.OrganizationChoices.TRIBAL, + 'organization_type': DomainRequest.OrgChoicesElectionOffice.TRIBAL_ELECTION + } + self.assert_expected_org_values_on_request_and_info(tribal_election_request, tribal_election_info, expected_values) + + # Run the populate script + try: + self.run_populate_organization_type() + except Exception as e: + self.fail(f"Could not run populate_organization_type script. Failed with exception: {e}") + + # Because we don't define this in the "csv", we expect that is election board will switch to False, + # and organization_type will now be tribal + expected_values["is_election_board"] = False + expected_values["organization_type"] = DomainRequest.OrgChoicesElectionOffice.TRIBAL + self.assert_expected_org_values_on_request_and_info(tribal_election_request, tribal_election_info, expected_values) + + @skip("TODO") + def test_transition_data(self): + """Tests for how this script interacts with prexisting data (for instance, stable)""" + # Make instead of mocking we can literally just run the transition domain scripts? + pass class TestPopulateFirstReady(TestCase): From 7edc6e75f453e6f01155e9b039f85e7a6b1a5b3d Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Fri, 5 Apr 2024 12:04:42 -0600 Subject: [PATCH 06/23] Unit tests --- .../commands/populate_organization_type.py | 75 ++++++++++--------- src/registrar/signals.py | 13 ++-- .../tests/test_management_scripts.py | 51 ++++++++----- 3 files changed, 77 insertions(+), 62 deletions(-) diff --git a/src/registrar/management/commands/populate_organization_type.py b/src/registrar/management/commands/populate_organization_type.py index f10b0ae3c..ce0dfae9f 100644 --- a/src/registrar/management/commands/populate_organization_type.py +++ b/src/registrar/management/commands/populate_organization_type.py @@ -90,24 +90,30 @@ def handle(self, domain_election_office_filename, **kwargs): self.update_domain_informations(domain_infos, debug) def update_domain_requests(self, domain_requests, debug): - with transaction.atomic(): - for request in domain_requests: - try: - if request.generic_org_type is not None: - domain_name = request.requested_domain.name - request.is_election_board = domain_name in self.domains_with_election_offices_set - request = create_or_update_organization_type(DomainRequest, request, return_instance=True) - self.request_to_update.append(request) - if debug: - logger.info(f"Updating {request} => {request.organization_type}") - else: + for request in domain_requests: + try: + if request.generic_org_type is not None: + domain_name = request.requested_domain.name + request.is_election_board = domain_name in self.domains_with_election_offices_set + new_request = create_or_update_organization_type(DomainRequest, request, return_instance=True) + print(f"what is the new request? {new_request}") + if not new_request: self.request_skipped.append(request) - if debug: - logger.warning(f"Skipped updating {request}. No generic_org_type was found.") - except Exception as err: - self.request_failed_to_update.append(request) - logger.error(err) - logger.error(f"{TerminalColors.FAIL}" f"Failed to update {request}" f"{TerminalColors.ENDC}") + logger.warning(f"Skipped updating {request}. No changes to be made.") + else: + request = new_request + self.request_to_update.append(request) + + if debug: + logger.info(f"Updating {request} => {request.organization_type}") + else: + self.request_skipped.append(request) + if debug: + logger.warning(f"Skipped updating {request}. No generic_org_type was found.") + except Exception as err: + self.request_failed_to_update.append(request) + logger.error(err) + logger.error(f"{TerminalColors.FAIL}" f"Failed to update {request}" f"{TerminalColors.ENDC}") # Do a bulk update on the organization_type field ScriptDataHelper.bulk_update_fields( @@ -121,24 +127,23 @@ def update_domain_requests(self, domain_requests, debug): ) def update_domain_informations(self, domain_informations, debug): - with transaction.atomic(): - for info in domain_informations: - try: - if info.generic_org_type is not None: - domain_name = info.domain.name - info.is_election_board = domain_name in self.domains_with_election_offices_set - info = create_or_update_organization_type(DomainInformation, info, return_instance=True) - self.di_to_update.append(info) - if debug: - logger.info(f"Updating {info} => {info.organization_type}") - else: - self.di_skipped.append(info) - if debug: - logger.warning(f"Skipped updating {info}. No generic_org_type was found.") - except Exception as err: - self.di_failed_to_update.append(info) - logger.error(err) - logger.error(f"{TerminalColors.FAIL}" f"Failed to update {info}" f"{TerminalColors.ENDC}") + for info in domain_informations: + try: + if info.generic_org_type is not None: + domain_name = info.domain.name + info.is_election_board = domain_name in self.domains_with_election_offices_set + info = create_or_update_organization_type(DomainInformation, info, return_instance=True) + self.di_to_update.append(info) + if debug: + logger.info(f"Updating {info} => {info.organization_type}") + else: + self.di_skipped.append(info) + if debug: + logger.warning(f"Skipped updating {info}. No generic_org_type was found.") + except Exception as err: + self.di_failed_to_update.append(info) + logger.error(err) + logger.error(f"{TerminalColors.FAIL}" f"Failed to update {info}" f"{TerminalColors.ENDC}") # Do a bulk update on the organization_type field ScriptDataHelper.bulk_update_fields( diff --git a/src/registrar/signals.py b/src/registrar/signals.py index aab267f5a..a4530b8c2 100644 --- a/src/registrar/signals.py +++ b/src/registrar/signals.py @@ -37,7 +37,6 @@ def create_or_update_organization_type(sender: DomainRequest | DomainInformation # A new record is added with organization_type not defined. # This happens from the regular domain request flow. is_new_instance = instance.id is None - if is_new_instance: # == Check for invalid conditions before proceeding == # @@ -54,7 +53,7 @@ def create_or_update_organization_type(sender: DomainRequest | DomainInformation # related field (generic org type <-> org type) has data and we should update according to that. if organization_type_needs_update: _update_org_type_from_generic_org_and_election(instance, generic_org_to_org_map) - elif generic_org_type_needs_update: + elif generic_org_type_needs_update and instance.organization_type is not None: _update_generic_org_and_election_from_org_type( instance, election_org_to_generic_org_map, generic_org_to_org_map ) @@ -63,12 +62,12 @@ def create_or_update_organization_type(sender: DomainRequest | DomainInformation # == Init variables == # # Instance is already in the database, fetch its current state current_instance = sender.objects.get(id=instance.id) - + print(f"what is the current instance? {current_instance.__dict__}") # Check the new and old values generic_org_type_changed = instance.generic_org_type != current_instance.generic_org_type is_election_board_changed = instance.is_election_board != current_instance.is_election_board organization_type_changed = instance.organization_type != current_instance.organization_type - + print(f"whats changing? generic {generic_org_type_changed} vs election {is_election_board_changed} vs org {organization_type_changed}") # == Check for invalid conditions before proceeding == # if organization_type_changed and (generic_org_type_changed or is_election_board_changed): # Since organization type is linked with generic_org_type and election board, @@ -88,7 +87,7 @@ def create_or_update_organization_type(sender: DomainRequest | DomainInformation # Update the field if organization_type_needs_update: _update_org_type_from_generic_org_and_election(instance, generic_org_to_org_map) - elif generic_org_type_needs_update: + elif generic_org_type_needs_update and instance.organization_type is not None: _update_generic_org_and_election_from_org_type( instance, election_org_to_generic_org_map, generic_org_to_org_map ) @@ -114,13 +113,13 @@ def _update_org_type_from_generic_org_and_election(instance, org_map): logger.warning("create_or_update_organization_type() -> is_election_board is out of sync. Updating value.") instance.is_election_board = False - instance.organization_type = org_map[generic_org_type] if instance.is_election_board else generic_org_type + instance.organization_type = org_map.get(generic_org_type) if instance.is_election_board else generic_org_type def _update_generic_org_and_election_from_org_type(instance, election_org_map, generic_org_map): """Given the field value for organization_type, update the generic_org_type and is_election_board field.""" - + # We convert to a string because the enum types are different # between OrgChoicesElectionOffice and OrganizationChoices. # But their names are the same (for the most part). diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 303c3dfd6..71ce01424 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -37,19 +37,23 @@ def setUp(self): name="lasers.gov", generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, is_election_board=True, + status=DomainRequest.DomainRequestStatus.IN_REVIEW, ) self.domain_request_2 = completed_domain_request( name="readysetgo.gov", generic_org_type=DomainRequest.OrganizationChoices.CITY, + status=DomainRequest.DomainRequestStatus.IN_REVIEW, ) self.domain_request_3 = completed_domain_request( name="manualtransmission.gov", generic_org_type=DomainRequest.OrganizationChoices.TRIBAL, + status=DomainRequest.DomainRequestStatus.IN_REVIEW, ) self.domain_request_4 = completed_domain_request( name="saladandfries.gov", generic_org_type=DomainRequest.OrganizationChoices.TRIBAL, is_election_board=True, + status=DomainRequest.DomainRequestStatus.IN_REVIEW, ) # Approve all three requests @@ -82,7 +86,7 @@ def tearDown(self): Contact.objects.all().delete() Website.objects.all().delete() - @less_console_noise_decorator + #@less_console_noise_decorator def run_populate_organization_type(self): """ This method executes the populate_organization_type command. @@ -109,14 +113,16 @@ def assert_expected_org_values_on_request_and_info( """ # Test domain request - self.assertEqual(domain_request.generic_org_type, expected_values['generic_org_type']) - self.assertEqual(domain_request.is_election_board, expected_values['is_election_board']) - self.assertEqual(domain_request.organization_type, expected_values['organization_type']) + with self.subTest(field="DomainRequest"): + self.assertEqual(domain_request.generic_org_type, expected_values['generic_org_type']) + self.assertEqual(domain_request.is_election_board, expected_values['is_election_board']) + self.assertEqual(domain_request.organization_type, expected_values['organization_type']) # Test domain info - self.assertEqual(domain_info.generic_org_type, expected_values['generic_org_type']) - self.assertEqual(domain_info.is_election_board, expected_values['is_election_board']) - self.assertEqual(domain_info.organization_type, expected_values['organization_type']) + with self.subTest(field="DomainInformation"): + self.assertEqual(domain_info.generic_org_type, expected_values['generic_org_type']) + self.assertEqual(domain_info.is_election_board, expected_values['is_election_board']) + self.assertEqual(domain_info.organization_type, expected_values['organization_type']) def test_request_and_info_city_not_in_csv(self): """Tests what happens to a city domain that is not defined in the CSV""" @@ -174,18 +180,19 @@ def test_request_and_info_tribal_add_election_office(self): for the domain request and the domain info """ + # Set org type fields to none to mimic an environment without this data tribal_request = self.domain_request_3 + tribal_request.organization_type = None + tribal_request.save() tribal_info = self.domain_info_3 + tribal_info.organization_type = None + tribal_info.save() # Make sure that all data is correct before proceeding. - # Because the presave fixture is in place when creating this, we should expect that the - # organization_type variable is already pre-populated. We will test what happens when - # it is not in another test. - expected_values = { 'is_election_board': False, 'generic_org_type': DomainRequest.OrganizationChoices.TRIBAL, - 'organization_type': DomainRequest.OrgChoicesElectionOffice.TRIBAL + 'organization_type': None, } self.assert_expected_org_values_on_request_and_info(tribal_request, tribal_info, expected_values) @@ -194,7 +201,10 @@ def test_request_and_info_tribal_add_election_office(self): self.run_populate_organization_type() except Exception as e: self.fail(f"Could not run populate_organization_type script. Failed with exception: {e}") - + + tribal_request.refresh_from_db() + tribal_info.refresh_from_db() + # Because we define this in the "csv", we expect that is election board will switch to True, # and organization_type will now be tribal_election expected_values["is_election_board"] = True @@ -209,8 +219,13 @@ def test_request_and_info_tribal_remove_election_office(self): for the domain request and the domain info """ + # Set org type fields to none to mimic an environment without this data tribal_election_request = self.domain_request_4 + tribal_election_request.organization_type = None + tribal_election_request.save() tribal_election_info = self.domain_info_4 + tribal_election_info.organization_type = None + tribal_election_info.save() # Make sure that all data is correct before proceeding. # Because the presave fixture is in place when creating this, we should expect that the @@ -219,7 +234,7 @@ def test_request_and_info_tribal_remove_election_office(self): expected_values = { 'is_election_board': True, 'generic_org_type': DomainRequest.OrganizationChoices.TRIBAL, - 'organization_type': DomainRequest.OrgChoicesElectionOffice.TRIBAL_ELECTION + 'organization_type': None } self.assert_expected_org_values_on_request_and_info(tribal_election_request, tribal_election_info, expected_values) @@ -233,14 +248,10 @@ def test_request_and_info_tribal_remove_election_office(self): # and organization_type will now be tribal expected_values["is_election_board"] = False expected_values["organization_type"] = DomainRequest.OrgChoicesElectionOffice.TRIBAL + tribal_election_request.refresh_from_db() + tribal_election_info.refresh_from_db() self.assert_expected_org_values_on_request_and_info(tribal_election_request, tribal_election_info, expected_values) - @skip("TODO") - def test_transition_data(self): - """Tests for how this script interacts with prexisting data (for instance, stable)""" - # Make instead of mocking we can literally just run the transition domain scripts? - pass - class TestPopulateFirstReady(TestCase): """Tests for the populate_first_ready script""" From 574cb1bafae8b211b419a3d4d4df3c29440158ca Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 8 Apr 2024 09:45:12 -0600 Subject: [PATCH 07/23] Update test_management_scripts.py --- src/registrar/tests/test_management_scripts.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 71ce01424..c2067556d 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -149,15 +149,13 @@ def test_request_and_info_city_not_in_csv(self): self.assert_expected_org_values_on_request_and_info(city_request, city_info, expected_values) def test_request_and_info_federal(self): - """Tests what happens to a federal domain with is_election_board=True (which should be reverted to None)""" + """Tests what happens to a federal domain after the script is run (should be unchanged)""" federal_request = self.domain_request_1 federal_info = self.domain_info_1 # Make sure that all data is correct before proceeding. # Since the presave fixture is in effect, we should expect that # is_election_board is equal to none, even though we tried to define it as "True" - # TODO - either add some logging if this happens or a ValueError in the original ticket. - # Or FSM? Probably FSM. expected_values = { 'is_election_board': None, 'generic_org_type': DomainRequest.OrganizationChoices.FEDERAL, From 212d2002e3c49dad4890b430c663661d628bb571 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 8 Apr 2024 14:06:13 -0600 Subject: [PATCH 08/23] Fix script --- .../management/commands/populate_organization_type.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/registrar/management/commands/populate_organization_type.py b/src/registrar/management/commands/populate_organization_type.py index ce0dfae9f..4f79008db 100644 --- a/src/registrar/management/commands/populate_organization_type.py +++ b/src/registrar/management/commands/populate_organization_type.py @@ -95,8 +95,6 @@ def update_domain_requests(self, domain_requests, debug): if request.generic_org_type is not None: domain_name = request.requested_domain.name request.is_election_board = domain_name in self.domains_with_election_offices_set - new_request = create_or_update_organization_type(DomainRequest, request, return_instance=True) - print(f"what is the new request? {new_request}") if not new_request: self.request_skipped.append(request) logger.warning(f"Skipped updating {request}. No changes to be made.") @@ -132,7 +130,6 @@ def update_domain_informations(self, domain_informations, debug): if info.generic_org_type is not None: domain_name = info.domain.name info.is_election_board = domain_name in self.domains_with_election_offices_set - info = create_or_update_organization_type(DomainInformation, info, return_instance=True) self.di_to_update.append(info) if debug: logger.info(f"Updating {info} => {info.organization_type}") From a976171179cd78fbd80ec27876a19ab1a409f7ad Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 8 Apr 2024 14:59:07 -0600 Subject: [PATCH 09/23] fix script --- .../commands/populate_organization_type.py | 13 ++++--------- .../management/commands/utility/terminal_helper.py | 1 + src/registrar/models/domain_information.py | 13 +++++++++++-- src/registrar/models/domain_request.py | 12 +++++++++--- 4 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/registrar/management/commands/populate_organization_type.py b/src/registrar/management/commands/populate_organization_type.py index 4f79008db..1610c1f9b 100644 --- a/src/registrar/management/commands/populate_organization_type.py +++ b/src/registrar/management/commands/populate_organization_type.py @@ -1,12 +1,10 @@ import argparse import logging import os -from registrar.signals import create_or_update_organization_type from typing import List from django.core.management import BaseCommand from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper, ScriptDataHelper -from registrar.models import DomainInformation, DomainRequest, Domain -from django.db import transaction +from registrar.models import DomainInformation, DomainRequest logger = logging.getLogger(__name__) @@ -95,12 +93,8 @@ def update_domain_requests(self, domain_requests, debug): if request.generic_org_type is not None: domain_name = request.requested_domain.name request.is_election_board = domain_name in self.domains_with_election_offices_set - if not new_request: - self.request_skipped.append(request) - logger.warning(f"Skipped updating {request}. No changes to be made.") - else: - request = new_request - self.request_to_update.append(request) + request.sync_organization_type() + self.request_to_update.append(request) if debug: logger.info(f"Updating {request} => {request.organization_type}") @@ -130,6 +124,7 @@ def update_domain_informations(self, domain_informations, debug): if info.generic_org_type is not None: domain_name = info.domain.name info.is_election_board = domain_name in self.domains_with_election_offices_set + info = info.sync_organization_type() self.di_to_update.append(info) if debug: logger.info(f"Updating {info} => {info.organization_type}") diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index 2149f429a..b54209750 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -49,6 +49,7 @@ def bulk_update_fields(model_class, update_list, fields_to_update, batch_size=10 Usage: bulk_update_fields(Domain, page.object_list, ["first_ready"]) """ + logger.info(f"{TerminalColors.YELLOW} Bulk updating fields... {TerminalColors.ENDC}") # Create a Paginator object. Bulk_update on the full dataset # is too memory intensive for our current app config, so we can chunk this data instead. paginator = Paginator(update_list, batch_size) diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index 2ed27504c..6bdc6c00d 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -236,8 +236,11 @@ def __str__(self): except Exception: return "" - def save(self, *args, **kwargs): - """Save override for custom properties""" + def sync_organization_type(self): + """ + Updates the organization_type (without saving) to match + the is_election_board and generic_organization_type fields. + """ # Define mappings between generic org and election org. # These have to be defined here, as you'd get a cyclical import error @@ -262,6 +265,12 @@ def save(self, *args, **kwargs): # Actually updates the organization_type field org_type_helper.create_or_update_organization_type() + + return self + + def save(self, *args, **kwargs): + """Save override for custom properties""" + self.sync_organization_type() super().save(*args, **kwargs) @classmethod diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index bd529f7e6..1b8a519a0 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -666,9 +666,11 @@ class RejectionReasons(models.TextChoices): help_text="Notes about this request", ) - def save(self, *args, **kwargs): - """Save override for custom properties""" - + def sync_organization_type(self): + """ + Updates the organization_type (without saving) to match + the is_election_board and generic_organization_type fields. + """ # Define mappings between generic org and election org. # These have to be defined here, as you'd get a cyclical import error # otherwise. @@ -692,6 +694,10 @@ def save(self, *args, **kwargs): # Actually updates the organization_type field org_type_helper.create_or_update_organization_type() + + def save(self, *args, **kwargs): + """Save override for custom properties""" + self.sync_organization_type() super().save(*args, **kwargs) def __str__(self): From c1b7e7476cf423a30862e870a5e178a3797f7be7 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 8 Apr 2024 20:05:27 -0600 Subject: [PATCH 10/23] Update populate_organization_type.py --- .../commands/populate_organization_type.py | 35 +++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/src/registrar/management/commands/populate_organization_type.py b/src/registrar/management/commands/populate_organization_type.py index 1610c1f9b..4c41716d2 100644 --- a/src/registrar/management/commands/populate_organization_type.py +++ b/src/registrar/management/commands/populate_organization_type.py @@ -5,6 +5,7 @@ from django.core.management import BaseCommand from registrar.management.commands.utility.terminal_helper import TerminalColors, TerminalHelper, ScriptDataHelper from registrar.models import DomainInformation, DomainRequest +from registrar.models.utility.generic_helper import CreateOrUpdateOrganizationTypeHelper logger = logging.getLogger(__name__) @@ -93,7 +94,7 @@ def update_domain_requests(self, domain_requests, debug): if request.generic_org_type is not None: domain_name = request.requested_domain.name request.is_election_board = domain_name in self.domains_with_election_offices_set - request.sync_organization_type() + request = self.sync_organization_type(DomainRequest, request) self.request_to_update.append(request) if debug: @@ -124,7 +125,7 @@ def update_domain_informations(self, domain_informations, debug): if info.generic_org_type is not None: domain_name = info.domain.name info.is_election_board = domain_name in self.domains_with_election_offices_set - info = info.sync_organization_type() + info = self.sync_organization_type(DomainInformation, info) self.di_to_update.append(info) if debug: logger.info(f"Updating {info} => {info.organization_type}") @@ -147,3 +148,33 @@ def update_domain_informations(self, domain_informations, debug): TerminalHelper.log_script_run_summary( self.di_to_update, self.di_failed_to_update, self.di_skipped, debug, log_header ) + + def sync_organization_type(self, sender, instance, force_update=False): + """ + Updates the organization_type (without saving) to match + the is_election_board and generic_organization_type fields. + """ + + # Define mappings between generic org and election org. + # These have to be defined here, as you'd get a cyclical import error + # otherwise. + + # For any given organization type, return the "_election" variant. + # For example: STATE_OR_TERRITORY => STATE_OR_TERRITORY_ELECTION + generic_org_map = DomainRequest.OrgChoicesElectionOffice.get_org_generic_to_org_election() + + # For any given "_election" variant, return the base org type. + # For example: STATE_OR_TERRITORY_ELECTION => STATE_OR_TERRITORY + election_org_map = DomainRequest.OrgChoicesElectionOffice.get_org_election_to_org_generic() + + # Manages the "organization_type" variable and keeps in sync with + # "is_election_office" and "generic_organization_type" + org_type_helper = CreateOrUpdateOrganizationTypeHelper( + sender=sender, + instance=instance, + generic_org_to_org_map=generic_org_map, + election_org_to_generic_org_map=election_org_map, + ) + + instance = org_type_helper.create_or_update_organization_type(force_update) + return instance From 5313ab33b3a8bc0bdb7dcf0a87e9be9ca4e2ef36 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Mon, 8 Apr 2024 21:03:32 -0600 Subject: [PATCH 11/23] fix unit tests --- .../commands/populate_organization_type.py | 4 +- .../tests/test_management_scripts.py | 74 +++++++++++-------- src/registrar/tests/test_signals.py | 3 +- 3 files changed, 46 insertions(+), 35 deletions(-) diff --git a/src/registrar/management/commands/populate_organization_type.py b/src/registrar/management/commands/populate_organization_type.py index 4c41716d2..9e2b1bf6a 100644 --- a/src/registrar/management/commands/populate_organization_type.py +++ b/src/registrar/management/commands/populate_organization_type.py @@ -149,7 +149,7 @@ def update_domain_informations(self, domain_informations, debug): self.di_to_update, self.di_failed_to_update, self.di_skipped, debug, log_header ) - def sync_organization_type(self, sender, instance, force_update=False): + def sync_organization_type(self, sender, instance): """ Updates the organization_type (without saving) to match the is_election_board and generic_organization_type fields. @@ -176,5 +176,5 @@ def sync_organization_type(self, sender, instance, force_update=False): election_org_to_generic_org_map=election_org_map, ) - instance = org_type_helper.create_or_update_organization_type(force_update) + instance = org_type_helper.create_or_update_organization_type() return instance diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index c2067556d..26ec6fd1d 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -1,6 +1,5 @@ import copy from datetime import date, datetime, time -from unittest import skip from django.utils import timezone from django.test import TestCase @@ -25,6 +24,7 @@ from .common import MockEppLib, less_console_noise, completed_domain_request from api.tests.common import less_console_noise_decorator + class TestPopulateOrganizationType(MockEppLib): """Tests for the populate_organization_type script""" @@ -86,7 +86,7 @@ def tearDown(self): Contact.objects.all().delete() Website.objects.all().delete() - #@less_console_noise_decorator + @less_console_noise_decorator def run_populate_organization_type(self): """ This method executes the populate_organization_type command. @@ -99,7 +99,7 @@ def run_populate_organization_type(self): return_value=True, ): call_command("populate_organization_type", "registrar/tests/data/fake_election_domains.csv", debug=True) - + def assert_expected_org_values_on_request_and_info( self, domain_request: DomainRequest, @@ -114,15 +114,15 @@ def assert_expected_org_values_on_request_and_info( # Test domain request with self.subTest(field="DomainRequest"): - self.assertEqual(domain_request.generic_org_type, expected_values['generic_org_type']) - self.assertEqual(domain_request.is_election_board, expected_values['is_election_board']) - self.assertEqual(domain_request.organization_type, expected_values['organization_type']) + self.assertEqual(domain_request.generic_org_type, expected_values["generic_org_type"]) + self.assertEqual(domain_request.is_election_board, expected_values["is_election_board"]) + self.assertEqual(domain_request.organization_type, expected_values["organization_type"]) # Test domain info with self.subTest(field="DomainInformation"): - self.assertEqual(domain_info.generic_org_type, expected_values['generic_org_type']) - self.assertEqual(domain_info.is_election_board, expected_values['is_election_board']) - self.assertEqual(domain_info.organization_type, expected_values['organization_type']) + self.assertEqual(domain_info.generic_org_type, expected_values["generic_org_type"]) + self.assertEqual(domain_info.is_election_board, expected_values["is_election_board"]) + self.assertEqual(domain_info.organization_type, expected_values["organization_type"]) def test_request_and_info_city_not_in_csv(self): """Tests what happens to a city domain that is not defined in the CSV""" @@ -133,9 +133,9 @@ def test_request_and_info_city_not_in_csv(self): # Since the presave fixture is in effect, we should expect that # is_election_board is equal to none, even though we tried to define it as "True" expected_values = { - 'is_election_board': False, - 'generic_org_type': DomainRequest.OrganizationChoices.CITY, - 'organization_type': DomainRequest.OrgChoicesElectionOffice.CITY, + "is_election_board": False, + "generic_org_type": DomainRequest.OrganizationChoices.CITY, + "organization_type": DomainRequest.OrgChoicesElectionOffice.CITY, } self.assert_expected_org_values_on_request_and_info(city_request, city_info, expected_values) @@ -144,7 +144,7 @@ def test_request_and_info_city_not_in_csv(self): self.run_populate_organization_type() except Exception as e: self.fail(f"Could not run populate_organization_type script. Failed with exception: {e}") - + # All values should be the same self.assert_expected_org_values_on_request_and_info(city_request, city_info, expected_values) @@ -157,9 +157,9 @@ def test_request_and_info_federal(self): # Since the presave fixture is in effect, we should expect that # is_election_board is equal to none, even though we tried to define it as "True" expected_values = { - 'is_election_board': None, - 'generic_org_type': DomainRequest.OrganizationChoices.FEDERAL, - 'organization_type': DomainRequest.OrgChoicesElectionOffice.FEDERAL, + "is_election_board": None, + "generic_org_type": DomainRequest.OrganizationChoices.FEDERAL, + "organization_type": DomainRequest.OrgChoicesElectionOffice.FEDERAL, } self.assert_expected_org_values_on_request_and_info(federal_request, federal_info, expected_values) @@ -168,10 +168,14 @@ def test_request_and_info_federal(self): self.run_populate_organization_type() except Exception as e: self.fail(f"Could not run populate_organization_type script. Failed with exception: {e}") - + # All values should be the same self.assert_expected_org_values_on_request_and_info(federal_request, federal_info, expected_values) + def do_nothing(self): + """Does nothing for mocking purposes""" + pass + def test_request_and_info_tribal_add_election_office(self): """ Tests if a tribal domain in the election csv changes organization_type to TRIBAL - ELECTION @@ -181,16 +185,18 @@ def test_request_and_info_tribal_add_election_office(self): # Set org type fields to none to mimic an environment without this data tribal_request = self.domain_request_3 tribal_request.organization_type = None - tribal_request.save() tribal_info = self.domain_info_3 tribal_info.organization_type = None - tribal_info.save() + with patch.object(DomainRequest, "sync_organization_type", self.do_nothing): + with patch.object(DomainInformation, "sync_organization_type", self.do_nothing): + tribal_request.save() + tribal_info.save() # Make sure that all data is correct before proceeding. expected_values = { - 'is_election_board': False, - 'generic_org_type': DomainRequest.OrganizationChoices.TRIBAL, - 'organization_type': None, + "is_election_board": False, + "generic_org_type": DomainRequest.OrganizationChoices.TRIBAL, + "organization_type": None, } self.assert_expected_org_values_on_request_and_info(tribal_request, tribal_info, expected_values) @@ -219,36 +225,42 @@ def test_request_and_info_tribal_remove_election_office(self): # Set org type fields to none to mimic an environment without this data tribal_election_request = self.domain_request_4 - tribal_election_request.organization_type = None - tribal_election_request.save() tribal_election_info = self.domain_info_4 + tribal_election_request.organization_type = None tribal_election_info.organization_type = None - tribal_election_info.save() + with patch.object(DomainRequest, "sync_organization_type", self.do_nothing): + with patch.object(DomainInformation, "sync_organization_type", self.do_nothing): + tribal_election_request.save() + tribal_election_info.save() # Make sure that all data is correct before proceeding. # Because the presave fixture is in place when creating this, we should expect that the # organization_type variable is already pre-populated. We will test what happens when # it is not in another test. expected_values = { - 'is_election_board': True, - 'generic_org_type': DomainRequest.OrganizationChoices.TRIBAL, - 'organization_type': None + "is_election_board": True, + "generic_org_type": DomainRequest.OrganizationChoices.TRIBAL, + "organization_type": None, } - self.assert_expected_org_values_on_request_and_info(tribal_election_request, tribal_election_info, expected_values) + self.assert_expected_org_values_on_request_and_info( + tribal_election_request, tribal_election_info, expected_values + ) # Run the populate script try: self.run_populate_organization_type() except Exception as e: self.fail(f"Could not run populate_organization_type script. Failed with exception: {e}") - + # Because we don't define this in the "csv", we expect that is election board will switch to False, # and organization_type will now be tribal expected_values["is_election_board"] = False expected_values["organization_type"] = DomainRequest.OrgChoicesElectionOffice.TRIBAL tribal_election_request.refresh_from_db() tribal_election_info.refresh_from_db() - self.assert_expected_org_values_on_request_and_info(tribal_election_request, tribal_election_info, expected_values) + self.assert_expected_org_values_on_request_and_info( + tribal_election_request, tribal_election_info, expected_values + ) class TestPopulateFirstReady(TestCase): diff --git a/src/registrar/tests/test_signals.py b/src/registrar/tests/test_signals.py index 7af6012a9..e796bd12a 100644 --- a/src/registrar/tests/test_signals.py +++ b/src/registrar/tests/test_signals.py @@ -1,7 +1,6 @@ from django.test import TestCase from django.contrib.auth import get_user_model -from registrar.models import Contact, DomainRequest, Domain, DomainInformation -from registrar.tests.common import completed_domain_request +from registrar.models import Contact class TestUserPostSave(TestCase): From 1ecbb2b7962b1330fbecc30567ae096b15473a09 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 9 Apr 2024 08:15:30 -0600 Subject: [PATCH 12/23] Update test_reports.py --- src/registrar/tests/test_reports.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/tests/test_reports.py b/src/registrar/tests/test_reports.py index 1fe923f5d..be66cb876 100644 --- a/src/registrar/tests/test_reports.py +++ b/src/registrar/tests/test_reports.py @@ -718,7 +718,7 @@ def test_get_sliced_domains(self): } # Test with distinct managed_domains_sliced_at_end_date = get_sliced_domains(filter_condition) - expected_content = [3, 4, 1, 0, 0, 0, 0, 0, 0, 0] + expected_content = [3, 2, 1, 0, 0, 0, 0, 0, 0, 0] self.assertEqual(managed_domains_sliced_at_end_date, expected_content) # Test without distinct From 7250e6587ca498f59ee85888473835f92f587d2b Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 10 Apr 2024 09:13:48 -0600 Subject: [PATCH 13/23] Add documentation --- docs/operations/data_migration.md | 44 +++++++++++++++++++ .../commands/populate_organization_type.py | 2 +- 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index b7e413c05..e40a1ee50 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -586,3 +586,47 @@ Example: `cf ssh getgov-za` | | Parameter | Description | |:-:|:-------------------------- |:----------------------------------------------------------------------------| | 1 | **debug** | Increases logging detail. Defaults to False. | + + +## Populate First Ready +This section outlines how to run the `populate_organization_type` script. + +### Running on sandboxes + +#### Step 1: Login to CloudFoundry +```cf login -a api.fr.cloud.gov --sso``` + +#### Step 2: SSH into your environment +```cf ssh getgov-{space}``` + +Example: `cf ssh getgov-za` + +#### Step 3: Create a shell instance +```/tmp/lifecycle/shell``` + +#### Step 4: Running the script +```./manage.py populate_organization_type {domain_election_office_filename} --debug``` + +- The domain_election_office_filename file must adhere to this format: + - example.gov\ + example2.gov\ + example3.gov + +Example: +`./manage.py populate_organization_type migrationdata/election-domains.csv --debug` + +### Running locally +```docker-compose exec app ./manage.py populate_organization_type {domain_election_office_filename} --debug``` + +Example (assuming that this is being ran from src/): +`docker-compose exec app ./manage.py populate_organization_type migrationdata/election-domains.csv --debug` + +##### Required parameters +| | Parameter | Description | +|:-:|:------------------------------------|:-------------------------------------------------------------------| +| 1 | **domain_election_office_filename** | A file containing every domain that is an election office. + +##### Optional parameters +| | Parameter | Description | +|:-:|:-------------------------- |:----------------------------------------------------------------------------| +| 1 | **debug** | Increases logging detail. Defaults to False. \ No newline at end of file diff --git a/src/registrar/management/commands/populate_organization_type.py b/src/registrar/management/commands/populate_organization_type.py index 9e2b1bf6a..a0a1c8633 100644 --- a/src/registrar/management/commands/populate_organization_type.py +++ b/src/registrar/management/commands/populate_organization_type.py @@ -33,7 +33,7 @@ def add_arguments(self, parser): parser.add_argument("--debug", action=argparse.BooleanOptionalAction) parser.add_argument( "domain_election_office_filename", - help=("A JSON file that holds the location and filenames" "of all the data files used for migrations"), + help=("A file that contains" " all the domains that are election offices."), ) def handle(self, domain_election_office_filename, **kwargs): From 8714c1c4d6a71df889f1c1c3e58223dd8177c284 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 11 Apr 2024 08:08:17 -0600 Subject: [PATCH 14/23] 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 e40a1ee50..59c2ad52c 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -588,7 +588,7 @@ Example: `cf ssh getgov-za` | 1 | **debug** | Increases logging detail. Defaults to False. | -## Populate First Ready +## Populate Organization type This section outlines how to run the `populate_organization_type` script. ### Running on sandboxes From c9e3948b04ccfc50e12c2e28adbfa4aee9d807d7 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 11 Apr 2024 09:13:56 -0600 Subject: [PATCH 15/23] PR suggestions --- docs/operations/data_migration.md | 31 +++++++-- .../commands/populate_organization_type.py | 64 +++++++++++++------ 2 files changed, 70 insertions(+), 25 deletions(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index 59c2ad52c..4bfe37174 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -589,13 +589,22 @@ Example: `cf ssh getgov-za` ## Populate Organization type -This section outlines how to run the `populate_organization_type` script. +This section outlines how to run the `populate_organization_type` script. +The script is used to update the organization_type field on DomainRequest and DomainInformation when it is None. +That data are synthesized from the generic_org_type field and the is_election_board field ### Running on sandboxes #### Step 1: Login to CloudFoundry ```cf login -a api.fr.cloud.gov --sso``` +#### Step 2: Get the domain_election_board file +The latest domain_election_board csv can be found [here](https://drive.google.com/file/d/1aDeCqwHmBnXBl2arvoFCN0INoZmsEGsQ/view). +After downloading this file, place it in `src/migrationdata` + +#### Step 2: Upload the domain_election_board file to your sandbox +Follow [Step 1: Transfer data to sandboxes](#step-1-transfer-data-to-sandboxes) and [Step 2: Transfer uploaded files to the getgov directory](#step-2-transfer-uploaded-files-to-the-getgov-directory) from the [Set Up Migrations on Sandbox](#set-up-migrations-on-sandbox) portion of this doc. + #### Step 2: SSH into your environment ```cf ssh getgov-{space}``` @@ -605,9 +614,9 @@ Example: `cf ssh getgov-za` ```/tmp/lifecycle/shell``` #### Step 4: Running the script -```./manage.py populate_organization_type {domain_election_office_filename} --debug``` +```./manage.py populate_organization_type {domain_election_board_filename} --debug``` -- The domain_election_office_filename file must adhere to this format: +- The domain_election_board_filename file must adhere to this format: - example.gov\ example2.gov\ example3.gov @@ -616,17 +625,25 @@ Example: `./manage.py populate_organization_type migrationdata/election-domains.csv --debug` ### Running locally -```docker-compose exec app ./manage.py populate_organization_type {domain_election_office_filename} --debug``` + +#### Step 1: Get the domain_election_board file +The latest domain_election_board csv can be found [here](https://drive.google.com/file/d/1aDeCqwHmBnXBl2arvoFCN0INoZmsEGsQ/view). +After downloading this file, place it in `src/migrationdata` + + +#### Step 2: Running the script +```docker-compose exec app ./manage.py populate_organization_type {domain_election_board_filename} --debug``` Example (assuming that this is being ran from src/): `docker-compose exec app ./manage.py populate_organization_type migrationdata/election-domains.csv --debug` -##### Required parameters + +### Required parameters | | Parameter | Description | |:-:|:------------------------------------|:-------------------------------------------------------------------| -| 1 | **domain_election_office_filename** | A file containing every domain that is an election office. +| 1 | **domain_election_board_filename** | A file containing every domain that is an election office. -##### Optional parameters +### Optional parameters | | Parameter | Description | |:-:|:-------------------------- |:----------------------------------------------------------------------------| | 1 | **debug** | Increases logging detail. Defaults to False. \ No newline at end of file diff --git a/src/registrar/management/commands/populate_organization_type.py b/src/registrar/management/commands/populate_organization_type.py index a0a1c8633..67fc9397e 100644 --- a/src/registrar/management/commands/populate_organization_type.py +++ b/src/registrar/management/commands/populate_organization_type.py @@ -11,7 +11,11 @@ class Command(BaseCommand): - help = "Loops through each valid DomainInformation and DomainRequest object and updates its organization_type value" + help = ( + "Loops through each valid DomainInformation and DomainRequest object and updates its organization_type value. " + "A valid DomainInformation/DomainRequest in this sense is one that has the value None for organization_type. " + "In other words, we populate the organization_type field if it is not already populated." + ) def __init__(self): super().__init__() @@ -26,34 +30,28 @@ def __init__(self): self.di_skipped: List[DomainInformation] = [] # Define a global variable for all domains with election offices - self.domains_with_election_offices_set = set() + self.domains_with_election_boards_set = set() def add_arguments(self, parser): """Adds command line arguments""" parser.add_argument("--debug", action=argparse.BooleanOptionalAction) parser.add_argument( - "domain_election_office_filename", + "domain_election_board_filename", help=("A file that contains" " all the domains that are election offices."), ) - def handle(self, domain_election_office_filename, **kwargs): + def handle(self, domain_election_board_filename, **kwargs): """Loops through each valid Domain object and updates its first_created value""" debug = kwargs.get("debug") # Check if the provided file path is valid - if not os.path.isfile(domain_election_office_filename): - raise argparse.ArgumentTypeError(f"Invalid file path '{domain_election_office_filename}'") + if not os.path.isfile(domain_election_board_filename): + raise argparse.ArgumentTypeError(f"Invalid file path '{domain_election_board_filename}'") - with open(domain_election_office_filename, "r") as file: - for line in file: - # Remove any leading/trailing whitespace - domain = line.strip() - if domain not in self.domains_with_election_offices_set: - self.domains_with_election_offices_set.add(domain) + # Read the election office csv + self.read_election_board_file(domain_election_board_filename) - domain_requests = DomainRequest.objects.filter( - organization_type__isnull=True, requested_domain__name__isnull=False - ) + domain_requests = DomainRequest.objects.filter(organization_type__isnull=True) # Code execution will stop here if the user prompts "N" TerminalHelper.prompt_for_execution( @@ -88,12 +86,37 @@ def handle(self, domain_election_office_filename, **kwargs): self.update_domain_informations(domain_infos, debug) + def read_election_board_file(self, domain_election_board_filename): + """ + Reads the election board file and adds each parsed domain to self.domains_with_election_boards_set. + As previously implied, this file contains information about Domains which have election boards. + + The file must adhere to this format: + ``` + domain1.gov + domain2.gov + domain3.gov + ``` + (and so on) + """ + with open(domain_election_board_filename, "r") as file: + for line in file: + # Remove any leading/trailing whitespace + domain = line.strip() + if domain not in self.domains_with_election_boards_set: + self.domains_with_election_boards_set.add(domain) + def update_domain_requests(self, domain_requests, debug): + """ + Updates the organization_type for a list of DomainRequest objects using the `sync_organization_type` function. + Results are then logged. + Debug mode provides detailed logging. + """ for request in domain_requests: try: if request.generic_org_type is not None: domain_name = request.requested_domain.name - request.is_election_board = domain_name in self.domains_with_election_offices_set + request.is_election_board = domain_name in self.domains_with_election_boards_set request = self.sync_organization_type(DomainRequest, request) self.request_to_update.append(request) @@ -120,11 +143,16 @@ def update_domain_requests(self, domain_requests, debug): ) def update_domain_informations(self, domain_informations, debug): + """ + Updates the organization_type for a list of DomainInformation objects using the `sync_organization_type` function. + Results are then logged. + Debug mode provides detailed logging. + """ for info in domain_informations: try: if info.generic_org_type is not None: domain_name = info.domain.name - info.is_election_board = domain_name in self.domains_with_election_offices_set + info.is_election_board = domain_name in self.domains_with_election_boards_set info = self.sync_organization_type(DomainInformation, info) self.di_to_update.append(info) if debug: @@ -168,7 +196,7 @@ def sync_organization_type(self, sender, instance): election_org_map = DomainRequest.OrgChoicesElectionOffice.get_org_election_to_org_generic() # Manages the "organization_type" variable and keeps in sync with - # "is_election_office" and "generic_organization_type" + # "is_election_board" and "generic_organization_type" org_type_helper = CreateOrUpdateOrganizationTypeHelper( sender=sender, instance=instance, From 09c0ad59cd388cfda86a5b90e18768c6bf7f233c Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Thu, 11 Apr 2024 09:25:08 -0600 Subject: [PATCH 16/23] Linting --- .../management/commands/populate_organization_type.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/management/commands/populate_organization_type.py b/src/registrar/management/commands/populate_organization_type.py index 67fc9397e..8428beb82 100644 --- a/src/registrar/management/commands/populate_organization_type.py +++ b/src/registrar/management/commands/populate_organization_type.py @@ -144,9 +144,9 @@ def update_domain_requests(self, domain_requests, debug): def update_domain_informations(self, domain_informations, debug): """ - Updates the organization_type for a list of DomainInformation objects using the `sync_organization_type` function. + Updates the organization_type for a list of DomainInformation objects + using the `sync_organization_type` function. Results are then logged. - Debug mode provides detailed logging. """ for info in domain_informations: try: From 582f35bc07e8c7a5188b8c493a1cbe8970c88587 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 16 Apr 2024 15:24:56 -0600 Subject: [PATCH 17/23] PR suggestions --- docs/operations/data_migration.md | 15 ++-- .../commands/populate_organization_type.py | 89 +++++++++++++------ src/registrar/models/domain_information.py | 2 +- src/registrar/models/domain_request.py | 2 +- .../tests/test_management_scripts.py | 49 ++++++++-- 5 files changed, 110 insertions(+), 47 deletions(-) diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index 4bfe37174..0846208de 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -591,7 +591,7 @@ Example: `cf ssh getgov-za` ## Populate Organization type This section outlines how to run the `populate_organization_type` script. The script is used to update the organization_type field on DomainRequest and DomainInformation when it is None. -That data are synthesized from the generic_org_type field and the is_election_board field +That data are synthesized from the generic_org_type field and the is_election_board field by concatenating " - Elections" on the end of generic_org_type string if is_elections_board is True. ### Running on sandboxes @@ -614,7 +614,7 @@ Example: `cf ssh getgov-za` ```/tmp/lifecycle/shell``` #### Step 4: Running the script -```./manage.py populate_organization_type {domain_election_board_filename} --debug``` +```./manage.py populate_organization_type {domain_election_board_filename}``` - The domain_election_board_filename file must adhere to this format: - example.gov\ @@ -622,7 +622,7 @@ Example: `cf ssh getgov-za` example3.gov Example: -`./manage.py populate_organization_type migrationdata/election-domains.csv --debug` +`./manage.py populate_organization_type migrationdata/election-domains.csv` ### Running locally @@ -632,18 +632,13 @@ After downloading this file, place it in `src/migrationdata` #### Step 2: Running the script -```docker-compose exec app ./manage.py populate_organization_type {domain_election_board_filename} --debug``` +```docker-compose exec app ./manage.py populate_organization_type {domain_election_board_filename}``` Example (assuming that this is being ran from src/): -`docker-compose exec app ./manage.py populate_organization_type migrationdata/election-domains.csv --debug` +`docker-compose exec app ./manage.py populate_organization_type migrationdata/election-domains.csv` ### Required parameters | | Parameter | Description | |:-:|:------------------------------------|:-------------------------------------------------------------------| | 1 | **domain_election_board_filename** | A file containing every domain that is an election office. - -### Optional parameters -| | Parameter | Description | -|:-:|:-------------------------- |:----------------------------------------------------------------------------| -| 1 | **debug** | Increases logging detail. Defaults to False. \ No newline at end of file diff --git a/src/registrar/management/commands/populate_organization_type.py b/src/registrar/management/commands/populate_organization_type.py index 8428beb82..f2d9c062d 100644 --- a/src/registrar/management/commands/populate_organization_type.py +++ b/src/registrar/management/commands/populate_organization_type.py @@ -34,7 +34,6 @@ def __init__(self): def add_arguments(self, parser): """Adds command line arguments""" - parser.add_argument("--debug", action=argparse.BooleanOptionalAction) parser.add_argument( "domain_election_board_filename", help=("A file that contains" " all the domains that are election offices."), @@ -42,7 +41,6 @@ def add_arguments(self, parser): def handle(self, domain_election_board_filename, **kwargs): """Loops through each valid Domain object and updates its first_created value""" - debug = kwargs.get("debug") # Check if the provided file path is valid if not os.path.isfile(domain_election_board_filename): @@ -66,7 +64,7 @@ def handle(self, domain_election_board_filename, **kwargs): ) logger.info("Updating DomainRequest(s)...") - self.update_domain_requests(domain_requests, debug) + self.update_domain_requests(domain_requests) # We should actually be targeting all fields with no value for organization type, # but do have a value for generic_org_type. This is because there is data that we can infer. @@ -84,7 +82,7 @@ def handle(self, domain_election_board_filename, **kwargs): ) logger.info("Updating DomainInformation(s)...") - self.update_domain_informations(domain_infos, debug) + self.update_domain_informations(domain_infos) def read_election_board_file(self, domain_election_board_filename): """ @@ -106,26 +104,37 @@ def read_election_board_file(self, domain_election_board_filename): if domain not in self.domains_with_election_boards_set: self.domains_with_election_boards_set.add(domain) - def update_domain_requests(self, domain_requests, debug): + def update_domain_requests(self, domain_requests): """ Updates the organization_type for a list of DomainRequest objects using the `sync_organization_type` function. Results are then logged. - Debug mode provides detailed logging. + + This function updates the following variables: + - self.request_to_update list is appended to if the field was updated successfully. + - self.request_skipped list is appended to if the field has `None` for `request.generic_org_type`. + - self.request_failed_to_update list is appended to if an exception is caught during update. """ for request in domain_requests: try: if request.generic_org_type is not None: - domain_name = request.requested_domain.name - request.is_election_board = domain_name in self.domains_with_election_boards_set - request = self.sync_organization_type(DomainRequest, request) - self.request_to_update.append(request) + domain_name = None + if ( + request.requested_domain is not None and + request.requested_domain.name is not None + ): + domain_name = request.requested_domain.name + + request_is_approved = request.state == DomainRequest.DomainRequestStatus.APPROVED + if request_is_approved and domain_name is not None: + request.is_election_board = domain_name in self.domains_with_election_boards_set - if debug: - logger.info(f"Updating {request} => {request.organization_type}") + self.sync_organization_type(DomainRequest, request) + + self.request_to_update.append(request) + logger.info(f"Updating {request} => {request.organization_type}") else: self.request_skipped.append(request) - if debug: - logger.warning(f"Skipped updating {request}. No generic_org_type was found.") + logger.warning(f"Skipped updating {request}. No generic_org_type was found.") except Exception as err: self.request_failed_to_update.append(request) logger.error(err) @@ -139,28 +148,44 @@ def update_domain_requests(self, domain_requests, debug): # Log what happened log_header = "============= FINISHED UPDATE FOR DOMAINREQUEST ===============" TerminalHelper.log_script_run_summary( - self.request_to_update, self.request_failed_to_update, self.request_skipped, debug, log_header + self.request_to_update, self.request_failed_to_update, self.request_skipped, True, log_header ) - def update_domain_informations(self, domain_informations, debug): + update_skipped_count = len(self.request_to_update) + if update_skipped_count > 0: + logger.warning( + f"""{TerminalColors.MAGENTA} + Note: Entries are skipped when generic_org_type is None + {TerminalColors.ENDC} + """ + ) + + def update_domain_informations(self, domain_informations): """ Updates the organization_type for a list of DomainInformation objects - using the `sync_organization_type` function. + and updates is_election_board if the domain is in the provided csv. Results are then logged. + + This function updates the following variables: + - self.di_to_update list is appended to if the field was updated successfully. + - self.di_skipped list is appended to if the field has `None` for `request.generic_org_type`. + - self.di_failed_to_update list is appended to if an exception is caught during update. """ for info in domain_informations: try: if info.generic_org_type is not None: domain_name = info.domain.name - info.is_election_board = domain_name in self.domains_with_election_boards_set - info = self.sync_organization_type(DomainInformation, info) + + if not info.is_election_board: + info.is_election_board = domain_name in self.domains_with_election_boards_set + + self.sync_organization_type(DomainInformation, info) + self.di_to_update.append(info) - if debug: - logger.info(f"Updating {info} => {info.organization_type}") + logger.info(f"Updating {info} => {info.organization_type}") else: self.di_skipped.append(info) - if debug: - logger.warning(f"Skipped updating {info}. No generic_org_type was found.") + logger.warning(f"Skipped updating {info}. No generic_org_type was found.") except Exception as err: self.di_failed_to_update.append(info) logger.error(err) @@ -170,13 +195,22 @@ def update_domain_informations(self, domain_informations, debug): ScriptDataHelper.bulk_update_fields( DomainInformation, self.di_to_update, ["organization_type", "is_election_board", "generic_org_type"] ) - + # Log what happened log_header = "============= FINISHED UPDATE FOR DOMAININFORMATION ===============" TerminalHelper.log_script_run_summary( - self.di_to_update, self.di_failed_to_update, self.di_skipped, debug, log_header + self.di_to_update, self.di_failed_to_update, self.di_skipped, True, log_header ) + update_skipped_count = len(self.di_skipped) + if update_skipped_count > 0: + logger.warning( + f"""{TerminalColors.MAGENTA} + Note: Entries are skipped when generic_org_type is None + {TerminalColors.ENDC} + """ + ) + def sync_organization_type(self, sender, instance): """ Updates the organization_type (without saving) to match @@ -187,7 +221,7 @@ def sync_organization_type(self, sender, instance): # These have to be defined here, as you'd get a cyclical import error # otherwise. - # For any given organization type, return the "_election" variant. + # For any given organization type, return the "_ELECTION" enum equivalent. # For example: STATE_OR_TERRITORY => STATE_OR_TERRITORY_ELECTION generic_org_map = DomainRequest.OrgChoicesElectionOffice.get_org_generic_to_org_election() @@ -204,5 +238,4 @@ def sync_organization_type(self, sender, instance): election_org_to_generic_org_map=election_org_map, ) - instance = org_type_helper.create_or_update_organization_type() - return instance + org_type_helper.create_or_update_organization_type() diff --git a/src/registrar/models/domain_information.py b/src/registrar/models/domain_information.py index 6bdc6c00d..b02385d84 100644 --- a/src/registrar/models/domain_information.py +++ b/src/registrar/models/domain_information.py @@ -246,7 +246,7 @@ def sync_organization_type(self): # These have to be defined here, as you'd get a cyclical import error # otherwise. - # For any given organization type, return the "_election" variant. + # For any given organization type, return the "_ELECTION" enum equivalent. # For example: STATE_OR_TERRITORY => STATE_OR_TERRITORY_ELECTION generic_org_map = DomainRequest.OrgChoicesElectionOffice.get_org_generic_to_org_election() diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 1b8a519a0..8fb4b78b9 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -675,7 +675,7 @@ def sync_organization_type(self): # These have to be defined here, as you'd get a cyclical import error # otherwise. - # For any given organization type, return the "_election" variant. + # For any given organization type, return the "_ELECTION" enum equivalent. # For example: STATE_OR_TERRITORY => STATE_OR_TERRITORY_ELECTION generic_org_map = self.OrgChoicesElectionOffice.get_org_generic_to_org_election() diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 26ec6fd1d..7b422e44b 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -107,9 +107,23 @@ def assert_expected_org_values_on_request_and_info( expected_values: dict, ): """ - This is a a helper function that ensures that: + This is a helper function that tests the following conditions: 1. DomainRequest and DomainInformation (on given objects) are equivalent 2. That generic_org_type, is_election_board, and organization_type are equal to passed in values + + Args: + domain_request (DomainRequest): The DomainRequest object to test + + domain_info (DomainInformation): The DomainInformation object to test + + expected_values (dict): Container for what we expect is_electionboard, generic_org_type, + and organization_type to be on DomainRequest and DomainInformation. + Example: + expected_values = { + "is_election_board": False, + "generic_org_type": DomainRequest.OrganizationChoices.CITY, + "organization_type": DomainRequest.OrgChoicesElectionOffice.CITY, + } """ # Test domain request @@ -124,8 +138,23 @@ def assert_expected_org_values_on_request_and_info( self.assertEqual(domain_info.is_election_board, expected_values["is_election_board"]) self.assertEqual(domain_info.organization_type, expected_values["organization_type"]) + def do_nothing(self): + """Does nothing for mocking purposes""" + pass + def test_request_and_info_city_not_in_csv(self): - """Tests what happens to a city domain that is not defined in the CSV""" + """ + Tests what happens to a city domain that is not defined in the CSV. + + Scenario: A domain request (of type city) is made that is not defined in the CSV file. + When a domain request is made for a city that is not listed in the CSV, + Then the `is_election_board` value should remain False, + and the `generic_org_type` and `organization_type` should both be `city`. + + Expected Result: The `is_election_board` and `generic_org_type` attributes should be unchanged. + The `organization_type` field should now be `city`. + """ + city_request = self.domain_request_2 city_info = self.domain_request_2 @@ -149,7 +178,17 @@ def test_request_and_info_city_not_in_csv(self): self.assert_expected_org_values_on_request_and_info(city_request, city_info, expected_values) def test_request_and_info_federal(self): - """Tests what happens to a federal domain after the script is run (should be unchanged)""" + """ + Tests what happens to a federal domain after the script is run (should be unchanged). + + Scenario: A domain request (of type federal) is processed after running the populate_organization_type script. + When a federal domain request is made, + Then the `is_election_board` value should remain None, + and the `generic_org_type` and `organization_type` fields should both be `federal`. + + Expected Result: The `is_election_board` and `generic_org_type` attributes should be unchanged. + The `organization_type` field should now be `federal`. + """ federal_request = self.domain_request_1 federal_info = self.domain_info_1 @@ -172,10 +211,6 @@ def test_request_and_info_federal(self): # All values should be the same self.assert_expected_org_values_on_request_and_info(federal_request, federal_info, expected_values) - def do_nothing(self): - """Does nothing for mocking purposes""" - pass - def test_request_and_info_tribal_add_election_office(self): """ Tests if a tribal domain in the election csv changes organization_type to TRIBAL - ELECTION From 50ede0347670d1effc299ef184112b209e6ade2e Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 16 Apr 2024 15:32:33 -0600 Subject: [PATCH 18/23] Linting / fix unit test passing in debug --- .../management/commands/populate_organization_type.py | 7 ++----- src/registrar/tests/test_management_scripts.py | 4 ++-- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/registrar/management/commands/populate_organization_type.py b/src/registrar/management/commands/populate_organization_type.py index f2d9c062d..cfa44d0a5 100644 --- a/src/registrar/management/commands/populate_organization_type.py +++ b/src/registrar/management/commands/populate_organization_type.py @@ -118,10 +118,7 @@ def update_domain_requests(self, domain_requests): try: if request.generic_org_type is not None: domain_name = None - if ( - request.requested_domain is not None and - request.requested_domain.name is not None - ): + if request.requested_domain is not None and request.requested_domain.name is not None: domain_name = request.requested_domain.name request_is_approved = request.state == DomainRequest.DomainRequestStatus.APPROVED @@ -195,7 +192,7 @@ def update_domain_informations(self, domain_informations): ScriptDataHelper.bulk_update_fields( DomainInformation, self.di_to_update, ["organization_type", "is_election_board", "generic_org_type"] ) - + # Log what happened log_header = "============= FINISHED UPDATE FOR DOMAININFORMATION ===============" TerminalHelper.log_script_run_summary( diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 7b422e44b..cad9e0ebe 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -98,7 +98,7 @@ def run_populate_organization_type(self): "registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit", # noqa return_value=True, ): - call_command("populate_organization_type", "registrar/tests/data/fake_election_domains.csv", debug=True) + call_command("populate_organization_type", "registrar/tests/data/fake_election_domains.csv") def assert_expected_org_values_on_request_and_info( self, @@ -118,7 +118,7 @@ def assert_expected_org_values_on_request_and_info( expected_values (dict): Container for what we expect is_electionboard, generic_org_type, and organization_type to be on DomainRequest and DomainInformation. - Example: + Example: expected_values = { "is_election_board": False, "generic_org_type": DomainRequest.OrganizationChoices.CITY, From 40a64a1e868ef61dcce9745b756527ac14b15ceb Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Tue, 16 Apr 2024 15:51:10 -0600 Subject: [PATCH 19/23] Fix typo causing test fail --- src/registrar/management/commands/populate_organization_type.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/management/commands/populate_organization_type.py b/src/registrar/management/commands/populate_organization_type.py index cfa44d0a5..cef8e9433 100644 --- a/src/registrar/management/commands/populate_organization_type.py +++ b/src/registrar/management/commands/populate_organization_type.py @@ -121,7 +121,7 @@ def update_domain_requests(self, domain_requests): if request.requested_domain is not None and request.requested_domain.name is not None: domain_name = request.requested_domain.name - request_is_approved = request.state == DomainRequest.DomainRequestStatus.APPROVED + request_is_approved = request.status == DomainRequest.DomainRequestStatus.APPROVED if request_is_approved and domain_name is not None: request.is_election_board = domain_name in self.domains_with_election_boards_set From 4b90ff833c1a1686229d2e15bc880f8bd52ccb00 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 17 Apr 2024 09:26:05 -0600 Subject: [PATCH 20/23] Fix unit test --- .../commands/populate_organization_type.py | 5 ++- .../models/utility/generic_helper.py | 33 +++++++++++-------- .../tests/test_management_scripts.py | 19 ++++++----- 3 files changed, 33 insertions(+), 24 deletions(-) diff --git a/src/registrar/management/commands/populate_organization_type.py b/src/registrar/management/commands/populate_organization_type.py index cef8e9433..a7dd98b24 100644 --- a/src/registrar/management/commands/populate_organization_type.py +++ b/src/registrar/management/commands/populate_organization_type.py @@ -122,11 +122,10 @@ def update_domain_requests(self, domain_requests): domain_name = request.requested_domain.name request_is_approved = request.status == DomainRequest.DomainRequestStatus.APPROVED - if request_is_approved and domain_name is not None: + if request_is_approved and domain_name is not None and not request.is_election_board: request.is_election_board = domain_name in self.domains_with_election_boards_set self.sync_organization_type(DomainRequest, request) - self.request_to_update.append(request) logger.info(f"Updating {request} => {request.organization_type}") else: @@ -235,4 +234,4 @@ def sync_organization_type(self, sender, instance): election_org_to_generic_org_map=election_org_map, ) - org_type_helper.create_or_update_organization_type() + org_type_helper.create_or_update_organization_type(force_update=True) diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 32f767ede..77351b11c 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -49,7 +49,7 @@ def __init__(self, sender, instance, generic_org_to_org_map, election_org_to_gen self.generic_org_to_org_map = generic_org_to_org_map self.election_org_to_generic_org_map = election_org_to_generic_org_map - def create_or_update_organization_type(self): + def create_or_update_organization_type(self, force_update = False): """The organization_type field on DomainRequest and DomainInformation is consituted from the generic_org_type and is_election_board fields. To keep the organization_type field up to date, we need to update it before save based off of those field @@ -59,6 +59,11 @@ def create_or_update_organization_type(self): one of the excluded types (FEDERAL, INTERSTATE, SCHOOL_DISTRICT), the organization_type is set to a corresponding election variant. Otherwise, it directly mirrors the generic_org_type value. + + args: + force_update (bool): If an existing instance has no values to change, + try to update the organization_type field (or related fields) anyway. + This is done by invoking the new instance handler. """ # A new record is added with organization_type not defined. @@ -67,7 +72,7 @@ def create_or_update_organization_type(self): if is_new_instance: self._handle_new_instance() else: - self._handle_existing_instance() + self._handle_existing_instance(force_update) return self.instance @@ -92,7 +97,7 @@ def _handle_new_instance(self): # Update the field self._update_fields(organization_type_needs_update, generic_org_type_needs_update) - def _handle_existing_instance(self): + def _handle_existing_instance(self, force_update_when_no_are_changes_found = False): # == Init variables == # # Instance is already in the database, fetch its current state current_instance = self.sender.objects.get(id=self.instance.id) @@ -109,17 +114,19 @@ def _handle_existing_instance(self): # This will not happen in normal flow as it is not possible otherwise. raise ValueError("Cannot update organization_type and generic_org_type simultaneously.") elif not organization_type_changed and (not generic_org_type_changed and not is_election_board_changed): - # No values to update - do nothing - return None - # == Program flow will halt here if there is no reason to update == # - - # == Update the linked values == # - # Find out which field needs updating - organization_type_needs_update = generic_org_type_changed or is_election_board_changed - generic_org_type_needs_update = organization_type_changed + # No changes found + if force_update_when_no_are_changes_found: + # If we want to force an update anyway, we can treat this record like + # its a new one in that we check for "None" values rather than changes. + self._handle_new_instance() + else: + # == Update the linked values == # + # Find out which field needs updating + organization_type_needs_update = generic_org_type_changed or is_election_board_changed + generic_org_type_needs_update = organization_type_changed - # Update the field - self._update_fields(organization_type_needs_update, generic_org_type_needs_update) + # Update the field + self._update_fields(organization_type_needs_update, generic_org_type_needs_update) def _update_fields(self, organization_type_needs_update, generic_org_type_needs_update): """ diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index cad9e0ebe..26161b272 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -251,11 +251,14 @@ def test_request_and_info_tribal_add_election_office(self): self.assert_expected_org_values_on_request_and_info(tribal_request, tribal_info, expected_values) - def test_request_and_info_tribal_remove_election_office(self): + def test_request_and_info_tribal_doesnt_remove_election_office(self): """ - Tests if a tribal domain in the election csv changes organization_type to TRIBAL - when it used to be TRIBAL - ELECTION - for the domain request and the domain info + Tests if a tribal domain in the election csv changes organization_type to TRIBAL_ELECTION + when the is_election_board is True, and generic_org_type is Tribal when it is not + present in the CSV. + + To avoid overwriting data, the script should not set any domain specified as + an election_office (that doesn't exist in the CSV) to false. """ # Set org type fields to none to mimic an environment without this data @@ -287,10 +290,10 @@ def test_request_and_info_tribal_remove_election_office(self): except Exception as e: self.fail(f"Could not run populate_organization_type script. Failed with exception: {e}") - # Because we don't define this in the "csv", we expect that is election board will switch to False, - # and organization_type will now be tribal - expected_values["is_election_board"] = False - expected_values["organization_type"] = DomainRequest.OrgChoicesElectionOffice.TRIBAL + # If we don't define this in the "csv", but the value was already true, + # we expect that is election board will stay True, and the org type will be tribal, + # and organization_type will now be tribal_election + expected_values["organization_type"] = DomainRequest.OrgChoicesElectionOffice.TRIBAL_ELECTION tribal_election_request.refresh_from_db() tribal_election_info.refresh_from_db() self.assert_expected_org_values_on_request_and_info( From cdd34936b2c4a33542c0b0a05709fd037291aac7 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 17 Apr 2024 09:29:09 -0600 Subject: [PATCH 21/23] Linter --- src/registrar/models/utility/generic_helper.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 77351b11c..991231841 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -49,7 +49,7 @@ def __init__(self, sender, instance, generic_org_to_org_map, election_org_to_gen self.generic_org_to_org_map = generic_org_to_org_map self.election_org_to_generic_org_map = election_org_to_generic_org_map - def create_or_update_organization_type(self, force_update = False): + def create_or_update_organization_type(self, force_update=False): """The organization_type field on DomainRequest and DomainInformation is consituted from the generic_org_type and is_election_board fields. To keep the organization_type field up to date, we need to update it before save based off of those field @@ -97,7 +97,7 @@ def _handle_new_instance(self): # Update the field self._update_fields(organization_type_needs_update, generic_org_type_needs_update) - def _handle_existing_instance(self, force_update_when_no_are_changes_found = False): + def _handle_existing_instance(self, force_update_when_no_are_changes_found=False): # == Init variables == # # Instance is already in the database, fetch its current state current_instance = self.sender.objects.get(id=self.instance.id) From 03a0a60b3a7c762db5de9541287b6c57b3cc08a1 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 17 Apr 2024 14:43:27 -0600 Subject: [PATCH 22/23] Add comment --- src/registrar/models/utility/generic_helper.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 991231841..4b08468a4 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -64,6 +64,9 @@ def create_or_update_organization_type(self, force_update=False): force_update (bool): If an existing instance has no values to change, try to update the organization_type field (or related fields) anyway. This is done by invoking the new instance handler. + + Use to force org type to be updated to the correct value even + if no other changes were made (including is_election). """ # A new record is added with organization_type not defined. From 4467571613f5d1c69665a6a2a9a7df723d5cbb91 Mon Sep 17 00:00:00 2001 From: zandercymatics <141044360+zandercymatics@users.noreply.github.com> Date: Wed, 17 Apr 2024 14:51:02 -0600 Subject: [PATCH 23/23] Its the final lintdown --- src/registrar/models/utility/generic_helper.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/models/utility/generic_helper.py b/src/registrar/models/utility/generic_helper.py index 4b08468a4..892298967 100644 --- a/src/registrar/models/utility/generic_helper.py +++ b/src/registrar/models/utility/generic_helper.py @@ -65,7 +65,7 @@ def create_or_update_organization_type(self, force_update=False): try to update the organization_type field (or related fields) anyway. This is done by invoking the new instance handler. - Use to force org type to be updated to the correct value even + Use to force org type to be updated to the correct value even if no other changes were made (including is_election). """