fix #1735, update create view tests, minor updates (#1706)

CHANGELOG.rst

@@ -31,7 +31,7 @@ Added
     - Enable ``tumor_normal_dna`` ISA-Tab template (#1697)
     - General iRODS access ticket management for assay collections (#804, #1717)
     - Disabled row delete button tooltips (#1731)
-    - ``IrodsDataRequest`` REST API views (#1588, #1706, #1734, #1736)
+    - ``IrodsDataRequest`` REST API views (#1588, #1706, #1734, #1735, #1736)
     - Davrods links in iRODS delete request list (#1339)
     - Batch accepting and rejecting for iRODS delete requests (#1340)
 - **Taskflowbackend**

samplesheets/migrations/0021_update_irodsdatarequest.py

@@ -1,11 +1,13 @@
-# Generated by Django 3.2.19 on 2023-07-24 14:56
+# Generated by Django 3.2.19 on 2023-07-27 13:52
 
 from django.db import migrations, models
+import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
 
     dependencies = [
+        ('projectroles', '0028_populate_finder_role'),
         ('samplesheets', '0020_update_irodsaccessticket'),
     ]
 
@@ -20,6 +22,16 @@ class Migration(migrations.Migration):
             name='action',
             field=models.CharField(default='DELETE', help_text='Action to be performed', max_length=64),
         ),
+        migrations.AlterField(
+            model_name='irodsdatarequest',
+            name='description',
+            field=models.CharField(blank=True, help_text='Request description (optional)', max_length=1024, null=True),
+        ),
+        migrations.AlterField(
+            model_name='irodsdatarequest',
+            name='project',
+            field=models.ForeignKey(help_text='Project to which the iRODS data request belongs', on_delete=django.db.models.deletion.CASCADE, related_name='irods_data_request', to='projectroles.project'),
+        ),
         migrations.AlterField(
             model_name='irodsdatarequest',
             name='status',

samplesheets/models.py

@@ -1320,7 +1320,10 @@ class Meta:
 
     #: Request description (optional)
     description = models.CharField(
-        max_length=1024, help_text='Request description'
+        blank=True,
+        null=True,
+        max_length=1024,
+        help_text='Request description (optional)',
     )
 
     #: DateTime of request creation

samplesheets/tests/test_views_api_taskflow.py

@@ -35,7 +35,10 @@
 from samplesheets.views_api import IRODS_QUERY_ERROR_MSG
 
 from samplesheets.tests.test_io import SampleSheetIOMixin, SHEET_DIR
-from samplesheets.tests.test_models import IrodsDataRequestMixin
+from samplesheets.tests.test_models import (
+    IrodsDataRequestMixin,
+    IRODS_REQUEST_DESC,
+)
 from samplesheets.tests.test_views_taskflow import (
     SampleSheetTaskflowMixin,
     IRODS_FILE_NAME,
@@ -113,27 +116,11 @@ def setUp(self):
             owner=self.user,
             description='description',
         )
-
-        # Import investigation
-        self.investigation = self.import_isa_from_file(SHEET_PATH, self.project)
-        self.study = self.investigation.studies.first()
-        self.assay = self.study.assays.first()
-
-        # Set up iRODS data
-        self.make_irods_colls(self.investigation)
-        self.assay_path = self.irods_backend.get_path(self.assay)
-        self.obj_path = os.path.join(self.assay_path, IRODS_FILE_NAME)
-        # TODO: Are md5 path and obj needed?
-        self.md5_path = os.path.join(self.assay_path, IRODS_FILE_NAME + '.md5')
-        self.file_obj = self.irods.data_objects.create(self.obj_path)
-        self.md5_obj = self.irods.data_objects.create(self.md5_path)
-
         # Init users (owner = user_cat, superuser = user)
         self.user_delegate = self.make_user('user_delegate')
         self.user_contrib = self.make_user('user_contrib')
         self.user_contrib2 = self.make_user('user_contrib2')
         self.user_guest = self.make_user('user_guest')
-
         self.make_assignment_taskflow(
             self.project, self.user_delegate, self.role_delegate
         )
@@ -147,18 +134,28 @@ def setUp(self):
             self.project, self.user_guest, self.role_guest
         )
 
-        # Get appalerts API and model
+        # Import investigation
+        self.investigation = self.import_isa_from_file(SHEET_PATH, self.project)
+        self.study = self.investigation.studies.first()
+        self.assay = self.study.assays.first()
+        # Set up iRODS data
+        self.make_irods_colls(self.investigation)
+        self.assay_path = self.irods_backend.get_path(self.assay)
+        self.obj_path = os.path.join(self.assay_path, IRODS_FILE_NAME)
+        self.file_obj = self.irods.data_objects.create(self.obj_path)
+
+        # Setup for tests
         self.app_alerts = get_backend_api('appalerts_backend')
         self.app_alert_model = self.app_alerts.get_model()
-
-        # Set create URL
         self.url = reverse(
             'samplesheets:api_irods_request_create',
             kwargs={'project': self.project.sodar_uuid},
         )
-        # Set default POST data
-        # TODO: Remove these "bla" things once #1735 has been fixed
-        self.post_data = {'path': self.obj_path, 'description': 'bla'}
+        self.post_data = {
+            'path': self.obj_path,
+            'description': IRODS_REQUEST_DESC,
+        }
+        self.token_contrib = self.get_token(self.user_contrib)
 
 
 class TestInvestigationRetrieveAPIView(TestSampleSheetAPITaskflowBase):
@@ -246,6 +243,10 @@ def test_post_created(self):
         self.assertEqual(response.status_code, 400)
 
 
+# NOTE: For TestIrodsDataRequestRetrieveAPIView, see test_views_api
+# NOTE: For TestIrodsDataRequestListAPIView, see test_views_api
+
+
 class TestIrodsDataRequestCreateAPIView(TestIrodsDataRequestAPIViewBase):
     """Tests for IrodsDataRequestCreateAPIView"""
 
@@ -254,75 +255,83 @@ def test_create(self):
         self.assertEqual(IrodsDataRequest.objects.count(), 0)
         self.assert_alert_count(CREATE_ALERT, self.user, 0)
         self.assert_alert_count(CREATE_ALERT, self.user_delegate, 0)
-
-        with self.login(self.user_contrib):
-            response = self.client.post(self.url, self.post_data)
-
+        response = self.request_knox(
+            self.url, 'POST', data=self.post_data, token=self.token_contrib
+        )
         self.assertEqual(response.status_code, 201)
         obj = IrodsDataRequest.objects.first()
         self.assertEqual(IrodsDataRequest.objects.count(), 1)
         self.assertEqual(obj.path, self.obj_path)
-        self.assertEqual(obj.description, 'bla')
+        self.assertEqual(obj.description, IRODS_REQUEST_DESC)
         self.assert_alert_count(CREATE_ALERT, self.user, 1)
         self.assert_alert_count(CREATE_ALERT, self.user_delegate, 1)
         self.assert_alert_count(CREATE_ALERT, self.user_contrib, 0)
         self.assert_alert_count(CREATE_ALERT, self.user_guest, 0)
 
+    def test_create_no_description(self):
+        """Test creating request without description"""
+        self.assertEqual(IrodsDataRequest.objects.count(), 0)
+        self.post_data['description'] = ''
+        response = self.request_knox(
+            self.url, 'POST', data=self.post_data, token=self.token_contrib
+        )
+        self.assertEqual(response.status_code, 201)
+        obj = IrodsDataRequest.objects.first()
+        self.assertEqual(IrodsDataRequest.objects.count(), 1)
+        self.assertEqual(obj.description, '')
+
     def test_create_trailing_slash(self):
         """Test creating a request with a trailing slash in path"""
         self.assertEqual(IrodsDataRequest.objects.count(), 0)
-        post_data = {'path': self.obj_path + '/', 'description': 'bla'}
-
-        with self.login(self.user_contrib):
-            response = self.client.post(self.url, post_data)
-
+        self.post_data['path'] += '/'
+        response = self.request_knox(
+            self.url, 'POST', data=self.post_data, token=self.token_contrib
+        )
         self.assertEqual(response.status_code, 201)
         obj = IrodsDataRequest.objects.first()
         self.assertEqual(IrodsDataRequest.objects.count(), 1)
         self.assertEqual(obj.path, self.obj_path + '/')
-        self.assertEqual(obj.description, 'bla')
+        self.assertEqual(obj.description, IRODS_REQUEST_DESC)
 
     def test_create_invalid_data(self):
         """Test creating a request with invalid data"""
         self.assertEqual(IrodsDataRequest.objects.count(), 0)
         self.assert_alert_count(CREATE_ALERT, self.user, 0)
         self.assert_alert_count(CREATE_ALERT, self.user_delegate, 0)
-        post_data = {'path': '/doesnt/exist', 'description': 'bla'}
-
-        with self.login(self.user_contrib):
-            response = self.client.post(self.url, post_data)
-
+        self.post_data['path'] = '/doesnt/exist'
+        response = self.request_knox(
+            self.url, 'POST', data=self.post_data, token=self.token_contrib
+        )
         self.assertEqual(response.status_code, 400)
         self.assertEqual(IrodsDataRequest.objects.count(), 0)
 
     def test_create_invalid_path_assay_collection(self):
         """Test creating a request with assay path (should fail)"""
         self.assertEqual(IrodsDataRequest.objects.count(), 0)
-        post_data = {'path': self.assay_path, 'description': 'bla'}
-
-        with self.login(self.user_contrib):
-            response = self.client.post(self.url, post_data)
-
+        self.post_data['path'] = self.assay_path
+        response = self.request_knox(
+            self.url, 'POST', data=self.post_data, token=self.token_contrib
+        )
         self.assertEqual(response.status_code, 400)
         self.assertEqual(IrodsDataRequest.objects.count(), 0)
 
     def test_create_multiple(self):
         """Test creating multiple requests"""
         path2 = os.path.join(self.assay_path, IRODS_FILE_NAME2)
-        path2_md5 = os.path.join(self.assay_path, IRODS_FILE_NAME2 + '.md5')
         self.irods.data_objects.create(path2)
-        self.irods.data_objects.create(path2_md5)
         self.assertEqual(IrodsDataRequest.objects.count(), 0)
         self.assert_alert_count(CREATE_ALERT, self.user, 0)
         self.assert_alert_count(CREATE_ALERT, self.user_delegate, 0)
 
-        with self.login(self.user_contrib):
-            response = self.client.post(self.url, self.post_data)
-            self.assertEqual(response.status_code, 201)
-            self.post_data['path'] = path2
-            response = self.client.post(self.url, self.post_data)
-            self.assertEqual(response.status_code, 201)
-
+        response = self.request_knox(
+            self.url, 'POST', data=self.post_data, token=self.token_contrib
+        )
+        self.assertEqual(response.status_code, 201)
+        self.post_data['path'] = path2
+        response = self.request_knox(
+            self.url, 'POST', data=self.post_data, token=self.token_contrib
+        )
+        self.assertEqual(response.status_code, 201)
         self.assertEqual(IrodsDataRequest.objects.count(), 2)
         self.assert_alert_count(CREATE_ALERT, self.user, 1)
         self.assert_alert_count(CREATE_ALERT, self.user_delegate, 1)
@@ -604,6 +613,7 @@ def test_accept_one_of_multiple(self):
         self.assertEqual(
             IrodsDataRequest.objects.filter(status='ACTIVE').count(), 1
         )
+        # TODO: Assert other iRODS object remains
 
     @override_settings(REDIS_URL=INVALID_REDIS_URL)
     def test_accept_lock_failure(self):
@@ -641,6 +651,7 @@ def test_accept_lock_failure(self):
 
     def test_accept_already_accepted(self):
         """Test accepting an already accepted request (should fail)"""
+        # TODO: Refactor this (no multiple post requests needed)
         with self.login(self.user_contrib):
             self.client.post(
                 reverse(
@@ -675,12 +686,11 @@ def test_accept_already_accepted(self):
         self.assertEqual(response.status_code, 400)
 
 
-# TODO: Move to test_views_api
 class TestIrodsDataRequestRejectAPIView(TestIrodsDataRequestAPIViewBase):
     """Tests for IrodsDataRequestRejectAPIView"""
 
-    def test_reject_admin(self):
-        """Test rejecting a request as admin"""
+    def test_reject_superuser(self):
+        """Test rejecting a request as superuser"""
         self.assertEqual(IrodsDataRequest.objects.count(), 0)
         self.assert_alert_count(REJECT_ALERT, self.user, 0)
         self.assert_alert_count(REJECT_ALERT, self.user_delegate, 0)
@@ -710,6 +720,7 @@ def test_reject_admin(self):
         self.assertEqual(response.status_code, 200)
         obj.refresh_from_db()
         self.assertEqual(obj.status, 'REJECTED')
+        self.assert_irods_obj(self.obj_path, True)
         self.assert_alert_count(REJECT_ALERT, self.user, 0)
         self.assert_alert_count(REJECT_ALERT, self.user_delegate, 0)
         self.assert_alert_count(REJECT_ALERT, self.user_contrib, 1)

samplesheets/views_api.py

@@ -45,7 +45,6 @@
     IrodsDataRequest,
     IRODS_REQUEST_STATUS_ACTIVE,
     IRODS_REQUEST_STATUS_FAILED,
-    IRODS_REQUEST_STATUS_REJECTED,
 )
 from samplesheets.rendering import SampleSheetTableBuilder
 from samplesheets.serializers import (
@@ -371,9 +370,6 @@ def get(self, request, *args, **kwargs):
             sodar_uuid=self.kwargs.get('irodsdatarequest')
         ).first()
 
-        irods_request.status = IRODS_REQUEST_STATUS_REJECTED
-        irods_request.save()
-
         try:
             self.reject_request(
                 irods_request,