From f6e10b658522ffa00f3cf4fb86f5b75b47c2afa8 Mon Sep 17 00:00:00 2001 From: matthewswspence Date: Tue, 27 Aug 2024 14:58:04 -0500 Subject: [PATCH] Add documentation and refactor custom filtering --- docs/developer/management_script_helpers.md | 3 ++- docs/operations/data_migration.md | 27 +++++++++++++++++++ .../management/commands/clean_tables.py | 2 +- .../commands/extend_expiration_dates.py | 2 +- .../commands/load_organization_data.py | 4 +-- .../commands/load_senior_official_table.py | 2 +- .../commands/load_transition_domain.py | 2 +- .../commands/patch_federal_agency_info.py | 4 +-- .../commands/populate_first_ready.py | 2 +- .../commands/populate_organization_type.py | 4 +-- .../management/commands/update_first_ready.py | 16 +++++++---- .../commands/utility/terminal_helper.py | 26 +++++++----------- 12 files changed, 61 insertions(+), 33 deletions(-) diff --git a/docs/developer/management_script_helpers.md b/docs/developer/management_script_helpers.md index 104e4dc13..a43bb16aa 100644 --- a/docs/developer/management_script_helpers.md +++ b/docs/developer/management_script_helpers.md @@ -62,4 +62,5 @@ The class provides the following optional configuration variables: The class also provides helper methods: - `get_class_name`: Returns a display-friendly class name for the terminal prompt - `get_failure_message`: Returns the message to display if a record fails to update -- `should_skip_record`: Defines the condition for skipping a record (by default, no records are skipped) \ No newline at end of file +- `should_skip_record`: Defines the condition for skipping a record (by default, no records are skipped) +- `custom_filter`: Allows for additional filters that cannot be expressed using django queryset field lookups \ No newline at end of file diff --git a/docs/operations/data_migration.md b/docs/operations/data_migration.md index cd748b22d..75164bf9e 100644 --- a/docs/operations/data_migration.md +++ b/docs/operations/data_migration.md @@ -816,3 +816,30 @@ Example: `cf ssh getgov-za` | | Parameter | Description | |:-:|:-------------------------- |:-----------------------------------------------------------------------------------| | 1 | **federal_cio_csv_path** | Specifies where the federal CIO csv is | + +## Update First Ready Values +This section outlines how to run the populate_first_ready 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 update_first_ready --debug``` + +### Running locally +```docker-compose exec app ./manage.py update_first_ready --debug``` + +##### Optional parameters +| | Parameter | Description | +|:-:|:-------------------------- |:----------------------------------------------------------------------------| +| 1 | **debug** | Increases logging detail. Defaults to False. | diff --git a/src/registrar/management/commands/clean_tables.py b/src/registrar/management/commands/clean_tables.py index 5d4439d95..66b3e772f 100644 --- a/src/registrar/management/commands/clean_tables.py +++ b/src/registrar/management/commands/clean_tables.py @@ -21,7 +21,7 @@ class Command(BaseCommand): TerminalHelper.prompt_for_execution( system_exit_on_terminate=True, - info_to_inspect=""" + prompt_message=""" This script will delete all rows from the following tables: * Contact * Domain diff --git a/src/registrar/management/commands/extend_expiration_dates.py b/src/registrar/management/commands/extend_expiration_dates.py index cefc38b9e..ac083da1d 100644 --- a/src/registrar/management/commands/extend_expiration_dates.py +++ b/src/registrar/management/commands/extend_expiration_dates.py @@ -130,7 +130,7 @@ class Command(BaseCommand): """Asks if the user wants to proceed with this action""" TerminalHelper.prompt_for_execution( system_exit_on_terminate=True, - info_to_inspect=f""" + prompt_message=f""" ==Extension Amount== Period: {extension_amount} year(s) diff --git a/src/registrar/management/commands/load_organization_data.py b/src/registrar/management/commands/load_organization_data.py index 122795400..35cc248ee 100644 --- a/src/registrar/management/commands/load_organization_data.py +++ b/src/registrar/management/commands/load_organization_data.py @@ -64,7 +64,7 @@ class Command(BaseCommand): # Will sys.exit() when prompt is "n" TerminalHelper.prompt_for_execution( system_exit_on_terminate=True, - info_to_inspect=f""" + prompt_message=f""" ==Master data file== domain_additional_filename: {org_args.domain_additional_filename} @@ -84,7 +84,7 @@ class Command(BaseCommand): # Will sys.exit() when prompt is "n" TerminalHelper.prompt_for_execution( system_exit_on_terminate=True, - info_to_inspect=f""" + prompt_message=f""" ==Master data file== domain_additional_filename: {org_args.domain_additional_filename} diff --git a/src/registrar/management/commands/load_senior_official_table.py b/src/registrar/management/commands/load_senior_official_table.py index 43f61d57a..cdbc607bf 100644 --- a/src/registrar/management/commands/load_senior_official_table.py +++ b/src/registrar/management/commands/load_senior_official_table.py @@ -27,7 +27,7 @@ class Command(BaseCommand): TerminalHelper.prompt_for_execution( system_exit_on_terminate=True, - info_to_inspect=f""" + prompt_message=f""" ==Proposed Changes== CSV: {federal_cio_csv_path} diff --git a/src/registrar/management/commands/load_transition_domain.py b/src/registrar/management/commands/load_transition_domain.py index 4132096c8..c2dd66f55 100644 --- a/src/registrar/management/commands/load_transition_domain.py +++ b/src/registrar/management/commands/load_transition_domain.py @@ -651,7 +651,7 @@ class Command(BaseCommand): title = "Do you wish to load additional data for TransitionDomains?" proceed = TerminalHelper.prompt_for_execution( system_exit_on_terminate=True, - info_to_inspect=f""" + prompt_message=f""" !!! ENSURE THAT ALL FILENAMES ARE CORRECT BEFORE PROCEEDING ==Master data file== domain_additional_filename: {domain_additional_filename} diff --git a/src/registrar/management/commands/patch_federal_agency_info.py b/src/registrar/management/commands/patch_federal_agency_info.py index b286f1516..51a98ffaa 100644 --- a/src/registrar/management/commands/patch_federal_agency_info.py +++ b/src/registrar/management/commands/patch_federal_agency_info.py @@ -91,7 +91,7 @@ class Command(BaseCommand): # Code execution will stop here if the user prompts "N" TerminalHelper.prompt_for_execution( system_exit_on_terminate=True, - info_to_inspect=f""" + prompt_message=f""" ==Proposed Changes== Number of DomainInformation objects to change: {len(human_readable_domain_names)} The following DomainInformation objects will be modified: {human_readable_domain_names} @@ -148,7 +148,7 @@ class Command(BaseCommand): # Code execution will stop here if the user prompts "N" TerminalHelper.prompt_for_execution( system_exit_on_terminate=True, - info_to_inspect=f""" + prompt_message=f""" ==File location== current-full.csv filepath: {file_path} diff --git a/src/registrar/management/commands/populate_first_ready.py b/src/registrar/management/commands/populate_first_ready.py index 9636476c2..04468029a 100644 --- a/src/registrar/management/commands/populate_first_ready.py +++ b/src/registrar/management/commands/populate_first_ready.py @@ -31,7 +31,7 @@ class Command(BaseCommand): # Code execution will stop here if the user prompts "N" TerminalHelper.prompt_for_execution( system_exit_on_terminate=True, - info_to_inspect=f""" + prompt_message=f""" ==Proposed Changes== Number of Domain objects to change: {len(domains)} """, diff --git a/src/registrar/management/commands/populate_organization_type.py b/src/registrar/management/commands/populate_organization_type.py index a7dd98b24..60d179cb8 100644 --- a/src/registrar/management/commands/populate_organization_type.py +++ b/src/registrar/management/commands/populate_organization_type.py @@ -54,7 +54,7 @@ class Command(BaseCommand): # Code execution will stop here if the user prompts "N" TerminalHelper.prompt_for_execution( system_exit_on_terminate=True, - info_to_inspect=f""" + prompt_message=f""" ==Proposed Changes== Number of DomainRequest objects to change: {len(domain_requests)} @@ -72,7 +72,7 @@ class Command(BaseCommand): # Code execution will stop here if the user prompts "N" TerminalHelper.prompt_for_execution( system_exit_on_terminate=True, - info_to_inspect=f""" + prompt_message=f""" ==Proposed Changes== Number of DomainInformation objects to change: {len(domain_infos)} diff --git a/src/registrar/management/commands/update_first_ready.py b/src/registrar/management/commands/update_first_ready.py index 4808471ea..3c7a12928 100644 --- a/src/registrar/management/commands/update_first_ready.py +++ b/src/registrar/management/commands/update_first_ready.py @@ -1,5 +1,6 @@ import logging from django.core.management import BaseCommand +from django.db.models.manager import BaseManager from registrar.management.commands.utility.terminal_helper import PopulateScriptTemplate, TerminalColors from registrar.models import Domain, TransitionDomain @@ -10,8 +11,8 @@ class Command(BaseCommand, PopulateScriptTemplate): def handle(self, **kwargs): """Loops through each valid Domain object and updates it's first_ready value if it is out of sync""" - filter_conditions={"state__in":[Domain.State.READY, Domain.State.ON_HOLD, Domain.State.DELETED]} - self.mass_update_records(Domain, filter_conditions, ["first_ready"], verbose=True, custom_filter=self.should_update) + filter_conditions={"state__in":[Domain.State.READY, Domain.State.ON_HOLD, Domain.State.DELETED, Domain.State.UNKNOWN]} + self.mass_update_records(Domain, filter_conditions, ["first_ready"], verbose=True) def update_record(self, record: Domain): """Defines how we update the first_ready field""" @@ -23,6 +24,11 @@ class Command(BaseCommand, PopulateScriptTemplate): ) # check if a transition domain object for this domain name exists, - # and if so whether its first_ready value matches its created_at date - def should_update(self, record: Domain) -> bool: - return TransitionDomain.objects.filter(domain_name=record.name).exists() and record.first_ready != record.created_at.date() \ No newline at end of file + # or if so whether its first_ready value matches its created_at date + def custom_filter(self, records: BaseManager[Domain]) -> BaseManager[Domain]: + to_include_pks = [] + for record in records: + if TransitionDomain.objects.filter(domain_name=record.name).exists() and record.first_ready != record.created_at.date(): + to_include_pks.append(record.pk) + + return records.filter(pk__in=to_include_pks) \ No newline at end of file diff --git a/src/registrar/management/commands/utility/terminal_helper.py b/src/registrar/management/commands/utility/terminal_helper.py index c045a2c25..8bb8a5723 100644 --- a/src/registrar/management/commands/utility/terminal_helper.py +++ b/src/registrar/management/commands/utility/terminal_helper.py @@ -3,6 +3,7 @@ import sys from abc import ABC, abstractmethod from django.core.paginator import Paginator from django.db.models import Model +from django.db.models.manager import BaseManager from typing import List from registrar.utility.enums import LogCode @@ -84,7 +85,7 @@ class PopulateScriptTemplate(ABC): """ raise NotImplementedError - def mass_update_records(self, object_class: Model, filter_conditions, fields_to_update, debug=True, verbose=False, custom_filter=None): + def mass_update_records(self, object_class: Model, filter_conditions, fields_to_update, debug=True, verbose=False): """Loops through each valid "object_class" object - specified by filter_conditions - and updates fields defined by fields_to_update using update_record. @@ -104,10 +105,6 @@ class PopulateScriptTemplate(ABC): verbose: Whether to print a detailed run summary *before* run confirmation. Default: False. - custom_filter: function taking in a single record and returning a boolean representing whether - the record should be included of the final record set. Used to allow for filters that can't be - represented by django queryset field lookups. Applied *after* filter_conditions. - Raises: NotImplementedError: If you do not define update_record before using this function. TypeError: If custom_filter is not Callable. @@ -116,16 +113,7 @@ class PopulateScriptTemplate(ABC): records = object_class.objects.filter(**filter_conditions) # apply custom filter - if custom_filter: - to_exclude_pks = [] - for record in records: - try: - if not custom_filter(record): - to_exclude_pks.append(record.pk) - except TypeError as e: - logger.error(f"Error applying custom filter: {e}") - sys.exit() - records=records.exclude(pk__in=to_exclude_pks) + records=self.custom_filter(records) readable_class_name = self.get_class_name(object_class) @@ -189,10 +177,16 @@ class PopulateScriptTemplate(ABC): def should_skip_record(self, record) -> bool: # noqa """Defines the condition in which we should skip updating a record. Override as needed. - The difference between this and Custom_filter is that records matching these conditions + The difference between this and custom_filter is that records matching these conditions *will* be included in the run but will be skipped (and logged as such).""" # By default - don't skip return False + + def custom_filter(self, records: BaseManager[Model]) -> BaseManager[Model]: + """Override to define filters that can't be represented by django queryset field lookups. + Applied to individual records *after* filter_conditions. True means """ + return records + class TerminalHelper: