Merge pull request #106 from NHSDigital/DTOSS-12877-amend-mwl-response-when-worklist-item-exists

Amend create worklist item response when item already exists

Author: steventux

src/services/mwl/create_worklist_item.py

@@ -1,7 +1,7 @@
 import logging
 
 from models import WorklistItem
-from services.storage import DuplicateWorklistItemError, MWLStorage
+from services.storage import MWLStorage, WorklistItemExistsError
 
 logger = logging.getLogger(__name__)
 
@@ -19,13 +19,14 @@ def call(self, payload: dict):
             params = payload.get("parameters", {})
 
             item = params.get("worklist_item", {})
+            accession_number = item.get("accession_number")
             participant = item.get("participant", {})
             scheduled = item.get("scheduled", {})
             procedure = item.get("procedure", {})
 
             self.storage.store_worklist_item(
                 WorklistItem(
-                    accession_number=item.get("accession_number"),
+                    accession_number=accession_number,
                     patient_id=participant.get("nhs_number"),
                     patient_name=participant.get("name"),
                     patient_birth_date=participant.get("birth_date"),
@@ -37,13 +38,11 @@ def call(self, payload: dict):
                     source_message_id=action_id,
                 )
             )
-            logger.info(f"Created worklist item: {item.get('accession_number')}")
+            logger.info(f"Created worklist item: {accession_number}")
             return {"status": "created", "action_id": action_id}
-        except DuplicateWorklistItemError:
-            logger.warning(
-                f"Duplicate worklist item ignored: accession_number={item.get('accession_number')!r}, action_id={action_id!r}"
-            )
-            return {"status": "duplicate", "action_id": action_id}
+        except WorklistItemExistsError:
+            logger.info(f"Worklist item exists: accession_number={accession_number}, action_id={action_id!r}")
+            return {"status": "exists", "action_id": action_id}
         except Exception as e:
             logger.error(f"Failed to create worklist item: {e}")
             return {"status": "error", "action_id": action_id, "error": str(e)}

src/services/storage.py

@@ -291,7 +291,7 @@ class InvalidStatusTransitionError(Exception):
     pass
 
 
-class DuplicateWorklistItemError(Exception):
+class WorklistItemExistsError(Exception):
     """Raised when a worklist item with the same accession number already exists."""
 
     pass
@@ -346,7 +346,7 @@ def store_worklist_item(
                 )
                 conn.commit()
         except sqlite3.IntegrityError:
-            raise DuplicateWorklistItemError(f"Worklist item already exists: {worklist_item.accession_number}")
+            raise WorklistItemExistsError(f"Worklist item already exists: {worklist_item.accession_number}")
 
         return worklist_item.accession_number
 

tests/services/mwl/test_create_worklist_item.py

@@ -1,64 +1,47 @@
 from unittest.mock import patch
 
+import pytest
+
 from services.mwl.create_worklist_item import CreateWorklistItem
-from services.storage import DuplicateWorklistItemError, WorklistItem
+from services.storage import MWLStorage
 
 
-@patch(f"{CreateWorklistItem.__module__}.MWLStorage")
 class TestCreateWorklistItem:
-    def test_call_success(self, mock_mwl_storage, listener_payload):
-        mock_storage_instance = mock_mwl_storage.return_value
-        subject = CreateWorklistItem(mock_storage_instance)
+    @pytest.fixture
+    def db_file(tmp_path):
+        return f"{tmp_path}/test.db"
+
+    @pytest.fixture
+    def mwl_storage(db_file):
+        return MWLStorage(str(db_file))
+
+    def test_call_success(self, mwl_storage, listener_payload):
+        subject = CreateWorklistItem(mwl_storage)
 
         response = subject.call(listener_payload)
         assert response == {"action_id": "action-12345", "status": "created"}
 
-        mock_storage_instance.store_worklist_item.assert_called_once_with(
-            WorklistItem(
-                accession_number="ACC999999",
-                patient_id="999123456",
-                patient_name="SMITH^JANE",
-                patient_birth_date="19900202",
-                patient_sex="F",
-                scheduled_date="20240615",
-                scheduled_time="101500",
-                modality="MG",
-                study_description="MAMMOGRAPHY",
-                source_message_id="action-12345",
-            )
-        )
-
-    def test_call_missing_action_id(self, mock_mwl_storage, listener_payload):
-        mock_storage_instance = mock_mwl_storage.return_value
-        subject = CreateWorklistItem(mock_storage_instance)
+    def test_call_missing_action_id(self, mwl_storage, listener_payload):
+        subject = CreateWorklistItem(mwl_storage)
 
         del listener_payload["action_id"]
 
         response = subject.call(listener_payload)
         assert response["status"] == "error"
         assert "Missing action_id" in response["error"]
 
-        mock_storage_instance.store_worklist_item.assert_not_called()
+    def test_call_existing_worklist_item(self, mwl_storage, listener_payload):
+        CreateWorklistItem(mwl_storage).call(listener_payload)
 
-    def test_call_duplicate_worklist_item(self, mock_mwl_storage, listener_payload):
-        mock_storage_instance = mock_mwl_storage.return_value
-        mock_storage_instance.store_worklist_item.side_effect = DuplicateWorklistItemError(
-            "Worklist item already exists: ACC999999"
-        )
-        subject = CreateWorklistItem(mock_storage_instance)
+        subject = CreateWorklistItem(mwl_storage)
 
         response = subject.call(listener_payload)
-        assert response == {"status": "duplicate", "action_id": "action-12345"}
-
-        mock_storage_instance.store_worklist_item.assert_called_once()
+        assert response == {"status": "exists", "action_id": "action-12345"}
 
-    def test_call_storage_exception(self, mock_mwl_storage, listener_payload):
-        mock_storage_instance = mock_mwl_storage.return_value
-        mock_storage_instance.store_worklist_item.side_effect = Exception("DB error")
-        subject = CreateWorklistItem(mock_storage_instance)
+    @patch(f"{CreateWorklistItem.__module__}.MWLStorage.store_worklist_item", side_effect=Exception("DB error"))
+    def test_call_storage_exception(self, _, mwl_storage, listener_payload):
+        subject = CreateWorklistItem(mwl_storage)
 
         response = subject.call(listener_payload)
         assert response["status"] == "error"
         assert "DB error" in response["error"]
-
-        mock_storage_instance.store_worklist_item.assert_called_once()

tests/services/test_storage.py

@@ -6,7 +6,13 @@
 from pydicom.uid import generate_uid
 
 from models import WorklistItem
-from services.storage import InvalidStatusTransitionError, MWLStorage, PACSStorage, WorklistItemNotFoundError
+from services.storage import (
+    InvalidStatusTransitionError,
+    MWLStorage,
+    PACSStorage,
+    WorklistItemExistsError,
+    WorklistItemNotFoundError,
+)
 
 
 @pytest.fixture
@@ -136,6 +142,13 @@ def test_store_worklist_item(self, mwl_storage, result):
         assert row is not None
         assert row["patient_id"] == item.patient_id
 
+    def test_store_worklist_item_already_exists(self, mwl_storage, result):
+        item = WorklistItem(**result)
+        mwl_storage.store_worklist_item(item)
+
+        with pytest.raises(WorklistItemExistsError):
+            mwl_storage.store_worklist_item(item)
+
     def test_find_worklist_items(self, mwl_storage, result):
         item = self._insert_item(mwl_storage, result)