From 79b232f79370a7ad0ba1daf4849a0f0403754ab1 Mon Sep 17 00:00:00 2001 From: Antonio Gonzalez Date: Wed, 16 Aug 2017 07:41:21 -0600 Subject: [PATCH 01/21] fix #2229 --- .../handlers/study_handlers/prep_template.py | 3 +- .../study_ajax/prep_summary_table.html | 63 +++++++++++-------- 2 files changed, 38 insertions(+), 28 deletions(-) diff --git a/qiita_pet/handlers/study_handlers/prep_template.py b/qiita_pet/handlers/study_handlers/prep_template.py index 78bc6dade..401c72986 100644 --- a/qiita_pet/handlers/study_handlers/prep_template.py +++ b/qiita_pet/handlers/study_handlers/prep_template.py @@ -48,7 +48,8 @@ def get(self): res = prep_template_summary_get_req(prep_id, self.current_user.id) self.render('study_ajax/prep_summary_table.html', pid=prep_id, - stats=res['summary'], editable=res['editable']) + stats=res['summary'], editable=res['editable'], + num_samples=res['num_samples']) class PrepTemplateAJAX(BaseHandler): diff --git a/qiita_pet/templates/study_ajax/prep_summary_table.html b/qiita_pet/templates/study_ajax/prep_summary_table.html index 024dafb57..2a2cac913 100644 --- a/qiita_pet/templates/study_ajax/prep_summary_table.html +++ b/qiita_pet/templates/study_ajax/prep_summary_table.html @@ -4,39 +4,48 @@ }); - -{% from future.utils import viewitems %}
Information summary
- - {% for i, (category, summary) in enumerate(stats, -1) %} - - - {% if len(summary) == 1 %} - - {% elif len(set([row[1] for row in summary])) == 1 %} - - {% else %} - - {% for row in summary %} - - - - + +
- - - {{category}}: {{summary[0][0]}} is repeated in all rows. - - {{category}}: All the values in this category are different. - {{category}}
 {{row[0]}}{{row[1]}}
+ {% for i, (category, summary) in enumerate(stats, -1) %} + + + + {% elif len(summary) == num_samples %} + {{category}}: All the values in this category are different. + + {% else %} + {{category}} +      + + + {% for row in summary %} + + + + + + {% end %} + {% end %} {% end %} - {% end %} - - {% end %} -
+ {% if editable %} + + {% else %} +   + {% end %} + + {% if len(summary) == 1 %} + {{category}}: {{summary[0][0]}} is repeated in all rows. +
 {{row[0]}}{{row[1]}}
+ + + + +
From 2d48a65907f21371e86a0d9ad312c77b1e1650b2 Mon Sep 17 00:00:00 2001 From: Antonio Gonzalez Date: Wed, 16 Aug 2017 07:42:17 -0600 Subject: [PATCH 02/21] cleaning html --- .../study_ajax/prep_summary_table.html | 63 +++++++++---------- 1 file changed, 30 insertions(+), 33 deletions(-) diff --git a/qiita_pet/templates/study_ajax/prep_summary_table.html b/qiita_pet/templates/study_ajax/prep_summary_table.html index 2a2cac913..cb8c19ee6 100644 --- a/qiita_pet/templates/study_ajax/prep_summary_table.html +++ b/qiita_pet/templates/study_ajax/prep_summary_table.html @@ -10,42 +10,39 @@
- - {% for i, (category, summary) in enumerate(stats, -1) %} - - + {% end %} +
- {% if editable %} - - {% else %} -   - {% end %} + + {% for i, (category, summary) in enumerate(stats, -1) %} + + + + {% elif len(summary) == num_samples %} + {{category}}: All the values in this category are different. - - {% elif len(summary) == num_samples %} - {{category}}: All the values in this category are different. - - {% else %} - {{category}} -      - - - {% for row in summary %} - - - - - - {% end %} + {% else %} + {{category}} +      + + + {% for row in summary %} + + + + + {% end %} - {% end %} -
+ {% if editable %} + + {% else %} +   + {% end %} + + {% if len(summary) == 1 %} + {{category}}: {{summary[0][0]}} is repeated in all rows. + - {% if len(summary) == 1 %} - {{category}}: {{summary[0][0]}} is repeated in all rows. -
 {{row[0]}}{{row[1]}}
 {{row[0]}}{{row[1]}}
