🐛 Correct handling of repeated instances vis-à-vis multiselect

Author: shnizzedy

python_jobs/src/hbnmigration/from_redcap/config.py

@@ -142,6 +142,7 @@ class export_247(metaclass=_ForCuriousMeta):
                 "mrn",
                 "par_rel",
                 "par_rel_2",
+                "parent_involvement",
                 "parent_last_name",
                 "parent_last_name_1821",
                 "parent_last_name_2",
@@ -185,6 +186,9 @@ class export_247(metaclass=_ForCuriousMeta):
                 "zipcode_1821",
                 "zipcode_2",
                 "zipcode_2_1821",
+                "adult_phone",
+                "permission_audiovideo_1821",
+                "permission_collab_1821",
             ]
         )
         """Fields to export from REDCap PID 247 for import into REDCap PID 625."""
@@ -319,6 +323,8 @@ class import_curious:
             "adult_address",
             "adult_apt",
             "adult_city",
+            "adult_email",
+            "adult_phone",
             "adult_state",
             "adult_zip",
             "aptnumber1",
@@ -349,11 +355,11 @@ class import_curious:
             "last_name",
             "middlename_y",
             "mrn",
+            "parent_involvement",
             "parent_last_name_2",
             "parentfirstname",
             "parentfirstname_2",
             "parentlastname",
-            "parent_involvement",
             "permission_audiovideo",
             "permission_audiovideo_participant",
             "permission_collab",
@@ -379,6 +385,14 @@ class import_curious:
     )
     """Fields to import into REDCap PID 625."""
 
+    class duplicate:
+        """One-to-many mappings from one DataFrame to another."""
+
+        redcap_consent_to_redcap_operations: Final[dict[str, list[str]]] = {
+            # "adult_email": ["adult_email", "email"],
+        }
+        """Columns to fan out from REDCap PID 247 to REDCap PID 625."""
+
     class rename:
         """Mappings to rename from one DataFrame to another."""
 
@@ -491,7 +505,6 @@ class redcap_consent_to_redcap_responder_tracking(ColumnRenameMapping):
             "parent_second_guardian_"
             "consent_complete": "complete_parent_second_guardian_consent",
             "dob_1821": "dob",
-            "adult_email": "email",
             "email_1821": "email",
             "email_2_1821": "email_2",
             "consent1": "first_name",
@@ -515,6 +528,8 @@ class redcap_consent_to_redcap_responder_tracking(ColumnRenameMapping):
             "parent_last_name_1821": "parentlastname",
             "permission_audiovideo_1113": "permission_audiovideo_participant",
             "permission_audiovideo_1417": "permission_audiovideo_participant",
+            "permission_audiovideo_1821": "permission_audiovideo_participant",
+            "permission_collab_1821": "permission_collab",
             "phone_1821": "phone",
             "phone_2_1821": "phone_2",
             "prefname_1821": "prefname",

python_jobs/src/hbnmigration/from_redcap/to_redcap.py

@@ -210,17 +210,34 @@ def format_data_for_redcap_operations(redcap_data: pd.DataFrame) -> pd.DataFrame
         df["field_name"] == "record_id", "record"
     ]
 
-    # Step 8: Deduplicate by keeping most recent repeat instance
+    # Step 8: For repeated instruments,
+    # keep only the latest instance's full set of rows.
+    # Then deduplicate non-repeated rows by record + field_name.
+
+    has_instance = df["redcap_repeat_instance"].notna()
+    repeated_df = df[has_instance].copy()
+    non_repeated_df = df[~has_instance].copy()
+
+    if not repeated_df.empty:
+        # Keep only rows from the highest instance per record + instrument
+        max_instance = repeated_df.groupby(["record", "redcap_repeat_instrument"])[
+            "redcap_repeat_instance"
+        ].transform("max")
+        repeated_df = repeated_df[repeated_df["redcap_repeat_instance"] == max_instance]
+
+    # For non-repeated rows, deduplicate on record + field_name
+    non_repeated_df = non_repeated_df.drop_duplicates(
+        subset=["record", "field_name"], keep="first"
+    )
+
     df = (
-        df.sort_values("redcap_repeat_instance", ascending=False, na_position="last")
-        .drop_duplicates(subset=["record", "field_name"], keep="first")
+        pd.concat([non_repeated_df, repeated_df], ignore_index=True)
         .drop(
             columns=["redcap_repeat_instrument", "redcap_repeat_instance"],
             errors="ignore",
         )
         .reset_index(drop=True)
     )
-
     # Step 9: Decrement permission_collab values by 1
     decrement_mask = df["field_name"] == "permission_collab"
     if decrement_mask.any():
@@ -447,7 +464,6 @@ def process_record_for_redcap_operations(record_id: str) -> dict[str, Any]:
                 "record_id": record_id,
                 "message": f"No data found in Intake (PID {_SOURCE_PID})",
             }
-
         # Extract event name for intake_ready field BEFORE formatting
         event_name = None
         intake_ready_rows = source_data[source_data["field_name"] == "intake_ready"]
@@ -615,7 +631,6 @@ def main() -> None:
             _TARGET_PID,
         )
         return
