Transmogrifier: For follows, create notifications last.

As the notification type changes depending on the follow state,
the notification should not be created and streamed out before the
state settles. For this reason, the notification creation has been
delayed until it's clear if the user has been followed or not.

This is a bit hacky but it will be properly rewritten using the
pipeline soon.

diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex
index 568db23483d585cda7bccc02701055e96895ffbb..4f7043c9240eff164f1bbfc8095e52eb68ceab55 100644 (file)
--- a/lib/pleroma/web/activity_pub/activity_pub.ex
+++ b/lib/pleroma/web/activity_pub/activity_pub.ex
@@ -363,19 +363,21 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
     end
   end
 
-  @spec follow(User.t(), User.t(), String.t() | nil, boolean()) ::
+  @spec follow(User.t(), User.t(), String.t() | nil, boolean(), keyword()) ::
           {:ok, Activity.t()} | {:error, any()}
-  def follow(follower, followed, activity_id \\ nil, local \\ true) do
+  def follow(follower, followed, activity_id \\ nil, local \\ true, opts \\ []) do
     with {:ok, result} <-
-           Repo.transaction(fn -> do_follow(follower, followed, activity_id, local) end) do
+           Repo.transaction(fn -> do_follow(follower, followed, activity_id, local, opts) end) do
       result
     end
   end
 
-  defp do_follow(follower, followed, activity_id, local) do
+  defp do_follow(follower, followed, activity_id, local, opts) do
+    skip_notify_and_stream = Keyword.get(opts, :skip_notify_and_stream, false)
+
     with data <- make_follow_data(follower, followed, activity_id),
          {:ok, activity} <- insert(data, local),
-         _ <- notify_and_stream(activity),
+         _ <- skip_notify_and_stream || notify_and_stream(activity),
          :ok <- maybe_federate(activity) do
       {:ok, activity}
     else

diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex
index b2461de2be7bc8ce2702f625bcc6db2e90ff4640..50f3216f36de8abc32a23d644350a42cd6d7018b 100644 (file)
--- a/lib/pleroma/web/activity_pub/transmogrifier.ex
+++ b/lib/pleroma/web/activity_pub/transmogrifier.ex
@@ -533,12 +533,12 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
            User.get_cached_by_ap_id(Containment.get_actor(%{"actor" => followed})),
          {:ok, %User{} = follower} <-
            User.get_or_fetch_by_ap_id(Containment.get_actor(%{"actor" => follower})),
-         {:ok, activity} <- ActivityPub.follow(follower, followed, id, false) do
+         {:ok, activity} <-
+           ActivityPub.follow(follower, followed, id, false, skip_notify_and_stream: true) do
       with deny_follow_blocked <- Pleroma.Config.get([:user, :deny_follow_blocked]),
            {_, false} <- {:user_blocked, User.blocks?(followed, follower) && deny_follow_blocked},
            {_, false} <- {:user_locked, User.locked?(followed)},
            {_, {:ok, follower}} <- {:follow, User.follow(follower, followed)},
-           _ <- Notification.update_notification_type(followed, activity),
            {_, {:ok, _}} <-
              {:follow_state_update, Utils.update_follow_state_for_all(activity, "accept")},
            {:ok, _relationship} <-
@@ -577,6 +577,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
           :noop
       end
 
+      ActivityPub.notify_and_stream(activity)
       {:ok, activity}
     else
       _e ->

diff --git a/test/web/activity_pub/transmogrifier/follow_handling_test.exs b/test/web/activity_pub/transmogrifier/follow_handling_test.exs
index 6b003b51c917b2750a77e5944e50ac3811b10eee..06c39eed67e074c00e83d4000e2435c6ee7d7491 100644 (file)
--- a/test/web/activity_pub/transmogrifier/follow_handling_test.exs
+++ b/test/web/activity_pub/transmogrifier/follow_handling_test.exs
@@ -13,6 +13,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.FollowHandlingTest do
 
   import Pleroma.Factory
   import Ecto.Query
+  import Mock
 
   setup_all do
     Tesla.Mock.mock_global(fn env -> apply(HttpRequestMock, :request, [env]) end)
@@ -151,6 +152,23 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.FollowHandlingTest do
       assert activity.data["state"] == "reject"
     end
 
+    test "it rejects incoming follow requests if the following errors for some reason" do
+      user = insert(:user)
+
+      data =
+        File.read!("test/fixtures/mastodon-follow-activity.json")
+        |> Poison.decode!()
+        |> Map.put("object", user.ap_id)
+
+      with_mock Pleroma.User, [:passthrough], follow: fn _, _ -> {:error, :testing} end do
+        {:ok, %Activity{data: %{"id" => id}}} = Transmogrifier.handle_incoming(data)
+
+        %Activity{} = activity = Activity.get_by_ap_id(id)
+
+        assert activity.data["state"] == "reject"
+      end
+    end
+
     test "it works for incoming follow requests from hubzilla" do
       user = insert(:user)