Skip to content

Commit 39ba64e

Browse files
authored
Merge pull request #22 from NHSDigital/fix/remove-c-find-status-clause
Fix - Remove C-FIND status clause
2 parents b876a74 + 4d34a1a commit 39ba64e

6 files changed

Lines changed: 41 additions & 40 deletions

File tree

src/server.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515

1616
from services.dicom.c_echo import CEcho
1717
from services.dicom.c_store import CStore
18-
from services.mwl.c_find import CFindHandler
18+
from services.mwl.c_find import CFind
1919
from services.mwl.n_create import NCreate
2020
from services.mwl.n_set import NSet
2121
from services.storage import MWLStorage, PACSStorage
@@ -106,7 +106,7 @@ def start(self):
106106
self.ae.add_supported_context(ModalityPerformedProcedureStep)
107107

108108
handlers = [
109-
(evt.EVT_C_FIND, CFindHandler(self.storage).call),
109+
(evt.EVT_C_FIND, CFind(self.storage).call),
110110
(evt.EVT_N_CREATE, NCreate(self.storage).call),
111111
(evt.EVT_N_SET, NSet(self.storage).call),
112112
]

src/services/mwl/c_find.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
logger = logging.getLogger(__name__)
1717

1818

19-
class CFindHandler:
19+
class CFind:
2020
"""Handler for C-FIND worklist queries."""
2121

2222
def __init__(self, storage: MWLStorage):
@@ -44,17 +44,18 @@ def call(self, event: evt.Event) -> Iterator[Tuple[int, Dataset | None]]:
4444
query_modality = procedure_sequence[0].get("Modality")
4545
query_date = procedure_sequence[0].get("ScheduledProcedureStepStartDate")
4646

47-
logger.debug(f"Query parameters: modality={query_modality}, date={query_date}, patient_id={query_patient_id}")
47+
logger.debug(
48+
"Query parameters: modality=%s, date=%s, patient_id=%s", query_modality, query_date, query_patient_id
49+
)
4850

4951
try:
5052
items = self.storage.find_worklist_items(
5153
modality=query_modality if query_modality else None,
5254
scheduled_date=query_date if query_date else None,
5355
patient_id=query_patient_id if query_patient_id else None,
54-
status="SCHEDULED",
5556
)
5657

57-
logger.info(f"Found {len(items)} matching worklist items")
58+
logger.info("Found %s matching worklist items", len(items))
5859

5960
for item in items:
6061
response_ds = self._build_worklist_response(item)
@@ -63,7 +64,7 @@ def call(self, event: evt.Event) -> Iterator[Tuple[int, Dataset | None]]:
6364
yield SUCCESS, None
6465

6566
except Exception as e:
66-
logger.error(f"Error processing C-FIND request: {e}", exc_info=True)
67+
logger.error("Error processing C-FIND request: %s", e, exc_info=True)
6768
yield FAILURE, None
6869

6970
def _build_worklist_response(self, item: WorklistItem) -> Dataset:
@@ -97,6 +98,6 @@ def _build_worklist_response(self, item: WorklistItem) -> Dataset:
9798

9899
ds.ScheduledProcedureStepSequence = [sps_item]
99100

100-
logger.debug(f"Built worklist response for accession {item.accession_number}")
101+
logger.debug("Built worklist response for accession %s", item.accession_number)
101102

102103
return ds

