Merge branch 'conversation-read-status-for-blocking-user' into 'develop'

Count only visible conversations in the user's unread_conversation_count

See merge request pleroma/pleroma!1886

lib/pleroma/conversation.ex

@@ -67,7 +67,13 @@ def create_or_bump_for(activity, opts \\ []) do
 
       participations =
         Enum.map(users, fn user ->
-          User.increment_unread_conversation_count(conversation, user)
+          invisible_conversation = Enum.any?(users, &User.blocks?(user, &1))
+
+          unless invisible_conversation do
+            User.increment_unread_conversation_count(conversation, user)
+          end
+
+          opts = Keyword.put(opts, :invisible_conversation, invisible_conversation)
 
           {:ok, participation} =
             Participation.create_for_user_and_conversation(user, conversation, opts)

lib/pleroma/conversation/participation.ex

@@ -32,11 +32,20 @@ def creation_cng(struct, params) do
 
   def create_for_user_and_conversation(user, conversation, opts \\ []) do
     read = !!opts[:read]
+    invisible_conversation = !!opts[:invisible_conversation]
+
+    update_on_conflict =
+      if(invisible_conversation, do: [], else: [read: read])
+      |> Keyword.put(:updated_at, NaiveDateTime.utc_now())
 
     %__MODULE__{}
-    |> creation_cng(%{user_id: user.id, conversation_id: conversation.id, read: read})
+    |> creation_cng(%{
+      user_id: user.id,
+      conversation_id: conversation.id,
+      read: invisible_conversation || read
+    })
     |> Repo.insert(
-      on_conflict: [set: [read: read, updated_at: NaiveDateTime.utc_now()]],
+      on_conflict: [set: update_on_conflict],
       returning: true,
       conflict_target: [:user_id, :conversation_id]
     )
@@ -69,7 +78,26 @@ def mark_as_read(participation) do
     end
   end
 
-  def mark_all_as_read(user) do
+  def mark_all_as_read(%User{local: true} = user, %User{} = target_user) do
+    target_conversation_ids =
+      __MODULE__
+      |> where([p], p.user_id == ^target_user.id)
+      |> select([p], p.conversation_id)
+      |> Repo.all()
+
+    __MODULE__
+    |> where([p], p.user_id == ^user.id)
+    |> where([p], p.conversation_id in ^target_conversation_ids)
+    |> update([p], set: [read: true])
+    |> Repo.update_all([])
+
+    {:ok, user} = User.set_unread_conversation_count(user)
+    {:ok, user, []}
+  end
+
+  def mark_all_as_read(%User{} = user, %User{}), do: {:ok, user, []}
+
+  def mark_all_as_read(%User{} = user) do
     {_, participations} =
       __MODULE__
       |> where([p], p.user_id == ^user.id)
@@ -78,8 +106,8 @@ def mark_all_as_read(user) do
       |> select([p], p)
       |> Repo.update_all([])
 
-    User.set_unread_conversation_count(user)
-    {:ok, participations}
+    {:ok, user} = User.set_unread_conversation_count(user)
+    {:ok, user, participations}
   end
 
   def mark_as_unread(participation) do

lib/pleroma/user.ex

@@ -971,7 +971,7 @@ def set_unread_conversation_count(%User{local: true} = user) do
     end
   end
 
-  def set_unread_conversation_count(_), do: :noop
+  def set_unread_conversation_count(user), do: {:ok, user}
 
   def increment_unread_conversation_count(conversation, %User{local: true} = user) do
     unread_query =
@@ -993,7 +993,7 @@ def increment_unread_conversation_count(conversation, %User{local: true} = user)
     end
   end
 
-  def increment_unread_conversation_count(_, _), do: :noop
+  def increment_unread_conversation_count(_, user), do: {:ok, user}
 
   def remove_duplicated_following(%User{following: following} = user) do
     uniq_following = Enum.uniq(following)
@@ -1077,7 +1077,7 @@ def block(blocker, %User{ap_id: ap_id} = blocked) do
     if following?(blocked, blocker), do: unfollow(blocked, blocker)
 
     {:ok, blocker} = update_follower_count(blocker)
-
+    {:ok, blocker, _} = Participation.mark_all_as_read(blocker, blocked)
     add_to_block(blocker, ap_id)
   end
 

lib/pleroma/web/pleroma_api/controllers/pleroma_api_controller.ex

@@ -80,7 +80,7 @@ def update_conversation(
   end
 
   def read_conversations(%{assigns: %{user: user}} = conn, _params) do
-    with {:ok, participations} <- Participation.mark_all_as_read(user) do
+    with {:ok, _, participations} <- Participation.mark_all_as_read(user) do
       conn
       |> add_link_headers(participations)
       |> put_view(ConversationView)

test/conversation/participation_test.exs

@@ -140,7 +140,7 @@ test "it marks all the user's participations as read" do
     participation2 = insert(:participation, %{read: false, user: user})
     participation3 = insert(:participation, %{read: false, user: other_user})
 
-    {:ok, [%{read: true}, %{read: true}]} = Participation.mark_all_as_read(user)
+    {:ok, _, [%{read: true}, %{read: true}]} = Participation.mark_all_as_read(user)
 
     assert Participation.get(participation1.id).read == true
     assert Participation.get(participation2.id).read == true
@@ -216,4 +216,134 @@ test "it sets recipients, always keeping the owner of the participation even whe
     assert user in participation.recipients
     assert other_user in participation.recipients
   end
+
+  describe "blocking" do
+    test "when the user blocks a recipient, the existing conversations with them are marked as read" do
+      blocker = insert(:user)
+      blocked = insert(:user)
+      third_user = insert(:user)
+
+      {:ok, _direct1} =
+        CommonAPI.post(third_user, %{
+          "status" => "Hi @#{blocker.nickname}",
+          "visibility" => "direct"
+        })
+
+      {:ok, _direct2} =
+        CommonAPI.post(third_user, %{
+          "status" => "Hi @#{blocker.nickname}, @#{blocked.nickname}",
+          "visibility" => "direct"
+        })
+
+      {:ok, _direct3} =
+        CommonAPI.post(blocked, %{
+          "status" => "Hi @#{blocker.nickname}",
+          "visibility" => "direct"
+        })
+
+      {:ok, _direct4} =
+        CommonAPI.post(blocked, %{
+          "status" => "Hi @#{blocker.nickname}, @#{third_user.nickname}",
+          "visibility" => "direct"
+        })
+
+      assert [%{read: false}, %{read: false}, %{read: false}, %{read: false}] =
+               Participation.for_user(blocker)
+
+      assert User.get_cached_by_id(blocker.id).unread_conversation_count == 4
+
+      {:ok, blocker} = User.block(blocker, blocked)
+
+      # The conversations with the blocked user are marked as read
+      assert [%{read: true}, %{read: true}, %{read: true}, %{read: false}] =
+               Participation.for_user(blocker)
+
+      assert User.get_cached_by_id(blocker.id).unread_conversation_count == 1
+
+      # The conversation is not marked as read for the blocked user
+      assert [_, _, %{read: false}] = Participation.for_user(blocked)
+      assert User.get_cached_by_id(blocked.id).unread_conversation_count == 1
+
+      # The conversation is not marked as read for the third user
+      assert [%{read: false}, _, _] = Participation.for_user(third_user)
+      assert User.get_cached_by_id(third_user.id).unread_conversation_count == 1
+    end
+
+    test "the new conversation with the blocked user is not marked as unread " do
+      blocker = insert(:user)
+      blocked = insert(:user)
+      third_user = insert(:user)
+
+      {:ok, blocker} = User.block(blocker, blocked)
+
+      # When the blocked user is the author
+      {:ok, _direct1} =
+        CommonAPI.post(blocked, %{
+          "status" => "Hi @#{blocker.nickname}",
+          "visibility" => "direct"
+        })
+
+      assert [%{read: true}] = Participation.for_user(blocker)
+      assert User.get_cached_by_id(blocker.id).unread_conversation_count == 0
+
+      # When the blocked user is a recipient
+      {:ok, _direct2} =
+        CommonAPI.post(third_user, %{
+          "status" => "Hi @#{blocker.nickname}, @#{blocked.nickname}",
+          "visibility" => "direct"
+        })
+
+      assert [%{read: true}, %{read: true}] = Participation.for_user(blocker)
+      assert User.get_cached_by_id(blocker.id).unread_conversation_count == 0
+
+      assert [%{read: false}, _] = Participation.for_user(blocked)
+      assert User.get_cached_by_id(blocked.id).unread_conversation_count == 1
+    end
+
+    test "the conversation with the blocked user is not marked as unread on a reply" do
+      blocker = insert(:user)
+      blocked = insert(:user)
+      third_user = insert(:user)
+
+      {:ok, _direct1} =
+        CommonAPI.post(blocker, %{
+          "status" => "Hi @#{third_user.nickname}, @#{blocked.nickname}",
+          "visibility" => "direct"
+        })
+
+      {:ok, blocker} = User.block(blocker, blocked)
+      assert [%{read: true}] = Participation.for_user(blocker)
+      assert User.get_cached_by_id(blocker.id).unread_conversation_count == 0
+
+      assert [blocked_participation] = Participation.for_user(blocked)
+
+      # When it's a reply from the blocked user
+      {:ok, _direct2} =
+        CommonAPI.post(blocked, %{
+          "status" => "reply",
+          "visibility" => "direct",
+          "in_reply_to_conversation_id" => blocked_participation.id
+        })
+
+      assert [%{read: true}] = Participation.for_user(blocker)
+      assert User.get_cached_by_id(blocker.id).unread_conversation_count == 0
+
+      assert [third_user_participation] = Participation.for_user(third_user)
+
+      # When it's a reply from the third user
+      {:ok, _direct3} =
+        CommonAPI.post(third_user, %{
+          "status" => "reply",
+          "visibility" => "direct",
+          "in_reply_to_conversation_id" => third_user_participation.id
+        })
+
+      assert [%{read: true}] = Participation.for_user(blocker)
+      assert User.get_cached_by_id(blocker.id).unread_conversation_count == 0
+
+      # Marked as unread for the blocked user
+      assert [%{read: false}] = Participation.for_user(blocked)
+      assert User.get_cached_by_id(blocked.id).unread_conversation_count == 1
+    end
+  end
 end