From 923ab6bdc0365a9074cce36a09273e989f3d8320 Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Fri, 19 Apr 2024 10:01:22 -0700 Subject: [PATCH 01/34] Add initial script to transition domain info fed agency --- .../commands/update_federal_agency_to_fk.py | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 src/registrar/management/commands/update_federal_agency_to_fk.py diff --git a/src/registrar/management/commands/update_federal_agency_to_fk.py b/src/registrar/management/commands/update_federal_agency_to_fk.py new file mode 100644 index 000000000..8083926df --- /dev/null +++ b/src/registrar/management/commands/update_federal_agency_to_fk.py @@ -0,0 +1,56 @@ +"""" +TODO: write description +""" + +import logging +import copy + +from django.core.management import BaseCommand +from registrar.models import DomainInformation, DomainRequest, FederalAgency + +logger = logging.getLogger(__name__) + + +class Command(BaseCommand): + help = "Transfers Domain Request and Domain Information federal agency field from string to FederalAgency fk" + + def __init__(self): + """Sets global variables for code tidiness""" + super().__init__() + # domains with errors, which do not successfully update federal agency to FederalAgency fk + self.domains_with_errors: list[str] = [] + # domains that successfull update federal agency to FederalAgency fk + self.domains_successfully_updated = 0 + + def handle(self, **options): + """ + Converts all ready and DNS needed domains with a non-default public contact + to disclose their public contact. + """ + logger.info("Transferring federal agencies to FederalAgency foreign key") + + # Initializes domain request and domain info objects that need to update federal agency + # filter out domains with federal agency null or Non-Federal Agency + domain_infos = DomainInformation.objects.filter( + federal_agency__isnull=False + ).exclude( + federal_agency="Non-Federal Agency" + ) + + logger.info(f"Found {len(domain_infos)} DomainInfo objects with federal agency.") + + # Update EPP contact for domains with a security contact + for domain in domain_infos: + # try: + federal_agency = domain.federal_agency # noqa on these items as we only want to call security_contact + logger.info(f"Domain {domain} federal agency: {federal_agency}") + # except Exception as err: + # # error condition if domain not in database + # self.domains_with_errors.append(copy.deepcopy(domain.name)) + # logger.error(f"error retrieving domain {domain.name} contact {domain.security_contact}: {err}") + + # # Inform user how many contacts were disclosed, skipped, and errored + # logger.info(f"Updated {self.disclosed_domain_contacts_count} contacts to disclosed.") + # logger.info( + # f"Error disclosing the following {len(self.domains_with_errors)} contacts: {self.domains_with_errors}" + # ) From b8425c2051f61498e7fe3ea8106ab5d79ab63473 Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Fri, 19 Apr 2024 16:10:46 -0700 Subject: [PATCH 02/34] Add initial script to transition domain info fed agency --- .../populate_domain_updated_federal_agency.py | 103 ++++++++++++++++++ .../commands/update_federal_agency_to_fk.py | 56 ---------- 2 files changed, 103 insertions(+), 56 deletions(-) create mode 100644 src/registrar/management/commands/populate_domain_updated_federal_agency.py delete mode 100644 src/registrar/management/commands/update_federal_agency_to_fk.py diff --git a/src/registrar/management/commands/populate_domain_updated_federal_agency.py b/src/registrar/management/commands/populate_domain_updated_federal_agency.py new file mode 100644 index 000000000..531ee4e27 --- /dev/null +++ b/src/registrar/management/commands/populate_domain_updated_federal_agency.py @@ -0,0 +1,103 @@ +"""" +TODO: write description +""" + +import logging +import copy + +from django.core.management import BaseCommand +from registrar.models import DomainInformation, DomainRequest, FederalAgency +from registrar.management.commands.utility.terminal_helper import ScriptDataHelper + + +logger = logging.getLogger(__name__) + + +class Command(BaseCommand): + help = "Transfers Domain Request and Domain Information federal agency field from string to FederalAgency object" + + # Deprecated federal agency names mapped to designated replacements + rename_deprecated_federal_agency = { + "Appraisal Subcommittee": "Appraisal Subcommittee of the Federal Financial Institutions Examination Council", + "Barry Goldwater Scholarship and Excellence in Education Program": "Barry Goldwater Scholarship and Excellence in Education Foundation", + "Federal Reserve System": "Federal Reserve Board of Governors", + "Harry S Truman Scholarship Foundation": "Harry S. Truman Scholarship Foundation", + "Japan-US Friendship Commission": "Japan-U.S. Friendship Commission", + "Japan-United States Friendship Commission": "Japan-U.S. Friendship Commission", + "John F. Kennedy Center for Performing Arts": "John F. Kennedy Center for the Performing Arts", + "Occupational Safety & Health Review Commission": "Occupational Safety and Health Review Commission", + "Corporation for National & Community Service": "Corporation for National and Community Service", + "Export/Import Bank of the U.S.": "Export-Import Bank of the United States", + "Medical Payment Advisory Commission": "Medicare Payment Advisory Commission", + "U.S. Peace Corps": "Peace Corps", + "Chemical Safety Board": "U.S. Chemical Safety Board", + "Nuclear Waste Technical Review Board": "U.S. Nuclear Waste Technical Review Board", + "State, Local, and Tribal Government": "Non-Federal Agency" + } + + def handle(self, **options): + """ + Converts all ready and DNS needed domains with a non-default public contact + to disclose their public contact. + """ + logger.info("Transferring federal agencies to FederalAgency object") + # DomainInforation object we populate with updated_federal_agency which are then bulk updated + domain_infos_to_update = [] + domain_requests_to_update = [] + # domain requests with null federal_agency that are not populated with updated_federal_agency + domain_requests_skipped = [] + domain_infos_with_errors = [] + domain_requests_with_errors = [] + + # Initializes domain request and domain info objects that need to update federal agency + # filter out domains with federal agency null or Non-Federal Agency + domain_infos = DomainInformation.objects.all() + domain_requests = DomainRequest.objects.all() + + logger.info(f"Found {len(domain_infos)} DomainInfo objects with federal agency.") + + for domain_info in domain_infos: + try: + federal_agency_row = self.find_federal_agency_row(domain_info) + domain_info.updated_federal_agency = federal_agency_row + domain_infos_to_update.append(domain_info) + logger.info(f"DomainInformation {domain_info} updated_federal_agency set to: {domain_info.updated_federal_agency}") + except Exception as err: + logger.info(f"DomainInformation for {domain_information} failed to update updated_federal_agency: {err}") + domain_infos_with_errors.append(domain_info) + + ScriptDataHelper.bulk_update_fields( + DomainInformation, domain_infos_to_update, ["updated_federal_agency"] + ) + + for domain_request in domain_requests: + try: + if not domain_request.federal_agency: + domain_requests_skipped.append(domain_request) + else: + federal_agency_row = self.find_federal_agency_row(domain_request) + domain_request.updated_federal_agency = federal_agency_row + domain_requests_to_update.append(domain_request) + logger.info(f"DomainRequest {domain_request} updated_federal_agency set to: {domain_request.updated_federal_agency}") + except Exception as err: + logger.info(f"DomainRequest for {domain_request} failed to update updated_federal_agency: {err}") + domain_requests_with_errors.append(domain_request) + + ScriptDataHelper.bulk_update_fields( + DomainRequest, domain_requests_to_update, ["updated_federal_agency"] + ) + + logger.info(f"{len(domain_infos_to_update)} DomainInformation rows updated update_federal_agency.") + logger.info(f"{len(domain_infos_with_errors)} DomainInformation rows errored when updating update_federal_agency.") + logger.info(f"{len(domain_requests_to_update)} DomainRequest rows updated update_federal_agency.") + logger.info(f"{len(domain_requests_skipped)} DomainRequest rows with null federal_agency skipped.") + logger.info(f"{len(domain_requests_with_errors)} DomainRequest rows errored when updating update_federal_agency.\n{domain_requests_with_errors}") + + def find_federal_agency_row(self, domain_object): + federal_agency = domain_object.federal_agency + # Domain Information objects without a federal agency default to Non-Federal Agency + if not federal_agency: + federal_agency = "Non-Federal Agency" + if federal_agency in self.rename_deprecated_federal_agency.keys(): + federal_agency = self.rename_deprecated_federal_agency[federal_agency] + return FederalAgency.objects.filter(agency=federal_agency).get() \ No newline at end of file diff --git a/src/registrar/management/commands/update_federal_agency_to_fk.py b/src/registrar/management/commands/update_federal_agency_to_fk.py deleted file mode 100644 index 8083926df..000000000 --- a/src/registrar/management/commands/update_federal_agency_to_fk.py +++ /dev/null @@ -1,56 +0,0 @@ -"""" -TODO: write description -""" - -import logging -import copy - -from django.core.management import BaseCommand -from registrar.models import DomainInformation, DomainRequest, FederalAgency - -logger = logging.getLogger(__name__) - - -class Command(BaseCommand): - help = "Transfers Domain Request and Domain Information federal agency field from string to FederalAgency fk" - - def __init__(self): - """Sets global variables for code tidiness""" - super().__init__() - # domains with errors, which do not successfully update federal agency to FederalAgency fk - self.domains_with_errors: list[str] = [] - # domains that successfull update federal agency to FederalAgency fk - self.domains_successfully_updated = 0 - - def handle(self, **options): - """ - Converts all ready and DNS needed domains with a non-default public contact - to disclose their public contact. - """ - logger.info("Transferring federal agencies to FederalAgency foreign key") - - # Initializes domain request and domain info objects that need to update federal agency - # filter out domains with federal agency null or Non-Federal Agency - domain_infos = DomainInformation.objects.filter( - federal_agency__isnull=False - ).exclude( - federal_agency="Non-Federal Agency" - ) - - logger.info(f"Found {len(domain_infos)} DomainInfo objects with federal agency.") - - # Update EPP contact for domains with a security contact - for domain in domain_infos: - # try: - federal_agency = domain.federal_agency # noqa on these items as we only want to call security_contact - logger.info(f"Domain {domain} federal agency: {federal_agency}") - # except Exception as err: - # # error condition if domain not in database - # self.domains_with_errors.append(copy.deepcopy(domain.name)) - # logger.error(f"error retrieving domain {domain.name} contact {domain.security_contact}: {err}") - - # # Inform user how many contacts were disclosed, skipped, and errored - # logger.info(f"Updated {self.disclosed_domain_contacts_count} contacts to disclosed.") - # logger.info( - # f"Error disclosing the following {len(self.domains_with_errors)} contacts: {self.domains_with_errors}" - # ) From e8d4d4ec7946d66775029f91b555201e947b2804 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 22 Apr 2024 10:31:19 -0700 Subject: [PATCH 03/34] Update description todos --- src/registrar/admin.py | 4 ++-- .../populate_domain_updated_federal_agency.py | 24 ++++++++++++------- 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index fb4c79b8e..75ed7492e 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -979,7 +979,7 @@ class DomainInformationAdmin(ListHeaderAdmin): "classes": ["collapse"], "fields": [ "federal_type", - # "updated_federal_agency", + "updated_federal_agency", # Above field commented out so it won't display "federal_agency", # TODO: remove later "tribe_name", @@ -1220,7 +1220,7 @@ class DomainRequestAdmin(ListHeaderAdmin): "classes": ["collapse"], "fields": [ "federal_type", - # "updated_federal_agency", + "updated_federal_agency", # Above field commented out so it won't display "federal_agency", # TODO: remove later "tribe_name", diff --git a/src/registrar/management/commands/populate_domain_updated_federal_agency.py b/src/registrar/management/commands/populate_domain_updated_federal_agency.py index 531ee4e27..edaccd30a 100644 --- a/src/registrar/management/commands/populate_domain_updated_federal_agency.py +++ b/src/registrar/management/commands/populate_domain_updated_federal_agency.py @@ -37,24 +37,24 @@ class Command(BaseCommand): def handle(self, **options): """ - Converts all ready and DNS needed domains with a non-default public contact - to disclose their public contact. + TODO: Update description here + If it's NULL for a domain request, it should return an error + """ logger.info("Transferring federal agencies to FederalAgency object") - # DomainInforation object we populate with updated_federal_agency which are then bulk updated + # DomainInformation object we populate with updated_federal_agency which are then bulk updated domain_infos_to_update = [] domain_requests_to_update = [] - # domain requests with null federal_agency that are not populated with updated_federal_agency + # Domain Requests with null federal_agency that are not populated with updated_federal_agency domain_requests_skipped = [] domain_infos_with_errors = [] domain_requests_with_errors = [] - # Initializes domain request and domain info objects that need to update federal agency - # filter out domains with federal agency null or Non-Federal Agency domain_infos = DomainInformation.objects.all() domain_requests = DomainRequest.objects.all() logger.info(f"Found {len(domain_infos)} DomainInfo objects with federal agency.") + logger.info(f"Found {len(domain_requests)} Domain Request objects with federal agency.") for domain_info in domain_infos: try: @@ -63,7 +63,7 @@ class Command(BaseCommand): domain_infos_to_update.append(domain_info) logger.info(f"DomainInformation {domain_info} updated_federal_agency set to: {domain_info.updated_federal_agency}") except Exception as err: - logger.info(f"DomainInformation for {domain_information} failed to update updated_federal_agency: {err}") + logger.info(f"DomainInformation for {domain_info} failed to update updated_federal_agency: {err}") domain_infos_with_errors.append(domain_info) ScriptDataHelper.bulk_update_fields( @@ -73,6 +73,8 @@ class Command(BaseCommand): for domain_request in domain_requests: try: if not domain_request.federal_agency: + # TODO: Make sure to clarify this in the description + # If null it's skipped bc user hasn't gotten to it yet domain_requests_skipped.append(domain_request) else: federal_agency_row = self.find_federal_agency_row(domain_request) @@ -94,9 +96,13 @@ class Command(BaseCommand): logger.info(f"{len(domain_requests_with_errors)} DomainRequest rows errored when updating update_federal_agency.\n{domain_requests_with_errors}") def find_federal_agency_row(self, domain_object): + ''' + TODO: Add description + We are grabbing the "previous set ups" Federal Agency + Domain Information objects without a federal agency default to Non-Federal Agency + ''' federal_agency = domain_object.federal_agency - # Domain Information objects without a federal agency default to Non-Federal Agency - if not federal_agency: + if not federal_agency: federal_agency = "Non-Federal Agency" if federal_agency in self.rename_deprecated_federal_agency.keys(): federal_agency = self.rename_deprecated_federal_agency[federal_agency] From 9e5db6ebfef42c77c3c083790aac01d8e10f5b1c Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 22 Apr 2024 13:20:56 -0700 Subject: [PATCH 04/34] Update function descriptions --- .../populate_domain_updated_federal_agency.py | 57 +++++++++++-------- 1 file changed, 33 insertions(+), 24 deletions(-) diff --git a/src/registrar/management/commands/populate_domain_updated_federal_agency.py b/src/registrar/management/commands/populate_domain_updated_federal_agency.py index edaccd30a..9e369089f 100644 --- a/src/registrar/management/commands/populate_domain_updated_federal_agency.py +++ b/src/registrar/management/commands/populate_domain_updated_federal_agency.py @@ -1,5 +1,7 @@ """" -TODO: write description +Data migration: Renaming deprecated Federal Agencies to +their new updated names ie (U.S. Peace Corps to Peace Corps) +within Domain Information and Domain Requests """ import logging @@ -32,14 +34,16 @@ class Command(BaseCommand): "U.S. Peace Corps": "Peace Corps", "Chemical Safety Board": "U.S. Chemical Safety Board", "Nuclear Waste Technical Review Board": "U.S. Nuclear Waste Technical Review Board", - "State, Local, and Tribal Government": "Non-Federal Agency" + "State, Local, and Tribal Government": "Non-Federal Agency", } def handle(self, **options): """ - TODO: Update description here - If it's NULL for a domain request, it should return an error + Renames the Federal Agency to the correct new naming + for both Domain Information and Domain Requests objects. + NOTE: If it's NULL for a domain request, we skip it as + a user most likely hasn't gotten to it yet. """ logger.info("Transferring federal agencies to FederalAgency object") # DomainInformation object we populate with updated_federal_agency which are then bulk updated @@ -61,49 +65,54 @@ class Command(BaseCommand): federal_agency_row = self.find_federal_agency_row(domain_info) domain_info.updated_federal_agency = federal_agency_row domain_infos_to_update.append(domain_info) - logger.info(f"DomainInformation {domain_info} updated_federal_agency set to: {domain_info.updated_federal_agency}") + logger.info( + f"DomainInformation {domain_info} updated_federal_agency set to: {domain_info.updated_federal_agency}" + ) except Exception as err: logger.info(f"DomainInformation for {domain_info} failed to update updated_federal_agency: {err}") domain_infos_with_errors.append(domain_info) - ScriptDataHelper.bulk_update_fields( - DomainInformation, domain_infos_to_update, ["updated_federal_agency"] - ) + ScriptDataHelper.bulk_update_fields(DomainInformation, domain_infos_to_update, ["updated_federal_agency"]) for domain_request in domain_requests: try: if not domain_request.federal_agency: - # TODO: Make sure to clarify this in the description - # If null it's skipped bc user hasn't gotten to it yet domain_requests_skipped.append(domain_request) else: federal_agency_row = self.find_federal_agency_row(domain_request) domain_request.updated_federal_agency = federal_agency_row domain_requests_to_update.append(domain_request) - logger.info(f"DomainRequest {domain_request} updated_federal_agency set to: {domain_request.updated_federal_agency}") + logger.info( + f"DomainRequest {domain_request} updated_federal_agency set to: {domain_request.updated_federal_agency}" + ) except Exception as err: logger.info(f"DomainRequest for {domain_request} failed to update updated_federal_agency: {err}") domain_requests_with_errors.append(domain_request) - ScriptDataHelper.bulk_update_fields( - DomainRequest, domain_requests_to_update, ["updated_federal_agency"] - ) - + ScriptDataHelper.bulk_update_fields(DomainRequest, domain_requests_to_update, ["updated_federal_agency"]) + logger.info(f"{len(domain_infos_to_update)} DomainInformation rows updated update_federal_agency.") - logger.info(f"{len(domain_infos_with_errors)} DomainInformation rows errored when updating update_federal_agency.") + logger.info( + f"{len(domain_infos_with_errors)} DomainInformation rows errored when updating update_federal_agency." + ) logger.info(f"{len(domain_requests_to_update)} DomainRequest rows updated update_federal_agency.") logger.info(f"{len(domain_requests_skipped)} DomainRequest rows with null federal_agency skipped.") - logger.info(f"{len(domain_requests_with_errors)} DomainRequest rows errored when updating update_federal_agency.\n{domain_requests_with_errors}") + logger.info( + f"{len(domain_requests_with_errors)} DomainRequest rows errored when updating update_federal_agency.\n{domain_requests_with_errors}" + ) def find_federal_agency_row(self, domain_object): - ''' - TODO: Add description - We are grabbing the "previous set ups" Federal Agency - Domain Information objects without a federal agency default to Non-Federal Agency - ''' + """ + We grab the "old" federal agency object to rename and set the new object. + If the old name is null, we set it to "Non-Federal Agency". + Otherwise, we grab the key of the older name in + the rename_deprecated_federal_agency list and set it to the + value which is the new updated name we want and then grab that object + """ + federal_agency = domain_object.federal_agency - if not federal_agency: + if not federal_agency: federal_agency = "Non-Federal Agency" if federal_agency in self.rename_deprecated_federal_agency.keys(): federal_agency = self.rename_deprecated_federal_agency[federal_agency] - return FederalAgency.objects.filter(agency=federal_agency).get() \ No newline at end of file + return FederalAgency.objects.filter(agency=federal_agency).get() From 556aee54fe47fb5f8230022d518ec9ce09f8f0dd Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 22 Apr 2024 15:23:30 -0700 Subject: [PATCH 05/34] Add attempt at unit tests --- src/registrar/tests/common.py | 12 +++- .../tests/test_management_scripts.py | 69 +++++++++++++++++++ 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 6dd88c1c1..cdc986888 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -26,6 +26,7 @@ from registrar.models import ( DomainInformation, PublicContact, Domain, + FederalAgency, ) from epplibwrapper import ( commands, @@ -803,6 +804,8 @@ def completed_domain_request( generic_org_type="federal", is_election_board=False, organization_type=None, + federal_agency=None, + updated_federal_agency=None, ): """A completed domain request.""" if not user: @@ -839,6 +842,8 @@ def completed_domain_request( last_name="Bob", is_staff=True, ) + if not updated_federal_agency: + updated_federal_agency, _ = FederalAgency.objects.get_or_create(agency="Stitches Is The Best") domain_request_kwargs = dict( generic_org_type=generic_org_type, is_election_board=is_election_board, @@ -856,6 +861,8 @@ def completed_domain_request( creator=user, status=status, investigator=investigator, + federal_agency=federal_agency, + updated_federal_agency=updated_federal_agency, ) if has_about_your_organization: domain_request_kwargs["about_your_organization"] = "e-Government" @@ -864,7 +871,10 @@ def completed_domain_request( if organization_type: domain_request_kwargs["organization_type"] = organization_type - + # if federal_agency: + # domain_request_kwargs["federal_agency"] = federal_agency + # if updated_federal_agency: + # domain_request_kwargs["updated_federal_agency"] = updated_federal_agency domain_request, _ = DomainRequest.objects.get_or_create(**domain_request_kwargs) if has_other_contacts: diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 26161b272..f2864417e 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -14,6 +14,7 @@ from registrar.models import ( TransitionDomain, DomainInformation, UserDomainRole, + FederalAgency, ) from registrar.models.public_contact import PublicContact @@ -743,3 +744,71 @@ class TestDiscloseEmails(MockEppLib): ) ] ) + + +class TestRenamingFederalAgency(MockEppLib): + def setUp(self): + super().setUp() + + # Get the domain requests + self.domain_request_1 = completed_domain_request( + name="stitches.gov", + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + is_election_board=True, + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + federal_agency="U.S. Peace Corps", + ) + self.outdated_federal_agency = FederalAgency.objects.get_or_create(agency="U.S. Peace Corps") + self.corrected_federal_agency = FederalAgency.objects.get_or_create(agency="Peace Corps") + + # Approve all three requests + self.domain_request_1.approve() + + # Get the domains + self.domain_1 = Domain.objects.get(name="stitches.gov") + + # Get the domain infos + self.domain_info_1 = DomainInformation.objects.get(domain=self.domain_1) + + def tearDown(self): + super().tearDown() + DomainInformation.objects.all().delete() + DomainRequest.objects.all().delete() + Domain.objects.all().delete() + + def run_populate_domain_updated_federal_agency(self): + """ + This method executes the populate_domain_updated_federal_agency command. + + The 'call_command' function from Django's management framework is then used to + execute the populate_domain_updated_federal_agency command. + """ + # with less_console_noise(): + print("!! We are in run_populate_domain_updated_federal_agency") + with patch( + "registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit", # noqa + return_value=True, + ): + call_command("populate_domain_updated_federal_agency") + + def test_domain_information_renaming_federal_agency_success(self): + """ + 1. Domain Information Update an outdated Federal Agency + 2. Domain Information should error out on null Update a Federal Agency that doesn't exist (should error out) + 2a. Domain Request should just skip? (maybe) + 3. Domain Request Updating a Null Federal Agency make sure it's updated to Non-Federal Agency + TODO: Have a todo for the next ticket pt 3 to remove the tests here RIP + """ + + # Test case #1 + self.run_populate_domain_updated_federal_agency() + print("!! self.domain_info_1 is", self.domain_info_1) + print("!! self.domain_info_1 dictionary is", self.domain_info_1.__dict__) + + previous_federal_agency_name = self.domain_info_1.federal_agency + updated_federal_agency_name = self.domain_info_1.updated_federal_agency + print("!! previous_federal_agency_name is ", previous_federal_agency_name) + print("!! updated_federal_agency_name is ", updated_federal_agency_name) + + self.assertEqual(previous_federal_agency_name, "U.S. Peace Corps") + self.assertEqual(updated_federal_agency_name, "Peace Corps") From a7f7e5501e86d9901e517de804a2f3292467ef47 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 23 Apr 2024 11:19:56 -0700 Subject: [PATCH 06/34] Add unit tests and update descriptions --- .../populate_domain_updated_federal_agency.py | 2 +- src/registrar/tests/common.py | 8 +- .../tests/test_management_scripts.py | 94 ++++++++++++++----- 3 files changed, 76 insertions(+), 28 deletions(-) diff --git a/src/registrar/management/commands/populate_domain_updated_federal_agency.py b/src/registrar/management/commands/populate_domain_updated_federal_agency.py index 9e369089f..f40e5be89 100644 --- a/src/registrar/management/commands/populate_domain_updated_federal_agency.py +++ b/src/registrar/management/commands/populate_domain_updated_federal_agency.py @@ -18,7 +18,7 @@ logger = logging.getLogger(__name__) class Command(BaseCommand): help = "Transfers Domain Request and Domain Information federal agency field from string to FederalAgency object" - # Deprecated federal agency names mapped to designated replacements + # Deprecated federal agency names mapped to designated replacements {old_value, new value} rename_deprecated_federal_agency = { "Appraisal Subcommittee": "Appraisal Subcommittee of the Federal Financial Institutions Examination Council", "Barry Goldwater Scholarship and Excellence in Education Program": "Barry Goldwater Scholarship and Excellence in Education Foundation", diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index cdc986888..8499925d8 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -790,6 +790,7 @@ def create_ready_domain(): return domain +# TODO in 1793: Remove the federal agency/updated federal agency fields def completed_domain_request( has_other_contacts=True, has_current_website=True, @@ -842,8 +843,7 @@ def completed_domain_request( last_name="Bob", is_staff=True, ) - if not updated_federal_agency: - updated_federal_agency, _ = FederalAgency.objects.get_or_create(agency="Stitches Is The Best") + domain_request_kwargs = dict( generic_org_type=generic_org_type, is_election_board=is_election_board, @@ -871,10 +871,6 @@ def completed_domain_request( if organization_type: domain_request_kwargs["organization_type"] = organization_type - # if federal_agency: - # domain_request_kwargs["federal_agency"] = federal_agency - # if updated_federal_agency: - # domain_request_kwargs["updated_federal_agency"] = updated_federal_agency domain_request, _ = DomainRequest.objects.get_or_create(**domain_request_kwargs) if has_other_contacts: diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index f2864417e..c40414b6c 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -746,7 +746,8 @@ class TestDiscloseEmails(MockEppLib): ) -class TestRenamingFederalAgency(MockEppLib): +# TODO in #1793: Remove this whole test class +class TestRenamingFederalAgency(TestCase): def setUp(self): super().setUp() @@ -758,17 +759,35 @@ class TestRenamingFederalAgency(MockEppLib): status=DomainRequest.DomainRequestStatus.IN_REVIEW, federal_agency="U.S. Peace Corps", ) - self.outdated_federal_agency = FederalAgency.objects.get_or_create(agency="U.S. Peace Corps") - self.corrected_federal_agency = FederalAgency.objects.get_or_create(agency="Peace Corps") + self.domain_request_2 = completed_domain_request( + name="fadoesntexist.gov", + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + is_election_board=True, + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + federal_agency="MEOWARDRULES", + ) + self.domain_request_3 = completed_domain_request( + name="nullfederalagency.gov", + generic_org_type=DomainRequest.OrganizationChoices.FEDERAL, + is_election_board=True, + status=DomainRequest.DomainRequestStatus.IN_REVIEW, + federal_agency=None, + ) # Approve all three requests self.domain_request_1.approve() + self.domain_request_2.approve() + self.domain_request_3.approve() # Get the domains self.domain_1 = Domain.objects.get(name="stitches.gov") + self.domain_2 = Domain.objects.get(name="fadoesntexist.gov") + self.domain_3 = Domain.objects.get(name="nullfederalagency.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) def tearDown(self): super().tearDown() @@ -783,32 +802,65 @@ class TestRenamingFederalAgency(MockEppLib): The 'call_command' function from Django's management framework is then used to execute the populate_domain_updated_federal_agency command. """ - # with less_console_noise(): - print("!! We are in run_populate_domain_updated_federal_agency") - with patch( - "registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit", # noqa - return_value=True, - ): - call_command("populate_domain_updated_federal_agency") + with less_console_noise(): + with patch( + "registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit", # noqa + return_value=True, + ): + call_command("populate_domain_updated_federal_agency") def test_domain_information_renaming_federal_agency_success(self): """ - 1. Domain Information Update an outdated Federal Agency - 2. Domain Information should error out on null Update a Federal Agency that doesn't exist (should error out) - 2a. Domain Request should just skip? (maybe) - 3. Domain Request Updating a Null Federal Agency make sure it's updated to Non-Federal Agency - TODO: Have a todo for the next ticket pt 3 to remove the tests here RIP + Domain Information updates successfully for an "outdated" Federal Agency """ - # Test case #1 self.run_populate_domain_updated_federal_agency() - print("!! self.domain_info_1 is", self.domain_info_1) - print("!! self.domain_info_1 dictionary is", self.domain_info_1.__dict__) + + self.domain_info_1.refresh_from_db() previous_federal_agency_name = self.domain_info_1.federal_agency - updated_federal_agency_name = self.domain_info_1.updated_federal_agency - print("!! previous_federal_agency_name is ", previous_federal_agency_name) - print("!! updated_federal_agency_name is ", updated_federal_agency_name) + updated_federal_agency_name = self.domain_info_1.updated_federal_agency.agency self.assertEqual(previous_federal_agency_name, "U.S. Peace Corps") self.assertEqual(updated_federal_agency_name, "Peace Corps") + + def test_domain_information_does_not_exist(self): + """ + Update a Federal Agency that doesn't exist + (should return None bc the Federal Agency didn't exist before) + """ + + self.run_populate_domain_updated_federal_agency() + + self.domain_info_2.refresh_from_db() + + self.assertEqual(self.domain_info_2.updated_federal_agency, None) + + def test_domain_request_is_skipped(self): + """ + Update a Domain Request that doesn't exist + (should return None bc the Federal Agency didn't exist before) + """ + + # Test case #2 + self.run_populate_domain_updated_federal_agency() + + self.domain_request_2.refresh_from_db() + + self.assertEqual(self.domain_request_2.updated_federal_agency, None) + + def test_domain_information_updating_null_federal_agency_to_non_federal_agency(self): + """ + Updating a Domain Information that was previously None + to Non-Federal Agency + """ + + self.run_populate_domain_updated_federal_agency() + + self.domain_info_3.refresh_from_db() + + previous_federal_agency_name = self.domain_info_3.federal_agency + updated_federal_agency_name = self.domain_info_3.updated_federal_agency.agency + + self.assertEqual(previous_federal_agency_name, None) + self.assertEqual(updated_federal_agency_name, "Non-Federal Agency") From 2088330db08d672336efec986db2a79d7ff4a6b3 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 23 Apr 2024 11:22:51 -0700 Subject: [PATCH 07/34] Leave todo reminder for updatefederalagency viewing in admin --- src/registrar/admin.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 75ed7492e..36f441e86 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -979,6 +979,7 @@ class DomainInformationAdmin(ListHeaderAdmin): "classes": ["collapse"], "fields": [ "federal_type", + # TODO 1975 BEFORE MERGING: COMMENT BELOW OUT "updated_federal_agency", # Above field commented out so it won't display "federal_agency", # TODO: remove later @@ -1220,6 +1221,7 @@ class DomainRequestAdmin(ListHeaderAdmin): "classes": ["collapse"], "fields": [ "federal_type", + # TODO 1975 BEFORE MERGING: COMMENT BELOW OUT "updated_federal_agency", # Above field commented out so it won't display "federal_agency", # TODO: remove later From 070ff6223a8ee1a6fd562a8d1fd6fe2d9cdcd6d0 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 23 Apr 2024 11:46:33 -0700 Subject: [PATCH 08/34] Fix linter issues --- .../populate_domain_updated_federal_agency.py | 13 ++++++++----- src/registrar/tests/common.py | 1 - src/registrar/tests/test_management_scripts.py | 1 - 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/registrar/management/commands/populate_domain_updated_federal_agency.py b/src/registrar/management/commands/populate_domain_updated_federal_agency.py index f40e5be89..5cfadd044 100644 --- a/src/registrar/management/commands/populate_domain_updated_federal_agency.py +++ b/src/registrar/management/commands/populate_domain_updated_federal_agency.py @@ -5,7 +5,6 @@ within Domain Information and Domain Requests """ import logging -import copy from django.core.management import BaseCommand from registrar.models import DomainInformation, DomainRequest, FederalAgency @@ -21,7 +20,8 @@ class Command(BaseCommand): # Deprecated federal agency names mapped to designated replacements {old_value, new value} rename_deprecated_federal_agency = { "Appraisal Subcommittee": "Appraisal Subcommittee of the Federal Financial Institutions Examination Council", - "Barry Goldwater Scholarship and Excellence in Education Program": "Barry Goldwater Scholarship and Excellence in Education Foundation", + "Barry Goldwater Scholarship and Excellence in Education Program": + "Barry Goldwater Scholarship and Excellence in Education Foundation", "Federal Reserve System": "Federal Reserve Board of Governors", "Harry S Truman Scholarship Foundation": "Harry S. Truman Scholarship Foundation", "Japan-US Friendship Commission": "Japan-U.S. Friendship Commission", @@ -66,7 +66,8 @@ class Command(BaseCommand): domain_info.updated_federal_agency = federal_agency_row domain_infos_to_update.append(domain_info) logger.info( - f"DomainInformation {domain_info} updated_federal_agency set to: {domain_info.updated_federal_agency}" + f"DomainInformation {domain_info} updated_federal_agency set to:" + f"{domain_info.updated_federal_agency}" ) except Exception as err: logger.info(f"DomainInformation for {domain_info} failed to update updated_federal_agency: {err}") @@ -83,7 +84,8 @@ class Command(BaseCommand): domain_request.updated_federal_agency = federal_agency_row domain_requests_to_update.append(domain_request) logger.info( - f"DomainRequest {domain_request} updated_federal_agency set to: {domain_request.updated_federal_agency}" + f"DomainRequest {domain_request} updated_federal_agency set to:" + f"{domain_request.updated_federal_agency}" ) except Exception as err: logger.info(f"DomainRequest for {domain_request} failed to update updated_federal_agency: {err}") @@ -98,7 +100,8 @@ class Command(BaseCommand): logger.info(f"{len(domain_requests_to_update)} DomainRequest rows updated update_federal_agency.") logger.info(f"{len(domain_requests_skipped)} DomainRequest rows with null federal_agency skipped.") logger.info( - f"{len(domain_requests_with_errors)} DomainRequest rows errored when updating update_federal_agency.\n{domain_requests_with_errors}" + f"{len(domain_requests_with_errors)} DomainRequest rows errored when updating update_federal_agency." + f"{domain_requests_with_errors}" ) def find_federal_agency_row(self, domain_object): diff --git a/src/registrar/tests/common.py b/src/registrar/tests/common.py index 8499925d8..c2f11b85d 100644 --- a/src/registrar/tests/common.py +++ b/src/registrar/tests/common.py @@ -26,7 +26,6 @@ from registrar.models import ( DomainInformation, PublicContact, Domain, - FederalAgency, ) from epplibwrapper import ( commands, diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index c40414b6c..746c4f8ab 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -14,7 +14,6 @@ from registrar.models import ( TransitionDomain, DomainInformation, UserDomainRole, - FederalAgency, ) from registrar.models.public_contact import PublicContact From 8bde2a25af5de127f345b4fed91a04f5459959ac Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 23 Apr 2024 11:54:30 -0700 Subject: [PATCH 09/34] Linter fix for a long line --- .../commands/populate_domain_updated_federal_agency.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/registrar/management/commands/populate_domain_updated_federal_agency.py b/src/registrar/management/commands/populate_domain_updated_federal_agency.py index 5cfadd044..176849413 100644 --- a/src/registrar/management/commands/populate_domain_updated_federal_agency.py +++ b/src/registrar/management/commands/populate_domain_updated_federal_agency.py @@ -20,8 +20,7 @@ class Command(BaseCommand): # Deprecated federal agency names mapped to designated replacements {old_value, new value} rename_deprecated_federal_agency = { "Appraisal Subcommittee": "Appraisal Subcommittee of the Federal Financial Institutions Examination Council", - "Barry Goldwater Scholarship and Excellence in Education Program": - "Barry Goldwater Scholarship and Excellence in Education Foundation", + "Barry Goldwater Scholarship and Excellence in Education Program": "Barry Goldwater Scholarship and Excellence in Education Foundation", # noqa "Federal Reserve System": "Federal Reserve Board of Governors", "Harry S Truman Scholarship Foundation": "Harry S. Truman Scholarship Foundation", "Japan-US Friendship Commission": "Japan-U.S. Friendship Commission", From ed9625e373d490cb66dbbad6aeaf9fad629d8386 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 25 Apr 2024 16:33:36 -0400 Subject: [PATCH 10/34] Implement audit trail for status changes on domain request change form --- src/registrar/models/domain_request.py | 9 +++++ .../admin/includes/detail_table_fieldset.html | 32 ++++++++++++++- src/registrar/tests/test_admin.py | 40 +++++++++++++++++++ 3 files changed, 80 insertions(+), 1 deletion(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 9ed35f489..234e0746d 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -16,12 +16,21 @@ from .utility.time_stamped_model import TimeStampedModel from ..utility.email import send_templated_email, EmailSendingError from itertools import chain +from auditlog.models import AuditlogHistoryField # type: ignore + logger = logging.getLogger(__name__) class DomainRequest(TimeStampedModel): """A registrant's domain request for a new domain.""" + # https://django-auditlog.readthedocs.io/en/latest/usage.html#object-history + # If we note any performace degradation due to this addition, + # we can query the auditlogs table in admin.py and add the results to + # extra_context in the change_view method for DomainRequestAdmin + # This is the more straightforward way so trying it first. + history = AuditlogHistoryField() + # Constants for choice fields class DomainRequestStatus(models.TextChoices): STARTED = "started", "Started" diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index 26baddff7..645f29bea 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -73,7 +73,37 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% block after_help_text %} - {% if field.field.name == "creator" %} + {% if field.field.name == "status" and original_object.history.count > 0 %} +
+ +
+ + + + + + + + + + + {% for log_entry in original_object.history.all %} + {% for key, value in log_entry.changes_display_dict.items %} + {% if key == "status" %} + + + + + + + {% endif %} + {% endfor %} + {% endfor %} + +
FromToChanged ByChange Date
{{ value.0|default:"None" }}{{ value.1|default:"None" }}{{ log_entry.actor|default:"None" }}{{ log_entry.timestamp|default:"None" }}
+
+
+ {% elif field.field.name == "creator" %}
{% include "django/admin/includes/contact_detail_list.html" with user=original_object.creator no_title_top_padding=field.is_readonly %} diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 42baae6ef..9b6ce6c09 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -887,6 +887,46 @@ class TestDomainRequestAdmin(MockEppLib): ] self.test_helper.assert_response_contains_distinct_values(response, expected_values) + @less_console_noise_decorator + def test_status_logs(self): + """ + Tests that the status changes are shown in a table on the domain request change form + """ + + # Create a fake domain request and domain + domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.STARTED) + + p = "adminpass" + self.client.login(username="superuser", password=p) + response = self.client.get( + "/admin/registrar/domainrequest/{}/change/".format(domain_request.pk), + follow=True, + ) + + # Make sure the page loaded, and that we're on the right page + self.assertEqual(response.status_code, 200) + self.assertContains(response, domain_request.requested_domain.name) + + # Table will contain From None To Started + self.assertContains(response, '') + self.assertContains(response, "None") + self.assertContains(response, "Started", count=1) + self.assertNotContains(response, "Submitted") + + domain_request.submit() + domain_request.save() + + response = self.client.get( + "/admin/registrar/domainrequest/{}/change/".format(domain_request.pk), + follow=True, + ) + + # Table will contain From None To Started + # Table will contain From Started To Submitted + self.assertContains(response, "None") + self.assertContains(response, "Started", count=2) + self.assertContains(response, "Submitted") + @less_console_noise_decorator def test_analyst_can_see_and_edit_alternative_domain(self): """Tests if an analyst can still see and edit the alternative domain field""" From 87e8310c6da76935b962b5e46d5518d72d00814c Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 25 Apr 2024 17:25:06 -0400 Subject: [PATCH 11/34] Set auditlog migration vars to False --- src/registrar/config/settings.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index 9817476bb..ff9302a40 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -785,6 +785,6 @@ if DEBUG: # Run: # cf run-task getgov-<> --wait --command 'python manage.py auditlogmigratejson --traceback' --name auditlogmigratejson # on our staging and stable, then remove these 2 variables or set to False -AUDITLOG_TWO_STEP_MIGRATION = True +AUDITLOG_TWO_STEP_MIGRATION = False -AUDITLOG_USE_TEXT_CHANGES_IF_JSON_IS_NOT_PRESENT = True +AUDITLOG_USE_TEXT_CHANGES_IF_JSON_IS_NOT_PRESENT = False From 8a8a6491f99bdc2736167de8042ffe9138f69368 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 25 Apr 2024 18:44:00 -0400 Subject: [PATCH 12/34] Teak table design and sorting --- src/registrar/assets/sass/_theme/_admin.scss | 19 ++++++++++++++++++- .../admin/includes/detail_table_fieldset.html | 11 +++++------ src/registrar/templatetags/custom_filters.py | 6 ++++++ 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/registrar/assets/sass/_theme/_admin.scss b/src/registrar/assets/sass/_theme/_admin.scss index f5717d067..e0db9ac57 100644 --- a/src/registrar/assets/sass/_theme/_admin.scss +++ b/src/registrar/assets/sass/_theme/_admin.scss @@ -112,12 +112,20 @@ html[data-theme="light"] { .change-list .usa-table--borderless thead th, .change-list .usa-table thead td, .change-list .usa-table thead th, + .change-form .usa-table, + .change-form .usa-table--striped tbody tr:nth-child(odd) td, + .change-form .usa-table--borderless thead th, + .change-form .usa-table thead td, + .change-form .usa-table thead th, body.dashboard, body.change-list, body.change-form, .analytics { color: var(--body-fg); } + .usa-table td { + background-color: transparent; + } } // Firefox needs this to be specifically set @@ -127,11 +135,20 @@ html[data-theme="dark"] { .change-list .usa-table--borderless thead th, .change-list .usa-table thead td, .change-list .usa-table thead th, + .change-form .usa-table, + .change-form .usa-table--striped tbody tr:nth-child(odd) td, + .change-form .usa-table--borderless thead th, + .change-form .usa-table thead td, + .change-form .usa-table thead th, body.dashboard, body.change-list, - body.change-form { + body.change-form, + .analytics { color: var(--body-fg); } + .usa-table td { + background-color: transparent; + } } #branding h1 a:link, #branding h1 a:visited { diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index 645f29bea..92eb433a9 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -1,5 +1,6 @@ {% extends "admin/fieldset.html" %} {% load static url_helpers %} +{% load custom_filters %} {% comment %} This is using a custom implementation fieldset.html (see admin/fieldset.html) @@ -80,18 +81,16 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) - - - - + + + - {% for log_entry in original_object.history.all %} + {% for log_entry in original_object.history.all|reverse_list %} {% for key, value in log_entry.changes_display_dict.items %} {% if key == "status" %} - diff --git a/src/registrar/templatetags/custom_filters.py b/src/registrar/templatetags/custom_filters.py index 9fa5c9aa9..bbed7b57c 100644 --- a/src/registrar/templatetags/custom_filters.py +++ b/src/registrar/templatetags/custom_filters.py @@ -67,3 +67,9 @@ def get_organization_long_name(generic_org_type): @register.filter(name="has_permission") def has_permission(user, permission): return user.has_perm(permission) + + +@register.filter(name='reverse_list') +def reverse_list(value): + return reversed(value) + From 909a9689f8db61b83c9884207ce335c96af2837f Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 25 Apr 2024 18:47:06 -0400 Subject: [PATCH 13/34] lint --- src/registrar/templatetags/custom_filters.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/registrar/templatetags/custom_filters.py b/src/registrar/templatetags/custom_filters.py index bbed7b57c..a9e546891 100644 --- a/src/registrar/templatetags/custom_filters.py +++ b/src/registrar/templatetags/custom_filters.py @@ -69,7 +69,6 @@ def has_permission(user, permission): return user.has_perm(permission) -@register.filter(name='reverse_list') +@register.filter(name="reverse_list") def reverse_list(value): return reversed(value) - From 0cdfa907ada05cedb687d9b4d211bee2d56879c3 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 25 Apr 2024 19:08:03 -0400 Subject: [PATCH 14/34] Revise unit test to account for ordering --- src/registrar/tests/test_admin.py | 69 +++++++++++++++++++++++++++---- 1 file changed, 60 insertions(+), 9 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 6de4bd181..89760ce24 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -890,7 +890,8 @@ class TestDomainRequestAdmin(MockEppLib): @less_console_noise_decorator def test_status_logs(self): """ - Tests that the status changes are shown in a table on the domain request change form + Tests that the status changes are shown in a table on the domain request change form, + accurately and in chronological order. """ # Create a fake domain request and domain @@ -907,9 +908,7 @@ class TestDomainRequestAdmin(MockEppLib): self.assertEqual(response.status_code, 200) self.assertContains(response, domain_request.requested_domain.name) - # Table will contain From None To Started - self.assertContains(response, '") + # Table will contain one row for Started self.assertContains(response, "", count=1) self.assertNotContains(response, "") @@ -921,11 +920,63 @@ class TestDomainRequestAdmin(MockEppLib): follow=True, ) - # Table will contain From None To Started - # Table will contain From Started To Submitted - self.assertContains(response, "") - self.assertContains(response, "", count=2) - self.assertContains(response, "") + # Table will contain and extra row for Submitted + self.assertContains(response, "", count=1) + self.assertContains(response, "", count=1) + + domain_request.in_review() + domain_request.save() + + response = self.client.get( + "/admin/registrar/domainrequest/{}/change/".format(domain_request.pk), + follow=True, + ) + + # Table will contain and extra row for In review + self.assertContains(response, "", count=1) + self.assertContains(response, "", count=1) + self.assertContains(response, "", count=1) + + domain_request.action_needed() + domain_request.save() + + response = self.client.get( + "/admin/registrar/domainrequest/{}/change/".format(domain_request.pk), + follow=True, + ) + + # Table will contain and extra row for Action needed + self.assertContains(response, "", count=1) + self.assertContains(response, "", count=1) + self.assertContains(response, "", count=1) + self.assertContains(response, "", count=1) + + domain_request.in_review() + domain_request.save() + + response = self.client.get( + "/admin/registrar/domainrequest/{}/change/".format(domain_request.pk), + follow=True, + ) + + # Define the expected sequence of status changes + expected_status_changes = [ + "", + "", + "", + "", + "", + ] + + # Test for the order of status changes + for status_change in expected_status_changes: + self.assertContains(response, status_change, html=True) + + # Table now contains 2 rows for Approved + self.assertContains(response, "", count=1) + self.assertContains(response, "", count=1) + self.assertContains(response, "", count=2) + self.assertContains(response, "", count=1) @less_console_noise_decorator def test_analyst_can_see_and_edit_alternative_domain(self): From 11f8081c5bbd4f17e19dd0c636708c2015f292d2 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Fri, 26 Apr 2024 13:14:11 -0700 Subject: [PATCH 15/34] Address feedback --- .../commands/populate_domain_updated_federal_agency.py | 6 +++--- src/registrar/tests/test_management_scripts.py | 8 ++------ 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/registrar/management/commands/populate_domain_updated_federal_agency.py b/src/registrar/management/commands/populate_domain_updated_federal_agency.py index 176849413..5443d3411 100644 --- a/src/registrar/management/commands/populate_domain_updated_federal_agency.py +++ b/src/registrar/management/commands/populate_domain_updated_federal_agency.py @@ -41,7 +41,7 @@ class Command(BaseCommand): Renames the Federal Agency to the correct new naming for both Domain Information and Domain Requests objects. - NOTE: If it's NULL for a domain request, we skip it as + NOTE: If it's None for a domain request, we skip it as a user most likely hasn't gotten to it yet. """ logger.info("Transferring federal agencies to FederalAgency object") @@ -65,7 +65,7 @@ class Command(BaseCommand): domain_info.updated_federal_agency = federal_agency_row domain_infos_to_update.append(domain_info) logger.info( - f"DomainInformation {domain_info} updated_federal_agency set to:" + f"DomainInformation {domain_info} => updated_federal_agency set to:" f"{domain_info.updated_federal_agency}" ) except Exception as err: @@ -83,7 +83,7 @@ class Command(BaseCommand): domain_request.updated_federal_agency = federal_agency_row domain_requests_to_update.append(domain_request) logger.info( - f"DomainRequest {domain_request} updated_federal_agency set to:" + f"DomainRequest {domain_request} => updated_federal_agency set to:" f"{domain_request.updated_federal_agency}" ) except Exception as err: diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 746c4f8ab..1bb6c59b7 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -746,7 +746,7 @@ class TestDiscloseEmails(MockEppLib): # TODO in #1793: Remove this whole test class -class TestRenamingFederalAgency(TestCase): +class TestPopulateDomainUpdatedFederalAgency(TestCase): def setUp(self): super().setUp() @@ -802,11 +802,7 @@ class TestRenamingFederalAgency(TestCase): execute the populate_domain_updated_federal_agency command. """ with less_console_noise(): - with patch( - "registrar.management.commands.utility.terminal_helper.TerminalHelper.query_yes_no_exit", # noqa - return_value=True, - ): - call_command("populate_domain_updated_federal_agency") + call_command("populate_domain_updated_federal_agency") def test_domain_information_renaming_federal_agency_success(self): """ From df2f21a7564a59a7f304d71c99e9ddb19fdb9730 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Mon, 29 Apr 2024 09:33:48 -0700 Subject: [PATCH 16/34] Comment out updatedfederalagency in admin --- src/registrar/admin.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index ac5510660..7e50701a6 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1007,8 +1007,7 @@ class DomainInformationAdmin(ListHeaderAdmin): "description": "Extends type of organization", "fields": [ "federal_type", - # TODO 1975 BEFORE MERGING: COMMENT BELOW OUT - "updated_federal_agency", + # "updated_federal_agency", # Above field commented out so it won't display "federal_agency", # TODO: remove later "tribe_name", @@ -1261,8 +1260,7 @@ class DomainRequestAdmin(ListHeaderAdmin): "description": "Extends type of organization", "fields": [ "federal_type", - # TODO 1975 BEFORE MERGING: COMMENT BELOW OUT - "updated_federal_agency", + # "updated_federal_agency", # Above field commented out so it won't display "federal_agency", # TODO: remove later "tribe_name", From 7086884c95993d8389cbe2130a6b71c541785b6a Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Mon, 29 Apr 2024 15:48:46 -0700 Subject: [PATCH 17/34] Change not checks to is None --- .../populate_domain_updated_federal_agency.py | 52 +++++++++++-------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/src/registrar/management/commands/populate_domain_updated_federal_agency.py b/src/registrar/management/commands/populate_domain_updated_federal_agency.py index 531ee4e27..a427c1811 100644 --- a/src/registrar/management/commands/populate_domain_updated_federal_agency.py +++ b/src/registrar/management/commands/populate_domain_updated_federal_agency.py @@ -32,9 +32,18 @@ class Command(BaseCommand): "U.S. Peace Corps": "Peace Corps", "Chemical Safety Board": "U.S. Chemical Safety Board", "Nuclear Waste Technical Review Board": "U.S. Nuclear Waste Technical Review Board", - "State, Local, and Tribal Government": "Non-Federal Agency" + "State, Local, and Tribal Government": "Non-Federal Agency", } + def find_federal_agency_row(self, domain_object): + federal_agency = domain_object.federal_agency + # Domain Information objects without a federal agency default to Non-Federal Agency + if federal_agency is None: + federal_agency = "Non-Federal Agency" + if federal_agency in self.rename_deprecated_federal_agency.keys(): + federal_agency = self.rename_deprecated_federal_agency[federal_agency] + return FederalAgency.objects.filter(agency=federal_agency).get() + def handle(self, **options): """ Converts all ready and DNS needed domains with a non-default public contact @@ -61,43 +70,40 @@ class Command(BaseCommand): federal_agency_row = self.find_federal_agency_row(domain_info) domain_info.updated_federal_agency = federal_agency_row domain_infos_to_update.append(domain_info) - logger.info(f"DomainInformation {domain_info} updated_federal_agency set to: {domain_info.updated_federal_agency}") + logger.info( + f"DomainInformation {domain_info} updated_federal_agency set to: {domain_info.updated_federal_agency}" + ) except Exception as err: - logger.info(f"DomainInformation for {domain_information} failed to update updated_federal_agency: {err}") + logger.info( + f"DomainInformation for {domain_information} failed to update updated_federal_agency: {err}" + ) domain_infos_with_errors.append(domain_info) - ScriptDataHelper.bulk_update_fields( - DomainInformation, domain_infos_to_update, ["updated_federal_agency"] - ) + ScriptDataHelper.bulk_update_fields(DomainInformation, domain_infos_to_update, ["updated_federal_agency"]) for domain_request in domain_requests: try: - if not domain_request.federal_agency: + if domain_request.federal_agency is None: domain_requests_skipped.append(domain_request) else: federal_agency_row = self.find_federal_agency_row(domain_request) domain_request.updated_federal_agency = federal_agency_row domain_requests_to_update.append(domain_request) - logger.info(f"DomainRequest {domain_request} updated_federal_agency set to: {domain_request.updated_federal_agency}") + logger.info( + f"DomainRequest {domain_request} updated_federal_agency set to: {domain_request.updated_federal_agency}" + ) except Exception as err: logger.info(f"DomainRequest for {domain_request} failed to update updated_federal_agency: {err}") domain_requests_with_errors.append(domain_request) - ScriptDataHelper.bulk_update_fields( - DomainRequest, domain_requests_to_update, ["updated_federal_agency"] - ) - + ScriptDataHelper.bulk_update_fields(DomainRequest, domain_requests_to_update, ["updated_federal_agency"]) + logger.info(f"{len(domain_infos_to_update)} DomainInformation rows updated update_federal_agency.") - logger.info(f"{len(domain_infos_with_errors)} DomainInformation rows errored when updating update_federal_agency.") + logger.info( + f"{len(domain_infos_with_errors)} DomainInformation rows errored when updating update_federal_agency." + ) logger.info(f"{len(domain_requests_to_update)} DomainRequest rows updated update_federal_agency.") logger.info(f"{len(domain_requests_skipped)} DomainRequest rows with null federal_agency skipped.") - logger.info(f"{len(domain_requests_with_errors)} DomainRequest rows errored when updating update_federal_agency.\n{domain_requests_with_errors}") - - def find_federal_agency_row(self, domain_object): - federal_agency = domain_object.federal_agency - # Domain Information objects without a federal agency default to Non-Federal Agency - if not federal_agency: - federal_agency = "Non-Federal Agency" - if federal_agency in self.rename_deprecated_federal_agency.keys(): - federal_agency = self.rename_deprecated_federal_agency[federal_agency] - return FederalAgency.objects.filter(agency=federal_agency).get() \ No newline at end of file + logger.info( + f"{len(domain_requests_with_errors)} DomainRequest rows errored when updating update_federal_agency.\n{domain_requests_with_errors}" + ) From 053bfc8822ffe492288b61494bf7be0f61f268e8 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 30 Apr 2024 10:35:19 -0700 Subject: [PATCH 18/34] Remove us china federal agency --- src/registrar/admin.py | 4 ++-- .../commands/populate_domain_updated_federal_agency.py | 1 + src/registrar/models/federal_agency.py | 1 - 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 7e50701a6..49d74f518 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1007,7 +1007,7 @@ class DomainInformationAdmin(ListHeaderAdmin): "description": "Extends type of organization", "fields": [ "federal_type", - # "updated_federal_agency", + "updated_federal_agency", # Above field commented out so it won't display "federal_agency", # TODO: remove later "tribe_name", @@ -1260,7 +1260,7 @@ class DomainRequestAdmin(ListHeaderAdmin): "description": "Extends type of organization", "fields": [ "federal_type", - # "updated_federal_agency", + "updated_federal_agency", # Above field commented out so it won't display "federal_agency", # TODO: remove later "tribe_name", diff --git a/src/registrar/management/commands/populate_domain_updated_federal_agency.py b/src/registrar/management/commands/populate_domain_updated_federal_agency.py index 29493fdb4..50cc93601 100644 --- a/src/registrar/management/commands/populate_domain_updated_federal_agency.py +++ b/src/registrar/management/commands/populate_domain_updated_federal_agency.py @@ -34,6 +34,7 @@ class Command(BaseCommand): "Chemical Safety Board": "U.S. Chemical Safety Board", "Nuclear Waste Technical Review Board": "U.S. Nuclear Waste Technical Review Board", "State, Local, and Tribal Government": "Non-Federal Agency", + "U.S. China Economic and Security Review Commission": "U.S.-China Economic and Security Review Commission", } def find_federal_agency_row(self, domain_object): diff --git a/src/registrar/models/federal_agency.py b/src/registrar/models/federal_agency.py index 89b15ab56..2f48ae0ad 100644 --- a/src/registrar/models/federal_agency.py +++ b/src/registrar/models/federal_agency.py @@ -176,7 +176,6 @@ class FederalAgency(TimeStampedModel): "U.S. Agency for International Development", "U.S. Capitol Police", "U.S. Chemical Safety Board", - "U.S. China Economic and Security Review Commission", "U.S. Commission for the Preservation of Americas Heritage Abroad", "U.S. Commission of Fine Arts", "U.S. Commission on Civil Rights", From e16f5b6598db95bb6e1dca910dee23c4653b4374 Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Tue, 30 Apr 2024 11:49:04 -0700 Subject: [PATCH 19/34] Enable updates to FederalAgency --- .../0089_create_federal_agencies_v02.py | 26 +++++++++++++++++++ src/registrar/models/federal_agency.py | 4 +-- 2 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 src/registrar/migrations/0089_create_federal_agencies_v02.py diff --git a/src/registrar/migrations/0089_create_federal_agencies_v02.py b/src/registrar/migrations/0089_create_federal_agencies_v02.py new file mode 100644 index 000000000..110382d5d --- /dev/null +++ b/src/registrar/migrations/0089_create_federal_agencies_v02.py @@ -0,0 +1,26 @@ +# Generated by Django 4.2.10 on 2024-03-22 22:18 +# Removes U.S. China Economic and Security Review Commission from Federal Agency options + +from django.db import migrations, models +from registrar.models import FederalAgency +from typing import Any + + +# For linting: RunPython expects a function reference. +def create_federal_agencies(apps, schema_editor) -> Any: + FederalAgency.create_federal_agencies(apps, schema_editor) + + +class Migration(migrations.Migration): + + dependencies = [ + ("registrar", "0088_domaininformation_cisa_representative_email_and_more"), + ] + + operations = [ + migrations.RunPython( + create_federal_agencies, + reverse_code=migrations.RunPython.noop, + atomic=True, + ), + ] diff --git a/src/registrar/models/federal_agency.py b/src/registrar/models/federal_agency.py index 2f48ae0ad..868cc3b0b 100644 --- a/src/registrar/models/federal_agency.py +++ b/src/registrar/models/federal_agency.py @@ -216,7 +216,7 @@ class FederalAgency(TimeStampedModel): logger.info("Creating federal agency table.") try: - agencies = [FederalAgency(agency=agency) for agency in AGENCIES] - FederalAgency.objects.bulk_create(agencies) + for agency in AGENCIES: + FederalAgency.objects.update_or_create(agency=agency) except Exception as e: logger.error(f"Error creating federal agencies: {e}") From 41b13e56453c150ce59b742ffac70a5a2f6267b8 Mon Sep 17 00:00:00 2001 From: Erin <121973038+erinysong@users.noreply.github.com> Date: Tue, 30 Apr 2024 12:12:43 -0700 Subject: [PATCH 20/34] Revert U.S. China duplicate for later ticket --- .../commands/populate_domain_updated_federal_agency.py | 2 +- src/registrar/models/federal_agency.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/registrar/management/commands/populate_domain_updated_federal_agency.py b/src/registrar/management/commands/populate_domain_updated_federal_agency.py index 50cc93601..871bc88ac 100644 --- a/src/registrar/management/commands/populate_domain_updated_federal_agency.py +++ b/src/registrar/management/commands/populate_domain_updated_federal_agency.py @@ -34,7 +34,7 @@ class Command(BaseCommand): "Chemical Safety Board": "U.S. Chemical Safety Board", "Nuclear Waste Technical Review Board": "U.S. Nuclear Waste Technical Review Board", "State, Local, and Tribal Government": "Non-Federal Agency", - "U.S. China Economic and Security Review Commission": "U.S.-China Economic and Security Review Commission", + # "U.S. China Economic and Security Review Commission": "U.S.-China Economic and Security Review Commission", } def find_federal_agency_row(self, domain_object): diff --git a/src/registrar/models/federal_agency.py b/src/registrar/models/federal_agency.py index 868cc3b0b..6337e6a87 100644 --- a/src/registrar/models/federal_agency.py +++ b/src/registrar/models/federal_agency.py @@ -176,6 +176,7 @@ class FederalAgency(TimeStampedModel): "U.S. Agency for International Development", "U.S. Capitol Police", "U.S. Chemical Safety Board", + "U.S. China Economic and Security Review Commission", "U.S. Commission for the Preservation of Americas Heritage Abroad", "U.S. Commission of Fine Arts", "U.S. Commission on Civil Rights", @@ -216,7 +217,7 @@ class FederalAgency(TimeStampedModel): logger.info("Creating federal agency table.") try: - for agency in AGENCIES: - FederalAgency.objects.update_or_create(agency=agency) + agencies = [FederalAgency(agency=agency) for agency in AGENCIES] + FederalAgency.objects.bulk_create(agencies, ["agency"]) except Exception as e: logger.error(f"Error creating federal agencies: {e}") From 6ea8df259231d06af05e48f488ed97faa53c1a72 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 30 Apr 2024 15:57:36 -0700 Subject: [PATCH 21/34] Add more print statements into the script --- .../commands/populate_domain_updated_federal_agency.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/registrar/management/commands/populate_domain_updated_federal_agency.py b/src/registrar/management/commands/populate_domain_updated_federal_agency.py index 871bc88ac..0c349b26b 100644 --- a/src/registrar/management/commands/populate_domain_updated_federal_agency.py +++ b/src/registrar/management/commands/populate_domain_updated_federal_agency.py @@ -81,7 +81,7 @@ class Command(BaseCommand): except Exception as err: logger.info(f"DomainInformation for {domain_info} failed to update updated_federal_agency: {err}") domain_infos_with_errors.append(domain_info) - + logger.info(f"DOMAIN INFO - {domain_info} - Federal Agency is {domain_info.federal_agency}") ScriptDataHelper.bulk_update_fields(DomainInformation, domain_infos_to_update, ["updated_federal_agency"]) for domain_request in domain_requests: @@ -99,12 +99,14 @@ class Command(BaseCommand): except Exception as err: logger.info(f"DomainRequest for {domain_request} failed to update updated_federal_agency: {err}") domain_requests_with_errors.append(domain_request) + logger.info(f"DOMAIN REQUEST - {domain_request} - Federal Agency is {domain_request.federal_agency}") ScriptDataHelper.bulk_update_fields(DomainRequest, domain_requests_to_update, ["updated_federal_agency"]) logger.info(f"{len(domain_infos_to_update)} DomainInformation rows updated update_federal_agency.") logger.info( f"{len(domain_infos_with_errors)} DomainInformation rows errored when updating update_federal_agency." + f"{domain_infos_with_errors}" ) logger.info(f"{len(domain_requests_to_update)} DomainRequest rows updated update_federal_agency.") logger.info(f"{len(domain_requests_skipped)} DomainRequest rows with null federal_agency skipped.") From f28a14771743a1b661475847d644266fb34c83e2 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Tue, 30 Apr 2024 16:00:25 -0700 Subject: [PATCH 22/34] Remove conflicting migration --- .../0089_create_federal_agencies_v02.py | 26 ------------------- 1 file changed, 26 deletions(-) delete mode 100644 src/registrar/migrations/0089_create_federal_agencies_v02.py diff --git a/src/registrar/migrations/0089_create_federal_agencies_v02.py b/src/registrar/migrations/0089_create_federal_agencies_v02.py deleted file mode 100644 index 110382d5d..000000000 --- a/src/registrar/migrations/0089_create_federal_agencies_v02.py +++ /dev/null @@ -1,26 +0,0 @@ -# Generated by Django 4.2.10 on 2024-03-22 22:18 -# Removes U.S. China Economic and Security Review Commission from Federal Agency options - -from django.db import migrations, models -from registrar.models import FederalAgency -from typing import Any - - -# For linting: RunPython expects a function reference. -def create_federal_agencies(apps, schema_editor) -> Any: - FederalAgency.create_federal_agencies(apps, schema_editor) - - -class Migration(migrations.Migration): - - dependencies = [ - ("registrar", "0088_domaininformation_cisa_representative_email_and_more"), - ] - - operations = [ - migrations.RunPython( - create_federal_agencies, - reverse_code=migrations.RunPython.noop, - atomic=True, - ), - ] From 658ab94622fd03771f69d73890ca61fa6d0eb46b Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 30 Apr 2024 21:11:21 -0400 Subject: [PATCH 23/34] Set auditlog migration vars to True, will set to False in a separate PR --- src/registrar/config/settings.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index ff9302a40..9817476bb 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -785,6 +785,6 @@ if DEBUG: # Run: # cf run-task getgov-<> --wait --command 'python manage.py auditlogmigratejson --traceback' --name auditlogmigratejson # on our staging and stable, then remove these 2 variables or set to False -AUDITLOG_TWO_STEP_MIGRATION = False +AUDITLOG_TWO_STEP_MIGRATION = True -AUDITLOG_USE_TEXT_CHANGES_IF_JSON_IS_NOT_PRESENT = False +AUDITLOG_USE_TEXT_CHANGES_IF_JSON_IS_NOT_PRESENT = True From cf3f87b1fd35ecd28d5abb2f5426b6cf4831116c Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 30 Apr 2024 21:42:23 -0400 Subject: [PATCH 24/34] Cleanup --- .../templates/django/admin/includes/detail_table_fieldset.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index 88e4a9122..3a861837b 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -96,7 +96,7 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html)
FromToChanged ByChange DateStatusUserChanged at
{{ value.0|default:"None" }} {{ value.1|default:"None" }} {{ log_entry.actor|default:"None" }} {{ log_entry.timestamp|default:"None" }}') - self.assertContains(response, "NoneStartedSubmittedNoneStartedSubmittedStartedSubmittedStartedSubmittedIn reviewStartedSubmittedIn reviewAction neededStartedSubmittedIn reviewAction neededIn reviewStartedSubmittedIn reviewAction needed
- {% if field.field.name == "creator" %} + {% elif field.field.name == "creator" %}
{% include "django/admin/includes/contact_detail_list.html" with user=original_object.creator no_title_top_padding=field.is_readonly user_verification_type=original_object.creator.get_verification_type_display%} From 947e476a718c7c004e8d5b58612a23564c042586 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 30 Apr 2024 21:48:14 -0400 Subject: [PATCH 25/34] lint --- src/registrar/tests/test_admin.py | 32 +++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index 71ac289c9..fd3a5fe9c 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -977,26 +977,26 @@ class TestDomainRequestAdmin(MockEppLib): self.assertContains(response, "Submitted", count=1) self.assertContains(response, "In review", count=2) self.assertContains(response, "Action needed", count=1) - + def test_collaspe_toggle_button_markup(self): - """ - Tests for the correct collapse toggle button markup - """ + """ + Tests for the correct collapse toggle button markup + """ - # Create a fake domain request and domain - domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW) + # Create a fake domain request and domain + domain_request = completed_domain_request(status=DomainRequest.DomainRequestStatus.IN_REVIEW) - p = "adminpass" - self.client.login(username="superuser", password=p) - response = self.client.get( - "/admin/registrar/domainrequest/{}/change/".format(domain_request.pk), - follow=True, - ) + p = "adminpass" + self.client.login(username="superuser", password=p) + response = self.client.get( + "/admin/registrar/domainrequest/{}/change/".format(domain_request.pk), + follow=True, + ) - # Make sure the page loaded, and that we're on the right page - self.assertEqual(response.status_code, 200) - self.assertContains(response, domain_request.requested_domain.name) - self.test_helper.assertContains(response, "Show details") + # Make sure the page loaded, and that we're on the right page + self.assertEqual(response.status_code, 200) + self.assertContains(response, domain_request.requested_domain.name) + self.test_helper.assertContains(response, "Show details") @less_console_noise_decorator def test_analyst_can_see_and_edit_alternative_domain(self): From d9faf7478aa84caa4c0e73d817c5c63193517f7a Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 30 Apr 2024 22:47:50 -0400 Subject: [PATCH 26/34] Change ordering to descending --- .../django/admin/includes/detail_table_fieldset.html | 3 +-- src/registrar/templatetags/custom_filters.py | 5 ----- src/registrar/tests/test_admin.py | 4 ++-- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index 3a861837b..84a56cbce 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -1,6 +1,5 @@ {% extends "admin/fieldset.html" %} {% load static url_helpers %} -{% load custom_filters %} {% comment %} This is using a custom implementation fieldset.html (see admin/fieldset.html) @@ -81,7 +80,7 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) - {% for log_entry in original_object.history.all|reverse_list %} + {% for log_entry in original_object.history.all %} {% for key, value in log_entry.changes_display_dict.items %} {% if key == "status" %} diff --git a/src/registrar/templatetags/custom_filters.py b/src/registrar/templatetags/custom_filters.py index a9e546891..9fa5c9aa9 100644 --- a/src/registrar/templatetags/custom_filters.py +++ b/src/registrar/templatetags/custom_filters.py @@ -67,8 +67,3 @@ def get_organization_long_name(generic_org_type): @register.filter(name="has_permission") def has_permission(user, permission): return user.has_perm(permission) - - -@register.filter(name="reverse_list") -def reverse_list(value): - return reversed(value) diff --git a/src/registrar/tests/test_admin.py b/src/registrar/tests/test_admin.py index fd3a5fe9c..cf00994be 100644 --- a/src/registrar/tests/test_admin.py +++ b/src/registrar/tests/test_admin.py @@ -961,11 +961,11 @@ class TestDomainRequestAdmin(MockEppLib): # Define the expected sequence of status changes expected_status_changes = [ - "Started", - "Submitted", "In review", "Action needed", "In review", + "Submitted", + "Started", ] # Test for the order of status changes From f702570d105315ddf257b9e220cc2364b80be2a3 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Tue, 30 Apr 2024 22:57:48 -0400 Subject: [PATCH 27/34] Tweak margin --- .../templates/django/admin/includes/detail_table_fieldset.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html index 84a56cbce..c7bb6325e 100644 --- a/src/registrar/templates/django/admin/includes/detail_table_fieldset.html +++ b/src/registrar/templates/django/admin/includes/detail_table_fieldset.html @@ -68,7 +68,7 @@ This is using a custom implementation fieldset.html (see admin/fieldset.html) {% block after_help_text %} {% if field.field.name == "status" and original_object.history.count > 0 %} -
+
From 95c03cb09f4eec785cc4266382326cc32022599b Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 1 May 2024 09:17:22 -0700 Subject: [PATCH 28/34] Remove code change --- src/registrar/models/federal_agency.py | 2 +- src/registrar/tests/test_management_scripts.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/registrar/models/federal_agency.py b/src/registrar/models/federal_agency.py index 6337e6a87..89b15ab56 100644 --- a/src/registrar/models/federal_agency.py +++ b/src/registrar/models/federal_agency.py @@ -218,6 +218,6 @@ class FederalAgency(TimeStampedModel): try: agencies = [FederalAgency(agency=agency) for agency in AGENCIES] - FederalAgency.objects.bulk_create(agencies, ["agency"]) + FederalAgency.objects.bulk_create(agencies) except Exception as e: logger.error(f"Error creating federal agencies: {e}") diff --git a/src/registrar/tests/test_management_scripts.py b/src/registrar/tests/test_management_scripts.py index 062136d1a..86ea1847f 100644 --- a/src/registrar/tests/test_management_scripts.py +++ b/src/registrar/tests/test_management_scripts.py @@ -912,6 +912,7 @@ class TestPopulateDomainUpdatedFederalAgency(TestCase): self.domain_info_1.refresh_from_db() previous_federal_agency_name = self.domain_info_1.federal_agency + updated_federal_agency_name = self.domain_info_1.updated_federal_agency.agency self.assertEqual(previous_federal_agency_name, "U.S. Peace Corps") From b71c34617e179890f1c4e92e64fc68028bded1a3 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 1 May 2024 14:33:05 -0400 Subject: [PATCH 29/34] cleanup --- src/registrar/models/domain_request.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index b78b77dd8..75fbadc3e 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -27,7 +27,7 @@ class DomainRequest(TimeStampedModel): # https://django-auditlog.readthedocs.io/en/latest/usage.html#object-history # If we note any performace degradation due to this addition, # we can query the auditlogs table in admin.py and add the results to - # extra_context in the change_view method for DomainRequestAdmin + # extra_context in the change_view method for DomainRequestAdmin. # This is the more straightforward way so trying it first. history = AuditlogHistoryField() From 481ea7bcb09fb1124a8ce3eeb7652bea18bde39b Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 1 May 2024 14:54:57 -0700 Subject: [PATCH 30/34] Add in checks for empty strings and update logs --- .../populate_domain_updated_federal_agency.py | 33 +++++++++++-------- 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/src/registrar/management/commands/populate_domain_updated_federal_agency.py b/src/registrar/management/commands/populate_domain_updated_federal_agency.py index 0c349b26b..dd8ceb3b2 100644 --- a/src/registrar/management/commands/populate_domain_updated_federal_agency.py +++ b/src/registrar/management/commands/populate_domain_updated_federal_agency.py @@ -40,7 +40,7 @@ class Command(BaseCommand): def find_federal_agency_row(self, domain_object): federal_agency = domain_object.federal_agency # Domain Information objects without a federal agency default to Non-Federal Agency - if federal_agency is None: + if (federal_agency is None) or (federal_agency == ""): federal_agency = "Non-Federal Agency" if federal_agency in self.rename_deprecated_federal_agency.keys(): federal_agency = self.rename_deprecated_federal_agency[federal_agency] @@ -75,42 +75,47 @@ class Command(BaseCommand): domain_info.updated_federal_agency = federal_agency_row domain_infos_to_update.append(domain_info) logger.info( - f"DomainInformation {domain_info} => updated_federal_agency set to:" - f"{domain_info.updated_federal_agency}" + f"DomainInformation {domain_info} => updated_federal_agency set to: \ + {domain_info.updated_federal_agency}" ) except Exception as err: - logger.info(f"DomainInformation for {domain_info} failed to update updated_federal_agency: {err}") domain_infos_with_errors.append(domain_info) - logger.info(f"DOMAIN INFO - {domain_info} - Federal Agency is {domain_info.federal_agency}") + logger.info( + f"DomainInformation {domain_info} failed to update updated_federal_agency \ + from federal_agency {domain_info.federal_agency}. Error: {err}" + ) + ScriptDataHelper.bulk_update_fields(DomainInformation, domain_infos_to_update, ["updated_federal_agency"]) for domain_request in domain_requests: try: - if domain_request.federal_agency is None: + if (domain_request.federal_agency is None) or (domain_request.federal_agency == ""): domain_requests_skipped.append(domain_request) else: federal_agency_row = self.find_federal_agency_row(domain_request) domain_request.updated_federal_agency = federal_agency_row domain_requests_to_update.append(domain_request) logger.info( - f"DomainRequest {domain_request} => updated_federal_agency set to:" - f"{domain_request.updated_federal_agency}" + f"DomainRequest {domain_request} => updated_federal_agency set to: \ + {domain_request.updated_federal_agency}" ) except Exception as err: - logger.info(f"DomainRequest for {domain_request} failed to update updated_federal_agency: {err}") domain_requests_with_errors.append(domain_request) - logger.info(f"DOMAIN REQUEST - {domain_request} - Federal Agency is {domain_request.federal_agency}") + logger.info( + f"DomainRequest {domain_request} failed to update updated_federal_agency \ + from federal_agency {domain_request.federal_agency}. Error: {err}" + ) ScriptDataHelper.bulk_update_fields(DomainRequest, domain_requests_to_update, ["updated_federal_agency"]) logger.info(f"{len(domain_infos_to_update)} DomainInformation rows updated update_federal_agency.") logger.info( - f"{len(domain_infos_with_errors)} DomainInformation rows errored when updating update_federal_agency." - f"{domain_infos_with_errors}" + f"{len(domain_infos_with_errors)} DomainInformation rows errored when updating update_federal_agency. \ + {domain_infos_with_errors}" ) logger.info(f"{len(domain_requests_to_update)} DomainRequest rows updated update_federal_agency.") logger.info(f"{len(domain_requests_skipped)} DomainRequest rows with null federal_agency skipped.") logger.info( - f"{len(domain_requests_with_errors)} DomainRequest rows errored when updating update_federal_agency." - f"{domain_requests_with_errors}" + f"{len(domain_requests_with_errors)} DomainRequest rows errored when updating update_federal_agency. \ + {domain_requests_with_errors}" ) From 511f432f601f64fa13ba25bc0190ec80bd45b551 Mon Sep 17 00:00:00 2001 From: Rebecca Hsieh Date: Wed, 1 May 2024 14:56:30 -0700 Subject: [PATCH 31/34] Hide display in admin page --- src/registrar/admin.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 5fce724c4..485751b3c 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -1017,7 +1017,7 @@ class DomainInformationAdmin(ListHeaderAdmin): "description": "Extends type of organization", "fields": [ "federal_type", - "updated_federal_agency", + # "updated_federal_agency", # Above field commented out so it won't display "federal_agency", # TODO: remove later "tribe_name", @@ -1270,7 +1270,7 @@ class DomainRequestAdmin(ListHeaderAdmin): "description": "Extends type of organization", "fields": [ "federal_type", - "updated_federal_agency", + # "updated_federal_agency", # Above field commented out so it won't display "federal_agency", # TODO: remove later "tribe_name", From 80efe8352716791bbb77426db103e5d84cd4d89b Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Wed, 1 May 2024 18:55:35 -0400 Subject: [PATCH 32/34] trigger PR --- src/registrar/models/domain_request.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index 75fbadc3e..a3cfe4792 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -28,7 +28,7 @@ class DomainRequest(TimeStampedModel): # If we note any performace degradation due to this addition, # we can query the auditlogs table in admin.py and add the results to # extra_context in the change_view method for DomainRequestAdmin. - # This is the more straightforward way so trying it first. + # This is the more straightforward way, so trying it first. history = AuditlogHistoryField() # Constants for choice fields From ed94173392056d104c771031e973f4ac6a2f20b0 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 2 May 2024 15:03:14 -0400 Subject: [PATCH 33/34] trigger PR --- src/registrar/models/domain_request.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/registrar/models/domain_request.py b/src/registrar/models/domain_request.py index a3cfe4792..75fbadc3e 100644 --- a/src/registrar/models/domain_request.py +++ b/src/registrar/models/domain_request.py @@ -28,7 +28,7 @@ class DomainRequest(TimeStampedModel): # If we note any performace degradation due to this addition, # we can query the auditlogs table in admin.py and add the results to # extra_context in the change_view method for DomainRequestAdmin. - # This is the more straightforward way, so trying it first. + # This is the more straightforward way so trying it first. history = AuditlogHistoryField() # Constants for choice fields From 3244f731c5f22fbc812021d4d3faa52925d63025 Mon Sep 17 00:00:00 2001 From: Rachid Mrad Date: Thu, 2 May 2024 17:04:55 -0400 Subject: [PATCH 34/34] Add missing JS --- src/registrar/assets/js/dja-collapse.js | 45 +++++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 src/registrar/assets/js/dja-collapse.js diff --git a/src/registrar/assets/js/dja-collapse.js b/src/registrar/assets/js/dja-collapse.js new file mode 100644 index 000000000..c33954192 --- /dev/null +++ b/src/registrar/assets/js/dja-collapse.js @@ -0,0 +1,45 @@ +/* + * We will run our own version of + * https://github.com/django/django/blob/195d885ca01b14e3ce9a1881c3b8f7074f953736/django/contrib/admin/static/admin/js/collapse.js + * Works with our fieldset override +*/ + +/*global gettext*/ +'use strict'; +{ + window.addEventListener('load', function() { + // Add anchor tag for Show/Hide link + const fieldsets = document.querySelectorAll('fieldset.collapse--dotgov'); + for (const [i, elem] of fieldsets.entries()) { + // Don't hide if fields in this fieldset have errors + if (elem.querySelectorAll('div.errors, ul.errorlist').length === 0) { + elem.classList.add('collapsed'); + const button = elem.querySelector('button'); + button.id = 'fieldsetcollapser' + i; + button.className = 'collapse-toggle--dotgov usa-button usa-button--unstyled'; + } + } + // Add toggle to hide/show anchor tag + const toggleFuncDotgov = function(e) { + e.preventDefault(); + e.stopPropagation(); + const fieldset = this.closest('fieldset'); + const spanElement = this.querySelector('span'); + const useElement = this.querySelector('use'); + if (fieldset.classList.contains('collapsed')) { + // Show + spanElement.textContent = 'Hide details'; + useElement.setAttribute('xlink:href', '/public/img/sprite.svg#expand_less'); + fieldset.classList.remove('collapsed'); + } else { + // Hide + spanElement.textContent = 'Show details'; + useElement.setAttribute('xlink:href', '/public/img/sprite.svg#expand_more'); + fieldset.classList.add('collapsed'); + } + }; + document.querySelectorAll('.collapse-toggle--dotgov').forEach(function(el) { + el.addEventListener('click', toggleFuncDotgov); + }); + }); +}