From c95f160216f10da14237a925c47a42ed3b84965c Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Mon, 25 Sep 2023 15:14:21 +1000 Subject: [PATCH] fix MoveUser errors and clean up - minor template fixes - notification logic fixes - don't dedupe on moved_to or also_known_as - add migration --- bookwyrm/activitypub/verbs.py | 12 +- .../migrations/0182_auto_20230924_0821.py | 161 ++++++++++++++++++ bookwyrm/models/move.py | 19 +-- bookwyrm/models/user.py | 6 +- bookwyrm/templates/notifications/item.html | 2 +- .../templates/preferences/alias_user.html | 10 +- bookwyrm/templates/preferences/move_user.html | 10 +- .../templates/snippets/move_user_buttons.html | 2 +- bookwyrm/views/preferences/move_user.py | 2 +- 9 files changed, 190 insertions(+), 34 deletions(-) create mode 100644 bookwyrm/migrations/0182_auto_20230924_0821.py diff --git a/bookwyrm/activitypub/verbs.py b/bookwyrm/activitypub/verbs.py index 0b04d9eba..bb6000459 100644 --- a/bookwyrm/activitypub/verbs.py +++ b/bookwyrm/activitypub/verbs.py @@ -75,9 +75,6 @@ class Update(Verb): """update a model instance from the dataclass""" if not self.object: return - # BUG: THIS IS THROWING A DUPLIATE USERNAME ERROR WHEN WE GET AN "UPDATE" AFTER/BEFORE A "MOVE" - # FROM MASTODON - BUT ONLY SINCE WE ADDED MOVEUSER - # is it something to do with the updated User model? self.object.to_model( allow_create=False, allow_external_connections=allow_external_connections ) @@ -252,6 +249,11 @@ class Move(Verb): if object_is_user: model = apps.get_model("bookwyrm.MoveUser") - self.to_model(model=model) + + self.to_model( + model=model, + save=True, + allow_external_connections=allow_external_connections, + ) else: - return + return None diff --git a/bookwyrm/migrations/0182_auto_20230924_0821.py b/bookwyrm/migrations/0182_auto_20230924_0821.py new file mode 100644 index 000000000..24f25b392 --- /dev/null +++ b/bookwyrm/migrations/0182_auto_20230924_0821.py @@ -0,0 +1,161 @@ +# Generated by Django 3.2.20 on 2023-09-24 08:21 + +import bookwyrm.models.activitypub_mixin +import bookwyrm.models.fields +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ("bookwyrm", "0181_merge_20230806_2302"), + ] + + operations = [ + migrations.AddField( + model_name="user", + name="also_known_as", + field=bookwyrm.models.fields.ManyToManyField(to=settings.AUTH_USER_MODEL), + ), + migrations.AddField( + model_name="user", + name="moved_to", + field=bookwyrm.models.fields.RemoteIdField( + max_length=255, + null=True, + validators=[bookwyrm.models.fields.validate_remote_id], + ), + ), + migrations.AlterField( + model_name="notification", + name="notification_type", + field=models.CharField( + choices=[ + ("FAVORITE", "Favorite"), + ("REPLY", "Reply"), + ("MENTION", "Mention"), + ("TAG", "Tag"), + ("FOLLOW", "Follow"), + ("FOLLOW_REQUEST", "Follow Request"), + ("BOOST", "Boost"), + ("IMPORT", "Import"), + ("ADD", "Add"), + ("REPORT", "Report"), + ("LINK_DOMAIN", "Link Domain"), + ("INVITE", "Invite"), + ("ACCEPT", "Accept"), + ("JOIN", "Join"), + ("LEAVE", "Leave"), + ("REMOVE", "Remove"), + ("GROUP_PRIVACY", "Group Privacy"), + ("GROUP_NAME", "Group Name"), + ("GROUP_DESCRIPTION", "Group Description"), + ("MOVE", "Move"), + ], + max_length=255, + ), + ), + migrations.CreateModel( + name="MoveUserNotification", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("created_date", models.DateTimeField(auto_now_add=True)), + ( + "target", + models.ForeignKey( + on_delete=django.db.models.deletion.PROTECT, + related_name="moved_user_notification_target", + to=settings.AUTH_USER_MODEL, + ), + ), + ( + "user", + models.ForeignKey( + on_delete=django.db.models.deletion.PROTECT, + related_name="moved_user_notifications", + to=settings.AUTH_USER_MODEL, + ), + ), + ], + ), + migrations.CreateModel( + name="Move", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ("created_date", models.DateTimeField(auto_now_add=True)), + ("updated_date", models.DateTimeField(auto_now=True)), + ( + "remote_id", + bookwyrm.models.fields.RemoteIdField( + max_length=255, + null=True, + validators=[bookwyrm.models.fields.validate_remote_id], + ), + ), + ("object", bookwyrm.models.fields.CharField(max_length=255)), + ( + "origin", + bookwyrm.models.fields.CharField( + blank=True, default="", max_length=255, null=True + ), + ), + ( + "user", + bookwyrm.models.fields.ForeignKey( + on_delete=django.db.models.deletion.PROTECT, + to=settings.AUTH_USER_MODEL, + ), + ), + ], + options={ + "abstract": False, + }, + bases=(bookwyrm.models.activitypub_mixin.ActivityMixin, models.Model), + ), + migrations.CreateModel( + name="MoveUser", + fields=[ + ( + "move_ptr", + models.OneToOneField( + auto_created=True, + on_delete=django.db.models.deletion.CASCADE, + parent_link=True, + primary_key=True, + serialize=False, + to="bookwyrm.move", + ), + ), + ( + "target", + bookwyrm.models.fields.ForeignKey( + on_delete=django.db.models.deletion.PROTECT, + related_name="move_target", + to=settings.AUTH_USER_MODEL, + ), + ), + ], + options={ + "abstract": False, + }, + bases=("bookwyrm.move",), + ), + ] diff --git a/bookwyrm/models/move.py b/bookwyrm/models/move.py index 5c8044ce3..e5c1d4719 100644 --- a/bookwyrm/models/move.py +++ b/bookwyrm/models/move.py @@ -21,7 +21,6 @@ class Move(ActivityMixin, BookWyrmModel): max_length=255, blank=False, null=False, - deduplication_field=True, activitypub_field="object", ) @@ -29,7 +28,6 @@ class Move(ActivityMixin, BookWyrmModel): max_length=255, blank=True, null=True, - deduplication_field=True, default="", activitypub_field="origin", ) @@ -54,16 +52,10 @@ class MoveUser(Move): activitypub_field="target", ) - # pylint: disable=unused-argument - @classmethod - def ignore_activity(cls, activity, allow_external_connections=True): - """don't bother with incoming moves of unknown users""" - return not User.objects.filter(remote_id=activity.origin).exists() - def save(self, *args, **kwargs): """update user info and broadcast it""" - notify_followers = False + # only allow if the source is listed in the target's alsoKnownAs if self.user in self.target.also_known_as.all(): self.user.also_known_as.add(self.target.id) @@ -75,12 +67,11 @@ class MoveUser(Move): kwargs[ "broadcast" ] = True # Only broadcast if we are initiating the Move - notify_followers = True super().save(*args, **kwargs) - if notify_followers: - for follower in self.user.followers.all(): + for follower in self.user.followers.all(): + if follower.local: MoveUserNotification.objects.create(user=follower, target=self.user) else: @@ -93,11 +84,11 @@ class MoveUserNotification(models.Model): created_date = models.DateTimeField(auto_now_add=True) user = models.ForeignKey( - "User", on_delete=models.CASCADE, related_name="moved_user_notifications" + "User", on_delete=models.PROTECT, related_name="moved_user_notifications" ) # user we are notifying target = models.ForeignKey( - "User", on_delete=models.CASCADE, related_name="moved_user_notification_target" + "User", on_delete=models.PROTECT, related_name="moved_user_notification_target" ) # new account of user who moved def save(self, *args, **kwargs): diff --git a/bookwyrm/models/user.py b/bookwyrm/models/user.py index 6c87af18c..c152cf445 100644 --- a/bookwyrm/models/user.py +++ b/bookwyrm/models/user.py @@ -143,12 +143,14 @@ class User(OrderedCollectionPageMixin, AbstractUser): # migration fields moved_to = fields.RemoteIdField( - null=True, unique=False, activitypub_field="movedTo" + null=True, unique=False, activitypub_field="movedTo", deduplication_field=False ) also_known_as = fields.ManyToManyField( "self", - symmetrical=True, + symmetrical=False, + unique=False, activitypub_field="alsoKnownAs", + deduplication_field=False, ) # options to turn features on and off diff --git a/bookwyrm/templates/notifications/item.html b/bookwyrm/templates/notifications/item.html index 6c9b295f7..bac7dc61f 100644 --- a/bookwyrm/templates/notifications/item.html +++ b/bookwyrm/templates/notifications/item.html @@ -35,6 +35,6 @@ {% include 'notifications/items/update.html' %} {% elif notification.notification_type == 'GROUP_DESCRIPTION' %} {% include 'notifications/items/update.html' %} - {% elif notification.notification_type == 'MOVE' %} +{% elif notification.notification_type == 'MOVE' %} {% include 'notifications/items/move_user.html' %} {% endif %} diff --git a/bookwyrm/templates/preferences/alias_user.html b/bookwyrm/templates/preferences/alias_user.html index 493bf83de..3cad65c35 100644 --- a/bookwyrm/templates/preferences/alias_user.html +++ b/bookwyrm/templates/preferences/alias_user.html @@ -18,16 +18,16 @@
{% csrf_token %} -
- - - {% include 'snippets/form_errors.html' with errors_list=form.password.errors id="desc_password" %} -
{% include 'snippets/form_errors.html' with errors_list=form.username.errors id="desc_username" %}
+
+ + + {% include 'snippets/form_errors.html' with errors_list=form.password.errors id="desc_password" %} +
diff --git a/bookwyrm/templates/preferences/move_user.html b/bookwyrm/templates/preferences/move_user.html index 95fa4811f..444e5aa5e 100644 --- a/bookwyrm/templates/preferences/move_user.html +++ b/bookwyrm/templates/preferences/move_user.html @@ -24,16 +24,16 @@
{% csrf_token %} -
- - - {% include 'snippets/form_errors.html' with errors_list=form.password.errors id="desc_password" %} -
{% include 'snippets/form_errors.html' with errors_list=form.target.errors id="desc_target" %}
+
+ + + {% include 'snippets/form_errors.html' with errors_list=form.password.errors id="desc_password" %} +
diff --git a/bookwyrm/templates/snippets/move_user_buttons.html b/bookwyrm/templates/snippets/move_user_buttons.html index e3a9cce39..fc8f792e9 100644 --- a/bookwyrm/templates/snippets/move_user_buttons.html +++ b/bookwyrm/templates/snippets/move_user_buttons.html @@ -7,7 +7,7 @@ {% endif %} \ No newline at end of file diff --git a/bookwyrm/views/preferences/move_user.py b/bookwyrm/views/preferences/move_user.py index cc04af4b1..57abef3ea 100644 --- a/bookwyrm/views/preferences/move_user.py +++ b/bookwyrm/views/preferences/move_user.py @@ -39,7 +39,7 @@ class MoveUser(View): user=request.user, object=request.user.remote_id, target=target ) - return redirect("/") + return redirect("user-feed", username=request.user.username) except (PermissionDenied): form.errors["target"] = [