Don't persist local undone follow (#194)

same deal but backwards this time

Co-authored-by: FloatingGhost <hannah@coffee-and-dreams.uk>
Reviewed-on: https://akkoma.dev/AkkomaGang/akkoma/pulls/194

diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex
index 03e72be58443f9f7e3822cc23a1d84499ac7d7d4..20acdf86eeddccf065fd4a4065e3523b3ec84f58 100644 (file)
--- a/lib/pleroma/web/activity_pub/activity_pub.ex
+++ b/lib/pleroma/web/activity_pub/activity_pub.ex
@@ -331,9 +331,9 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
 
   defp do_unfollow(follower, followed, activity_id, local) when local == true do
     with %Activity{} = follow_activity <- fetch_latest_follow(follower, followed),
-         {:ok, follow_activity} <- update_follow_state(follow_activity, "cancelled"),
          unfollow_data <- make_unfollow_data(follower, followed, follow_activity, activity_id),
          {:ok, activity} <- insert(unfollow_data, local),
+         {:ok, _activity} <- Repo.delete(follow_activity),
          _ <- notify_and_stream(activity),
          :ok <- maybe_federate(activity) do
       {:ok, activity}
@@ -349,7 +349,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
     with %Activity{} = follow_activity <- fetch_latest_follow(follower, followed),
          {:ok, _activity} <- Repo.delete(follow_activity),
          unfollow_data <- make_unfollow_data(follower, followed, follow_activity, activity_id),
-         unfollow_activity <- remote_unfollow_data(unfollow_data),
+         unfollow_activity <- make_unfollow_activity(unfollow_data, false),
          _ <- notify_and_stream(unfollow_activity) do
       {:ok, unfollow_activity}
     else
@@ -358,12 +358,12 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
     end
   end
 
-  defp remote_unfollow_data(data) do
+  defp make_unfollow_activity(data, local) do
     {recipients, _, _} = get_recipients(data)
 
     %Activity{
       data: data,
-      local: false,
+      local: local,
       actor: data["actor"],
       recipients: recipients
     }

diff --git a/lib/pleroma/web/activity_pub/utils.ex b/lib/pleroma/web/activity_pub/utils.ex
index 5e5df488839aaf64103477065c74d593852164b9..b920e8c1d1648d12e847adc81958bb6f2fb10690 100644 (file)
--- a/lib/pleroma/web/activity_pub/utils.ex
+++ b/lib/pleroma/web/activity_pub/utils.ex
@@ -472,18 +472,6 @@ defmodule Pleroma.Web.ActivityPub.Utils do
     {:ok, activity}
   end
 
-  def update_follow_state(
-        %Activity{} = activity,
-        state
-      ) do
-    new_data = Map.put(activity.data, "state", state)
-    changeset = Changeset.change(activity, data: new_data)
-
-    with {:ok, activity} <- Repo.update(changeset) do
-      {:ok, activity}
-    end
-  end
-
   @doc """
   Makes a follow activity data for the given follower and followed
   """

diff --git a/priv/repo/migrations/20220831170605_remove_local_cancelled_follows.exs b/priv/repo/migrations/20220831170605_remove_local_cancelled_follows.exs
new file mode 100644 (file)
index 0000000..16597f8
--- /dev/null
+++ b/priv/repo/migrations/20220831170605_remove_local_cancelled_follows.exs
@@ -0,0 +1,22 @@
+defmodule Pleroma.Repo.Migrations.RemoveLocalCancelledFollows do
+  use Ecto.Migration
+
+  def up do
+    statement = """
+    DELETE FROM
+        activities
+    WHERE
+        (data->>'type') = 'Follow'
+    AND
+        (data->>'state') = 'cancelled'
+    AND
+        local = true;
+    """
+
+    execute(statement)
+  end
+
+  def down do
+    :ok
+  end
+end

diff --git a/test/mix/tasks/pleroma/relay_test.exs b/test/mix/tasks/pleroma/relay_test.exs
index db75b363009778eb20ee0b7f4fd2d23881c16ff6..d456904183759bf0f44d76d051e96affe3f26f35 100644 (file)
--- a/test/mix/tasks/pleroma/relay_test.exs
+++ b/test/mix/tasks/pleroma/relay_test.exs
@@ -65,7 +65,7 @@ defmodule Mix.Tasks.Pleroma.RelayTest do
       Mix.Tasks.Pleroma.Relay.run(["unfollow", target_instance])
 
       cancelled_activity = Activity.get_by_ap_id(follow_activity.data["id"])
