Skip to content

Commit 937b13f

Browse files
josenavasantgonza
authored andcommitted
Analysis refactor gui part4 (#2079)
* fix #1505 * improving some GUI stuff * improving some GUI stuff - missing lines * addressing all comments * ready for review * fix #1987 * initial commit * requested changes * fix filter job list * Fixing server cert (#2051) * fix get_studies * flake8 * fix #503 * fix #2010 * fix #1913 * fix errors * addressing @josenavas comment * flake8 * fix #1010 * fix #1066 (#2058) * addressing @josenavas comments * fix #1961 * fix #1837 * Automatic jobs & new stats (#2057) * fix #814, fix #1636 * fixing error in test-env * fixing stats.html call * adding img * addressing @josenavas comments * rm for loops * addresssing @ElDeveloper comments * generalizing this functionality * fix #1816 * fix #1959 * addressing @josenavas comments * addressing @josenavas comments * fixing error * fixed? * addressing @josenavas comments * addressing @wasade comments * fix flake8 * generate biom and metadata release (#2066) * initial commit * adding portal * addressing @josenavas comments * pid -> qiita_artifact_id * addressing @josenavas comments * addressing @ElDeveloper comments * rm 50.sql * database changes to fix 969 * adding delete * addressing @josenavas comments * addressing @ElDeveloper comments * duh! * fix generate_biom_and_metadata_release (#2072) * fix generate_biom_and_metadata_release * addressing @ElDeveloper comment * Removing qiita ware code that will not be used anymore * Organizing the handlers and new analysis description page * Connecting the analysis creation and making interface responsive * Addressing @antgonza's comments * Initial artifact GUI refactor * Removing unused code * Adding can_edit call to the analysis * Fixing artifact rest API since not all artifacts have study * Adding can_be_publicized call to analysis * Adding QiitaHTTPError to handle errors gracefully * Adding safe_execution contextmanager * Fixing typo * Adding qiita test checker * Adapting some artifact handlers * Abstracting the graph reloading and adding some documentation * Fixing typo * Fixing changing artifact visibility * Fixing delete * Fixing artifact deletion
1 parent 015dd41 commit 937b13f

File tree

9 files changed

+274
-48
lines changed

9 files changed

+274
-48
lines changed

qiita_db/artifact.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,10 @@ def delete(cls, artifact_id):
571571
sql = "DELETE FROM qiita.study_artifact WHERE artifact_id = %s"
572572
qdb.sql_connection.TRN.add(sql, [artifact_id])
573573

574+
# Detach the artifact from the analysis_artifact table
575+
sql = "DELETE FROM qiita.analysis_artifact WHERE artifact_id = %s"
576+
qdb.sql_connection.TRN.add(sql, [artifact_id])
577+
574578
# Delete the row in the artifact table
575579
sql = "DELETE FROM qiita.artifact WHERE artifact_id = %s"
576580
qdb.sql_connection.TRN.add(sql, [artifact_id])

qiita_db/test/test_artifact.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -919,6 +919,21 @@ def test_delete(self):
919919
with self.assertRaises(qdb.exceptions.QiitaDBUnknownIDError):
920920
qdb.artifact.Artifact(test.id)
921921

922+
# Analysis artifact
923+
parameters = qdb.software.Parameters.from_default_params(
924+
qdb.software.DefaultParameters(1), {'input_data': 1})
925+
test = qdb.artifact.Artifact.create(
926+
self.filepaths_processed, "Demultiplexed",
927+
parents=[qdb.artifact.Artifact(9)],
928+
processing_parameters=parameters)
929+
930+
self._clean_up_files.extend(
931+
[join(uploads_fp, basename(fp)) for _, fp, _ in test.filepaths])
932+
qdb.artifact.Artifact.delete(test.id)
933+
934+
with self.assertRaises(qdb.exceptions.QiitaDBUnknownIDError):
935+
qdb.artifact.Artifact(test.id)
936+
922937
def test_delete_with_html(self):
923938
fd, html_fp = mkstemp(suffix=".html")
924939
close(fd)

qiita_pet/handlers/analysis_handlers/base_handlers.py

Lines changed: 41 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@
66
# The full license is in the file LICENSE, distributed with this software.
77
# -----------------------------------------------------------------------------
88

9+
from json import loads
10+
911
from tornado.web import authenticated
12+
from moi import r_client
1013

1114
from qiita_core.util import execute_as_transaction
1215
from qiita_core.qiita_settings import qiita_config
@@ -29,16 +32,49 @@ def post(self):
2932
% (qiita_config.portal_dir, analysis.id))
3033

3134

