-
Notifications
You must be signed in to change notification settings - Fork 290
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
Functionality to delete slide from file system #341
Conversation
@Vedant1202 , Chances are there that due to some reason, the data is not deleted from DB, but the image file gets deleted, with this approach. It would be better that the file is deleted using the node backend only (Caracal repository) and only when the delete from MongoDB database is successful. I have made this feature in PR-23 of Caracal repository. |
@cjchirag7, I've already considered this issue and have chained the processes to each other by promises. So, unless one promise is handled successfully, the other does'nt get executed. I've looked at your PR and there are some serious issues in it please look into it here. |
Also, putting all file access code in SlideLoader is much better just to make sure all related code stays in one place. |
@Vedant1202 , yeah I didn't notice you're chaining the promises correctly. But your PR #25 has some major problems with the route security, which I have commented on the PR. These should be looked into, carefully. |
@cjchirag7 I did the updates in PR-25. You can have a look. |
I think this looks good; just will have to wait to co-test this with camicroscope/SlideLoader#25 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's clear that a lot of work has gone into this, thank you very much for your time and effort! Approved, pending backend change.
This fix contains:
This fix solves #324 .