Merge pull request #680 from jbernal0019/master

Implement the execute control for PACS queries

Author: jbernal0019

chris_backend/pacsfiles/migrations/0007_pacsquery_execute.py

@@ -0,0 +1,18 @@
+# Generated by Django 5.2.9 on 2025-12-27 17:21
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('pacsfiles', '0006_alter_pacs_identifier'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='pacsquery',
+            name='execute',
+            field=models.BooleanField(blank=True, default=True),
+        ),
+    ]

chris_backend/pacsfiles/models.py

@@ -55,6 +55,7 @@ class PACSQuery(models.Model):
     title = models.CharField(max_length=300, db_index=True)
     query = models.JSONField()
     description = models.CharField(max_length=700, blank=True)
+    execute = models.BooleanField(blank=True, default=True)
     result = models.TextField(blank=True)
     status = models.CharField(max_length=10, choices=PACS_QUERY_STATUS_CHOICES,
                               default='created')
@@ -108,8 +109,8 @@ class PACSQueryFilter(FilterSet):
     class Meta:
         model = PACSQuery
         fields = ['id', 'min_creation_date', 'max_creation_date', 'title_exact',
-                  'title', 'status', 'description', 'pacs_id', 'pacs_identifier',
-                  'owner_username']
+                  'title', 'status', 'execute', 'description', 'pacs_id',
+                  'pacs_identifier', 'owner_username']
 
 
 PACS_RETRIEVE_STATUS_CHOICES = PACS_QUERY_STATUS_CHOICES

chris_backend/pacsfiles/serializers.py

@@ -18,6 +18,7 @@
 
 
 class PACSSerializer(serializers.HyperlinkedModelSerializer):
+    active = serializers.BooleanField(required=False, default=True)
     folder_path = serializers.ReadOnlyField(source='folder.path')
     folder = serializers.HyperlinkedRelatedField(view_name='chrisfolder-detail',
                                                  read_only=True)
@@ -32,17 +33,20 @@ class Meta:
 
 
 class PACSQuerySerializer(serializers.HyperlinkedModelSerializer):
+    title = serializers.CharField(max_length=300, required=False)
     query = serializers.JSONField(binary=True, required=False)
     pacs_identifier = serializers.ReadOnlyField(source='pacs.identifier')
     owner_username = serializers.ReadOnlyField(source='owner.username')
     result = serializers.ReadOnlyField()
     status = serializers.ReadOnlyField()
+    # explicitly set default to True for boolean fields
+    execute = serializers.BooleanField(required=False, default=True)
     retrieve_list = serializers.HyperlinkedIdentityField(view_name='pacsretrieve-list')
 
     class Meta:
         model = PACSQuery
         fields = ('url', 'id', 'creation_date', 'title', 'query', 'description',
-                  'status', 'pacs_identifier', 'owner_username', 'result',
+                  'status', 'pacs_identifier', 'owner_username',  'execute', 'result',
                   'retrieve_list')
 
     def __init__(self, *args, **kwargs):
@@ -83,11 +87,25 @@ def update(self, instance, validated_data):
                          f'for pacs {pacs.identifier}')
             raise serializers.ValidationError([error_msg])
 
+    def validate_execute(self, execute):
+        """
+        Overriden to validate a change of execute status.
+        """
+        instance = self.instance
+        if instance and instance.execute and not execute:
+            msg = "Can not change field execute from true to false."
+            raise serializers.ValidationError([msg])
+        return execute
+
     def validate(self, data):
         """
-        Overriden to validate that the query field is in data when creating a new query.
+        Overriden to validate that required fields are in data when creating a new query.
         """
         if not self.instance:  # on create
+            if 'title' not in data:
+                raise serializers.ValidationError(
+                    {'title': ["This field is required."]})
+
             if 'query' not in data:
                 raise serializers.ValidationError(
                     {'query': ["This field is required."]})

chris_backend/pacsfiles/tests/test_serializers.py

@@ -1,7 +1,6 @@
 
 import logging
 import io
-import os
 
 from django.contrib.auth.models import User
 from django.test import TestCase, tag