-
     # Build mapping of MRN -> source record_id BEFORE formatting
     mrn_to_source_record = (
         data_operations[data_operations["field_name"] == "mrn"]

python_jobs/src/tests/test_redcap.py

@@ -1678,3 +1678,167 @@ def test_send_continues_after_single_failure(
             assert len(failures) == 1
             assert failures[0] == "2"
             assert mocks["new_account"].call_count == 3
+
+
+class TestFormatDataForRedcapOperations:
+    """Tests for format_data_for_redcap_operations."""
+
+    class TestDeduplication:
+        """Tests for Step 8: repeat instance deduplication logic."""
+
+        def test_keeps_all_values_from_latest_repeat_instance(self):
+            """Checkbox fields with multiple values in the latest instance are kept."""
+            df = pd.DataFrame(
+                {
+                    "record": ["1465"] * 6,
+                    "field_name": [
+                        "parent_involvement",
+                        "parent_involvement",
+                        "email",
+                        "parent_involvement",
+                        "parent_involvement",
+                        "email",
+                    ],
+                    "value": [
+                        "1",
+                        "2",
+                        "old@example.com",
+                        "1",
+                        "2",
+                        "new@example.com",
+                    ],
+                    "redcap_event_name": ["event_1"] * 6,
+                    "redcap_repeat_instrument": ["adult_consent"] * 6,
+                    "redcap_repeat_instance": [1, 1, 1, 2, 2, 2],
+                }
+            )
+
+            result = format_data_for_redcap_operations(df)
+
+            # Should keep both parent_involvement rows from instance 2
+            pi_rows = result[result["field_name"] == "parent_involvement"]
+            assert len(pi_rows) == 2
+            assert set(pi_rows["value"]) == {"1", "2"}
+
+            # Should keep only the email from instance 2
+            email_rows = result[result["field_name"] == "email"]
+            assert len(email_rows) == 1
+            assert email_rows.iloc[0]["value"] == "new@example.com"
+
+        def test_discards_older_repeat_instances(self):
+            """Only the highest repeat instance per record+instrument is kept."""
+            df = pd.DataFrame(
+                {
+                    "record": ["100"] * 5,
+                    "field_name": [
+                        "parent_involvement",
+                        "parent_involvement",
+                        "parent_involvement",
+                        "parent_involvement",
+                        "parent_involvement",
+                    ],
+                    "value": ["0", "1", "2", "1", "3"],
+                    "redcap_event_name": ["event_1"] * 5,
+                    "redcap_repeat_instrument": ["adult_consent"] * 5,
+                    "redcap_repeat_instance": [1, 1, 1, 2, 2],
+                }
+            )
+
+            result = format_data_for_redcap_operations(df)
+
+            pi_rows = result[result["field_name"] == "parent_involvement"]
+            assert len(pi_rows) == 2
+            assert set(pi_rows["value"]) == {"1", "3"}
+
+        def test_non_repeated_rows_deduplicate_by_record_and_field(self):
+            """Non-repeated rows (NaN instance) deduplicate on record + field_name."""
+            df = pd.DataFrame(
+                {
+                    "record": ["200", "200"],
+                    "field_name": ["email", "email"],
+                    "value": ["first@example.com", "second@example.com"],
+                    "redcap_event_name": ["event_1", "event_1"],
+                    "redcap_repeat_instrument": [None, None],
+                    "redcap_repeat_instance": [None, None],
+                }
+            )
+
+            result = format_data_for_redcap_operations(df)
+
+            email_rows = result[result["field_name"] == "email"]
+            assert len(email_rows) == 1
+
+        def test_mixed_repeated_and_non_repeated(self):
+            """Records with both repeated and non-repeated fields are handled."""
+            df = pd.DataFrame(
+                {
+                    "record": ["300"] * 5,
+                    "field_name": [
+                        "dob",
+                        "parent_involvement",
+                        "parent_involvement",
+                        "parent_involvement",
+                        "email",
+                    ],
+                    "value": ["2010-01-01", "0", "1", "2", "test@example.com"],
+                    "redcap_event_name": ["event_1"] * 5,
+                    "redcap_repeat_instrument": [
+                        None,
+                        "adult_consent",
+                        "adult_consent",
+                        "adult_consent",
+                        None,
+                    ],
+                    "redcap_repeat_instance": [None, 1, 1, 1, None],
+                }
+            )
+
+            result = format_data_for_redcap_operations(df)
+
+            # All 3 checkbox values kept (only one instance, so all kept)
+            pi_rows = result[result["field_name"] == "parent_involvement"]
+            assert len(pi_rows) == 3
+
+            # Non-repeated fields kept as single rows
+            assert len(result[result["field_name"] == "dob"]) == 1
+            assert len(result[result["field_name"] == "email"]) == 1
+
+    class TestFieldRenaming:
+        """Tests for field name rename and fan-out logic."""
+
+        def test_permission_audiovideo_1821_renamed(self):
+            """Test that `permission_audiovideo_1821` is renamed."""
+            df = pd.DataFrame(
+                {
+                    "record": ["400"],
+                    "field_name": ["permission_audiovideo_1821"],
+                    "value": ["1"],
+                    "redcap_event_name": ["event_1"],
+                    "redcap_repeat_instrument": [None],
+                    "redcap_repeat_instance": [None],
+                }
+            )
+
+            result = format_data_for_redcap_operations(df)
+
+            assert "permission_audiovideo_1821" not in result["field_name"].values
+            assert "permission_audiovideo_participant" in result["field_name"].values
+
+        def test_permission_collab_1821_renamed_and_decremented(self):
+            """Test that `permission_collab_1821` is renamed and decremented."""
+            df = pd.DataFrame(
+                {
+                    "record": ["500"],
+                    "field_name": ["permission_collab_1821"],
+                    "value": ["1"],
+                    "redcap_event_name": ["event_1"],
+                    "redcap_repeat_instrument": [None],
+                    "redcap_repeat_instance": [None],
+                }
+            )
+
+            result = format_data_for_redcap_operations(df)
+
+            collab_rows = result[result["field_name"] == "permission_collab"]
+            assert len(collab_rows) == 1
+            assert collab_rows.iloc[0]["value"] == "0"  # decremented from 1