Skip to content

Comments

Fix - Remove C-FIND status clause#22

Merged
steventux merged 2 commits intomainfrom
fix/remove-c-find-status-clause
Feb 13, 2026
Merged

Fix - Remove C-FIND status clause#22
steventux merged 2 commits intomainfrom
fix/remove-c-find-status-clause

Conversation

@steventux
Copy link
Contributor

@steventux steventux commented Feb 10, 2026

Description

C-FIND should not implicitly filter worklist items that are not SCHEDULED but this is currently the default.
There's no mention of filtering by status with C-FIND query identifiers.
See https://dicom.nema.org/dicom/2013/output/chtml/part04/sect_K.6.html#table_K.6-1 where attributes like Modality have notes like The Modality shall be retrieved with Single Value Matching. there is no retrieval note for ScheduledProcedureStepStatus.
This PR removes the query clause.

Jira link

https://nhsd-jira.digital.nhs.uk/browse/DTOSS-12237

Review notes

Review checklist

  • Check database queries are correctly scoped to current_provider

Modalities will likely need SCHEDULED and IN PROGRESS items at the very
least. Defaulting to SCHEDULED may obscure items.
Remove the status clause, we can implement this as a query parameter.
@steventux steventux force-pushed the fix/remove-c-find-status-clause branch from 745d71a to 4d34a1a Compare February 10, 2026 17:26
@steventux steventux requested review from a team, carlosmartinez and gpeng February 11, 2026 15:35
)

logger.info(f"Found {len(items)} matching worklist items")
logger.info("Found %s matching worklist items", len(items))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

y u hate fstrings so much?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a (very) minor optimisation. If the log statement in question is not called (ie the log level is higher) then the string interpolation does not occur. Whereas with fstring interpolation is implicit.
IIRC there is a Python linter somewhere that enforces this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah well, if we're saving the planet than i approve

Copy link
Contributor

@carlosmartinez carlosmartinez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@steventux steventux marked this pull request as draft February 13, 2026 11:09
@steventux steventux marked this pull request as ready for review February 13, 2026 11:10
@steventux steventux marked this pull request as draft February 13, 2026 11:31
@steventux steventux marked this pull request as ready for review February 13, 2026 11:31
@steventux steventux merged commit 39ba64e into main Feb 13, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants