Mastodon API: ensure the notification endpoint doesn't return less than the...

Mastodon API: ensure the notification endpoint doesn't return less than the requested amount of records unless it's the last page

CHANGELOG.md

@@ -49,6 +49,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Filtering of push notifications on activities from blocked domains
 - Resolving Peertube accounts with Webfinger
 - `blob:` urls not being allowed by connect-src CSP
+- Mastodon API: fix `GET /api/v1/notifications` not returning the full result set
 
 ## [Unreleased (patch)]
 

lib/pleroma/notification.ex

@@ -166,8 +166,16 @@ defp exclude_visibility(query, %{exclude_visibilities: visibility})
       query
       |> join(:left, [n, a], mutated_activity in Pleroma.Activity,
         on:
-          fragment("?->>'context'", a.data) ==
-            fragment("?->>'context'", mutated_activity.data) and
+          fragment(
+            "COALESCE((?->'object')->>'id', ?->>'object')",
+            a.data,
+            a.data
+          ) ==
+            fragment(
+              "COALESCE((?->'object')->>'id', ?->>'object')",
+              mutated_activity.data,
+              mutated_activity.data
+            ) and
             fragment("(?->>'type' = 'Like' or ?->>'type' = 'Announce')", a.data, a.data) and
             fragment("?->>'type'", mutated_activity.data) == "Create",
         as: :mutated_activity
@@ -541,6 +549,7 @@ def exclude_thread_muter_ap_ids(ap_ids, %Activity{} = activity) do
   def skip?(%Activity{} = activity, %User{} = user) do
     [
       :self,
+      :invisible,
       :followers,
       :follows,
       :non_followers,
@@ -557,6 +566,12 @@ def skip?(:self, %Activity{} = activity, %User{} = user) do
     activity.data["actor"] == user.ap_id
   end
 
+  def skip?(:invisible, %Activity{} = activity, _) do
+    actor = activity.data["actor"]
+    user = User.get_cached_by_ap_id(actor)
+    User.invisible?(user)
+  end
+
   def skip?(
         :followers,
         %Activity{} = activity,

lib/pleroma/user.ex

@@ -1488,6 +1488,7 @@ def perform(:delete, %User{} = user) do
     end)
 
     delete_user_activities(user)
+    delete_notifications_from_user_activities(user)
 
     delete_outgoing_pending_follow_requests(user)
 
@@ -1576,6 +1577,13 @@ def follow_import(%User{} = follower, followed_identifiers)
     })
   end
 
+  def delete_notifications_from_user_activities(%User{ap_id: ap_id}) do
+    Notification
+    |> join(:inner, [n], activity in assoc(n, :activity))
+    |> where([n, a], fragment("? = ?", a.actor, ^ap_id))
+    |> Repo.delete_all()
+  end
+
   def delete_user_activities(%User{ap_id: ap_id} = user) do
     ap_id
     |> Activity.Queries.by_actor()

lib/pleroma/web/mastodon_api/views/notification_view.ex

