update tests & add check for cursor validity

Author: m-goggins

src/recordlinker/database/mpi_service.py

@@ -398,7 +398,7 @@ def get_orphaned_patients(
 def get_orphaned_persons(
     session: orm.Session,
     limit: int | None = 50,
-    cursor: uuid.UUID | None = None,
+    cursor: int | None = None,
 ) -> typing.Sequence[models.Person]:
     """
     Retrieve orphaned Persons in the MPI database, up to the provided limit. If a

src/recordlinker/routes/person_router.py

@@ -115,7 +115,26 @@ def get_orphaned_persons(
     """
     Retrieve person_reference_id(s) for all Persons that are not linked to any Patients.
     """
-    persons = service.get_orphaned_persons(session, limit, cursor)
+    # Check if the cursor is a valid Person reference_id
+    if cursor:
+        person = service.get_persons_by_reference_ids(session, cursor)
+        if not person or person[0] is None:
+            raise fastapi.HTTPException(
+                status_code=fastapi.status.HTTP_422_UNPROCESSABLE_ENTITY,
+                detail=[
+                    {
+                        "loc": ["query", "cursor"],
+                        "msg": "Cursor is an invalid Person reference_id",
+                        "type": "value_error",
+                    }
+                ],
+            )
+        # Replace the cursor with the Patient id instead of reference_id
+        cur = person[0].id
+    else:
+        cur = None
+
+    persons = service.get_orphaned_persons(session, limit, cur)
     if not persons:
         return schemas.PaginatedRefs(
             data=[], meta=schemas.PaginatedMetaData(next_cursor=None, next=None)

tests/unit/database/test_mpi_service.py

@@ -985,7 +985,7 @@ def test_get_orphaned_persons_cursor(self, session: Session):
         person2 = models.Person(id=2)
         person3 = models.Person(id=3)
         person4 = models.Person(id=4)
-        patient = models.Patient(person=person1, data={})
+        patient = models.Patient(person=person4, data={})
         session.add_all([patient, person2, person3, person4])
         session.flush()
 
@@ -996,3 +996,7 @@ def test_get_orphaned_persons_cursor(self, session: Session):
             person2,
             person3,
         ]
+        assert mpi_service.get_orphaned_persons(session, limit=5, cursor=person1.id) == [
+            person2,
+            person3,
+        ]

tests/unit/routes/test_patient_router.py

@@ -195,12 +195,9 @@ def test_no_orphaned_patients(self, client):
         }
 
     def test_get_orphaned_patients_with_cursor(self, client):
-        ordered_uuids = [uuid.uuid4() for _ in range(3)]
-        ordered_uuids.sort()
-
-        patient1 = models.Patient(person=None, reference_id=ordered_uuids[0])
-        patient2 = models.Patient(person=None, reference_id=ordered_uuids[1])
-        patient3 = models.Patient(person=None, reference_id=ordered_uuids[2])
+        patient1 = models.Patient(person=None, data={"id": 1})
+        patient2 = models.Patient(person=None, data={"id": 2})
+        patient3 = models.Patient(person=None, data={"id": 3})
         client.session.add_all([patient1, patient2, patient3])
         client.session.flush()
 
@@ -211,8 +208,8 @@ def test_get_orphaned_patients_with_cursor(self, client):
         assert response.json() == {
             "data": [str(patient2.reference_id)],
             "meta": {
-                "next_cursor": str(ordered_uuids[1]),
-                "next": f"http://testserver/patient/orphaned?limit=1&cursor={str(ordered_uuids[1])}",
+                "next_cursor": str(patient2.reference_id),
+                "next": f"http://testserver/patient/orphaned?limit=1&cursor={str(patient2.reference_id)}",
             },
         }
 
@@ -221,8 +218,8 @@ def test_get_orphaned_patients_with_cursor(self, client):
         assert response.json() == {
             "data": [str(patient2.reference_id), str(patient3.reference_id)],
             "meta": {
-                "next_cursor": str(ordered_uuids[2]),
-                "next": f"http://testserver/patient/orphaned?limit=2&cursor={ordered_uuids[2]}",
+                "next_cursor": str(patient3.reference_id),
+                "next": f"http://testserver/patient/orphaned?limit=2&cursor={str(patient3.reference_id)}",
             },
         }
 

tests/unit/routes/test_person_router.py

@@ -226,3 +226,115 @@ def testInvalidPersonId(self, client):
     def testInvalidPerson(self, client):
         response = client.delete(f"/person/{uuid.uuid4()}")
         assert response.status_code == 404
+
+
+class TestGetOrphanedPersons:
+    def testGetOrphanedPersons(self, client):
+        person1 = models.Person()
+        patient1 = models.Patient(person=person1, data={})
+
+        person2 = models.Person()
+        client.session.add_all([patient1, person2])
+        client.session.flush()
+
+        response = client.get("/person/orphaned")
+        assert response.status_code == 200
+        assert response.json() == {
+            "data": [str(person2.reference_id)],
+            "meta": {"next_cursor": None, "next": None},
+        }
+
+    def test_no_orphaned_persons(self, client):
+        response = client.get("/person/orphaned")
+        assert response.status_code == 200
+        assert response.json() == {
+            "data": [],
+            "meta": {"next_cursor": None, "next": None},
+        }
+
+    def test_get_orphaned_persons_with_limit(self, client):
+        person1 = models.Person(id=1)
+        person2 = models.Person(id=2)
+        person3 = models.Person(id=3)
+        person4 = models.Person(id=4)
+        client.session.add_all([person1, person2, person3, person4])
+        client.session.flush()
+
+        response = client.get("/person/orphaned?limit=2")
+        assert response.status_code == 200
+        assert response.json() == {
+            "data": [str(person1.reference_id), str(person2.reference_id)],
+            "meta": {
+                "next_cursor": str(person2.reference_id),
+                "next": f"http://testserver/person/orphaned?limit=2&cursor={str(person2.reference_id)}",
+            },
+        }
+
+        response = client.get(f"/person/orphaned?limit=2&cursor={person2.reference_id}")
+        assert response.json() == {
+            "data": [str(person3.reference_id), str(person4.reference_id)],
+            "meta": {
+                "next_cursor": str(person4.reference_id),
+                "next": f"http://testserver/person/orphaned?limit=2&cursor={str(person4.reference_id)}",
+            },
+        }
+
+        response = client.get(f"/person/orphaned?limit=5&cursor={person2.reference_id}")
+        assert response.json() == {
+            "data": [str(person3.reference_id), str(person4.reference_id)],
+            "meta": {"next_cursor": None, "next": None},
+        }
+
+    def test_get_orphaned_persons_with_cursor(self, client):
+        person1 = models.Person(reference_id=uuid.uuid4(), id=1)
+        patient1 = models.Patient(person=person1)
+        patient2 = models.Patient(person=None)
+        person3 = models.Person(id=3)
+        person4 = models.Person(id=4)
+        client.session.add_all([patient1, patient2, person3, person4])
+        client.session.flush()
+
+        # Retrieve 1 person after person 1, return no cursor
+        response = client.get(f"/person/orphaned?&cursor={person3.reference_id}")
+        assert response.status_code == 200
+        assert response.json() == {
+            "data": [str(person4.reference_id)],
+            "meta": {
+                "next_cursor": None,
+                "next": None,
+            },
+        }
+
+        # Retrieve 1 person after person 1, return cursor for person 3
+        response = client.get(f"/person/orphaned?limit=1&cursor={person1.reference_id}")
+        assert response.json() == {
+            "data": [str(person3.reference_id)],
+            "meta": {
+                "next_cursor": str(person3.reference_id),
+                "next": f"http://testserver/person/orphaned?limit=1&cursor={str(person3.reference_id)}",
+            },
+        }
+
+        # Retrieve 2 persons after person 2, return cursor for person 4
+        response = client.get(f"/person/orphaned?limit=2&cursor={person1.reference_id}")
+        assert response.json() == {
+            "data": [str(person3.reference_id), str(person4.reference_id)],
+            "meta": {
+                "next_cursor": str(person4.reference_id),
+                "next": f"http://testserver/person/orphaned?limit=2&cursor={str(person4.reference_id)}",
+            },
+        }
+
+    def test_invalid_cursor(self, client):
+        # Return 422 if bad patient reference_id is provided as cursor
+        response = client.get(f"/person/orphaned?limit=1&cursor={uuid.uuid4()}")
+        assert response.status_code == 422
+        assert response.json() == {
+            "detail": [
+                {
+                    "loc": ["query", "cursor"],
+                    "msg": "Cursor is an invalid Person reference_id",
+                    "type": "value_error",
+                }
+            ]
+        }