@@ -111,6 +110,37 @@ def test_update_failure_pacs_user_title_combination_already_exists(self):
         with self.assertRaises(serializers.ValidationError):
             pacs_query_serializer.update(pacs_query, data)
 
+    def test_validate_execute(self):
+        """
+        Test whether overriden validate_execute method raises a serializers.ValidationError
+        when the execute field changes from true to false.
+        """
+        user = User.objects.get(username=self.username)
+        pacs = PACS.objects.get(identifier=self.pacs_name)
+        query = {'SeriesInstanceUID': '1.3.12.2.1107'}
+
+        pacs_query, _ = PACSQuery.objects.get_or_create(title='query5', query=query,
+                                                        owner=user, pacs=pacs)
+        pacs_query_serializer = PACSQuerySerializer(pacs_query)
+
+        with self.assertRaises(serializers.ValidationError):
+            pacs_query_serializer.validate_execute(False)
+
+
+    def test_validate_validates_required_query_field_on_create(self):
+        """
+        Test whether overriden validate method validates that the query field must
+        be provided when creating a new PACS query.
+        """
+        user = User.objects.get(username=self.username)
+        pacs = PACS.objects.get(identifier=self.pacs_name)
+
+        data = {'title': 'query6', 'owner': user, 'pacs': pacs}
+        pacs_query_serializer = PACSQuerySerializer(data=data)
+
+        with self.assertRaises(serializers.ValidationError):
+            pacs_query_serializer.validate(data)
+
 
 class PACSSeriesSerializerTests(SerializerTests):
 

chris_backend/pacsfiles/tests/test_views.py

@@ -188,13 +188,13 @@ def setUp(self):
 
         self.create_read_url = reverse("pacsquery-list", kwargs={"pk": pacs.id})
 
-        query = {'SeriesInstanceUID': '2.3.15.2.1057'}
-        pacs_query, _ = PACSQuery.objects.get_or_create(title='query10', query=query,
+        self.query = {'SeriesInstanceUID': '2.3.15.2.1057'}
+        pacs_query, _ = PACSQuery.objects.get_or_create(title='query10', query=self.query,
                                                         owner=user, pacs=pacs)
 
         self.post = json.dumps(
             {"template": {"data": [{"name": "title", "value": 'test1'},
-                                   {"name": "query", "value": json.dumps(query)}]}})
+                                   {"name": "query", "value": json.dumps(self.query)}]}})
 
     def test_pacs_query_list_success(self):
         self.client.login(username=self.username, password=self.password)
@@ -217,7 +217,7 @@ def test_pacs_query_list_failure_unauthenticated(self):
         response = self.client.get(self.create_read_url)
         self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED)
 
-    def test_pacs_query_create_success(self):
+    def test_pacs_query_create_success_execute(self):
         with mock.patch.object(views.send_pacs_query, 'delay',
                                return_value=None) as delay_mock:
             # make API request
@@ -230,6 +230,25 @@ def test_pacs_query_create_success(self):
             delay_mock.assert_called_with(response.data['id'])
             self.assertEqual(response.data['status'], 'created')
 
+    def test_pacs_query_create_success_do_not_execute(self):
+
+        post = json.dumps(
+            {"template": {"data": [{"name": "title", "value": 'test2'},
+                                   {"name": "execute", "value": False},
+                                   {"name": "query", "value": json.dumps(self.query)}]}})
+
+        with mock.patch.object(views.send_pacs_query, 'delay',
+                               return_value=None) as delay_mock:
+            # make API request
+            self.client.login(username=self.username, password=self.password)
+            response = self.client.post(self.create_read_url, data=post,
+                                        content_type=self.content_type)
+            self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+
+            # check that the send_pacs_query task was not called
+            delay_mock.assert_not_called()
+            self.assertEqual(response.data['status'], 'created')
+
     def test_pacs_query_create_failure_unauthenticated(self):
         response = self.client.post(self.create_read_url, data=self.post,
                                     content_type=self.content_type)
