Merge branch 'main' into remove-tags

bookwyrm/tests/views/inbox/test_inbox.py

@@ -1,11 +1,13 @@
 """ tests incoming activities"""
 import json
+import pathlib
 from unittest.mock import patch
 
 from django.http import HttpResponseNotAllowed, HttpResponseNotFound
 from django.test import TestCase, Client
+from django.test.client import RequestFactory
 
-from bookwyrm import models
+from bookwyrm import models, views
 
 
 # pylint: disable=too-many-public-methods
@@ -15,6 +17,7 @@ class Inbox(TestCase):
     def setUp(self):
         """ basic user and book data """
         self.client = Client()
+        self.factory = RequestFactory()
         local_user = models.User.objects.create_user(
             "mouse@example.com",
             "mouse@mouse.com",
@@ -24,6 +27,16 @@ class Inbox(TestCase):
         )
         local_user.remote_id = "https://example.com/user/mouse"
         local_user.save(broadcast=False)
+        with patch("bookwyrm.models.user.set_remote_server.delay"):
+            self.remote_user = models.User.objects.create_user(
+                "rat",
+                "rat@rat.com",
+                "ratword",
+                local=False,
+                remote_id="https://example.com/users/rat",
+                inbox="https://example.com/users/rat/inbox",
+                outbox="https://example.com/users/rat/outbox",
+            )
         self.create_json = {
             "id": "hi",
             "type": "Create",
@@ -106,3 +119,44 @@ class Inbox(TestCase):
                     "/inbox", json.dumps(activity), content_type="application/json"
                 )
         self.assertEqual(result.status_code, 200)
+
+    def test_is_blocked_user_agent(self):
+        """ check for blocked servers """
+        request = self.factory.post(
+            "",
+            HTTP_USER_AGENT="http.rb/4.4.1 (Mastodon/3.3.0; +https://mastodon.social/)",
+        )
+        self.assertFalse(views.inbox.is_blocked_user_agent(request))
+
+        models.FederatedServer.objects.create(
+            server_name="mastodon.social", status="blocked"
+        )
+        self.assertTrue(views.inbox.is_blocked_user_agent(request))
+
+    def test_is_blocked_activity(self):
+        """ check for blocked servers """
+        activity = {"actor": "https://mastodon.social/user/whaatever/else"}
+        self.assertFalse(views.inbox.is_blocked_activity(activity))
+
+        models.FederatedServer.objects.create(
+            server_name="mastodon.social", status="blocked"
+        )
+        self.assertTrue(views.inbox.is_blocked_activity(activity))
+
+    def test_create_by_deactivated_user(self):
+        """ don't let deactivated users post """
+        self.remote_user.delete(broadcast=False)
+        self.assertTrue(self.remote_user.deleted)
+        datafile = pathlib.Path(__file__).parent.joinpath("../../data/ap_note.json")
+        status_data = json.loads(datafile.read_bytes())
+        activity = self.create_json
+        activity["actor"] = self.remote_user.remote_id
+        activity["object"] = status_data
+
+        with patch("bookwyrm.views.inbox.has_valid_signature") as mock_valid:
+            mock_valid.return_value = True
+
+            result = self.client.post(
+                "/inbox", json.dumps(activity), content_type="application/json"
+            )
+        self.assertEqual(result.status_code, 403)

bookwyrm/tests/views/inbox/test_inbox_add.py

@@ -94,6 +94,7 @@ class InboxAdd(TestCase):
                 "type": "ListItem",
                 "book": self.book.remote_id,
                 "id": "https://bookwyrm.social/listbook/6189",
+                "order": 1,
             },
             "target": "https://bookwyrm.social/user/mouse/list/to-read",
             "@context": "https://www.w3.org/ns/activitystreams",

bookwyrm/tests/views/inbox/test_inbox_announce.py

@@ -136,6 +136,9 @@ class InboxActivities(TestCase):
             "id": "http://www.faraway.com/boost/12",
             "actor": self.remote_user.remote_id,
             "object": status.remote_id,
+            "to": ["https://www.w3.org/ns/activitystreams#public"],
+            "cc": ["https://example.com/user/mouse/followers"],
+            "published": "Mon, 25 May 2020 19:31:20 GMT",
         }
         responses.add(
             responses.GET, status.remote_id, json=status.to_activity(), status=200
@@ -185,6 +188,7 @@ class InboxActivities(TestCase):
                 "id": "http://fake.com/unknown/boost",
                 "actor": self.remote_user.remote_id,
                 "object": self.status.remote_id,
+                "published": "Mon, 25 May 2020 19:31:20 GMT",
             },
         }
         views.inbox.activity_task(activity)

bookwyrm/tests/views/inbox/test_inbox_create.py

@@ -6,6 +6,7 @@ from unittest.mock import patch
 from django.test import TestCase
 
 from bookwyrm import models, views
+from bookwyrm.activitypub import ActivitySerializerError
 
 
 # pylint: disable=too-many-public-methods
