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

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