simple user data deletion #3597
Conversation
…simple-user-data-deletion
| def hard_delete_post(post: Post): | ||
| if question := post.question: | ||
| question.delete() | ||
| if group_of_questions := post.group_of_questions: | ||
| group_of_questions.delete() | ||
| if conditional := post.conditional: | ||
| conditional.delete() | ||
| if notebook := post.notebook: | ||
| notebook.delete() | ||
| post.delete() |
There was a problem hiding this comment.
You don’t need to handle deletion manually — Django and the database will take care of it based on the ForeignKey deletion behavior (like CASCADE in our case). So you can simply call post.delete() and everything will be cleaned up accordingly
There was a problem hiding this comment.
That doesn't work. We've had issues in the past (one of our admins hard-deleting Post objects lead to orphaned questions with 404s the admin page).
looking through the relations, Post has a OneToOneField relation with Question,, Conditional, GroupOfQuestions and Notebook, all with deletion behavior "CASCADE", but that deletion behavior only goes one direction, namely the Post will be deleted if the relative object is deleted but not the other way around.
You can try it locally and see that it works that way:
In:
post = Post.objects.get(id=39817)
question_id = post.question_id
print(Question.objects.filter(id=question_id).exists())
print(post.delete())
print(Question.objects.filter(id=question_id).exists())
Out:
True
(
156,
{
"questions.UserForecastNotification": 2,
"misc.PostArticle": 119,
"posts.Post_projects": 3,
"posts.PostSubscription": 5,
"posts.PostUserSnapshot": 14,
"posts.Post": 1,
"questions.Forecast": 8,
"comments.Comment": 4,
},
)
True
| self.last_name = "" | ||
| self.email = "" | ||
| self.set_password(None) | ||
| self.save() |
There was a problem hiding this comment.
Hm, why can't we just delete user entirely?
There was a problem hiding this comment.
Ah, maybe I get it now. Another approach to handling “can’t delete because user has undeletable posts/questions” could be to create a generic “system user” — then, instead of blocking the deletion, we transfer ownership of those items to that system user and delete current. WDYT?
There was a problem hiding this comment.
We need to keep their forecasts isolated from other user's forecasts, so we can't just redistribute ownership unfortunately. :(
There was a problem hiding this comment.
comment and post/question authorship wouldn't be a big deal though
| for post in posts: | ||
| if post.curation_status != Post.CurationStatus.APPROVED: | ||
| hard_delete_post(post) | ||
| return |
There was a problem hiding this comment.
Did you not mean continue here instead of return?
| # Post is either a notebook or a quesiton with others' forecasts | ||
| # nothing required | ||
|
|
||
| self.save() |
There was a problem hiding this comment.
minor: I don't think this is needed.
3 participants
closes #2450
linked to https://www.notion.so/metaculus/Information-Audit-1d16aaf4f1018064b28dd0e42ac14836
I don't plan to merge this until after going through this with Leonard, and a quick test on Dev. But it is ready for code review.