Merge branch 'exclude-visibilities-for-like-notifications' into 'develop'

Fix exclude_visibilities filter for followers-only Like notifications See merge request pleroma/pleroma!1957
2024-11-23 15:05:45 +00:00 · 2019-12-09 14:55:42 +00:00 · 2019-12-09 14:55:42 +00:00 · b3c80b8404
parent f3c2363f4a f86a7d5d8b
commit b3c80b8404
2 changed files with 153 additions and 49 deletions
--- a/lib/pleroma/notification.ex
+++ b/lib/pleroma/notification.ex
@ -121,10 +121,28 @@ defmodule Pleroma.Notification do
       when is_list(visibility) do
    if Enum.all?(visibility, &(&1 in @valid_visibilities)) do
      query
+      |> join(:left, [n, a], mutated_activity in Pleroma.Activity,
+        on:
+          fragment("?->>'context'", a.data) ==
+            fragment("?->>'context'", mutated_activity.data) and
+            fragment("(?->>'type' = 'Like' or ?->>'type' = 'Announce')", a.data, a.data) and
+            fragment("?->>'type'", mutated_activity.data) == "Create",
+        as: :mutated_activity
+      )
      |> where(
-        [n, a],
+        [n, a, mutated_activity: mutated_activity],
        not fragment(
-          "activity_visibility(?, ?, ?) = ANY (?)",
+          """
+          CASE WHEN (?->>'type') = 'Like' or (?->>'type') = 'Announce'
+            THEN (activity_visibility(?, ?, ?) = ANY (?))
+            ELSE (activity_visibility(?, ?, ?) = ANY (?)) END
+          """,
+          a.data,
+          a.data,
+          mutated_activity.actor,
+          mutated_activity.recipients,
+          mutated_activity.data,
+          ^visibility,
          a.actor,
          a.recipients,
          a.data,
@ -139,17 +157,7 @@ defmodule Pleroma.Notification do

  defp exclude_visibility(query, %{exclude_visibilities: visibility})
       when visibility in @valid_visibilities do
-    query
-    |> where(
-      [n, a],
-      not fragment(
-        "activity_visibility(?, ?, ?) = (?)",
-        a.actor,
-        a.recipients,
-        a.data,
-        ^visibility
-      )
-    )
+    exclude_visibility(query, [visibility])
  end

  defp exclude_visibility(query, %{exclude_visibilities: visibility})
--- a/test/web/mastodon_api/controllers/notification_controller_test.exs
+++ b/test/web/mastodon_api/controllers/notification_controller_test.exs
@ -137,55 +137,151 @@ defmodule Pleroma.Web.MastodonAPI.NotificationControllerTest do
    assert [%{"id" => ^notification3_id}, %{"id" => ^notification2_id}] = result
  end

-  test "filters notifications using exclude_visibilities", %{conn: conn} do
-    user = insert(:user)
-    other_user = insert(:user)
+  describe "exclude_visibilities" do
+    test "filters notifications for mentions", %{conn: conn} do
+      user = insert(:user)
+      other_user = insert(:user)

-    {:ok, public_activity} =
-      CommonAPI.post(other_user, %{"status" => "@#{user.nickname}", "visibility" => "public"})
+      {:ok, public_activity} =
+        CommonAPI.post(other_user, %{"status" => "@#{user.nickname}", "visibility" => "public"})

-    {:ok, direct_activity} =
-      CommonAPI.post(other_user, %{"status" => "@#{user.nickname}", "visibility" => "direct"})
+      {:ok, direct_activity} =
+        CommonAPI.post(other_user, %{"status" => "@#{user.nickname}", "visibility" => "direct"})

-    {:ok, unlisted_activity} =
-      CommonAPI.post(other_user, %{"status" => "@#{user.nickname}", "visibility" => "unlisted"})
+      {:ok, unlisted_activity} =
+        CommonAPI.post(other_user, %{"status" => "@#{user.nickname}", "visibility" => "unlisted"})

-    {:ok, private_activity} =
-      CommonAPI.post(other_user, %{"status" => "@#{user.nickname}", "visibility" => "private"})
+      {:ok, private_activity} =
+        CommonAPI.post(other_user, %{"status" => "@#{user.nickname}", "visibility" => "private"})

-    conn = assign(conn, :user, user)
+      conn = assign(conn, :user, user)

-    conn_res =
-      get(conn, "/api/v1/notifications", %{
-        exclude_visibilities: ["public", "unlisted", "private"]
-      })
+      conn_res =
+        get(conn, "/api/v1/notifications", %{
+          exclude_visibilities: ["public", "unlisted", "private"]
+        })

-    assert [%{"status" => %{"id" => id}}] = json_response(conn_res, 200)
-    assert id == direct_activity.id
+      assert [%{"status" => %{"id" => id}}] = json_response(conn_res, 200)
+      assert id == direct_activity.id

-    conn_res =
-      get(conn, "/api/v1/notifications", %{
-        exclude_visibilities: ["public", "unlisted", "direct"]
-      })
+      conn_res =
+        get(conn, "/api/v1/notifications", %{
+          exclude_visibilities: ["public", "unlisted", "direct"]
+        })

-    assert [%{"status" => %{"id" => id}}] = json_response(conn_res, 200)
-    assert id == private_activity.id
+      assert [%{"status" => %{"id" => id}}] = json_response(conn_res, 200)
+      assert id == private_activity.id

-    conn_res =
-      get(conn, "/api/v1/notifications", %{
-        exclude_visibilities: ["public", "private", "direct"]
-      })
+      conn_res =
+        get(conn, "/api/v1/notifications", %{
+          exclude_visibilities: ["public", "private", "direct"]
+        })

-    assert [%{"status" => %{"id" => id}}] = json_response(conn_res, 200)
-    assert id == unlisted_activity.id
+      assert [%{"status" => %{"id" => id}}] = json_response(conn_res, 200)
+      assert id == unlisted_activity.id

-    conn_res =
-      get(conn, "/api/v1/notifications", %{
-        exclude_visibilities: ["unlisted", "private", "direct"]
-      })
+      conn_res =
+        get(conn, "/api/v1/notifications", %{
+          exclude_visibilities: ["unlisted", "private", "direct"]
+        })

-    assert [%{"status" => %{"id" => id}}] = json_response(conn_res, 200)
-    assert id == public_activity.id
+      assert [%{"status" => %{"id" => id}}] = json_response(conn_res, 200)
+      assert id == public_activity.id
+    end
+
+    test "filters notifications for Like activities", %{conn: conn} do
+      user = insert(:user)
+      other_user = insert(:user)
+
+      {:ok, public_activity} =
+        CommonAPI.post(other_user, %{"status" => ".", "visibility" => "public"})
+
+      {:ok, direct_activity} =
+        CommonAPI.post(other_user, %{"status" => "@#{user.nickname}", "visibility" => "direct"})
+
+      {:ok, unlisted_activity} =
+        CommonAPI.post(other_user, %{"status" => ".", "visibility" => "unlisted"})
+
+      {:ok, private_activity} =
+        CommonAPI.post(other_user, %{"status" => ".", "visibility" => "private"})
+
+      {:ok, _, _} = CommonAPI.favorite(public_activity.id, user)
+      {:ok, _, _} = CommonAPI.favorite(direct_activity.id, user)
+      {:ok, _, _} = CommonAPI.favorite(unlisted_activity.id, user)
+      {:ok, _, _} = CommonAPI.favorite(private_activity.id, user)
+
+      activity_ids =
+        conn
+        |> assign(:user, other_user)
+        |> get("/api/v1/notifications", %{exclude_visibilities: ["direct"]})
+        |> json_response(200)
+        |> Enum.map(& &1["status"]["id"])
+
+      assert public_activity.id in activity_ids
+      assert unlisted_activity.id in activity_ids
+      assert private_activity.id in activity_ids
+      refute direct_activity.id in activity_ids
+
+      activity_ids =
+        conn
+        |> assign(:user, other_user)
+        |> get("/api/v1/notifications", %{exclude_visibilities: ["unlisted"]})
+        |> json_response(200)
+        |> Enum.map(& &1["status"]["id"])
+
+      assert public_activity.id in activity_ids
+      refute unlisted_activity.id in activity_ids
+      assert private_activity.id in activity_ids
+      assert direct_activity.id in activity_ids
+
+      activity_ids =
+        conn
+        |> assign(:user, other_user)
+        |> get("/api/v1/notifications", %{exclude_visibilities: ["private"]})
+        |> json_response(200)
+        |> Enum.map(& &1["status"]["id"])
+
+      assert public_activity.id in activity_ids
+      assert unlisted_activity.id in activity_ids
+      refute private_activity.id in activity_ids
+      assert direct_activity.id in activity_ids
+
+      activity_ids =
+        conn
+        |> assign(:user, other_user)
+        |> get("/api/v1/notifications", %{exclude_visibilities: ["public"]})
+        |> json_response(200)
+        |> Enum.map(& &1["status"]["id"])
+
+      refute public_activity.id in activity_ids
+      assert unlisted_activity.id in activity_ids
+      assert private_activity.id in activity_ids
+      assert direct_activity.id in activity_ids
+    end
+
+    test "filters notifications for Announce activities", %{conn: conn} do
+      user = insert(:user)
+      other_user = insert(:user)
+
+      {:ok, public_activity} =
+        CommonAPI.post(other_user, %{"status" => ".", "visibility" => "public"})
+
+      {:ok, unlisted_activity} =
+        CommonAPI.post(other_user, %{"status" => ".", "visibility" => "unlisted"})
+
+      {:ok, _, _} = CommonAPI.repeat(public_activity.id, user)
+      {:ok, _, _} = CommonAPI.repeat(unlisted_activity.id, user)
+
+      activity_ids =
+        conn
+        |> assign(:user, other_user)
+        |> get("/api/v1/notifications", %{exclude_visibilities: ["unlisted"]})
+        |> json_response(200)
+        |> Enum.map(& &1["status"]["id"])
+
+      assert public_activity.id in activity_ids
+      refute unlisted_activity.id in activity_ids
+    end
  end

  test "filters notifications using exclude_types", %{conn: conn} do