Adds a database field for is_deleted on user

bookwyrm/migrations/0183_auto_20231105_1607.py

@@ -1,18 +1,6 @@
 # Generated by Django 3.2.20 on 2023-11-05 16:07
 
-from django.db import migrations
-from bookwyrm.models import User
-
-
-def erase_deleted_user_data(apps, schema_editor):
-    """Retroactively clear user data"""
-    for user in User.get_permanently_deleted_users():
-        user.erase_user_data()
-        user.save(
-            broadcast=False,
-            update_fields=["email", "avatar", "preview_image", "summary", "name"],
-        )
-        user.erase_user_statuses(broadcast=False)
+from django.db import migrations, models
 
 
 class Migration(migrations.Migration):
@@ -22,7 +10,9 @@ class Migration(migrations.Migration):
     ]
 
     operations = [
-        migrations.RunPython(
-            erase_deleted_user_data, reverse_code=migrations.RunPython.noop
-        )
+        migrations.AddField(
+            model_name="user",
+            name="is_deleted",
+            field=models.BooleanField(default=False),
+        ),
     ]

bookwyrm/migrations/0184_auto_20231106_0421.py

@@ -0,0 +1,49 @@
+# Generated by Django 3.2.20 on 2023-11-06 04:21
+
+from django.db import migrations
+from bookwyrm.models import User
+
+
+def update_deleted_users(apps, schema_editor):
+    """Find all the users who are deleted, not just inactive, and set deleted"""
+    users = apps.get_model("bookwyrm", "User")
+    db_alias = schema_editor.connection.alias
+    users.objects.using(db_alias).filter(
+        is_active=False,
+        deactivation_reason__in=[
+            "self_deletion",
+            "moderator_deletion",
+        ],
+    ).update(is_deleted=True)
+
+    # differente rules for remote users
+    users.objects.using(db_alias).filter(is_active=False, local=False,).exclude(
+        deactivation_reason="moderator_deactivation",
+    ).update(is_deleted=True)
+
+
+def erase_deleted_user_data(apps, schema_editor):
+    """Retroactively clear user data"""
+    for user in User.objects.filter(is_deleted=True):
+        user.erase_user_data()
+        user.save(
+            broadcast=False,
+            update_fields=["email", "avatar", "preview_image", "summary", "name"],
+        )
+        user.erase_user_statuses(broadcast=False)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("bookwyrm", "0183_auto_20231105_1607"),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            update_deleted_users, reverse_code=migrations.RunPython.noop
+        ),
+        migrations.RunPython(
+            erase_deleted_user_data, reverse_code=migrations.RunPython.noop
+        ),
+    ]

bookwyrm/models/user.py

@@ -8,7 +8,7 @@ from django.contrib.auth.models import AbstractUser
 from django.contrib.postgres.fields import ArrayField, CICharField
 from django.core.exceptions import PermissionDenied, ObjectDoesNotExist
 from django.dispatch import receiver
-from django.db import models, transaction
+from django.db import models, transaction, IntegrityError
 from django.utils import timezone
 from django.utils.translation import gettext_lazy as _
 from model_utils import FieldTracker
@@ -54,6 +54,7 @@ class User(OrderedCollectionPageMixin, AbstractUser):
 
     username = fields.UsernameField()
     email = models.EmailField(unique=True, null=True)
+    is_deleted = models.BooleanField(default=False)
 
     key_pair = fields.OneToOneField(
         "KeyPair",
@@ -263,17 +264,6 @@ class User(OrderedCollectionPageMixin, AbstractUser):
             is_active=True,
         ).distinct()
 
-    @classmethod
-    def get_permanently_deleted_users(cls):
-        """a list of users who are permanently deleted"""
-        return cls.objects.filter(
-            is_active=False,
-            deactivation_reason__in=[
-                "self_deletion",
-                "moderator_deletion",
-            ],
-        ).distinct()
-
     def update_active_date(self):
         """this user is here! they are doing things!"""
         self.last_active_date = timezone.now()
@@ -407,6 +397,7 @@ class User(OrderedCollectionPageMixin, AbstractUser):
         # pylint: disable=attribute-defined-outside-init
         self.is_active = False
         self.allow_reactivation = False
+        self.is_deleted = True
 
         self.erase_user_data()
         self.erase_user_statuses()
@@ -419,6 +410,11 @@ class User(OrderedCollectionPageMixin, AbstractUser):
 
     def erase_user_data(self):
         """Wipe a user's custom data"""
+        if not self.is_deleted:
+            raise IntegrityError(
+                "Trying to erase user data on user that is not deleted"
+            )
+
         # mangle email address
         self.email = f"{uuid4()}@deleted.user"
 
@@ -431,18 +427,14 @@ class User(OrderedCollectionPageMixin, AbstractUser):
 
     def erase_user_statuses(self, broadcast=True):
         """Wipe the data on all the user's statuses"""
+        if not self.is_deleted:
+            raise IntegrityError(
+                "Trying to erase user data on user that is not deleted"
+            )
+
         for status in self.status_set.all():
             status.delete(broadcast=broadcast)
 
