Merge branch 'no-federation-for-group-draft-posts' into 'master'

Only federate group draft posts to members

Closes #615

See merge request framasoft/mobilizon!847

lib/federation/activity_pub/activity_pub.ex

@@ -710,7 +710,7 @@ defmodule Mobilizon.Federation.ActivityPub do
   @spec publish(Actor.t(), Activity.t()) :: :ok
   def publish(actor, %Activity{recipients: recipients} = activity) do
     Logger.debug("Publishing an activity")
-    Logger.debug(inspect(activity))
+    Logger.debug(inspect(activity, pretty: true))
 
     public = Visibility.is_public?(activity)
     Logger.debug("is public ? #{public}")

lib/federation/activity_pub/audience.ex

@@ -123,19 +123,29 @@ defmodule Mobilizon.Federation.ActivityPub.Audience do
   end
 
   def calculate_to_and_cc_from_mentions(%Post{
-        attributed_to: %Actor{members_url: members_url},
+        attributed_to: %Actor{members_url: members_url, followers_url: followers_url},
-        visibility: visibility
+        visibility: visibility,
+        draft: draft
       }) do
-    case visibility do
+    cond do
-      :public ->
+      # If the post is draft we send it only to members
-        %{"to" => [@ap_public, members_url], "cc" => []}
+      draft == true ->
+        %{"to" => [members_url], "cc" => []}
 
-      :unlisted ->
+      # If public everyone
-        %{"to" => [members_url], "cc" => [@ap_public]}
+      visibility == :public ->
+        %{"to" => [@ap_public, members_url], "cc" => [followers_url]}
 
-      :private ->
+      # Otherwise just followers
+      visibility == :unlisted ->
+        %{"to" => [followers_url, members_url], "cc" => [@ap_public]}
+
+      visibility == :private ->
         # Private is restricted to only the members
         %{"to" => [members_url], "cc" => []}
+
+      true ->
+        %{"to" => [], "cc" => []}
     end
   end
 

lib/federation/activity_pub/types/posts.ex

@@ -24,10 +24,8 @@ defmodule Mobilizon.Federation.ActivityPub.Types.Posts do
          {:ok, %Actor{} = group} <- Actors.get_group_by_actor_id(group_id),
          %Actor{} = creator <- Actors.get_actor(creator_id),
          post_as_data <-
-           Convertible.model_to_as(%{post | attributed_to: group, author: creator}),
+           Convertible.model_to_as(%{post | attributed_to: group, author: creator}) do
-         audience <-
+      create_data = make_create_data(post_as_data, additional)
-           Audience.calculate_to_and_cc_from_mentions(post) do
-      create_data = make_create_data(post_as_data, Map.merge(audience, additional))
 
       {:ok, post, create_data}
     else

lib/federation/activity_pub/utils.ex

@@ -421,7 +421,13 @@ defmodule Mobilizon.Federation.ActivityPub.Utils do
          ["https://www.w3.org/ns/activitystreams#Public"]}
       else
         if actor_type == :Group do
-          {[actor.followers_url, actor.members_url], []}
+          to =
+            (object["to"] || [])
+            |> MapSet.new()
+            |> MapSet.intersection(MapSet.new([actor.followers_url, actor.members_url]))
+            |> MapSet.to_list()
+
+          {to, []}
         else
           {[actor.followers_url], []}
         end

lib/federation/activity_stream/converter/event.ex

@@ -70,7 +70,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Event do
         status: object |> Map.get("ical:status", "CONFIRMED") |> String.downcase(),
         online_address: object |> Map.get("attachment", []) |> get_online_address(),
         phone_address: object["phoneAddress"],
-        draft: false,
+        draft: object["draft"] == true,
         url: object["id"],
         uuid: object["uuid"],
         tags: tags,
@@ -119,7 +119,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Event do
       "commentsEnabled" => event.options.comment_moderation == :allow_all,
       "anonymousParticipationEnabled" => event.options.anonymous_participation,
       "attachment" => [],
