remove `unread_conversation_count` from User

This commit is contained in:
Maksim Pechnikov 2020-09-01 09:37:08 +03:00
parent e0eb90f62a
commit 0d5088c2b8
9 changed files with 92 additions and 94 deletions

View File

@ -43,7 +43,7 @@ def get_for_ap_id(ap_id) do
def maybe_create_recipientships(participation, activity) do def maybe_create_recipientships(participation, activity) do
participation = Repo.preload(participation, :recipients) participation = Repo.preload(participation, :recipients)
if participation.recipients |> Enum.empty?() do if Enum.empty?(participation.recipients) do
recipients = User.get_all_by_ap_id(activity.recipients) recipients = User.get_all_by_ap_id(activity.recipients)
RecipientShip.create(recipients, participation) RecipientShip.create(recipients, participation)
end end
@ -69,10 +69,6 @@ def create_or_bump_for(activity, opts \\ []) do
Enum.map(users, fn user -> Enum.map(users, fn user ->
invisible_conversation = Enum.any?(users, &User.blocks?(user, &1)) 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) opts = Keyword.put(opts, :invisible_conversation, invisible_conversation)
{:ok, participation} = {:ok, participation} =

View File

@ -63,21 +63,10 @@ def mark_as_read(%User{} = user, %Conversation{} = conversation) do
end end
end end
def mark_as_read(participation) do def mark_as_read(%__MODULE__{} = participation) do
__MODULE__ participation
|> where(id: ^participation.id) |> change(read: true)
|> update(set: [read: true]) |> Repo.update()
|> select([p], p)
|> Repo.update_all([])
|> case do
{1, [participation]} ->
participation = Repo.preload(participation, :user)
User.set_unread_conversation_count(participation.user)
{:ok, participation}
error ->
error
end
end end
def mark_all_as_read(%User{local: true} = user, %User{} = target_user) do def mark_all_as_read(%User{local: true} = user, %User{} = target_user) do
@ -93,7 +82,6 @@ def mark_all_as_read(%User{local: true} = user, %User{} = target_user) do
|> update([p], set: [read: true]) |> update([p], set: [read: true])
|> Repo.update_all([]) |> Repo.update_all([])
{:ok, user} = User.set_unread_conversation_count(user)
{:ok, user, []} {:ok, user, []}
end end
@ -108,7 +96,6 @@ def mark_all_as_read(%User{} = user) do
|> select([p], p) |> select([p], p)
|> Repo.update_all([]) |> Repo.update_all([])
{:ok, user} = User.set_unread_conversation_count(user)
{:ok, user, participations} {:ok, user, participations}
end end
@ -220,6 +207,12 @@ def set_recipients(participation, user_ids) do
{:ok, Repo.preload(participation, :recipients, force: true)} {:ok, Repo.preload(participation, :recipients, force: true)}
end end
@spec unread_count(User.t()) :: integer()
def unread_count(%User{id: user_id}) do
from(q in __MODULE__, where: q.user_id == ^user_id and q.read == false)
|> Repo.aggregate(:count, :id)
end
def unread_conversation_count_for_user(user) do def unread_conversation_count_for_user(user) do
from(p in __MODULE__, from(p in __MODULE__,
where: p.user_id == ^user.id, where: p.user_id == ^user.id,

View File

@ -129,7 +129,6 @@ defmodule Pleroma.User do
field(:hide_followers, :boolean, default: false) field(:hide_followers, :boolean, default: false)
field(:hide_follows, :boolean, default: false) field(:hide_follows, :boolean, default: false)
field(:hide_favorites, :boolean, default: true) field(:hide_favorites, :boolean, default: true)
field(:unread_conversation_count, :integer, default: 0)
field(:pinned_activities, {:array, :string}, default: []) field(:pinned_activities, {:array, :string}, default: [])
field(:email_notifications, :map, default: %{"digest" => false}) field(:email_notifications, :map, default: %{"digest" => false})
field(:mascot, :map, default: nil) field(:mascot, :map, default: nil)
@ -1295,47 +1294,6 @@ def update_following_count(%User{local: true} = user) do
|> update_and_set_cache() |> update_and_set_cache()
end end
def set_unread_conversation_count(%User{local: true} = user) do
unread_query = Participation.unread_conversation_count_for_user(user)
User
|> join(:inner, [u], p in subquery(unread_query))
|> update([u, p],
set: [unread_conversation_count: p.count]
)
|> where([u], u.id == ^user.id)
|> select([u], u)
|> Repo.update_all([])
|> case do
{1, [user]} -> set_cache(user)
_ -> {:error, user}
end
end
def set_unread_conversation_count(user), do: {:ok, user}
def increment_unread_conversation_count(conversation, %User{local: true} = user) do
unread_query =
Participation.unread_conversation_count_for_user(user)
|> where([p], p.conversation_id == ^conversation.id)
User
|> join(:inner, [u], p in subquery(unread_query))
|> update([u, p],
inc: [unread_conversation_count: 1]
)
|> where([u], u.id == ^user.id)
|> where([u, p], p.count == 0)
|> select([u], u)
|> Repo.update_all([])
|> case do
{1, [user]} -> set_cache(user)
_ -> {:error, user}
end
end
def increment_unread_conversation_count(_, user), do: {:ok, user}
@spec get_users_from_set([String.t()], keyword()) :: [User.t()] @spec get_users_from_set([String.t()], keyword()) :: [User.t()]
def get_users_from_set(ap_ids, opts \\ []) do def get_users_from_set(ap_ids, opts \\ []) do
local_only = Keyword.get(opts, :local_only, true) local_only = Keyword.get(opts, :local_only, true)

View File

@ -386,7 +386,7 @@ defp maybe_put_unread_conversation_count(data, %User{id: user_id} = user, %User{
data data
|> Kernel.put_in( |> Kernel.put_in(
[:pleroma, :unread_conversation_count], [:pleroma, :unread_conversation_count],
user.unread_conversation_count Pleroma.Conversation.Participation.unread_count(user)
) )
end end

View File

@ -0,0 +1,38 @@
defmodule Pleroma.Repo.Migrations.RemoveUnreadConversationCountFromUser do
use Ecto.Migration
import Ecto.Query
alias Pleroma.Repo
def up do
alter table(:users) do
remove_if_exists(:unread_conversation_count, :integer)
end
end
def down do
alter table(:users) do
add_if_not_exists(:unread_conversation_count, :integer, default: 0)
end
flush()
recalc_unread_conversation_count()
end
defp recalc_unread_conversation_count do
participations_subquery =
from(
p in "conversation_participations",
where: p.read == false,
group_by: p.user_id,
select: %{user_id: p.user_id, unread_conversation_count: count(p.id)}
)
from(
u in "users",
join: p in subquery(participations_subquery),
on: p.user_id == u.id,
update: [set: [unread_conversation_count: p.unread_conversation_count]]
)
|> Repo.update_all([])
end
end

View File

@ -0,0 +1,12 @@
defmodule Pleroma.Repo.Migrations.AddUnreadIndexToConversationParticipation do
use Ecto.Migration
def change do
create(
index(:conversation_participations, [:user_id],
where: "read = false",
name: "unread_conversation_participation_count_index"
)
)
end
end

View File

@ -37,9 +37,8 @@ test "for a new conversation or a reply, it doesn't mark the author's participat
[%{read: true}] = Participation.for_user(user) [%{read: true}] = Participation.for_user(user)
[%{read: false} = participation] = Participation.for_user(other_user) [%{read: false} = participation] = Participation.for_user(other_user)
assert Participation.unread_count(user) == 0
assert User.get_cached_by_id(user.id).unread_conversation_count == 0 assert Participation.unread_count(other_user) == 1
assert User.get_cached_by_id(other_user.id).unread_conversation_count == 1
{:ok, _} = {:ok, _} =
CommonAPI.post(other_user, %{ CommonAPI.post(other_user, %{
@ -54,8 +53,8 @@ test "for a new conversation or a reply, it doesn't mark the author's participat
[%{read: false}] = Participation.for_user(user) [%{read: false}] = Participation.for_user(user)
[%{read: true}] = Participation.for_user(other_user) [%{read: true}] = Participation.for_user(other_user)
assert User.get_cached_by_id(user.id).unread_conversation_count == 1 assert Participation.unread_count(user) == 1
assert User.get_cached_by_id(other_user.id).unread_conversation_count == 0 assert Participation.unread_count(other_user) == 0
end end
test "for a new conversation, it sets the recipents of the participation" do test "for a new conversation, it sets the recipents of the participation" do
@ -264,7 +263,7 @@ test "when the user blocks a recipient, the existing conversations with them are
assert [%{read: false}, %{read: false}, %{read: false}, %{read: false}] = assert [%{read: false}, %{read: false}, %{read: false}, %{read: false}] =
Participation.for_user(blocker) Participation.for_user(blocker)
assert User.get_cached_by_id(blocker.id).unread_conversation_count == 4 assert Participation.unread_count(blocker) == 4
{:ok, _user_relationship} = User.block(blocker, blocked) {:ok, _user_relationship} = User.block(blocker, blocked)
@ -272,15 +271,15 @@ test "when the user blocks a recipient, the existing conversations with them are
assert [%{read: true}, %{read: true}, %{read: true}, %{read: false}] = assert [%{read: true}, %{read: true}, %{read: true}, %{read: false}] =
Participation.for_user(blocker) Participation.for_user(blocker)
assert User.get_cached_by_id(blocker.id).unread_conversation_count == 1 assert Participation.unread_count(blocker) == 1
# The conversation is not marked as read for the blocked user # The conversation is not marked as read for the blocked user
assert [_, _, %{read: false}] = Participation.for_user(blocked) assert [_, _, %{read: false}] = Participation.for_user(blocked)
assert User.get_cached_by_id(blocked.id).unread_conversation_count == 1 assert Participation.unread_count(blocker) == 1
# The conversation is not marked as read for the third user # The conversation is not marked as read for the third user
assert [%{read: false}, _, _] = Participation.for_user(third_user) assert [%{read: false}, _, _] = Participation.for_user(third_user)
assert User.get_cached_by_id(third_user.id).unread_conversation_count == 1 assert Participation.unread_count(third_user) == 1
end end
test "the new conversation with the blocked user is not marked as unread " do test "the new conversation with the blocked user is not marked as unread " do
@ -298,7 +297,7 @@ test "the new conversation with the blocked user is not marked as unread " do
}) })
assert [%{read: true}] = Participation.for_user(blocker) assert [%{read: true}] = Participation.for_user(blocker)
assert User.get_cached_by_id(blocker.id).unread_conversation_count == 0 assert Participation.unread_count(blocker) == 0
# When the blocked user is a recipient # When the blocked user is a recipient
{:ok, _direct2} = {:ok, _direct2} =
@ -308,10 +307,10 @@ test "the new conversation with the blocked user is not marked as unread " do
}) })
assert [%{read: true}, %{read: true}] = Participation.for_user(blocker) assert [%{read: true}, %{read: true}] = Participation.for_user(blocker)
assert User.get_cached_by_id(blocker.id).unread_conversation_count == 0 assert Participation.unread_count(blocker) == 0
assert [%{read: false}, _] = Participation.for_user(blocked) assert [%{read: false}, _] = Participation.for_user(blocked)
assert User.get_cached_by_id(blocked.id).unread_conversation_count == 1 assert Participation.unread_count(blocked) == 1
end end
test "the conversation with the blocked user is not marked as unread on a reply" do test "the conversation with the blocked user is not marked as unread on a reply" do
@ -327,8 +326,8 @@ test "the conversation with the blocked user is not marked as unread on a reply"
{:ok, _user_relationship} = User.block(blocker, blocked) {:ok, _user_relationship} = User.block(blocker, blocked)
assert [%{read: true}] = Participation.for_user(blocker) assert [%{read: true}] = Participation.for_user(blocker)
assert User.get_cached_by_id(blocker.id).unread_conversation_count == 0
assert Participation.unread_count(blocker) == 0
assert [blocked_participation] = Participation.for_user(blocked) assert [blocked_participation] = Participation.for_user(blocked)
# When it's a reply from the blocked user # When it's a reply from the blocked user
@ -340,8 +339,8 @@ test "the conversation with the blocked user is not marked as unread on a reply"
}) })
assert [%{read: true}] = Participation.for_user(blocker) assert [%{read: true}] = Participation.for_user(blocker)
assert User.get_cached_by_id(blocker.id).unread_conversation_count == 0
assert Participation.unread_count(blocker) == 0
assert [third_user_participation] = Participation.for_user(third_user) assert [third_user_participation] = Participation.for_user(third_user)
# When it's a reply from the third user # When it's a reply from the third user
@ -353,11 +352,12 @@ test "the conversation with the blocked user is not marked as unread on a reply"
}) })
assert [%{read: true}] = Participation.for_user(blocker) assert [%{read: true}] = Participation.for_user(blocker)
assert User.get_cached_by_id(blocker.id).unread_conversation_count == 0 assert Participation.unread_count(blocker) == 0
# Marked as unread for the blocked user # Marked as unread for the blocked user
assert [%{read: false}] = Participation.for_user(blocked) assert [%{read: false}] = Participation.for_user(blocked)
assert User.get_cached_by_id(blocked.id).unread_conversation_count == 1
assert Participation.unread_count(blocked) == 1
end end
end end
end end