-      assert cancelled_activity.data["state"] == "cancelled"
+      assert is_nil(cancelled_activity)
 
       [undo_activity] =
         ActivityPub.fetch_activities([], %{
@@ -78,7 +78,6 @@ defmodule Mix.Tasks.Pleroma.RelayTest do
 
       assert undo_activity.data["type"] == "Undo"
       assert undo_activity.data["actor"] == local_user.ap_id
-      assert undo_activity.data["object"]["id"] == cancelled_activity.data["id"]
       refute "#{target_instance}/followers" in User.following(local_user)
     end
 
@@ -142,7 +141,7 @@ defmodule Mix.Tasks.Pleroma.RelayTest do
       Mix.Tasks.Pleroma.Relay.run(["unfollow", target_instance, "--force"])
 
       cancelled_activity = Activity.get_by_ap_id(follow_activity.data["id"])
-      assert cancelled_activity.data["state"] == "cancelled"
+      assert is_nil(cancelled_activity)
 
       [undo_activity] =
         ActivityPub.fetch_activities(
@@ -152,7 +151,6 @@ defmodule Mix.Tasks.Pleroma.RelayTest do
 
       assert undo_activity.data["type"] == "Undo"
       assert undo_activity.data["actor"] == local_user.ap_id
-      assert undo_activity.data["object"]["id"] == cancelled_activity.data["id"]
       refute "#{target_instance}/followers" in User.following(local_user)
     end
   end

diff --git a/test/pleroma/notification_test.exs b/test/pleroma/notification_test.exs
index 4354dd2b6c3f36e9b0dec73ae49198c2ec8e9278..8db2088789475e89fe7e2f2de7fd139bd5e9c50b 100644 (file)
--- a/test/pleroma/notification_test.exs
+++ b/test/pleroma/notification_test.exs
@@ -427,13 +427,12 @@ defmodule Pleroma.NotificationTest do
 
       {:ok, _, _, _activity} = CommonAPI.follow(user, followed_user)
       assert FollowingRelationship.following?(user, followed_user)
-      assert [notification] = Notification.for_user(followed_user)
+      assert [_notification] = Notification.for_user(followed_user)
 
       CommonAPI.unfollow(user, followed_user)
       {:ok, _, _, _activity_dupe} = CommonAPI.follow(user, followed_user)
 
-      notification_id = notification.id
-      assert [%{id: ^notification_id}] = Notification.for_user(followed_user)
+      assert Enum.count(Notification.for_user(followed_user)) == 1
     end
 
     test "dismisses the notification on follow request rejection" do

diff --git a/test/pleroma/web/activity_pub/activity_pub_test.exs b/test/pleroma/web/activity_pub/activity_pub_test.exs
index 50eff94317bcf8424b861c6dff378438db6fec0f..ec562ac7b2e8abcd9caf57c89eb5efd1a0e2334d 100644 (file)
--- a/test/pleroma/web/activity_pub/activity_pub_test.exs
+++ b/test/pleroma/web/activity_pub/activity_pub_test.exs
@@ -1373,6 +1373,25 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
       assert embedded_object["id"] == follow_activity.data["id"]
     end
 
+    test "it removes the follow activity if it was local" do
+      follower = insert(:user, local: true)
+      followed = insert(:user)
+
+      {:ok, _, _, follow_activity} = CommonAPI.follow(follower, followed)
+      {:ok, activity} = ActivityPub.unfollow(follower, followed, nil, true)
+
+      assert activity.data["type"] == "Undo"
+      assert activity.data["actor"] == follower.ap_id
+
+      follow_activity = Activity.get_by_id(follow_activity.id)
+      assert is_nil(follow_activity)
+      assert is_nil(Utils.fetch_latest_follow(follower, followed))
+
+      # We need to keep our own undo
+      undo_activity = Activity.get_by_ap_id(activity.data["id"])
+      refute is_nil(undo_activity)
+    end
+
     test "it removes the follow activity if it was remote" do
       follower = insert(:user, local: false)
       followed = insert(:user)
@@ -1383,9 +1402,12 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
       assert activity.data["type"] == "Undo"
       assert activity.data["actor"] == follower.ap_id
 
-      activity = Activity.get_by_id(follow_activity.id)
-      assert is_nil(activity)
+      follow_activity = Activity.get_by_id(follow_activity.id)
+      assert is_nil(follow_activity)
       assert is_nil(Utils.fetch_latest_follow(follower, followed))
+
+      undo_activity = Activity.get_by_ap_id(activity.data["id"])
+      assert is_nil(undo_activity)
     end
   end
 

diff --git a/test/pleroma/web/activity_pub/utils_test.exs b/test/pleroma/web/activity_pub/utils_test.exs
index 0d88303e34d8a3913095fe8c2d1641cd461e8f5a..e45af3aec35cc233580870c97a7b9d412c8ac981 100644 (file)
--- a/test/pleroma/web/activity_pub/utils_test.exs
+++ b/test/pleroma/web/activity_pub/utils_test.exs
@@ -229,29 +229,6 @@ defmodule Pleroma.Web.ActivityPub.UtilsTest do
     end
   end
 
-  describe "update_follow_state/2" do
-    test "updates the state of the given follow activity" do
-      user = insert(:user, is_locked: true)
-      follower = insert(:user)
-
-      {:ok, _, _, follow_activity} = CommonAPI.follow(follower, user)
-      {:ok, _, _, follow_activity_two} = CommonAPI.follow(follower, user)
-
-      data =
-        follow_activity_two.data
-        |> Map.put("state", "accept")
-
-      cng = Ecto.Changeset.change(follow_activity_two, data: data)
-
-      {:ok, follow_activity_two} = Repo.update(cng)
-
-      {:ok, follow_activity_two} = Utils.update_follow_state(follow_activity_two, "reject")
-
-      assert refresh_record(follow_activity).data["state"] == "pending"
-      assert refresh_record(follow_activity_two).data["state"] == "reject"
-    end
-  end
-
   describe "update_element_in_object/3" do
     test "updates likes" do
       user = insert(:user)

diff --git a/test/pleroma/web/common_api_test.exs b/test/pleroma/web/common_api_test.exs
index fa751bf60654db60536499551175a94c033b2c69..840d74d2fbe3a49438efa39c2b7c231d8b9521c7 100644 (file)
--- a/test/pleroma/web/common_api_test.exs
+++ b/test/pleroma/web/common_api_test.exs
@@ -1058,24 +1058,23 @@ defmodule Pleroma.Web.CommonAPITest do
       refute User.subscribed_to?(follower, followed)
     end
 
-    test "cancels a pending follow for a local user" do
+    test "removes a pending follow for a local user" do
       follower = insert(:user)
       followed = insert(:user, is_locked: true)
 
-      assert {:ok, follower, followed, %{id: activity_id, data: %{"state" => "pending"}}} =
+      assert {:ok, follower, followed, %{id: _activity_id, data: %{"state" => "pending"}}} =
                CommonAPI.follow(follower, followed)
 
       assert User.get_follow_state(follower, followed) == :follow_pending
       assert {:ok, follower} = CommonAPI.unfollow(follower, followed)
       assert User.get_follow_state(follower, followed) == nil
 
-      assert %{id: ^activity_id, data: %{"state" => "cancelled"}} =
-               Pleroma.Web.ActivityPub.Utils.fetch_latest_follow(follower, followed)
+      assert is_nil(Pleroma.Web.ActivityPub.Utils.fetch_latest_follow(follower, followed))
 
       assert %{
                data: %{
                  "type" => "Undo",
-                 "object" => %{"type" => "Follow", "state" => "cancelled"}
+                 "object" => %{"type" => "Follow"}
                }
              } = Pleroma.Web.ActivityPub.Utils.fetch_latest_undo(follower)
     end
@@ -1084,20 +1083,19 @@ defmodule Pleroma.Web.CommonAPITest do
       follower = insert(:user)
       followed = insert(:user, is_locked: true, local: false, ap_enabled: true)
 
-      assert {:ok, follower, followed, %{id: activity_id, data: %{"state" => "pending"}}} =
+      assert {:ok, follower, followed, %{id: _activity_id, data: %{"state" => "pending"}}} =
                CommonAPI.follow(follower, followed)
 
       assert User.get_follow_state(follower, followed) == :follow_pending
       assert {:ok, follower} = CommonAPI.unfollow(follower, followed)
       assert User.get_follow_state(follower, followed) == nil
 
-      assert %{id: ^activity_id, data: %{"state" => "cancelled"}} =
-               Pleroma.Web.ActivityPub.Utils.fetch_latest_follow(follower, followed)
+      assert is_nil(Pleroma.Web.ActivityPub.Utils.fetch_latest_follow(follower, followed))
 
       assert %{
                data: %{
                  "type" => "Undo",
-                 "object" => %{"type" => "Follow", "state" => "cancelled"}
+                 "object" => %{"type" => "Follow"}
                }
              } = Pleroma.Web.ActivityPub.Utils.fetch_latest_undo(follower)
     end