src/services/storage.py

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,6 @@ def find_worklist_items(
358358
modality: Optional[str] = None,
359359
scheduled_date: Optional[str] = None,
360360
patient_id: Optional[str] = None,
361-
status: str = "SCHEDULED",
362361
) -> List[WorklistItem]:
363362
"""
364363
Query worklist items with optional filters.
@@ -367,7 +366,6 @@ def find_worklist_items(
367366
modality: Filter by modality (e.g., "MG")
368367
scheduled_date: Filter by scheduled date (YYYYMMDD)
369368
patient_id: Filter by patient ID
370-
status: Filter by status (default: "SCHEDULED")
371369
372370
Returns:
373371
List of WorklistItem instances matching the criteria
@@ -376,22 +374,26 @@ def find_worklist_items(
376374
"SELECT accession_number, modality, patient_birth_date, patient_id, "
377375
"patient_name, patient_sex, procedure_code, scheduled_date, scheduled_time, "
378376
"source_message_id, study_description, study_instance_uid, status, mpps_instance_uid "
379-
"FROM worklist_items WHERE status = ?"
377+
"FROM worklist_items"
380378
)
381-
params = [status]
379+
where_clauses = []
380+
params = []
382381

383382
if modality:
384-
query += " AND modality = ?"
383+
where_clauses.append("modality = ?")
385384
params.append(modality)
386385

387386
if scheduled_date:
388-
query += " AND scheduled_date = ?"
387+
where_clauses.append("scheduled_date = ?")
389388
params.append(scheduled_date)
390389

391390
if patient_id:
392-
query += " AND patient_id = ?"
391+
where_clauses.append("patient_id = ?")
393392
params.append(patient_id)
394393

394+
if where_clauses:
395+
query += " WHERE " + " AND ".join(where_clauses)
396+
395397
query += " ORDER BY scheduled_date, scheduled_time"
396398

397399
with self._get_connection() as conn:

tests/integration/test_c_find_returns_worklist_items.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
from pydicom.uid import generate_uid
66

77
from services.dicom import PENDING, SUCCESS
8-
from services.mwl.c_find import CFindHandler
8+
from services.mwl.c_find import CFind
99
from services.storage import MWLStorage, WorklistItem
1010

1111

@@ -63,7 +63,7 @@ def event(self):
6363
return event
6464

6565
def test_cfind_returns_scheduled_items(self, event, storage):
66-
results = list(CFindHandler(storage).call(event))
66+
results = list(CFind(storage).call(event))
6767
assert len(results) == 3
6868

6969
status, ds = results[0]
@@ -99,7 +99,7 @@ def test_cfind_returns_scheduled_items(self, event, storage):
9999
def test_cfind_filters_by_scheduled_date(self, event, storage):
100100
event.identifier.ScheduledProcedureStepSequence[0].ScheduledProcedureStepStartDate = "20240101"
101101

102-
results = list(CFindHandler(storage).call(event))
102+
results = list(CFind(storage).call(event))
103103

104104
assert len(results) == 2
105105

@@ -137,7 +137,7 @@ def test_cfind_filters_by_modality(self, event, storage):
137137

138138
event.identifier.ScheduledProcedureStepSequence[0].Modality = "MG"
139139

140-
results = list(CFindHandler(storage).call(event))
140+
results = list(CFind(storage).call(event))
141141

142142
assert len(results) == 3
143143

@@ -156,7 +156,7 @@ def test_cfind_filters_by_modality(self, event, storage):
156156
def test_cfind_filters_by_patient_id(self, event, storage):
157157
event.identifier.PatientID = "999234567"
158158

159-
results = list(CFindHandler(storage).call(event))
159+
results = list(CFind(storage).call(event))
160160

161161
assert len(results) == 2
162162

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from pydicom import Dataset
77

88
from services.dicom import FAILURE, PENDING, SUCCESS
9-
from services.mwl.c_find import CFindHandler
9+
from services.mwl.c_find import CFind
1010
from services.storage import WorklistItem
1111

1212

@@ -17,7 +17,7 @@ def mock_storage():
1717

1818
@pytest.fixture
1919
def handler(mock_storage):
20-
return CFindHandler(mock_storage)
20+
return CFind(mock_storage)
2121

2222

2323
@pytest.fixture
@@ -46,8 +46,8 @@ def sample_worklist_item():
4646
}
4747

4848

49-
class TestCFindHandler:
50-
"""Tests for CFindHandler class."""
49+
class TestCFind:
50+
"""Tests for CFind class."""
5151

5252
def test_call_with_no_results(self, handler, mock_storage, mock_event):
5353
mock_storage.find_worklist_items.return_value = []
@@ -116,9 +116,7 @@ def test_call_with_modality_filter(self, handler, mock_storage, mock_event):
116116

117117
list(handler.call(mock_event))
118118

119-
mock_storage.find_worklist_items.assert_called_once_with(
120-
modality="MG", scheduled_date=None, patient_id=None, status="SCHEDULED"
121-
)
119+
mock_storage.find_worklist_items.assert_called_once_with(modality="MG", scheduled_date=None, patient_id=None)
122120

123121
def test_call_with_date_filter(self, handler, mock_storage, mock_event):
124122
sps_item = Dataset()
@@ -129,7 +127,7 @@ def test_call_with_date_filter(self, handler, mock_storage, mock_event):
129127
list(handler.call(mock_event))
130128

131129
mock_storage.find_worklist_items.assert_called_once_with(
132-
modality=None, scheduled_date="20260107", patient_id=None, status="SCHEDULED"
130+
modality=None, scheduled_date="20260107", patient_id=None
133131
)
134132

135133
def test_call_with_patient_id_filter(self, handler, mock_storage, mock_event):
@@ -139,7 +137,7 @@ def test_call_with_patient_id_filter(self, handler, mock_storage, mock_event):
139137
list(handler.call(mock_event))
140138

141139
mock_storage.find_worklist_items.assert_called_once_with(
142-
modality=None, scheduled_date=None, patient_id="9876543210", status="SCHEDULED"
140+
modality=None, scheduled_date=None, patient_id="9876543210"
143141
)
144142

145143
def test_call_handles_storage_exception(self, handler, mock_storage, mock_event):

tests/services/test_storage.py

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -211,9 +211,9 @@ def test_find_worklist_items(self, mock_db, tmp_dir, result):
211211
"SELECT accession_number, modality, patient_birth_date, patient_id, "
212212
"patient_name, patient_sex, procedure_code, scheduled_date, scheduled_time, "
213213
"source_message_id, study_description, study_instance_uid, status, mpps_instance_uid "
214-
"FROM worklist_items WHERE status = ? ORDER BY scheduled_date, scheduled_time"
214+
"FROM worklist_items ORDER BY scheduled_date, scheduled_time"
215215
),
216-
["SCHEDULED"],
216+
[],
217217
)
218218

219219
assert len(results) == 1
@@ -236,9 +236,9 @@ def test_find_worklist_items_with_filters(self, mock_db, tmp_dir):
236236
"SELECT accession_number, modality, patient_birth_date, patient_id, "
237237
"patient_name, patient_sex, procedure_code, scheduled_date, scheduled_time, "
238238
"source_message_id, study_description, study_instance_uid, status, mpps_instance_uid "
239-
"FROM worklist_items WHERE status = ? AND patient_id = ? ORDER BY scheduled_date, scheduled_time"
239+
"FROM worklist_items WHERE patient_id = ? ORDER BY scheduled_date, scheduled_time"
240240
),
241-
["SCHEDULED", "999123456"],
241+
["999123456"],
242242
)
243243

244244
mock_connection.reset_mock()
@@ -249,9 +249,9 @@ def test_find_worklist_items_with_filters(self, mock_db, tmp_dir):
249249
"SELECT accession_number, modality, patient_birth_date, patient_id, "
250250
"patient_name, patient_sex, procedure_code, scheduled_date, scheduled_time, "
251251
"source_message_id, study_description, study_instance_uid, status, mpps_instance_uid "
252-
"FROM worklist_items WHERE status = ? AND modality = ? ORDER BY scheduled_date, scheduled_time"
252+
"FROM worklist_items WHERE modality = ? ORDER BY scheduled_date, scheduled_time"
253253
),
254-
["SCHEDULED", "CT"],
254+
["CT"],
255255
)
256256

257257
mock_connection.reset_mock()
@@ -262,10 +262,10 @@ def test_find_worklist_items_with_filters(self, mock_db, tmp_dir):
262262
"SELECT accession_number, modality, patient_birth_date, patient_id, "
263263
"patient_name, patient_sex, procedure_code, scheduled_date, scheduled_time, "
264264
"source_message_id, study_description, study_instance_uid, status, mpps_instance_uid "
265-
"FROM worklist_items WHERE status = ? AND scheduled_date = ? "
265+
"FROM worklist_items WHERE scheduled_date = ? "
266266
"ORDER BY scheduled_date, scheduled_time"
267267
),
268-
["SCHEDULED", "20240101"],
268+
["20240101"],
269269
)
270270

271271
mock_connection.reset_mock()
@@ -276,11 +276,11 @@ def test_find_worklist_items_with_filters(self, mock_db, tmp_dir):
276276
"SELECT accession_number, modality, patient_birth_date, patient_id, "
277277
"patient_name, patient_sex, procedure_code, scheduled_date, scheduled_time, "
278278
"source_message_id, study_description, study_instance_uid, status, mpps_instance_uid "
279-
"FROM worklist_items WHERE status = ? "
280-
"AND modality = ? AND scheduled_date = ? AND patient_id = ? "
279+
"FROM worklist_items "
280+
"WHERE modality = ? AND scheduled_date = ? AND patient_id = ? "
281281
"ORDER BY scheduled_date, scheduled_time"
282282
),
283-
["SCHEDULED", "MG", "20240101", "999123456"],
283+
["MG", "20240101", "999123456"],
284284
)
285285

286286
def test_get_worklist_item(self, mock_db, tmp_dir, result):

0 commit comments

Comments
 (0)