refactor IrodsDataRequest model and views, add validation (#1706)

samplesheets/forms.py

@@ -26,6 +26,8 @@
     ISATab,
     IrodsAccessTicket,
     IrodsDataRequest,
+    IRODS_REQUEST_STATUS_ACTIVE,
+    IRODS_REQUEST_STATUS_FAILED,
 )
 
 
@@ -154,7 +156,7 @@ def save(self, *args, **kwargs):
 
 
 class SheetTemplateCreateForm(forms.Form):
-    """Form for creating sample sheets from an ISA-Tab template."""
+    """Form for creating sample sheets from an ISA-Tab template"""
 
     @classmethod
     def _get_tsv_data(cls, path, file_names):
@@ -378,8 +380,8 @@ def clean(self):
         return self.cleaned_data
 
 
-class IrodsRequestForm(forms.ModelForm):
-    """Form for the iRODS delete request creation and editing"""
+class IrodsDataRequestForm(forms.ModelForm):
+    """Form for iRODS data request creation and editing"""
 
     class Meta:
         model = IrodsDataRequest
@@ -397,7 +399,11 @@ def clean(self):
         cleaned_data['path'] = cleaned_data['path'].rstrip('/')
 
         old_request = IrodsDataRequest.objects.filter(
-            path=cleaned_data['path'], status__in=['ACTIVE', 'FAILED']
+            path=cleaned_data['path'],
+            status__in=[
+                IRODS_REQUEST_STATUS_ACTIVE,
+                IRODS_REQUEST_STATUS_FAILED,
+            ],
         ).first()
         if old_request and old_request != self.instance:
             self.add_error('path', ERROR_MSG_EXISTING)
@@ -454,8 +460,8 @@ def clean(self):
         return cleaned_data
 
 
-class IrodsRequestAcceptForm(forms.Form):
-    """Form accepting an iRODS delete request."""
+class IrodsDataRequestAcceptForm(forms.Form):
+    """Form for accepting an iRODS data requests"""
 
     confirm = forms.BooleanField(required=True)
 
@@ -468,7 +474,7 @@ def __init__(self, *args, **kwargs):
 
 
 class SheetVersionEditForm(forms.ModelForm):
-    """Form for editing a saved ISA-Tab version of the sample sheets."""
+    """Form for editing a saved ISA-Tab version of sample sheets"""
 
     class Meta:
         model = ISATab

samplesheets/migrations/0021_update_irodsdatarequest.py

@@ -0,0 +1,28 @@
+# Generated by Django 3.2.19 on 2023-07-24 14:56
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('samplesheets', '0020_update_irodsaccessticket'),
+    ]
+
+    operations = [
+        migrations.AlterModelManagers(
+            name='irodsaccessticket',
+            managers=[
+            ],
+        ),
+        migrations.AlterField(
+            model_name='irodsdatarequest',
+            name='action',
+            field=models.CharField(default='DELETE', help_text='Action to be performed', max_length=64),
+        ),
+        migrations.AlterField(
+            model_name='irodsdatarequest',
+            name='status',
+            field=models.CharField(default='ACTIVE', help_text='Status of the request', max_length=16),
+        ),
+    ]

samplesheets/models.py

@@ -67,14 +67,10 @@
     'REPLACE': 'Replacing a previous ISA-Tab',
 }
 
-IRODS_DATA_REQUEST_STATUS_CHOICES = [
-    ('ACTIVE', 'active'),
-    ('ACCEPTED', 'accepted'),
-    ('FAILED', 'failed'),
-]
-IRODS_REQUEST_ACTION_DELETE = 'delete'
-IRODS_REQUEST_STATUS_ACTIVE = 'ACTIVE'
+IRODS_REQUEST_ACTION_DELETE = 'DELETE'
 IRODS_REQUEST_STATUS_ACCEPTED = 'ACCEPTED'
+IRODS_REQUEST_STATUS_ACTIVE = 'ACTIVE'
+IRODS_REQUEST_STATUS_FAILED = 'FAILED'
 IRODS_REQUEST_STATUS_REJECTED = 'REJECTED'
 
 # ISA-Tab SODAR metadata comment key for assay plugin override
@@ -1270,16 +1266,16 @@ class Meta:
         Project,
         null=False,
         related_name='irods_data_request',
-        help_text='Project to which the iRODS delete request belongs',
+        help_text='Project to which the iRODS data request belongs',
         on_delete=models.CASCADE,
     )
 
-    #: Action to be performed (default currently supported)
+    #: Action to be performed (only DELETE is currently supported)
     action = models.CharField(
         max_length=64,
         unique=False,
         blank=False,
-        default='delete',
+        default=IRODS_REQUEST_ACTION_DELETE,
         help_text='Action to be performed',
     )
 
@@ -1313,8 +1309,7 @@ class Meta:
     status = models.CharField(
         max_length=16,
         null=False,
-        choices=IRODS_DATA_REQUEST_STATUS_CHOICES,
-        default=IRODS_DATA_REQUEST_STATUS_CHOICES[0][0],
+        default=IRODS_REQUEST_STATUS_ACTIVE,
         help_text='Status of the request',
     )
 
