Delete analysis #1175
Delete analysis #1175
Conversation
There was a problem hiding this comment.
@antgonza there is still another table in which the analysis can be present: analysis_chain. Right now we are not using that table for anything, but we can easily forget about it on deletion once we start using it. There are 2 possible solutions:
- Open an issue, so we don't loose track of it
- We add that table here. This implies that you also need to add a check: a study cannot be deleted if it has 'child' analyses.
I'm fine with adding an issue, since currently we are not using that and a lot of other changes will be needed once that gets in place.
There was a problem hiding this comment.
I happy adding the check and the delete of those rows ... shouldn't be a problem.
There was a problem hiding this comment.
Decided to leave it as an issue #1176 so the solution is complete when implemented.
There was a problem hiding this comment.
Thanks @antgonza just one minor thing: can you add a comment with a reference to the issue? Like # TODO: issue #1176, so we have a pointer in the code and we don't forget about it.
|
Few comments @antgonza |
|
@antgonza I see some errors in your build. After a quick look, they don't look like it's related to your changes, but we will need to investigate more. I pulled down your code and it looks good and works awesome! |
|
@antgonza @josenavas seeing the same thing over in #1172. |
|
Thanks for pointing that out. I'm wondering if one of our dependencies changed something, so that's why we are seeing that behavior... |
|
Ready for another review ... |
|
👍 |
|
One comment, but looks good otherwise |
4 participants
Adding the possibility of deleting analysis. The first step to be able to delete an analysis is to make sure it exists, thus also adding the exist function to the Analysis object.