@@ -358,6 +377,44 @@ def test_pacs_query_update_success(self):
                                    content_type=self.content_type)
         self.assertContains(response, "Test query")
 
+    def test_pacs_query_update_success_execute(self):
+        pacs = PACS.objects.get(identifier=self.pacs_name)
+        user = User.objects.get(username=self.username)
+        query = {'SeriesInstanceUID': '2.3.15.2.1057'}
+
+        pacs_query, _ = PACSQuery.objects.get_or_create(title='query2', execute=False,
+                                                        query=query, owner=user, pacs=pacs)
+        read_update_delete_url = reverse("pacsquery-detail", kwargs={"pk":pacs_query.id})
+
+        put = json.dumps({
+            "template": {"data": [{"name": "title", "value": "Test query2"},
+                                  {"name": "execute", "value": True}]}})
+
+        self.client.login(username=self.username, password=self.password)
+
+        with mock.patch.object(views.send_pacs_query, 'delay',
+                               return_value=None) as delay_mock:
+            response = self.client.put(read_update_delete_url, data=put,
+                                       content_type=self.content_type)
+            # check that the send_pacs_query task was called with appropriate args
+            delay_mock.assert_called_with(response.data['id'])
+            self.assertContains(response, "Test query2")
+
+    def test_pacs_query_update_success_do_not_execute_again(self):
+        put = json.dumps({
+            "template": {"data": [{"name": "title", "value": "Test query"},
+                                  {"name": "execute", "value": True}]}})
+
+        self.client.login(username=self.username, password=self.password)
+
+        with mock.patch.object(views.send_pacs_query, 'delay',
+                               return_value=None) as delay_mock:
+            response = self.client.put(self.read_update_delete_url, data=put,
+                                       content_type=self.content_type)
+            # check that the send_pacs_query task was not called
+            delay_mock.assert_not_called()
+            self.assertContains(response, "Test query")
+
     def test_pacs_query_update_failure_unauthenticated(self):
         response = self.client.put(self.read_update_delete_url, data=self.put,
                                    content_type=self.content_type)

chris_backend/pacsfiles/views.py

@@ -156,7 +156,7 @@ def list(self, request, *args, **kwargs):
         response = services.append_collection_links(response, links)
 
         # append write template
-        template_data = {'title': '', 'query': '', 'description': ''}
+        template_data = {'title': '', 'query': '', 'description': '', 'execute': ''}
         return services.append_collection_template(response, template_data)
 
     def get_pacs_queries_queryset(self):
@@ -175,11 +175,13 @@ def get_pacs_queries_queryset(self):
     def perform_create(self, serializer):
         """
         Overriden to associate the owner and the pacs with the PACS query before first
-        saving to the DB. Then the PACS query operation is sent to the remote PACS.
+        saving to the DB. Then the PACS query operation is sent to the remote PACS if
+        the execute field is set to True.
         """
         pacs = self.get_object()
         pacs_query = serializer.save(owner=self.request.user, pacs=pacs)
-        send_pacs_query.delay(pacs_query.id)  # call async task
+        if pacs_query.execute:
+            send_pacs_query.delay(pacs_query.id)  # call async task
 
 
 class AllPACSQueryList(generics.ListAPIView):
@@ -249,9 +251,22 @@ def retrieve(self, request, *args, **kwargs):
         Overriden to append a collection+json template to the response.
         """
         response = super(PACSQueryDetail, self).retrieve(request, *args, **kwargs)
-        template_data = {'title': '', 'description': ''}
+        template_data = {'title': '', 'description': '', 'execute': ''}
         return services.append_collection_template(response, template_data)
 
+    def perform_update(self, serializer):
+        """
+        Overriden to execute the PACS query if the execute field changes from False
+        to True.
+        """
+        if 'execute' in self.request.data:
+            instance = self.get_object()
+
+            if self.request.data['execute'] and not instance.execute:
+                send_pacs_query.delay(instance.id)  # call async task
+
+        super(PACSQueryDetail, self).perform_update(serializer)
+
 
 class PACSRetrieveList(generics.ListCreateAPIView):
     """