diff --git a/lib/federation/activity_pub/transmogrifier.ex b/lib/federation/activity_pub/transmogrifier.ex index fcba654e..2963deac 100644 --- a/lib/federation/activity_pub/transmogrifier.ex +++ b/lib/federation/activity_pub/transmogrifier.ex @@ -71,8 +71,8 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do case Discussions.get_comment_from_url_with_preload(object["id"]) do {:error, :comment_not_found} -> case Converter.Comment.as_to_model_data(object) do - %{visibility: visibility} = object_data - when visibility === :private -> + %{visibility: visibility, attributed_to_id: attributed_to_id} = object_data + when visibility === :private and is_nil(attributed_to_id) -> Actions.Create.create(:conversation, object_data, false) object_data when is_map(object_data) -> diff --git a/lib/federation/activity_pub/types/conversation.ex b/lib/federation/activity_pub/types/conversation.ex index 0cdd614a..b72cef02 100644 --- a/lib/federation/activity_pub/types/conversation.ex +++ b/lib/federation/activity_pub/types/conversation.ex @@ -23,37 +23,43 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Conversations do @impl Entity @spec create(map(), map()) :: {:ok, Conversation.t(), ActivityStream.t()} - | {:error, :conversation_not_found | :last_comment_not_found | Ecto.Changeset.t()} + | {:error, + :conversation_not_found + | :last_comment_not_found + | :empty_participants + | Ecto.Changeset.t()} def create(%{conversation_id: conversation_id} = args, additional) when not is_nil(conversation_id) do Logger.debug("Creating a reply to a conversation #{inspect(args, pretty: true)}") args = prepare_args(args) Logger.debug("Creating a reply to a conversation #{inspect(args, pretty: true)}") - case Conversations.get_conversation(conversation_id) do - %Conversation{} = conversation -> - case Conversations.reply_to_conversation(conversation, args) do - {:ok, %Conversation{last_comment_id: last_comment_id} = conversation} -> - ConversationActivity.insert_activity(conversation, subject: "conversation_replied") - maybe_publish_graphql_subscription(conversation) + with args when is_map(args) <- prepare_args(args) do + case Conversations.get_conversation(conversation_id) do + %Conversation{} = conversation -> + case Conversations.reply_to_conversation(conversation, args) do + {:ok, %Conversation{last_comment_id: last_comment_id} = conversation} -> + ConversationActivity.insert_activity(conversation, subject: "conversation_replied") + maybe_publish_graphql_subscription(conversation) - case Discussions.get_comment_with_preload(last_comment_id) do - %Comment{} = last_comment -> - comment_as_data = Convertible.model_to_as(last_comment) - audience = Audience.get_audience(conversation) - create_data = make_create_data(comment_as_data, Map.merge(audience, additional)) - {:ok, conversation, create_data} + case Discussions.get_comment_with_preload(last_comment_id) do + %Comment{} = last_comment -> + comment_as_data = Convertible.model_to_as(last_comment) + audience = Audience.get_audience(conversation) + create_data = make_create_data(comment_as_data, Map.merge(audience, additional)) + {:ok, conversation, create_data} - nil -> - {:error, :last_comment_not_found} - end + nil -> + {:error, :last_comment_not_found} + end - {:error, _, %Ecto.Changeset{} = err, _} -> - {:error, err} - end + {:error, _, %Ecto.Changeset{} = err, _} -> + {:error, err} + end - nil -> - {:error, :discussion_not_found} + nil -> + {:error, :discussion_not_found} + end end end diff --git a/test/federation/activity_pub/transmogrifier/comments_test.exs b/test/federation/activity_pub/transmogrifier/comments_test.exs index e0846c0d..74929d5b 100644 --- a/test/federation/activity_pub/transmogrifier/comments_test.exs +++ b/test/federation/activity_pub/transmogrifier/comments_test.exs @@ -6,6 +6,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.CommentsTest do import ExUnit.CaptureLog alias Mobilizon.Actors alias Mobilizon.Actors.Actor + alias Mobilizon.Conversations.Conversation alias Mobilizon.Discussions alias Mobilizon.Discussions.Comment alias Mobilizon.Events.Event @@ -218,7 +219,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.CommentsTest do end) =~ "[warning] Parent object is something we don't handle" end - test "it ignores incoming private notes" do + test "it accepts incoming private notes" do actor_data = File.read!("test/fixtures/mastodon-actor.json") |> Jason.decode!() Mock @@ -245,12 +246,28 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.CommentsTest do end) data = File.read!("test/fixtures/mastodon-post-activity-private.json") |> Jason.decode!() - event = insert(:event) + %Event{organizer_actor_id: organizer_actor_id} = event = insert(:event) object = data["object"] object = Map.put(object, "inReplyTo", event.url) data = Map.put(data, "object", object) - :error = Transmogrifier.handle_incoming(data) + {:ok, %Activity{data: _data, local: false}, %Conversation{} = conversation} = + Transmogrifier.handle_incoming(data) + + {:ok, %Actor{id: actor_1_id}} = Actors.get_actor_by_url("https://framapiaf.org/users/tcit") + {:ok, %Actor{id: actor_2_id}} = Actors.get_actor_by_url("https://framapiaf.org/users/admin") + + assert conversation.event_id == event.id + assert conversation.last_comment.visibility == :private + assert conversation.last_comment.actor_id == actor_2_id + assert conversation.last_comment_id == conversation.origin_comment_id + assert is_nil(conversation.last_comment.in_reply_to_comment_id) + assert is_nil(conversation.last_comment.origin_comment_id) + + participants_id = Enum.map(conversation.participants, & &1.id) + + assert MapSet.new(participants_id) == + MapSet.new([organizer_actor_id, actor_1_id, actor_2_id]) end test "it works for incoming notices" do diff --git a/test/graphql/resolvers/conversation_test.exs b/test/graphql/resolvers/conversation_test.exs index 7c1479f4..f8d891f1 100644 --- a/test/graphql/resolvers/conversation_test.exs +++ b/test/graphql/resolvers/conversation_test.exs @@ -1,8 +1,6 @@ defmodule Mobilizon.GraphQL.Resolvers.ConversationTest do use Mobilizon.Web.ConnCase - alias Mobilizon.Actors - alias Mobilizon.Actors.Actor - + alias Mobilizon.Discussions alias Mobilizon.GraphQL.AbsintheHelpers import Mobilizon.Factory @@ -47,7 +45,10 @@ defmodule Mobilizon.GraphQL.Resolvers.ConversationTest do test "for a given event", %{conn: conn, user: user, actor: actor} do event = insert(:event, organizer_actor: actor) conversation = insert(:conversation, event: event) - another_comment = build(:comment, origin_comment: conversation.origin_comment) + another_comment = insert(:comment, origin_comment: conversation.origin_comment) + + Discussions.update_comment(conversation.origin_comment, %{conversation_id: conversation.id}) + Discussions.update_comment(another_comment, %{conversation_id: conversation.id}) conversation_participant = insert(:conversation_participant, actor: actor, conversation: conversation) @@ -66,10 +67,18 @@ defmodule Mobilizon.GraphQL.Resolvers.ConversationTest do conversation_data = hd(res["data"]["event"]["conversations"]["elements"]) assert conversation_data["id"] == to_string(conversation.id) assert conversation_data["lastComment"]["text"] == conversation.last_comment.text - assert conversation_data["comments"]["total"] == 2 - assert conversation_data["comments"]["elements"] - |> Enum.any?(fn comment -> comment.text == conversation.origin_comment.text end) + assert conversation_data["comments"]["total"] == 2 + comments = conversation_data["comments"]["elements"] + + assert MapSet.new(Enum.map(comments, & &1["id"])) == + [conversation.origin_comment.id, another_comment.id] + |> Enum.map(&to_string/1) + |> MapSet.new() + + assert Enum.any?(comments, fn comment -> + comment["text"] == conversation.origin_comment.text + end) assert conversation_data["actor"]["preferredUsername"] == conversation_participant.actor.preferred_username diff --git a/test/mobilizon/conversations_test.exs b/test/mobilizon/conversations_test.exs index 1ff2e64a..8572fe74 100644 --- a/test/mobilizon/conversations_test.exs +++ b/test/mobilizon/conversations_test.exs @@ -1,4 +1,4 @@ -defmodule Mobilizon.DiscussionsTest do +defmodule Mobilizon.ConversationsTest do use Mobilizon.DataCase import Mobilizon.Factory diff --git a/test/service/activity/conversation_test.exs b/test/service/activity/conversation_test.exs index bd6b819e..2639d1fe 100644 --- a/test/service/activity/conversation_test.exs +++ b/test/service/activity/conversation_test.exs @@ -5,7 +5,7 @@ defmodule Mobilizon.Service.Activity.ConversationTest do alias Mobilizon.Actors.Actor alias Mobilizon.Conversations - alias Mobilizon.Conversations.Conversation + alias Mobilizon.Conversations.{Conversation, ConversationParticipant} alias Mobilizon.Discussions.Comment alias Mobilizon.Service.Activity.Conversation, as: ConversationActivity alias Mobilizon.Service.Workers.LegacyNotifierBuilder @@ -20,11 +20,19 @@ defmodule Mobilizon.Service.Activity.ConversationTest do %User{} = user = insert(:user) %Actor{id: actor_id} = actor = insert(:actor, user: user) - %Conversation{id: conversation_id, last_comment: %Comment{actor_id: last_comment_actor_id}} = - conversation = insert(:conversation) + %Conversation{ + id: conversation_id, + last_comment: %Comment{actor_id: last_comment_actor_id} + } = + conversation = insert(:conversation, event: nil) - insert(:conversation_participant, actor: actor, conversation: conversation) - insert(:conversation_participant, conversation: conversation) + %ConversationParticipant{id: conversation_participant_actor_id} = + insert(:conversation_participant, actor: actor, conversation: conversation) + + %ConversationParticipant{ + id: conversation_participant_id, + actor: %Actor{id: conversation_other_participant_actor_id} + } = insert(:conversation_participant, conversation: conversation) conversation = Conversations.get_conversation(conversation_id) @@ -35,13 +43,14 @@ defmodule Mobilizon.Service.Activity.ConversationTest do worker: LegacyNotifierBuilder, args: %{ "author_id" => last_comment_actor_id, - "participants" => [actor_id], + "participant" => %{"actor_id" => actor_id, "id" => conversation_participant_actor_id}, "object_id" => to_string(conversation_id), "object_type" => "conversation", "op" => "legacy_notify", "subject" => "conversation_created", "subject_params" => %{ - "conversation_id" => conversation_id + "conversation_id" => conversation_id, + "conversation_participant_id" => conversation_participant_actor_id }, "type" => "conversation" } @@ -50,19 +59,20 @@ defmodule Mobilizon.Service.Activity.ConversationTest do assert_enqueued( worker: LegacyNotifierBuilder, args: %{ - "author_id" => author_id, - "object_id" => to_string(comment_id), - "object_type" => "comment", - "op" => "legacy_notify", - "subject" => "event_new_comment", - "subject_params" => %{ - "comment_reply_to_uuid" => nil, - "event_title" => event_title, - "event_uuid" => event_uuid, - "comment_reply_to" => false, - "comment_uuid" => comment_uuid + "author_id" => last_comment_actor_id, + "participant" => %{ + "actor_id" => conversation_other_participant_actor_id, + "id" => conversation_participant_id }, - "type" => "comment" + "object_id" => to_string(conversation_id), + "object_type" => "conversation", + "op" => "legacy_notify", + "subject" => "conversation_created", + "subject_params" => %{ + "conversation_id" => conversation_id, + "conversation_participant_id" => conversation_participant_id + }, + "type" => "conversation" } ) end