@@ -1353,6 +1348,33 @@ def __repr__(self):
         )
         return 'IrodsDataRequest({})'.format(', '.join(repr(v) for v in values))
 
+    # Saving and validation
+
+    def save(self, *args, **kwargs):
+        """Custom validation and saving method"""
+        self._validate_action()
+        self._validate_status()
+        super().save(*args, **kwargs)
+
+    def _validate_action(self):
+        """Validate the action field"""
+        if self.action != IRODS_REQUEST_ACTION_DELETE:
+            raise ValidationError(
+                'This model currently only supports the action "{}"'.format(
+                    IRODS_REQUEST_ACTION_DELETE
+                )
+            )
+
+    def _validate_status(self):
+        """Validate the status field"""
+        if self.status not in [
+            IRODS_REQUEST_STATUS_ACCEPTED,
+            IRODS_REQUEST_STATUS_ACTIVE,
+            IRODS_REQUEST_STATUS_FAILED,
+            IRODS_REQUEST_STATUS_REJECTED,
+        ]:
+            raise ValidationError('Unknown status "{}"'.format(self.status))
+
     # Custom row-level functions
 
     def get_display_name(self):

samplesheets/tests/test_models.py

@@ -10,6 +10,7 @@
 from datetime import timedelta
 
 from django.conf import settings
+from django.core.exceptions import ValidationError
 from django.forms.models import model_to_dict
 from django.utils import timezone
 
@@ -1576,6 +1577,18 @@ def test__repr__(self):
         )
         self.assertEqual(repr(self.request), expected)
 
