Merge branch 'bugfix/repeating-announce' into 'develop'

CommonAPI: Don't make repeating announces possible

See merge request pleroma/pleroma!2418

4 files changed, 28 insertions, 51 deletions

diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex
index f50a909aa..d1efe0c36 100644
--- a/lib/pleroma/web/common_api/common_api.ex
+++ b/lib/pleroma/web/common_api/common_api.ex
@@ -86,8 +86,9 @@ defmodule Pleroma.Web.CommonAPI do
     end
   end
 
-  def repeat(id_or_ap_id, user, params \\ %{}) do
-    with {_, %Activity{} = activity} <- {:find_activity, get_by_id_or_ap_id(id_or_ap_id)},
+  def repeat(id, user, params \\ %{}) do
+    with {_, %Activity{data: %{"type" => "Create"}} = activity} <-
+           {:find_activity, Activity.get_by_id(id)},
          object <- Object.normalize(activity),
          announce_activity <- Utils.get_existing_announce(user.ap_id, object),
          public <- public_announce?(object, params) do
@@ -102,8 +103,9 @@ defmodule Pleroma.Web.CommonAPI do
     end
   end
 
-  def unrepeat(id_or_ap_id, user) do
-    with {_, %Activity{} = activity} <- {:find_activity, get_by_id_or_ap_id(id_or_ap_id)} do
+  def unrepeat(id, user) do
+    with {_, %Activity{data: %{"type" => "Create"}} = activity} <-
+           {:find_activity, Activity.get_by_id(id)} do
       object = Object.normalize(activity)
       ActivityPub.unannounce(user, object)
     else
@@ -160,8 +162,9 @@ defmodule Pleroma.Web.CommonAPI do
     end
   end
 
-  def unfavorite(id_or_ap_id, user) do
-    with {_, %Activity{} = activity} <- {:find_activity, get_by_id_or_ap_id(id_or_ap_id)} do
+  def unfavorite(id, user) do
+    with {_, %Activity{data: %{"type" => "Create"}} = activity} <-
+           {:find_activity, Activity.get_by_id(id)} do
       object = Object.normalize(activity)
       ActivityPub.unlike(user, object)
     else
@@ -332,12 +335,12 @@ defmodule Pleroma.Web.CommonAPI do
 
   defp maybe_create_activity_expiration(result, _), do: result
 
-  def pin(id_or_ap_id, %{ap_id: user_ap_id} = user) do
+  def pin(id, %{ap_id: user_ap_id} = user) do
     with %Activity{
            actor: ^user_ap_id,
            data: %{"type" => "Create"},
            object: %Object{data: %{"type" => object_type}}
-         } = activity <- get_by_id_or_ap_id(id_or_ap_id),
+         } = activity <- Activity.get_by_id_with_object(id),
          true <- object_type in ["Note", "Article", "Question"],
          true <- Visibility.is_public?(activity),
          {:ok, _user} <- User.add_pinnned_activity(user, activity) do
@@ -348,8 +351,8 @@ defmodule Pleroma.Web.CommonAPI do
     end
   end
 
-  def unpin(id_or_ap_id, user) do
-    with %Activity{} = activity <- get_by_id_or_ap_id(id_or_ap_id),
+  def unpin(id, user) do
+    with %Activity{data: %{"type" => "Create"}} = activity <- Activity.get_by_id(id),
          {:ok, _user} <- User.remove_pinnned_activity(user, activity) do
       {:ok, activity}
     else
diff --git a/lib/pleroma/web/common_api/utils.ex b/lib/pleroma/web/common_api/utils.ex
index 7eec5aa09..945e63e22 100644
--- a/lib/pleroma/web/common_api/utils.ex
+++ b/lib/pleroma/web/common_api/utils.ex
@@ -22,24 +22,6 @@ defmodule Pleroma.Web.CommonAPI.Utils do
   require Logger
   require Pleroma.Constants
 
