Require JavaScript for selective transfer (never worked well enough w…

…ithout it)

TODO.md

@@ -3,12 +3,10 @@
 ## Top
 
 - Before new release
+  -- Fix order of forms that source / destination fields are at the top again
   -- test job_utils
-  -- move mixins stuff over to consumer and delete the stuff in slective transfer view (we never post there)
   -- exclude autoreload when tests are saved (Custom Filter in server command watched files)
   -- Test canceled task/job in test_workers.py
-  -- Test locked of queued task
-  -- Fix pyright (wait for new release)
 
 - Rename C_STORE to C-STORE and so on in dimse connector
 - Replace AssertionError with assert

adit/batch_query/utils/query_utils.py

@@ -23,6 +23,7 @@ class QueryExecutor:
     def __init__(self, query_task: BatchQueryTask) -> None:
         self.query_task = query_task
 
+        # Make sure source is accessible by the user
         source_node = DicomNode.objects.accessible_by_user(self.query_task.job.owner, "source").get(
             pk=self.query_task.source.pk
         )

adit/core/templates/core/js_required_hint.html

@@ -0,0 +1,8 @@
+{% extends "core/core_layout.html" %}
+{% block heading %}
+    <h4 class="mb-3">JavaScript Required</h4>
+{% endblock heading %}
+{% block content %}
+    <div class="alert alert-warning" role="alert">{{ hint }}</div>
+    <a href="" class="link-primary">← Go Back</a>
+{% endblock content %}

adit/core/utils/transfer_utils.py

@@ -30,6 +30,7 @@ class TransferExecutor:
     def __init__(self, transfer_task: TransferTask) -> None:
         self.transfer_task = transfer_task
 
+        # Make sure source and destination are accessible by the user
         source_node = DicomNode.objects.accessible_by_user(
             self.transfer_task.job.owner, "source"
         ).get(pk=self.transfer_task.source.pk)

adit/selective_transfer/consumers.py

@@ -3,7 +3,8 @@
 import logging
 import threading
 from concurrent.futures import ThreadPoolExecutor
-from typing import Any
+from datetime import datetime
+from typing import Any, Iterator, cast
 
 from asgiref.sync import async_to_sync
 from channels.db import database_sync_to_async
@@ -13,11 +14,13 @@
 from django.template.loader import render_to_string
 
 from adit.accounts.models import User
-from adit.core.utils.dicom_dataset import ResultDataset
+from adit.core.models import DicomNode
+from adit.core.utils.dicom_dataset import QueryDataset, ResultDataset
 from adit.core.utils.dicom_operator import DicomOperator
+from adit.core.utils.job_utils import queue_pending_tasks
 
 from .forms import SelectiveTransferJobForm
-from .mixins import SelectiveTransferJobCreateMixin
+from .models import SelectiveTransferJob, SelectiveTransferTask
 from .utils.debounce import debounce
 from .views import SELECTIVE_TRANSFER_ADVANCED_OPTIONS_COLLAPSED
 
@@ -47,7 +50,7 @@ def render_error_message(message) -> str:
     )
 
 
-class SelectiveTransferConsumer(SelectiveTransferJobCreateMixin, AsyncJsonWebsocketConsumer):
+class SelectiveTransferConsumer(AsyncJsonWebsocketConsumer):
     async def connect(self):
         logger.debug("Connected to WebSocket client.")
 