- - - +
From ee0deb461a381fb2c0132a3c807efb06b37a35fd Mon Sep 17 00:00:00 2001 From: Antonio Gonzalez Date: Wed, 16 Aug 2017 08:48:03 -0600 Subject: [PATCH 03/21] fix #2225 and partial #2224 --- qiita_db/test/test_util.py | 6 +++--- qiita_db/util.py | 8 +++++--- qiita_pet/handlers/study_handlers/listing_handlers.py | 3 +++ .../study_handlers/tests/test_listing_handlers.py | 3 +++ qiita_pet/support_files/doc/source/tutorials/sharing.rst | 4 ++-- qiita_pet/templates/list_studies.html | 9 ++++++--- 6 files changed, 22 insertions(+), 11 deletions(-) diff --git a/qiita_db/test/test_util.py b/qiita_db/test/test_util.py index ebeae96a1..b49d483b3 100644 --- a/qiita_db/test/test_util.py +++ b/qiita_db/test/test_util.py @@ -800,9 +800,9 @@ def test_generate_study_list(self): qdb.user.User('shared@foo.bar'), 'test_study_1', info=info) exp_info = [ - {'status': 'private', 'metadata_complete': True, - 'study_tags': None, 'publication_doi': [ - '10.100/123456', '10.100/7891011'], + {'owner': 'test@foo.bar', 'status': 'private', + 'metadata_complete': True, 'study_tags': None, + 'publication_doi': ['10.100/123456', '10.100/7891011'], 'study_title': ('Identification of the Microbiomes for ' 'Cannabis Soils'), 'publication_pid': ['123456', '7891011'], diff --git a/qiita_db/util.py b/qiita_db/util.py index ed5789b3c..70a55ea8b 100644 --- a/qiita_db/util.py +++ b/qiita_db/util.py @@ -1205,7 +1205,7 @@ def generate_study_list(study_ids, public_only=False): ----- The main select might look scary but it's pretty simple: - We select the requiered fields from qiita.study and qiita.study_person - SELECT metadata_complete, study_abstract, study_id, + SELECT email, metadata_complete, study_abstract, study_id, study_title, ebi_study_accession, ebi_submission_status, qiita.study_person.name AS pi_name, qiita.study_person.email AS pi_email, @@ -1238,8 +1238,9 @@ def generate_study_list(study_ids, public_only=False): """ with qdb.sql_connection.TRN: sql = """ - SELECT metadata_complete, study_abstract, study_id, - study_title, ebi_study_accession, ebi_submission_status, + SELECT qiita.study.email as owner, metadata_complete, + study_abstract, study_id, study_title, ebi_study_accession, + ebi_submission_status, qiita.study_person.name AS pi_name, qiita.study_person.email AS pi_email, (SELECT COUNT(sample_id) FROM qiita.study_sample @@ -1303,6 +1304,7 @@ def generate_study_list(study_ids, public_only=False): del info["shared_with_email"] infolist.append({ + 'owner': 'test@foo.bar', 'metadata_complete': info['metadata_complete'], 'publication_pid': info['publication_pid'], 'ebi_submission_status': info['ebi_submission_status'], diff --git a/qiita_pet/handlers/study_handlers/listing_handlers.py b/qiita_pet/handlers/study_handlers/listing_handlers.py index b28c8b6fd..c4dccaa02 100644 --- a/qiita_pet/handlers/study_handlers/listing_handlers.py +++ b/qiita_pet/handlers/study_handlers/listing_handlers.py @@ -217,11 +217,14 @@ def get(self, ignore): study_proc = proc_samples = None info = _build_study_info(self.current_user, search_type, study_proc, proc_samples) + print info # linkifying data len_info = len(info) for i in range(len_info): info[i]['shared'] = ", ".join([study_person_linkifier(element) for element in info[i]['shared']]) + info[i]['owner'] = study_person_linkifier( + (info[i]['owner'], info[i]['owner'])) ppid = [pubmed_linkifier([p]) for p in info[i]['publication_pid']] pdoi = [doi_linkifier([p]) for p in info[i]['publication_doi']] diff --git a/qiita_pet/handlers/study_handlers/tests/test_listing_handlers.py b/qiita_pet/handlers/study_handlers/tests/test_listing_handlers.py index c7eaa38c2..774496898 100644 --- a/qiita_pet/handlers/study_handlers/tests/test_listing_handlers.py +++ b/qiita_pet/handlers/study_handlers/tests/test_listing_handlers.py @@ -32,6 +32,7 @@ def setUp(self): self.single_exp = { 'study_id': 1, + 'owner': 'test@foo.bar', 'status': 'private', 'study_abstract': 'This is a preliminary study to examine the microbiota ' @@ -126,6 +127,7 @@ def test_build_study_info_empty_study(self): 'publication_doi': [], 'study_abstract': 'abstract', 'study_id': 2, + 'owner': 'test@foo.bar', 'ebi_study_accession': None, 'study_title': 'My study', 'study_tags': None, @@ -260,6 +262,7 @@ def setUp(self): 'metadata_complete': True, 'ebi_submission_status': 'submitted', 'study_id': 1, + 'owner': 'test@foo.bar', 'ebi_study_accession': 'EBI123456-BB', 'shared': ('' 'Shared'), diff --git a/qiita_pet/support_files/doc/source/tutorials/sharing.rst b/qiita_pet/support_files/doc/source/tutorials/sharing.rst index a174595b0..16db3a19b 100644 --- a/qiita_pet/support_files/doc/source/tutorials/sharing.rst +++ b/qiita_pet/support_files/doc/source/tutorials/sharing.rst @@ -27,7 +27,7 @@ permissions that the owner of the Study has. These permissions are: Sharing a Study --------------- -In the “Your Studies (includes shared with you)” section of the Studies List +In the “Your Studies” section of the Studies List you have a “Shared With These Users” column that lists all User names that your study is shared with. You can click on the “Modify” button and add/remove users. See below. @@ -47,5 +47,5 @@ will have a link to share it. See below .. figure:: images/sharing_study.gif :align: center - + Analysis sharing example diff --git a/qiita_pet/templates/list_studies.html b/qiita_pet/templates/list_studies.html index f7db79034..e3899159a 100644 --- a/qiita_pet/templates/list_studies.html +++ b/qiita_pet/templates/list_studies.html @@ -127,15 +127,17 @@ { "data": "number_samples_collected" }, { "data": "shared" }, { "data": "pi" }, + { "data": "owner" }, { "data": "pubs" }, { "data": "status" }, { "data": "ebi_info" } ], columnDefs: [ - {type:'natural', targets:[2,6,7]}, + {type:'natural', targets:[2,5,6,7]}, {"targets": [ 2 ], "visible": false}, // render zero {"render": function ( data, type, row, meta ) { + console.log(row) if (data !== null && data !== undefined && data.length != 0){ return '
'+ '
' + @@ -408,7 +410,7 @@
-

Your Studies (includes shared with you)

+

Your Studies

@@ -419,13 +421,14 @@

Your Studies (includes shared with you)

+
Samples Shared With These Users Principal InvestigatorOwner Publications Status Qiita EBI submission
-

Other Studies

+

Public Studies

