Skip to content

[$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

Closed
maxceem opened this issue Feb 4, 2021 · 5 comments
Closed

[$100] Refactor soft-deleting #132

maxceem opened this issue Feb 4, 2021 · 5 comments

Comments

@maxceem
Copy link
Contributor

maxceem commented Feb 4, 2021

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 with deletedAt 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:

  • enable paranoid mode for each model by adding paranoid: true, deletedAt: 'deletedAt' to each model.
  • remove all the custom logic to filter deleted records like this { deletedAt: null } https://github.com/topcoder-platform/taas-apis/blob/dev/src/services/JobService.js#L414-L416
  • at the moment we have to manually exclude deletedAt when we are calling findAll or findOne, can this be done automatically when paranoind mode is enabled? We should still have a way to retrieve deletedAt if we disable paranoid mode in findAll or findOne
  • update logic for deleting records like this one https://github.com/topcoder-platform/taas-apis/blob/dev/src/services/JobService.js#L283. Instead of setting deletedAt we have to use method destroy. When paranoid is enabled it suppose to set deletedAt automatically
  • is there anything else what we should fix to make soft-deleting work automatically?
  • at the end, all the equalize methods which we are using like findAll,findOne and custom findById should not return deleted records automatically by utilizing paranoid mode.
@maxceem maxceem added enhancement New feature or request tech debt and removed enhancement New feature or request labels Feb 4, 2021
@maxceem maxceem changed the title Refactor soft-deleting [$100] Refactor soft-deleting Feb 6, 2021
@maxceem
Copy link
Contributor Author

maxceem commented Feb 6, 2021

Challenge https://www.topcoder.com/challenges/4edcc446-dd6d-4d78-80fe-1dd239b8e6f7 has been created for this ticket.

This is an automated message for maxceem via Topcoder X

@maxceem
Copy link
Contributor Author

maxceem commented Feb 6, 2021

@imcaizheng this is open for pickup.

Could you, please, verify that after this is implemented there are no bad side-efects, in particular:

  • deletedAt is automatically NOT returned by API
  • deletedAt is not indexed by ES processor and not indexed by index:* commands
  • deleted records are not indexed and not returned
  • deleted records are not exported by npm run data:export command

Also, if see any other possible drawbacks, please, let me know.

@maxceem
Copy link
Contributor Author

maxceem commented Feb 6, 2021

Challenge https://www.topcoder.com/challenges/4edcc446-dd6d-4d78-80fe-1dd239b8e6f7 has been assigned to aaron2017.

This is an automated message for maxceem via Topcoder X

@imcaizheng
Copy link
Contributor

@maxceem Will let you know when it is not going to be a perfect implementation.

@maxceem
Copy link
Contributor Author

maxceem commented Feb 9, 2021

Payment task has been updated: https://www.topcoder.com/challenges/4edcc446-dd6d-4d78-80fe-1dd239b8e6f7
Payments Complete
Winner: aaron2017
Copilot: maxceem
Challenge 4edcc446-dd6d-4d78-80fe-1dd239b8e6f7 has been paid and closed.

This is an automated message for maxceem via Topcoder X

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants