Skip to content
63 changes: 63 additions & 0 deletions qiita_db/study.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@
from .util import (check_required_columns, check_table_cols, convert_to_id,
get_environmental_packages, get_table_cols, infer_status)
from .sql_connection import SQLConnectionHandler
from .util import exists_table


class Study(QiitaObject):
Expand Down Expand Up @@ -347,6 +348,68 @@ def create(cls, owner, title, efo, info, investigation=None):

return cls(study_id)

@classmethod
def delete(cls, id_):
r"""Deletes the study from the database

Parameters
----------
id_ : integer
The object identifier

Raises
------
QiitaDBError
If the sample_(id_) table exists means a sample template exists
"""
cls._check_subclass()

Copy link
Contributor

Choose a reason for hiding this comment

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

Need to check if the study with id 'id_' exists.

Off topic: This is another reason why I think delete should be an instance method.

Copy link
Member Author

Choose a reason for hiding this comment

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

See #1078. For the time being, I added a instantiation of Study(id_) to validate this case and a test just to be sure.

# checking that the id_ exists
cls(id_)
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 use cls._check_id(id_) instead? Thanks.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

No, but you are only instantiating the object to check if the id exists. I think that is unnecessary given that we have the function _check_id. i.e. _check_id is called in __init__ (among other things) so you avoid all these other things by using the function that you really want.

Copy link
Contributor

Choose a reason for hiding this comment

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

@antgonza and I discussed this offline. The _check_id method is not a classmethod so it cannot be called here.

As a note, I think this brings up the discussion I started in #1045, so we can avoid all these checks...


conn_handler = SQLConnectionHandler()
if exists_table('sample_%d' % id_, conn_handler):
Copy link
Contributor

Choose a reason for hiding this comment

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

A question about the future. Is a user going to be able to add raw data, preprocessed data or processed data (otu tables) w/o a sample template? If so, we should check if the study has something else...

Copy link
Member Author

Choose a reason for hiding this comment

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

The way it works right now is that you need a raw data to have a preprocessed data, a preprocessed data to have a processed data, which I think is fine and give us the possibility of "play" with the options. For example, when we allow to upload a process data we can link it to the same default raw data for faster searches.

raise QiitaDBError('Study "%s" cannot be erased because it has a '
'sample template' % cls(id_).title)

queue = "delete_study_%d" % id_
conn_handler.create_queue(queue)

conn_handler.add_to_queue(
queue,
"DELETE FROM qiita.study_sample_columns WHERE study_id = %s",
(id_, ))

conn_handler.add_to_queue(
queue,
"DELETE FROM qiita.study_experimental_factor WHERE study_id = %s",
(id_, ))

conn_handler.add_to_queue(
queue,
"DELETE FROM qiita.study_pmid WHERE study_id = %s", (id_, ))

conn_handler.add_to_queue(
queue,
"DELETE FROM qiita.study_environmental_package WHERE study_id = "
"%s", (id_, ))

conn_handler.add_to_queue(
queue,
"DELETE FROM qiita.study_users WHERE study_id = %s", (id_, ))

conn_handler.add_to_queue(
queue,
"DELETE FROM qiita.investigation_study WHERE study_id = "
"%s", (id_, ))

conn_handler.add_to_queue(
queue,
"DELETE FROM qiita.study WHERE study_id = %s", (id_, ))

conn_handler.execute_queue(queue)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm surprised that this is actually working. It should fail since you are not removing the row from the study_users and investigation_study table. Does this mean that we are missing a FK on these tables?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added #1079

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for adding the issue. However you still should remove those rows here, otherwise the DB is going to be in an inconsistent state.



# --- Attributes ---
@property
def title(self):
Expand Down
16 changes: 15 additions & 1 deletion qiita_db/test/test_study.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@
from qiita_db.user import User
from qiita_db.data import RawData
from qiita_db.util import convert_to_id
from qiita_db.exceptions import QiitaDBColumnError, QiitaDBStatusError
from qiita_db.exceptions import (
QiitaDBColumnError, QiitaDBStatusError, QiitaDBError,
QiitaDBUnknownIDError)

# -----------------------------------------------------------------------------
# Copyright (c) 2014--, The Qiita Development Team.
Expand Down Expand Up @@ -438,6 +440,18 @@ def test_create_unknown_db_col(self):
Study.create(User('test@foo.bar'), "Fried Chicken Microbiome",
[1], self.info)

def test_delete(self):
title = "Fried chicken microbiome"
study = Study.create(User('test@foo.bar'), title, [1], self.info)
study.delete(study.id)
self.assertFalse(study.exists(title))

with self.assertRaises(QiitaDBError):
Study.delete(1)

with self.assertRaises(QiitaDBUnknownIDError):
Study.delete(41)

def test_retrieve_title(self):
self.assertEqual(self.study.title, 'Identification of the Microbiomes'
' for Cannabis Soils')
Expand Down
35 changes: 35 additions & 0 deletions qiita_pet/handlers/study_handlers/description_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
QiitaDBDuplicateHeaderError, QiitaDBError)
from qiita_pet.handlers.base_handlers import BaseHandler
from qiita_pet.handlers.util import check_access
from qiita_pet.handlers.study_handlers.listing_handlers import (
ListStudiesHandler)

html_error_message = "<b>An error occurred %s %s</b></br>%s"

Expand Down Expand Up @@ -640,6 +642,38 @@ def display_template(self, study, user, msg, msg_level, full_access,
sub_tab=sub_tab,
prep_tab=prep_tab)

