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.
This commit is contained in:
lain 2020-06-05 12:26:07 +02:00
parent cc8a7dc205
commit 0efa8aa0b9
3 changed files with 28 additions and 7 deletions

View file

@ -363,19 +363,21 @@ def update(%{to: to, cc: cc, actor: actor, object: object} = params) 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

View file

@ -533,12 +533,12 @@ def handle_incoming(
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 @@ def handle_incoming(
:noop
end
ActivityPub.notify_and_stream(activity)
{:ok, activity}
else
_e ->

View file

@ -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 @@ test "it rejects incoming follow requests from blocked users when deny_follow_bl
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)