From 4af6d6185d81930bae50547ed08c42526cdf9165 Mon Sep 17 00:00:00 2001 From: Mikko Nieminen Date: Tue, 25 Jul 2023 18:08:07 +0200 Subject: [PATCH] refactor api views (wip) (#1706, #1734) --- CHANGELOG.rst | 2 +- .../tests/test_permissions_api_taskflow.py | 6 +- .../tests/test_views_ajax_taskflow.py | 10 +- samplesheets/tests/test_views_api.py | 64 ++++- samplesheets/tests/test_views_api_taskflow.py | 223 ++++++++---------- samplesheets/tests/test_views_taskflow.py | 37 ++- samplesheets/urls.py | 2 +- samplesheets/views.py | 8 +- samplesheets/views_api.py | 98 +++----- 9 files changed, 217 insertions(+), 233 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index fbd0aba6..545cdbe8 100644 --- a/CHANGELOG.rst +++ b/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) + - ``IrodsDataRequest`` REST API views (#1588, #1706, #1734) - Davrods links in iRODS delete request list (#1339) - Batch accepting and rejecting for iRODS delete requests (#1340) - **Taskflowbackend** diff --git a/samplesheets/tests/test_permissions_api_taskflow.py b/samplesheets/tests/test_permissions_api_taskflow.py index 742d3e9d..2ed17c05 100644 --- a/samplesheets/tests/test_permissions_api_taskflow.py +++ b/samplesheets/tests/test_permissions_api_taskflow.py @@ -19,7 +19,7 @@ ) from samplesheets.tests.test_views_taskflow import ( SampleSheetTaskflowMixin, - TEST_FILE_NAME, + IRODS_FILE_NAME, ) @@ -111,8 +111,8 @@ def setUp(self): # Set up iRODS data self.make_irods_colls(self.investigation) self.assay_path = self.irods_backend.get_path(self.assay) - self.path = os.path.join(self.assay_path, TEST_FILE_NAME) - self.path_md5 = os.path.join(self.assay_path, f'{TEST_FILE_NAME}.md5') + self.path = os.path.join(self.assay_path, IRODS_FILE_NAME) + self.path_md5 = os.path.join(self.assay_path, f'{IRODS_FILE_NAME}.md5') # Create objects self.file_obj = self.irods.data_objects.create(self.path) self.md5_obj = self.irods.data_objects.create(self.path_md5) diff --git a/samplesheets/tests/test_views_ajax_taskflow.py b/samplesheets/tests/test_views_ajax_taskflow.py index a982c5ab..6d9b9e00 100644 --- a/samplesheets/tests/test_views_ajax_taskflow.py +++ b/samplesheets/tests/test_views_ajax_taskflow.py @@ -18,7 +18,7 @@ from samplesheets.tests.test_views_taskflow import ( TestIrodsDataRequestViewsBase, SampleSheetTaskflowMixin, - TEST_FILE_NAME2, + IRODS_FILE_NAME2, ) from samplesheets.views import IRODS_REQ_CREATE_ALERT as CREATE_ALERT from samplesheets.views_ajax import ALERT_LIB_FILES_EXIST @@ -330,8 +330,8 @@ def test_create_exists_as_contributor_by_contributor2(self): def test_create_multiple(self): """Test creating multiple delete requests""" - path2 = os.path.join(self.assay_path, TEST_FILE_NAME2) - path2_md5 = os.path.join(self.assay_path, TEST_FILE_NAME2 + '.md5') + 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) @@ -473,8 +473,8 @@ def test_delete_no_request(self): def test_delete_one_of_multiple(self): """Test deleting one of multiple requests""" - path2 = os.path.join(self.assay_path, TEST_FILE_NAME2) - path2_md5 = os.path.join(self.assay_path, TEST_FILE_NAME2 + '.md5') + 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) diff --git a/samplesheets/tests/test_views_api.py b/samplesheets/tests/test_views_api.py index a80be40c..66b732ab 100644 --- a/samplesheets/tests/test_views_api.py +++ b/samplesheets/tests/test_views_api.py @@ -1,6 +1,7 @@ """Tests for REST API views in the samplesheets app""" import json +import os from django.test import override_settings from django.urls import reverse @@ -15,12 +16,23 @@ # Sodarcache dependency from sodarcache.models import JSONCacheItem +# Timeline dependency +from timeline.models import ProjectEvent + # Landingzones dependency from landingzones.models import LandingZone from landingzones.tests.test_models import LandingZoneMixin from samplesheets.io import SampleSheetIO -from samplesheets.models import Investigation, Assay, GenericMaterial, ISATab +from samplesheets.models import ( + Investigation, + Assay, + GenericMaterial, + ISATab, + IrodsDataRequest, + IRODS_REQUEST_ACTION_DELETE, + IRODS_REQUEST_STATUS_ACTIVE, +) from samplesheets.rendering import ( SampleSheetTableBuilder, STUDY_TABLE_CACHE_ITEM, @@ -31,6 +43,7 @@ SHEET_DIR, SHEET_DIR_SPECIAL, ) +from samplesheets.tests.test_models import IrodsDataRequestMixin from samplesheets.tests.test_sheet_config import SheetConfigMixin from samplesheets.tests.test_views import ( TestViewsBase, @@ -58,6 +71,7 @@ SHEET_NAME_ALT = 'i_small.zip' SHEET_PATH_ALT = SHEET_DIR + 'i_small2_alt.zip' SHEET_PATH_NO_PLUGIN_ASSAY = SHEET_DIR_SPECIAL + 'i_small_assay_no_plugin.zip' +IRODS_FILE_NAME = 'test1.txt' IRODS_FILE_MD5 = '0b26e313ed4a7ca6904b0e9369e5b957' @@ -609,6 +623,54 @@ def test_get_json(self): self.assertEqual(response.data, expected) +class TestIrodsDataRequestDestroyAPIView( + IrodsDataRequestMixin, TestSampleSheetAPIBase +): + """Tests for IrodsDataRequestDestroyAPIView""" + + def _assert_tl_count(self, count): + """Assert timeline ProjectEvent count""" + self.assertEqual( + ProjectEvent.objects.filter( + event_name='irods_request_delete' + ).count(), + count, + ) + + def setUp(self): + super().setUp() + # 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) + self.obj_path = os.path.join(self.assay_path, IRODS_FILE_NAME) + + def test_delete(self): + """Test deleting a request""" + self._assert_tl_count(0) + obj = self.make_irods_request( + project=self.project, + action=IRODS_REQUEST_ACTION_DELETE, + path=self.obj_path, + status=IRODS_REQUEST_STATUS_ACTIVE, + user=self.user, + ) + self.assertEqual(IrodsDataRequest.objects.count(), 1) + with self.login(self.user): + response = self.client.delete( + reverse( + 'samplesheets:api_irods_request_delete', + kwargs={'irodsdatarequest': obj.sodar_uuid}, + ) + ) + self.assertEqual(response.status_code, 204) + self.assertEqual(IrodsDataRequest.objects.count(), 0) + self._assert_tl_count(1) + + class TestSampleDataFileExistsAPIView(TestSampleSheetAPIBase): """Tests for SampleDataFileExistsAPIView""" diff --git a/samplesheets/tests/test_views_api_taskflow.py b/samplesheets/tests/test_views_api_taskflow.py index db3398a8..621783b4 100644 --- a/samplesheets/tests/test_views_api_taskflow.py +++ b/samplesheets/tests/test_views_api_taskflow.py @@ -14,12 +14,19 @@ from projectroles.models import SODAR_CONSTANTS from projectroles.plugins import get_backend_api +# Timeline dependency +from timeline.models import ProjectEvent + # Taskflowbackend dependency from taskflowbackend.tests.base import ( TaskflowAPIViewTestBase, ) -from samplesheets.models import IrodsDataRequest +from samplesheets.models import ( + IrodsDataRequest, + IRODS_REQUEST_ACTION_DELETE, + IRODS_REQUEST_STATUS_ACTIVE, +) from samplesheets.views import ( IRODS_REQ_CREATE_ALERT as CREATE_ALERT, IRODS_REQ_ACCEPT_ALERT as ACCEPT_ALERT, @@ -28,9 +35,11 @@ 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_views_taskflow import ( SampleSheetTaskflowMixin, - TEST_FILE_NAME, + IRODS_FILE_NAME, + IRODS_FILE_NAME2, INVALID_REDIS_URL, ) @@ -43,7 +52,6 @@ SHEET_PATH_EDITED = SHEET_DIR + 'i_small2_edited.zip' SHEET_PATH_ALT = SHEET_DIR + 'i_small2_alt.zip' IRODS_FILE_PATH = os.path.dirname(__file__) + '/irods/test1.txt' -IRODS_FILE_NAME = 'test1.txt' IRODS_FILE_MD5 = '0b26e313ed4a7ca6904b0e9369e5b957' @@ -73,6 +81,7 @@ class TestIrodsDataRequestAPIViewBase( ): """Base samplesheets API view test class for iRODS delete requests""" + # TODO: Retrieve this from a common base/helper class instead of redef def assert_alert_count(self, alert_name, user, count, project=None): """ Assert expected app alert count. If project is not specified, default to @@ -113,11 +122,11 @@ def setUp(self): # Set up iRODS data self.make_irods_colls(self.investigation) self.assay_path = self.irods_backend.get_path(self.assay) - self.path = os.path.join(self.assay_path, TEST_FILE_NAME) - self.path_md5 = os.path.join(self.assay_path, f'{TEST_FILE_NAME}.md5') - # Create objects - self.file_obj = self.irods.data_objects.create(self.path) - self.md5_obj = self.irods.data_objects.create(self.path_md5) + 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') @@ -148,11 +157,7 @@ def setUp(self): kwargs={'project': self.project.sodar_uuid}, ) # Set default POST data - self.post_data = {'path': self.path, 'description': 'bla'} - - def tearDown(self): - self.irods.collections.get('/sodarZone/projects').remove(force=True) - super().tearDown() + self.post_data = {'path': self.obj_path, 'description': ''} class TestInvestigationRetrieveAPIView(TestSampleSheetAPITaskflowBase): @@ -255,8 +260,8 @@ def test_create(self): self.assertEqual(response.status_code, 200) obj = IrodsDataRequest.objects.first() self.assertEqual(IrodsDataRequest.objects.count(), 1) - self.assertEqual(obj.path, self.path) - self.assertEqual(obj.description, 'bla') + self.assertEqual(obj.path, self.obj_path) + self.assertEqual(obj.description, '') 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) @@ -265,7 +270,7 @@ def test_create(self): 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.path + '/', 'description': 'bla'} + post_data = {'path': self.obj_path + '/', 'description': ''} with self.login(self.user_contrib): response = self.client.post(self.url, post_data) @@ -273,15 +278,15 @@ def test_create_trailing_slash(self): self.assertEqual(response.status_code, 200) obj = IrodsDataRequest.objects.first() self.assertEqual(IrodsDataRequest.objects.count(), 1) - self.assertEqual(obj.path, self.path + '/') - self.assertEqual(obj.description, 'bla') + self.assertEqual(obj.path, self.obj_path + '/') + self.assertEqual(obj.description, '') 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'} + post_data = {'path': '/doesnt/exist', 'description': ''} with self.login(self.user_contrib): response = self.client.post(self.url, post_data) @@ -292,7 +297,7 @@ def test_create_invalid_data(self): 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'} + post_data = {'path': self.assay_path, 'description': ''} with self.login(self.user_contrib): response = self.client.post(self.url, post_data) @@ -302,8 +307,8 @@ def test_create_invalid_path_assay_collection(self): def test_create_multiple(self): """Test creating multiple requests""" - path2 = os.path.join(self.assay_path, TEST_FILE_NAME + '_2') - path2_md5 = os.path.join(self.assay_path, TEST_FILE_NAME + '_2.md5') + 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) @@ -322,127 +327,89 @@ def test_create_multiple(self): self.assert_alert_count(CREATE_ALERT, self.user_delegate, 1) -class TestIrodsDataRequestUpdateAPIView(TestIrodsDataRequestAPIViewBase): +class TestIrodsDataRequestUpdateAPIView( + IrodsDataRequestMixin, TestIrodsDataRequestAPIViewBase +): """Tests for IrodsDataRequestUpdateAPIView""" - def test_update(self): - """Test updating a request""" - update_data = {'path': self.path, 'description': 'Updated'} + def _assert_tl_count(self, count): + """Assert timeline ProjectEvent count""" + self.assertEqual( + ProjectEvent.objects.filter( + event_name='irods_request_update' + ).count(), + count, + ) + def setUp(self): + super().setUp() + self.request = self.make_irods_request( + project=self.project, + action=IRODS_REQUEST_ACTION_DELETE, + path=self.obj_path, + status=IRODS_REQUEST_STATUS_ACTIVE, + description='', + user=self.user_contrib, + ) + + def test_put(self): + """Test PUT to update request""" + self.assertEqual(IrodsDataRequest.objects.count(), 1) + self._assert_tl_count(0) + update_data = {'path': self.obj_path, 'description': 'Updated'} with self.login(self.user_contrib): - self.client.post( - reverse( - 'samplesheets:api_irods_request_create', - kwargs={'project': self.project.sodar_uuid}, - ), - self.post_data, - ) - self.assertEqual(IrodsDataRequest.objects.count(), 1) - obj = IrodsDataRequest.objects.first() response = self.client.put( reverse( 'samplesheets:api_irods_request_update', - kwargs={'irodsdatarequest': obj.sodar_uuid}, + kwargs={'irodsdatarequest': self.request.sodar_uuid}, ), update_data, ) - self.assertEqual(response.status_code, 200) - obj.refresh_from_db() - self.assertEqual(obj.description, 'Updated') - self.assertEqual(obj.path, self.path) + self.request.refresh_from_db() + self.assertEqual(self.request.description, 'Updated') + self.assertEqual(self.request.path, self.obj_path) + self.assertEqual(IrodsDataRequest.objects.count(), 1) + self._assert_tl_count(1) - def test_update_invalid_data(self): - """Test updating a request with invalid data""" + def test_put_invalid_data(self): + """Test PUT to update request with invalid data""" + self._assert_tl_count(0) update_data = {'path': '/doesnt/exist', 'description': 'Updated'} - with self.login(self.user_contrib): - self.client.post( - reverse( - 'samplesheets:api_irods_request_create', - kwargs={'project': self.project.sodar_uuid}, - ), - self.post_data, - ) - self.assertEqual(IrodsDataRequest.objects.count(), 1) - obj = IrodsDataRequest.objects.first() response = self.client.put( reverse( 'samplesheets:api_irods_request_update', - kwargs={'irodsdatarequest': obj.sodar_uuid}, + kwargs={'irodsdatarequest': self.request.sodar_uuid}, ), update_data, ) - self.assertEqual(response.status_code, 400) - obj.refresh_from_db() - self.assertEqual(obj.description, 'bla') - self.assertEqual(obj.path, self.path) - - -class TestIrodsDataRequestDeleteAPIView(TestIrodsDataRequestAPIViewBase): - """Tests for IrodsDataRequestDeleteAPIView""" - - def test_delete(self): - """Test deleting a request""" + self.request.refresh_from_db() + self.assertEqual(self.request.description, '') + self.assertEqual(self.request.path, self.obj_path) + self._assert_tl_count(0) + + def test_patch(self): + """Test PATCH to update request""" + self._assert_tl_count(0) + update_data = {'description': 'Updated'} with self.login(self.user_contrib): - self.client.post( + response = self.client.patch( reverse( - 'samplesheets:api_irods_request_create', - kwargs={'project': self.project.sodar_uuid}, + 'samplesheets:api_irods_request_update', + kwargs={'irodsdatarequest': self.request.sodar_uuid}, ), - self.post_data, - ) - - self.assertEqual(IrodsDataRequest.objects.count(), 1) - obj = IrodsDataRequest.objects.first() - - with self.login(self.user_contrib): - response = self.client.delete( - reverse( - 'samplesheets:api_irods_request_delete', - kwargs={'irodsdatarequest': obj.sodar_uuid}, - ) + update_data, ) - self.assertEqual(response.status_code, 200) - self.assertEqual(IrodsDataRequest.objects.count(), 0) + self.request.refresh_from_db() + self.assertEqual(self.request.description, 'Updated') + self.assertEqual(self.request.path, self.obj_path) + self._assert_tl_count(1) - def test_delete_one_of_multiple(self): - """Test deleting one of multiple requests""" - path2 = os.path.join(self.assay_path, TEST_FILE_NAME + '_2') - path2_md5 = os.path.join(self.assay_path, TEST_FILE_NAME + '_2.md5') - self.irods.data_objects.create(path2) - self.irods.data_objects.create(path2_md5) - self.assertEqual(IrodsDataRequest.objects.count(), 0) - - with self.login(self.user_contrib): - self.client.post( - reverse( - 'samplesheets:api_irods_request_create', - kwargs={'project': self.project.sodar_uuid}, - ), - self.post_data, - ) - self.post_data['path'] = path2 - self.client.post( - reverse( - 'samplesheets:api_irods_request_create', - kwargs={'project': self.project.sodar_uuid}, - ), - self.post_data, - ) - self.assertEqual(IrodsDataRequest.objects.count(), 2) - obj = IrodsDataRequest.objects.first() - response = self.client.delete( - reverse( - 'samplesheets:api_irods_request_delete', - kwargs={'irodsdatarequest': obj.sodar_uuid}, - ) - ) - self.assertEqual(response.status_code, 200) - self.assertEqual(IrodsDataRequest.objects.count(), 1) +# NOTE: For TestIrodsDataRequestDestroyAPIView, see test_views_api class TestIrodsDataRequestAcceptAPIView(TestIrodsDataRequestAPIViewBase): @@ -479,7 +446,7 @@ def test_accept(self): self.assert_alert_count(ACCEPT_ALERT, self.user, 0) self.assert_alert_count(ACCEPT_ALERT, self.user_delegate, 0) self.assert_alert_count(ACCEPT_ALERT, self.user_contrib, 1) - self.assert_irods_obj(self.path, False) + self.assert_irods_obj(self.obj_path, False) def test_accept_no_request(self): """Test accepting a non-existing request""" @@ -523,11 +490,11 @@ def test_accept_invalid_data(self): self.assert_alert_count(ACCEPT_ALERT, self.user, 0) self.assert_alert_count(ACCEPT_ALERT, self.user_delegate, 0) self.assert_alert_count(ACCEPT_ALERT, self.user_contrib, 1) - self.assert_irods_obj(self.path, False) + self.assert_irods_obj(self.obj_path, False) def test_accept_delegate(self): """Test accepting a request as delegate""" - self.assert_irods_obj(self.path) + self.assert_irods_obj(self.obj_path) with self.login(self.user_contrib): self.client.post( @@ -555,14 +522,14 @@ def test_accept_delegate(self): self.assertEqual(response.status_code, 200) obj.refresh_from_db() self.assertEqual(obj.status, 'ACCEPTED') - self.assert_irods_obj(self.path, False) + self.assert_irods_obj(self.obj_path, False) self.assert_alert_count(ACCEPT_ALERT, self.user, 0) self.assert_alert_count(ACCEPT_ALERT, self.user_delegate, 0) self.assert_alert_count(ACCEPT_ALERT, self.user_contrib, 1) def test_accept_contributor(self): """Test accepting a request as contributor""" - self.assert_irods_obj(self.path) + self.assert_irods_obj(self.obj_path) with self.login(self.user_contrib): self.client.post( @@ -590,15 +557,15 @@ def test_accept_contributor(self): self.assertEqual(response.status_code, 403) obj.refresh_from_db() self.assertEqual(obj.status, 'ACTIVE') - self.assert_irods_obj(self.path, True) + self.assert_irods_obj(self.obj_path, True) self.assert_alert_count(ACCEPT_ALERT, self.user, 0) self.assert_alert_count(ACCEPT_ALERT, self.user_delegate, 0) self.assert_alert_count(ACCEPT_ALERT, self.user_contrib, 0) def test_accept_one_of_multiple(self): """Test accepting one of multiple requests""" - path2 = os.path.join(self.assay_path, TEST_FILE_NAME + '_2') - path2_md5 = os.path.join(self.assay_path, TEST_FILE_NAME + '_2.md5') + 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) @@ -640,7 +607,7 @@ def test_accept_one_of_multiple(self): @override_settings(REDIS_URL=INVALID_REDIS_URL) def test_accept_lock_failure(self): """Test accepting a request with project lock failure""" - self.assert_irods_obj(self.path) + self.assert_irods_obj(self.obj_path) with self.login(self.user_contrib): self.client.post( @@ -665,11 +632,11 @@ def test_accept_lock_failure(self): self.assertEqual(response.status_code, 400) obj.refresh_from_db() self.assertEqual(obj.status, 'FAILED') - self.assert_irods_obj(self.path, True) + self.assert_irods_obj(self.obj_path, True) self.assert_alert_count(ACCEPT_ALERT, self.user, 0) self.assert_alert_count(ACCEPT_ALERT, self.user_delegate, 0) self.assert_alert_count(ACCEPT_ALERT, self.user_contrib, 0) - self.assert_irods_obj(self.path, True) + self.assert_irods_obj(self.obj_path, True) def test_accept_already_accepted(self): """Test accepting an already accepted request (should fail)""" @@ -811,8 +778,8 @@ def test_reject_contributor(self): def test_reject_one_of_multiple(self): """Test rejecting one of multipe requests""" - path2 = os.path.join(self.assay_path, TEST_FILE_NAME + '_2') - path2_md5 = os.path.join(self.assay_path, TEST_FILE_NAME + '_2.md5') + 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) diff --git a/samplesheets/tests/test_views_taskflow.py b/samplesheets/tests/test_views_taskflow.py index 85111401..354a6738 100644 --- a/samplesheets/tests/test_views_taskflow.py +++ b/samplesheets/tests/test_views_taskflow.py @@ -69,8 +69,8 @@ # Local constants APP_NAME = 'samplesheets' SHEET_PATH = SHEET_DIR + 'i_small.zip' -TEST_FILE_NAME = 'test1' -TEST_FILE_NAME2 = 'test2' +IRODS_FILE_NAME = 'test1.txt' +IRODS_FILE_NAME2 = 'test2.txt' DUMMY_UUID = 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa' PUBLIC_USER_NAME = 'user_no_roles' PUBLIC_USER_PASS = 'password' @@ -301,7 +301,7 @@ def _setup_files(self): self.assert_irods_coll(self.study) self.assert_irods_coll(self.assay) self.assay_path = self.irods_backend.get_path(self.assay) - self.file_path = self.assay_path + '/' + TEST_FILE_NAME + self.file_path = self.assay_path + '/' + IRODS_FILE_NAME self.irods.data_objects.create(self.file_path) self.assertEqual(self.irods.data_objects.exists(self.file_path), True) @@ -1057,6 +1057,7 @@ class TestIrodsDataRequestViewsBase( ): """Base test class for iRODS delete requests""" + # TODO: Retrieve this from a common base/helper class instead of redef def _assert_alert_count(self, alert_name, user, count, project=None): """ Assert expected app alert count. If project is not specified, default to @@ -1097,16 +1098,14 @@ def setUp(self): # 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, TEST_FILE_NAME) + self.obj_path = os.path.join(self.assay_path, IRODS_FILE_NAME) self.obj_path_md5 = os.path.join( - self.assay_path, f'{TEST_FILE_NAME}.md5' + self.assay_path, f'{IRODS_FILE_NAME}.md5' ) - # Second file - self.obj_path2 = os.path.join(self.assay_path, TEST_FILE_NAME2) + self.obj_path2 = os.path.join(self.assay_path, IRODS_FILE_NAME2) self.obj_path2_md5 = os.path.join( - self.assay_path, f'{TEST_FILE_NAME2}.md5' + self.assay_path, f'{IRODS_FILE_NAME2}.md5' ) - # Create objects self.file_obj = self.irods.data_objects.create(self.obj_path) self.md5_obj = self.irods.data_objects.create(self.obj_path_md5) self.file_obj2 = self.irods.data_objects.create(self.obj_path2) @@ -1256,8 +1255,8 @@ def test_create_invalid_path_assay_collection(self): def test_create_multiple(self): """Test creating multiple_requests""" - path2 = os.path.join(self.assay_path, TEST_FILE_NAME2) - path2_md5 = os.path.join(self.assay_path, TEST_FILE_NAME2 + '.md5') + 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) @@ -1429,8 +1428,8 @@ def test_delete_contributor(self): def test_delete_one_of_multiple(self): """Test deleting one of multiple requests""" - path2 = os.path.join(self.assay_path, TEST_FILE_NAME2) - path2_md5 = os.path.join(self.assay_path, TEST_FILE_NAME2 + '.md5') + 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) @@ -1730,8 +1729,8 @@ def test_accept_contributor(self): def test_accept_one_of_multiple(self): """Test accepting one of multiple requests""" - path2 = os.path.join(self.assay_path, TEST_FILE_NAME2) - path2_md5 = os.path.join(self.assay_path, TEST_FILE_NAME2 + '.md5') + 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) @@ -1803,7 +1802,7 @@ def test_accept_lock_failure(self): def test_accept_collection(self): """Test accepting a collection request with multiple objects inside""" coll_path = os.path.join(self.assay_path, 'request_coll') - obj_path = os.path.join(coll_path, TEST_FILE_NAME) + obj_path = os.path.join(coll_path, IRODS_FILE_NAME) self.irods.collections.create(coll_path) self.irods.data_objects.create(obj_path) self.assertEqual(self.irods.collections.exists(coll_path), True) @@ -2296,8 +2295,8 @@ def test_reject_contributor(self): def test_reject_one_of_multiple(self): """Test rejecting one of multiple requests""" - path2 = os.path.join(self.assay_path, TEST_FILE_NAME2) - path2_md5 = os.path.join(self.assay_path, TEST_FILE_NAME2 + '.md5') + 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) @@ -2601,7 +2600,7 @@ def setUp(self): self.sample_path = self.irods_backend.get_sample_path(self.project) # Create test file - self.file_path = self.sample_path + '/' + TEST_FILE_NAME + self.file_path = self.sample_path + '/' + IRODS_FILE_NAME self.irods.data_objects.create(self.file_path) def tearDown(self): diff --git a/samplesheets/urls.py b/samplesheets/urls.py index 54980cd9..c2f00299 100644 --- a/samplesheets/urls.py +++ b/samplesheets/urls.py @@ -195,7 +195,7 @@ ), path( route='api/irods/request/delete/', - view=samplesheets.views_api.IrodsDataRequestDeleteAPIView.as_view(), + view=samplesheets.views_api.IrodsDataRequestDestroyAPIView.as_view(), name='api_irods_request_delete', ), path( diff --git a/samplesheets/views.py b/samplesheets/views.py index f8be274a..9ac98aa0 100644 --- a/samplesheets/views.py +++ b/samplesheets/views.py @@ -837,6 +837,7 @@ def add_tl_create(cls, irods_request): :param irods_request: IrodsDataRequest object """ + # TODO: Unify timeline api init timeline = get_backend_api('timeline_backend') if not timeline: return @@ -855,13 +856,13 @@ def add_tl_create(cls, irods_request): ) @classmethod - def add_tl_update(cls, irods_request, timeline=None): + def add_tl_update(cls, irods_request): """ Create timeline event for iRODS data request update. :param irods_request: IrodsDataRequest object - :param timeline: TimelineAPI instance or None """ + timeline = get_backend_api('timeline_backend') if not timeline: return tl_event = timeline.add_event( @@ -2509,12 +2510,11 @@ class IrodsDataRequestUpdateView( slug_field = 'sodar_uuid' def form_valid(self, form): - timeline = get_backend_api('timeline_backend') obj = form.save(commit=False) obj.user = self.request.user obj.project = self.get_project() obj.save() - self.add_tl_update(obj, timeline=timeline) + self.add_tl_update(obj) messages.success( self.request, 'iRODS data request "{}" updated.'.format(obj.get_display_name()), diff --git a/samplesheets/views_api.py b/samplesheets/views_api.py index 94b71931..7d5ec2b4 100644 --- a/samplesheets/views_api.py +++ b/samplesheets/views_api.py @@ -21,6 +21,7 @@ RetrieveAPIView, CreateAPIView, UpdateAPIView, + DestroyAPIView, ) from rest_framework.permissions import AllowAny, IsAuthenticated from rest_framework.response import Response @@ -198,6 +199,7 @@ def get(self, request, *args, **kwargs): return Response(content, status=status.HTTP_200_OK) +# TODO: Refactor class IrodsDataRequestCreateAPIView( IrodsDataRequestModifyMixin, SODARAPIBaseProjectMixin, CreateAPIView ): @@ -243,7 +245,7 @@ def post(self, request, *args, **kwargs): class IrodsDataRequestUpdateAPIView( - IrodsDataRequestModifyMixin, SODARAPIBaseProjectMixin, UpdateAPIView + IrodsDataRequestModifyMixin, SODARAPIGenericProjectMixin, UpdateAPIView ): """ Update an iRODS data request for a project. @@ -253,92 +255,46 @@ class IrodsDataRequestUpdateAPIView( **Methods:** ``PUT``, ``PATCH`` """ - http_method_names = ['put', 'patch'] - permission_classes = (IsAuthenticated,) - - def get_object(self): - """Override to allow POST requests""" - return IrodsDataRequest.objects.get( - sodar_uuid=self.kwargs.get('irodsdatarequest') - ) + # http_method_names = ['put', 'patch'] + lookup_url_kwarg = 'irodsdatarequest' + permission_classes = [IsAuthenticated] + serializer_class = IrodsDataRequestSerializer - def put(self, request, *args, **kwargs): - """PUT request for updating an iRODS data request""" - obj = self.get_object() - if not self.has_irods_request_perms(request, obj): + def perform_update(self, serializer): + """Override perform_update() to update IrodsDataRequest""" + if not self.has_irods_request_perms(self.request, serializer.instance): raise PermissionDenied('Insufficient permissions') - - irods_request = IrodsDataRequest.objects.filter( - sodar_uuid=self.kwargs.get('irodsdatarequest') - ).first() - serializer = IrodsDataRequestSerializer( - irods_request, data=request.data, partial=True - ) - if serializer.is_valid(): - serializer.save() - else: - raise ValidationError('Invalid data: {}'.format(serializer.errors)) - - irods_request = IrodsDataRequest.objects.filter( - sodar_uuid=serializer.data.get('sodar_uuid') - ).first() - # Create timeline event - self.add_tl_update(irods_request) - - return Response( - { - 'detail': 'iRODS data request updated', - 'request': irods_request.sodar_uuid, - }, - status=status.HTTP_200_OK, - ) + serializer.save() + # Add timeline event + self.add_tl_update(serializer.instance) -class IrodsDataRequestDeleteAPIView( - IrodsDataRequestModifyMixin, SODARAPIBaseProjectMixin, APIView +class IrodsDataRequestDestroyAPIView( + IrodsDataRequestModifyMixin, SODARAPIGenericProjectMixin, DestroyAPIView ): """ - Delete an iRODS data request for a project. + Delete an iRODS data request object. **URL:** ``/samplesheets/api/irods/request/delete/{IrodsDataRequest.sodar_uuid}`` **Methods:** ``DELETE`` """ - http_method_names = ['delete'] - # permission_required = 'samplesheets.edit_sheet' - permission_classes = (IsAuthenticated,) - - def get_object(self): - """Override to allow POST requests""" - return IrodsDataRequest.objects.get( - sodar_uuid=self.kwargs.get('irodsdatarequest') - ) + lookup_url_kwarg = 'irodsdatarequest' + permission_classes = [IsAuthenticated] + serializer_class = IrodsDataRequestSerializer - def delete(self, request, *args, **kwargs): - """DELETE request for deleting an iRODS data request""" - obj = self.get_object() - if not self.has_irods_request_perms(request, obj): + def perform_destroy(self, instance): + """ + Override perform_destroy() to delete IrodsDataRequest + """ + if not self.has_irods_request_perms(self.request, instance): raise PermissionDenied('Insufficient permissions') - - irods_request = IrodsDataRequest.objects.filter( - sodar_uuid=self.kwargs.get('irodsdatarequest') - ).first() - try: - irods_request.delete() - except Exception as ex: - raise APIException( - '{} {}'.format('Deleting ' + IRODS_EX_MSG + ':', ex) - ) - + instance.delete() # Add timeline event - self.add_tl_delete(irods_request) + self.add_tl_delete(instance) # Handle project alerts - self.handle_alerts_deactivate(irods_request) - return Response( - {'detail': 'iRODS data request deleted'}, - status=status.HTTP_200_OK, - ) + self.handle_alerts_deactivate(instance) class IrodsDataRequestAcceptAPIView(