Skip to content

delete sample template #1059

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

Merged
merged 9 commits into from
Apr 15, 2015
Merged
Show file tree
Hide file tree
Changes from all 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
37 changes: 0 additions & 37 deletions qiita_db/metadata_template/base_metadata_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -772,43 +772,6 @@ def _add_common_creation_steps_to_queue(cls, md_template, obj_id,
', '.join(["%s"] * len(headers))),
values, many=True)

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

Parameters
----------
id_ : obj
The object identifier

Raises
------
QiitaDBUnknownIDError
If no metadata_template with id id_ exists
"""
cls._check_subclass()
if not cls.exists(id_):
raise QiitaDBUnknownIDError(id_, cls.__name__)

table_name = cls._table_name(id_)
conn_handler = SQLConnectionHandler()

# Delete the sample template filepaths
conn_handler.execute(
"DELETE FROM qiita.sample_template_filepath WHERE "
"study_id = %s", (id_, ))

conn_handler.execute(
"DROP TABLE qiita.{0}".format(table_name))
conn_handler.execute(
"DELETE FROM qiita.{0} where {1} = %s".format(cls._table,
cls._id_column),
(id_,))
conn_handler.execute(
"DELETE FROM qiita.{0} where {1} = %s".format(cls._column_table,
cls._id_column),
(id_,))

@classmethod
def exists(cls, obj_id):
r"""Checks if already exists a MetadataTemplate for the provided object
Expand Down
59 changes: 58 additions & 1 deletion qiita_db/metadata_template/sample_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

from qiita_core.exceptions import IncompetentQiitaDeveloperError
from qiita_db.exceptions import (QiitaDBDuplicateError, QiitaDBError,
QiitaDBWarning)
QiitaDBWarning, QiitaDBUnknownIDError)
from qiita_db.sql_connection import SQLConnectionHandler
from qiita_db.util import (get_table_cols, get_required_sample_info_status,
get_mountpoint, scrub_data)
Expand Down Expand Up @@ -152,6 +152,63 @@ def create(cls, md_template, study):

return st

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

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

Raises
------
QiitaDBUnknownIDError
If no sample template with id id_ exists
QiitaDBError
If the study that owns this sample template has raw datas
"""
cls._check_subclass()

if not cls.exists(id_):
raise QiitaDBUnknownIDError(id_, cls.__name__)

raw_datas = [str(rd) for rd in Study(cls(id_).study_id).raw_data()]
if raw_datas:
raise QiitaDBError("Sample template can not be erased because "
"there are raw datas (%s) associated." %
', '.join(raw_datas))

table_name = cls._table_name(id_)
conn_handler = SQLConnectionHandler()

# Delete the sample template filepaths
queue = "delete_sample_template_%d" % id_
conn_handler.create_queue(queue)

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

conn_handler.add_to_queue(
queue,
"DROP TABLE qiita.{0}".format(table_name))

conn_handler.add_to_queue(
queue,
"DELETE FROM qiita.{0} where {1} = %s".format(cls._table,
cls._id_column),
(id_,))

conn_handler.add_to_queue(
queue,
"DELETE FROM qiita.{0} where {1} = %s".format(cls._column_table,
cls._id_column),
(id_,))

conn_handler.execute_queue(queue)

@property
def study_id(self):
"""Gets the study id with which this sample template is associated
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

from unittest import TestCase, main

from qiita_core.util import qiita_test_checker
from qiita_core.exceptions import IncompetentQiitaDeveloperError
from qiita_db.study import Study
from qiita_db.metadata_template.base_metadata_template import (
Expand Down Expand Up @@ -64,12 +63,5 @@ def test_clean_validate_template(self):
MetadataTemplate._clean_validate_template(None, 1, None, None)


@qiita_test_checker()
class TestMetadataTemplateReadWrite(TestCase):
def test_delete(self):
"""delete raises an error because it's not called from a subclass"""
with self.assertRaises(IncompetentQiitaDeveloperError):
MetadataTemplate.delete(1)

if __name__ == '__main__':
main()
10 changes: 8 additions & 2 deletions qiita_db/metadata_template/test/test_sample_template.py
Original file line number Diff line number Diff line change
Expand Up @@ -1229,20 +1229,26 @@ def test_create_already_prefixed_samples(self):

def test_delete(self):
"""Deletes Sample template 1"""
SampleTemplate.create(self.metadata, self.new_study)
SampleTemplate.delete(2)
st = SampleTemplate.create(self.metadata, self.new_study)
SampleTemplate.delete(st.id)

obs = self.conn_handler.execute_fetchall(
"SELECT * FROM qiita.required_sample_info WHERE study_id=2")
exp = []
self.assertEqual(obs, exp)

obs = self.conn_handler.execute_fetchall(
"SELECT * FROM qiita.study_sample_columns WHERE study_id=2")
exp = []
self.assertEqual(obs, exp)

with self.assertRaises(QiitaDBExecutionError):
self.conn_handler.execute_fetchall(
"SELECT * FROM qiita.sample_2")

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

def test_delete_unkonwn_id_error(self):
"""Try to delete a non existent prep template"""
with self.assertRaises(QiitaDBUnknownIDError):
Expand Down
28 changes: 28 additions & 0 deletions qiita_pet/handlers/study_handlers/description_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,33 @@ def display_template(self, study, user, msg, msg_level, full_access,
sub_tab=sub_tab,
prep_tab=prep_tab)

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

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
"""
sample_template_id = int(self.get_argument('sample_template_id'))

try:
SampleTemplate.delete(sample_template_id)
msg = ("Sample template %d has been deleted from study: "
"<b><i>%s</i></b>" % (sample_template_id, study.title))
msg_level = "success"
except Exception as e:
msg = "Couldn't remove %d sample template: %s" % (
sample_template_id, str(e))
msg_level = "danger"

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

def delete_raw_data(self, study, user, callback):
"""Delete the selected raw data

Expand Down Expand Up @@ -781,6 +808,7 @@ def post(self, study_id):
request_approval=self.request_approval,
make_sandbox=self.make_sandbox,
update_investigation_type=self.update_investigation_type,
delete_sample_template=self.delete_sample_template,
delete_raw_data=self.delete_raw_data,
delete_prep_template=self.delete_prep_template,
delete_preprocessed_data=self.delete_preprocessed_data,
Expand Down
23 changes: 22 additions & 1 deletion qiita_pet/templates/study_description.html
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,25 @@
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 + '?')) {
var form = $("<form>")
.attr("action", window.location.href)
.attr("method", "post")
.append($("<input>")
.attr("type", "hidden")
.attr("name", "sample_template_id")
.attr("value", sample_template_id))
.append($("<input>")
.attr("type", "hidden")
.attr("name", "action")
.attr("value", "delete_sample_template"));
$("body").append(form);
form.submit();
}
}