-      # "draft" => event.draft,
+      "draft" => event.draft,
       "ical:status" => event.status |> to_string |> String.upcase(),
       "id" => event.url,
       "url" => event.url

lib/federation/activity_stream/converter/post.ex

@@ -7,7 +7,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Post do
   """
   alias Mobilizon.Actors.Actor
   alias Mobilizon.Federation.ActivityPub
-  alias Mobilizon.Federation.ActivityPub.Utils
+  alias Mobilizon.Federation.ActivityPub.{Audience, Utils}
   alias Mobilizon.Federation.ActivityStream.{Converter, Convertible}
   alias Mobilizon.Federation.ActivityStream.Converter.Media, as: MediaConverter
   alias Mobilizon.Posts.Post
@@ -36,26 +36,25 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Post do
   def model_to_as(
         %Post{
           author: %Actor{url: actor_url},
-          attributed_to: %Actor{url: creator_url, followers_url: followers_url}
+          attributed_to: %Actor{
+            url: creator_url
+          }
         } = post
       ) do
-    to =
+    audience = Audience.calculate_to_and_cc_from_mentions(post)
-      if post.visibility == :public,
-        do: ["https://www.w3.org/ns/activitystreams#Public"],
-        else: [followers_url]
 
     %{
       "type" => "Article",
-      "to" => to,
-      "cc" => [],
       "actor" => actor_url,
       "id" => post.url,
       "name" => post.title,
       "content" => post.body,
       "attributedTo" => creator_url,
       "published" => (post.publish_at || post.inserted_at) |> to_date(),
-      "attachment" => []
+      "attachment" => [],
+      "draft" => post.draft
     }
+    |> Map.merge(audience)
     |> maybe_add_post_picture(post)
     |> maybe_add_inline_media(post)
   end
@@ -81,7 +80,8 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Post do
         local: false,
         publish_at: object["published"],
         picture_id: picture_id,
-        medias: medias
+        medias: medias,
+        draft: object["draft"] == true
       }
     else
       {:error, err} -> {:error, err}
@@ -93,6 +93,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Post do
   defp get_actor(nil), do: {:error, "nil property found for actor data"}
   defp get_actor(actor), do: actor |> Utils.get_url() |> ActivityPub.get_or_fetch_actor_by_url()
 
+  defp to_date(nil), do: nil
   defp to_date(%DateTime{} = date), do: DateTime.to_iso8601(date)
   defp to_date(%NaiveDateTime{} = date), do: NaiveDateTime.to_iso8601(date)
 

test/federation/activity_pub/transmogrifier/posts_test.exs

@@ -0,0 +1,92 @@
+defmodule Mobilizon.Federation.ActivityPub.Transmogrifier.PostsTest do
+  use Mobilizon.DataCase
+
+  import Mobilizon.Factory
+  import Mox
+  alias Mobilizon.Actors.Actor
+  alias Mobilizon.Federation.ActivityPub.{Activity, Transmogrifier}
+  alias Mobilizon.Federation.ActivityStream.Convertible
+  alias Mobilizon.Posts.Post
+  alias Mobilizon.Service.HTTP.ActivityPub.Mock
+
+  describe "handle incoming posts" do
+    setup :verify_on_exit!
+
+    test "it ignores an incoming post if we already have it" do
+      post = insert(:post)
+      post = Repo.preload(post, [:author, :attributed_to, :picture, :media])
+
+      activity = %{
+        "type" => "Create",
+        "to" => ["https://www.w3.org/ns/activitystreams#Public"],
+        "actor" => post.author.url,
+        "attributedTo" => post.attributed_to.url,
+        "object" => Convertible.model_to_as(post)
+      }
+
+      data =
+        File.read!("test/fixtures/mobilizon-post-activity-group.json")
+        |> Jason.decode!()
+        |> Map.merge(activity)
+
+      assert {:ok, nil, _} = Transmogrifier.handle_incoming(data)
+    end
+
+    test "it receives a draft post correctly as a member" do
+      %Actor{} = group = insert(:group, domain: "remote.tld", url: "https://remote.tld/@group")
+      %Actor{} = author = insert(:actor, domain: "remote.tld", url: "https://remote.tld/@author")
+      insert(:member, parent: group, actor: author, role: :moderator)
+      insert(:member, parent: group, role: :member)
+
+      object =
+        Convertible.model_to_as(%Post{
+          url: "https://remote.tld/@group/some-slug",
+          author: author,
+          attributed_to: group,
+          picture: nil,
+          media: [],
+          body: "my body",
+          title: "my title",
+          draft: true
+        })
+
+      data =
+        File.read!("test/fixtures/mobilizon-post-activity-group.json")
+        |> Jason.decode!()
+        |> Map.put("object", object)
+
+      assert {:ok, %Activity{}, %Post{draft: true}} = Transmogrifier.handle_incoming(data)
+    end
+
+    test "it publishes a previously draft post correctly as a member" do
+      %Actor{} = group = insert(:group, domain: "remote.tld", url: "https://remote.tld/@group")
+      %Actor{} = author = insert(:actor, domain: "remote.tld", url: "https://remote.tld/@author")
+      insert(:member, parent: group, actor: author, role: :moderator)
+      insert(:member, parent: group, role: :member)
+
+      %Post{} =
+        post =
+        insert(:post,
+          url: "https://remote.tld/@group/some-slug",
+          author: author,
+          attributed_to: group,
+          draft: true
+        )
+
+      activity = %{
+        "type" => "Update",
+        "to" => ["https://www.w3.org/ns/activitystreams#Public"],
+        "actor" => post.author.url,
+        "attributedTo" => post.attributed_to.url,
+        "object" => Convertible.model_to_as(%Post{post | draft: false})
+      }
+
+      data =
+        File.read!("test/fixtures/mobilizon-post-activity-group.json")
+        |> Jason.decode!()
+        |> Map.merge(activity)
+
+      assert {:ok, %Activity{}, %Post{draft: false}} = Transmogrifier.handle_incoming(data)
+    end
+  end
+end

test/graphql/resolvers/post_test.exs

@@ -107,6 +107,8 @@ defmodule Mobilizon.GraphQL.Resolvers.PostTest do
     %Post{} =
       post_private = insert(:post, attributed_to: group, author: actor, visibility: :private)
 
+    insert(:follower, target_actor: group, approved: true)
+
     {:ok,
      user: user,
      group: group,
@@ -485,6 +487,32 @@ defmodule Mobilizon.GraphQL.Resolvers.PostTest do
       assert res["data"]["createPost"]["slug"] == "my-post-#{ShortUUID.encode!(id)}"
     end
 
+    test "create_post/3 creates a draft post for a group", %{
+      conn: conn,
+      user: user,
+      group: group
+    } do
+      res =
+        conn
+        |> auth_conn(user)
+        |> AbsintheHelpers.graphql_query(
+          query: @create_post,
+          variables: %{
+            title: @post_title,
+            body: "My new post is here",
+            attributedToId: group.id,
+            draft: true
+          }
+        )
+
+      assert is_nil(res["errors"])
+
+      assert res["data"]["createPost"]["title"] == @post_title
+      id = res["data"]["createPost"]["id"]
+      assert res["data"]["createPost"]["slug"] == "my-post-#{ShortUUID.encode!(id)}"
+      assert res["data"]["createPost"]["draft"] == true
+    end
+
     test "create_post/3 doesn't create a post if no group is defined", %{
       conn: conn,
       user: user

test/service/clean_old_activity_test.exs

@@ -7,7 +7,7 @@ defmodule Mobilizon.Service.CleanOldActivityTest do
   alias Mobilizon.Service.CleanOldActivity
 
   @activity_inserted_at_1 DateTime.from_iso8601("2019-01-02T10:33:39.207493Z") |> elem(1)
-  @activity_inserted_at_2 DateTime.from_iso8601("2021-03-02T10:33:39.207493Z") |> elem(1)
+  @activity_inserted_at_2 DateTime.utc_now()
 
   setup do
     group1 = insert(:group)