@@ -46,6 +46,7 @@ def render("index.json", %{notifications: notifications, for: reading_user} = op
             activities
             |> Enum.filter(&(&1.data["type"] == "Move"))
             |> Enum.map(&User.get_cached_by_ap_id(&1.data["target"]))
+            |> Enum.filter(& &1)
 
           actors =
             activities
@@ -84,50 +85,45 @@ def render(
     # Note: :relationships contain user mutes (needed for :muted flag in :status)
     status_render_opts = %{relationships: opts[:relationships]}
 
-    with %{id: _} = account <-
-           AccountView.render(
-             "show.json",
-             %{user: actor, for: reading_user}
-           ) do
-      response = %{
-        id: to_string(notification.id),
-        type: notification.type,
-        created_at: CommonAPI.Utils.to_masto_date(notification.inserted_at),
-        account: account,
-        pleroma: %{
-          is_seen: notification.seen
-        }
+    account =
+      AccountView.render(
+        "show.json",
+        %{user: actor, for: reading_user}
+      )
+
+    response = %{
+      id: to_string(notification.id),
+      type: notification.type,
+      created_at: CommonAPI.Utils.to_masto_date(notification.inserted_at),
+      account: account,
+      pleroma: %{
+        is_seen: notification.seen
       }
+    }
 
-      case notification.type do
-        "mention" ->
-          put_status(response, activity, reading_user, status_render_opts)
+    case notification.type do
+      "mention" ->
+        put_status(response, activity, reading_user, status_render_opts)
 
-        "favourite" ->
-          put_status(response, parent_activity_fn.(), reading_user, status_render_opts)
+      "favourite" ->
+        put_status(response, parent_activity_fn.(), reading_user, status_render_opts)
 
-        "reblog" ->
-          put_status(response, parent_activity_fn.(), reading_user, status_render_opts)
+      "reblog" ->
+        put_status(response, parent_activity_fn.(), reading_user, status_render_opts)
 
-        "move" ->
-          put_target(response, activity, reading_user, %{})
+      "move" ->
+        put_target(response, activity, reading_user, %{})
 
-        "pleroma:emoji_reaction" ->
-          response
-          |> put_status(parent_activity_fn.(), reading_user, status_render_opts)
-          |> put_emoji(activity)
+      "pleroma:emoji_reaction" ->
+        response
+        |> put_status(parent_activity_fn.(), reading_user, status_render_opts)
+        |> put_emoji(activity)
 
-        "pleroma:chat_mention" ->
-          put_chat_message(response, activity, reading_user, status_render_opts)
+      "pleroma:chat_mention" ->
+        put_chat_message(response, activity, reading_user, status_render_opts)
 
-        type when type in ["follow", "follow_request"] ->
-          response
-
-        _ ->
-          nil
-      end
-    else
-      _ -> nil
+      type when type in ["follow", "follow_request"] ->
+        response
     end
   end
 

priv/repo/migrations/20200527163635_delete_notifications_from_invisible_users.exs

+defmodule Pleroma.Repo.Migrations.DeleteNotificationsFromInvisibleUsers do
+  use Ecto.Migration
+
+  import Ecto.Query
+  alias Pleroma.Repo
+
+  def up do
+    Pleroma.Notification
+    |> join(:inner, [n], activity in assoc(n, :activity))
+    |> where(
+      [n, a],
+      fragment("? in (SELECT ap_id FROM users WHERE invisible = true)", a.actor)
+    )
+    |> Repo.delete_all()
+  end
+
+  def down, do: :ok
+end

test/notification_test.exs

@@ -306,6 +306,14 @@ test "it doesn't create subscription notifications if the recipient cannot see t
 
       assert {:ok, []} == Notification.create_notifications(status)
     end
+
+    test "it disables notifications from people who are invisible" do
+      author = insert(:user, invisible: true)
+      user = insert(:user)
+
+      {:ok, status} = CommonAPI.post(author, %{status: "hey @#{user.nickname}"})
+      refute Notification.create_notification(status, user)
+    end
   end
 
   describe "follow / follow_request notifications" do

test/web/mastodon_api/controllers/notification_controller_test.exs

@@ -313,6 +313,33 @@ test "filters notifications for Announce activities" do
       assert public_activity.id in activity_ids
       refute unlisted_activity.id in activity_ids
     end
+
+    test "doesn't return less than the requested amount of records when the user's reply is liked" do
+      user = insert(:user)
+      %{user: other_user, conn: conn} = oauth_access(["read:notifications"])
+
+      {:ok, mention} =
+        CommonAPI.post(user, %{status: "@#{other_user.nickname}", visibility: "public"})
+
+      {:ok, activity} = CommonAPI.post(user, %{status: ".", visibility: "public"})
+
+      {:ok, reply} =
+        CommonAPI.post(other_user, %{
+          status: ".",
+          visibility: "public",
+          in_reply_to_status_id: activity.id
+        })
+
+      {:ok, _favorite} = CommonAPI.favorite(user, reply.id)
+
+      activity_ids =
+        conn
+        |> get("/api/v1/notifications?exclude_visibilities[]=direct&limit=2")
+        |> json_response_and_validate_schema(200)
+        |> Enum.map(& &1["status"]["id"])
+
+      assert [reply.id, mention.id] == activity_ids
+    end
   end
 
   test "filters notifications using exclude_types" do

test/web/mastodon_api/views/notification_view_test.exs

@@ -139,9 +139,7 @@ test "Follow notification" do
     test_notifications_rendering([notification], followed, [expected])
 
     User.perform(:delete, follower)
-    notification = Notification |> Repo.one() |> Repo.preload(:activity)
-
-    test_notifications_rendering([notification], followed, [])
+    refute Repo.one(Notification)
   end
 
   @tag capture_log: true