35+
def analysis_description_handler_get_request(analysis_id, user):
36+
"""Returns the analysis information
37+
38+
Parameters
39+
----------
40+
analysis_id : int
41+
The analysis id
42+
user : qiita_db.user.User
43+
The user performing the request
44+
"""
45+
analysis = Analysis(analysis_id)
46+
check_analysis_access(user, analysis)
47+
48+
job_info = r_client.get("analysis_%s" % analysis.id)
49+
alert_type = 'info'
50+
alert_msg = ''
51+
if job_info:
52+
job_info = loads(job_info)
53+
job_id = job_info['job_id']
54+
if job_id:
55+
redis_info = loads(r_client.get(job_id))
56+
if redis_info['status_msg'] == 'running':
57+
alert_msg = 'An artifact is being deleted from this analysis'
58+
elif redis_info['return'] is not None:
59+
alert_type = redis_info['return']['status']
60+
alert_msg = redis_info['return']['message'].replace(
61+
'\n', '</br>')
62+
63+
return {'analysis_name': analysis.name,
64+
'analysis_id': analysis.id,
65+
'analysis_description': analysis.description,
66+
'alert_type': alert_type,
67+
'alert_msg': alert_msg}
68+
69+
3270
class AnalysisDescriptionHandler(BaseHandler):
3371
@authenticated
3472
@execute_as_transaction
3573
def get(self, analysis_id):
36-
analysis = Analysis(analysis_id)
37-
check_analysis_access(self.current_user, analysis)
74+
res = analysis_description_handler_get_request(analysis_id,
75+
self.current_user)
3876

39-
self.render("analysis_description.html", analysis_name=analysis.name,
40-
analysis_id=analysis_id,
41-
analysis_description=analysis.description)
77+
self.render("analysis_description.html", **res)
4278

4379

4480
def analyisis_graph_handler_get_request(analysis_id, user):

qiita_pet/handlers/analysis_handlers/tests/test_base_handlers.py

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,56 @@
77
# -----------------------------------------------------------------------------
88

99
from unittest import TestCase, main
10-
from json import loads
10+
from json import loads, dumps
1111

1212
from tornado.web import HTTPError
13+
from moi import r_client
1314

1415
from qiita_core.util import qiita_test_checker
1516
from qiita_db.user import User
1617
from qiita_db.analysis import Analysis
1718
from qiita_pet.test.tornado_test_base import TestHandlerBase
1819
from qiita_pet.handlers.analysis_handlers.base_handlers import (
19-
analyisis_graph_handler_get_request)
20+
analyisis_graph_handler_get_request,
21+
analysis_description_handler_get_request)
2022

2123

2224
@qiita_test_checker()
2325
class TestBaseHandlersUtils(TestCase):
26+
def tearDown(self):
27+
r_client.flushdb()
28+
29+
def test_analysis_description_handler_get_request(self):
30+
obs = analysis_description_handler_get_request(1, User('test@foo.bar'))
31+
exp = {'analysis_name': 'SomeAnalysis',
32+
'analysis_id': 1,
33+
'analysis_description': 'A test analysis',
34+
'alert_type': 'info',
35+
'alert_msg': ''}
36+
self.assertEqual(obs, exp)
37+
38+
r_client.set('analysis_1', dumps({'job_id': 'job_id'}))
39+
r_client.set('job_id', dumps({'status_msg': 'running'}))
40+
obs = analysis_description_handler_get_request(1, User('test@foo.bar'))
41+
exp = {'analysis_name': 'SomeAnalysis',
42+
'analysis_id': 1,
43+
'analysis_description': 'A test analysis',
44+
'alert_type': 'info',
45+
'alert_msg': 'An artifact is being deleted from this analysis'}
46+
self.assertEqual(obs, exp)
47+
48+
r_client.set('job_id', dumps(
49+
{'status_msg': 'Success',
50+
'return': {'status': 'danger',
51+
'message': 'Error deleting artifact'}}))
52+
obs = analysis_description_handler_get_request(1, User('test@foo.bar'))
53+
exp = {'analysis_name': 'SomeAnalysis',
54+
'analysis_id': 1,
55+
'analysis_description': 'A test analysis',
56+
'alert_type': 'danger',
57+
'alert_msg': 'Error deleting artifact'}
58+
self.assertEqual(obs, exp)
59+
2460
def test_analyisis_graph_handler_get_request(self):
2561
obs = analyisis_graph_handler_get_request(1, User('test@foo.bar'))
2662
# The job id is randomly generated in the test environment. Gather

qiita_pet/handlers/artifact_handlers/base_handlers.py

Lines changed: 66 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,24 @@
77
# -----------------------------------------------------------------------------
88

99
from os.path import basename
10+
from json import dumps
1011

1112
from tornado.web import authenticated
13+
from moi import r_client
1214

