Skip to content
Closed
12 changes: 4 additions & 8 deletions qiita_db/analysis.py
Original file line number Diff line number Diff line change
Expand Up @@ -683,13 +683,11 @@ def share(self, user):
user: User object
The user to share the analysis with
"""
with qdb.sql_connection.TRN:
self._lock_check()

# Make sure the analysis is not already shared with the given user
if user.id in self.shared_with:
return
# Make sure the analysis is not already shared with the given user
if user.id == self.owner or user.id in self.shared_with:
return

with qdb.sql_connection.TRN:
sql = """INSERT INTO qiita.analysis_users (analysis_id, email)
VALUES (%s, %s)"""
qdb.sql_connection.TRN.add(sql, [self._id, user.id])
Expand All @@ -704,8 +702,6 @@ def unshare(self, user):
The user to unshare the analysis with
"""
with qdb.sql_connection.TRN:
self._lock_check()

sql = """DELETE FROM qiita.analysis_users
WHERE analysis_id = %s AND email = %s"""
qdb.sql_connection.TRN.add(sql, [self._id, user.id])
Expand Down
2 changes: 1 addition & 1 deletion qiita_db/support_files/populate_test_db.sql
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,7 @@ INSERT INTO qiita.job (data_type_id, job_status_id, command_id, options, input_f
INSERT INTO qiita.job_results_filepath (job_id, filepath_id) VALUES (1, 13), (2, 14);

-- Insert Analysis
INSERT INTO qiita.analysis (email, name, description, analysis_status_id, pmid) VALUES ('test@foo.bar', 'SomeAnalysis', 'A test analysis', 1, '121112'), ('test@foo.bar', 'SomeSecondAnalysis', 'Another test analysis', 1, '22221112');
INSERT INTO qiita.analysis (email, name, description, analysis_status_id, pmid) VALUES ('test@foo.bar', 'SomeAnalysis', 'A test analysis', 1, '121112'), ('admin@foo.bar', 'SomeSecondAnalysis', 'Another test analysis', 1, '22221112');
INSERT INTO qiita.analysis_portal (analysis_id, portal_type_id) VALUES (1, 1), (2, 1);
-- Insert Analysis Workflow
INSERT INTO qiita.analysis_workflow (analysis_id, step) VALUES (1, 3), (2, 3);
Expand Down
2 changes: 1 addition & 1 deletion qiita_db/test/test_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ def test_get_shared_studies(self):
def test_get_private_analyses(self):
user = qdb.user.User('test@foo.bar')
qiita_config.portal = "QIITA"
exp = {qdb.analysis.Analysis(1), qdb.analysis.Analysis(2)}
exp = {qdb.analysis.Analysis(1)}
self.assertEqual(user.private_analyses, exp)

qiita_config.portal = "EMP"
Expand Down
58 changes: 55 additions & 3 deletions qiita_pet/handlers/analysis_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,19 +17,22 @@
from functools import partial

from tornado.web import authenticated, HTTPError, StaticFileHandler
from tornado.gen import coroutine, Task
from moi import ctx_default, r_client
from moi.job import submit
from moi.group import get_id_from_user, create_info

from qiita_pet.util import is_localhost
from qiita_pet.handlers.base_handlers import BaseHandler
from qiita_pet.handlers.util import download_link_or_path
from qiita_pet.handlers.util import (
download_link_or_path, get_shared_links)
from qiita_pet.exceptions import QiitaPetAuthorizationError
from qiita_ware.dispatchable import run_analysis
from qiita_db.analysis import Analysis
from qiita_db.artifact import Artifact
from qiita_db.job import Command
from qiita_db.util import (get_db_files_base_dir,
from qiita_db.user import User
from qiita_db.util import (get_db_files_base_dir, add_message,
check_access_to_analysis_result,
filepath_ids_to_rel_paths, get_filepath_id)
from qiita_db.exceptions import QiitaDBUnknownIDError
Expand Down Expand Up @@ -172,10 +175,13 @@ def get(self, analysis_id):
data_type = proc_data.data_type
dropped[data_type].append((proc_data.study.title, len(samples),
', '.join(samples)))
share_access = (self.current_user.id in analysis.shared_with or
self.current_user.id == analysis.owner)

self.render("analysis_results.html", analysis_id=analysis_id,
jobres=jobres, aname=analysis.name, dropped=dropped,
basefolder=get_db_files_base_dir())
basefolder=get_db_files_base_dir(),
share_access=share_access)

@authenticated
@execute_as_transaction
Expand Down Expand Up @@ -349,3 +355,49 @@ class AnalysisSummaryAJAX(BaseHandler):
def get(self):
info = self.current_user.default_analysis.summary_data()
self.write(dumps(info))


class ShareAnalysisAJAX(BaseHandler):
@execute_as_transaction
def _get_shared_for_study(self, analysis, callback):
shared_links = get_shared_links(analysis)
users = [u.email for u in analysis.shared_with]
callback((users, shared_links))

@execute_as_transaction
def _share(self, analysis, user, callback):
user = User(user)
add_message('Analysis <a href="/analysis/results/%d">\'%s\'</a> '
'has been shared with you.' %
(analysis.id, analysis.name), [user])
callback(analysis.share(user))

@execute_as_transaction
def _unshare(self, analysis, user, callback):
user = User(user)
add_message('Analysis \'%s\' has been unshared from you.' %
analysis.name, [user])
callback(analysis.unshare(user))

@authenticated
@coroutine
@execute_as_transaction
def get(self):
analysis_id = int(self.get_argument('id'))
analysis = Analysis(analysis_id)
if self.current_user != analysis.owner:
raise HTTPError(403, 'User %s does not have permissions to share '
'analysis %s' % (
self.current_user.id, analysis.id))

selected = self.get_argument('selected', None)
deselected = self.get_argument('deselected', None)

if selected is not None:
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens if both are None?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No share or unshare actions, but the user email links and user list is still returned. This is leveraged for initial population of the shared list in the page.

yield Task(self._share, analysis, selected)
if deselected is not None:
yield Task(self._unshare, analysis, deselected)

users, links = yield Task(self._get_shared_for_study, analysis)

self.write(dumps({'users': users, 'links': links}))
32 changes: 11 additions & 21 deletions qiita_pet/handlers/study_handlers/listing_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,28 +23,13 @@
from qiita_db.logger import LogEntry
from qiita_db.exceptions import QiitaDBIncompatibleDatatypeError
from qiita_db.reference import Reference
from qiita_db.util import get_pubmed_ids_from_dois
from qiita_db.util import get_pubmed_ids_from_dois, add_message
from qiita_core.exceptions import IncompetentQiitaDeveloperError
from qiita_core.util import execute_as_transaction
from qiita_pet.handlers.base_handlers import BaseHandler
from qiita_pet.handlers.util import (
study_person_linkifier, doi_linkifier, pubmed_linkifier, check_access)


@execute_as_transaction
def _get_shared_links_for_study(study):
shared = []
for person in study.shared_with:
name = person.info['name']
email = person.email
# Name is optional, so default to email if non existant
if name:
shared.append(study_person_linkifier(
(email, name)))
else:
shared.append(study_person_linkifier(
(email, email)))
return ", ".join(shared)
study_person_linkifier, doi_linkifier, pubmed_linkifier, check_access,
get_shared_links)


