fix reject PR

- rationalise activitypub.Reject and fix model being undefined
- fix not being able to follow users from followers page: 'delete' option now in user_options dropdown
- revert bookwyrm.js
- fix delete_follow_request deleting instead of rejecting
- add user id to 'remove-follow' path
This commit is contained in:
Hugh Rundle 2023-11-21 20:13:56 +11:00
parent 2ba0e3d7ff
commit 2c9ebba5d7
No known key found for this signature in database
GPG key ID: A7E35779918253F9
8 changed files with 49 additions and 67 deletions

View file

@ -173,35 +173,17 @@ class Reject(Verb):
def action(self, allow_external_connections=True): def action(self, allow_external_connections=True):
"""reject a follow or follow request""" """reject a follow or follow request"""
if self.object.type == "Follow": for model_name in ["UserFollowRequest", "UserFollows", None]:
model = apps.get_model("bookwyrm.UserFollowRequest") model = apps.get_model(f"bookwyrm.{model_name}") if model_name else None
obj = self.object.to_model( if obj := self.object.to_model(
model=model, model=model,
save=False, save=False,
allow_create=False, allow_create=False,
allow_external_connections=allow_external_connections, allow_external_connections=allow_external_connections,
) ):
if not obj: # Reject the first model that can be built.
# This is a deletion (soft-block) of an accepted follow obj.reject()
model = apps.get_model("bookwyrm.UserFollows") break
obj = self.object.to_model(
model=model,
save=False,
allow_create=False,
allow_external_connections=allow_external_connections,
)
else:
# it's something else
obj = self.object.to_model(
model=model,
save=False,
allow_create=False,
allow_external_connections=allow_external_connections,
)
if not obj:
# if we don't have the object, we can't reject it.
return
obj.reject()
@dataclass(init=False) @dataclass(init=False)

View file

