Merge pull request #955 from bookwyrm-social/delete-user-errors

Delete user errors
This commit is contained in:
Mouse Reeve 2021-04-17 15:29:23 -07:00 committed by GitHub
commit b53c2c2196
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 74 additions and 10 deletions

View file

@ -16,11 +16,10 @@ class Verb(ActivityObject):
def action(self):
""" usually we just want to update and save """
obj = self.object
# it may return None if the object is invalid in an expected way
# self.object may return None if the object is invalid in an expected way
# ie, Question type
if obj:
obj.to_model()
if self.object:
self.object.to_model()
@dataclass(init=False)
@ -43,7 +42,16 @@ class Delete(Verb):
def action(self):
""" find and delete the activity object """
if not self.object:
return
if isinstance(self.object, str):
# Deleted users are passed as strings. Not wild about this fix
model = apps.get_model("bookwyrm.User")
obj = model.find_existing_by_remote_id(self.object)
else:
obj = self.object.to_model(save=False, allow_create=False)
if obj:
obj.delete()
# if we can't find it, we don't need to delete it because we don't have it
@ -58,8 +66,7 @@ class Update(Verb):
def action(self):
""" update a model instance from the dataclass """
obj = self.object
if obj:
if self.object:
self.object.to_model(allow_create=False)

View file

@ -241,7 +241,9 @@ class ObjectMixin(ActivitypubMixin):
return
# is this a deletion?
if hasattr(self, "deleted") and self.deleted:
if (hasattr(self, "deleted") and self.deleted) or (
hasattr(self, "is_active") and not self.is_active
):
activity = self.to_delete_activity(user)
else:
activity = self.to_update_activity(user)

View file

@ -205,6 +205,9 @@ class User(OrderedCollectionPageMixin, AbstractUser):
def to_activity(self, **kwargs):
"""override default AP serializer to add context object
idk if this is the best way to go about this"""
if not self.is_active:
return self.remote_id
activity_object = super().to_activity(**kwargs)
activity_object["@context"] = [
"https://www.w3.org/ns/activitystreams",
@ -283,6 +286,12 @@ class User(OrderedCollectionPageMixin, AbstractUser):
editable=False,
).save(broadcast=False)
def delete(self, *args, **kwargs):
""" deactivate rather than delete a user """
self.is_active = False
# skip the logic in this class's save()
super().save(*args, **kwargs)
@property
def local_path(self):
""" this model doesn't inherit bookwyrm model, so here we are """

View file

@ -1,4 +1,5 @@
""" testing models """
import json
from unittest.mock import patch
from django.test import TestCase
import responses
@ -152,3 +153,17 @@ class User(TestCase):
self.assertEqual(server.server_name, DOMAIN)
self.assertIsNone(server.application_type)
self.assertIsNone(server.application_version)
def test_delete_user(self):
""" deactivate a user """
self.assertTrue(self.user.is_active)
with patch(
"bookwyrm.models.activitypub_mixin.broadcast_task.delay"
) as broadcast_mock:
self.user.delete()
self.assertEqual(broadcast_mock.call_count, 1)
activity = json.loads(broadcast_mock.call_args[0][1])
self.assertEqual(activity["type"], "Delete")
self.assertEqual(activity["object"], self.user.remote_id)
self.assertFalse(self.user.is_active)

View file

@ -49,7 +49,7 @@ class InboxActivities(TestCase):
}
models.SiteSettings.objects.create()
def test_handle_delete_status(self):
def test_delete_status(self):
""" remove a status """
self.assertFalse(self.status.deleted)
activity = {
@ -70,7 +70,7 @@ class InboxActivities(TestCase):
self.assertTrue(status.deleted)
self.assertIsInstance(status.deleted_date, datetime)
def test_handle_delete_status_notifications(self):
def test_delete_status_notifications(self):
""" remove a status with related notifications """
models.Notification.objects.create(
related_status=self.status,
@ -104,3 +104,34 @@ class InboxActivities(TestCase):
# notifications should be truly deleted
self.assertEqual(models.Notification.objects.count(), 1)
self.assertEqual(models.Notification.objects.get(), notif)
def test_delete_user(self):
""" delete a user """
self.assertTrue(models.User.objects.get(username="rat@example.com").is_active)
activity = {
"@context": "https://www.w3.org/ns/activitystreams",
"id": "https://example.com/users/test-user#delete",
"type": "Delete",
"actor": "https://example.com/users/test-user",
"to": ["https://www.w3.org/ns/activitystreams#Public"],
"object": self.remote_user.remote_id,
}
views.inbox.activity_task(activity)
self.assertFalse(models.User.objects.get(username="rat@example.com").is_active)
def test_delete_user_unknown(self):
""" don't worry about it if we don't know the user """
self.assertEqual(models.User.objects.filter(is_active=True).count(), 2)
activity = {
"@context": "https://www.w3.org/ns/activitystreams",
"id": "https://example.com/users/test-user#delete",
"type": "Delete",
"actor": "https://example.com/users/test-user",
"to": ["https://www.w3.org/ns/activitystreams#Public"],
"object": "https://example.com/users/test-user",
}
# nothing happens.
views.inbox.activity_task(activity)
self.assertEqual(models.User.objects.filter(is_active=True).count(), 2)