From 2c9ebba5d72adf4fef99291fc07ebe13cb1b36f4 Mon Sep 17 00:00:00 2001 From: Hugh Rundle Date: Tue, 21 Nov 2023 20:13:56 +1100 Subject: [PATCH] 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 --- bookwyrm/activitypub/verbs.py | 32 ++++-------------- bookwyrm/static/js/bookwyrm.js | 33 ++++++------------- .../templates/snippets/follow_button.html | 14 ++------ .../snippets/remove_follower_button.html | 5 +++ bookwyrm/templates/snippets/user_options.html | 5 +++ bookwyrm/tests/views/test_follow.py | 12 +++++++ bookwyrm/urls.py | 4 ++- bookwyrm/views/follow.py | 11 +++---- 8 files changed, 49 insertions(+), 67 deletions(-) create mode 100644 bookwyrm/templates/snippets/remove_follower_button.html diff --git a/bookwyrm/activitypub/verbs.py b/bookwyrm/activitypub/verbs.py index 2753104b2..a365f4cc0 100644 --- a/bookwyrm/activitypub/verbs.py +++ b/bookwyrm/activitypub/verbs.py @@ -173,35 +173,17 @@ class Reject(Verb): def action(self, allow_external_connections=True): """reject a follow or follow request""" - if self.object.type == "Follow": - model = apps.get_model("bookwyrm.UserFollowRequest") - obj = self.object.to_model( + for model_name in ["UserFollowRequest", "UserFollows", None]: + model = apps.get_model(f"bookwyrm.{model_name}") if model_name else None + if obj := self.object.to_model( model=model, save=False, allow_create=False, allow_external_connections=allow_external_connections, - ) - if not obj: - # This is a deletion (soft-block) of an accepted follow - model = apps.get_model("bookwyrm.UserFollows") - 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() + ): + # Reject the first model that can be built. + obj.reject() + break @dataclass(init=False) diff --git a/bookwyrm/static/js/bookwyrm.js b/bookwyrm/static/js/bookwyrm.js index 7093219e5..dcde9cc72 100644 --- a/bookwyrm/static/js/bookwyrm.js +++ b/bookwyrm/static/js/bookwyrm.js @@ -330,30 +330,17 @@ let BookWyrm = new (class { const bookwyrm = this; const form = event.currentTarget; - const formAction = event.submitter.getAttribute("formaction") || form.action; const relatedforms = document.querySelectorAll(`.${form.dataset.id}`); - // Toggle class on all related forms. - if (formAction == "/remove-follow") { - // Remove ALL follow/unfollow/remote buttons - relatedforms.forEach((relatedForm) => relatedForm.classList.add("is-hidden")); + relatedforms.forEach((relatedForm) => + bookwyrm.addRemoveClass( + relatedForm, + "is-hidden", + relatedForm.className.indexOf("is-hidden") == -1 + ) + ); - // Remove orphaned user-options dropdown - 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) => { + this.ajaxPost(form).catch((error) => { // @todo Display a notification in the UI instead. console.warn("Request failed:", error); }); @@ -365,8 +352,8 @@ let BookWyrm = new (class { * @param {object} form - Form to be submitted * @return {Promise} */ - ajaxPost(target, form) { - return fetch(target, { + ajaxPost(form) { + return fetch(form.action, { method: "POST", body: new FormData(form), headers: { diff --git a/bookwyrm/templates/snippets/follow_button.html b/bookwyrm/templates/snippets/follow_button.html index 5c0839065..28b979987 100644 --- a/bookwyrm/templates/snippets/follow_button.html +++ b/bookwyrm/templates/snippets/follow_button.html @@ -12,7 +12,6 @@
- {% if not followers_page %} - {% endif %} -
{% if not minimal %}
- {% 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" %}
{% endif %}
diff --git a/bookwyrm/templates/snippets/remove_follower_button.html b/bookwyrm/templates/snippets/remove_follower_button.html new file mode 100644 index 000000000..28bef6842 --- /dev/null +++ b/bookwyrm/templates/snippets/remove_follower_button.html @@ -0,0 +1,5 @@ +{% load i18n %} + + {% csrf_token %} + + diff --git a/bookwyrm/templates/snippets/user_options.html b/bookwyrm/templates/snippets/user_options.html index 35abc98c2..ab028a494 100644 --- a/bookwyrm/templates/snippets/user_options.html +++ b/bookwyrm/templates/snippets/user_options.html @@ -20,4 +20,9 @@
  • {% include 'snippets/block_button.html' with user=user class="is-fullwidth" blocks=False %}
    • +{% if followers_page %} +
  • + {% include 'snippets/remove_follower_button.html' with user=user class="is-fullwidth" blocks=False %} +
    • +{% endif %} {% endblock %} diff --git a/bookwyrm/tests/views/test_follow.py b/bookwyrm/tests/views/test_follow.py index d18e24f89..8d73a666c 100644 --- a/bookwyrm/tests/views/test_follow.py +++ b/bookwyrm/tests/views/test_follow.py @@ -180,6 +180,18 @@ class FollowViews(TestCase): # follow relationship should not exist 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, *_): """check ostatus subscribe template loads""" request = self.factory.get( diff --git a/bookwyrm/urls.py b/bookwyrm/urls.py index ab1dca378..41eff3b8c 100644 --- a/bookwyrm/urls.py +++ b/bookwyrm/urls.py @@ -763,7 +763,9 @@ urlpatterns = [ # following re_path(r"^follow/?$", views.follow, name="follow"), 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\d+)/?$", views.remove_follow, name="remove-follow" + ), re_path(r"^accept-follow-request/?$", views.accept_follow_request), re_path(r"^delete-follow-request/?$", views.delete_follow_request), re_path(r"^ostatus_follow/?$", views.remote_follow, name="remote-follow"), diff --git a/bookwyrm/views/follow.py b/bookwyrm/views/follow.py index f9a09e2c9..dcb1c695c 100644 --- a/bookwyrm/views/follow.py +++ b/bookwyrm/views/follow.py @@ -71,11 +71,10 @@ def unfollow(request): @login_required @require_POST -def remove_follow(request): +def remove_follow(request, user_id): """remove a previously approved follower without blocking them""" - username = request.POST["user"] - to_remove = get_user_from_username(request.user, username) + to_remove = get_object_or_404(models.User, id=user_id) try: models.UserFollows.objects.get( @@ -93,8 +92,8 @@ def remove_follow(request): if is_api_request(request): 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 @@ -128,7 +127,7 @@ def delete_follow_request(request): ) follow_request.raise_not_deletable(request.user) - follow_request.delete() + follow_request.reject() return redirect(f"/user/{request.user.localname}")