@@ -149,19 +152,24 @@ def _build_form_error_response(self, form: SelectiveTransferJobForm, message: st
     async def _make_query(self, form: SelectiveTransferJobForm, message_id: int) -> None:
         loop = asyncio.get_event_loop()
         await loop.run_in_executor(
-            self.pool, self._create_and_send_query_response, form, message_id
+            self.pool, self._generate_and_send_query_response, form, message_id
         )
 
-    def _create_and_send_query_response(
+    def _generate_and_send_query_response(
         self, form: SelectiveTransferJobForm, message_id: int
     ) -> None:
         with lock:
             if message_id != self.current_message_id:
                 return
 
-            # TODO: we have to query multiple servers here by creating multiple operators,
-            # also the source must be attached somehow
-            operator = self.create_source_operator(form)
+            # Make sure source is accessible by the user
+            source = cast(DicomNode, form.cleaned_data["source"])
+            source_node = DicomNode.objects.accessible_by_user(self.user, "source").get(
+                pk=source.pk
+            )
+            assert source_node.node_type == DicomNode.NodeType.SERVER
+            operator = DicomOperator(source_node.dicomserver)
+
             self.query_operators.append(operator)
 
         try:
@@ -191,6 +199,38 @@ def _create_and_send_query_response(
 
         return None
 
+    def query_studies(
+        self, operator: DicomOperator, form: SelectiveTransferJobForm, limit_results: int
+    ) -> Iterator[ResultDataset]:
+        data = form.cleaned_data
+
+        if data["modality"] in settings.EXCLUDED_MODALITIES:
+            return []
+
+        studies = operator.find_studies(
+            QueryDataset.create(
+                PatientID=data["patient_id"],
+                PatientName=data["patient_name"],
+                PatientBirthDate=data["patient_birth_date"],
+                AccessionNumber=data["accession_number"],
+                StudyDate=data["study_date"],
+                ModalitiesInStudy=data["modality"],
+            ),
+            limit_results=limit_results,
+        )
+
+        def has_only_excluded_modalities(study: ResultDataset):
+            modalities_in_study = set(study.ModalitiesInStudy)
+            excluded_modalities = set(settings.EXCLUDED_MODALITIES)
+            not_excluded_modalities = list(modalities_in_study - excluded_modalities)
+            return len(not_excluded_modalities) == 0
+
+        for study in studies:
+            if has_only_excluded_modalities(study):
+                continue
+
+            yield study
+
     @debounce()
     def send_query_response(
         self,
@@ -201,7 +241,11 @@ def send_query_response(
         # Rerender form to remove potential previous error messages
         rendered_form = render_crispy_form(form)
 
-        studies = self.sort_studies(studies)
+        studies = sorted(
+            studies,
+            key=lambda study: datetime.combine(study.StudyDate, study.StudyTime),
+            reverse=True,
+        )
 
         rendered_query_results = render_to_string(
             "selective_transfer/_query_results.html",
@@ -240,3 +284,40 @@ def build_transfer_response(
             {"transfer": True, "created_job": job},
         )
         return rendered_form + rendered_created_job
+
+    def transfer_selected_studies(
+        self, user: User, form: SelectiveTransferJobForm, selected_studies: list[str]
+    ) -> SelectiveTransferJob:
+        if not selected_studies:
+            raise ValueError("At least one study to transfer must be selected.")
+        if len(selected_studies) > 10 and not user.is_staff:
+            raise ValueError("Maximum 10 studies per selective transfer are allowed.")
+
+        form.instance.owner = user
+        job = form.save()
+
+        pseudonym = form.cleaned_data["pseudonym"]
+        for selected_study in selected_studies:
+            study_data = selected_study.split("\\")
+            patient_id = study_data[0]
+            study_uid = study_data[1]
+            SelectiveTransferTask.objects.create(
+                job=job,
+                source=form.cleaned_data["source"],
+                destination=form.cleaned_data["destination"],
+                patient_id=patient_id,
+                study_uid=study_uid,
+                pseudonym=pseudonym,
+            )
+
+        if user.is_staff or settings.SELECTIVE_TRANSFER_UNVERIFIED:
+            job.status = SelectiveTransferJob.Status.PENDING
+            job.save()
+
+            queue_pending_tasks(
+                job,
+                settings.SELECTIVE_TRANSFER_DEFAULT_PRIORITY,
+                settings.SELECTIVE_TRANSFER_URGENT_PRIORITY,
+            )
+
+        return job

adit/selective_transfer/forms.py

@@ -56,7 +56,7 @@ class Meta:
 
     def __init__(self, *args, **kwargs):
         self.user = kwargs.pop("user")
-        self.action = kwargs.pop("action")
+        self.action = kwargs.pop("action", "")
         self.advanced_options_collapsed = kwargs.pop("advanced_options_collapsed")
 
         super().__init__(*args, **kwargs)

adit/selective_transfer/mixins.py

@@ -1,111 +1,9 @@
-from datetime import datetime
-from typing import Iterator
-
-from django.conf import settings
-
-from adit.accounts.models import User
 from adit.core.mixins import LockedMixin
-from adit.core.utils.dicom_dataset import QueryDataset, ResultDataset
-from adit.core.utils.dicom_operator import DicomOperator
-from adit.core.utils.job_utils import queue_pending_tasks
 
 from .apps import SECTION_NAME
-from .forms import SelectiveTransferJobForm
-from .models import SelectiveTransferJob, SelectiveTransferSettings, SelectiveTransferTask
+from .models import SelectiveTransferSettings
 
 
 class SelectiveTransferLockedMixin(LockedMixin):
     settings_model = SelectiveTransferSettings
     section_name = SECTION_NAME
-
-
-class SelectiveTransferJobCreateMixin:
-    """A mixin shared by the "SelectiveTransferJobCreateView and the SelectiveTransferConsumer.
-
-    It contains common logic to create a selective transfer job. The view has some rudimentary
-    functionality to make a query, display the result and create the job. The consumer
-    does this more interactively and is the preferred way, but depends on JavaScript and
-    WebSockets. The view is a fallback for browsers without JavaScript support.
-    TODO: As more and more things inside ADIT now depend on JavaScript, we should consider
-    to remove the view logic to query and create a job from the view and only use the consumer.
-    The view then just renders the initial form and the consumer does the rest.
-    """
-
-    def create_source_operator(self, form: SelectiveTransferJobForm) -> DicomOperator:
-        return DicomOperator(form.cleaned_data["source"].dicomserver)
-
-    def query_studies(
-        self, operator: DicomOperator, form: SelectiveTransferJobForm, limit_results: int
-    ) -> Iterator[ResultDataset]:
-        data = form.cleaned_data
-
-        if data["modality"] in settings.EXCLUDED_MODALITIES:
-            return []
-
-        studies = operator.find_studies(
-            QueryDataset.create(
-                PatientID=data["patient_id"],
-                PatientName=data["patient_name"],
-                PatientBirthDate=data["patient_birth_date"],
-                AccessionNumber=data["accession_number"],
-                StudyDate=data["study_date"],
-                ModalitiesInStudy=data["modality"],
-            ),
-            limit_results=limit_results,
-        )
-
-        def has_only_excluded_modalities(study: ResultDataset):
-            modalities_in_study = set(study.ModalitiesInStudy)
-            excluded_modalities = set(settings.EXCLUDED_MODALITIES)
-            not_excluded_modalities = list(modalities_in_study - excluded_modalities)
-            return len(not_excluded_modalities) == 0
-
-        for study in studies:
-            if has_only_excluded_modalities(study):
-                continue
-
-            yield study
-
-    def sort_studies(self, studies: list[ResultDataset]) -> list[ResultDataset]:
-        return sorted(
-            studies,
-            key=lambda study: datetime.combine(study.StudyDate, study.StudyTime),
-            reverse=True,
-        )
-
-    def transfer_selected_studies(
-        self, user: User, form: SelectiveTransferJobForm, selected_studies: list[str]
-    ) -> SelectiveTransferJob:
-        if not selected_studies:
-            raise ValueError("At least one study to transfer must be selected.")
-        if len(selected_studies) > 10 and not user.is_staff:
-            raise ValueError("Maximum 10 studies per selective transfer are allowed.")
-
-        form.instance.owner = user
-        job = form.save()
-
-        pseudonym = form.cleaned_data["pseudonym"]
-        for selected_study in selected_studies:
-            study_data = selected_study.split("\\")
-            patient_id = study_data[0]
-            study_uid = study_data[1]
-            SelectiveTransferTask.objects.create(
-                job=job,
-                source=form.cleaned_data["source"],
-                destination=form.cleaned_data["destination"],
-                patient_id=patient_id,
-                study_uid=study_uid,
-                pseudonym=pseudonym,
-            )
-
-        if user.is_staff or settings.SELECTIVE_TRANSFER_UNVERIFIED:
-            job.status = SelectiveTransferJob.Status.PENDING
-            job.save()
-
-            queue_pending_tasks(
-                job,
-                settings.SELECTIVE_TRANSFER_DEFAULT_PRIORITY,
-                settings.SELECTIVE_TRANSFER_URGENT_PRIORITY,
-            )
-
-        return job