Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 28 additions & 2 deletions qiita_db/study.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

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 Returns header.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Returns
-------
list of str
Copy link
Contributor

Choose a reason for hiding this comment

The 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()
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will return an empty list if nothing is in the SQL.

Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Expand Down Expand Up @@ -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')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If optional and not present this is going to fail.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Expand Down
117 changes: 117 additions & 0 deletions qiita_pet/handlers/api_proxy.py
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
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

The 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:
Copy link
Contributor

Choose a reason for hiding this comment

The 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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add whatever error this raises to a Raises section in the docstring?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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:
Copy link
Contributor

Choose a reason for hiding this comment

The 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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add this error to the Raises section of the docstring?

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
4 changes: 3 additions & 1 deletion qiita_pet/handlers/study_handlers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,10 @@
from .ebi_handlers import EBISubmitHandler
from .metadata_summary_handlers import MetadataSummaryHandler
from .vamps_handlers import VAMPSHandler
from .base import StudyIndexHandler, StudyBaseInfoAJAX

__all__ = ['ListStudiesHandler', 'StudyApprovalList', 'ShareStudyAJAX',
'StudyEditHandler', 'CreateStudyAJAX', 'StudyDescriptionHandler',
'PreprocessingSummaryHandler', 'EBISubmitHandler',
'MetadataSummaryHandler', 'VAMPSHandler', 'SearchStudiesAJAX']
'MetadataSummaryHandler', 'VAMPSHandler', 'SearchStudiesAJAX',
'StudyIndexHandler', 'StudyBaseInfoAJAX']
41 changes: 41 additions & 0 deletions qiita_pet/handlers/study_handlers/base.py
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)
25 changes: 25 additions & 0 deletions qiita_pet/handlers/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The 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"

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
7 changes: 7 additions & 0 deletions qiita_pet/templates/study_ajax/base_info.html
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']}}
72 changes: 72 additions & 0 deletions qiita_pet/templates/study_base.html
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} )
Copy link
Contributor

Choose a reason for hiding this comment

The 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 />
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 %}
Copy link
Contributor

Choose a reason for hiding this comment

The 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] %}
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The 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
opened for this as otherwise it will lead to a rather strange-looking
UI.

On (Dec-14-15|12:50), Joshua Shorenstein wrote:

  • Data Types

  •  <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 %}
    
  •    {% for prep in prep_info[dt] %}
    

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.


Reply to this email directly or view it on GitHub:
https://github.com/biocore/qiita/pull/1571/files#r47555626

<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 %}
6 changes: 5 additions & 1 deletion qiita_pet/webserver.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
ShowAnalysesHandler, ResultsHandler, SelectedSamplesHandler,
AnalysisSummaryAJAX)
from qiita_pet.handlers.study_handlers import (
StudyIndexHandler, StudyBaseInfoAJAX,
StudyEditHandler, ListStudiesHandler, SearchStudiesAJAX,
StudyDescriptionHandler, MetadataSummaryHandler, EBISubmitHandler,
CreateStudyAJAX, ShareStudyAJAX, StudyApprovalList,
Expand Down Expand Up @@ -103,7 +104,10 @@ def __init__(self):
(r"/study/preprocess", PreprocessHandler),
(r"/study/process", ProcessHandler),
(r"/study/sharing/", ShareStudyAJAX),
(r"/study/description/(.*)", StudyDescriptionHandler),
# ORDER FOR /study/description/ SUBPAGES HERE MATTERS.
# Same reasoning as below. /study/description/(.*) should be last.
(r"/study/description/baseinfo/", StudyBaseInfoAJAX),
(r"/study/description/(.*)", StudyIndexHandler),
(r"/study/upload/(.*)", StudyUploadFileHandler),
(r"/upload/", UploadFileHandler),
(r"/check_study/", CreateStudyAJAX),
Expand Down