+    def test_validate_action(self):
+        """Test _validate_action()"""
+        with self.assertRaises(ValidationError):
+            self.request.action = 'MOVE'
+            self.request.save()
+
+    def test_validate_status(self):
+        """Test _validate_status()"""
+        with self.assertRaises(ValidationError):
+            self.request.status = 'NOT A VALID STATUS'
+            self.request.save()
+
     def test_get_display_name(self):
         """Test get_display_name()"""
         expected = '{} {}'.format(

samplesheets/tests/test_permissions_ajax.py

@@ -841,8 +841,8 @@ def test_display_config_archive(self):
         self.assert_response(url, self.user_guest, 400, method='POST')
         self.assert_response(url, self.anonymous, 403, method='POST')
 
-    # TODO: Test IrodsRequestCreateAjaxView (see sodar_core#823)
-    # TODO: Test IrodsRequestDeleteAjaxView (see sodar_core#823)
+    # TODO: Test IrodsDataRequestCreateAjaxView (see sodar_core#823)
+    # TODO: Test IrodsDataRequestDeleteAjaxView (see sodar_core#823)
 
     def test_version_compare(self):
         """Test SheetVersionCompareAjaxView permissions"""

samplesheets/tests/test_permissions_api_taskflow.py

@@ -83,10 +83,10 @@ def test_get_archive(self):
         self.assert_response_api(url, self.anonymous, 401, data=self.post_data)
 
 
-class TestIrodsRequestAPIViewBase(
+class TestIrodsDataRequestAPIViewBase(
     SampleSheetIOMixin, SampleSheetTaskflowMixin, TaskflowAPIPermissionTestBase
 ):
-    """Base test class for IrodsRequestAPIView permission tests"""
+    """Base class for iRODS data request API view permission tests"""
 
     def create_request(self):
         """Helper function to create a request"""
@@ -118,8 +118,8 @@ def setUp(self):
         self.md5_obj = self.irods.data_objects.create(self.path_md5)
 
 
-class TestIrodsRequestCreateAPIView(TestIrodsRequestAPIViewBase):
-    """Test permissions for IrodsRequestCreateAPIView"""
+class TestIrodsDataRequestCreateAPIView(TestIrodsDataRequestAPIViewBase):
+    """Test permissions for IrodsDataRequestCreateAPIView"""
 
     def setUp(self):
         super().setUp()
@@ -132,7 +132,7 @@ def setUp(self):
         self.post_data = {'path': self.path + '/', 'description': 'bla'}
 
     def test_create(self):
-        """Test post() in IrodsRequestCreateAPIView"""
+        """Test post() in IrodsDataRequestCreateAPIView"""
         good_users = [
             self.superuser,
             self.user_owner_cat,
@@ -161,7 +161,7 @@ def test_create(self):
 
     @override_settings(PROJECTROLES_ALLOW_ANONYMOUS=True)
     def test_create_anon(self):
-        """Test post() in IrodsRequestCreateAPIView with anonymous access"""
+        """Test post() in IrodsDataRequestCreateAPIView with anonymous access"""
         self.project.set_public()
         self.assert_response_api(
             self.url,
@@ -172,7 +172,7 @@ def test_create_anon(self):
         )
 
     def test_create_archived(self):
-        """Test post() in IrodsRequestCreateAPIView with archived project"""
+        """Test post() in IrodsDataRequestCreateAPIView with archived project"""
         self.project.set_archive()
         good_users = [self.superuser]
         bad_users = [
@@ -199,8 +199,8 @@ def test_create_archived(self):
         )
 
 
-class TestIrodsRequestUpdateAPIView(TestIrodsRequestAPIViewBase):
-    """Test permissions for IrodsRequestUpdateAPIView"""
+class TestIrodsDataRequestUpdateAPIView(TestIrodsDataRequestAPIViewBase):
+    """Test permissions for IrodsDataRequestUpdateAPIView"""
 
     def setUp(self):
         super().setUp()
@@ -212,7 +212,7 @@ def setUp(self):
         self.update_data = {'path': self.path, 'description': 'Updated'}
 
     def test_update(self):
-        """Test post() in IrodsRequestUpdateAPIView"""
+        """Test post() in IrodsDataRequestUpdateAPIView"""
         good_users = [
             self.superuser,
             self.user_owner_cat,
@@ -240,14 +240,14 @@ def test_update(self):
 
     @override_settings(PROJECTROLES_ALLOW_ANONYMOUS=True)
     def test_update_anon(self):
-        """Test post() in IrodsRequestUpdateAPIView with anonymous access"""
+        """Test post() in IrodsDataRequestUpdateAPIView with anonymous access"""
         self.project.set_public()
         self.assert_response_api(
             self.url, self.anonymous, 401, method='PUT', data=self.update_data
         )
 
     def test_update_archived(self):
-        """Test post() in IrodsRequestUpdateAPIView with archived project"""
+        """Test post() in IrodsDataRequestUpdateAPIView with archived project"""
         self.project.set_archive()
         good_users = [self.superuser]
         bad_users = [
@@ -273,11 +273,11 @@ def test_update_archived(self):
         )
 
 
-class TestIrodsRequestDeleteAPIView(TestIrodsRequestAPIViewBase):
-    """Test permissions for IrodsRequestDeleteAPIView"""
+class TestIrodsDataRequestDeleteAPIView(TestIrodsDataRequestAPIViewBase):
+    """Test permissions for IrodsDataRequestDeleteAPIView"""
 
     def test_delete(self):
-        """Test delete() in IrodsRequestDeleteAPIView"""
+        """Test delete() in IrodsDataRequestDeleteAPIView"""
         good_users = [
             self.superuser,
             self.user_owner_cat,
@@ -325,11 +325,11 @@ def test_delete(self):
             )
 
 
-class TestIrodsRequestAcceptAPIView(TestIrodsRequestAPIViewBase):
-    """Test permissions for TestIrodsRequestAcceptAPIView"""
+class TestIrodsDataRequestAcceptAPIView(TestIrodsDataRequestAPIViewBase):
+    """Test permissions for TestIrodsDataRequestAcceptAPIView"""
 
     def test_accept(self):
-        """Test post() in IrodsRequestAcceptAPIView"""
+        """Test post() in IrodsDataRequestAcceptAPIView"""
         good_users = [
             self.superuser,
             self.user_owner_cat,
@@ -384,7 +384,7 @@ def test_accept(self):
 
     @override_settings(PROJECTROLES_ALLOW_ANONYMOUS=True)
     def test_accept_anon(self):
-        """Test post() in IrodsRequestAcceptAPIView with anonymous access"""
+        """Test post() in IrodsDataRequestAcceptAPIView with anonymous access"""
         obj = self.create_request()
         self.url_accept = reverse(
             'samplesheets:api_irods_request_accept',
@@ -398,7 +398,7 @@ def test_accept_anon(self):
         )
 
     def test_accept_archived(self):
-        """Test post() in IrodsRequestUpdateAPIView with archived project"""
+        """Test post() in IrodsDataRequestUpdateAPIView with archived project"""
         self.project.set_archive()
         good_users = [self.superuser]
         bad_users = [
@@ -451,11 +451,11 @@ def test_accept_archived(self):
         )
 
 
-class TestIrodsRequestRejectAPIView(TestIrodsRequestAPIViewBase):
-    """Test permissions for TestIrodsRequestRejectAPIView"""
+class TestIrodsDataRequestRejectAPIView(TestIrodsDataRequestAPIViewBase):
+    """Test permissions for TestIrodsDataRequestRejectAPIView"""
 
     def test_reject(self):
-        """Test get() in IrodsRequestRejectAPIView"""
+        """Test get() in IrodsDataRequestRejectAPIView"""
         good_users = [
             self.superuser,
             self.user_owner_cat,
@@ -510,7 +510,7 @@ def test_reject(self):
 
     @override_settings(PROJECTROLES_ALLOW_ANONYMOUS=True)
     def test_accept_anon(self):
-        """Test get() in IrodsRequestRejectAPIView with anonymous access"""
+        """Test get() in IrodsDataRequestRejectAPIView with anonymous access"""
         obj = self.create_request()
         self.url_reject = reverse(
             'samplesheets:api_irods_request_reject',
@@ -524,7 +524,7 @@ def test_accept_anon(self):
         )
 
     def test_reject_archived(self):
-        """Test post() in IrodsRequestUpdateAPIView with archived project"""
+        """Test post() in IrodsDataRequestUpdateAPIView with archived project"""
         self.project.set_archive()
         good_users = [self.superuser]
         bad_users = [