Conversation
There was a problem hiding this comment.
Pull Request Overview
Updates development dependencies and improves type safety while fixing an incorrect redirect after deleting a batch query job. Key changes include dependency upgrades, a redirect bug fix, stricter typing via cast usage, authentication handling in the WebSocket consumer, and test helper adjustments to avoid connection reuse issues.
- Fix incorrect redirect target for batch query job deletion
- Enhance WebSocket consumer with authentication guard and type narrowing changes
- Add connection handling tweak and type adjustments in test utilities
Reviewed Changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pyproject.toml | Adjusts dev dependencies (removes upper bound for pyright and drops pytest-only). |
| adit/selective_transfer/views.py | Adds type cast for success_url. |
| adit/batch_transfer/views.py | Adds type cast for success_url. |
| adit/batch_query/views.py | Corrects redirect URL name and adds type cast. |
| adit/selective_transfer/consumers.py | Adds authentication check and casts action before form handling. |
| adit/core/utils/testing_helpers.py | Returns MagicMock for association mock and forces per-request HTTP connections in tests. |
| adit/core/filters.py | Uses with_form_helper wrapper for assigning form helper. |
| adit/batch_query/filters.py | Same helper pattern applied with line wrapping. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
adit/selective_transfer/consumers.py
Outdated
| typed_action = cast(Literal["query", "transfer"], action) | ||
|
|
||
| # We are now in a query or transfer action so we have to process the form | ||
| form = await self.get_form(action, content) | ||
| form = await self.get_form(typed_action, content) |
There was a problem hiding this comment.
[nitpick] Casting action to Literal["query", "transfer"] before validating that it is one of those values suppresses type checker warnings and can mask unsupported actions; perform an explicit value check (or use a guard if action not in {"query","transfer"}) instead of an unchecked cast.
| if getattr(client, "_session", None): | ||
| client._session.headers["Connection"] = "close" |
There was a problem hiding this comment.
[nitpick] Accessing the private attribute _session couples the test helper to an internal implementation detail of DICOMwebClient; prefer a public API (if available) or add a helper/utility wrapper so future library changes do not break this silently.
Small bug fix with a wrong redirect when a query job got deleted.