✨(backend) specific user delete method to delete its relations

Deleting a user is not possible when it has created docs because the
on_delete on the Document class id RESTRICT and we don't want to change
it. We decided to have a specific workflow for correctly delete a user.
The document where the user is the sole owner must be deleted, the other
only the owner access must be deleted. For the remaining Documents where
the user is the creator, we set it to `null`, then the user can be
delete, remaining relations are deleted in cascade.

CHANGELOG.md

@@ -12,6 +12,7 @@ and this project adheres to
 - ✨(frontend) leave a document #2410
 - ✨(frontend) add top parent on sub docs search #1952
 - ✨(frontend) unauthenticated users can search #2407
+- ✨(backend) specific user delete method to delete its relations
 
 ### Changed
 

src/backend/core/models.py

@@ -20,6 +20,7 @@ from django.core.files.base import ContentFile
 from django.core.files.storage import default_storage
 from django.core.mail import send_mail
 from django.db import models, transaction
+from django.db.models import Count
 from django.db.models.functions import Left, Length
 from django.template.loader import render_to_string
 from django.utils import timezone
@@ -225,6 +226,55 @@ class User(AbstractBaseUser, BaseModel, auth_models.PermissionsMixin):
             self._duplicate_onboarding_sandbox_document()
             self._convert_valid_invitations()
 