@execute_as_transaction
Expand Down Expand Up @@ -83,7 +68,7 @@ def _build_single_study_info(study, info, study_proc, proc_samples):
info['pmid'] = ""
if info["number_samples_collected"] is None:
info["number_samples_collected"] = 0
info["shared"] = _get_shared_links_for_study(study)
info["shared"] = get_shared_links(study)
# raw data is any artifact that is not Demultiplexed or BIOM

info["num_raw_data"] = len([a for a in study.artifacts()
Expand Down Expand Up @@ -258,25 +243,30 @@ def get(self):
class ShareStudyAJAX(BaseHandler):
@execute_as_transaction
def _get_shared_for_study(self, study, callback):
shared_links = _get_shared_links_for_study(study)
shared_links = get_shared_links(study)
users = [u.email for u in study.shared_with]
callback((users, shared_links))

@execute_as_transaction
def _share(self, study, user, callback):
user = User(user)
add_message('Study <a href="/study/description/%d">\'%s\'</a> '
'has been shared with you.' %
(study.id, study.title), [user])
callback(study.share(user))

@execute_as_transaction
def _unshare(self, study, user, callback):
user = User(user)
add_message('Study \'%s\' has been unshared from you.' %
study.title, [user])
callback(study.unshare(user))

@authenticated
@coroutine
@execute_as_transaction
def get(self):
study_id = int(self.get_argument('study_id'))
study_id = int(self.get_argument('id'))
study = Study(study_id)
check_access(self.current_user, study, no_public=True,
raise_error=True)
Expand Down
25 changes: 15 additions & 10 deletions qiita_pet/handlers/study_handlers/tests/test_listing_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@
from qiita_core.exceptions import IncompetentQiitaDeveloperError
from qiita_pet.test.tornado_test_base import TestHandlerBase
from qiita_pet.handlers.study_handlers.listing_handlers import (
_get_shared_links_for_study, _build_study_info, _build_single_study_info,
_build_single_proc_data_info)
_build_study_info, _build_single_study_info, _build_single_proc_data_info)
from qiita_pet.handlers.base_handlers import BaseHandler
from qiita_db.study import Study
from qiita_db.user import User
Expand Down Expand Up @@ -134,11 +133,6 @@ def setUp(self):
}
self.exp = [self.single_exp]

def test_get_shared_links_for_study(self):
obs = _get_shared_links_for_study(Study(1))
exp = '<a target="_blank" href="mailto:shared@foo.bar">Shared</a>'
self.assertEqual(obs, exp)

