-
Notifications
You must be signed in to change notification settings - Fork 33
[$100] Refactor soft-deleting #132
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
Comments
Challenge https://www.topcoder.com/challenges/4edcc446-dd6d-4d78-80fe-1dd239b8e6f7 has been created for this ticket. |
@imcaizheng this is open for pickup. Could you, please, verify that after this is implemented there are no bad side-efects, in particular:
Also, if see any other possible drawbacks, please, let me know. |
Challenge https://www.topcoder.com/challenges/4edcc446-dd6d-4d78-80fe-1dd239b8e6f7 has been assigned to aaron2017. |
@maxceem Will let you know when it is not going to be a perfect implementation. |
Payment task has been updated: https://www.topcoder.com/challenges/4edcc446-dd6d-4d78-80fe-1dd239b8e6f7 |
We are using soft-deleting in the code. So when a record is deleted we don't delete it for real, but just set
deletedAt
and later we treat records withdeletedAt
not null as deleted and should never return it.sequalize
already has support for this feature automatically called paranoid mode. But in our code base we are re-implemented the same functionality manually. So we have to always remember about it, and should always manually exclude deleted records in each place where we make requests. This is very easy to forget, and in fact we already had such issue when during implementing script for indexing data from ES to DB the member didn't notice it and indexed deleted records. See how it was fixed after.We have to reimplement soft-deleting by utilizing paranoid mode of the equalize and remove our custom logic. Some steps to do which I can see:
paranoid
mode for each model by addingparanoid: true, deletedAt: 'deletedAt'
to each model.{ deletedAt: null }
https://github.com/topcoder-platform/taas-apis/blob/dev/src/services/JobService.js#L414-L416deletedAt
when we are callingfindAll
orfindOne
, can this be done automatically whenparanoind
mode is enabled? We should still have a way to retrievedeletedAt
if we disable paranoid mode infindAll
orfindOne
deletedAt
we have to use methoddestroy
. When paranoid is enabled it suppose to setdeletedAt
automaticallyfindAll
,findOne
and customfindById
should not return deleted records automatically by utilizingparanoid mode
.The text was updated successfully, but these errors were encountered: