Chats: Remove `unread` from the db, calculate from unseen messages.
authorlain <lain@soykaf.club>
Wed, 3 Jun 2020 12:26:50 +0000 (14:26 +0200)
committerlain <lain@soykaf.club>
Wed, 3 Jun 2020 12:26:50 +0000 (14:26 +0200)
lib/pleroma/chat.ex
lib/pleroma/chat_message_reference.ex
lib/pleroma/web/activity_pub/side_effects.ex
lib/pleroma/web/pleroma_api/controllers/chat_controller.ex
lib/pleroma/web/pleroma_api/views/chat_view.ex
priv/repo/migrations/20200603120448_remove_unread_from_chats.exs [new file with mode: 0644]
test/chat_test.exs
test/web/activity_pub/side_effects_test.exs
test/web/pleroma_api/controllers/chat_controller_test.exs

index 65938c7a450ba162c26b2438c697fbacb2518019..5aefddc5e6895dc62d1cabe4439520e1888507ee 100644 (file)
@@ -19,14 +19,13 @@ defmodule Pleroma.Chat do
   schema "chats" do
     belongs_to(:user, User, type: FlakeId.Ecto.CompatType)
     field(:recipient, :string)
-    field(:unread, :integer, default: 0, read_after_writes: true)
 
     timestamps()
   end
 
   def creation_cng(struct, params) do
     struct