function delete_raw_data(raw_data_filetype, raw_data_id) {
if (confirm('Are you sure you want to delete raw data: ' + raw_data_filetype + ' (ID: ' + raw_data_id + ')?')) {
var form = $("<form>")
Expand Down Expand Up @@ -537,7 +556,9 @@ <h2><i>{{study_alias}}</i></h2>

<!-- Nav tabs -->
<ul class="nav nav-tabs" role="tablist" id="myTab">
<li class="active"><a href="#study_information_tab" role="tab" data-toggle="tab">Study information</a></li>
<li class="active">
<a href="#study_information_tab" role="tab" data-toggle="tab">Study information</a>
</li>
{% if show_data_tabs %}
<li><a href="#raw_data_tab" role="tab" data-toggle="tab">Raw data</a></li>
<li><a href="#preprocessed_data_tab" role="tab" data-toggle="tab">Preprocessed data</a></li>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,12 @@

<table>
<tr>
{% if not sample_templates %}
<td>
<a class="btn btn-primary" onclick="process_sample_template();">Process sample template</a>
</td>
<td>&nbsp; &nbsp;</td>
{% end %}
{% if sample_templates %}
<td>
<a class="btn btn-primary" onclick="update_sample_template();">Update sample template</a>
Expand All @@ -55,6 +57,10 @@
<td>
<a class="btn btn-primary" onclick="extend_sample_template();">Add samples to study</a>
</td>
<td>&nbsp; &nbsp;</td>
<td>
<a class="btn btn-danger glyphicon glyphicon-trash" onclick="delete_sample_template();"></a>
</td>
{% end %}
</table>
</div>
54 changes: 54 additions & 0 deletions qiita_pet/test/test_study_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -382,5 +382,59 @@ class TestEBISubmitHandler(TestHandlerBase):
pass


class TestDelete(TestHandlerBase):
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 tests should include database = True; since they can potentially modify the database, in case of malfunction; making a lot of tests to fail and harder to find the original problem.

database = True

def test_delete_sample_template(self):
response = self.post('/study/description/1',
{'sample_template_id': 1,
'action': 'delete_sample_template'})
self.assertEqual(response.code, 200)
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 also make sure that the sample template is actually deleted here? Same goes for all other delete tests. Just want to make sure the functionality is actually happening along with the page loading fine.

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.

Oh no, this isn't to make sure the delete function works, just that the delete page actually calls delete and the items are actually deleted when you hit the buttons on the page.

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.

Just because the delete function works in qiita_db, doesn't mean the call works in qiita_pet. The delete function may change, breaking qiita_pet, or some other thing may change. If you test in qiita pet that the deletion actually happens, you know the functionality always works. As an example, in test_delete_raw_data, you can make sure that the QiitaDBIDError is raised when instantiating RawData(1) to make sure it was deleted by the page call.

Copy link
Contributor

Choose a reason for hiding this comment

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

Concrete example here Where the database change called for in the page is explicitly tested that it happens.


# checking that the action was sent
self.assertIn("Sample template can not be erased because there are "
"raw datas", response.body)

def test_delete_raw_data(self):
response = self.post('/study/description/1',
{'raw_data_id': 1,
'action': 'delete_raw_data'})
self.assertEqual(response.code, 200)

# checking that the action was sent
self.assertIn("Raw data 1 has prep template(s) associated so it can't "
"be erased", response.body)

def test_delete_prep_template(self):
response = self.post('/study/description/1',
{'prep_template_id': 1,
'action': 'delete_prep_template'})
self.assertEqual(response.code, 200)

# checking that the action was sent
self.assertIn('Cannot remove prep template 1 because a preprocessed '
'data has been already generated using it.',
response.body)

def test_delete_preprocessed_data(self):
response = self.post('/study/description/1',
{'preprocessed_data_id': 1,
'action': 'delete_preprocessed_data'})
self.assertEqual(response.code, 200)

# checking that the action was sent
self.assertIn('Illegal operation on non sandboxed preprocessed data',
response.body)

def test_delete_processed_data(self):
response = self.post('/study/description/1',
{'processed_data_id': 1,
'action': 'delete_processed_data'})
self.assertEqual(response.code, 200)

# checking that the action was sent
self.assertIn('Illegal operation on non sandboxed processed data',
response.body)

if __name__ == "__main__":
main()