Merge pull request #983 from bookwyrm-social/follow-bug

Fixes bug that causes remote follows to be removed

bookwyrm/models/fields.py

@@ -275,9 +275,12 @@ class ManyToManyField(ActivitypubFieldMixin, models.ManyToManyField):
         return [i.remote_id for i in value.all()]
 
     def field_from_activity(self, value):
-        items = []
         if value is None or value is MISSING:
-            return []
+            return None
+        if not isinstance(value, list):
+            # If this is a link, we currently aren't doing anything with it
+            return None
+        items = []
         for remote_id in value:
             try:
                 validate_remote_id(remote_id)

bookwyrm/tests/data/ap_user_rat.json

@@ -0,0 +1,39 @@
+{
+    "@context": [
+        "https://www.w3.org/ns/activitystreams",
+        "https://w3id.org/security/v1",
+        {
+            "manuallyApprovesFollowers": "as:manuallyApprovesFollowers",
+            "schema": "http://schema.org#",
+            "PropertyValue": "schema:PropertyValue",
+            "value": "schema:value"
+        }
+    ],
+    "id": "https://example.com/users/rat",
+    "type": "Person",
+    "preferredUsername": "rat",
+    "name": "RAT???",
+    "inbox": "https://example.com/users/rat/inbox",
+    "outbox": "https://example.com/users/rat/outbox",
+    "followers": "https://example.com/users/rat/followers",
+    "following": "https://example.com/users/rat/following",
+    "summary": "",
+    "publicKey": {
+        "id": "https://example.com/users/rat/#main-key",
+        "owner": "https://example.com/users/rat",
+        "publicKeyPem": "-----BEGIN PUBLIC KEY-----\nMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQC6QisDrjOQvkRo/MqNmSYPwqtt\nCxg/8rCW+9jKbFUKvqjTeKVotEE85122v/DCvobCCdfQuYIFdVMk+dB1xJ0iPGPg\nyU79QHY22NdV9mFKA2qtXVVxb5cxpA4PlwOHM6PM/k8B+H09OUrop2aPUAYwy+vg\n+MXyz8bAXrIS1kq6fQIDAQAB\n-----END PUBLIC KEY-----"
+    },
+    "endpoints": {
+        "sharedInbox": "https://example.com/inbox"
+    },
+    "bookwyrmUser": true,
+    "manuallyApprovesFollowers": false,
+    "discoverable": true,
+    "devices": "https://friend.camp/users/tripofmice/collections/devices",
+    "tag": [],
+    "icon": {
+        "type": "Image",
+        "mediaType": "image/png",
+        "url": "https://example.com/images/avatars/AL-2-crop-50.png"
+    }
+}

bookwyrm/tests/views/inbox/test_inbox_update.py

@@ -80,14 +80,22 @@ class InboxUpdate(TestCase):
 
     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",
@@ -98,12 +106,15 @@ 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)
 
+        # 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")