From 9514458faaa66b97667be2d7ffe227e8c626ad47 Mon Sep 17 00:00:00 2001 From: Alexander Strizhakov Date: Sat, 20 Feb 2021 16:36:58 +0300 Subject: [PATCH] resolving comments - added patch edpoint for user tags in api v2 namespace - returned old put update method - fixed duplication in users search with tags - suggested renamings and code changes --- lib/pleroma/tag.ex | 2 +- lib/pleroma/user.ex | 4 +- lib/pleroma/user/query.ex | 11 +++-- .../web/admin_api/controllers/tag_controller.ex | 8 ++-- .../web/api_spec/operations/admin/tag_operation.ex | 13 +++++- lib/pleroma/web/router.ex | 7 +++- .../migrations/20201014064744_create_user_tag.exs | 1 - .../admin_api/controllers/tag_controller_test.exs | 48 +++++++++++++++++++++- test/pleroma/web/admin_api/search_test.exs | 12 ++++++ 9 files changed, 93 insertions(+), 13 deletions(-) diff --git a/lib/pleroma/tag.ex b/lib/pleroma/tag.ex index dbbb99068..40a41e519 100644 --- a/lib/pleroma/tag.ex +++ b/lib/pleroma/tag.ex @@ -37,7 +37,7 @@ defmodule Pleroma.Tag do @spec upsert_tags(list(String.t())) :: {integer(), nil | [term()]} def upsert_tags(names) do - date = NaiveDateTime.utc_now() + date = NaiveDateTime.truncate(NaiveDateTime.utc_now(), :second) tags = names diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 0ef5f4661..a6b3f27d0 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -2027,7 +2027,7 @@ defmodule Pleroma.User do def tag(%User{} = user, tags) do tag_names = Pleroma.Tag.normalize_tags(tags) Pleroma.Tag.upsert_tags(tag_names) - update_user_tags(user, tag_names) + append_user_tags(user, tag_names) end @spec untag([String.t()] | String.t() | t(), [String.t() | String.t()]) :: {:ok, [t()]} | t() @@ -2054,7 +2054,7 @@ defmodule Pleroma.User do preload_tags_and_set_cache(user) end - defp update_user_tags(%User{} = user, new_tags) do + defp append_user_tags(%User{} = user, new_tags) do {:ok, user_id} = FlakeId.Ecto.Type.dump(user.id) tags = diff --git a/lib/pleroma/user/query.ex b/lib/pleroma/user/query.ex index 36e03466a..2c29aef3d 100644 --- a/lib/pleroma/user/query.ex +++ b/lib/pleroma/user/query.ex @@ -109,9 +109,14 @@ defmodule Pleroma.User.Query do end defp compose_query({:tags, tags}, query) when is_list(tags) and length(tags) > 0 do - query - |> join(:inner, [u], t in assoc(u, :tags), as: :tags) - |> where([tags: t], t.name in ^tags) + users_with_tags_query = + from(u in User, + join: t in assoc(u, :tags), + where: t.name in ^tags, + distinct: u + ) + + from(u in query, join: ut in subquery(users_with_tags_query), on: ut.id == u.id) end defp compose_query({:is_admin, bool}, query) do diff --git a/lib/pleroma/web/admin_api/controllers/tag_controller.ex b/lib/pleroma/web/admin_api/controllers/tag_controller.ex index 7fdee98d6..a025860d7 100644 --- a/lib/pleroma/web/admin_api/controllers/tag_controller.ex +++ b/lib/pleroma/web/admin_api/controllers/tag_controller.ex @@ -15,12 +15,12 @@ defmodule Pleroma.Web.AdminAPI.TagController do plug( OAuthScopesPlug, - %{scopes: ["write:accounts"], admin: true} when action in [:update, :delete] + %{scopes: ["admin:write:accounts"]} when action in [:update, :append, :delete] ) plug( OAuthScopesPlug, - %{scopes: ["read:accounts"], admin: true} when action in [:index] + %{scopes: ["admin:read:accounts"]} when action == :index ) plug(ApiSpec.CastAndValidate) @@ -35,7 +35,9 @@ defmodule Pleroma.Web.AdminAPI.TagController do json(conn, tags) end - def update( + def update(conn, params), do: append(conn, params) + + def append( %{assigns: %{user: admin}, body_params: %{nicknames: nicknames, tags: tags}} = conn, _ ) do diff --git a/lib/pleroma/web/api_spec/operations/admin/tag_operation.ex b/lib/pleroma/web/api_spec/operations/admin/tag_operation.ex index 1716e5b7d..1af7d9520 100644 --- a/lib/pleroma/web/api_spec/operations/admin/tag_operation.ex +++ b/lib/pleroma/web/api_spec/operations/admin/tag_operation.ex @@ -32,10 +32,21 @@ defmodule Pleroma.Web.ApiSpec.Admin.TagOperation do end def update_operation do + %{ + append_op() + | description: + "Deprecated. Using [/api/v2/pleroma/admin/users/tags](#operation/AdminAPI.TagController.append) instead is recommended.", + operationId: "AdminAPI.TagController.update" + } + end + + def append_operation, do: append_op() + + defp append_op do %Operation{ tags: ["Admin", "Tags"], summary: "Adds tags to users.", - operationId: "AdminAPI.TagController.update", + operationId: "AdminAPI.TagController.append", parameters: admin_api_params(), requestBody: request_body( diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index bf5bf8e19..d5ad15007 100644 --- a/lib/pleroma/web/router.ex +++ b/lib/pleroma/web/router.ex @@ -155,13 +155,18 @@ defmodule Pleroma.Web.Router do post("/uploader_callback/:upload_path", UploaderController, :callback) end + scope "/api/v2/pleroma/admin", Pleroma.Web.AdminAPI do + pipe_through(:admin_api) + patch("/users/tags", TagController, :append) + end + scope "/api/v1/pleroma/admin", Pleroma.Web.AdminAPI do pipe_through(:admin_api) put("/users/disable_mfa", AdminAPIController, :disable_mfa) get("/user_tags", TagController, :index) - patch("/users/tags", TagController, :update) + put("/users/tags", TagController, :update) delete("/users/tags", TagController, :delete) get("/users/:nickname/permission_group", AdminAPIController, :right_get) diff --git a/priv/repo/migrations/20201014064744_create_user_tag.exs b/priv/repo/migrations/20201014064744_create_user_tag.exs index af8fa0952..201052587 100644 --- a/priv/repo/migrations/20201014064744_create_user_tag.exs +++ b/priv/repo/migrations/20201014064744_create_user_tag.exs @@ -8,7 +8,6 @@ defmodule Pleroma.Repo.Migrations.CreateUserTag do end create_if_not_exists(index(:users_tags, [:tag_id])) - create_if_not_exists(index(:users_tags, [:user_id])) create_if_not_exists(unique_index(:users_tags, [:user_id, :tag_id])) end end diff --git a/test/pleroma/web/admin_api/controllers/tag_controller_test.exs b/test/pleroma/web/admin_api/controllers/tag_controller_test.exs index 34b532a39..2855c2d19 100644 --- a/test/pleroma/web/admin_api/controllers/tag_controller_test.exs +++ b/test/pleroma/web/admin_api/controllers/tag_controller_test.exs @@ -58,7 +58,53 @@ defmodule Pleroma.Web.AdminAPI.TagControllerTest do assert conn |> put_req_header("content-type", "application/json") - |> patch("/api/pleroma/admin/users/tags", %{ + |> put("/api/pleroma/admin/users/tags", %{ + nicknames: [user1.nickname, user2.nickname], + tags: ["foo", "bar"] + }) + |> json_response_and_validate_schema(204) + + %{user1: user1, user2: user2, user3: user3} + end + + test "it appends specified tags to users with specified nicknames", %{ + admin: admin, + user1: user1, + user2: user2 + } do + {:ok, tags} = Repo.get_assoc(User.get_cached_by_id(user1.id), :tags) + assert Enum.map(tags, & &1.name) == ["x", "foo", "bar"] + {:ok, tags} = Repo.get_assoc(User.get_cached_by_id(user2.id), :tags) + assert Enum.map(tags, & &1.name) == ["y", "foo", "bar"] + + log_entry = Repo.one(ModerationLog) + + users = + [user1.nickname, user2.nickname] + |> Enum.map(&"@#{&1}") + |> Enum.join(", ") + + tags = ["foo", "bar"] |> Enum.join(", ") + + assert ModerationLog.get_log_entry_message(log_entry) == + "@#{admin.nickname} added tags: #{tags} to users: #{users}" + end + + test "it does not modify tags of not specified users", %{user3: user3} do + {:ok, tags} = Repo.get_assoc(User.get_cached_by_id(user3.id), :tags) + assert Enum.map(tags, & &1.name) == ["unchanged"] + end + end + + describe "PATCH /api/v2/pleroma/admin/users/tags" do + setup %{conn: conn} do + user1 = insert(:user, %{tags: [build(:tag, name: "x")]}) + user2 = insert(:user, %{tags: [build(:tag, name: "y")]}) + user3 = insert(:user, %{tags: [build(:tag, name: "unchanged")]}) + + assert conn + |> put_req_header("content-type", "application/json") + |> patch("/api/v2/pleroma/admin/users/tags", %{ nicknames: [user1.nickname, user2.nickname], tags: ["foo", "bar"] }) diff --git a/test/pleroma/web/admin_api/search_test.exs b/test/pleroma/web/admin_api/search_test.exs index 40e0007e8..276eb9e5a 100644 --- a/test/pleroma/web/admin_api/search_test.exs +++ b/test/pleroma/web/admin_api/search_test.exs @@ -213,5 +213,17 @@ defmodule Pleroma.Web.AdminAPI.SearchTest do assert total == 2 end + + test "tags deduplication" do + first_tag = insert(:tag, name: "first") + second_tag = insert(:tag, name: "second") + + user1 = insert(:user, tags: [first_tag, second_tag]) + user2 = insert(:user, tags: [first_tag, second_tag]) + + {:ok, users, count} = Search.user(%{tags: ["first", "second"], page_size: 2}) + + assert {Enum.sort(Enum.map(users, & &1.id)), count} == {Enum.sort([user1.id, user2.id]), 2} + end end end