-
Couldn't load subscription status.
- Fork 79
Base for new study page #1571
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
Base for new study page #1571
Changes from 5 commits
5b807d2
16260fb
e8987fd
500143d
e27be76
f251b7b
224e55b
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 |
|---|---|---|
|
|
@@ -166,6 +166,18 @@ def status(self): | |
| return qdb.util.infer_status( | ||
| qdb.sql_connection.TRN.execute_fetchindex()) | ||
|
|
||
| @staticmethod | ||
| def all_data_types(): | ||
| """Returns list of all the data types available in the system | ||
| Returns | ||
| ------- | ||
| list of str | ||
|
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. Mind describing what the strings represent? I know its repetitive, but that's the suggested way. |
||
| """ | ||
| with qdb.sql_connection.TRN: | ||
| sql = "SELECT DISTINCT data_type FROM qiita.data_type" | ||
| qdb.sql_connection.TRN.add(sql) | ||
| return qdb.sql_connection.TRN.execute_fetchflatten() | ||
|
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. Seems like this could return None, should this be checking and instead returning an empty list? Otherwise it would probably be worth updating the docstring. My preference would be to return an empty list, not sure what others think about that. 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. This will return an empty list if nothing is in the SQL. 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. Got it, then I'm confused what you are checking for in line 89 of api_proxy.py, maybe that's unnecessary then? 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. ah, yes that's a redundant check. |
||
|
|
||
| @classmethod | ||
| def get_by_status(cls, status): | ||
| """Returns study id for all Studies with given status | ||
|
|
@@ -500,9 +512,23 @@ def info(self): | |
| # remove non-info items from info | ||
| for item in self._non_info: | ||
| info.pop(item) | ||
| # This is an optional column, but should not be considered part | ||
| # of the info | ||
| # removed because redundant to the id already stored in the object | ||
| info.pop('study_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. If optional and not present this is going to fail. 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. The wording is bad, but I think it's supposed to mean it could be part of the main info, but we thought no. I'll reword, since really it's removed because it's redundant to the study id already stored in the object. |
||
|
|
||
| if info['principal_investigator_id']: | ||
| info['principal_investigator'] = qdb.study.StudyPerson( | ||
| info["principal_investigator_id"]) | ||
| else: | ||
| info['principal_investigator'] = None | ||
| del info['principal_investigator_id'] | ||
|
|
||
| if info['lab_person_id']: | ||
| info['lab_person'] = qdb.study.StudyPerson( | ||
| info["lab_person_id"]) | ||
| else: | ||
| info['lab_person'] = None | ||
| del info['lab_person_id'] | ||
|
|
||
| return info | ||
|
|
||
| @info.setter | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| # ----------------------------------------------------------------------------- | ||
| # Copyright (c) 2014--, The Qiita Development Team. | ||
| # | ||
| # Distributed under the terms of the BSD 3-clause License. | ||
| # | ||
| # The full license is in the file LICENSE, distributed with this software. | ||
| # ----------------------------------------------------------------------------- | ||
| from __future__ import division | ||
|
|
||
| # This is the only file in qiita_pet that should import from outside qiita_pet | ||
|
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. Are we sure we want to have this? Regrettably we have a few pieces of code that are "to be removed when X happens" and this compromises the usability, and the stability of those pieces of code and everything that gets built on top of them. If nobody sees this is as a possible problem, then lets leave as is. 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. The idea is qiita_pet will be it's own self contained client for the qiita RESTful API. Since we are developing the UI before developing the RESTful interface, this proxy is needed so I don't have to completely redevelop the UI again once the API goes into place. 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. OK, sounds good then! |
||
| # The idea is that this proxies the call and response dicts we expect from the | ||
| # QIITA API once we build it. This will be removed and replaced with API calls | ||
|
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. QIITA -> Qiita. |
||
| # when the API is complete. | ||
| from qiita_core.qiita_settings import qiita_config | ||
| from qiita_db.study import Study | ||
|
|
||
| from qiita_pet.handlers.base_handlers import BaseHandler | ||
| from qiita_pet.handlers.util import check_access | ||
|
|
||
| html_error_message = "<b>An error occurred %s %s</b></br>%s" | ||
|
|
||
|
|
||
| def _approve(level): | ||
| """Check if the study can be approved based on user level and configuration | ||
|
|
||
| Parameters | ||
| ---------- | ||
| level : str | ||
| The level of the current user | ||
|
|
||
| Returns | ||
| ------- | ||
| bool | ||
| Whether the study can be approved or not | ||
| """ | ||
| return True if not qiita_config.require_approval else level == 'admin' | ||
|
|
||
|
|
||
| class StudyAPIProxy(BaseHandler): | ||
| """Adds API proxy functions to the handler. Can be removed once the restful | ||
|
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. restful -> RESTful |
||
| API is in place.""" | ||
| def study_prep_proxy(self, study_id): | ||
| """Proxies expected json from the API for existing prep templates | ||
|
|
||
| Parameters | ||
| ---------- | ||
| study_id : int | ||
| Study id to get prep template info for | ||
|
|
||
| Returns: | ||
|
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 format this with underscores and remove the semicolon? |
||
| dict of list of dict | ||
| prep template information seperated by data type, in the form | ||
| {data_type: [{prep 1 info dict}, ....], ...} | ||
| """ | ||
| # Can only pass ids over API, so need to instantiate object | ||
| study = Study(study_id) | ||
| check_access(self.current_user, study, raise_error=True) | ||
|
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 whatever error this raises to a 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 |
||
| prep_info = {} | ||
| for dtype in study.data_types: | ||
| prep_info[dtype] = [] | ||
| for prep in study.prep_templates(dtype): | ||
| start_artifact = prep.artifact | ||
| info = { | ||
| 'name': 'PREP %d NAME' % prep.id, | ||
| 'id': prep.id, | ||
| 'status': prep.status, | ||
| 'start_artifact': start_artifact.artifact_type, | ||
| 'start_artifact_id': start_artifact.id, | ||
| 'last_artifact': 'TODO new gui' | ||
| } | ||
| prep_info[dtype].append(info) | ||
| return prep_info | ||
|
|
||
| def study_data_types_proxy(self): | ||
| """Proxies expected json from the API for available data types | ||
|
|
||
| Returns | ||
| ------- | ||
| list of str | ||
| Data types available on the system | ||
| """ | ||
| data_types = Study.all_data_types() | ||
| return data_types if data_types else [] | ||
|
|
||
| def study_info_proxy(self, study_id): | ||
| """Proxies expected json from the API for base study info | ||
|
|
||
| Parameters | ||
| ---------- | ||
| study_id : int | ||
| Study id to get prep template info for | ||
|
|
||
| Returns: | ||
|
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 fix this header. |
||
| dict of list of dict | ||
| prep template information seperated by data type, in the form | ||
| {data_type: [{prep 1 info dict}, ....], ...} | ||
| """ | ||
| # Can only pass ids over API, so need to instantiate object | ||
| study = Study(study_id) | ||
| check_access(self.current_user, study, raise_error=True) | ||
|
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 this error to the |
||
| study_info = study.info | ||
| # Add needed info that is not part of the initial info pull | ||
| study_info['publications'] = study.publications | ||
| study_info['study_id'] = study.id | ||
| study_info['study_title'] = study.title | ||
|
|
||
| # Clean up StudyPerson objects to string for display | ||
| pi = study_info["principal_investigator"] | ||
| study_info["principal_investigator"] = '%s (%s)' % (pi.name, | ||
| pi.affiliation) | ||
| lab_person = study_info["lab_person"] | ||
| study_info["lab_person"] = '%s (%s)' % (lab_person.name, | ||
| lab_person.affiliation) | ||
|
|
||
| samples = study.sample_template.keys() | ||
| study_info['num_samples'] = 0 if samples is None else len(set(samples)) | ||
| return study_info | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| # ----------------------------------------------------------------------------- | ||
| # Copyright (c) 2014--, The Qiita Development Team. | ||
| # | ||
| # Distributed under the terms of the BSD 3-clause License. | ||
| # | ||
| # The full license is in the file LICENSE, distributed with this software. | ||
| # ----------------------------------------------------------------------------- | ||
| from __future__ import division | ||
|
|
||
| from tornado.web import authenticated | ||
|
|
||
| from qiita_pet.handlers.util import to_int, doi_linkifier | ||
| # ONLY IMPORT FROM qiita_pet HERE. All other imports must be made in | ||
| # api_proxy.py so they will be removed when we get the API in place. | ||
| from qiita_pet.handlers.api_proxy import StudyAPIProxy | ||
|
|
||
|
|
||
| class StudyIndexHandler(StudyAPIProxy): | ||
| @authenticated | ||
| def get(self, study_id): | ||
| study = to_int(study_id) | ||
| # Proxies for what will become API requests | ||
| prep_info = self.study_prep_proxy(study) | ||
| data_types = self.study_data_types_proxy() | ||
| study_info = self.study_info_proxy(study) | ||
|
|
||
| self.render("study_base.html", prep_info=prep_info, | ||
| data_types=data_types, study_info=study_info) | ||
|
|
||
|
|
||
| class StudyBaseInfoAJAX(StudyAPIProxy): | ||
| @authenticated | ||
| def get(self): | ||
| study = to_int(self.get_argument('study_id')) | ||
| # Proxy for what will become API request | ||
| study_info = self.study_info_proxy(study) | ||
| study_doi = ' '.join( | ||
| [doi_linkifier(p) for p in study_info['publications']]) | ||
|
|
||
| self.render('study_ajax/base_info.html', | ||
| study_info=study_info, publications=study_doi) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,3 +64,28 @@ def download_link_or_path(is_local_request, filepath, fp_id, label): | |
|
|
||
| doi_linkifier = partial( | ||
| linkify, "<a target=\"_blank\" href=\"http://dx.doi.org/{0}\">{0}</a>") | ||
|
|
||
|
|
||
| def to_int(value): | ||
| """Transforms `value` to an integer | ||
|
|
||
| Parameters | ||
| ---------- | ||
| value : str or int | ||
| The value to transform | ||
|
|
||
| Returns | ||
| ------- | ||
| int | ||
| `value` as an integer | ||
|
|
||
| Raises | ||
| ------ | ||
| HTTPError | ||
| If `value` cannot be transformed to an integer | ||
| """ | ||
| try: | ||
| res = int(value) | ||
| except ValueError: | ||
| raise HTTPError(500, "%s cannot be converted to an integer" % value) | ||
|
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. Not sure if this is something we want to be specific about or simply say: "Not valid study identifier" 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. I don't think so, since this is now in handlers.util, so it's generic. 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. I think this is actually a 4xx error, the best candidates from what I've read so far are: 417, 406 and 400. http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html 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. Best candidate is probably 400 then, since it is a bad study/prep/whatever ID being passed in the URL. |
||
| return res | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| <p style="font-weight:bold;">Abstract</p> | ||
| <p>{{study_info['study_abstract']}}</p> | ||
| Study ID: {{study_info['study_id']}}<br /> | ||
| Publications: {% raw publications %}<br /> | ||
| PI: {{study_info['principal_investigator']}}<br /> | ||
| Lab Contact: {{study_info['lab_person']}}<br /> | ||
| Samples: {{study_info['num_samples']}} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| {% extends sitebase.html %} | ||
| {% block head %} | ||
| <script type="text/javascript"> | ||
| function fill_base(sid) { | ||
| $.get( "/study/description/baseinfo/", { study_id: sid} ) | ||
|
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. This block is under-indented, should be two spaces in. |
||
| .done(function( data ) { | ||
| $("#study-main").html(data); | ||
| $("#study-prep-graph").hide(); | ||
| $("#study-file-breadcrumbs").hide(); | ||
| }); | ||
| } | ||
|
|
||
| $(document).ready(function() { | ||
| fill_base({{study_info['study_id']}}); | ||
| }); | ||
| </script> | ||
| {% end %} | ||
| {% block content %} | ||
| <div class="row"> | ||
| <div class="col-md-3"> | ||
| <h3>Study Information</h3> | ||
| <a href="#">Sample Template</a><br /> | ||
|
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. Some of these options should be showed if you are not the owner of this study, right? 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. I agree. I've added an if statement to show certain things only if study is yours or shared with you. |
||
| <a href="#">Sample Summary</a><br /> | ||
| <a href="/study/upload/{{study_info['study_id']}}">Upload Files</a><br /> | ||
| <a href="#">Edit Study Information</a><br /> | ||
| <a href="#">Delete Study</a> | ||
|
|
||
| <h3>Data Types</h3> | ||
| <div class="panel-group" id="accordion" role="tablist" aria-multiselectable="true"> | ||
| <div class="panel panel-default"> | ||
| {% for dt in prep_info %} | ||
| {% set cleaned_dt = dt.replace(' ', '_') %} | ||
| <div class="panel-heading" role="tab" id="heading{{cleaned_dt}}"> | ||
| <h4 class="panel-title"> | ||
| <a role="button" data-toggle="collapse" data-parent="#accordion" href="#collapse{{cleaned_dt}}" aria-expanded="true" aria-controls="collapse{{cleaned_dt}}">{{dt}}</a> | ||
| </h4> | ||
| </div> | ||
| {% end %} | ||
|
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. I believe this statement is over-indented by two spaces, same thing for the statement below. |
||
| {% for prep in prep_info[dt] %} | ||
|
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. Only owners of the study should see "all" the information, right, this should be filtered for things the user can see, which are presumably only "public" artifacts right? 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. I'm leveraging the API on this one, since this design keeps with the current assumption that the API call object only returns what information is viewable by the user. 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. OK, got it, that makes perfect sense, we should really keep an eye On (Dec-14-15|12:50), Joshua Shorenstein wrote:
|
||
| <div id="collapse{{cleaned_dt}}" class="panel-collapse collapse" role="tabpanel" aria-labelledby="heading{{cleaned_dt}}"> | ||
| <div class="panel-body"> | ||
| <a href="#" style="display:block;color:black;"> | ||
| {{prep['name']}} - ID {{prep['id']}} - {{prep['status']}}<br/> | ||
| {{prep['start_artifact']}} - ID {{prep['start_artifact_id']}}<br/> | ||
| {{prep['last_artifact']}} | ||
| </a> | ||
| </div> | ||
| </div> | ||
| {% end %} | ||
| </div> | ||
| </div> | ||
| </div> | ||
| <div class="col-md-9"> | ||
| <div class="row"> | ||
| <div class="col-md-12" id="study-base-info"> | ||
| <h2>{{study_info['study_title']}} - ID {{study_info['study_id']}}</h2> | ||
| <h3>{{study_info['study_alias']}}</h3> | ||
| </div> | ||
| </div> | ||
| {% if user_level == 'admin' %} | ||
| <div class="row"> | ||
| <div class="col-md-12" id="study-admin"> | ||
| ADMIN STUFF HERE | ||
| </div> | ||
| </div> | ||
| {% end %} | ||
| <div class="row"><div class="col-md-12" id="study-prep-graph"></div></div> | ||
| <div class="row"><div class="col-md-12" id="study-file-breadcrumbs"></div></div> | ||
| <div class="row"><div class="col-md-12" id="study-main"></div></div> | ||
| </div> | ||
| </div> | ||
| {% end %} | ||
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.
This is missing a space between the description and the
Returnsheader.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.
Done