1315
from qiita_core.qiita_settings import qiita_config
1416
from qiita_pet.handlers.base_handlers import BaseHandler
1517
from qiita_pet.handlers.util import safe_execution
1618
from qiita_pet.exceptions import QiitaHTTPError
19+
from qiita_ware.context import safe_submit
20+
from qiita_ware.dispatchable import delete_artifact
1721
from qiita_db.artifact import Artifact
1822
from qiita_db.software import Command, Parameters
1923
from qiita_db.processing_job import ProcessingJob
24+
from qiita_db.util import get_visibilities
25+
26+
27+
PREP_TEMPLATE_KEY_FORMAT = 'prep_template_%s'
2028

2129

2230
def check_artifact_access(user, artifact):
@@ -131,7 +139,7 @@ def artifact_summary_get_request(user, artifact_id):
131139
# If the artifact is part of an analysis, we don't require admin
132140
# approval, and the artifact can be made public only if all the
133141
# artifacts used to create the initial artifact set are public
134-
if analysis.can_be_publicized:
142+
if analysis.can_be_publicized and visibility != 'public':
135143
buttons.append(btn_base % ('make public', 'public', 'Make public'))
136144

137145
else:
@@ -145,13 +153,12 @@ def artifact_summary_get_request(user, artifact_id):
145153
buttons.append(
146154
btn_base % ('request approval for', 'awaiting_approval',
147155
'Request approval'))
148-
149-
elif user.level == 'admin' and visibility == 'awaiting_approval':
150-
# The approve artifact button only appears if the user is an admin
151-
# the artifact is waiting to be approvaed and the qiita config
152-
# requires artifact approval
153-
buttons.append(btn_base % ('approve', 'private',
154-
'Approve artifact'))
156+
elif user.level == 'admin' and visibility == 'awaiting_approval':
157+
# The approve artifact button only appears if the user is an
158+
# admin the artifact is waiting to be approvaed and the qiita
159+
# config requires artifact approval
160+
buttons.append(btn_base % ('approve', 'private',
161+
'Approve artifact'))
155162

156163
if visibility == 'private':
157164
# The make public button only appears if the artifact is private
@@ -206,8 +213,7 @@ def artifact_summary_get_request(user, artifact_id):
206213
'processing_jobs': processing_jobs,
207214
'summary': summary,
208215
'job': job_info,
209-
'errored_jobs': errored_jobs
210-
}
216+
'errored_jobs': errored_jobs}
211217

212218

