PR suggestions

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.    

src/registrar/management/commands/populate_organization_type.py

@@ -34,7 +34,6 @@ class Command(BaseCommand):
 
     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 @@ class Command(BaseCommand):
 
     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 @@ class Command(BaseCommand):
         )
         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 @@ class Command(BaseCommand):
         )
         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 @@ class Command(BaseCommand):
                 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
+                    domain_name = None
-                    request.is_election_board = domain_name in self.domains_with_election_boards_set
+                    if (
-                    request = self.sync_organization_type(DomainRequest, request)
+                        request.requested_domain is not None and
-                    self.request_to_update.append(request)
+                        request.requested_domain.name is not None
+                    ):
+                        domain_name = request.requested_domain.name
 
-                    if debug:
+                    request_is_approved = request.state == DomainRequest.DomainRequestStatus.APPROVED
-                        logger.info(f"Updating {request} => {request.organization_type}")
+                    if request_is_approved and domain_name is not None:
+                        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:
                     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 @@ class Command(BaseCommand):
         # Log what happened
         log_header = "============= FINISHED UPDATE FOR DOMAINREQUEST ==============="
         TerminalHelper.log_script_run_summary(
-            self.request_to_update, self.request_failed_to_update, self.request_skipped, 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 @@ class Command(BaseCommand):
         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 @@ class Command(BaseCommand):
         # 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 @@ class Command(BaseCommand):
             election_org_to_generic_org_map=election_org_map,
         )
 
-        instance = org_type_helper.create_or_update_organization_type()
+        org_type_helper.create_or_update_organization_type()
-        return instance

src/registrar/models/domain_information.py

@@ -246,7 +246,7 @@ class DomainInformation(TimeStampedModel):
         # 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()
 

src/registrar/models/domain_request.py

@@ -675,7 +675,7 @@ class DomainRequest(TimeStampedModel):
         # 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()
 

src/registrar/tests/test_management_scripts.py

@@ -107,9 +107,23 @@ class TestPopulateOrganizationType(MockEppLib):
         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 @@ class TestPopulateOrganizationType(MockEppLib):
             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 @@ class TestPopulateOrganizationType(MockEppLib):
         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 @@ class TestPopulateOrganizationType(MockEppLib):
         # 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