From 4dc760b6fe81d46ce54af699fe6fce6bed00eb2e Mon Sep 17 00:00:00 2001 From: Antonio Gonzalez Date: Wed, 16 Aug 2017 09:41:22 -0600 Subject: [PATCH 04/21] fix errors --- qiita_db/test/test_util.py | 3 ++- qiita_pet/handlers/study_handlers/listing_handlers.py | 1 - .../handlers/study_handlers/tests/test_listing_handlers.py | 3 ++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/qiita_db/test/test_util.py b/qiita_db/test/test_util.py index b49d483b3..0d268c487 100644 --- a/qiita_db/test/test_util.py +++ b/qiita_db/test/test_util.py @@ -822,7 +822,8 @@ def test_generate_study_list(self): 'pi': ('PI_dude@foo.bar', 'PIDude'), 'artifact_biom_ids': [4, 5, 6, 7], 'number_samples_collected': 27}, - {'status': 'sandbox', 'metadata_complete': True, + {'owner': 'test@foo.bar', + 'status': 'sandbox', 'metadata_complete': True, 'study_tags': None, 'publication_doi': [], 'study_title': 'test_study_1', 'publication_pid': [], 'ebi_submission_status': 'not submitted', diff --git a/qiita_pet/handlers/study_handlers/listing_handlers.py b/qiita_pet/handlers/study_handlers/listing_handlers.py index c4dccaa02..d5d3fb210 100644 --- a/qiita_pet/handlers/study_handlers/listing_handlers.py +++ b/qiita_pet/handlers/study_handlers/listing_handlers.py @@ -217,7 +217,6 @@ def get(self, ignore): study_proc = proc_samples = None info = _build_study_info(self.current_user, search_type, study_proc, proc_samples) - print info # linkifying data len_info = len(info) for i in range(len_info): diff --git a/qiita_pet/handlers/study_handlers/tests/test_listing_handlers.py b/qiita_pet/handlers/study_handlers/tests/test_listing_handlers.py index 774496898..d18944aab 100644 --- a/qiita_pet/handlers/study_handlers/tests/test_listing_handlers.py +++ b/qiita_pet/handlers/study_handlers/tests/test_listing_handlers.py @@ -262,7 +262,8 @@ def setUp(self): 'metadata_complete': True, 'ebi_submission_status': 'submitted', 'study_id': 1, - 'owner': 'test@foo.bar', + 'owner': ('' + 'test@foo.bar'), 'ebi_study_accession': 'EBI123456-BB', 'shared': ('' 'Shared'), From 868a9e01c24d81e1e32a6bdedef6439b4b339608 Mon Sep 17 00:00:00 2001 From: Antonio Gonzalez Date: Thu, 17 Aug 2017 08:34:25 -0600 Subject: [PATCH 05/21] fix #2224 and fix #2223 --- qiita_db/test/test_util.py | 3 ++- qiita_db/util.py | 9 +++++---- .../tests/test_listing_handlers.py | 3 +++ qiita_pet/templates/list_studies.html | 17 +++++++++-------- 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/qiita_db/test/test_util.py b/qiita_db/test/test_util.py index 0d268c487..eae8996f9 100644 --- a/qiita_db/test/test_util.py +++ b/qiita_db/test/test_util.py @@ -801,6 +801,7 @@ def test_generate_study_list(self): exp_info = [ {'owner': 'test@foo.bar', 'status': 'private', + 'study_alias': 'Cannabis Soils', 'metadata_complete': True, 'study_tags': None, 'publication_doi': ['10.100/123456', '10.100/7891011'], 'study_title': ('Identification of the Microbiomes for ' @@ -822,7 +823,7 @@ def test_generate_study_list(self): 'pi': ('PI_dude@foo.bar', 'PIDude'), 'artifact_biom_ids': [4, 5, 6, 7], 'number_samples_collected': 27}, - {'owner': 'test@foo.bar', + {'owner': 'test@foo.bar', "study_alias": "TST", 'status': 'sandbox', 'metadata_complete': True, 'study_tags': None, 'publication_doi': [], 'study_title': 'test_study_1', 'publication_pid': [], diff --git a/qiita_db/util.py b/qiita_db/util.py index 70a55ea8b..0c86d5e30 100644 --- a/qiita_db/util.py +++ b/qiita_db/util.py @@ -1205,7 +1205,7 @@ def generate_study_list(study_ids, public_only=False): ----- The main select might look scary but it's pretty simple: - We select the requiered fields from qiita.study and qiita.study_person - SELECT email, metadata_complete, study_abstract, study_id, + SELECT email, metadata_complete, study_abstract, study_id, study_alias, study_title, ebi_study_accession, ebi_submission_status, qiita.study_person.name AS pi_name, qiita.study_person.email AS pi_email, @@ -1239,8 +1239,8 @@ def generate_study_list(study_ids, public_only=False): with qdb.sql_connection.TRN: sql = """ SELECT qiita.study.email as owner, metadata_complete, - study_abstract, study_id, study_title, ebi_study_accession, - ebi_submission_status, + study_abstract, study_id, study_alias, study_title, + ebi_study_accession, ebi_submission_status, qiita.study_person.name AS pi_name, qiita.study_person.email AS pi_email, (SELECT COUNT(sample_id) FROM qiita.study_sample @@ -1304,7 +1304,8 @@ def generate_study_list(study_ids, public_only=False): del info["shared_with_email"] infolist.append({ - 'owner': 'test@foo.bar', + 'owner': info['owner'], + 'study_alias': info['study_alias'], 'metadata_complete': info['metadata_complete'], 'publication_pid': info['publication_pid'], 'ebi_submission_status': info['ebi_submission_status'], diff --git a/qiita_pet/handlers/study_handlers/tests/test_listing_handlers.py b/qiita_pet/handlers/study_handlers/tests/test_listing_handlers.py index d18944aab..158580399 100644 --- a/qiita_pet/handlers/study_handlers/tests/test_listing_handlers.py +++ b/qiita_pet/handlers/study_handlers/tests/test_listing_handlers.py @@ -33,6 +33,7 @@ def setUp(self): self.single_exp = { 'study_id': 1, 'owner': 'test@foo.bar', + 'study_alias': 'Cannabis Soils', 'status': 'private', 'study_abstract': 'This is a preliminary study to examine the microbiota ' @@ -128,6 +129,7 @@ def test_build_study_info_empty_study(self): 'study_abstract': 'abstract', 'study_id': 2, 'owner': 'test@foo.bar', + 'study_alias': 'Cannabis Soils', 'ebi_study_accession': None, 'study_title': 'My study', 'study_tags': None, @@ -262,6 +264,7 @@ def setUp(self): 'metadata_complete': True, 'ebi_submission_status': 'submitted', 'study_id': 1, + 'study_alias': 'Cannabis Soils', 'owner': ('' 'test@foo.bar'), 'ebi_study_accession': 'EBI123456-BB', diff --git a/qiita_pet/templates/list_studies.html b/qiita_pet/templates/list_studies.html index e3899159a..9133ce710 100644 --- a/qiita_pet/templates/list_studies.html +++ b/qiita_pet/templates/list_studies.html @@ -127,17 +127,15 @@ { "data": "number_samples_collected" }, { "data": "shared" }, { "data": "pi" }, - { "data": "owner" }, { "data": "pubs" }, { "data": "status" }, - { "data": "ebi_info" } - ], + { "data": "ebi_info" }, + { "data": "study_alias" }], columnDefs: [ - {type:'natural', targets:[2,5,6,7]}, - {"targets": [ 2 ], "visible": false}, + {type:'natural', targets:[2,6,7]}, + {"targets": [ 2, 10 ], "visible": false}, // render zero {"render": function ( data, type, row, meta ) { - console.log(row) if (data !== null && data !== undefined && data.length != 0){ return '
'+ '
' + @@ -168,7 +166,10 @@ {"render": function ( data, type, row, meta ) { var glyph = 'remove'; if(data === true) { glyph = 'ok' } - return ""+ data +"
Modify"; + result = "Modify
"; + result += "Owner: " + row.owner + "
"; + result += ""+ data +""; + return result; }, targets: [5]}, ], "language": { @@ -421,10 +422,10 @@

Your Studies

 - +
Samples Shared With These Users Principal InvestigatorOwner Publications Status Qiita EBI submissionStudy Alias
From e5ff2c848b45aec493643e2fe6ebe0ec42e9e35d Mon Sep 17 00:00:00 2001 From: Antonio Gonzalez Date: Thu, 17 Aug 2017 09:43:26 -0600 Subject: [PATCH 06/21] fixing comments/errors --- qiita_db/test/test_util.py | 49 +++++++++---------- qiita_db/util.py | 14 ++++-- .../handlers/api_proxy/tests/test_studies.py | 2 +- .../study_handlers/listing_handlers.py | 2 - .../tests/test_listing_handlers.py | 11 ++--- 5 files changed, 39 insertions(+), 39 deletions(-) diff --git a/qiita_db/test/test_util.py b/qiita_db/test/test_util.py index eae8996f9..9ddacb845 100644 --- a/qiita_db/test/test_util.py +++ b/qiita_db/test/test_util.py @@ -800,38 +800,37 @@ def test_generate_study_list(self): qdb.user.User('shared@foo.bar'), 'test_study_1', info=info) exp_info = [ - {'owner': 'test@foo.bar', 'status': 'private', - 'study_alias': 'Cannabis Soils', - 'metadata_complete': True, 'study_tags': None, - 'publication_doi': ['10.100/123456', '10.100/7891011'], - 'study_title': ('Identification of the Microbiomes for ' - 'Cannabis Soils'), - 'publication_pid': ['123456', '7891011'], + {'status': 'private', 'study_title': ( + 'Identification of the Microbiomes for Cannabis Soils'), + 'metadata_complete': True, 'publication_pid': [ + '123456', '7891011'], 'artifact_biom_ids': [4, 5, 6, 7], 'ebi_submission_status': 'submitted', 'study_id': 1, - 'ebi_study_accession': 'EBI123456-BB', + 'ebi_study_accession': 'EBI123456-BB', 'owner': 'Dude', 'shared': [('shared@foo.bar', 'Shared')], 'study_abstract': ( 'This is a preliminary study to examine the microbiota ' - 'associated with the Cannabis plant. Soils samples from the ' - 'bulk soil, soil associated with the roots, and the ' - 'rhizosphere were extracted and the DNA sequenced. Roots from ' - 'three independent plants of different strains were examined. ' - 'These roots were obtained November 11, 2011 from plants that ' - 'had been harvested in the summer. Future studies will ' - 'attempt to analyze the soils and rhizospheres from the same ' - 'location at different time points in the plant lifecycle.'), - 'pi': ('PI_dude@foo.bar', 'PIDude'), - 'artifact_biom_ids': [4, 5, 6, 7], + 'associated with the Cannabis plant. Soils samples from ' + 'the bulk soil, soil associated with the roots, and the ' + 'rhizosphere were extracted and the DNA sequenced. Roots ' + 'from three independent plants of different strains were ' + 'examined. These roots were obtained November 11, 2011 from ' + 'plants that had been harvested in the summer. Future studies ' + 'will attempt to analyze the soils and rhizospheres from the ' + 'same location at different time points in the plant ' + 'lifecycle.'), 'pi': ('PI_dude@foo.bar', 'PIDude'), + 'publication_doi': ['10.100/123456', '10.100/7891011'], + 'study_alias': 'Cannabis Soils', 'study_tags': None, 'number_samples_collected': 27}, - {'owner': 'test@foo.bar', "study_alias": "TST", - 'status': 'sandbox', 'metadata_complete': True, - 'study_tags': None, 'publication_doi': [], - 'study_title': 'test_study_1', 'publication_pid': [], + {'status': 'sandbox', 'study_title': 'test_study_1', + 'metadata_complete': True, 'publication_pid': [], + 'artifact_biom_ids': None, 'ebi_submission_status': 'not submitted', 'study_id': new_study.id, 'ebi_study_accession': None, - 'shared': [], 'study_abstract': 'Some abstract goes here', - 'pi': ('lab_dude@foo.bar', 'LabDude'), - 'artifact_biom_ids': None, 'number_samples_collected': 0}] + 'owner': 'Shared', 'shared': [], + 'study_abstract': 'Some abstract goes here', + 'pi': ('lab_dude@foo.bar', 'LabDude'), 'publication_doi': [], + 'study_alias': 'TST', 'study_tags': None, + 'number_samples_collected': 0}] obs_info = qdb.util.generate_study_list([1, 2, 3, 4], True) self.assertEqual(obs_info, exp_info) diff --git a/qiita_db/util.py b/qiita_db/util.py index 0c86d5e30..6de4650ed 100644 --- a/qiita_db/util.py +++ b/qiita_db/util.py @@ -1205,7 +1205,7 @@ def generate_study_list(study_ids, public_only=False): ----- The main select might look scary but it's pretty simple: - We select the requiered fields from qiita.study and qiita.study_person - SELECT email, metadata_complete, study_abstract, study_id, study_alias, + SELECT metadata_complete, study_abstract, study_id, study_alias, study_title, ebi_study_accession, ebi_submission_status, qiita.study_person.name AS pi_name, qiita.study_person.email AS pi_email, @@ -1235,12 +1235,14 @@ def generate_study_list(study_ids, public_only=False): - all study tags (SELECT array_agg(study_tag) FROM qiita.per_study_tags WHERE study_id=qiita.study.study_id) AS study_tags + - study owner + (SELECT name FROM qiita.qiita_user + WHERE email=qiita.study.email) AS owner """ with qdb.sql_connection.TRN: sql = """ - SELECT qiita.study.email as owner, metadata_complete, - study_abstract, study_id, study_alias, study_title, - ebi_study_accession, ebi_submission_status, + SELECT metadata_complete, study_abstract, study_id, study_alias, + study_title, ebi_study_accession, ebi_submission_status, qiita.study_person.name AS pi_name, qiita.study_person.email AS pi_email, (SELECT COUNT(sample_id) FROM qiita.study_sample @@ -1262,7 +1264,9 @@ def generate_study_list(study_ids, public_only=False): LEFT JOIN qiita.qiita_user USING (email) WHERE study_id=qiita.study.study_id) AS shared_with_email, (SELECT array_agg(study_tag) FROM qiita.per_study_tags - WHERE study_id=qiita.study.study_id) AS study_tags + WHERE study_id=qiita.study.study_id) AS study_tags, + (SELECT name FROM qiita.qiita_user + WHERE email=qiita.study.email) AS owner FROM qiita.study LEFT JOIN qiita.study_person ON ( study_person_id=principal_investigator_id) diff --git a/qiita_pet/handlers/api_proxy/tests/test_studies.py b/qiita_pet/handlers/api_proxy/tests/test_studies.py index a838862ad..559994090 100644 --- a/qiita_pet/handlers/api_proxy/tests/test_studies.py +++ b/qiita_pet/handlers/api_proxy/tests/test_studies.py @@ -92,7 +92,7 @@ def test_study_get_req(self): 'study_title': 'Identification of the Microbiomes for ' 'Cannabis Soils', 'number_samples_collected': 27, - 'owner': 'test@foo.bar', + 'owner': 'Dude', 'ebi_submission_status': 'submitted', 'has_access_to_raw_data': True, 'show_biom_download_button': True, diff --git a/qiita_pet/handlers/study_handlers/listing_handlers.py b/qiita_pet/handlers/study_handlers/listing_handlers.py index d5d3fb210..b28c8b6fd 100644 --- a/qiita_pet/handlers/study_handlers/listing_handlers.py +++ b/qiita_pet/handlers/study_handlers/listing_handlers.py @@ -222,8 +222,6 @@ def get(self, ignore): for i in range(len_info): info[i]['shared'] = ", ".join([study_person_linkifier(element) for element in info[i]['shared']]) - info[i]['owner'] = study_person_linkifier( - (info[i]['owner'], info[i]['owner'])) ppid = [pubmed_linkifier([p]) for p in info[i]['publication_pid']] pdoi = [doi_linkifier([p]) for p in info[i]['publication_doi']] diff --git a/qiita_pet/handlers/study_handlers/tests/test_listing_handlers.py b/qiita_pet/handlers/study_handlers/tests/test_listing_handlers.py index 158580399..cc9fc5de5 100644 --- a/qiita_pet/handlers/study_handlers/tests/test_listing_handlers.py +++ b/qiita_pet/handlers/study_handlers/tests/test_listing_handlers.py @@ -32,7 +32,7 @@ def setUp(self): self.single_exp = { 'study_id': 1, - 'owner': 'test@foo.bar', + 'owner': 'Dude', 'study_alias': 'Cannabis Soils', 'status': 'private', 'study_abstract': @@ -115,8 +115,8 @@ def test_build_study_info_empty_study(self): 'study_alias': 'alias', 'study_abstract': 'abstract'} Study.create(User('test@foo.bar'), "My study", info=info) - obs = _build_study_info(User('test@foo.bar'), 'user') + obs = _build_study_info(User('test@foo.bar'), 'user') self.exp.append({ 'metadata_complete': False, 'ebi_submission_status': @@ -128,8 +128,8 @@ def test_build_study_info_empty_study(self): 'publication_doi': [], 'study_abstract': 'abstract', 'study_id': 2, - 'owner': 'test@foo.bar', - 'study_alias': 'Cannabis Soils', + 'owner': 'Dude', + 'study_alias': 'alias', 'ebi_study_accession': None, 'study_title': 'My study', 'study_tags': None, @@ -265,8 +265,7 @@ def setUp(self): 'ebi_submission_status': 'submitted', 'study_id': 1, 'study_alias': 'Cannabis Soils', - 'owner': ('' - 'test@foo.bar'), + 'owner': 'Dude', 'ebi_study_accession': 'EBI123456-BB', 'shared': ('' 'Shared'), From 24b7d8e2ee92f3ad17556d0825aebe5122bc1b0d Mon Sep 17 00:00:00 2001 From: Jose Navas Date: Thu, 17 Aug 2017 10:40:43 -0700 Subject: [PATCH 07/21] Remove race condition - Fixes 2143 (#2203) * Removing race condition * Submit the validator release as another job * Solving some tests * Fixing test * Addressing @antgonza's comments --- qiita_db/private.py | 21 +++++++- qiita_db/processing_job.py | 74 ++++++++++++++++++++------- qiita_db/support_files/patches/57.sql | 19 +++++++ qiita_db/test/test_artifact.py | 1 + qiita_db/test/test_processing_job.py | 21 ++++++-- 5 files changed, 113 insertions(+), 23 deletions(-) create mode 100644 qiita_db/support_files/patches/57.sql diff --git a/qiita_db/private.py b/qiita_db/private.py index 78a286f51..0f10187ca 100644 --- a/qiita_db/private.py +++ b/qiita_db/private.py @@ -47,8 +47,27 @@ def build_analysis_files(job): j.submit() sleep(1) + # The validator jobs no longer finish the job automatically so we need + # to release the validators here + job.release_validators() -TASK_DICT = {'build_analysis_files': build_analysis_files} + +def release_validators(job): + """Waits until all the validators of a job are completed + + Parameters + ---------- + job : qiita_db.processing_job.ProcessingJob + The processing job with the information of the parent job + """ + with qdb.sql_connection.TRN: + qdb.processing_job.ProcessingJob( + job.parameters.values['job']).release_validators() + job._set_status('success') + + +TASK_DICT = {'build_analysis_files': build_analysis_files, + 'release_validators': release_validators} def private_task(job_id): diff --git a/qiita_db/processing_job.py b/qiita_db/processing_job.py index fa4d93341..86453d4f5 100644 --- a/qiita_db/processing_job.py +++ b/qiita_db/processing_job.py @@ -14,6 +14,7 @@ from itertools import chain from collections import defaultdict from json import dumps, loads +from time import sleep from future.utils import viewitems, viewvalues import networkx as nx @@ -420,8 +421,9 @@ def release_validators(self): "Only artifact transformation and private jobs can " "release validators") - # Check if all the validators are ready by checking that there is - # no validator processing job whose status is not waiting + # Check if all the validators are completed. Validator jobs can be + # in two states when completed: 'waiting' in case of success + # or 'error' otherwise sql = """SELECT COUNT(1) FROM qiita.processing_job_validator pjv JOIN qiita.processing_job pj ON @@ -429,12 +431,49 @@ def release_validators(self): JOIN qiita.processing_job_status USING (processing_job_status_id) WHERE pjv.processing_job_id = %s - AND processing_job_status != %s""" - qdb.sql_connection.TRN.add(sql, [self.id, 'waiting']) + AND processing_job_status NOT IN %s""" + sql_args = [self.id, ('waiting', 'error')] + qdb.sql_connection.TRN.add(sql, sql_args) remaining = qdb.sql_connection.TRN.execute_fetchlast() - if remaining == 0: - # All validators have completed + # Active polling - wait until all validator jobs are completed + while remaining != 0: + self.step = "Validating outputs (%d remaining)" % remaining + sleep(10) + qdb.sql_connection.TRN.add(sql, sql_args) + remaining = qdb.sql_connection.TRN.execute_fetchlast() + + # Check if any of the validators errored + sql = """SELECT validator_id + FROM qiita.processing_job_validator pjv + JOIN qiita.processing_job pj + ON pjv.validator_id = pj.processing_job_id + JOIN qiita.processing_job_status USING + (processing_job_status_id) + WHERE pjv.processing_job_id = %s AND + processing_job_status = %s""" + qdb.sql_connection.TRN.add(sql, [self.id, 'error']) + errored = qdb.sql_connection.TRN.execute_fetchflatten() + + if errored: + # At least one of the validators failed, Set the rest of the + # validators and the current job as failed + qdb.sql_connection.TRN.add(sql, [self.id, 'waiting']) + waiting = qdb.sql_connection.TRN.execute_fetchflatten() + + common_error = "\n".join( + ["Validator %s error message: %s" + % (j, ProcessingJob(j).log.msg) for j in errored]) + + val_error = "%d sister validator jobs failed: %s" % ( + len(errored), common_error) + for j in waiting: + ProcessingJob(j)._set_error(val_error) + + self._set_error('%d validator jobs failed: %s' + % (len(errored), common_error)) + else: + # All validators have successfully completed sql = """SELECT validator_id FROM qiita.processing_job_validator WHERE processing_job_id = %s""" @@ -460,8 +499,6 @@ def release_validators(self): self._update_and_launch_children(mapping) self._set_status('success') - else: - self.step = "Validating outputs (%d remaining)" % remaining def _complete_artifact_definition(self, artifact_data): """"Performs the needed steps to complete an artifact definition job @@ -487,7 +524,6 @@ def _complete_artifact_definition(self, artifact_data): if job_params['provenance'] is not None: # The artifact is a result from a previous job provenance = loads(job_params['provenance']) - job = ProcessingJob(provenance['job']) if provenance.get('data_type') is not None: artifact_data = {'data_type': provenance['data_type'], 'artifact_data': artifact_data} @@ -500,7 +536,6 @@ def _complete_artifact_definition(self, artifact_data): qdb.sql_connection.TRN.execute() # Can't create the artifact until all validators are completed self._set_status('waiting') - job.release_validators() else: # The artifact is uploaded by the user or is the initial # artifact of an analysis @@ -619,6 +654,16 @@ def _complete_artifact_transformation(self, artifacts_data): for j in validator_jobs: j.submit() + # Submit the job that will release all the validators + plugin = qdb.software.Software.from_name_and_version( + 'Qiita', 'alpha') + cmd = plugin.get_command('release_validators') + params = qdb.software.Parameters.load( + cmd, values_dict={'job': self.id}) + job = ProcessingJob.create(self.user, params) + # Doing the submission outside of the transaction + job.submit() + def _set_validator_jobs(self, validator_jobs): """Sets the validator jobs for the current job @@ -673,15 +718,6 @@ def complete(self, success, artifacts_data=None, error=None): else: self._set_status('success') else: - if self.command.software.type == 'artifact definition': - job_params = self.parameters.values - if job_params.get('provenance') is not None: - # This artifact definition job is a result of a command - # run, if it fails, set up the status of the "parent" - # job also as failed, and assign the sem error message - provenance = loads(job_params['provenance']) - job = ProcessingJob(provenance['job']) - job._set_error(error) self._set_error(error) @property diff --git a/qiita_db/support_files/patches/57.sql b/qiita_db/support_files/patches/57.sql new file mode 100644 index 000000000..ac5970659 --- /dev/null +++ b/qiita_db/support_files/patches/57.sql @@ -0,0 +1,19 @@ +-- Aug 8, 2017 +-- Add release validators internal Qiita command + +DO $do$ +DECLARE + qiita_sw_id bigint; + rv_cmd_id bigint; +BEGIN + SELECT software_id INTO qiita_sw_id + FROM qiita.software + WHERE name = 'Qiita' AND version = 'alpha'; + + INSERT INTO qiita.software_command (software_id, name, description) + VALUES (qiita_sw_id, 'release_validators', 'Releases the job validators') + RETURNING command_id INTO rv_cmd_id; + + INSERT INTO qiita.command_parameter (command_id, parameter_name, parameter_type, required, default_value) + VALUES (rv_cmd_id, 'job', 'string', True, NULL); +END $do$; diff --git a/qiita_db/test/test_artifact.py b/qiita_db/test/test_artifact.py index 6441f6baf..ca4160c3b 100644 --- a/qiita_db/test/test_artifact.py +++ b/qiita_db/test/test_artifact.py @@ -1011,6 +1011,7 @@ def test_delete_as_output_job(self): job.complete(True, artifacts_data=data) job = qdb.processing_job.ProcessingJob( "bcc7ebcd-39c1-43e4-af2d-822e3589f14d") + job.release_validators() artifact = job.outputs['OTU table'] self._clean_up_files.extend([afp for _, afp, _ in artifact.filepaths]) diff --git a/qiita_db/test/test_processing_job.py b/qiita_db/test/test_processing_job.py index 0bfdbb540..84fb9ef4f 100644 --- a/qiita_db/test/test_processing_job.py +++ b/qiita_db/test/test_processing_job.py @@ -359,6 +359,12 @@ def test_complete_multiple_outputs(self): artifact_data_2 = {'filepaths': [(fp2, 'biom')], 'artifact_type': 'BIOM'} obs2._complete_artifact_definition(artifact_data_2) + self.assertEqual(obs1.status, 'waiting') + self.assertEqual(obs2.status, 'waiting') + self.assertEqual(job.status, 'running') + + job.release_validators() + self.assertEqual(obs1.status, 'success') self.assertEqual(obs2.status, 'success') self.assertEqual(job.status, 'success') @@ -386,7 +392,8 @@ def test_complete_artifact_definition(self): qdb.user.User('test@foo.bar'), params) job._set_validator_jobs([obs]) obs._complete_artifact_definition(artifact_data) - self.assertEqual(job.status, 'success') + self.assertEqual(obs.status, 'waiting') + self.assertEqual(job.status, 'running') # Upload case implicitly tested by "test_complete_type" def test_complete_artifact_transformation(self): @@ -476,7 +483,9 @@ def test_complete_success(self): obsjobs = set(self._get_all_job_ids()) - self.assertEqual(len(obsjobs), len(alljobs) + 1) + # The complete call above submits 2 new jobs: the validator job and + # the release validators job. Hence the +2 + self.assertEqual(len(obsjobs), len(alljobs) + 2) self._wait_for_job(job) def test_complete_failure(self): @@ -501,12 +510,17 @@ def test_complete_failure(self): ) obs = qdb.processing_job.ProcessingJob.create( qdb.user.User('test@foo.bar'), params) + job._set_validator_jobs([obs]) obs.complete(False, error="Validation failure") self.assertEqual(obs.status, 'error') self.assertEqual(obs.log.msg, 'Validation failure') + self.assertEqual(job.status, 'running') + job.release_validators() self.assertEqual(job.status, 'error') - self.assertEqual(job.log.msg, 'Validation failure') + self.assertEqual( + job.log.msg, '1 validator jobs failed: Validator %s ' + 'error message: Validation failure' % obs.id) def test_complete_error(self): with self.assertRaises( @@ -628,6 +642,7 @@ def test_outputs(self): job._set_validator_jobs([obs]) exp_artifact_count = qdb.util.get_count('qiita.artifact') + 1 obs._complete_artifact_definition(artifact_data) + job.release_validators() self.assertEqual(job.status, 'success') obs = job.outputs From 51ff4f92199448678f892d22f2d6f75b04242894 Mon Sep 17 00:00:00 2001 From: Antonio Gonzalez Date: Thu, 17 Aug 2017 11:47:49 -0600 Subject: [PATCH 08/21] fixing error --- .../handlers/api_proxy/tests/test_studies.py | 64 ++++++++----------- 1 file changed, 28 insertions(+), 36 deletions(-) diff --git a/qiita_pet/handlers/api_proxy/tests/test_studies.py b/qiita_pet/handlers/api_proxy/tests/test_studies.py index 559994090..b461c47d7 100644 --- a/qiita_pet/handlers/api_proxy/tests/test_studies.py +++ b/qiita_pet/handlers/api_proxy/tests/test_studies.py @@ -50,18 +50,16 @@ def test_study_get_req(self): obs = study_get_req(1, 'test@foo.bar') exp = { 'status': 'success', - 'message': '', 'study_info': { - 'mixs_compliant': True, - 'metadata_complete': True, - 'reprocess': False, - 'emp_person_id': 2, - 'number_samples_promised': 27, - 'funding': None, - 'vamps_id': None, + 'mixs_compliant': True, 'metadata_complete': True, + 'reprocess': False, 'owner': 'test@foo.bar', + 'emp_person_id': 2, 'number_samples_promised': 27, + 'funding': None, 'show_biom_download_button': True, + 'publication_pid': ['123456', '7891011'], 'vamps_id': None, 'first_contact': datetime(2014, 5, 19, 16, 10), - 'timeseries_type_id': 1, - 'study_abstract': + 'ebi_submission_status': 'submitted', + 'show_raw_download_button': True, 'timeseries_type_id': 1, + 'study_abstract': ( 'This is a preliminary study to examine the microbiota ' 'associated with the Cannabis plant. Soils samples from ' 'the bulk soil, soil associated with the roots, and the ' @@ -71,33 +69,25 @@ def test_study_get_req(self): 'from plants that had been harvested in the summer. ' 'Future studies will attempt to analyze the soils and ' 'rhizospheres from the same location at different time ' - 'points in the plant lifecycle.', - 'status': 'private', - 'spatial_series': False, - 'study_description': 'Analysis of the Cannabis Plant ' - 'Microbiome', - 'shared_with': ['shared@foo.bar'], - 'lab_person': {'affiliation': 'knight lab', - 'name': 'LabDude', - 'email': 'lab_dude@foo.bar'}, - 'principal_investigator': {'affiliation': 'Wash U', - 'name': 'PIDude', - 'email': 'PI_dude@foo.bar'}, - 'study_alias': 'Cannabis Soils', - 'study_id': 1, + 'points in the plant lifecycle.'), + 'status': 'private', 'spatial_series': False, + 'study_description': ( + 'Analysis of the Cannabis Plant Microbiome'), + 'shared_with': ['shared@foo.bar'], 'publication_doi': [ + '10.100/123456', '10.100/7891011'], + 'has_access_to_raw_data': True, 'lab_person': { + 'affiliation': 'knight lab', 'name': 'LabDude', + 'email': 'lab_dude@foo.bar'}, + 'principal_investigator': { + 'affiliation': 'Wash U', 'name': 'PIDude', + 'email': 'PI_dude@foo.bar'}, + 'study_alias': 'Cannabis Soils', 'study_id': 1, 'most_recent_contact': datetime(2014, 5, 19, 16, 11), - 'publication_doi': ['10.100/123456', '10.100/7891011'], - 'publication_pid': ['123456', '7891011'], - 'num_samples': 27, - 'study_title': 'Identification of the Microbiomes for ' - 'Cannabis Soils', - 'number_samples_collected': 27, - 'owner': 'Dude', - 'ebi_submission_status': 'submitted', - 'has_access_to_raw_data': True, - 'show_biom_download_button': True, - 'show_raw_download_button': True, - 'ebi_study_accession': 'EBI123456-BB'}, + 'ebi_study_accession': 'EBI123456-BB', 'num_samples': 27, + 'study_title': ( + 'Identification of the Microbiomes for Cannabis Soils'), + 'number_samples_collected': 27}, + 'message': '', 'editable': True} self.assertEqual(obs, exp) @@ -139,6 +129,8 @@ def test_study_get_req(self): 'study_description': 'DESC', 'shared_with': [], 'lab_person': None, + 'study_alias': "FCM", + 'owner': 'Dude', 'principal_investigator': {'affiliation': 'Wash U', 'name': 'PIDude', 'email': 'PI_dude@foo.bar'}, From adf98a3ac3079e9064c816c494b1a95383e6fc09 Mon Sep 17 00:00:00 2001 From: Antonio Gonzalez Date: Thu, 17 Aug 2017 12:22:57 -0600 Subject: [PATCH 09/21] fix #2216 --- qiita_pet/handlers/artifact_handlers/base_handlers.py | 2 ++ .../handlers/artifact_handlers/tests/test_base_handlers.py | 7 +++++++ qiita_pet/templates/artifact_ajax/artifact_summary.html | 1 + 3 files changed, 10 insertions(+) diff --git a/qiita_pet/handlers/artifact_handlers/base_handlers.py b/qiita_pet/handlers/artifact_handlers/base_handlers.py index d56c62c6a..c87cdbb48 100644 --- a/qiita_pet/handlers/artifact_handlers/base_handlers.py +++ b/qiita_pet/handlers/artifact_handlers/base_handlers.py @@ -216,6 +216,8 @@ def artifact_summary_get_request(user, artifact_id): 'processing_jobs': processing_jobs, 'summary': summary, 'job': job_info, + 'artifact_timestamp': artifact.timestamp.strftime( + "%Y-%m-%d %H:%m"), 'errored_jobs': errored_jobs} diff --git a/qiita_pet/handlers/artifact_handlers/tests/test_base_handlers.py b/qiita_pet/handlers/artifact_handlers/tests/test_base_handlers.py index 5b34f639e..6136276db 100644 --- a/qiita_pet/handlers/artifact_handlers/tests/test_base_handlers.py +++ b/qiita_pet/handlers/artifact_handlers/tests/test_base_handlers.py @@ -92,6 +92,7 @@ def test_artifact_summary_get_request(self): (2L, '1_s_G1_L001_sequences_barcodes.fastq.gz (raw barcodes)')] exp = {'name': 'Raw data 1', 'artifact_id': 1, + 'artifact_timestamp': '2012-10-01 09:10', 'visibility': 'private', 'editable': True, 'buttons': ('