@ -330,30 +330,17 @@ let BookWyrm = new (class {
const bookwyrm = this; const bookwyrm = this;
const form = event.currentTarget; const form = event.currentTarget;
const formAction = event.submitter.getAttribute("formaction") || form.action;
const relatedforms = document.querySelectorAll(`.${form.dataset.id}`); const relatedforms = document.querySelectorAll(`.${form.dataset.id}`);
// Toggle class on all related forms. relatedforms.forEach((relatedForm) =>
if (formAction == "/remove-follow") { bookwyrm.addRemoveClass(
// Remove ALL follow/unfollow/remote buttons relatedForm,
relatedforms.forEach((relatedForm) => relatedForm.classList.add("is-hidden")); "is-hidden",
relatedForm.className.indexOf("is-hidden") == -1
)
);
// Remove orphaned user-options dropdown this.ajaxPost(form).catch((error) => {
const parent = form.parentElement;
const next = parent.nextElementSibling;
next.classList.add("is-hidden");
} else {
relatedforms.forEach((relatedForm) =>
bookwyrm.addRemoveClass(
relatedForm,
"is-hidden",
relatedForm.className.indexOf("is-hidden") == -1
)
);
}
this.ajaxPost(formAction, form).catch((error) => {
// @todo Display a notification in the UI instead. // @todo Display a notification in the UI instead.
console.warn("Request failed:", error); console.warn("Request failed:", error);
}); });
@ -365,8 +352,8 @@ let BookWyrm = new (class {
* @param {object} form - Form to be submitted * @param {object} form - Form to be submitted
* @return {Promise} * @return {Promise}
*/ */
ajaxPost(target, form) { ajaxPost(form) {
return fetch(target, { return fetch(form.action, {
method: "POST", method: "POST",
body: new FormData(form), body: new FormData(form),
headers: { headers: {

View file

@ -12,7 +12,6 @@
<div class="field{% if not minimal %} has-addons{% else %} mb-0{% endif %}"> <div class="field{% if not minimal %} has-addons{% else %} mb-0{% endif %}">
<div class="control"> <div class="control">
{% if not followers_page %}
<form action="{% url 'follow' %}" method="POST" class="interaction follow_{{ user.id }} {% if relationship.is_following or relationship.is_follow_pending %}is-hidden{%endif %}" data-id="follow_{{ user.id }}"> <form action="{% url 'follow' %}" method="POST" class="interaction follow_{{ user.id }} {% if relationship.is_following or relationship.is_follow_pending %}is-hidden{%endif %}" data-id="follow_{{ user.id }}">
{% csrf_token %} {% csrf_token %}
<input type="hidden" name="user" value="{{ user.username }}"> <input type="hidden" name="user" value="{{ user.username }}">
@ -24,22 +23,13 @@
{% endif %} {% endif %}
</button> </button>
</form> </form>
{% endif %} <form action="{% url 'unfollow' %}" method="POST" class="interaction follow_{{ user.id }} {% if not relationship.is_following and not relationship.is_follow_pending %}is-hidden{%endif %}" data-id="follow_{{ user.id }}">
<form action="{% url 'unfollow' %}" method="POST" class="interaction follow_{{ user.id }}" data-id="follow_{{ user.id }}">
{% csrf_token %} {% csrf_token %}
<input type="hidden" name="user" value="{{ user.username }}"> <input type="hidden" name="user" value="{{ user.username }}">
{% if relationship.is_follow_pending %} {% if relationship.is_follow_pending %}
<button class="button is-small is-danger is-light" type="submit"> <button class="button is-small is-danger is-light" type="submit">
{% trans "Undo follow request" %} {% trans "Undo follow request" %}
</button> </button>
{% elif followers_page %}
<button class="button is-small is-danger is-light" type="submit" formaction="{% url 'remove-follow' %}">
{% if show_username %}
{% blocktrans with username=user.localname %}Remove @{{ username }}{% endblocktrans %}
{% else %}
{% trans "Remove" %}
{% endif %}
</button>
{% else %} {% else %}
<button class="button is-small is-danger is-light" type="submit"> <button class="button is-small is-danger is-light" type="submit">
{% if show_username %} {% if show_username %}
@ -53,7 +43,7 @@
</div> </div>
{% if not minimal %} {% if not minimal %}
<div class="control"> <div class="control">
{% include 'snippets/user_options.html' with user=user class="is-small" %} {% include 'snippets/user_options.html' with user=user followers_page=followers_page class="is-small" %}
</div> </div>
{% endif %} {% endif %}
</div> </div>

View file

@ -0,0 +1,5 @@
{% load i18n %}
<form name="remove" method="post" action="/remove-follow/{{ user.id }}">
{% csrf_token %}
<button class="button is-danger is-light is-small {{ class }}" type="submit">{% trans "Remove" %}</button>
</form>

View file

@ -20,4 +20,9 @@
<li role="menuitem"> <li role="menuitem">
{% include 'snippets/block_button.html' with user=user class="is-fullwidth" blocks=False %} {% include 'snippets/block_button.html' with user=user class="is-fullwidth" blocks=False %}
</li> </li>
{% if followers_page %}
<li role="menuitem">
{% include 'snippets/remove_follower_button.html' with user=user class="is-fullwidth" blocks=False %}
</li>
{% endif %}
{% endblock %} {% endblock %}

View file

@ -180,6 +180,18 @@ class FollowViews(TestCase):
# follow relationship should not exist # follow relationship should not exist
self.assertEqual(models.UserFollows.objects.filter(id=rel.id).count(), 0) self.assertEqual(models.UserFollows.objects.filter(id=rel.id).count(), 0)
def test_handle_reject_existing(self, *_):
"""reject a follow previously approved"""
request = self.factory.post("", {"user": self.remote_user.username})
request.user = self.local_user
rel = models.UserFollows.objects.create(
user_subject=self.remote_user, user_object=self.local_user
)
with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
views.remove_follow(request, self.remote_user.id)
# follow relationship should not exist
self.assertEqual(models.UserFollows.objects.filter(id=rel.id).count(), 0)
def test_ostatus_follow_request(self, *_): def test_ostatus_follow_request(self, *_):
"""check ostatus subscribe template loads""" """check ostatus subscribe template loads"""
request = self.factory.get( request = self.factory.get(

View file

@ -763,7 +763,9 @@ urlpatterns = [
# following # following
re_path(r"^follow/?$", views.follow, name="follow"), re_path(r"^follow/?$", views.follow, name="follow"),
re_path(r"^unfollow/?$", views.unfollow, name="unfollow"), re_path(r"^unfollow/?$", views.unfollow, name="unfollow"),
re_path(r"^remove-follow/?$", views.remove_follow, name="remove-follow"), re_path(
r"^remove-follow/(?P<user_id>\d+)/?$", views.remove_follow, name="remove-follow"
),
re_path(r"^accept-follow-request/?$", views.accept_follow_request), re_path(r"^accept-follow-request/?$", views.accept_follow_request),
re_path(r"^delete-follow-request/?$", views.delete_follow_request), re_path(r"^delete-follow-request/?$", views.delete_follow_request),
re_path(r"^ostatus_follow/?$", views.remote_follow, name="remote-follow"), re_path(r"^ostatus_follow/?$", views.remote_follow, name="remote-follow"),

View file

@ -71,11 +71,10 @@ def unfollow(request):
@login_required @login_required
@require_POST @require_POST
def remove_follow(request): def remove_follow(request, user_id):
"""remove a previously approved follower without blocking them""" """remove a previously approved follower without blocking them"""
username = request.POST["user"] to_remove = get_object_or_404(models.User, id=user_id)
to_remove = get_user_from_username(request.user, username)
try: try:
models.UserFollows.objects.get( models.UserFollows.objects.get(
@ -93,8 +92,8 @@ def remove_follow(request):
if is_api_request(request): if is_api_request(request):
return HttpResponse() return HttpResponse()
# this is handled with ajax so it shouldn't really matter
return redirect("/") return redirect(f"{request.user.local_path}/followers")
@login_required @login_required
@ -128,7 +127,7 @@ def delete_follow_request(request):
) )
follow_request.raise_not_deletable(request.user) follow_request.raise_not_deletable(request.user)
follow_request.delete() follow_request.reject()
return redirect(f"/user/{request.user.localname}") return redirect(f"/user/{request.user.localname}")