+    def delete(self, using=None, keep_parents=False):
+        """Completely delete a user and its relations."""
+        with transaction.atomic():
+            self._delete_user_shared_documents_accesses()
+            self._delete_documents_single_owner()
+            self._clear_user_created_documents()
+
+            return super().delete(using=using, keep_parents=keep_parents)
+
+    def _delete_user_shared_documents_accesses(self):
+        """
+        accesses to delete where there are more than one owner.
+
+        Create first a subquery to filter all the the accesses having more than one
+        owner. Then use this subquery to filter the accesses belonging to this list of
+        documents and to the user to delete
+        """
+        docs_ids = (
+            DocumentAccess.objects.filter(role=RoleChoices.OWNER)
+            .values("document_id")
+            .annotate(owner_count=Count("id"))
+            .filter(owner_count__gte=2)
+            .values("document_id")
+        )
+        DocumentAccess.objects.filter(user=self, document_id__in=docs_ids).delete()
+
+        logger.info(
+            "user_delete: shared documents accesses for user %s have been deleted",
+            self.id,
+        )
+
+    def _delete_documents_single_owner(self):
+        """Delete the documents where the user is the single owner."""
+        Document.objects.filter(
+            accesses__user=self, accesses__role=RoleChoices.OWNER
+        ).delete()
+        logger.info(
+            "user_delete: documents where the user %s is the sole owner deleted",
+            self.id,
+        )
+
+    def _clear_user_created_documents(self):
+        """Set creator to Null for documents where the user is the creator."""
+        Document.objects.filter(creator=self).update(creator=None)
+        logger.info(
+            "user_delete: documents created by user %s have been cleared",
+            self.id,
+        )
+
     def _handle_onboarding_documents_access(self):
         """
         If the user is new and there are documents configured to be given to new users,

src/backend/core/tests/test_models_users.py

@@ -7,7 +7,7 @@ from concurrent.futures import ThreadPoolExecutor
 from unittest.mock import patch
 
 from django.core.exceptions import ValidationError
-from django.db import connection
+from django.db import DatabaseError, connection
 from django.test.utils import override_settings
 
 import pytest
@@ -366,3 +366,280 @@ def test_models_users_duplicate_onboarding_sandbox_race_condition():
 
         assert isinstance(user1, models.User)
         assert isinstance(user2, models.User)
+
+
+@pytest.mark.django_db(transaction=True)
+def test_task_user_delete():
+    """test user_delete task with a complete scenario"""
+
+    user_to_delete = factories.UserFactory()
+    other_user = factories.UserFactory()
+
+    # Documents where the user is the sole owner
+    documents_to_delete = factories.DocumentFactory.create_batch(
+        3, creator=user_to_delete, users=[(user_to_delete, models.RoleChoices.OWNER)]
+    )
+    # Documents where the user is not the sole owner
+    documents_to_keep = factories.DocumentFactory.create_batch(
+        4,
+        creator=other_user,
+        users=[
+            (user_to_delete, models.RoleChoices.OWNER),
+            (other_user, models.RoleChoices.OWNER),
+        ],
+    )
+    # descendants of a document to delete should also be removed
+    depth = 5
+    for _ in range(depth + 1):
+        documents_to_delete.append(
+            factories.DocumentFactory(parent=documents_to_delete[-1])
+        )
+
+    # Documents created by the user to delete and should be set to null
+    documents_creator_set_to_null = factories.DocumentFactory.create_batch(
+        3, creator=user_to_delete
+    )
+
+    # Non creator nor owner document but with accesses to delete
+    for role in models.RoleChoices.values:
+        if role == models.RoleChoices.OWNER.value:
+            continue
+
+        factories.DocumentFactory(users=[(user_to_delete, role)])
+
+    # create 3 link traces
+    #
+    factories.DocumentFactory.create_batch(3, link_traces=[user_to_delete, other_user])
+    # Create other link traces for the other user
+    #
+    factories.DocumentFactory.create_batch(3, link_traces=[other_user])
+
+    # Threads created by the user to delete should be set to null
+    threads_owned_by_user_to_delete = factories.ThreadFactory.create_batch(
+        2, creator=user_to_delete
+    )
+    # Threads created by the other user should not change
+    threads_owned_by_other_user = factories.ThreadFactory.create_batch(
+        2, creator=other_user
+    )
+
+    # create comments for both users in all existing threads
+    comments = []
+    for thread in threads_owned_by_user_to_delete + threads_owned_by_other_user:
+        comments.append(factories.CommentFactory(thread=thread, user=user_to_delete))
+        comments.append(factories.CommentFactory(thread=thread, user=other_user))
+
+    assert models.DocumentAccess.objects.filter(user=user_to_delete).count() == 11
+    assert models.Document.objects.all().count() == 30
+    assert models.LinkTrace.objects.all().count() == 9
+    assert models.Thread.objects.all().count() == 4
+    assert models.Comment.objects.all().count() == 8
+    assert len(documents_to_delete) == 9
+
+    user_to_delete.delete()
+
+    for document in documents_to_delete:
+        assert models.Document.objects.filter(id=document.id).exists() is False
+
+    for document in documents_to_keep:
+        assert models.Document.objects.filter(id=document.id).exists()
+
+    for document in documents_creator_set_to_null:
+        document.refresh_from_db()
+        assert document.creator is None
+
+    assert models.DocumentAccess.objects.filter(user_id=user_to_delete.id).count() == 0
+    assert (
+        models.Document.objects.all().count() == 21
+    )  # models.Document.objects.all().count() - len(documents_to_delete)
+    assert models.LinkTrace.objects.all().count() == 6
+
+    # Number of threads and comments should not have changed
+    assert models.Thread.objects.all().count() == 4
+    assert models.Comment.objects.all().count() == 8
+
+    for thread in threads_owned_by_user_to_delete:
+        thread.refresh_from_db()
+        assert thread.creator is None
+
+        assert (
+            models.Comment.objects.filter(thread=thread, user__isnull=True).count() == 1
+        )
+        assert (
+            models.Comment.objects.filter(thread=thread, user=other_user).count() == 1
+        )
+
+    for thread in threads_owned_by_other_user:
+        thread.refresh_from_db()
+        assert thread.creator == other_user
+        assert (
+            models.Comment.objects.filter(thread=thread, user__isnull=True).count() == 1
+        )
+        assert (
+            models.Comment.objects.filter(thread=thread, user=other_user).count() == 1
+        )
+
+    assert models.User.objects.filter(id=user_to_delete.id).exists() is False
+
+
+@pytest.mark.django_db(transaction=True)
+def test_tasks_user_delete_nothing_to_delete():
+    """Delete a user not having creating data yet."""
+
+    user_to_delete = factories.UserFactory()
+    other_user = factories.UserFactory()
+
+    factories.DocumentFactory.create_batch(
+        4,
+        creator=other_user,
+        users=[
+            (other_user, models.RoleChoices.OWNER),
+        ],
+    )
+
+    # Create other link traces for the other user
+    #
+    factories.DocumentFactory.create_batch(3, link_traces=[other_user])
+
+    threads_owned_by_other_user = factories.ThreadFactory.create_batch(
+        2, creator=other_user
+    )
+
+    # create comments for bot users in all existing threads
+    comments = []
+    for thread in threads_owned_by_other_user:
+        comments.append(factories.CommentFactory(thread=thread, user=other_user))
+
+    assert models.DocumentAccess.objects.count() == 4
+    assert models.Document.objects.all().count() == 9
+    assert models.LinkTrace.objects.all().count() == 3
+    assert models.Thread.objects.all().count() == 2
+    assert models.Comment.objects.all().count() == 2
+
+    user_to_delete.delete()
+
+    assert models.DocumentAccess.objects.count() == 4
+    assert models.Document.objects.all().count() == 9
+    assert models.LinkTrace.objects.all().count() == 3
+    assert models.Thread.objects.all().count() == 2
+    assert models.Comment.objects.all().count() == 2
+    assert models.User.objects.filter(id=user_to_delete.id).exists() is False
+
+
+@pytest.mark.django_db(transaction=True)
+def test_tasks_user_delete_only_owner_but_not_creator():
+    """Test delete a user who is sole owner of a document but not the creator should work."""
+
+    user_to_delete = factories.UserFactory()
+    other_user = factories.UserFactory()
+
+    document_to_delete = factories.DocumentFactory(
+        creator=other_user, users=[(user_to_delete, models.RoleChoices.OWNER)]
+    )
+
+    document_to_keep = factories.DocumentFactory(
+        creator=other_user,
+        users=[
+            (other_user, models.RoleChoices.OWNER),
+            (user_to_delete, models.RoleChoices.OWNER),
+        ],
+    )
+
+    assert models.DocumentAccess.objects.filter(user=user_to_delete).count() == 2
+    assert models.Document.objects.all().count() == 2
+
+    user_to_delete.delete()
+
+    assert models.DocumentAccess.objects.filter(user_id=user_to_delete.id).count() == 0
+    assert models.Document.objects.all().count() == 1
+
+    assert models.Document.objects.filter(id=document_to_delete.id).exists() is False
+    assert models.Document.objects.filter(id=document_to_keep.id).exists() is True
+
+    assert models.User.objects.filter(id=user_to_delete.id).exists() is False
+
+
+@pytest.mark.django_db(transaction=True)
+def test_tasks_user_delete_error_during_deletion_should_rollback_deletion(monkeypatch):
+    """Test transaction is correctly working by forcing an error during user deletion."""
+
+    user_to_delete = factories.UserFactory()
+    other_user = factories.UserFactory()
+
+    # Documents where the user is the sole owner
+    documents_to_delete = factories.DocumentFactory.create_batch(
+        3, creator=user_to_delete, users=[(user_to_delete, models.RoleChoices.OWNER)]
+    )
+    # Documents where the user is not the sole owner
+    factories.DocumentFactory.create_batch(
+        4,
+        creator=other_user,
+        users=[
+            (user_to_delete, models.RoleChoices.OWNER),
+            (other_user, models.RoleChoices.OWNER),
+        ],
+    )
+    # descendants of a document to delete should also be removed
+    depth = 5
+    for _ in range(depth + 1):
+        documents_to_delete.append(
+            factories.DocumentFactory(parent=documents_to_delete[-1])
+        )
+
+    # Documents created by the user to delete and should be set to null
+    factories.DocumentFactory.create_batch(3, creator=user_to_delete)
+
+    # Non creator nor owner document but with accesses to delete
+    for role in models.RoleChoices.values:
+        if role == models.RoleChoices.OWNER.value:
+            continue
+
+        factories.DocumentFactory(users=[(user_to_delete, role)])
+
+    # create 3 link traces
+    #
+    factories.DocumentFactory.create_batch(3, link_traces=[user_to_delete, other_user])
+    # Create other link traces for the other user
+    #
+    factories.DocumentFactory.create_batch(3, link_traces=[other_user])
+
+    # Threads created by the user to delete should be set to null
+    threads_owned_by_user_to_delete = factories.ThreadFactory.create_batch(
+        2, creator=user_to_delete
+    )
+    # Threads created by the other user should not change
+    threads_owned_by_other_user = factories.ThreadFactory.create_batch(
+        2, creator=other_user
+    )
+
+    # create comments for bot users in all existing threads
+    comments = []
+    for thread in threads_owned_by_user_to_delete + threads_owned_by_other_user:
+        comments.append(factories.CommentFactory(thread=thread, user=user_to_delete))
+        comments.append(factories.CommentFactory(thread=thread, user=other_user))
+
+    assert models.DocumentAccess.objects.filter(user=user_to_delete).count() == 11
+    assert models.Document.objects.all().count() == 30
+    assert models.LinkTrace.objects.all().count() == 9
+    assert models.Thread.objects.all().count() == 4
+    assert models.Comment.objects.all().count() == 8
+    assert len(documents_to_delete) == 9
+
+    def mock_clear_user_created_documents(self):
+        raise DatabaseError()
+
+    monkeypatch.setattr(
+        models.User, "_clear_user_created_documents", mock_clear_user_created_documents
+    )
+
+    with pytest.raises(DatabaseError):
+        user_to_delete.delete()
+
+    assert models.DocumentAccess.objects.filter(user=user_to_delete).count() == 11
+    assert models.Document.objects.all().count() == 30
+    assert models.LinkTrace.objects.all().count() == 9
+    assert models.Thread.objects.all().count() == 4
+    assert models.Comment.objects.all().count() == 8
+    assert len(documents_to_delete) == 9
+
+    assert models.User.objects.filter(id=user_to_delete.id).exists() is True