-    @property
-    def is_permanently_deleted(self):
-        """is this user inactive, or really truly deleted?"""
-        return not self.is_active and self.deactivation_reason in [
-            "self_deletion",
-            "moderator_deletion",
-            "activitypub_deletion",
-        ]
-
     def deactivate(self):
         """Disable the user but allow them to reactivate"""
         # pylint: disable=attribute-defined-outside-init

bookwyrm/templates/snippets/user_active_tag.html

@@ -8,7 +8,7 @@
         {% trans "Active" as text %}
         {% include "snippets/user_active_tag_item.html" with icon="check" text=text level="success" %}
     {% endif %}
-{% elif user.is_permanently_deleted %}
+{% elif user.is_deleted %}
     {% trans "Deleted" as text %}
     {% include "snippets/user_active_tag_item.html" with icon="x" text=text level="danger" deactivation_reason=user.get_deactivation_reason_display %}
 {% else %}

bookwyrm/tests/migrations/test_0184.py

@@ -1,23 +1,20 @@
 """ testing migrations """
-import json
 from unittest.mock import patch
 
-from django.apps import apps
 from django.test import TestCase
 from django.db.migrations.executor import MigrationExecutor
 from django.db import connection
-import responses
 
 from bookwyrm import models
 from bookwyrm.management.commands import initdb
-from bookwyrm.settings import USE_HTTPS, DOMAIN
+from bookwyrm.settings import DOMAIN
 
 # pylint: disable=missing-class-docstring
 # pylint: disable=missing-function-docstring
 class EraseDeletedUserDataMigration(TestCase):
 
-    migrate_from = "0182_auto_20231027_1122"
-    migrate_to = "0183_auto_20231105_1607"
+    migrate_from = "0183_auto_20231105_1607"
+    migrate_to = "0184_auto_20231106_0421"
 
     # pylint: disable=invalid-name
     def setUp(self):
@@ -68,11 +65,6 @@ class EraseDeletedUserDataMigration(TestCase):
         initdb.init_groups()
         initdb.init_permissions()
 
-        assert (
-            self.migrate_from and self.migrate_to
-        ), "TestCase '{}' must define migrate_from and migrate_to properties".format(
-            type(self).__name__
-        )
         self.migrate_from = [("bookwyrm", self.migrate_from)]
         self.migrate_to = [("bookwyrm", self.migrate_to)]
         executor = MigrationExecutor(connection)
@@ -104,12 +96,14 @@ class EraseDeletedUserDataMigration(TestCase):
         self.deleted_status.refresh_from_db()
 
         self.assertTrue(self.active_user.is_active)
+        self.assertFalse(self.active_user.is_deleted)
         self.assertEqual(self.active_user.name, "a name")
         self.assertNotEqual(self.deleted_user.email, "activeuser@activeuser.activeuser")
         self.assertFalse(self.active_status.deleted)
         self.assertEqual(self.active_status.content, "don't delete me")
 
         self.assertFalse(self.inactive_user.is_active)
+        self.assertFalse(self.inactive_user.is_deleted)
         self.assertEqual(self.inactive_user.name, "name name")
         self.assertNotEqual(
             self.deleted_user.email, "inactiveuser@inactiveuser.inactiveuser"
@@ -118,6 +112,7 @@ class EraseDeletedUserDataMigration(TestCase):
         self.assertEqual(self.inactive_status.content, "also don't delete me")
 
         self.assertFalse(self.deleted_user.is_active)
+        self.assertTrue(self.deleted_user.is_deleted)
         self.assertIsNone(self.deleted_user.name)
         self.assertNotEqual(
             self.deleted_user.email, "deleteduser@deleteduser.deleteduser"

bookwyrm/tests/models/test_user_model.py

@@ -325,40 +325,3 @@ class User(TestCase):
         results = models.User.admins()
         self.assertEqual(results.count(), 1)
         self.assertEqual(results.first(), self.user)
-
-    def test_get_permanently_deleted_users(self):
-
-        with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch(
-            "bookwyrm.activitystreams.populate_stream_task.delay"
-        ), patch("bookwyrm.lists_stream.populate_lists_task.delay"):
-            models.User.objects.create_user(
-                f"activeuser@{DOMAIN}",
-                "activeuser@activeuser.activeuser",
-                "activeuserword",
-                local=True,
-                localname="active",
-            )
-            models.User.objects.create_user(
-                f"deleteduser@{DOMAIN}",
-                "deleteduser@deleteduser.deleteduser",
-                "deleteduserword",
-                local=True,
-                localname="deleted",
-                is_active=False,
-                deactivation_reason="self_deletion",
-            )
-            models.User.objects.create_user(
-                f"inactiveuser@{DOMAIN}",
-                "inactiveuser@inactiveuser.inactiveuser",
-                "inactiveuserword",
-                local=True,
-                localname="inactive",
-                is_active=False,
-                deactivation_reason="self_deactivation",
-            )
-
-        deleted_users = models.User.get_permanently_deleted_users()
-
-        self.assertTrue(deleted_users.filter(localname="deleted").exists())
-        self.assertFalse(deleted_users.filter(localname="active").exists())
-        self.assertFalse(deleted_users.filter(localname="inactive").exists())