Skip to content
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

Add SoftDeleteModel, ActiveUndeleteManager; tests for same #130

Closed
wants to merge 1 commit into from

Conversation

jkachel
Copy link
Contributor

@jkachel jkachel commented Jan 8, 2024

What are the relevant tickets?

Came up in mitodl/unified-ecommerce#31

Description (What does it do?)

Adds a SoftDeleteModel to the common library, so we can have soft deletions in a common spot. Adds a manager (ActiveUndeleteManager) so models that use the SoftDeleteModel can also optionally default to hiding items that have been deleted.

How can this be tested?

Automated tests should pass. Your model can inherit from SoftDeleteModel, and that should add an is_active flag to the model and override the delete function to set the flag to False. Your model can optionally also use the ActiveUndeleteManager so that items that are deleted (is_active == False) are excluded from the queryset by default.

@jkachel jkachel force-pushed the jkachel/move-soft-undelete-models-to-common branch from d241105 to 458bd9e Compare January 8, 2024 17:48
@mbertrand mbertrand self-assigned this Jan 9, 2024
Copy link
Member

@mbertrand mbertrand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good at first glance, but in ocw-studio a 3rd-party library was used to add this functionality (django-safedelete). What do you think of using that instead, if it covers all our use cases?

@jkachel
Copy link
Contributor Author

jkachel commented Jan 9, 2024

Looks good at first glance, but in ocw-studio a 3rd-party library was used to add this functionality (django-safedelete). What do you think of using that instead, if it covers all our use cases?

I'm ok with that - the one that I've got here (mostly lifted out of mitxonline) is pretty simple and django-safedelete adds a bunch of stuff we'd probably want anyway (like resurrecting deleted records). So, will close this in favor of that.

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

Successfully merging this pull request may close these issues.

2 participants