fix MoveUser errors and clean up

- minor template fixes
- notification logic fixes
- don't dedupe on moved_to or also_known_as
- add migration
This commit is contained in:
Hugh Rundle 2023-09-25 15:14:21 +10:00
parent 5b051631ec
commit c95f160216
No known key found for this signature in database
GPG key ID: A7E35779918253F9
9 changed files with 190 additions and 34 deletions

View file

@ -75,9 +75,6 @@ class Update(Verb):
"""update a model instance from the dataclass""" """update a model instance from the dataclass"""
if not self.object: if not self.object:
return 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( self.object.to_model(
allow_create=False, allow_external_connections=allow_external_connections allow_create=False, allow_external_connections=allow_external_connections
) )
@ -252,6 +249,11 @@ class Move(Verb):
if object_is_user: if object_is_user:
model = apps.get_model("bookwyrm.MoveUser") 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: else:
return return None

View file

@ -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",),
),
]

View file

@ -21,7 +21,6 @@ class Move(ActivityMixin, BookWyrmModel):
max_length=255, max_length=255,
blank=False, blank=False,
null=False, null=False,
deduplication_field=True,
activitypub_field="object", activitypub_field="object",
) )
@ -29,7 +28,6 @@ class Move(ActivityMixin, BookWyrmModel):
max_length=255, max_length=255,
blank=True, blank=True,
null=True, null=True,
deduplication_field=True,
default="", default="",
activitypub_field="origin", activitypub_field="origin",
) )
@ -54,16 +52,10 @@ class MoveUser(Move):
activitypub_field="target", 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): def save(self, *args, **kwargs):
"""update user info and broadcast it""" """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(): if self.user in self.target.also_known_as.all():
self.user.also_known_as.add(self.target.id) self.user.also_known_as.add(self.target.id)
@ -75,12 +67,11 @@ class MoveUser(Move):
kwargs[ kwargs[
"broadcast" "broadcast"
] = True # Only broadcast if we are initiating the Move ] = True # Only broadcast if we are initiating the Move
notify_followers = True
super().save(*args, **kwargs) 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) MoveUserNotification.objects.create(user=follower, target=self.user)
else: else:
@ -93,11 +84,11 @@ class MoveUserNotification(models.Model):
created_date = models.DateTimeField(auto_now_add=True) created_date = models.DateTimeField(auto_now_add=True)
user = models.ForeignKey( 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 ) # user we are notifying
target = models.ForeignKey( 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 ) # new account of user who moved
def save(self, *args, **kwargs): def save(self, *args, **kwargs):

View file

@ -143,12 +143,14 @@ class User(OrderedCollectionPageMixin, AbstractUser):
# migration fields # migration fields
moved_to = fields.RemoteIdField( 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( also_known_as = fields.ManyToManyField(
"self", "self",
symmetrical=True, symmetrical=False,
unique=False,
activitypub_field="alsoKnownAs", activitypub_field="alsoKnownAs",
deduplication_field=False,
) )
# options to turn features on and off # options to turn features on and off

View file

@ -35,6 +35,6 @@
{% include 'notifications/items/update.html' %} {% include 'notifications/items/update.html' %}
{% elif notification.notification_type == 'GROUP_DESCRIPTION' %} {% elif notification.notification_type == 'GROUP_DESCRIPTION' %}
{% include 'notifications/items/update.html' %} {% include 'notifications/items/update.html' %}
{% elif notification.notification_type == 'MOVE' %} {% elif notification.notification_type == 'MOVE' %}
{% include 'notifications/items/move_user.html' %} {% include 'notifications/items/move_user.html' %}
{% endif %} {% endif %}

View file

@ -18,16 +18,16 @@
</div> </div>
<form name="alias-user" action="{% url 'prefs-alias' %}" method="post"> <form name="alias-user" action="{% url 'prefs-alias' %}" method="post">
{% csrf_token %} {% csrf_token %}
<div class="field">
<label class="label" for="id_password">{% trans "Confirm password:" %}</label>
<input class="input {% if form.password.errors %}is-danger{% endif %}" type="password" name="password" id="id_password" required aria-describedby="desc_password">
{% include 'snippets/form_errors.html' with errors_list=form.password.errors id="desc_password" %}
</div>
<div class="field"> <div class="field">
<label class="label" for="id_target">{% trans "Enter the username for the account you want to add as an alias e.g. <em>user@example.com </em>:" %}</label> <label class="label" for="id_target">{% trans "Enter the username for the account you want to add as an alias e.g. <em>user@example.com </em>:" %}</label>
<input class="input {% if form.username.errors %}is-danger{% endif %}" type="text" name="username" id="id_username" required aria-describedby="desc_username"> <input class="input {% if form.username.errors %}is-danger{% endif %}" type="text" name="username" id="id_username" required aria-describedby="desc_username">
{% include 'snippets/form_errors.html' with errors_list=form.username.errors id="desc_username" %} {% include 'snippets/form_errors.html' with errors_list=form.username.errors id="desc_username" %}
</div> </div>
<div class="field">
<label class="label" for="id_password">{% trans "Confirm your password:" %}</label>
<input class="input {% if form.password.errors %}is-danger{% endif %}" type="password" name="password" id="id_password" required aria-describedby="desc_password">
{% include 'snippets/form_errors.html' with errors_list=form.password.errors id="desc_password" %}
</div>
<button type="submit" class="button is-success">{% trans "Create Alias" %}</button> <button type="submit" class="button is-success">{% trans "Create Alias" %}</button>
</form> </form>
</div> </div>

View file

@ -24,16 +24,16 @@
</div> </div>
<form name="move-user" action="{% url 'prefs-move' %}" method="post"> <form name="move-user" action="{% url 'prefs-move' %}" method="post">
{% csrf_token %} {% csrf_token %}
<div class="field">
<label class="label" for="id_password">{% trans "Confirm password:" %}</label>
<input class="input {% if form.password.errors %}is-danger{% endif %}" type="password" name="password" id="id_password" required aria-describedby="desc_password">
{% include 'snippets/form_errors.html' with errors_list=form.password.errors id="desc_password" %}
</div>
<div class="field"> <div class="field">
<label class="label" for="id_target">{% trans "Enter the username for the account you want to move to e.g. <em>user@example.com </em>:" %}</label> <label class="label" for="id_target">{% trans "Enter the username for the account you want to move to e.g. <em>user@example.com </em>:" %}</label>
<input class="input {% if form.target.errors %}is-danger{% endif %}" type="text" name="target" id="id_target" required aria-describedby="desc_target"> <input class="input {% if form.target.errors %}is-danger{% endif %}" type="text" name="target" id="id_target" required aria-describedby="desc_target">
{% include 'snippets/form_errors.html' with errors_list=form.target.errors id="desc_target" %} {% include 'snippets/form_errors.html' with errors_list=form.target.errors id="desc_target" %}
</div> </div>
<div class="field">
<label class="label" for="id_password">{% trans "Confirm your password:" %}</label>
<input class="input {% if form.password.errors %}is-danger{% endif %}" type="password" name="password" id="id_password" required aria-describedby="desc_password">
{% include 'snippets/form_errors.html' with errors_list=form.password.errors id="desc_password" %}
</div>
<button type="submit" class="button is-danger">{% trans "Move Account" %}</button> <button type="submit" class="button is-danger">{% trans "Move Account" %}</button>
</form> </form>
</div> </div>

View file

@ -7,7 +7,7 @@
<form action="{% url 'follow' %}" method="POST" class="interaction follow_{{ related_users.0.id }}" data-id="follow_{{ related_users.0.id }}"> <form action="{% url 'follow' %}" method="POST" class="interaction follow_{{ related_users.0.id }}" data-id="follow_{{ related_users.0.id }}">
{% csrf_token %} {% csrf_token %}
<input type="hidden" name="user" value="{% id_to_username related_user_moved_to %}"> <input type="hidden" name="user" value="{% id_to_username related_user_moved_to %}">
<button class="button is-link is-small" type="submit">{% trans "Follow new account" %}</button> <button class="button is-link is-small" type="submit">{% trans "Follow at new account" %}</button>
</form> </form>
</div> </div>
{% endif %} {% endif %}

View file

@ -39,7 +39,7 @@ class MoveUser(View):
user=request.user, object=request.user.remote_id, target=target user=request.user, object=request.user.remote_id, target=target
) )
return redirect("/") return redirect("user-feed", username=request.user.username)
except (PermissionDenied): except (PermissionDenied):
form.errors["target"] = [ form.errors["target"] = [