@@ -31,7 +32,7 @@ class InboxCreate(TestCase):
                     remote_id="https://example.com/status/1",
                 )
         with patch("bookwyrm.models.user.set_remote_server.delay"):
-            models.User.objects.create_user(
+            self.remote_user = models.User.objects.create_user(
                 "rat",
                 "rat@rat.com",
                 "ratword",
@@ -51,7 +52,7 @@ class InboxCreate(TestCase):
         }
         models.SiteSettings.objects.create()
 
-    def test_handle_create_status(self):
+    def test_create_status(self):
         """ the "it justs works" mode """
         self.assertEqual(models.Status.objects.count(), 1)
 
@@ -82,7 +83,7 @@ class InboxCreate(TestCase):
         views.inbox.activity_task(activity)
         self.assertEqual(models.Status.objects.count(), 2)
 
-    def test_handle_create_status_remote_note_with_mention(self):
+    def test_create_status_remote_note_with_mention(self):
         """ should only create it under the right circumstances """
         self.assertEqual(models.Status.objects.count(), 1)
         self.assertFalse(
@@ -105,7 +106,7 @@ class InboxCreate(TestCase):
         )
         self.assertEqual(models.Notification.objects.get().notification_type, "MENTION")
 
-    def test_handle_create_status_remote_note_with_reply(self):
+    def test_create_status_remote_note_with_reply(self):
         """ should only create it under the right circumstances """
         self.assertEqual(models.Status.objects.count(), 1)
         self.assertFalse(models.Notification.objects.filter(user=self.local_user))
@@ -126,7 +127,7 @@ class InboxCreate(TestCase):
         self.assertTrue(models.Notification.objects.filter(user=self.local_user))
         self.assertEqual(models.Notification.objects.get().notification_type, "REPLY")
 
-    def test_handle_create_list(self):
+    def test_create_list(self):
         """ a new list """
         activity = self.create_json
         activity["object"] = {
@@ -149,3 +150,23 @@ class InboxCreate(TestCase):
         self.assertEqual(book_list.curation, "curated")
         self.assertEqual(book_list.description, "summary text")
         self.assertEqual(book_list.remote_id, "https://example.com/list/22")
+
+    def test_create_unsupported_type(self):
+        """ ignore activities we know we can't handle """
+        activity = self.create_json
+        activity["object"] = {
+            "id": "https://example.com/status/887",
+            "type": "Question",
+        }
+        # just observer how it doesn't throw an error
+        views.inbox.activity_task(activity)
+
+    def test_create_unknown_type(self):
+        """ ignore activities we know we've never heard of """
+        activity = self.create_json
+        activity["object"] = {
+            "id": "https://example.com/status/887",
+            "type": "Threnody",
+        }
+        with self.assertRaises(ActivitySerializerError):
+            views.inbox.activity_task(activity)

bookwyrm/tests/views/inbox/test_inbox_delete.py

@@ -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)

bookwyrm/tests/views/inbox/test_inbox_follow.py

@@ -1,4 +1,5 @@
 """ tests incoming activities"""
+import json
 from unittest.mock import patch
 
 from django.test import TestCase
@@ -34,7 +35,7 @@ class InboxRelationships(TestCase):
 
         models.SiteSettings.objects.create()
 
-    def test_handle_follow(self):
+    def test_follow(self):
         """ remote user wants to follow local user """
         activity = {
             "@context": "https://www.w3.org/ns/activitystreams",
@@ -48,6 +49,8 @@ class InboxRelationships(TestCase):
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
             views.inbox.activity_task(activity)
             self.assertEqual(mock.call_count, 1)
+            response_activity = json.loads(mock.call_args[0][1])
+            self.assertEqual(response_activity["type"], "Accept")
 
         # notification created
         notification = models.Notification.objects.get()
@@ -61,7 +64,34 @@ class InboxRelationships(TestCase):
         follow = models.UserFollows.objects.get(user_object=self.local_user)
         self.assertEqual(follow.user_subject, self.remote_user)
 
-    def test_handle_follow_manually_approved(self):
+    def test_follow_duplicate(self):
+        """ remote user wants to follow local user twice """
+        activity = {
+            "@context": "https://www.w3.org/ns/activitystreams",
+            "id": "https://example.com/users/rat/follows/123",
+            "type": "Follow",
+            "actor": "https://example.com/users/rat",
+            "object": "https://example.com/user/mouse",
+        }
+
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            views.inbox.activity_task(activity)
+
+        # the follow relationship should exist
+        follow = models.UserFollows.objects.get(user_object=self.local_user)
+        self.assertEqual(follow.user_subject, self.remote_user)
+
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
+            views.inbox.activity_task(activity)
+            self.assertEqual(mock.call_count, 1)
+            response_activity = json.loads(mock.call_args[0][1])
+            self.assertEqual(response_activity["type"], "Accept")
+
+        # the follow relationship should STILL exist
+        follow = models.UserFollows.objects.get(user_object=self.local_user)
+        self.assertEqual(follow.user_subject, self.remote_user)
+
+    def test_follow_manually_approved(self):
         """ needs approval before following """
         activity = {
             "@context": "https://www.w3.org/ns/activitystreams",
@@ -91,7 +121,7 @@ class InboxRelationships(TestCase):
         follow = models.UserFollows.objects.all()
         self.assertEqual(list(follow), [])
 
-    def test_handle_undo_follow_request(self):
+    def test_undo_follow_request(self):
         """ the requester cancels a follow request """
         self.local_user.manually_approves_followers = True
         self.local_user.save(broadcast=False)
@@ -121,7 +151,7 @@ class InboxRelationships(TestCase):
 
         self.assertFalse(self.local_user.follower_requests.exists())
 
-    def test_handle_unfollow(self):
+    def test_unfollow(self):
         """ remove a relationship """
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
             rel = models.UserFollows.objects.create(
@@ -146,7 +176,7 @@ class InboxRelationships(TestCase):
         views.inbox.activity_task(activity)
         self.assertIsNone(self.local_user.followers.first())
 
-    def test_handle_follow_accept(self):
+    def test_follow_accept(self):
         """ a remote user approved a follow request from local """
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
             rel = models.UserFollowRequest.objects.create(
@@ -177,7 +207,7 @@ class InboxRelationships(TestCase):
         self.assertEqual(follows.count(), 1)
         self.assertEqual(follows.first(), self.local_user)
 
-    def test_handle_follow_reject(self):
+    def test_follow_reject(self):
         """ turn down a follow request """
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
             rel = models.UserFollowRequest.objects.create(

bookwyrm/tests/views/inbox/test_inbox_remove.py

@@ -80,6 +80,7 @@ class InboxRemove(TestCase):
                 user=self.local_user,
                 book=self.book,
                 book_list=booklist,
+                order=1,
             )
         self.assertEqual(booklist.books.count(), 1)
 

bookwyrm/tests/views/inbox/test_inbox_update.py

@@ -23,6 +23,16 @@ class InboxUpdate(TestCase):
         )
         self.local_user.remote_id = "https://example.com/user/mouse"
         self.local_user.save(broadcast=False)
+        with patch("bookwyrm.models.user.set_remote_server.delay"):
+            self.remote_user = models.User.objects.create_user(
+                "rat",
+                "rat@rat.com",
+                "ratword",
+                local=False,
+                remote_id="https://example.com/users/rat",
+                inbox="https://example.com/users/rat/inbox",
+                outbox="https://example.com/users/rat/outbox",
+            )
 
         self.create_json = {
             "id": "hi",
@@ -34,7 +44,7 @@ class InboxUpdate(TestCase):
         }
         models.SiteSettings.objects.create()
 
-    def test_handle_update_list(self):
+    def test_update_list(self):
         """ a new list """
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
             book_list = models.List.objects.create(
@@ -68,16 +78,24 @@ class InboxUpdate(TestCase):
         self.assertEqual(book_list.description, "summary text")
         self.assertEqual(book_list.remote_id, "https://example.com/list/22")
 
-    def test_handle_update_user(self):
+    def test_update_user(self):
         """ update an existing user """
-        # we only do this with remote users
-        self.local_user.local = False
-        self.local_user.save()
+        models.UserFollows.objects.create(
+            user_subject=self.local_user,
+            user_object=self.remote_user,
+        )
+        models.UserFollows.objects.create(
+            user_subject=self.remote_user,
+            user_object=self.local_user,
+        )
+        self.assertTrue(self.remote_user in self.local_user.followers.all())
+        self.assertTrue(self.local_user in self.remote_user.followers.all())
 
-        datafile = pathlib.Path(__file__).parent.joinpath("../../data/ap_user.json")
+        datafile = pathlib.Path(__file__).parent.joinpath("../../data/ap_user_rat.json")
         userdata = json.loads(datafile.read_bytes())
         del userdata["icon"]
-        self.assertIsNone(self.local_user.name)
+        self.assertIsNone(self.remote_user.name)
+        self.assertFalse(self.remote_user.discoverable)
         views.inbox.activity_task(
             {
                 "type": "Update",
@@ -88,13 +106,16 @@ class InboxUpdate(TestCase):
                 "object": userdata,
             }
         )
-        user = models.User.objects.get(id=self.local_user.id)
-        self.assertEqual(user.name, "MOUSE?? MOUSE!!")
-        self.assertEqual(user.username, "mouse@example.com")
-        self.assertEqual(user.localname, "mouse")
+        user = models.User.objects.get(id=self.remote_user.id)
+        self.assertEqual(user.name, "RAT???")
+        self.assertEqual(user.username, "rat@example.com")
         self.assertTrue(user.discoverable)
 
-    def test_handle_update_edition(self):
+        # make sure relationships aren't disrupted
+        self.assertTrue(self.remote_user in self.local_user.followers.all())
+        self.assertTrue(self.local_user in self.remote_user.followers.all())
+
+    def test_update_edition(self):
         """ update an existing edition """
         datafile = pathlib.Path(__file__).parent.joinpath("../../data/bw_edition.json")
         bookdata = json.loads(datafile.read_bytes())
@@ -122,8 +143,9 @@ class InboxUpdate(TestCase):
             )
         book = models.Edition.objects.get(id=book.id)
         self.assertEqual(book.title, "Piranesi")
+        self.assertEqual(book.last_edited_by, self.remote_user)
 
-    def test_handle_update_work(self):
+    def test_update_work(self):
         """ update an existing edition """
         datafile = pathlib.Path(__file__).parent.joinpath("../../data/bw_work.json")
         bookdata = json.loads(datafile.read_bytes())

bookwyrm/tests/views/test_book.py

@@ -47,6 +47,39 @@ class BookViews(TestCase):
         )
         models.SiteSettings.objects.create()
 
+    def test_date_regression(self):
+        """ensure that creating a new book actually saves the published date fields
+
+        this was initially a regression due to using a custom date picker tag
+        """
+        first_published_date = "2021-04-20"
+        published_date = "2022-04-20"
+        self.local_user.groups.add(self.group)
+        view = views.EditBook.as_view()
+        form = forms.EditionForm(
+            {
+                "title": "New Title",
+                "last_edited_by": self.local_user.id,
+                "first_published_date": first_published_date,
+                "published_date": published_date,
+            }
+        )
+        request = self.factory.post("", form.data)
+        request.user = self.local_user
+
+        with patch("bookwyrm.connectors.connector_manager.local_search"):
+            result = view(request)
+        result.render()
+
+        self.assertContains(
+            result,
+            f'<input type="date" name="first_published_date" class="input" id="id_first_published_date" value="{first_published_date}">',
+        )
+        self.assertContains(
+            result,
+            f'<input type="date" name="published_date" class="input" id="id_published_date" value="{published_date}">',
+        )
+
     def test_book_page(self):
         """ there are so many views, this just makes sure it LOADS """
         view = views.Book.as_view()

bookwyrm/tests/views/test_federation.py

@@ -1,9 +1,12 @@
 """ test for app action functionality """
+import json
+from unittest.mock import patch
+from django.core.files.uploadedfile import SimpleUploadedFile
 from django.template.response import TemplateResponse
 from django.test import TestCase
 from django.test.client import RequestFactory
 
-from bookwyrm import models, views
+from bookwyrm import forms, models, views
 
 
 class FederationViews(TestCase):
@@ -19,6 +22,16 @@ class FederationViews(TestCase):
             local=True,
             localname="mouse",
         )
+        with patch("bookwyrm.models.user.set_remote_server.delay"):
+            self.remote_user = models.User.objects.create_user(
+                "rat",
+                "rat@rat.com",
+                "ratword",
+                local=False,
+                remote_id="https://example.com/users/rat",
+                inbox="https://example.com/users/rat/inbox",
+                outbox="https://example.com/users/rat/outbox",
+            )
         models.SiteSettings.objects.create()
 
     def test_federation_page(self):
@@ -44,3 +57,111 @@ class FederationViews(TestCase):
         self.assertIsInstance(result, TemplateResponse)
         result.render()
         self.assertEqual(result.status_code, 200)
+
+    def test_server_page_block(self):
+        """ block a server """
+        server = models.FederatedServer.objects.create(server_name="hi.there.com")
+        self.remote_user.federated_server = server
+        self.remote_user.save()
+
+        self.assertEqual(server.status, "federated")
+
+        view = views.federation.block_server
+        request = self.factory.post("")
+        request.user = self.local_user
+        request.user.is_superuser = True
+
+        view(request, server.id)
+        server.refresh_from_db()
+        self.remote_user.refresh_from_db()
+        self.assertEqual(server.status, "blocked")
+        # and the user was deactivated
+        self.assertFalse(self.remote_user.is_active)
+
+    def test_server_page_unblock(self):
+        """ unblock a server """
+        server = models.FederatedServer.objects.create(
+            server_name="hi.there.com", status="blocked"
+        )
+        self.remote_user.federated_server = server
+        self.remote_user.is_active = False
+        self.remote_user.deactivation_reason = "domain_block"
+        self.remote_user.save()
+
+        request = self.factory.post("")
+        request.user = self.local_user
+        request.user.is_superuser = True
+
+        views.federation.unblock_server(request, server.id)
+        server.refresh_from_db()
+        self.remote_user.refresh_from_db()
+        self.assertEqual(server.status, "federated")
+        # and the user was re-activated
+        self.assertTrue(self.remote_user.is_active)
+
+    def test_add_view_get(self):
+        """ there are so many views, this just makes sure it LOADS """
+        # create mode
+        view = views.AddFederatedServer.as_view()
+        request = self.factory.get("")
+        request.user = self.local_user
+        request.user.is_superuser = True
+
+        result = view(request)
+        self.assertIsInstance(result, TemplateResponse)
+        result.render()
+        self.assertEqual(result.status_code, 200)
+
+    def test_add_view_post_create(self):
+        """ create a server entry """
+        form = forms.ServerForm()
+        form.data["server_name"] = "remote.server"
+        form.data["application_type"] = "coolsoft"
+        form.data["status"] = "blocked"
+
+        view = views.AddFederatedServer.as_view()
+        request = self.factory.post("", form.data)
+        request.user = self.local_user
+        request.user.is_superuser = True
+
+        view(request)
+        server = models.FederatedServer.objects.get()
+        self.assertEqual(server.server_name, "remote.server")
+        self.assertEqual(server.application_type, "coolsoft")
+        self.assertEqual(server.status, "blocked")
+
+    def test_import_blocklist(self):
+        """ load a json file with a list of servers to block """
+        server = models.FederatedServer.objects.create(server_name="hi.there.com")
+        self.remote_user.federated_server = server
+        self.remote_user.save()
+
+        data = [
+            {"instance": "server.name", "url": "https://explanation.url"},  # new server
+            {"instance": "hi.there.com", "url": "https://explanation.url"},  # existing
+            {"a": "b"},  # invalid
+        ]
+        json.dump(data, open("file.json", "w"))
+
+        view = views.ImportServerBlocklist.as_view()
+        request = self.factory.post(
+            "",
+            {
+                "json_file": SimpleUploadedFile(
+                    "file.json", open("file.json", "rb").read()
+                )
+            },
+        )
+        request.user = self.local_user
+        request.user.is_superuser = True
+
+        view(request)
+        server.refresh_from_db()
+        self.remote_user.refresh_from_db()
+
+        self.assertEqual(models.FederatedServer.objects.count(), 2)
+        self.assertEqual(server.status, "blocked")
+        self.assertFalse(self.remote_user.is_active)
+        created = models.FederatedServer.objects.get(server_name="server.name")
+        self.assertEqual(created.status, "blocked")
+        self.assertEqual(created.notes, "https://explanation.url")

bookwyrm/tests/views/test_helpers.py

@@ -146,6 +146,15 @@ class ViewsHelpers(TestCase):
             self.assertIsInstance(result, models.User)
             self.assertEqual(result.username, "mouse@example.com")
 
+    def test_user_on_blocked_server(self, _):
+        """ find a remote user using webfinger """
+        models.FederatedServer.objects.create(
+            server_name="example.com", status="blocked"
+        )
+
+        result = views.helpers.handle_remote_webfinger("@mouse@example.com")
+        self.assertIsNone(result)
+
     def test_handle_reading_status_to_read(self, _):
         """ posts shelve activities """
         shelf = self.local_user.shelf_set.get(identifier="to-read")
@@ -190,66 +199,6 @@ class ViewsHelpers(TestCase):
             )
         self.assertFalse(models.GeneratedNote.objects.exists())
 
-    def test_object_visible_to_user(self, _):
-        """ does a user have permission to view an object """
-        obj = models.Status.objects.create(
-            content="hi", user=self.remote_user, privacy="public"
-        )
-        self.assertTrue(views.helpers.object_visible_to_user(self.local_user, obj))
-
-        obj = models.Shelf.objects.create(
-            name="test", user=self.remote_user, privacy="unlisted"
-        )
-        self.assertTrue(views.helpers.object_visible_to_user(self.local_user, obj))
-
-        obj = models.Status.objects.create(
-            content="hi", user=self.remote_user, privacy="followers"
-        )
-        self.assertFalse(views.helpers.object_visible_to_user(self.local_user, obj))
-
-        obj = models.Status.objects.create(
-            content="hi", user=self.remote_user, privacy="direct"
-        )
-        self.assertFalse(views.helpers.object_visible_to_user(self.local_user, obj))
-
-        obj = models.Status.objects.create(
-            content="hi", user=self.remote_user, privacy="direct"
-        )
-        obj.mention_users.add(self.local_user)
-        self.assertTrue(views.helpers.object_visible_to_user(self.local_user, obj))
-
-    def test_object_visible_to_user_follower(self, _):
-        """ what you can see if you follow a user """
-        self.remote_user.followers.add(self.local_user)
-        obj = models.Status.objects.create(
-            content="hi", user=self.remote_user, privacy="followers"
-        )
-        self.assertTrue(views.helpers.object_visible_to_user(self.local_user, obj))
-
-        obj = models.Status.objects.create(
-            content="hi", user=self.remote_user, privacy="direct"
-        )
-        self.assertFalse(views.helpers.object_visible_to_user(self.local_user, obj))
-
-        obj = models.Status.objects.create(
-            content="hi", user=self.remote_user, privacy="direct"
-        )
-        obj.mention_users.add(self.local_user)
-        self.assertTrue(views.helpers.object_visible_to_user(self.local_user, obj))
-
-    def test_object_visible_to_user_blocked(self, _):
-        """ you can't see it if they block you """
-        self.remote_user.blocks.add(self.local_user)
-        obj = models.Status.objects.create(
-            content="hi", user=self.remote_user, privacy="public"
-        )
-        self.assertFalse(views.helpers.object_visible_to_user(self.local_user, obj))
-
-        obj = models.Shelf.objects.create(
-            name="test", user=self.remote_user, privacy="unlisted"
-        )
-        self.assertFalse(views.helpers.object_visible_to_user(self.local_user, obj))
-
     def test_get_annotated_users(self, _):
         """ list of people you might know """
         user_1 = models.User.objects.create_user(

bookwyrm/tests/views/test_list.py

@@ -39,6 +39,25 @@ class ListViews(TestCase):
             remote_id="https://example.com/book/1",
             parent_work=work,
         )
+        work_two = models.Work.objects.create(title="Labori")
+        self.book_two = models.Edition.objects.create(
+            title="Example Edition 2",
+            remote_id="https://example.com/book/2",
+            parent_work=work_two,
+        )
+        work_three = models.Work.objects.create(title="Trabajar")
+        self.book_three = models.Edition.objects.create(
+            title="Example Edition 3",
+            remote_id="https://example.com/book/3",
+            parent_work=work_three,
+        )
+        work_four = models.Work.objects.create(title="Travailler")
+        self.book_four = models.Edition.objects.create(
+            title="Example Edition 4",
+            remote_id="https://example.com/book/4",
+            parent_work=work_four,
+        )
+
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
             self.list = models.List.objects.create(
                 name="Test List", user=self.local_user
@@ -194,6 +213,7 @@ class ListViews(TestCase):
                 user=self.local_user,
                 book=self.book,
                 approved=False,
+                order=1,
             )
 
         request = self.factory.post(
@@ -208,7 +228,7 @@ class ListViews(TestCase):
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay") as mock:
             view(request, self.list.id)
 
-        self.assertEqual(mock.call_count, 1)
+        self.assertEqual(mock.call_count, 2)
         activity = json.loads(mock.call_args[0][1])
         self.assertEqual(activity["type"], "Add")
         self.assertEqual(activity["actor"], self.local_user.remote_id)
@@ -228,6 +248,7 @@ class ListViews(TestCase):
                 user=self.local_user,
                 book=self.book,
                 approved=False,
+                order=1,
             )
 
         request = self.factory.post(
@@ -268,6 +289,261 @@ class ListViews(TestCase):
         self.assertEqual(item.user, self.local_user)
         self.assertTrue(item.approved)
 
+    def test_add_two_books(self):
+        """
+        Putting two books on the list. The first should have an order value of
+        1 and the second should have an order value of 2.
+        """
+        request_one = self.factory.post(
+            "",
+            {
+                "book": self.book.id,
+                "list": self.list.id,
+            },
+        )
+        request_one.user = self.local_user
+
+        request_two = self.factory.post(
+            "",
+            {
+                "book": self.book_two.id,
+                "list": self.list.id,
+            },
+        )
+        request_two.user = self.local_user
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            views.list.add_book(request_one)
+            views.list.add_book(request_two)
+
+        items = self.list.listitem_set.order_by("order").all()
+        self.assertEqual(items[0].book, self.book)
+        self.assertEqual(items[1].book, self.book_two)
+        self.assertEqual(items[0].order, 1)
+        self.assertEqual(items[1].order, 2)
+
+    def test_add_three_books_and_remove_second(self):
+        """
+        Put three books on a list and then remove the one in the middle. The
+        ordering of the list should adjust to not have a gap.
+        """
+        request_one = self.factory.post(
+            "",
+            {
+                "book": self.book.id,
+                "list": self.list.id,
+            },
+        )
+        request_one.user = self.local_user
+
+        request_two = self.factory.post(
+            "",
+            {
+                "book": self.book_two.id,
+                "list": self.list.id,
+            },
+        )
+        request_two.user = self.local_user
+
+        request_three = self.factory.post(
+            "",
+            {
+                "book": self.book_three.id,
+                "list": self.list.id,
+            },
+        )
+        request_three.user = self.local_user
+
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            views.list.add_book(request_one)
+            views.list.add_book(request_two)
+            views.list.add_book(request_three)
+
+        items = self.list.listitem_set.order_by("order").all()
+        self.assertEqual(items[0].book, self.book)
+        self.assertEqual(items[1].book, self.book_two)
+        self.assertEqual(items[2].book, self.book_three)
+        self.assertEqual(items[0].order, 1)
+        self.assertEqual(items[1].order, 2)
+        self.assertEqual(items[2].order, 3)
+
+        remove_request = self.factory.post("", {"item": items[1].id})
+        remove_request.user = self.local_user
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            views.list.remove_book(remove_request, self.list.id)
+        items = self.list.listitem_set.order_by("order").all()
+        self.assertEqual(items[0].book, self.book)
+        self.assertEqual(items[1].book, self.book_three)
+        self.assertEqual(items[0].order, 1)
+        self.assertEqual(items[1].order, 2)
+
+    def test_adding_book_with_a_pending_book(self):
+        """
+        When a list contains any pending books, the pending books should have
+        be at the end of the list by order. If a book is added while a book is
+        pending, its order should precede the pending books.
+        """
+        request = self.factory.post(
+            "",
+            {
+                "book": self.book_three.id,
+                "list": self.list.id,
+            },
+        )
+        request.user = self.local_user
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            models.ListItem.objects.create(
+                book_list=self.list,
+                user=self.local_user,
+                book=self.book,
+                approved=True,
+                order=1,
+            )
+            models.ListItem.objects.create(
+                book_list=self.list,
+                user=self.rat,
+                book=self.book_two,
+                approved=False,
+                order=2,
+            )
+            views.list.add_book(request)
+
+        items = self.list.listitem_set.order_by("order").all()
+        self.assertEqual(items[0].book, self.book)
+        self.assertEqual(items[0].order, 1)
+        self.assertTrue(items[0].approved)
+
+        self.assertEqual(items[1].book, self.book_three)
+        self.assertEqual(items[1].order, 2)
+        self.assertTrue(items[1].approved)
+
+        self.assertEqual(items[2].book, self.book_two)
+        self.assertEqual(items[2].order, 3)
+        self.assertFalse(items[2].approved)
+
+    def test_approving_one_pending_book_from_multiple(self):
+        """
+        When a list contains any pending books, the pending books should have
+        be at the end of the list by order. If a pending book is approved, then
+        its order should be at the end of the approved books and before the
+        remaining pending books.
+        """
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            models.ListItem.objects.create(
+                book_list=self.list,
+                user=self.local_user,
+                book=self.book,
+                approved=True,
+                order=1,
+            )
+            models.ListItem.objects.create(
+                book_list=self.list,
+                user=self.local_user,
+                book=self.book_two,
+                approved=True,
+                order=2,
+            )
+            models.ListItem.objects.create(
+                book_list=self.list,
+                user=self.rat,
+                book=self.book_three,
+                approved=False,
+                order=3,
+            )
+            to_be_approved = models.ListItem.objects.create(
+                book_list=self.list,
+                user=self.rat,
+                book=self.book_four,
+                approved=False,
+                order=4,
+            )
+
+        view = views.Curate.as_view()
+        request = self.factory.post(
+            "",
+            {
+                "item": to_be_approved.id,
+                "approved": "true",
+            },
+        )
+        request.user = self.local_user
+
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            view(request, self.list.id)
+
+        items = self.list.listitem_set.order_by("order").all()
+        self.assertEqual(items[0].book, self.book)
+        self.assertEqual(items[0].order, 1)
+        self.assertTrue(items[0].approved)
+
+        self.assertEqual(items[1].book, self.book_two)
+        self.assertEqual(items[1].order, 2)
+        self.assertTrue(items[1].approved)
+
+        self.assertEqual(items[2].book, self.book_four)
+        self.assertEqual(items[2].order, 3)
+        self.assertTrue(items[2].approved)
+
+        self.assertEqual(items[3].book, self.book_three)
+        self.assertEqual(items[3].order, 4)
+        self.assertFalse(items[3].approved)
+
+    def test_add_three_books_and_move_last_to_first(self):
+        """
+        Put three books on the list and move the last book to the first
+        position.
+        """
+        request_one = self.factory.post(
+            "",
+            {
+                "book": self.book.id,
+                "list": self.list.id,
+            },
+        )
+        request_one.user = self.local_user
+
+        request_two = self.factory.post(
+            "",
+            {
+                "book": self.book_two.id,
+                "list": self.list.id,
+            },
+        )
+        request_two.user = self.local_user
+
+        request_three = self.factory.post(
+            "",
+            {
+                "book": self.book_three.id,
+                "list": self.list.id,
+            },
+        )
+        request_three.user = self.local_user
+
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            views.list.add_book(request_one)
+            views.list.add_book(request_two)
+            views.list.add_book(request_three)
+
+        items = self.list.listitem_set.order_by("order").all()
+        self.assertEqual(items[0].book, self.book)
+        self.assertEqual(items[1].book, self.book_two)
+        self.assertEqual(items[2].book, self.book_three)
+        self.assertEqual(items[0].order, 1)
+        self.assertEqual(items[1].order, 2)
+        self.assertEqual(items[2].order, 3)
+
+        set_position_request = self.factory.post("", {"position": 1})
+        set_position_request.user = self.local_user
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            views.list.set_book_position(set_position_request, items[2].id)
+        items = self.list.listitem_set.order_by("order").all()
+        self.assertEqual(items[0].book, self.book_three)
+        self.assertEqual(items[1].book, self.book)
+        self.assertEqual(items[2].book, self.book_two)
+        self.assertEqual(items[0].order, 1)
+        self.assertEqual(items[1].order, 2)
+        self.assertEqual(items[2].order, 3)
+
     def test_add_book_outsider(self):
         """ put a book on a list """
         self.list.curation = "open"
@@ -358,6 +634,7 @@ class ListViews(TestCase):
                 book_list=self.list,
                 user=self.local_user,
                 book=self.book,
+                order=1,
             )
         self.assertTrue(self.list.listitem_set.exists())
 
@@ -369,16 +646,15 @@ class ListViews(TestCase):
         )
         request.user = self.local_user
 
