refactor IrodsDataRequestListAPIView, add tests (#1706)

samplesheets/serializers.py

@@ -12,6 +12,7 @@
 from projectroles.serializers import (
     SODARProjectModelSerializer,
     SODARNestedListSerializer,
+    SODARUserSerializer,
 )
 
 from samplesheets.forms import ERROR_MSG_EXISTING, ERROR_MSG_INVALID_PATH
@@ -97,9 +98,25 @@ class Meta:
 class IrodsDataRequestSerializer(SODARProjectModelSerializer):
     """Serializer for the IrodsDataRequest model"""
 
+    user = SODARUserSerializer()
+
     class Meta:
         model = IrodsDataRequest
-        fields = ['path', 'description']
+        fields = [
+            'project',
+            'action',
+            'path',
+            'target_path',
+            'user',
+            'status',
+            'status_info',
+            'description',
+            'date_created',
+            'sodar_uuid',
+        ]
+        read_only_fields = [
+            f for f in fields if f not in ['path', 'description']
+        ]
 
     def validate_path(self, value):
         irods_backend = get_backend_api('omics_irods')

samplesheets/tests/test_permissions_api.py

@@ -35,8 +35,6 @@
 
 class TestInvestigationRetrieveAPIView(
     SampleSheetIOMixin,
-    RemoteSiteMixin,
-    RemoteProjectMixin,
     TestProjectAPIPermissionBase,
 ):
     """Tests for InvestigationRetrieveAPIView permissions"""
@@ -117,12 +115,7 @@ def test_get_archive(self):
         self.assert_response_api(url, self.anonymous, 401)
 
 
-class TestSheetImportAPIView(
-    SampleSheetIOMixin,
-    RemoteSiteMixin,
-    RemoteProjectMixin,
-    TestProjectAPIPermissionBase,
-):
+class TestSheetImportAPIView(SampleSheetIOMixin, TestProjectAPIPermissionBase):
     """Tests for SheetImportAPIView permissions"""
 
     def _cleanup_import(self):
@@ -293,8 +286,6 @@ def test_post_archive(self):
 
 class TestSheetISAExportAPIView(
     SampleSheetIOMixin,
-    RemoteSiteMixin,
-    RemoteProjectMixin,
     TestProjectAPIPermissionBase,
 ):
     """Tests for SheetISAExportAPIView permissions"""
@@ -367,6 +358,70 @@ def test_get_archive(self):
         self.assert_response_api(url, self.anonymous, 401)
 
 
+class TestIrodsDataRequestListAPIView(TestProjectAPIPermissionBase):
+    """Tests for TestIrodsDataRequestListAPIView permissions"""
+
+    def setUp(self):
+        super().setUp()
+        self.url = reverse(
+            'samplesheets:api_irods_request_list',
+            kwargs={'project': self.project.sodar_uuid},
+        )
+
+    def test_get(self):
+        """Test get() in IrodsDataRequestListAPIView"""
+        good_users = [
+            self.superuser,
+            self.user_owner_cat,
+            self.user_delegate_cat,
+            self.user_contributor_cat,
+            self.user_owner,
+            self.user_delegate,
+            self.user_contributor,
+        ]
+        bad_users = [
+            self.user_guest_cat,
+            self.user_finder_cat,
+            self.user_guest,
+            self.user_no_roles,
+        ]
+        self.assert_response_api(self.url, good_users, 200)
+        self.assert_response_api(self.url, bad_users, 403)
+        self.assert_response_api(self.url, self.anonymous, 401)
+        # Test public project
+        self.project.set_public()
+        self.assert_response_api(
+            self.url, [self.user_finder_cat, self.user_no_roles], 403
+        )
+        self.assert_response_api(self.url, self.anonymous, 401)
+
+    @override_settings(PROJECTROLES_ALLOW_ANONYMOUS=True)
+    def test_get_anon(self):
+        """Test get() with anonymous guest access"""
+        self.project.set_public()
+        self.assert_response_api(self.url, self.anonymous, 401)
+
+    def test_get_archive(self):
+        """Test get() with archived project"""
+        self.project.set_archive()
+        good_users = [self.superuser]
+        bad_users = [
+            self.user_owner_cat,
+            self.user_delegate_cat,
+            self.user_contributor_cat,
+            self.user_guest_cat,
+            self.user_finder_cat,
+            self.user_owner,
+            self.user_delegate,
+            self.user_contributor,
+            self.user_guest,
+            self.user_no_roles,
+        ]
+        self.assert_response_api(self.url, good_users, 200)
+        self.assert_response_api(self.url, bad_users, 403)
+        self.assert_response_api(self.url, self.anonymous, 401)
+
+
 class TestIrodsDataRequestDestroyAPIView(
     SampleSheetIOMixin, IrodsDataRequestMixin, TestProjectAPIPermissionBase
 ):

samplesheets/tests/test_views_api.py

@@ -31,7 +31,9 @@
     ISATab,
     IrodsDataRequest,
     IRODS_REQUEST_ACTION_DELETE,
+    IRODS_REQUEST_STATUS_ACCEPTED,
     IRODS_REQUEST_STATUS_ACTIVE,
+    IRODS_REQUEST_STATUS_FAILED,
 )
 from samplesheets.rendering import (
     SampleSheetTableBuilder,
@@ -623,6 +625,95 @@ def test_get_json(self):
         self.assertEqual(response.data, expected)
 
 
+class TestIrodsDataRequestListAPIView(
+    IrodsDataRequestMixin, TestSampleSheetAPIBase
+):
+    """Tests for IrodsDataRequestListAPIView"""
+
+    def setUp(self):
+        super().setUp()
+        # Add contributor user
+        self.user_contrib = self.make_user('user_contributor')
+        self.make_assignment(
+            self.project, self.user_contrib, self.role_contributor
+        )
+        # 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 backend and paths
+        self.irods_backend = get_backend_api('omics_irods')
+        self.assay_path = self.irods_backend.get_path(self.assay)
+        # Make request
+        self.request = self.make_irods_request(
+            project=self.project,
+            action=IRODS_REQUEST_ACTION_DELETE,
+            path=os.path.join(self.assay_path, IRODS_FILE_NAME),
+            status=IRODS_REQUEST_STATUS_ACTIVE,
+            user=self.user_contrib,
+        )
+        self.url = reverse(
+            'samplesheets:api_irods_request_list',
+            kwargs={'project': self.project.sodar_uuid},
+        )
+
+    def test_get(self):
+        """Test retrieving iRODS data request list"""
+        response = self.request_knox(self.url)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(len(response.data), 1)
+        response_data = json.loads(response.content)
+        expected = {
+            'project': str(self.project.sodar_uuid),
+            'action': IRODS_REQUEST_ACTION_DELETE,
+            'path': self.request.path,
+            'target_path': '',
+            'user': self.get_serialized_user(self.user_contrib),
+            'status': IRODS_REQUEST_STATUS_ACTIVE,
+            'status_info': '',
+            'description': self.request.description,
+            'date_created': self.get_drf_datetime(self.request.date_created),
+            'sodar_uuid': str(self.request.sodar_uuid),
+        }
+        self.assertEqual(response_data[0], expected)
+
+    def test_get_failed_as_superuser(self):
+        """Test retrieving list as superuser with failed request"""
+        self.request.status = IRODS_REQUEST_STATUS_FAILED
+        self.request.save()
+        response = self.request_knox(self.url)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(len(response.data), 1)
+
+    def test_get_accepted_as_superuser(self):
+        """Test retrieving list as superuser with accepted request"""
+        self.request.status = IRODS_REQUEST_STATUS_ACCEPTED
+        self.request.save()
+        response = self.request_knox(self.url)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(len(response.data), 0)
+
+    def test_get_accepted_as_owner(self):
+        """Test retrieving list as owner with accepted request"""
+        self.request.status = IRODS_REQUEST_STATUS_ACCEPTED
+        self.request.save()
+        response = self.request_knox(
+            self.url, token=self.get_token(self.user_owner)
+        )
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(len(response.data), 0)
+
+    def test_get_accepted_as_request_creator(self):
+        """Test retrieving list as request creator with accepted request"""
+        self.request.status = IRODS_REQUEST_STATUS_ACCEPTED
+        self.request.save()
+        response = self.request_knox(
+            self.url, token=self.get_token(self.user_contrib)
+        )
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(len(response.data), 1)
+
+
 class TestIrodsDataRequestDestroyAPIView(
     IrodsDataRequestMixin, TestSampleSheetAPIBase
 ):

samplesheets/views_api.py

@@ -18,10 +18,11 @@
     PermissionDenied,
 )
 from rest_framework.generics import (
-    RetrieveAPIView,
     CreateAPIView,
-    UpdateAPIView,
     DestroyAPIView,
+    ListAPIView,
+    RetrieveAPIView,
+    UpdateAPIView,
 )
 from rest_framework.permissions import AllowAny, IsAuthenticated
 from rest_framework.response import Response
