commit: b4bd5e40e491c8b777c75ec4f096c0c466a6b4e2
parent: a44d87f0594ad10809afe269e20e8d4c777b5d5c
Author: lambda <pleromagit@rogerbraun.net>
Date: Fri, 9 Nov 2018 16:00:24 +0000
Merge branch 'bugfix/no-cc-mentions' into 'develop'
align to/cc addressing pattern with friendica, hubzilla instead of mastodon
Closes #341
See merge request pleroma/pleroma!436
Diffstat:
7 files changed, 204 insertions(+), 38 deletions(-)
diff --git a/lib/pleroma/notification.ex b/lib/pleroma/notification.ex
@@ -1,6 +1,6 @@
defmodule Pleroma.Notification do
use Ecto.Schema
- alias Pleroma.{User, Activity, Notification, Repo}
+ alias Pleroma.{User, Activity, Notification, Repo, Object}
import Ecto.Query
schema "notifications" do
@@ -95,7 +95,7 @@ defmodule Pleroma.Notification do
def create_notifications(%Activity{id: _, data: %{"to" => _, "type" => type}} = activity)
when type in ["Create", "Like", "Announce", "Follow"] do
- users = User.get_notified_from_activity(activity)
+ users = get_notified_from_activity(activity)
notifications = Enum.map(users, fn user -> create_notification(activity, user) end)
{:ok, notifications}
@@ -113,4 +113,64 @@ defmodule Pleroma.Notification do
notification
end
end
+
+ def get_notified_from_activity(activity, local_only \\ true)
+
+ def get_notified_from_activity(
+ %Activity{data: %{"to" => _, "type" => type} = data} = activity,
+ local_only
+ )
+ when type in ["Create", "Like", "Announce", "Follow"] do
+ recipients =
+ []
+ |> maybe_notify_to_recipients(activity)
+ |> maybe_notify_mentioned_recipients(activity)
+ |> Enum.uniq()
+
+ User.get_users_from_set(recipients, local_only)
+ end
+
+ def get_notified_from_activity(_, local_only), do: []
+
+ defp maybe_notify_to_recipients(
+ recipients,
+ %Activity{data: %{"to" => to, "type" => type}} = activity
+ ) do
+ recipients ++ to
+ end
+
+ defp maybe_notify_mentioned_recipients(
+ recipients,
+ %Activity{data: %{"to" => to, "type" => type} = data} = activity
+ )
+ when type == "Create" do
+ object = Object.normalize(data["object"])
+
+ object_data =
+ cond do
+ !is_nil(object) ->
+ object.data
+
+ is_map(data["object"]) ->
+ data["object"]
+
+ true ->
+ %{}
+ end
+
+ tagged_mentions = maybe_extract_mentions(object_data)
+
+ recipients ++ tagged_mentions
+ end
+
+ defp maybe_notify_mentioned_recipients(recipients, _), do: recipients
+
+ defp maybe_extract_mentions(%{"tag" => tag}) do
+ tag
+ |> Enum.filter(fn x -> is_map(x) end)
+ |> Enum.filter(fn x -> x["type"] == "Mention" end)
+ |> Enum.map(fn x -> x["href"] end)
+ end
+
+ defp maybe_extract_mentions(_), do: []
end
diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex
@@ -464,36 +464,25 @@ defmodule Pleroma.User do
update_and_set_cache(cs)
end
- def get_notified_from_activity_query(to) do
+ def get_users_from_set_query(ap_ids, false) do
from(
u in User,
- where: u.ap_id in ^to,
- where: u.local == true
+ where: u.ap_id in ^ap_ids
)
end
- def get_notified_from_activity(%Activity{recipients: to, data: %{"type" => "Announce"} = data}) do
- object = Object.normalize(data["object"])
- actor = User.get_cached_by_ap_id(data["actor"])
-
- # ensure that the actor who published the announced object appears only once
- to =
- if actor.nickname != nil do
- to ++ [object.data["actor"]]
- else
- to
- end
- |> Enum.uniq()
-
- query = get_notified_from_activity_query(to)
+ def get_users_from_set_query(ap_ids, true) do
+ query = get_users_from_set_query(ap_ids, false)
- Repo.all(query)
+ from(
+ u in query,
+ where: u.local == true
+ )
end
- def get_notified_from_activity(%Activity{recipients: to}) do
- query = get_notified_from_activity_query(to)
-
- Repo.all(query)
+ def get_users_from_set(ap_ids, local_only \\ true) do
+ get_users_from_set_query(ap_ids, local_only)
+ |> Repo.all()
end
def get_recipients_from_activity(%Activity{recipients: to}) do
diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex
@@ -693,12 +693,9 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do
end
def add_mention_tags(object) do
- recipients = object["to"] ++ (object["cc"] || [])
-
mentions =
- recipients
- |> Enum.map(fn ap_id -> User.get_cached_by_ap_id(ap_id) end)
- |> Enum.filter(& &1)
+ object
+ |> Utils.get_notified_from_object()
|> Enum.map(fn user ->
%{"type" => "Mention", "href" => user.ap_id, "name" => "@#{user.nickname}"}
end)
diff --git a/lib/pleroma/web/activity_pub/utils.ex b/lib/pleroma/web/activity_pub/utils.ex
@@ -1,11 +1,13 @@
defmodule Pleroma.Web.ActivityPub.Utils do
- alias Pleroma.{Repo, Web, Object, Activity, User}
+ alias Pleroma.{Repo, Web, Object, Activity, User, Notification}
alias Pleroma.Web.Router.Helpers
alias Pleroma.Web.Endpoint
alias Ecto.{Changeset, UUID}
import Ecto.Query
require Logger
+ @supported_object_types ["Article", "Note", "Video", "Page"]
+
# Some implementations send the actor URI as the actor field, others send the entire actor object,
# so figure out what the actor's URI is based on what we have.
def get_ap_id(object) do
@@ -95,6 +97,21 @@ defmodule Pleroma.Web.ActivityPub.Utils do
"#{Web.base_url()}/#{type}/#{UUID.generate()}"
end
+ def get_notified_from_object(%{"type" => type} = object) when type in @supported_object_types do
+ fake_create_activity = %{
+ "to" => object["to"],
+ "cc" => object["cc"],
+ "type" => "Create",
+ "object" => object
+ }
+
+ Notification.get_notified_from_activity(%Activity{data: fake_create_activity}, false)
+ end
+
+ def get_notified_from_object(object) do
+ Notification.get_notified_from_activity(%Activity{data: object}, false)
+ end
+
def create_context(context) do
context = context || generate_id("contexts")
changeset = Object.context_mapping(context)
@@ -164,7 +181,7 @@ defmodule Pleroma.Web.ActivityPub.Utils do
Inserts a full object if it is contained in an activity.
"""
def insert_full_object(%{"object" => %{"type" => type} = object_data})
- when is_map(object_data) and type in ["Article", "Note", "Video", "Page"] do
+ when is_map(object_data) and type in @supported_object_types do
with {:ok, _} <- Object.create(object_data) do
:ok
end
diff --git a/lib/pleroma/web/common_api/utils.ex b/lib/pleroma/web/common_api/utils.ex
@@ -34,21 +34,29 @@ defmodule Pleroma.Web.CommonAPI.Utils do
end
def to_for_user_and_mentions(user, mentions, inReplyTo, "public") do
- to = ["https://www.w3.org/ns/activitystreams#Public"]
-
mentioned_users = Enum.map(mentions, fn {_, %{ap_id: ap_id}} -> ap_id end)
- cc = [user.follower_address | mentioned_users]
+
+ to = ["https://www.w3.org/ns/activitystreams#Public" | mentioned_users]
+ cc = [user.follower_address]
if inReplyTo do
- {to, Enum.uniq([inReplyTo.data["actor"] | cc])}
+ {Enum.uniq([inReplyTo.data["actor"] | to]), cc}
else
{to, cc}
end
end
def to_for_user_and_mentions(user, mentions, inReplyTo, "unlisted") do
- {to, cc} = to_for_user_and_mentions(user, mentions, inReplyTo, "public")
- {cc, to}
+ mentioned_users = Enum.map(mentions, fn {_, %{ap_id: ap_id}} -> ap_id end)
+
+ to = [user.follower_address | mentioned_users]
+ cc = ["https://www.w3.org/ns/activitystreams#Public"]
+
+ if inReplyTo do
+ {Enum.uniq([inReplyTo.data["actor"] | to]), cc}
+ else
+ {to, cc}
+ end
end
def to_for_user_and_mentions(user, mentions, inReplyTo, "private") do
diff --git a/test/notification_test.exs b/test/notification_test.exs
@@ -3,6 +3,7 @@ defmodule Pleroma.NotificationTest do
alias Pleroma.Web.TwitterAPI.TwitterAPI
alias Pleroma.Web.CommonAPI
alias Pleroma.{User, Notification}
+ alias Pleroma.Web.ActivityPub.Transmogrifier
import Pleroma.Factory
describe "create_notifications" do
@@ -156,6 +157,100 @@ defmodule Pleroma.NotificationTest do
end
end
+ describe "notification target determination" do
+ test "it sends notifications to addressed users in new messages" do
+ user = insert(:user)
+ other_user = insert(:user)
+
+ {:ok, activity} =
+ CommonAPI.post(user, %{
+ "status" => "hey @#{other_user.nickname}!"
+ })
+
+ assert other_user in Notification.get_notified_from_activity(activity)
+ end
+
+ test "it sends notifications to mentioned users in new messages" do
+ user = insert(:user)
+ other_user = insert(:user)
+
+ create_activity = %{
+ "@context" => "https://www.w3.org/ns/activitystreams",
+ "type" => "Create",
+ "to" => ["https://www.w3.org/ns/activitystreams#Public"],
+ "actor" => user.ap_id,
+ "object" => %{
+ "type" => "Note",
+ "content" => "message with a Mention tag, but no explicit tagging",
+ "tag" => [
+ %{
+ "type" => "Mention",
+ "href" => other_user.ap_id,
+ "name" => other_user.nickname
+ }
+ ],
+ "attributedTo" => user.ap_id
+ }
+ }
+
+ {:ok, activity} = Transmogrifier.handle_incoming(create_activity)
+
+ assert other_user in Notification.get_notified_from_activity(activity)
+ end
+
+ test "it does not send notifications to users who are only cc in new messages" do
+ user = insert(:user)
+ other_user = insert(:user)
+
+ create_activity = %{
+ "@context" => "https://www.w3.org/ns/activitystreams",
+ "type" => "Create",
+ "to" => ["https://www.w3.org/ns/activitystreams#Public"],
+ "cc" => [other_user.ap_id],
+ "actor" => user.ap_id,
+ "object" => %{
+ "type" => "Note",
+ "content" => "hi everyone",
+ "attributedTo" => user.ap_id
+ }
+ }
+
+ {:ok, activity} = Transmogrifier.handle_incoming(create_activity)
+
+ assert other_user not in Notification.get_notified_from_activity(activity)
+ end
+
+ test "it does not send notification to mentioned users in likes" do
+ user = insert(:user)
+ other_user = insert(:user)
+ third_user = insert(:user)
+
+ {:ok, activity_one} =
+ CommonAPI.post(user, %{
+ "status" => "hey @#{other_user.nickname}!"
+ })
+
+ {:ok, activity_two, _} = CommonAPI.favorite(activity_one.id, third_user)
+
+ assert other_user not in Notification.get_notified_from_activity(activity_two)
+ end
+
+ test "it does not send notification to mentioned users in announces" do
+ user = insert(:user)
+ other_user = insert(:user)
+ third_user = insert(:user)
+
+ {:ok, activity_one} =
+ CommonAPI.post(user, %{
+ "status" => "hey @#{other_user.nickname}!"
+ })
+
+ {:ok, activity_two, _} = CommonAPI.repeat(activity_one.id, third_user)
+
+ assert other_user not in Notification.get_notified_from_activity(activity_two)
+ end
+ end
+
describe "notification lifecycle" do
test "liking an activity results in 1 notification, then 0 if the activity is deleted" do
user = insert(:user)
diff --git a/test/web/twitter_api/twitter_api_test.exs b/test/web/twitter_api/twitter_api_test.exs
@@ -48,7 +48,7 @@ defmodule Pleroma.Web.TwitterAPI.TwitterAPITest do
"https://www.w3.org/ns/activitystreams#Public"
)
- assert Enum.member?(get_in(activity.data, ["cc"]), "shp")
+ assert Enum.member?(get_in(activity.data, ["to"]), "shp")
assert activity.local == true
assert %{"moominmamma" => "http://localhost:4001/finmoji/128px/moominmamma-128.png"} =