-    |> cast(params, [:user_id, :recipient, :unread])
+    |> cast(params, [:user_id, :recipient])
     |> validate_change(:recipient, fn
       :recipient, recipient ->
         case User.get_cached_by_ap_id(recipient) do
@@ -61,16 +60,10 @@ defmodule Pleroma.Chat do
 
   def bump_or_create(user_id, recipient) do
     %__MODULE__{}
-    |> creation_cng(%{user_id: user_id, recipient: recipient, unread: 1})
+    |> creation_cng(%{user_id: user_id, recipient: recipient})
     |> Repo.insert(
-      on_conflict: [set: [updated_at: NaiveDateTime.utc_now()], inc: [unread: 1]],
+      on_conflict: [set: [updated_at: NaiveDateTime.utc_now()]],
       conflict_target: [:user_id, :recipient]
     )
   end
-
-  def mark_as_read(chat) do
-    chat
-    |> change(%{unread: 0})
-    |> Repo.update()
-  end
 end
index 6808d13657c9d71e847460250bdecae8f99fc2cb..ad174b29451f27a0ecb9291c5b245c75fe2cd9b4 100644 (file)
@@ -84,4 +84,20 @@ defmodule Pleroma.ChatMessageReference do
     |> changeset(params)
     |> Repo.insert()
   end
+
+  def unread_count_for_chat(chat) do
+    chat
+    |> for_chat_query()
+    |> where([cmr], cmr.seen == false)
+    |> Repo.aggregate(:count)
+  end
+
+  def set_all_seen_for_chat(chat) do
+    chat
+    |> for_chat_query()
+    |> exclude(:order_by)
+    |> exclude(:preload)
+    |> where([cmr], cmr.seen == false)
+    |> Repo.update_all(set: [seen: true])
+  end
 end
index cda52b00e92e6c5173423dd95109c929dbd785fd..884d399d043e542545dfbd930d536c78f04b5638 100644 (file)
@@ -139,13 +139,8 @@ defmodule Pleroma.Web.ActivityPub.SideEffects do
       [[actor, recipient], [recipient, actor]]
       |> Enum.each(fn [user, other_user] ->
         if user.local do
-          if user.ap_id == actor.ap_id do
-            {:ok, chat} = Chat.get_or_create(user.id, other_user.ap_id)
-            ChatMessageReference.create(chat, object, true)
-          else
-            {:ok, chat} = Chat.bump_or_create(user.id, other_user.ap_id)
-            ChatMessageReference.create(chat, object, false)
-          end
+          {:ok, chat} = Chat.bump_or_create(user.id, other_user.ap_id)
+          ChatMessageReference.create(chat, object, user.ap_id == actor.ap_id)
         end
       end)
 
index f22f33de975e580994665576ad09cd97c19c927e..29922da999dfdb109eff3c345d572ff988291b9b 100644 (file)
@@ -90,7 +90,7 @@ defmodule Pleroma.Web.PleromaAPI.ChatController do
 
   def mark_as_read(%{assigns: %{user: %{id: user_id}}} = conn, %{id: id}) do
     with %Chat{} = chat <- Repo.get_by(Chat, id: id, user_id: user_id),
-         {:ok, chat} <- Chat.mark_as_read(chat) do
+         {_n, _} <- ChatMessageReference.set_all_seen_for_chat(chat) do
       conn
       |> put_view(ChatView)
       |> render("show.json", chat: chat)
index 331c1d2823e7421a4dd8fff26875328992058813..c903a71fde3d6f2bcd5642a5b049135469eeee5e 100644 (file)
@@ -20,7 +20,7 @@ defmodule Pleroma.Web.PleromaAPI.ChatView do
     %{
       id: chat.id |> to_string(),
       account: AccountView.render("show.json", Map.put(opts, :user, recipient)),
-      unread: chat.unread,
+      unread: ChatMessageReference.unread_count_for_chat(chat),
       last_message:
         last_message &&
           ChatMessageReferenceView.render("show.json", chat_message_reference: last_message),
diff --git a/priv/repo/migrations/20200603120448_remove_unread_from_chats.exs b/priv/repo/migrations/20200603120448_remove_unread_from_chats.exs
new file mode 100644 (file)
index 0000000..6322137
--- /dev/null
@@ -0,0 +1,9 @@
+defmodule Pleroma.Repo.Migrations.RemoveUnreadFromChats do
+  use Ecto.Migration
+
+  def change do
+    alter table(:chats) do
+      remove(:unread, :integer, default: 0)
+    end
+  end
+end
index 42e01fe2728933405e52be8f16970ba1ded9e8c8..332f2180a1d5e92dc0d837cc1b9f9da8ba7525e2 100644 (file)
@@ -6,7 +6,6 @@ defmodule Pleroma.ChatTest do
   use Pleroma.DataCase, async: true
 
   alias Pleroma.Chat
-  alias Pleroma.Web.CommonAPI
 
   import Pleroma.Factory
 
@@ -35,7 +34,6 @@ defmodule Pleroma.ChatTest do
       {:ok, chat_two} = Chat.bump_or_create(user.id, other_user.ap_id)
 
       assert chat.id == chat_two.id
-      assert chat_two.unread == 2
     end
 
     test "it returns a chat for a user and recipient if it already exists" do
@@ -48,15 +46,13 @@ defmodule Pleroma.ChatTest do
       assert chat.id == chat_two.id
     end
 
-    test "a returning chat will have an updated `update_at` field and an incremented unread count" do
+    test "a returning chat will have an updated `update_at` field" do
       user = insert(:user)
       other_user = insert(:user)
 
       {:ok, chat} = Chat.bump_or_create(user.id, other_user.ap_id)
-      assert chat.unread == 1
       :timer.sleep(1500)
       {:ok, chat_two} = Chat.bump_or_create(user.id, other_user.ap_id)
-      assert chat_two.unread == 2
 
       assert chat.id == chat_two.id
       assert chat.updated_at != chat_two.updated_at
index ff6b3ac15c6645d408838f28a428562f73de3983..f2fa062b4f30bc07bca5f644c5888cadc8c4573d 100644 (file)
@@ -346,7 +346,6 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do
         SideEffects.handle(create_activity, local: false, object_data: chat_message_data)
 
       chat = Chat.get(author.id, recipient.ap_id)
-      assert chat.unread == 0
 
       [cm_ref] = ChatMessageReference.for_chat_query(chat) |> Repo.all()
 
@@ -354,7 +353,6 @@ defmodule Pleroma.Web.ActivityPub.SideEffectsTest do
       assert cm_ref.seen == true
 
       chat = Chat.get(recipient.id, author.ap_id)
-      assert chat.unread == 1
 
       [cm_ref] = ChatMessageReference.for_chat_query(chat) |> Repo.all()
 
index bd4024c09965aa62a73ecfe6fa5ef1400a56776e..e62b717995c941f7c4d4457fee7b32dd986a1aaf 100644 (file)
@@ -19,9 +19,12 @@ defmodule Pleroma.Web.PleromaAPI.ChatControllerTest do
     test "it marks all messages in a chat as read", %{conn: conn, user: user} do
       other_user = insert(:user)
 
-      {:ok, chat} = Chat.bump_or_create(user.id, other_user.ap_id)
+      {:ok, create} = CommonAPI.post_chat_message(other_user, user, "sup")
+      {:ok, chat} = Chat.get_or_create(user.id, other_user.ap_id)
+      object = Object.normalize(create, false)
+      cm_ref = ChatMessageReference.for_chat_and_object(chat, object)
 
-      assert chat.unread == 1
+      assert cm_ref.seen == false
 
       result =
         conn
@@ -30,9 +33,9 @@ defmodule Pleroma.Web.PleromaAPI.ChatControllerTest do
 
       assert result["unread"] == 0
 
-      {:ok, chat} = Chat.get_or_create(user.id, other_user.ap_id)
+      cm_ref = ChatMessageReference.for_chat_and_object(chat, object)
 
-      assert chat.unread == 0
+      assert cm_ref.seen == true
     end
   end