[Issue #2493] Allow certain agency fields to be null from the legacy …

…values (#2513)

## Summary
Fixes #2493

### Time to review: __5 mins__

## Changes proposed
Modified three fields (ldapGp, description, label) in the agency table
to be nullable

Adjusted the transform agency process to allow these fields to be
nullable as well

## Context for reviewers
This isn't an issue in prod right now, but is accounting for a problem I
found in the dev/staging environments. The agency data is stored in a
very different way (key-value pairs) in the legacy data, and these
fields are sometimes not present in the test data. There isn't any
reason we should require them as we likely won't use them, so I'm
letting them be nullable to let these agencies with the values null get
populated in the lower envs.

## Additional information
There is some additional context on
#2494

---------

Co-authored-by: nava-platform-bot <[email protected]>

api/src/data_migration/transformation/subtask/transform_agency.py

@@ -26,6 +26,9 @@
 NULLABLE_FIELDS = {
     "AgencyCode",  # Note this is the sub_agency_code in our system
     "AgencyContactEMail2",
+    "ldapGp",
+    "description",
+    "label",
 }
 
 AGENCY_FIELD_MAP = {
@@ -77,9 +80,6 @@
     "AgencyCFDA",
     "AgencyDownload",
     "AgencyNotify",
-    "ldapGp",
-    "description",
-    "label",
     "AgencyContactName",
     "AgencyContactAddress1",
     "AgencyContactCity",

api/src/db/migrations/versions/2024_10_17_legacy_null_agency.py

@@ -0,0 +1,32 @@
+"""legacy null agency
+
+Revision ID: 558ad9563e9a
+Revises: 56448a3ecb8f
+Create Date: 2024-10-17 13:54:32.420425
+
+"""
+
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = "558ad9563e9a"
+down_revision = "56448a3ecb8f"
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.alter_column("agency", "ldap_group", existing_type=sa.TEXT(), nullable=True, schema="api")
+    op.alter_column("agency", "description", existing_type=sa.TEXT(), nullable=True, schema="api")
+    op.alter_column("agency", "label", existing_type=sa.TEXT(), nullable=True, schema="api")
+    # ### end Alembic commands ###
+
+
+def downgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    op.alter_column("agency", "label", existing_type=sa.TEXT(), nullable=False, schema="api")
+    op.alter_column("agency", "description", existing_type=sa.TEXT(), nullable=False, schema="api")
+    op.alter_column("agency", "ldap_group", existing_type=sa.TEXT(), nullable=False, schema="api")
+    # ### end Alembic commands ###

api/src/db/models/agency_models.py

@@ -72,9 +72,9 @@ class Agency(ApiSchemaTable, TimestampMixin):
 
     # These values come from the legacy system, but their exact usage isn't entirely
     # clear at this point in time.
-    ldap_group: Mapped[str]
-    description: Mapped[str]
-    label: Mapped[str]
+    ldap_group: Mapped[str | None]
+    description: Mapped[str | None]
+    label: Mapped[str | None]
 
     is_multilevel_agency: Mapped[bool] = mapped_column(default=False)
     is_multiproject: Mapped[bool] = mapped_column(default=False)

api/tests/src/data_migration/transformation/conftest.py

@@ -721,6 +721,7 @@ def validate_agency(
     expect_values_to_match: bool = True,
     is_test_agency: bool = False,
     non_matching_fields: set | None = None,
+    deleted_fields: set | None = None,
 ):
     agency_code = source_tgroups[0].get_agency_code()
     agency = db_session.query(Agency).filter(Agency.agency_code == agency_code).one_or_none()
@@ -739,6 +740,13 @@ def validate_agency(
     else:
         agency_field_mapping = AGENCY_FIELD_MAPPING
 
+    if deleted_fields is not None:
+        agency_field_mapping = [m for m in agency_field_mapping if m[0] not in deleted_fields]
+
+        deleted_field_mapping = [m for m in AGENCY_FIELD_MAPPING if m[0] in deleted_fields]
+        for deleted_field in deleted_field_mapping:
+            assert getattr(agency, deleted_field[1]) is None
+
     validate_matching_fields(tgroup_map, agency, agency_field_mapping, expect_values_to_match)
     assert agency.is_test_agency == is_test_agency
 

api/tests/src/data_migration/transformation/subtask/test_transform_agency.py

@@ -31,14 +31,21 @@ def test_process_agencies(self, db_session, transform_agency):
         insert_agency1 = setup_agency("INSERT-AGENCY-1", create_existing=False)
         insert_agency2 = setup_agency("INSERT-AGENCY-2", create_existing=False)
         insert_agency3 = setup_agency("INSERT-AGENCY-3", create_existing=False)
-        insert_agency4 = setup_agency("INSERT-AGENCY-4", create_existing=False)
+        insert_agency4 = setup_agency(
+            "INSERT-AGENCY-4",
+            create_existing=False,
+            # None passed in here will make it not appear at all in the tgroups rows
+            source_values={"ldapGp": None, "description": None, "label": None},
+        )
         insert_test_agency = setup_agency("GDIT", create_existing=False)
 
         # Already processed fields are ones that were handled on a prior run and won't be updated
         # during this specific run
         update_agency1 = setup_agency("UPDATE-AGENCY-1", create_existing=True)
         update_agency2 = setup_agency(
-            "UPDATE-AGENCY-2", create_existing=True, deleted_fields={"AgencyContactEMail2"}
+            "UPDATE-AGENCY-2",
+            create_existing=True,
+            deleted_fields={"AgencyContactEMail2", "ldapGp", "description"},
         )
         update_agency3 = setup_agency(
             "UPDATE-AGENCY-3",
@@ -82,7 +89,7 @@ def test_process_agencies(self, db_session, transform_agency):
         validate_agency(db_session, insert_test_agency, is_test_agency=True)
 
         validate_agency(db_session, update_agency1)
-        validate_agency(db_session, update_agency2)
+        validate_agency(db_session, update_agency2, deleted_fields={"ldapGp", "description"})
         validate_agency(
             db_session,
             update_agency3,

api/tests/src/db/models/factories.py

@@ -1743,9 +1743,9 @@ def create_tgroups_agency(
     **kwargs,
 ) -> list[staging.tgroups.Tgroups]:
     # The agency_code value is actually just the first bit (the top-level agency)
-    kwargs["AgencyCode"] = agency_code.split("-")[0]
-    kwargs["AgencyEnroll"] = agency_code
-    kwargs["ldapGp"] = agency_code
+    kwargs.setdefault("AgencyCode", agency_code.split("-")[-1])
+    kwargs.setdefault("AgencyEnroll", agency_code)
+    kwargs.setdefault("ldapGp", agency_code)
 
     field_values = StagingTgroupsAgencyFactory.build(**kwargs)