-
Couldn't load subscription status.
- Fork 79
Admin actions #1593
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Admin actions #1593
Changes from 12 commits
d165db7
5028250
0747611
ca21dd2
7460dab
792c5d2
7d1af81
3e3131a
2119749
a5889a7
b7dcdda
8a34ffe
bca59b5
8e24a0a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,10 @@ | |
| # The full license is in the file LICENSE, distributed with this software. | ||
| # ----------------------------------------------------------------------------- | ||
| from qiita_db.artifact import Artifact | ||
| from qiita_db.user import User | ||
| from qiita_db.util import get_visibilities | ||
| from qiita_db.exceptions import QiitaDBArtifactDeletionError | ||
| from qiita_core.qiita_settings import qiita_config | ||
| from qiita_pet.handlers.api_proxy.util import check_access | ||
|
|
||
|
|
||
|
|
@@ -53,3 +57,128 @@ def artifact_graph_get_req(artifact_id, direction, user_id): | |
| 'node_labels': node_labels, | ||
| 'status': 'success', | ||
| 'message': ''} | ||
|
|
||
|
|
||
| def artifact_get_req(artifact_id, user_id): | ||
| """Get information about the artifact | ||
|
|
||
| Parameters | ||
| ---------- | ||
| artifact_id : int | ||
| Artifact being acted on | ||
| user_id : str | ||
| The user requesting the action | ||
|
|
||
| Returns | ||
| ------- | ||
| dict | ||
| information about the artifact | ||
| """ | ||
| pd = Artifact(int(artifact_id)) | ||
| access_error = check_access(pd.study.id, user_id) | ||
| if access_error: | ||
| return access_error | ||
|
|
||
| can_submit_to_ebi = pd.can_be_submitted_to_ebi | ||
| ebi_run_accessions = pd.ebi_run_accessions if can_submit_to_ebi else None | ||
|
|
||
| can_submit_to_vamps = pd.can_be_submitted_to_vamps | ||
| is_submitted_to_vamps = pd.is_submitted_to_vamps if can_submit_to_vamps \ | ||
| else False | ||
|
|
||
| return { | ||
| 'timestamp': pd.timestamp, | ||
| 'processing_parameters': pd.processing_parameters, | ||
| 'visibility': pd.visibility, | ||
| 'artifact_type': pd.artifact_type, | ||
| 'data_type': pd.data_type, | ||
| 'can_be_submitted_to_ebi': can_submit_to_ebi, | ||
| 'can_be_submitted_to_vamps': can_submit_to_vamps, | ||
| 'is_submitted_to_vamps': is_submitted_to_vamps, | ||
| 'filepaths': pd.filepaths, | ||
| 'parents': [a.id for a in pd.parents], | ||
| 'prep_templates': [p.id for p in pd.prep_templates], | ||
| 'ebi_run_accessions': ebi_run_accessions, | ||
| 'study': pd.study.id} | ||
|
|
||
|
|
||
| def artifact_delete_req(artifact_id, user_id): | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a test for this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add doc strings? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like there were more merge issues then I thought, since they were both there before. Let me dig on those. |
||
| """Deletes the artifact | ||
|
|
||
| Parameters | ||
| ---------- | ||
| artifact_id : int | ||
| Artifact being acted on | ||
| user_id : str | ||
| The user requesting the action | ||
|
|
||
| Returns | ||
| ------- | ||
| dict | ||
| Status of action, in the form {'status': status, 'message': msg} | ||
| status: status of the action, either success or error | ||
| message: Human readable message for status | ||
| """ | ||
| pd = Artifact(int(artifact_id)) | ||
| access_error = check_access(pd.study.id, user_id) | ||
| if access_error: | ||
| return access_error | ||
| try: | ||
| Artifact.delete(int(artifact_id)) | ||
| except QiitaDBArtifactDeletionError as e: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a test for the except? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
| return {'status': 'error', | ||
| 'message': str(e)} | ||
| return {'status': 'success', | ||
| 'message': ''} | ||
|
|
||
|
|
||
| def artifact_status_put_req(artifact_id, user_id, visibility): | ||
| """Set the status of the artifact given | ||
|
|
||
| Parameters | ||
| ---------- | ||
| artifact_id : int | ||
| Artifact being acted on | ||
| user_id : str | ||
| The user requesting the action | ||
| visibility : {'sandbox', 'awaiting_approval', 'private', 'public'} | ||
| What to change the visibility to | ||
|
|
||
| Returns | ||
| ------- | ||
| dict | ||
| Status of action, in the form {'status': status, 'message': msg} | ||
| status: status of the action, either success or error | ||
| message: Human readable message for status | ||
| """ | ||
| if visibility not in get_visibilities(): | ||
| return {'status': 'error', | ||
| 'message': 'Unknown visiblity value: %s' % visibility} | ||
|
|
||
| pd = Artifact(int(artifact_id)) | ||
| access_error = check_access(pd.study.id, user_id) | ||
| if access_error: | ||
| return access_error | ||
| user = User(str(user_id)) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. out of curiosity - why the specific cast to str in here? it looks unnecessary as you've already said before that everything is passed as a string. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was casting explicitly to int for the other objects so I just followed the same pattern and did an explicit cast here as well. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, the reasoning doesn't apply here but it is ok - no need to change. |
||
| # Set the approval to private if needs approval and admin | ||
| if visibility == 'private': | ||
| if all([qiita_config.require_approval, user.level == 'admin']): | ||
|
||
| pd.visibility = 'private' | ||
| status = 'success' | ||
| msg = 'Artifact visibility changed to private' | ||
| # Set the approval to private if approval not required | ||
| elif not qiita_config.require_approval: | ||
| pd.visibility = 'private' | ||
| status = 'success' | ||
| msg = 'Artifact visibility changed to private' | ||
| # Trying to set approval without admin privileges | ||
| else: | ||
| status = 'error' | ||
| msg = 'User does not have permissions to approve change' | ||
| else: | ||
| pd.visibility = visibility | ||
| status = 'success' | ||
|
||
| msg = 'Artifact visibility changed to %s' % visibility | ||
|
|
||
| return {'status': status, | ||
| 'message': msg} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,11 +6,103 @@ | |
| # The full license is in the file LICENSE, distributed with this software. | ||
| # ----------------------------------------------------------------------------- | ||
| from unittest import TestCase, main | ||
| from datetime import datetime | ||
| from os.path import join | ||
|
|
||
| from qiita_pet.handlers.api_proxy.artifact import artifact_graph_get_req | ||
| from qiita_core.util import qiita_test_checker | ||
| from qiita_core.qiita_settings import qiita_config | ||
| from qiita_db.artifact import Artifact | ||
| from qiita_db.exceptions import QiitaDBUnknownIDError | ||
| from qiita_pet.handlers.api_proxy.artifact import ( | ||
| artifact_get_req, artifact_status_put_req, artifact_graph_get_req, | ||
| artifact_delete_req) | ||
|
|
||
|
|
||
| @qiita_test_checker() | ||
| class TestArtifactAPI(TestCase): | ||
| def tearDown(self): | ||
| Artifact(1).visibility = 'private' | ||
|
|
||
| def test_artifact_get_req(self): | ||
| obs = artifact_get_req(1, 'test@foo.bar') | ||
| exp = {'is_submitted_to_vamps': False, | ||
| 'data_type': '18S', | ||
| 'can_be_submitted_to_vamps': False, | ||
| 'can_be_submitted_to_ebi': False, | ||
| 'timestamp': datetime(2012, 10, 1, 9, 30, 27), | ||
| 'prep_templates': [1], | ||
| 'visibility': 'private', | ||
| 'study': 1, | ||
| 'processing_parameters': None, | ||
| 'ebi_run_accessions': None, | ||
| 'parents': [], | ||
| 'filepaths': [ | ||
| (1, join(qiita_config.base_data_dir, | ||
| 'raw_data/1_s_G1_L001_sequences.fastq.gz'), | ||
| 'raw_forward_seqs'), | ||
| (2, join(qiita_config.base_data_dir, | ||
| 'raw_data/1_s_G1_L001_sequences_barcodes.' | ||
| 'fastq.gz'), | ||
| 'raw_barcodes')], | ||
| 'artifact_type': 'FASTQ'} | ||
| self.assertEqual(obs, exp) | ||
|
|
||
| def test_artifact_get_req_no_access(self): | ||
| obs = artifact_get_req(1, 'demo@microbio.me') | ||
| exp = {'status': 'error', | ||
| 'message': 'User does not have access to study'} | ||
| self.assertEqual(obs, exp) | ||
|
|
||
| def test_artifact_delete_req(self): | ||
| obs = artifact_delete_req(3, 'test@foo.bar') | ||
| exp = {'status': 'success', 'message': ''} | ||
| self.assertEqual(obs, exp) | ||
|
|
||
| with self.assertRaises(QiitaDBUnknownIDError): | ||
| Artifact(3) | ||
|
|
||
| def test_artifact_delete_req_error(self): | ||
| obs = artifact_delete_req(1, 'test@foo.bar') | ||
| exp = {'status': 'error', | ||
| 'message': 'Cannot delete artifact 1: it has children: 2, 3'} | ||
| self.assertEqual(obs, exp) | ||
|
|
||
| def test_artifact_delete_req_no_access(self): | ||
| obs = artifact_delete_req(3, 'demo@microbio.me') | ||
| exp = {'status': 'error', | ||
| 'message': 'User does not have access to study'} | ||
| self.assertEqual(obs, exp) | ||
|
|
||
| def test_artifact_status_put_req(self): | ||
| obs = artifact_status_put_req(1, 'test@foo.bar', 'sandbox') | ||
| exp = {'status': 'success', | ||
| 'message': 'Artifact visibility changed to sandbox'} | ||
| self.assertEqual(obs, exp) | ||
|
|
||
| def test_artifact_status_put_req_private(self): | ||
| obs = artifact_status_put_req(1, 'admin@foo.bar', 'private') | ||
| exp = {'status': 'success', | ||
| 'message': 'Artifact visibility changed to private'} | ||
| self.assertEqual(obs, exp) | ||
|
|
||
| def test_artifact_status_put_req_private_bad_permissions(self): | ||
| obs = artifact_status_put_req(1, 'test@foo.bar', 'private') | ||
| exp = {'status': 'error', | ||
| 'message': 'User does not have permissions to approve change'} | ||
| self.assertEqual(obs, exp) | ||
|
|
||
| def test_artifact_status_put_req_no_access(self): | ||
| obs = artifact_status_put_req(1, 'demo@microbio.me', 'sandbox') | ||
| exp = {'status': 'error', | ||
| 'message': 'User does not have access to study'} | ||
| self.assertEqual(obs, exp) | ||
|
|
||
| def test_artifact_status_put_req_unknown_status(self): | ||
| obs = artifact_status_put_req(1, 'test@foo.bar', 'BADSTAT') | ||
| exp = {'status': 'error', | ||
| 'message': 'Unknown visiblity value: BADSTAT'} | ||
| self.assertEqual(obs, exp) | ||
|
|
||
| def test_artifact_graph_get_req_ancestors(self): | ||
| obs = artifact_graph_get_req(1, 'ancestors', 'test@foo.bar') | ||
| exp = {'status': 'success', | ||
|
|
@@ -23,25 +115,24 @@ def test_artifact_graph_get_req_descendants(self): | |
| obs = artifact_graph_get_req(1, 'descendants', 'test@foo.bar') | ||
| exp = {'status': 'success', | ||
| 'message': '', | ||
| 'edge_list': [(1, 3), (1, 2), (2, 4)], | ||
| 'node_labels': [(1, 'Raw data 1 - FASTQ'), | ||
| (3, 'Demultiplexed 2 - Demultiplexed'), | ||
| (2, 'Demultiplexed 1 - Demultiplexed'), | ||
| (4, 'BIOM - BIOM')]} | ||
| self.assertEqual(obs, exp) | ||
|
|
||
| def test_artifact_graph_get_req_bad(self): | ||
| obs = artifact_graph_get_req(1, 'UNKNOWN', 'test@foo.bar') | ||
| exp = {'status': 'error', | ||
| 'message': 'Unknown directon UNKNOWN'} | ||
| (4, 'BIOM - BIOM')], | ||
| 'edge_list': [(1, 3), (1, 2), (2, 4)]} | ||
| self.assertEqual(obs, exp) | ||
|
|
||
| def test_artifact_graph_get_req_no_access(self): | ||
| obs = artifact_graph_get_req(1, 'descendants', 'demo@microbio.me') | ||
| obs = artifact_graph_get_req(1, 'ancestors', 'demo@microbio.me') | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why this change? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. probably something weird in the merge conflict resolution. |
||
| exp = {'status': 'error', | ||
| 'message': 'User does not have access to study'} | ||
| self.assertEqual(obs, exp) | ||
|
|
||
| def test_artifact_graph_get_req_bad_direction(self): | ||
| obs = artifact_graph_get_req(1, 'WRONG', 'test@foo.bar') | ||
| exp = {'status': 'error', 'message': 'Unknown directon WRONG'} | ||
| self.assertEqual(obs, exp) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| main() | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,6 @@ | |
| from os.path import join, exists | ||
|
|
||
| from qiita_core.util import qiita_test_checker | ||
| from qiita_core.qiita_settings import qiita_config | ||
| import qiita_db as qdb | ||
| from qiita_pet.handlers.api_proxy.sample_template import ( | ||
| sample_template_summary_get_req, sample_template_post_req, | ||
|
|
@@ -21,34 +20,34 @@ | |
|
|
||
| @qiita_test_checker() | ||
| class TestSampleAPI(TestCase): | ||
| info = { | ||
| "timeseries_type_id": 1, | ||
| "metadata_complete": True, | ||
| "mixs_compliant": True, | ||
| "number_samples_collected": 25, | ||
| "number_samples_promised": 28, | ||
| "study_alias": "FCM", | ||
| "study_description": "DESC", | ||
| "study_abstract": "ABS", | ||
| "emp_person_id": qdb.study.StudyPerson(2), | ||
| "principal_investigator_id": qdb.study.StudyPerson(3), | ||
| "lab_person_id": qdb.study.StudyPerson(1) | ||
| def setUp(self): | ||
| info = { | ||
| "timeseries_type_id": 1, | ||
| "metadata_complete": True, | ||
| "mixs_compliant": True, | ||
| "number_samples_collected": 25, | ||
| "number_samples_promised": 28, | ||
| "study_alias": "FCM", | ||
| "study_description": "DESC", | ||
| "study_abstract": "ABS", | ||
| "emp_person_id": qdb.study.StudyPerson(2), | ||
| "principal_investigator_id": qdb.study.StudyPerson(3), | ||
| "lab_person_id": qdb.study.StudyPerson(1) | ||
| } | ||
|
|
||
| new_study = qdb.study.Study.create( | ||
| qdb.user.User('test@foo.bar'), "Some New Study", [1], | ||
| info) | ||
| self.new_study = qdb.study.Study.create( | ||
| qdb.user.User('test@foo.bar'), "Some New Study", [1], | ||
| info) | ||
|
|
||
| def setUp(self): | ||
| fp = join(qiita_config.base_data_dir, 'uploads', | ||
| str(self.new_study.id), 'uploaded_file.txt') | ||
| base_dir = qdb.util.get_mountpoint('uploads')[0][1] | ||
| fp = join(base_dir, str(self.new_study.id), 'uploaded_file.txt') | ||
|
||
| if not exists(fp): | ||
| with open(fp, 'w') as f: | ||
| f.write('') | ||
|
|
||
| def tearDown(self): | ||
| fp = join(qiita_config.base_data_dir, 'uploads', '1', | ||
| 'uploaded_file.txt') | ||
| base_dir = qdb.util.get_mountpoint('uploads')[0][1] | ||
| fp = join(base_dir, '1', 'uploaded_file.txt') | ||
| if not exists(fp): | ||
| with open(fp, 'w') as f: | ||
| f.write('') | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing tests