-  # This is a hack for twidere.
-  def get_by_id_or_ap_id(id) do
-    activity =
-      with true <- FlakeId.flake_id?(id),
-           %Activity{} = activity <- Activity.get_by_id_with_object(id) do
-        activity
-      else
-        _ -> Activity.get_create_by_object_ap_id_with_object(id)
-      end
-
-    activity &&
-      if activity.data["type"] == "Create" do
-        activity
-      else
-        Activity.get_create_by_object_ap_id_with_object(activity.data["object"])
-      end
-  end
-
   def attachments_from_ids(%{"media_ids" => ids, "descriptions" => desc} = _) do
     attachments_from_ids_descs(ids, desc)
   end
diff --git a/test/web/common_api/common_api_test.exs b/test/web/common_api/common_api_test.exs
index 68a29108a..e87193c83 100644
--- a/test/web/common_api/common_api_test.exs
+++ b/test/web/common_api/common_api_test.exs
@@ -283,6 +283,16 @@ defmodule Pleroma.Web.CommonAPITest do
       {:ok, %Activity{}, _} = CommonAPI.repeat(activity.id, user)
     end
 
+    test "can't repeat a repeat" do
+      user = insert(:user)
+      other_user = insert(:user)
+      {:ok, activity} = CommonAPI.post(other_user, %{"status" => "cofe"})
+
+      {:ok, %Activity{} = announce, _} = CommonAPI.repeat(activity.id, other_user)
+
+      refute match?({:ok, %Activity{}, _}, CommonAPI.repeat(announce.id, user))
+    end
+
     test "repeating a status privately" do
       user = insert(:user)
       other_user = insert(:user)
@@ -312,8 +322,8 @@ defmodule Pleroma.Web.CommonAPITest do
       other_user = insert(:user)
 
       {:ok, activity} = CommonAPI.post(other_user, %{"status" => "cofe"})
-      {:ok, %Activity{} = activity, object} = CommonAPI.repeat(activity.id, user)
-      {:ok, ^activity, ^object} = CommonAPI.repeat(activity.id, user)
+      {:ok, %Activity{} = announce, object} = CommonAPI.repeat(activity.id, user)
+      {:ok, ^announce, ^object} = CommonAPI.repeat(activity.id, user)
     end
 
     test "favoriting a status twice returns ok, but without the like activity" do
@@ -387,7 +397,9 @@ defmodule Pleroma.Web.CommonAPITest do
 
       user = refresh_record(user)
 
-      assert {:ok, ^activity} = CommonAPI.unpin(activity.id, user)
+      id = activity.id
+
+      assert match?({:ok, %{id: ^id}}, CommonAPI.unpin(activity.id, user))
 
       user = refresh_record(user)
 
diff --git a/test/web/common_api/common_api_utils_test.exs b/test/web/common_api/common_api_utils_test.exs
index b21445fe9..18a3b3b87 100644
--- a/test/web/common_api/common_api_utils_test.exs
+++ b/test/web/common_api/common_api_utils_test.exs
@@ -335,26 +335,6 @@ defmodule Pleroma.Web.CommonAPI.UtilsTest do
     end
   end
 
-  describe "get_by_id_or_ap_id/1" do
-    test "get activity by id" do
-      activity = insert(:note_activity)
-      %Pleroma.Activity{} = note = Utils.get_by_id_or_ap_id(activity.id)
-      assert note.id == activity.id
-    end
-
-    test "get activity by ap_id" do
-      activity = insert(:note_activity)
-      %Pleroma.Activity{} = note = Utils.get_by_id_or_ap_id(activity.data["object"])
-      assert note.id == activity.id
-    end
-
-    test "get activity by object when type isn't `Create` " do
-      activity = insert(:like_activity)
-      %Pleroma.Activity{} = like = Utils.get_by_id_or_ap_id(activity.id)
-      assert like.data["object"] == activity.data["object"]
-    end
-  end
-
   describe "to_master_date/1" do
     test "removes microseconds from date (NaiveDateTime)" do
       assert Utils.to_masto_date(~N[2015-01-23 23:50:07.123]) == "2015-01-23T23:50:07.000Z"