feat: Update T1OO logic

We expand the logic to exclude living, de-registered patients on the
basis that they may have registered a T1OO elsewhere which has not
propagated to our database.

cohortextractor/tpp_backend.py

@@ -416,13 +416,50 @@ def get_queries(self, covariate_definitions):
 
         if not self.include_t1oo:
             # If this query has not been explictly flagged as including T1OO patients
-            # then we add an extra LEFT OUTER JOIN on the T1OO table and WHERE clause
-            # which will exclude any patient IDs found in the T1OO table
-            joins.append(
-                f"LEFT OUTER JOIN {T1OO_TABLE} ON {T1OO_TABLE}.Patient_ID = {patient_id_expr}",
+            # then we need to add some extra conditions ...
+
+            # First, we create a temporary table containing just those patients with a
+            # currently active registration
+            reg_table = "#_current_registration"
+            empty_datetime = "9999-12-31T00:00:00"
+            table_queries[reg_table] = [
+                f"""
+                SELECT
+                  DISTINCT Patient_ID AS patient_id
+                INTO {reg_table}
+                FROM RegistrationHistory
+                WHERE EndDate = '{empty_datetime}'
+                """,
+                f"CREATE CLUSTERED INDEX patient_id_ix ON {reg_table} (patient_id)",
+            ]
+
+            # We join this current registrations table to the query along with the list
+            # of known opt outs
+            joins.extend(
+                [
+                    f"LEFT OUTER JOIN {reg_table} ON {reg_table}.patient_ID = {patient_id_expr}",
+                    f"LEFT OUTER JOIN {T1OO_TABLE} ON {T1OO_TABLE}.Patient_ID = {patient_id_expr}",
+                ]
             )
-            wheres.append(
-                f"{T1OO_TABLE}.Patient_ID IS NULL",
+            # There's some unpleasantness here in that we also need to join on the
+            # `Patient` table (to get `DateOfDeath`) but only if it's not already
+            # included. By construction, the `joins` list will not include the `Patient`
+            # table so the place we need to check is the `primary_table`.
+            if primary_table != "Patient":
+                joins.append(
+                    f"LEFT OUTER JOIN Patient ON Patient.Patient_ID = {patient_id_expr}",
+                )
+            # Finally we add the condition itself which is that patient has no recorded
+            # T1OO and is either currently registered or has died
+            wheres.extend(
+                [
+                    f"""
+                    {T1OO_TABLE}.Patient_ID IS NULL AND (
+                      {reg_table}.patient_id IS NOT NULL
+                      OR Patient.DateOfDeath != '{empty_datetime}'
+                    )
+                    """,
+                ]
             )
 
         joins_str = "\n          ".join(joins)

tests/test_tpp_backend.py

@@ -268,20 +268,43 @@ def test_minimal_study_with_t1oo_default():
 @pytest.mark.parametrize(
     "flag,expected",
     [
-        ("false", ["1", "4"]),
-        ("true", ["1", "2", "3", "4"]),
+        ("false", ["1", "4", "6", "8"]),
+        ("true", ["1", "2", "3", "4", "5", "6", "7", "8"]),
     ],
 )
 def test_minimal_study_with_t1oo_flag(set_database_url_with_t1oo, flag, expected):
     set_database_url_with_t1oo(flag)
     # Test that type 1 opt-outs are only included if flag is explicitly set to "True"
     fixtures = [
+        # Included: no opt-out and a current registration
         Patient(Patient_ID=1),
+        RegistrationHistory(Patient_ID=1, StartDate="2020-01-01", EndDate="9999-12-31"),
+        # Excluded: current registration but opted out
         Patient(Patient_ID=2),
-        Patient(Patient_ID=3),
-        Patient(Patient_ID=4),
+        RegistrationHistory(Patient_ID=1, StartDate="2020-01-01", EndDate="9999-12-31"),
         PatientsWithTypeOneDissent(Patient_ID=2),
-        PatientsWithTypeOneDissent(Patient_ID=3),
+        # Excluded: no opt-out, but has deregistered
+        Patient(Patient_ID=3),
+        RegistrationHistory(Patient_ID=3, StartDate="2020-01-01", EndDate="2024-07-01"),
+        # Included: no opt-out, has deregistered, but death recorded
+        Patient(Patient_ID=4, DateOfDeath="2024-06-01"),
+        RegistrationHistory(Patient_ID=4, StartDate="2020-01-01", EndDate="2024-07-01"),
+        # Excluded: death recorded, registered, but had opt out in place
+        Patient(Patient_ID=5, DateOfDeath="2024-07-01"),
+        RegistrationHistory(Patient_ID=5, StartDate="2020-01-01", EndDate="9999-12-31"),
+        PatientsWithTypeOneDissent(Patient_ID=5),
+        # Included: no opt-out and a deregistration followed by a subsequent
+        # re-registration
+        Patient(Patient_ID=6),
+        RegistrationHistory(Patient_ID=6, StartDate="2010-01-01", EndDate="2015-10-20"),
+        RegistrationHistory(Patient_ID=6, StartDate="2022-01-01", EndDate="9999-12-31"),
+        # Excluded: no registration history (only possible if an external dataset
+        # contains patients never registered with SystmOne – unexpected but not
+        # impossible)
+        Patient(Patient_ID=7),
+        # Included: no registration history but death recorded (not possible as far as I
+        # can see, but we should handle all the edge cases)
+        Patient(Patient_ID=8, DateOfDeath="2024-06-01"),
     ]
     session = make_session()
     session.add_all(fixtures)