Merge pull request #3156 from hughrun/user-migration

fix upsert_statuses

bookwyrm/models/bookwyrm_import_job.py

@@ -178,21 +178,25 @@ def upsert_statuses(user, cls, data, book_remote_id):
     find or create the instances in the database"""
 
     for status in data:
-
+        if is_alias(
+            user, status["attributedTo"]
+        ):  # don't let l33t hax0rs steal other people's posts
             # update ids and remove replies
             status["attributedTo"] = user.remote_id
             status["to"] = update_followers_address(user, status["to"])
             status["cc"] = update_followers_address(user, status["cc"])
             status[
                 "replies"
-        ] = {}  # this parses incorrectly but we can't set it without knowing the new id
+            ] = (
+                {}
+            )  # this parses incorrectly but we can't set it without knowing the new id
             status["inReplyToBook"] = book_remote_id
-
-        # save new status or do update it if it already exists
             parsed = activitypub.parse(status)
-        instance = parsed.to_model(model=cls, save=True, overwrite=True)
+            if not status_already_exists(
+                user, parsed
+            ):  # don't duplicate posts on multiple import
 
-        print(instance.id, instance.privacy)
+                instance = parsed.to_model(model=cls, save=True, overwrite=True)
 
                 for val in [
                     "progress",
@@ -202,9 +206,13 @@ def upsert_statuses(user, cls, data, book_remote_id):
                     "position_mode",
                 ]:
                     if status.get(val):
-                print(val, status[val])
                         instance.val = status[val]
-        instance.save()
+
+                instance.remote_id = instance.get_remote_id()  # update the remote_id
+                instance.save()  # save and broadcast
+
+        else:
+            logger.info("User does not have permission to import statuses")
 
 
 def upsert_lists(user, lists, book_id):
@@ -369,7 +377,7 @@ def upsert_follows(user, values):
 
             if not created:
                 # this request probably failed to connect with the remote
-                # that means we should save to trigger a re-broadcast
+                # and should save to trigger a re-broadcast
                 follow_request.save()
 
 
@@ -419,3 +427,33 @@ def update_followers_address(user, field):
             field[i] = user.followers_url
 
     return field
+
+
+def is_alias(user, remote_id):
+    """check that the user is listed as movedTo or also_known_as
+    in the remote user's profile"""
+
+    remote_user = activitypub.resolve_remote_id(
+        remote_id=remote_id, model=models.User, save=False
+    )
+
+    if remote_user:
+
+        if remote_user.moved_to:
+            return user.remote_id == remote_user.moved_to
+
+        if remote_user.also_known_as:
+            return user in remote_user.also_known_as.all()
+
+    return False
+
+
+def status_already_exists(user, status):
+    """check whether this status has already been published
+    by this user. We can't rely on to_model() because it
+    only matches on remote_id, which we have to change
+    *after* saving because it needs the primary key (id)"""
+
+    return models.Status.objects.filter(
+        user=user, content=status.content, published_date=status.published
+    ).exists()

bookwyrm/templates/import/import_user.html

@@ -13,7 +13,11 @@
         {% trans "Not a valid import file" %}
     </div>
     {% endif %}
-
+    <p class="notification is-warning">
+        {% spaceless %}
+        {% trans "If you wish to migrate any statuses (comments, reviews, or quotes) you must either set this account as an <strong>alias</strong> of the one you are migrating from, or <strong>move</strong> that account to this one, before you import your user data." %}
+        {% endspaceless %}
+    </p>
     {% if not site.imports_enabled %}
     <div class="box notification has-text-centered is-warning m-6 content">
         <p class="mt-5">

bookwyrm/templates/preferences/export-user.html

@@ -41,6 +41,11 @@
             {% endblocktrans %}
         </div>
         <p class="block">{% trans "In your new BookWyrm account can choose what to import: you will not have to import everything that is exported." %}</p>
+        <p class="notification is-warning">
+            {% spaceless %}
+            {% trans "If you wish to migrate any statuses (comments, reviews, or quotes) you must either set the account you are moving to as an <strong>alias</strong> of this one, or <strong>move</strong> this account to the new account, before you import your user data." %}
+            {% endspaceless %}
+        </p>
     {% if next_available %}
     <p class="notification is-warning">
         {% blocktrans trimmed %}

bookwyrm/tests/models/test_bookwyrm_import_job.py

@@ -312,7 +312,10 @@ class BookwyrmImport(TestCase):  # pylint: disable=too-many-public-methods
 
         self.assertEqual(models.Review.objects.filter(user=self.local_user).count(), 0)
         reviews = self.json_data["books"][0]["reviews"]
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
+        with patch(
+            "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
+        ), patch("bookwyrm.models.bookwyrm_import_job.is_alias", return_value=True):
+
             bookwyrm_import_job.upsert_statuses(
                 self.local_user, models.Review, reviews, self.book.remote_id
             )
@@ -345,7 +348,11 @@ class BookwyrmImport(TestCase):  # pylint: disable=too-many-public-methods
 
         self.assertEqual(models.Comment.objects.filter(user=self.local_user).count(), 0)
         comments = self.json_data["books"][1]["comments"]
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
+
+        with patch(
+            "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
+        ), patch("bookwyrm.models.bookwyrm_import_job.is_alias", return_value=True):
+
             bookwyrm_import_job.upsert_statuses(
                 self.local_user, models.Comment, comments, self.book.remote_id
             )
@@ -371,7 +378,10 @@ class BookwyrmImport(TestCase):  # pylint: disable=too-many-public-methods
             models.Quotation.objects.filter(user=self.local_user).count(), 0
         )
         quotes = self.json_data["books"][1]["quotations"]
-        with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
+        with patch(
+            "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
+        ), patch("bookwyrm.models.bookwyrm_import_job.is_alias", return_value=True):
+
             bookwyrm_import_job.upsert_statuses(
                 self.local_user, models.Quotation, quotes, self.book.remote_id
             )
@@ -394,6 +404,24 @@ class BookwyrmImport(TestCase):  # pylint: disable=too-many-public-methods
             models.Quotation.objects.filter(book=self.book).first().position_mode, "PG"
         )
 
+    def test_get_or_create_quote_unauthorized(self):
+        """Test get_or_create_review_status with a quote but not authorized"""
+
+        self.assertEqual(
+            models.Quotation.objects.filter(user=self.local_user).count(), 0
+        )
+        quotes = self.json_data["books"][1]["quotations"]
+        with patch(
+            "bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
+        ), patch("bookwyrm.models.bookwyrm_import_job.is_alias", return_value=False):
+
+            bookwyrm_import_job.upsert_statuses(
+                self.local_user, models.Quotation, quotes, self.book.remote_id
+            )
+        self.assertEqual(
+            models.Quotation.objects.filter(user=self.local_user).count(), 0
+        )
+
     def test_upsert_list_existing(self):
         """Take a list and ListItems as JSON and create DB entries
         if they don't already exist"""