def test_build_single_study_info(self):
study_proc = {1: {'18S': [4, 5, 6]}}
proc_samples = {4: self.proc_data_exp[0]['samples'],
Expand Down Expand Up @@ -236,18 +230,23 @@ class TestShareStudyAjax(TestHandlerBase):
def test_get_deselected(self):
s = Study(1)
u = User('shared@foo.bar')
args = {'deselected': u.id, 'study_id': s.id}
args = {'deselected': u.id, 'id': s.id}
self.assertEqual(s.shared_with, [u])
response = self.get('/study/sharing/', args)
self.assertEqual(response.code, 200)
exp = {'users': [], 'links': ''}
self.assertEqual(loads(response.body), exp)
self.assertEqual(s.shared_with, [])

# Make sure unshared message added to the system
self.assertEqual('Study \'Identification of the Microbiomes for '
'Cannabis Soils\' has been unshared from you.',
u.messages()[0][1])

def test_get_selected(self):
s = Study(1)
u = User('admin@foo.bar')
args = {'selected': u.id, 'study_id': s.id}
args = {'selected': u.id, 'id': s.id}
response = self.get('/study/sharing/', args)
self.assertEqual(response.code, 200)
exp = {
Expand All @@ -258,6 +257,12 @@ def test_get_selected(self):
self.assertEqual(loads(response.body), exp)
self.assertEqual(s.shared_with, [User('shared@foo.bar'), u])

# Make sure shared message added to the system
self.assertEqual('Study <a href="/study/description/1">'
'\'Identification of the Microbiomes for Cannabis '
'Soils\'</a> has been shared with you.',
u.messages()[0][1])

def test_get_no_access(self):
# Create a new study belonging to the 'shared' user, so 'test' doesn't
# have access
Expand All @@ -274,7 +279,7 @@ def test_get_no_access(self):
s = Study.create(u, 'test_study', efo=[1], info=info)
self.assertEqual(s.shared_with, [])

args = {'selected': 'test@foo.bar', 'study_id': s.id}
args = {'selected': 'test@foo.bar', 'id': s.id}
response = self.get('/study/sharing/', args)
self.assertEqual(response.code, 403)
self.assertEqual(s.shared_with, [])
Expand Down
29 changes: 29 additions & 0 deletions qiita_pet/handlers/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,3 +89,32 @@ def to_int(value):
except ValueError:
raise HTTPError(400, "%s cannot be converted to an integer" % value)
return res


@execute_as_transaction
def get_shared_links(obj):
Copy link
Member

Choose a reason for hiding this comment

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

why rewrite this one vs. using the one for studies?

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 is the one from studies. listing_handlers.py has it removed and moved here, since this function is now used in more than just that file.

"""Creates email links for the users obj is shared with

Parameters
----------
obj : QiitaObject
A qiita object with a 'shared_with' property that returns a list of
User objects

Returns
-------
str
Email links for each person obj is shared with
"""
shared = []
for person in obj.shared_with:
name = person.info['name']
email = person.email
# Name is optional, so default to email if non existant
if name:
shared.append(study_person_linkifier(
(email, name)))
else:
shared.append(study_person_linkifier(
(email, email)))
return ", ".join(shared)
20 changes: 10 additions & 10 deletions qiita_pet/static/js/sharing.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
var current_study = null;
var current_id = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can the global variable be removed? Recommend taking a look at the data variables. They're pretty useful and they allowed me to remove all the global variables that was in the previous code.

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 was discussed with @ElDeveloper in the last PR and it is not feasible to do at this time.

Copy link
Contributor

Choose a reason for hiding this comment

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

All global variables are possible to remove if you want to - please check the link I sent above and remove them.


$(document).ready(function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually I just saw this - this is a library, why does it has a $(document).ready call? It seems weird that this is in the library and this may be the source of the difficulties that you where finding. If this is used to initialize the different parts of this library, it may be useful to actually have a sharing_init call that you call in the $(document).ready of the actual HTML pages that is using this library.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because you can have as many document.ready calls in a single page as needed, and this also standardizes naming and initialization of the sharing dropdown for use in the other helper functions.

$('#shares-select').select2({
Expand All @@ -18,18 +18,18 @@ $(document).ready(function () {
});

$('#shares-select').on("select2:select", function (e) {
update_share({selected: e.params.data.text});
update_share(e.target.classList[0], {selected: e.params.data.text});
});

$('#shares-select').on("select2:unselect", function (e) {
update_share({deselected: e.params.data.text});
update_share(e.target.classList[0], {deselected: e.params.data.text});
});
});

function modify_sharing(study_id) {
function modify_sharing(share_type, id) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the indentation in this function is messed up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

var shared_list;
current_study = study_id;
$.get('/study/sharing/', {study_id: study_id})
current_id = id;
$.get('/' + share_type + '/sharing/', {id: id})
.done(function(data) {
var users_links = JSON.parse(data);
var users = users_links.users;
Expand All @@ -43,13 +43,13 @@ $.get('/study/sharing/', {study_id: study_id})
});
}

function update_share(params) {
function update_share(share_type, params) {
data = params || {};
data.study_id = current_study;
$.get('/study/sharing/', data)
data.id = current_id;
$.get('/' + share_type + '/sharing/', data)
.done(function(data) {
users_links = JSON.parse(data);
links = users_links.links;
$("#shared_html_"+current_study).html(links);
$("#shared_html_"+current_id).html(links);
});
}
Loading