Merge branch 'notification-fixes' into 'develop'

Notification performance fixes

See merge request pleroma/pleroma!2595

lib/pleroma/notification.ex

@@ -92,8 +92,9 @@ def for_user_query(user, opts \\ %{}) do
     |> join(:left, [n, a], object in Object,
       on:
         fragment(
-          "(?->>'id') = COALESCE((? -> 'object'::text) ->> 'id'::text)",
+          "(?->>'id') = COALESCE(?->'object'->>'id', ?->>'object')",
           object.data,
+          a.data,
           a.data
         )
     )
@@ -224,18 +225,8 @@ def set_read_up_to(%{id: user_id} = user, id) do
       |> Marker.multi_set_last_read_id(user, "notifications")
       |> Repo.transaction()
 
-    Notification
+    for_user_query(user)
     |> where([n], n.id in ^notification_ids)
-    |> join(:inner, [n], activity in assoc(n, :activity))
-    |> join(:left, [n, a], object in Object,
-      on:
-        fragment(
-          "(?->>'id') = COALESCE((? -> 'object'::text) ->> 'id'::text)",
-          object.data,
-          a.data
-        )
-    )
-    |> preload([n, a, o], activity: {a, object: o})
     |> Repo.all()
   end
 

priv/repo/migrations/20200527104138_change_notification_user_index.exs

@@ -0,0 +1,8 @@
+defmodule Pleroma.Repo.Migrations.ChangeNotificationUserIndex do
+  use Ecto.Migration
+
+  def change do
+    drop_if_exists(index(:notifications, [:user_id]))
+    create_if_not_exists(index(:notifications, [:user_id, "id desc nulls last"]))
+  end
+end

test/notification_test.exs

@@ -454,8 +454,7 @@ test "it sets all notifications as read up to a specified notification ID" do
           status: "hey again @#{other_user.nickname}!"
         })
 
-      [n2, n1] = notifs = Notification.for_user(other_user)
+      [n2, n1] = Notification.for_user(other_user)
-      assert length(notifs) == 2
 
       assert n2.id > n1.id
 
@@ -464,7 +463,9 @@ test "it sets all notifications as read up to a specified notification ID" do
           status: "hey yet again @#{other_user.nickname}!"
         })
 
-      Notification.set_read_up_to(other_user, n2.id)
+      [_, read_notification] = Notification.set_read_up_to(other_user, n2.id)
+
+      assert read_notification.activity.object
 
       [n3, n2, n1] = Notification.for_user(other_user)
 
@@ -972,7 +973,9 @@ test "it returns notifications for muted user without notifications" do
 
       {:ok, _activity} = CommonAPI.post(muted, %{status: "hey @#{user.nickname}"})
 
-      assert length(Notification.for_user(user)) == 1
+      [notification] = Notification.for_user(user)
+
+      assert notification.activity.object
     end
 
     test "it doesn't return notifications for muted user with notifications" do