From 7fcadb1d4d9d4c5001fc734376cbd184bb8768d7 Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Wed, 13 Dec 2023 20:55:38 +1100 Subject: [PATCH] fix upsert_statuses - remote_id is now updated on import of statuses - statuses cannot be imported unless source has target listed in alsoKnownAs or movedTo - add alert boxes to import and export screens advising of the above - update tests accordingly --- bookwyrm/models/bookwyrm_import_job.py | 86 +++++++++++++------ bookwyrm/templates/import/import_user.html | 6 +- .../templates/preferences/export-user.html | 5 ++ .../tests/models/test_bookwyrm_import_job.py | 34 +++++++- 4 files changed, 103 insertions(+), 28 deletions(-) diff --git a/bookwyrm/models/bookwyrm_import_job.py b/bookwyrm/models/bookwyrm_import_job.py index 461f2cf0f..9a11fd932 100644 --- a/bookwyrm/models/bookwyrm_import_job.py +++ b/bookwyrm/models/bookwyrm_import_job.py @@ -178,33 +178,41 @@ 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 + status["inReplyToBook"] = book_remote_id + parsed = activitypub.parse(status) + if not status_already_exists( + user, parsed + ): # don't duplicate posts on multiple import - # 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 - status["inReplyToBook"] = book_remote_id + instance = parsed.to_model(model=cls, save=True, overwrite=True) - # 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) + for val in [ + "progress", + "progress_mode", + "position", + "endposition", + "position_mode", + ]: + if status.get(val): + instance.val = status[val] - print(instance.id, instance.privacy) + instance.remote_id = instance.get_remote_id() # update the remote_id + instance.save() # save and broadcast - for val in [ - "progress", - "progress_mode", - "position", - "endposition", - "position_mode", - ]: - if status.get(val): - print(val, status[val]) - instance.val = status[val] - instance.save() + 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() diff --git a/bookwyrm/templates/import/import_user.html b/bookwyrm/templates/import/import_user.html index f94236958..70b21673c 100644 --- a/bookwyrm/templates/import/import_user.html +++ b/bookwyrm/templates/import/import_user.html @@ -13,7 +13,11 @@ {% trans "Not a valid import file" %} {% endif %} - +

+ {% spaceless %} + {% trans "If you wish to migrate any statuses (comments, reviews, or quotes) you must either set this account as an alias of the one you are migrating from, or move that account to this one, before you import your user data." %} + {% endspaceless %} +

{% if not site.imports_enabled %}

diff --git a/bookwyrm/templates/preferences/export-user.html b/bookwyrm/templates/preferences/export-user.html index 8ecca1863..a468c3f74 100644 --- a/bookwyrm/templates/preferences/export-user.html +++ b/bookwyrm/templates/preferences/export-user.html @@ -41,6 +41,11 @@ {% endblocktrans %}

{% trans "In your new BookWyrm account can choose what to import: you will not have to import everything that is exported." %}

+

+ {% 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 alias of this one, or move this account to the new account, before you import your user data." %} + {% endspaceless %} +

{% if next_available %}

{% blocktrans trimmed %} diff --git a/bookwyrm/tests/models/test_bookwyrm_import_job.py b/bookwyrm/tests/models/test_bookwyrm_import_job.py index 3f72f7205..adc04706c 100644 --- a/bookwyrm/tests/models/test_bookwyrm_import_job.py +++ b/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"""