Delete study #1068
Delete study #1068
Conversation
|
This was an installation problem, not a problem with the PR. |
qiita_db/study.py
Outdated
There was a problem hiding this comment.
It took me a while to understand this comment, would you mind rephrasing it:
_id -> id_
== -> means
|
I think this looks good and all the comments are rather minor, can you add either a close or cancel button to the modal view? |
|
None of the modals have a close/cancel button ...
|
|
That's not true, the "create analysis" modal has a |
|
I can add and we should add to all of them, including those to create
meta-analysis.
|
|
Thanks, aside from those comments, the functionality seems to work just fine! Thanks! |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
See #1078. For the time being, I added a instantiation of Study(id_) to validate this case and a test just to be sure.
|
Few comments. Some of them could have raised a higher problem on the DB and we should check those issues before moving forward... |
There was a problem hiding this comment.
Can you use cls._check_id(id_) instead? Thanks.
There was a problem hiding this comment.
There was a problem hiding this comment.
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.
|
2 more comments @antgonza |
4 participants
No description provided.