213219
def artifact_summary_post_request(user, artifact_id):
@@ -311,6 +317,23 @@ def artifact_patch_request(user, artifact_id, req_op, req_path, req_value=None,
311317
if attribute == 'name':
312318
artifact.name = req_value
313319
return
320+
elif attribute == 'visibility':
321+
if req_value not in get_visibilities():
322+
raise QiitaHTTPError(400, 'Unknown visibility value: %s'
323+
% req_value)
324+
# Set the approval to private if needs approval and admin
325+
if req_value == 'private':
326+
if not qiita_config.require_approval:
327+
artifact.visibility = 'private'
328+
# Set the approval to private if approval not required
329+
elif user.level == 'admin':
330+
artifact.visibility = 'private'
331+
# Trying to set approval without admin privileges
332+
else:
333+
raise QiitaHTTPError(403, 'User does not have permissions '
334+
'to approve change')
335+
else:
336+
artifact.visibility = req_value
314337
else:
315338
# We don't understand the attribute so return an error
316339
raise QiitaHTTPError(404, 'Attribute "%s" not found. Please, '
@@ -320,7 +343,40 @@ def artifact_patch_request(user, artifact_id, req_op, req_path, req_value=None,
320343
'supported operations: replace' % req_op)
321344

322345

346+
def artifact_post_req(user, artifact_id):
347+
"""Deletes the artifact
348+
349+
Parameters
350+
----------
351+
user : qiita_db.user.User
352+
The user requesting the action
353+
artifact_id : int
354+
Id of the artifact being deleted
355+
"""
356+
artifact_id = int(artifact_id)
357+
artifact = Artifact(artifact_id)
358+
check_artifact_access(user, artifact)
359+
360+
analysis = artifact.analysis
361+
362+
if analysis:
363+
# Do something when deleting in the analysis part to keep track of it
364+
redis_key = "analysis_%s" % analysis.id
365+
else:
366+
pt_id = artifact.prep_templates[0].id
367+
redis_key = PREP_TEMPLATE_KEY_FORMAT % pt_id
368+
369+
job_id = safe_submit(user.id, delete_artifact, artifact_id)
370+
r_client.set(redis_key, dumps({'job_id': job_id, 'is_qiita_job': False}))
371+
372+
323373
class ArtifactAJAX(BaseHandler):
374+
@authenticated
375+
def post(self, artifact_id):
376+
with safe_execution():
377+
artifact_post_req(self.current_user, artifact_id)
378+
self.finish()
379+
324380
@authenticated
325381
def patch(self, artifact_id):
326382
"""Patches a prep template in the system

qiita_pet/handlers/artifact_handlers/tests/test_base_handlers.py

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -252,9 +252,10 @@ def test_artifact_summary_post_request(self):
252252

253253
def test_artifact_patch_request(self):
254254
a = Artifact(1)
255+
test_user = User('test@foo.bar')
255256
self.assertEqual(a.name, 'Raw data 1')
256257

257-
artifact_patch_request(User('test@foo.bar'), 1, 'replace', '/name/',
258+
artifact_patch_request(test_user, 1, 'replace', '/name/',
258259
req_value='NEW_NAME')
259260
self.assertEqual(a.name, 'NEW_NAME')
260261

@@ -268,24 +269,44 @@ def test_artifact_patch_request(self):
268269

269270
# Incorrect path parameter
270271
with self.assertRaises(QiitaHTTPError):
271-
artifact_patch_request(User('test@foo.bar'), 1, 'replace',
272+
artifact_patch_request(test_user, 1, 'replace',
272273
'/name/wrong/', req_value='NEW_NAME')
273274

274275
# Missing value
275276
with self.assertRaises(QiitaHTTPError):
276-
artifact_patch_request(User('test@foo.bar'), 1, 'replace',
277-
'/name/')
277+
artifact_patch_request(test_user, 1, 'replace', '/name/')
278278

279279
# Wrong attribute
280280
with self.assertRaises(QiitaHTTPError):
281-
artifact_patch_request(User('test@foo.bar'), 1, 'replace',
281+
artifact_patch_request(test_user, 1, 'replace',
282282
'/wrong/', req_value='NEW_NAME')
283283

284284
# Wrong operation
285285
with self.assertRaises(QiitaHTTPError):
286-
artifact_patch_request(User('test@foo.bar'), 1, 'add', '/name/',
286+
artifact_patch_request(test_user, 1, 'add', '/name/',
287287
req_value='NEW_NAME')
288288

289+
# Changing visibility
290+
self.assertEqual(a.visibility, 'private')
291+
artifact_patch_request(test_user, 1, 'replace', '/visibility/',
292+
req_value='sandbox')
293+
self.assertEqual(a.visibility, 'sandbox')
294+
295+
# Admin can change to private
296+
artifact_patch_request(User('admin@foo.bar'), 1, 'replace',
297+
'/visibility/', req_value='private')
298+
self.assertEqual(a.visibility, 'private')
299+
300+
# Test user can't change to private
301+
with self.assertRaises(QiitaHTTPError):
302+
artifact_patch_request(test_user, 1, 'replace', '/visibility/',
303+
req_value='private')
304+
305+
# Unkown req value
306+
with self.assertRaises(QiitaHTTPError):
307+
artifact_patch_request(test_user, 1, 'replace', '/visibility/',
308+
req_value='wrong')
309+
289310

290311
class TestBaseHandlers(TestHandlerBase):
291312
def test_get_artifact_summary_ajax_handler(self):

qiita_pet/templates/analysis_description.html

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,29 @@
22
{% block head %}
33
<script type="text/javascript">
44

5+
/**
6+
*
7+
* This function is needed by the artifact subsection of the page. Since the
8+
* artifact can be either embedded in the analysis pipeline or in the study
9+
* pipeline, we take advantage of the templating system to abstract the way
10+
* the graph should be updated. In the analysis pipeline (current) the graph
11+
* is reloaded by calling the update_graph function of the Vue component.
12+
*/
13+
function reload_graph() {
14+
$("#analysis-network-div").data('data-graph-vue').update_graph();
15+
}
16+
17+
/**
18+
*
19+
* This function is needed by the artifact subsection of the page. After the
20+
* artifact is sent for deletion, we need to reload the current page to
21+
* update the UI.
22+
*
23+
*/
24+
function reload_UI_post_artifact_delete() {
25+
location.reload();
26+
}
27+
528
/**
629
* Creates the HTML with the job information and adds it to `target`
730
*
@@ -170,6 +193,9 @@
170193

171194
// Add the vue object to the div, so we avoid to have global variables
172195
$("#analysis-network-div").data('data-graph-vue', vueGraph);
196+
{% if alert_msg %}
197+
bootstrapAlert("{{alert_msg}}", "{{alert_type}}");
198+
{% end %}
173199
});
174200
</script>
175201
<style>

0 commit comments

Comments
 (0)