@@ -155,49 +156,34 @@ def post(self, request, *args, **kwargs):
         )
 
 
-# TODO: Refactor
-class IrodsDataRequestListAPIView(
-    IrodsDataRequestModifyMixin, SODARAPIBaseProjectMixin, APIView
-):
+class IrodsDataRequestListAPIView(SODARAPIBaseProjectMixin, ListAPIView):
     """
     List iRODS data requests for a project.
 
     **URL:** ``/samplesheets/api/irods/requests/{Project.sodar_uuid}``
 
     **Methods:** ``GET``
 
-    **Returns:**
-
-    - ``requests``: List of iRODS data requests (JSON)
+    **Returns:** List of iRODS data requests
     """
 
-    http_method_names = ['get']
     permission_required = 'samplesheets.edit_sheet'
+    serializer_class = IrodsDataRequestSerializer
 
-    def get(self, request, *args, **kwargs):
-        """GET request for listing iRODS data requests"""
+    def get_queryset(self):
         project = self.get_project()
-        irods_requests = IrodsDataRequest.objects.filter(project=project)
-        content = {'detail': 'No iRODS data requests found'}
-
-        # For superusers, owners and delegates,
-        # display active/failed requests from all users
+        requests = IrodsDataRequest.objects.filter(project=project)
+        # For superusers, owners and delegates, display requests from all users
         if self.request.user.is_superuser or project.is_owner_or_delegate(
             self.request.user
         ):
-            requests = irods_requests.filter(
+            return requests.filter(
                 status__in=[
                     IRODS_REQUEST_STATUS_ACTIVE,
                     IRODS_REQUEST_STATUS_FAILED,
                 ]
             )
-        else:
-            # For regular users, dispaly their own requests regardless of status
-            requests = irods_requests.filter(user=self.request.user)
-        if requests:
-            content['requests'] = requests
-            content['detail'] = 'iRODS data requests listed'
-        return Response(content, status=status.HTTP_200_OK)
+        return requests.filter(user=self.request.user)
 
 
 # TODO: Refactor