-        views.list.remove_book(request, self.list.id)
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            views.list.remove_book(request, self.list.id)
         self.assertFalse(self.list.listitem_set.exists())
 
     def test_remove_book_unauthorized(self):
         """ take an item off a list """
         with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
             item = models.ListItem.objects.create(
-                book_list=self.list,
-                user=self.local_user,
-                book=self.book,
+                book_list=self.list, user=self.local_user, book=self.book, order=1
             )
         self.assertTrue(self.list.listitem_set.exists())
         request = self.factory.post(

bookwyrm/tests/views/test_reports.py

@@ -1,5 +1,4 @@
 """ test for app action functionality """
-from unittest.mock import patch
 from django.template.response import TemplateResponse
 from django.test import TestCase
 from django.test.client import RequestFactory
@@ -115,22 +114,19 @@ class ReportViews(TestCase):
         report.refresh_from_db()
         self.assertFalse(report.resolved)
 
-    def test_deactivate_user(self):
+    def test_suspend_user(self):
         """ toggle whether a user is able to log in """
         self.assertTrue(self.rat.is_active)
-        report = models.Report.objects.create(reporter=self.local_user, user=self.rat)
         request = self.factory.post("")
         request.user = self.local_user
         request.user.is_superuser = True
 
         # de-activate
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
-            views.deactivate_user(request, report.id)
+        views.suspend_user(request, self.rat.id)
         self.rat.refresh_from_db()
         self.assertFalse(self.rat.is_active)
 
         # re-activate
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
-            views.deactivate_user(request, report.id)
+        views.suspend_user(request, self.rat.id)
         self.rat.refresh_from_db()
         self.assertTrue(self.rat.is_active)

bookwyrm/tests/views/test_user.py

@@ -30,6 +30,14 @@ class UserViews(TestCase):
         self.rat = models.User.objects.create_user(
             "rat@local.com", "rat@rat.rat", "password", local=True, localname="rat"
         )
+        self.book = models.Edition.objects.create(title="test")
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            models.ShelfBook.objects.create(
+                book=self.book,
+                user=self.local_user,
+                shelf=self.local_user.shelf_set.first(),
+            )
+
         models.SiteSettings.objects.create()
         self.anonymous_user = AnonymousUser
         self.anonymous_user.is_authenticated = False

bookwyrm/tests/views/test_user_admin.py

@@ -1,4 +1,6 @@
 """ test for app action functionality """
+from unittest.mock import patch
+from django.contrib.auth.models import Group
 from django.template.response import TemplateResponse
 from django.test import TestCase
 from django.test.client import RequestFactory
@@ -21,9 +23,9 @@ class UserAdminViews(TestCase):
         )
         models.SiteSettings.objects.create()
 
