From 10a7a4a868fbc5ff09516892b99e11100d5d3660 Mon Sep 17 00:00:00 2001
From: lain <lain@soykaf.club>
Date: Tue, 26 Mar 2019 16:40:09 +0100
Subject: [PATCH] AP UserView: Refactor banner / avatar display code, add test.

---
 lib/pleroma/user.ex                           | 16 ++++---------
 lib/pleroma/web/activity_pub/utils.ex         | 15 ------------
 .../web/activity_pub/views/user_view.ex       | 20 ++++++++++++----
 .../web/activity_pub/views/user_view_test.exs | 23 +++++++++++++++++++
 4 files changed, 42 insertions(+), 32 deletions(-)

diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex
index ee5eb8efa..efb4540b9 100644
--- a/lib/pleroma/user.ex
+++ b/lib/pleroma/user.ex
@@ -83,25 +83,17 @@ def superuser?(%User{local: true, info: %User.Info{is_admin: true}}), do: true
   def superuser?(%User{local: true, info: %User.Info{is_moderator: true}}), do: true
   def superuser?(_), do: false
 
-  def avatar_url(user) do
+  def avatar_url(user, options \\ []) do
     case user.avatar do
       %{"url" => [%{"href" => href} | _]} -> href
-      _ -> "#{Web.base_url()}/images/avi.png"
+      _ -> !options[:no_default] && "#{Web.base_url()}/images/avi.png"
     end
   end
 
-  # Do not return instance default avatar for federation
-  def avatar_url_ap(user) do
-    case user.avatar do
-      %{"url" => [%{"href" => href} | _]} -> href
-      _ -> nil
-    end
-  end
-
-  def banner_url(user) do
+  def banner_url(user, options \\ []) do
     case user.info.banner do
       %{"url" => [%{"href" => href} | _]} -> href
-      _ -> "#{Web.base_url()}/images/banner.png"
+      _ -> !options[:no_default] && "#{Web.base_url()}/images/banner.png"
     end
   end
 
diff --git a/lib/pleroma/web/activity_pub/utils.ex b/lib/pleroma/web/activity_pub/utils.ex
index 86b6e7029..2e9ffe41c 100644
--- a/lib/pleroma/web/activity_pub/utils.ex
+++ b/lib/pleroma/web/activity_pub/utils.ex
@@ -230,21 +230,6 @@ def update_object_in_activities(%{data: %{"id" => id}} = object) do
     end)
   end
 
-  # Only federate user icon if not nil
-  # Prevents federating instance default avatars
-  def maybe_make_icon(user) do
-    if User.avatar_url_ap(user) do
-      %{
-        "icon" => %{
-          "type" => "Image",
-          "url" => User.avatar_url_ap(user)
-        }
-      }
-    else
-      %{}
-    end
-  end
-
   #### Like-related helpers
 
   @doc """
diff --git a/lib/pleroma/web/activity_pub/views/user_view.ex b/lib/pleroma/web/activity_pub/views/user_view.ex
index f5c86d360..5926a3294 100644
--- a/lib/pleroma/web/activity_pub/views/user_view.ex
+++ b/lib/pleroma/web/activity_pub/views/user_view.ex
@@ -87,13 +87,10 @@ def render("user.json", %{user: user}) do
         "publicKeyPem" => public_key
       },
       "endpoints" => endpoints,
-      "image" => %{
-        "type" => "Image",
-        "url" => User.banner_url(user)
-      },
       "tag" => user.info.source_data["tag"] || []
     }
-    |> Map.merge(Utils.maybe_make_icon(user))
+    |> Map.merge(maybe_make_image(&User.avatar_url/2, "icon", user))
+    |> Map.merge(maybe_make_image(&User.banner_url/2, "image", user))
     |> Map.merge(Utils.make_json_ld_header())
   end
 
@@ -291,4 +288,17 @@ def collection(collection, iri, page, show_items \\ true, total \\ nil) do
       map
     end
   end
+
+  defp maybe_make_image(func, key, user) do
+    if image = func.(user, no_default: true) do
+      %{
+        key => %{
+          "type" => "Image",
+          "url" => image
+        }
+      }
+    else
+      %{}
+    end
+  end
 end
diff --git a/test/web/activity_pub/views/user_view_test.exs b/test/web/activity_pub/views/user_view_test.exs
index 0bc1d4728..9fb9455d2 100644
--- a/test/web/activity_pub/views/user_view_test.exs
+++ b/test/web/activity_pub/views/user_view_test.exs
@@ -16,6 +16,29 @@ test "Renders a user, including the public key" do
     assert String.contains?(result["publicKey"]["publicKeyPem"], "BEGIN PUBLIC KEY")
   end
 
+  test "Does not add an avatar image if the user hasn't set one" do
+    user = insert(:user)
+    {:ok, user} = Pleroma.Web.WebFinger.ensure_keys_present(user)
+
+    result = UserView.render("user.json", %{user: user})
+    refute result["icon"]
+    refute result["image"]
+
+    user =
+      insert(:user,
+        avatar: %{"url" => [%{"href" => "https://someurl"}]},
+        info: %{
+          banner: %{"url" => [%{"href" => "https://somebanner"}]}
+        }
+      )
+
+    {:ok, user} = Pleroma.Web.WebFinger.ensure_keys_present(user)
+
+    result = UserView.render("user.json", %{user: user})
+    assert result["icon"]["url"] == "https://someurl"
+    assert result["image"]["url"] == "https://somebanner"
+  end
+
   describe "endpoints" do
     test "local users have a usable endpoints structure" do
       user = insert(:user)