def delete_study(self, study, user, callback):
"""Delete study

Parameters
----------
study : Study
The current study object
user : User
The current user object
callback : function
The callback function to call with the results once the processing
is done and it fails
"""
study_id = study.id
study_title = study.title

try:
Study.delete(study_id)

# redirecting to list but also passing messages
# we need to change the request.method to GET
self.request.method = 'GET'
ListStudiesHandler(self.application, self.request)._execute(
[t(self.request) for t in self.application.transforms],
message=('Study "%s" has been deleted' % study_title),
msg_level='success')
except Exception as e:
msg = "Couldn't remove study %d: %s" % (study_id, str(e))
msg_level = "danger"

callback((msg, msg_level, 'study_information_tab', None, None))

def delete_sample_template(self, study, user, callback):
"""Delete sample template

Expand Down Expand Up @@ -808,6 +842,7 @@ def post(self, study_id):
request_approval=self.request_approval,
make_sandbox=self.make_sandbox,
update_investigation_type=self.update_investigation_type,
delete_study=self.delete_study,
delete_sample_template=self.delete_sample_template,
delete_raw_data=self.delete_raw_data,
delete_prep_template=self.delete_prep_template,
Expand Down
9 changes: 6 additions & 3 deletions qiita_pet/handlers/study_handlers/listing_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,13 +123,16 @@ def _check_owner(user, study):
class ListStudiesHandler(BaseHandler):
@authenticated
@coroutine
def get(self):
def get(self, message="", msg_level=None):
all_emails_except_current = yield Task(self._get_all_emails)
all_emails_except_current.remove(self.current_user.id)
avail_meta = SampleTemplate.metadata_headers() +\
get_table_cols("study")
self.render('list_studies.html', availmeta=avail_meta,
all_emails_except_current=all_emails_except_current)
self.render('list_studies.html',
availmeta=avail_meta,
all_emails_except_current=all_emails_except_current,
message=message,
msg_level=msg_level)

def _get_all_emails(self, callback):
callback(list(User.iter()))
Expand Down
51 changes: 51 additions & 0 deletions qiita_pet/templates/study_description.html
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,36 @@
form.submit();
}

function validate_delete_study_text() {
if ($("#study-alias").val() == "{{study_alias}}") {
$('#delete-study-button').prop('disabled', false);
} else {
$('#delete-study-button').prop('disabled', true);
}
}

function delete_study() {
if ($("#study-alias").val() != "{{study_alias}}") {
alert("The added name doesn't match the study alias");
return false;
}
if (confirm('Are you sure you want to delete "{{study_title}}"?')) {
var form = $("<form>")
.attr("action", window.location.href)
.attr("method", "post")
.append($("<input>")
.attr("type", "hidden")
.attr("name", "study_id")
.attr("value", {{study.id}}))
.append($("<input>")
.attr("type", "hidden")
.attr("name", "action")
.attr("value", "delete_study"));
$("body").append(form);
form.submit();
}
}

function delete_sample_template() {
sample_template_id = {{study.sample_template}};
if (confirm('Are you sure you want to delete sample template ID: ' + sample_template_id + '?')) {
Expand Down Expand Up @@ -548,6 +578,7 @@ <h2><i>{{study_alias}}</i></h2>
<a class="btn btn-default glyphicon glyphicon-edit" href="/study/edit/{{study.id}}" title="Edit the study information" style="margin:5px; word-spacing: -10px;"> Edit</a>
{% end %}
<a href="/study/upload/{{study.id}}" class="btn btn-default glyphicon glyphicon-upload" title="Upload study files" style="margin:5px; word-spacing: -10px;"> Upload</a>
<a class="btn btn-danger glyphicon glyphicon-trash" style="display: inline-block;" data-toggle="modal" data-target="#delete-study"> Delete-study</a>
</td>
</tr>
</table>
Expand Down Expand Up @@ -580,4 +611,24 @@ <h2><i>{{study_alias}}</i></h2>
{% end %}
</div>


<div class="modal fade delete-study" tabindex="-1" role="dialog" id="delete-study">
<div class="modal-dialog modal-sm">
<div class="modal-content">
<div class="modal-header">
<button type="button" class="close" data-dismiss="modal" aria-hidden="true">&times;</button>
<h3>Deleting:<br/></h3><h4>{{study_title}}</h4>
</div>
<div class="modal-body">
Copy link
Contributor

Choose a reason for hiding this comment

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

And if you follow the comment above, this should be moved to the "modal-footer" section.

Copy link
Member Author

Choose a reason for hiding this comment

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

I tested your 2 suggestions and I do not like them.

You will only be able to delete a study that has no data associated with it.
</div>
<div class="modal-footer">
To continue you need to write the alias name of the study:
<input type="text" name="study-alias" id="study-alias" onkeyup="validate_delete_study_text();">
<button class="btn btn-danger glyphicon glyphicon-trash" onclick="delete_study();" id="delete-study-button" disabled></button>
</div>
</div>
</div>
</div>

{% end %}
9 changes: 9 additions & 0 deletions qiita_pet/test/test_study_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,15 @@ class TestEBISubmitHandler(TestHandlerBase):
class TestDelete(TestHandlerBase):
database = True

def test_delete_study(self):
response = self.post('/study/description/1',
{'study_id': 1,
'action': 'delete_study'})
self.assertEqual(response.code, 200)

# checking that the action was sent
self.assertIn("Couldn't remove study", response.body)

def test_delete_sample_template(self):
response = self.post('/study/description/1',
{'sample_template_id': 1,
Expand Down