-    def test_user_admin_page(self):
+    def test_user_admin_list_page(self):
         """ there are so many views, this just makes sure it LOADS """
-        view = views.UserAdmin.as_view()
+        view = views.UserAdminList.as_view()
         request = self.factory.get("")
         request.user = self.local_user
         request.user.is_superuser = True
@@ -31,3 +33,38 @@ class UserAdminViews(TestCase):
         self.assertIsInstance(result, TemplateResponse)
         result.render()
         self.assertEqual(result.status_code, 200)
+
+    def test_user_admin_page(self):
+        """ there are so many views, this just makes sure it LOADS """
+        view = views.UserAdmin.as_view()
+        request = self.factory.get("")
+        request.user = self.local_user
+        request.user.is_superuser = True
+
+        result = view(request, self.local_user.id)
+
+        self.assertIsInstance(result, TemplateResponse)
+        result.render()
+        self.assertEqual(result.status_code, 200)
+
+    def test_user_admin_page_post(self):
+        """ set the user's group """
+        group = Group.objects.create(name="editor")
+        self.assertEqual(
+            list(self.local_user.groups.values_list("name", flat=True)), []
+        )
+
+        view = views.UserAdmin.as_view()
+        request = self.factory.post("", {"groups": [group.id]})
+        request.user = self.local_user
+        request.user.is_superuser = True
+
+        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.delay"):
+            result = view(request, self.local_user.id)
+
+        self.assertIsInstance(result, TemplateResponse)
+        result.render()
+
+        self.assertEqual(
+            list(self.local_user.groups.values_list("name", flat=True)), ["editor"]
+        )