From e07e9cb75e6605218acea1ef41772ca29124bd0d Mon Sep 17 00:00:00 2001 From: kaniini Date: Sat, 5 Oct 2019 10:00:05 +0000 Subject: [PATCH] Revert "Merge branch 'user-info-unread-direct-conversation' into 'develop'" This reverts merge request !1737 --- CHANGELOG.md | 1 - docs/API/differences_in_mastoapi_responses.md | 1 - lib/pleroma/conversation.ex | 2 - lib/pleroma/conversation/participation.ex | 17 --- lib/pleroma/user.ex | 56 --------- lib/pleroma/user/info.ex | 1 - .../web/mastodon_api/views/account_view.ex | 11 -- test/conversation/participation_test.exs | 4 - .../conversation_controller_test.exs | 109 +++--------------- .../mastodon_api/views/account_view_test.exs | 21 ---- .../pleroma_api_controller_test.exs | 2 - 11 files changed, 14 insertions(+), 211 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db505591b..b42b13018 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Pleroma API: `GET /api/v1/pleroma/accounts/:id/scrobbles` to get a list of recently scrobbled items - Pleroma API: `POST /api/v1/pleroma/scrobble` to scrobble a media item - Mastodon API: Add `upload_limit`, `avatar_upload_limit`, `background_upload_limit`, and `banner_upload_limit` to `/api/v1/instance` -- Mastodon API: Add `pleroma.unread_conversation_count` to the Account entity ### Changed - **Breaking:** Elixir >=1.8 is now required (was >= 1.7) diff --git a/docs/API/differences_in_mastoapi_responses.md b/docs/API/differences_in_mastoapi_responses.md index 21b297529..d007a69c3 100644 --- a/docs/API/differences_in_mastoapi_responses.md +++ b/docs/API/differences_in_mastoapi_responses.md @@ -56,7 +56,6 @@ Has these additional fields under the `pleroma` object: - `settings_store`: A generic map of settings for frontends. Opaque to the backend. Only returned in `verify_credentials` and `update_credentials` - `chat_token`: The token needed for Pleroma chat. Only returned in `verify_credentials` - `deactivated`: boolean, true when the user is deactivated -- `unread_conversation_count`: The count of unread conversations. Only returned to the account owner. ### Source diff --git a/lib/pleroma/conversation.ex b/lib/pleroma/conversation.ex index 098016af2..be5821ad7 100644 --- a/lib/pleroma/conversation.ex +++ b/lib/pleroma/conversation.ex @@ -67,8 +67,6 @@ def create_or_bump_for(activity, opts \\ []) do participations = Enum.map(users, fn user -> - User.increment_unread_conversation_count(conversation, user) - {:ok, participation} = Participation.create_for_user_and_conversation(user, conversation, opts) diff --git a/lib/pleroma/conversation/participation.ex b/lib/pleroma/conversation/participation.ex index ab81f3217..e946f6de2 100644 --- a/lib/pleroma/conversation/participation.ex +++ b/lib/pleroma/conversation/participation.ex @@ -52,15 +52,6 @@ def mark_as_read(participation) do participation |> read_cng(%{read: true}) |> Repo.update() - |> case do - {:ok, participation} -> - participation = Repo.preload(participation, :user) - User.set_unread_conversation_count(participation.user) - {:ok, participation} - - error -> - error - end end def mark_as_unread(participation) do @@ -144,12 +135,4 @@ def set_recipients(participation, user_ids) do {:ok, Repo.preload(participation, :recipients, force: true)} end - - def unread_conversation_count_for_user(user) do - from(p in __MODULE__, - where: p.user_id == ^user.id, - where: not p.read, - select: %{count: count(p.id)} - ) - end end diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 494a67f22..c2f8fa0d7 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -11,7 +11,6 @@ defmodule Pleroma.User do alias Comeonin.Pbkdf2 alias Ecto.Multi alias Pleroma.Activity - alias Pleroma.Conversation.Participation alias Pleroma.Delivery alias Pleroma.Keys alias Pleroma.Notification @@ -843,61 +842,6 @@ def maybe_update_following_count(%User{local: false} = user) do def maybe_update_following_count(user), do: user - def set_unread_conversation_count(%User{local: true} = user) do - unread_query = Participation.unread_conversation_count_for_user(user) - - User - |> where([u], u.id == ^user.id) - |> join(:inner, [u], p in subquery(unread_query)) - |> update([u, p], - set: [ - info: - fragment( - "jsonb_set(?, '{unread_conversation_count}', ?::varchar::jsonb, true)", - u.info, - p.count - ) - ] - ) - |> select([u], u) - |> Repo.update_all([]) - |> case do - {1, [%{info: %User.Info{}} = user]} -> set_cache(user) - _ -> {:error, user} - end - end - - def set_unread_conversation_count(_), do: :noop - - 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], - set: [ - info: - fragment( - "jsonb_set(?, '{unread_conversation_count}', ((?->>'unread_conversation_count')::int + 1)::varchar::jsonb, true)", - u.info, - u.info - ) - ] - ) - |> where([u], u.id == ^user.id) - |> where([u, p], p.count == 0) - |> select([u], u) - |> Repo.update_all([]) - |> case do - {1, [%{info: %User.Info{}} = user]} -> set_cache(user) - _ -> {:error, user} - end - end - - def increment_unread_conversation_count(_, _), do: :noop - def remove_duplicated_following(%User{following: following} = user) do uniq_following = Enum.uniq(following) diff --git a/lib/pleroma/user/info.ex b/lib/pleroma/user/info.ex index 4b5b43d7f..ebd4ddebf 100644 --- a/lib/pleroma/user/info.ex +++ b/lib/pleroma/user/info.ex @@ -47,7 +47,6 @@ defmodule Pleroma.User.Info do field(:hide_followers, :boolean, default: false) field(:hide_follows, :boolean, default: false) field(:hide_favorites, :boolean, default: true) - field(:unread_conversation_count, :integer, default: 0) field(:pinned_activities, {:array, :string}, default: []) field(:email_notifications, :map, default: %{"digest" => false}) field(:mascot, :map, default: nil) diff --git a/lib/pleroma/web/mastodon_api/views/account_view.ex b/lib/pleroma/web/mastodon_api/views/account_view.ex index 2d4976891..99169ef95 100644 --- a/lib/pleroma/web/mastodon_api/views/account_view.ex +++ b/lib/pleroma/web/mastodon_api/views/account_view.ex @@ -167,7 +167,6 @@ defp do_render("show.json", %{user: user} = opts) do |> maybe_put_chat_token(user, opts[:for], opts) |> maybe_put_activation_status(user, opts[:for]) |> maybe_put_follow_requests_count(user, opts[:for]) - |> maybe_put_unread_conversation_count(user, opts[:for]) end defp username_from_nickname(string) when is_binary(string) do @@ -249,16 +248,6 @@ defp maybe_put_activation_status(data, user, %User{info: %{is_admin: true}}) do defp maybe_put_activation_status(data, _, _), do: data - defp maybe_put_unread_conversation_count(data, %User{id: user_id} = user, %User{id: user_id}) do - data - |> Kernel.put_in( - [:pleroma, :unread_conversation_count], - user.info.unread_conversation_count - ) - end - - defp maybe_put_unread_conversation_count(data, _, _), do: data - defp image_url(%{"url" => [%{"href" => href} | _]}), do: href defp image_url(_), do: nil end diff --git a/test/conversation/participation_test.exs b/test/conversation/participation_test.exs index f430bdf75..a27167d42 100644 --- a/test/conversation/participation_test.exs +++ b/test/conversation/participation_test.exs @@ -6,7 +6,6 @@ defmodule Pleroma.Conversation.ParticipationTest do use Pleroma.DataCase import Pleroma.Factory alias Pleroma.Conversation.Participation - alias Pleroma.User alias Pleroma.Web.CommonAPI test "getting a participation will also preload things" do @@ -31,8 +30,6 @@ test "for a new conversation, it sets the recipents of the participation" do {:ok, activity} = CommonAPI.post(user, %{"status" => "Hey @#{other_user.nickname}.", "visibility" => "direct"}) - user = User.get_cached_by_id(user.id) - other_user = User.get_cached_by_id(user.id) [participation] = Participation.for_user(user) participation = Pleroma.Repo.preload(participation, :recipients) @@ -158,7 +155,6 @@ test "it sets recipients, always keeping the owner of the participation even whe [participation] = Participation.for_user_with_last_activity_id(user) participation = Repo.preload(participation, :recipients) - user = User.get_cached_by_id(user.id) assert participation.recipients |> length() == 1 assert user in participation.recipients diff --git a/test/web/mastodon_api/controllers/conversation_controller_test.exs b/test/web/mastodon_api/controllers/conversation_controller_test.exs index a308a7620..7117fc76a 100644 --- a/test/web/mastodon_api/controllers/conversation_controller_test.exs +++ b/test/web/mastodon_api/controllers/conversation_controller_test.exs @@ -10,23 +10,19 @@ defmodule Pleroma.Web.MastodonAPI.ConversationControllerTest do import Pleroma.Factory - test "returns a list of conversations", %{conn: conn} do + test "Conversations", %{conn: conn} do user_one = insert(:user) user_two = insert(:user) user_three = insert(:user) {:ok, user_two} = User.follow(user_two, user_one) - assert User.get_cached_by_id(user_two.id).info.unread_conversation_count == 0 - {:ok, direct} = CommonAPI.post(user_one, %{ "status" => "Hi @#{user_two.nickname}, @#{user_three.nickname}!", "visibility" => "direct" }) - assert User.get_cached_by_id(user_two.id).info.unread_conversation_count == 1 - {:ok, _follower_only} = CommonAPI.post(user_one, %{ "status" => "Hi @#{user_two.nickname}!", @@ -56,100 +52,23 @@ test "returns a list of conversations", %{conn: conn} do assert is_binary(res_id) assert unread == true assert res_last_status["id"] == direct.id - assert User.get_cached_by_id(user_one.id).info.unread_conversation_count == 1 - end - - test "updates the last_status on reply", %{conn: conn} do - user_one = insert(:user) - user_two = insert(:user) - - {:ok, direct} = - CommonAPI.post(user_one, %{ - "status" => "Hi @#{user_two.nickname}", - "visibility" => "direct" - }) - - {:ok, direct_reply} = - CommonAPI.post(user_two, %{ - "status" => "reply", - "visibility" => "direct", - "in_reply_to_status_id" => direct.id - }) - - [%{"last_status" => res_last_status}] = - conn - |> assign(:user, user_one) - |> get("/api/v1/conversations") - |> json_response(200) - - assert res_last_status["id"] == direct_reply.id - end - - test "the user marks a conversation as read", %{conn: conn} do - user_one = insert(:user) - user_two = insert(:user) - - {:ok, direct} = - CommonAPI.post(user_one, %{ - "status" => "Hi @#{user_two.nickname}", - "visibility" => "direct" - }) - - [%{"id" => direct_conversation_id, "unread" => true}] = - conn - |> assign(:user, user_one) - |> get("/api/v1/conversations") - |> json_response(200) - - %{"unread" => false} = - conn - |> assign(:user, user_one) - |> post("/api/v1/conversations/#{direct_conversation_id}/read") - |> json_response(200) - - assert User.get_cached_by_id(user_one.id).info.unread_conversation_count == 0 - - # The conversation is marked as unread on reply - {:ok, _} = - CommonAPI.post(user_two, %{ - "status" => "reply", - "visibility" => "direct", - "in_reply_to_status_id" => direct.id - }) - - [%{"unread" => true}] = - conn - |> assign(:user, user_one) - |> get("/api/v1/conversations") - |> json_response(200) - - assert User.get_cached_by_id(user_one.id).info.unread_conversation_count == 1 - - # A reply doesn't increment the user's unread_conversation_count if the conversation is unread - {:ok, _} = - CommonAPI.post(user_two, %{ - "status" => "reply", - "visibility" => "direct", - "in_reply_to_status_id" => direct.id - }) - - assert User.get_cached_by_id(user_one.id).info.unread_conversation_count == 1 - end - - test "(vanilla) Mastodon frontend behaviour", %{conn: conn} do - user_one = insert(:user) - user_two = insert(:user) - - {:ok, direct} = - CommonAPI.post(user_one, %{ - "status" => "Hi @#{user_two.nickname}!", - "visibility" => "direct" - }) + # Apparently undocumented API endpoint res_conn = conn |> assign(:user, user_one) - |> get("/api/v1/statuses/#{direct.id}/context") + |> post("/api/v1/conversations/#{res_id}/read") + + assert response = json_response(res_conn, 200) + assert length(response["accounts"]) == 2 + assert response["last_status"]["id"] == direct.id + assert response["unread"] == false + + # (vanilla) Mastodon frontend behaviour + res_conn = + conn + |> assign(:user, user_one) + |> get("/api/v1/statuses/#{res_last_status["id"]}/context") assert %{"ancestors" => [], "descendants" => []} == json_response(res_conn, 200) end diff --git a/test/web/mastodon_api/views/account_view_test.exs b/test/web/mastodon_api/views/account_view_test.exs index b7a4938a6..62b2ab7e3 100644 --- a/test/web/mastodon_api/views/account_view_test.exs +++ b/test/web/mastodon_api/views/account_view_test.exs @@ -418,27 +418,6 @@ test "shows actual follower/following count to the account owner" do following_count: 1 } = AccountView.render("show.json", %{user: user, for: user}) end - - test "shows unread_conversation_count only to the account owner" do - user = insert(:user) - other_user = insert(:user) - - {:ok, _activity} = - CommonAPI.post(user, %{ - "status" => "Hey @#{other_user.nickname}.", - "visibility" => "direct" - }) - - user = User.get_cached_by_ap_id(user.ap_id) - - assert AccountView.render("show.json", %{user: user, for: other_user})[:pleroma][ - :unread_conversation_count - ] == nil - - assert AccountView.render("show.json", %{user: user, for: user})[:pleroma][ - :unread_conversation_count - ] == 1 - end end describe "follow requests counter" do diff --git a/test/web/pleroma_api/controllers/pleroma_api_controller_test.exs b/test/web/pleroma_api/controllers/pleroma_api_controller_test.exs index 8a6528cbb..7eaeda4a0 100644 --- a/test/web/pleroma_api/controllers/pleroma_api_controller_test.exs +++ b/test/web/pleroma_api/controllers/pleroma_api_controller_test.exs @@ -8,7 +8,6 @@ defmodule Pleroma.Web.PleromaAPI.PleromaAPIControllerTest do alias Pleroma.Conversation.Participation alias Pleroma.Notification alias Pleroma.Repo - alias Pleroma.User alias Pleroma.Web.CommonAPI import Pleroma.Factory @@ -74,7 +73,6 @@ test "PATCH /api/v1/pleroma/conversations/:id", %{conn: conn} do participation = Repo.preload(participation, :recipients) - user = User.get_cached_by_id(user.id) assert [user] == participation.recipients assert other_user not in participation.recipients