View File

@ -5,6 +5,7 @@
defmodule Pleroma.Web.MastodonAPI.ConversationControllerTest do defmodule Pleroma.Web.MastodonAPI.ConversationControllerTest do
use Pleroma.Web.ConnCase use Pleroma.Web.ConnCase
alias Pleroma.Conversation.Participation
alias Pleroma.User alias Pleroma.User
alias Pleroma.Web.CommonAPI alias Pleroma.Web.CommonAPI
@ -28,10 +29,10 @@ test "returns correct conversations", %{
user_three: user_three, user_three: user_three,
conn: conn conn: conn
} do } do
assert User.get_cached_by_id(user_two.id).unread_conversation_count == 0 assert Participation.unread_count(user_two) == 0
{:ok, direct} = create_direct_message(user_one, [user_two, user_three]) {:ok, direct} = create_direct_message(user_one, [user_two, user_three])
assert User.get_cached_by_id(user_two.id).unread_conversation_count == 1 assert Participation.unread_count(user_two) == 1
{:ok, _follower_only} = {:ok, _follower_only} =
CommonAPI.post(user_one, %{ CommonAPI.post(user_one, %{
@ -59,7 +60,7 @@ test "returns correct conversations", %{
assert is_binary(res_id) assert is_binary(res_id)
assert unread == false assert unread == false
assert res_last_status["id"] == direct.id assert res_last_status["id"] == direct.id
assert User.get_cached_by_id(user_one.id).unread_conversation_count == 0 assert Participation.unread_count(user_one) == 0
end end
test "observes limit params", %{ test "observes limit params", %{
@ -134,8 +135,8 @@ test "the user marks a conversation as read", %{user: user_one, conn: conn} do
user_two = insert(:user) user_two = insert(:user)
{:ok, direct} = create_direct_message(user_one, [user_two]) {:ok, direct} = create_direct_message(user_one, [user_two])
assert User.get_cached_by_id(user_one.id).unread_conversation_count == 0 assert Participation.unread_count(user_one) == 0
assert User.get_cached_by_id(user_two.id).unread_conversation_count == 1 assert Participation.unread_count(user_two) == 1
user_two_conn = user_two_conn =
build_conn() build_conn()
@ -155,8 +156,8 @@ test "the user marks a conversation as read", %{user: user_one, conn: conn} do
|> post("/api/v1/conversations/#{direct_conversation_id}/read") |> post("/api/v1/conversations/#{direct_conversation_id}/read")
|> json_response_and_validate_schema(200) |> json_response_and_validate_schema(200)
assert User.get_cached_by_id(user_one.id).unread_conversation_count == 0 assert Participation.unread_count(user_one) == 0
assert User.get_cached_by_id(user_two.id).unread_conversation_count == 0 assert Participation.unread_count(user_two) == 0
# The conversation is marked as unread on reply # The conversation is marked as unread on reply
{:ok, _} = {:ok, _} =
@ -171,8 +172,8 @@ test "the user marks a conversation as read", %{user: user_one, conn: conn} do
|> get("/api/v1/conversations") |> get("/api/v1/conversations")
|> json_response_and_validate_schema(200) |> json_response_and_validate_schema(200)
assert User.get_cached_by_id(user_one.id).unread_conversation_count == 1 assert Participation.unread_count(user_one) == 1
assert User.get_cached_by_id(user_two.id).unread_conversation_count == 0 assert Participation.unread_count(user_two) == 0
# A reply doesn't increment the user's unread_conversation_count if the conversation is unread # A reply doesn't increment the user's unread_conversation_count if the conversation is unread
{:ok, _} = {:ok, _} =
@ -182,8 +183,8 @@ test "the user marks a conversation as read", %{user: user_one, conn: conn} do
in_reply_to_status_id: direct.id in_reply_to_status_id: direct.id
}) })
assert User.get_cached_by_id(user_one.id).unread_conversation_count == 1 assert Participation.unread_count(user_one) == 1
assert User.get_cached_by_id(user_two.id).unread_conversation_count == 0 assert Participation.unread_count(user_two) == 0
end end
test "(vanilla) Mastodon frontend behaviour", %{user: user_one, conn: conn} do test "(vanilla) Mastodon frontend behaviour", %{user: user_one, conn: conn} do

View File

@ -121,7 +121,7 @@ test "POST /api/v1/pleroma/conversations/read" do
[participation2, participation1] = Participation.for_user(other_user) [participation2, participation1] = Participation.for_user(other_user)
assert Participation.get(participation2.id).read == false assert Participation.get(participation2.id).read == false
assert Participation.get(participation1.id).read == false assert Participation.get(participation1.id).read == false
assert User.get_cached_by_id(other_user.id).unread_conversation_count == 2 assert Participation.unread_count(other_user) == 2
[%{"unread" => false}, %{"unread" => false}] = [%{"unread" => false}, %{"unread" => false}] =
conn conn
@ -131,6 +131,6 @@ test "POST /api/v1/pleroma/conversations/read" do
[participation2, participation1] = Participation.for_user(other_user) [participation2, participation1] = Participation.for_user(other_user)
assert Participation.get(participation2.id).read == true assert Participation.get(participation2.id).read == true
assert Participation.get(participation1.id).read == true assert Participation.get(participation1.id).read == true
assert User.get_cached_by_id(other_user.id).unread_conversation_count == 0 assert Participation.unread_count(other_user) == 0
end end
end end