From 2883f75a3a25599c6217460133578cddcd34ebb4 Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Thu, 28 Feb 2019 01:11:56 +0300 Subject: [PATCH 01/14] Add pagination to users admin API --- lib/pleroma/user.ex | 21 ++++++++++++++++--- .../web/admin_api/admin_api_controller.ex | 20 +++++++++++++----- .../web/twitter_api/views/user_view.ex | 9 +++++--- .../admin_api/admin_api_controller_test.exs | 20 +++++++++++------- 4 files changed, 51 insertions(+), 19 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 01d532ab3..80e4ae296 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -772,10 +772,25 @@ def search(query, resolve \\ false, for_user \\ nil) do Enum.uniq_by(fts_results ++ trigram_results, & &1.id) end - def all_except_one(user) do - query = from(u in User, where: u.id != ^user.id) + def all_except_one(user, page, page_size) do + from( + u in User, + where: u.id != ^user.id, + limit: ^page_size, + offset: ^((page - 1) * page_size), + order_by: u.id + ) + |> Repo.all() + end - Repo.all(query) + def count_all_except_one(user) do + query = + from( + u in User, + where: u.id != ^user.id + ) + + Repo.aggregate(query, :count, :id) end defp do_search(subquery, for_user, options \\ []) do diff --git a/lib/pleroma/web/admin_api/admin_api_controller.ex b/lib/pleroma/web/admin_api/admin_api_controller.ex index ef72509fe..d75b7e7e7 100644 --- a/lib/pleroma/web/admin_api/admin_api_controller.ex +++ b/lib/pleroma/web/admin_api/admin_api_controller.ex @@ -3,6 +3,8 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Web.AdminAPI.AdminAPIController do + @users_page_size 50 + use Pleroma.Web, :controller alias Pleroma.User alias Pleroma.Web.ActivityPub.Relay @@ -61,11 +63,19 @@ def untag_users(conn, %{"nicknames" => nicknames, "tags" => tags}) do do: json_response(conn, :no_content, "") end - def list_users(%{assigns: %{user: admin}} = conn, _data) do - users = User.all_except_one(admin) - - conn - |> json(UserView.render("index_for_admin.json", %{users: users})) + def list_users(%{assigns: %{user: admin}} = conn, %{"page" => page_string}) do + with {page, _} <- Integer.parse(page_string), + users <- User.all_except_one(admin, page, @users_page_size), + count <- User.count_all_except_one(admin), + do: + conn + |> json( + UserView.render("index_for_admin.json", %{ + users: users, + count: count, + page_size: @users_page_size + }) + ) end def right_add(conn, %{"permission_group" => permission_group, "nickname" => nickname}) diff --git a/lib/pleroma/web/twitter_api/views/user_view.ex b/lib/pleroma/web/twitter_api/views/user_view.ex index c5034cf36..e8514d3ba 100644 --- a/lib/pleroma/web/twitter_api/views/user_view.ex +++ b/lib/pleroma/web/twitter_api/views/user_view.ex @@ -27,9 +27,12 @@ def render("user.json", %{user: user = %User{}} = assigns) do else: %{} end - def render("index_for_admin.json", %{users: users} = opts) do - users - |> render_many(UserView, "show_for_admin.json", opts) + def render("index_for_admin.json", %{users: users, count: count, page_size: page_size} = opts) do + %{ + users: render_many(users, UserView, "show_for_admin.json", opts), + count: count, + page_size: page_size + } end def render("show_for_admin.json", %{user: user}) do diff --git a/test/web/admin_api/admin_api_controller_test.exs b/test/web/admin_api/admin_api_controller_test.exs index f6ae16844..1b0a2f5be 100644 --- a/test/web/admin_api/admin_api_controller_test.exs +++ b/test/web/admin_api/admin_api_controller_test.exs @@ -338,15 +338,19 @@ test "GET /api/pleroma/admin/users" do conn = build_conn() |> assign(:user, admin) - |> get("/api/pleroma/admin/users") + |> get("/api/pleroma/admin/users?page=1") - assert json_response(conn, 200) == [ - %{ - "deactivated" => user.info.deactivated, - "id" => user.id, - "nickname" => user.nickname - } - ] + assert json_response(conn, 200) == %{ + "count" => 1, + "page_size" => 50, + "users" => [ + %{ + "deactivated" => user.info.deactivated, + "id" => user.id, + "nickname" => user.nickname + } + ] + } end test "PATCH /api/pleroma/admin/users/:nickname/toggle_activation" do From 72b7a0797ec42a021f4f8f50dce859fb0f12bf75 Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Thu, 28 Feb 2019 17:43:09 +0300 Subject: [PATCH 02/14] Use Mastodon API views in Admin API --- .../web/admin_api/admin_api_controller.ex | 6 ++--- .../mastodon_api/views/admin/account_view.ex | 25 +++++++++++++++++++ .../web/twitter_api/views/user_view.ex | 17 ------------- 3 files changed, 28 insertions(+), 20 deletions(-) create mode 100644 lib/pleroma/web/mastodon_api/views/admin/account_view.ex diff --git a/lib/pleroma/web/admin_api/admin_api_controller.ex b/lib/pleroma/web/admin_api/admin_api_controller.ex index d75b7e7e7..d8e3d57e1 100644 --- a/lib/pleroma/web/admin_api/admin_api_controller.ex +++ b/lib/pleroma/web/admin_api/admin_api_controller.ex @@ -8,7 +8,7 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do use Pleroma.Web, :controller alias Pleroma.User alias Pleroma.Web.ActivityPub.Relay - alias Pleroma.Web.TwitterAPI.UserView + alias Pleroma.Web.MastodonAPI.Admin.AccountView import Pleroma.Web.ControllerHelper, only: [json_response: 3] @@ -50,7 +50,7 @@ def user_toggle_activation(conn, %{"nickname" => nickname}) do {:ok, updated_user} = User.deactivate(user, !user.info.deactivated) conn - |> json(UserView.render("show_for_admin.json", %{user: updated_user})) + |> json(AccountView.render("show.json", %{user: updated_user})) end def tag_users(conn, %{"nicknames" => nicknames, "tags" => tags}) do @@ -70,7 +70,7 @@ def list_users(%{assigns: %{user: admin}} = conn, %{"page" => page_string}) do do: conn |> json( - UserView.render("index_for_admin.json", %{ + AccountView.render("index.json", %{ users: users, count: count, page_size: @users_page_size diff --git a/lib/pleroma/web/mastodon_api/views/admin/account_view.ex b/lib/pleroma/web/mastodon_api/views/admin/account_view.ex new file mode 100644 index 000000000..74ca13564 --- /dev/null +++ b/lib/pleroma/web/mastodon_api/views/admin/account_view.ex @@ -0,0 +1,25 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2019 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.MastodonAPI.Admin.AccountView do + use Pleroma.Web, :view + + alias Pleroma.Web.MastodonAPI.Admin.AccountView + + def render("index.json", %{users: users, count: count, page_size: page_size}) do + %{ + users: render_many(users, AccountView, "show.json", as: :user), + count: count, + page_size: page_size + } + end + + def render("show.json", %{user: user}) do + %{ + "id" => user.id, + "nickname" => user.nickname, + "deactivated" => user.info.deactivated + } + end +end diff --git a/lib/pleroma/web/twitter_api/views/user_view.ex b/lib/pleroma/web/twitter_api/views/user_view.ex index e8514d3ba..df7384476 100644 --- a/lib/pleroma/web/twitter_api/views/user_view.ex +++ b/lib/pleroma/web/twitter_api/views/user_view.ex @@ -9,7 +9,6 @@ defmodule Pleroma.Web.TwitterAPI.UserView do alias Pleroma.User alias Pleroma.Web.CommonAPI.Utils alias Pleroma.Web.MediaProxy - alias Pleroma.Web.TwitterAPI.UserView def render("show.json", %{user: user = %User{}} = assigns) do render_one(user, Pleroma.Web.TwitterAPI.UserView, "user.json", assigns) @@ -27,22 +26,6 @@ def render("user.json", %{user: user = %User{}} = assigns) do else: %{} end - def render("index_for_admin.json", %{users: users, count: count, page_size: page_size} = opts) do - %{ - users: render_many(users, UserView, "show_for_admin.json", opts), - count: count, - page_size: page_size - } - end - - def render("show_for_admin.json", %{user: user}) do - %{ - "id" => user.id, - "nickname" => user.nickname, - "deactivated" => user.info.deactivated - } - end - def render("short.json", %{ user: %User{ nickname: nickname, From 70e82a3465ee10004d0ae347934524e779bd778a Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Thu, 28 Feb 2019 17:54:02 +0300 Subject: [PATCH 03/14] Add test for the second page --- .../admin_api/admin_api_controller_test.exs | 54 ++++++++++++------- 1 file changed, 36 insertions(+), 18 deletions(-) diff --git a/test/web/admin_api/admin_api_controller_test.exs b/test/web/admin_api/admin_api_controller_test.exs index 1b0a2f5be..893387ef5 100644 --- a/test/web/admin_api/admin_api_controller_test.exs +++ b/test/web/admin_api/admin_api_controller_test.exs @@ -331,26 +331,44 @@ test "/api/pleroma/admin/password_reset" do assert conn.status == 200 end - test "GET /api/pleroma/admin/users" do - admin = insert(:user, info: %{is_admin: true}) - user = insert(:user) + describe "GET /api/pleroma/admin/users" do + test "renders users array for the first page" do + admin = insert(:user, info: %{is_admin: true}) + user = insert(:user) - conn = - build_conn() - |> assign(:user, admin) - |> get("/api/pleroma/admin/users?page=1") + conn = + build_conn() + |> assign(:user, admin) + |> get("/api/pleroma/admin/users?page=1") - assert json_response(conn, 200) == %{ - "count" => 1, - "page_size" => 50, - "users" => [ - %{ - "deactivated" => user.info.deactivated, - "id" => user.id, - "nickname" => user.nickname - } - ] - } + assert json_response(conn, 200) == %{ + "count" => 1, + "page_size" => 50, + "users" => [ + %{ + "deactivated" => user.info.deactivated, + "id" => user.id, + "nickname" => user.nickname + } + ] + } + end + + test "renders empty array for the second page" do + admin = insert(:user, info: %{is_admin: true}) + user = insert(:user) + + conn = + build_conn() + |> assign(:user, admin) + |> get("/api/pleroma/admin/users?page=2") + + assert json_response(conn, 200) == %{ + "count" => 1, + "page_size" => 50, + "users" => [] + } + end end test "PATCH /api/pleroma/admin/users/:nickname/toggle_activation" do From 46f29b9da1cfdcc2ab14616f999f061fa0c87ddc Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Thu, 28 Feb 2019 19:04:47 +0300 Subject: [PATCH 04/14] Add search users endpoint --- lib/pleroma/user.ex | 8 +++---- .../web/admin_api/admin_api_controller.ex | 13 ++++++++++ lib/pleroma/web/router.ex | 1 + .../admin_api/admin_api_controller_test.exs | 24 ++++++++++++++++++- 4 files changed, 41 insertions(+), 5 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 80e4ae296..52df171c5 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -755,18 +755,18 @@ def get_recipients_from_activity(%Activity{recipients: to}) do Repo.all(query) end - def search(query, resolve \\ false, for_user \\ nil) do + def search(query, resolve \\ false, for_user \\ nil, limit \\ 20) do # Strip the beginning @ off if there is a query query = String.trim_leading(query, "@") if resolve, do: get_or_fetch(query) - fts_results = do_search(fts_search_subquery(query), for_user) + fts_results = do_search(fts_search_subquery(query), for_user, %{limit: limit}) {:ok, trigram_results} = Repo.transaction(fn -> Ecto.Adapters.SQL.query(Repo, "select set_limit(0.25)", []) - do_search(trigram_search_subquery(query), for_user) + do_search(trigram_search_subquery(query), for_user, %{limit: limit}) end) Enum.uniq_by(fts_results ++ trigram_results, & &1.id) @@ -793,7 +793,7 @@ def count_all_except_one(user) do Repo.aggregate(query, :count, :id) end - defp do_search(subquery, for_user, options \\ []) do + defp do_search(subquery, for_user, options) do q = from( s in subquery(subquery), diff --git a/lib/pleroma/web/admin_api/admin_api_controller.ex b/lib/pleroma/web/admin_api/admin_api_controller.ex index d8e3d57e1..37159cd40 100644 --- a/lib/pleroma/web/admin_api/admin_api_controller.ex +++ b/lib/pleroma/web/admin_api/admin_api_controller.ex @@ -78,6 +78,19 @@ def list_users(%{assigns: %{user: admin}} = conn, %{"page" => page_string}) do ) end + def search_users(%{assigns: %{user: admin}} = conn, %{"query" => query}) do + users = User.search(query, true, admin, @users_page_size) + + conn + |> json( + AccountView.render("index.json", %{ + users: users, + count: length(users), + page_size: @users_page_size + }) + ) + end + def right_add(conn, %{"permission_group" => permission_group, "nickname" => nickname}) when permission_group in ["moderator", "admin"] do user = User.get_by_nickname(nickname) diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index 3b1fd46a5..6fcb46878 100644 --- a/lib/pleroma/web/router.ex +++ b/lib/pleroma/web/router.ex @@ -140,6 +140,7 @@ defmodule Pleroma.Web.Router do pipe_through([:admin_api, :oauth_write]) get("/users", AdminAPIController, :list_users) + get("/users/search", AdminAPIController, :search_users) delete("/user", AdminAPIController, :user_delete) patch("/users/:nickname/toggle_activation", AdminAPIController, :user_toggle_activation) post("/user", AdminAPIController, :user_create) diff --git a/test/web/admin_api/admin_api_controller_test.exs b/test/web/admin_api/admin_api_controller_test.exs index 893387ef5..14625af32 100644 --- a/test/web/admin_api/admin_api_controller_test.exs +++ b/test/web/admin_api/admin_api_controller_test.exs @@ -356,7 +356,7 @@ test "renders users array for the first page" do test "renders empty array for the second page" do admin = insert(:user, info: %{is_admin: true}) - user = insert(:user) + insert(:user) conn = build_conn() @@ -387,4 +387,26 @@ test "PATCH /api/pleroma/admin/users/:nickname/toggle_activation" do "nickname" => user.nickname } end + + test "GET /api/pleroma/admin/users/search" do + admin = insert(:user, info: %{is_admin: true}) + user = insert(:user, nickname: "bob") + + conn = + build_conn() + |> assign(:user, admin) + |> get("/api/pleroma/admin/users/search?query=bo") + + assert json_response(conn, 200) == %{ + "count" => 1, + "page_size" => 50, + "users" => [ + %{ + "deactivated" => user.info.deactivated, + "id" => user.id, + "nickname" => user.nickname + } + ] + } + end end From adac7455122d37058bff2e4a805066f2cd24fbf9 Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Fri, 1 Mar 2019 17:34:14 +0300 Subject: [PATCH 05/14] Add docs to /users/search --- docs/Admin-API.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/docs/Admin-API.md b/docs/Admin-API.md index 508981d38..ed19bc875 100644 --- a/docs/Admin-API.md +++ b/docs/Admin-API.md @@ -20,6 +20,26 @@ Authentication is required and the user must be an admin. ] ``` +## `/api/pleroma/admin/users/search?query={query}` + +### Search users + +- Method `GET` +- Params: + - `query` +- Response: + +```JSON +[ + { + "deactivated": bool, + "id": integer, + "nickname": string + }, + ... +] +``` + ## `/api/pleroma/admin/user` ### Remove a user From 5b08b470f69738f4528455a58fefe3a8d4acae02 Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Fri, 1 Mar 2019 20:13:02 +0300 Subject: [PATCH 06/14] Add "local" params to users search --- docs/Admin-API.md | 7 ++- lib/pleroma/user.ex | 39 ++++++++---- .../web/admin_api/admin_api_controller.ex | 10 ++- .../mastodon_api/mastodon_api_controller.ex | 6 +- .../web/twitter_api/twitter_api_controller.ex | 2 +- .../admin_api/admin_api_controller_test.exs | 62 +++++++++++++------ 6 files changed, 87 insertions(+), 39 deletions(-) diff --git a/docs/Admin-API.md b/docs/Admin-API.md index ed19bc875..4403620bf 100644 --- a/docs/Admin-API.md +++ b/docs/Admin-API.md @@ -20,13 +20,14 @@ Authentication is required and the user must be an admin. ] ``` -## `/api/pleroma/admin/users/search?query={query}` +## `/api/pleroma/admin/users/search?query={query}&local={local}` -### Search users +### Search users by name or nickname - Method `GET` - Params: - - `query` + - `query`: **string** search term + - `local`: **bool** whether to return only local users - Response: ```JSON diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 52df171c5..af3ce705d 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -755,18 +755,25 @@ def get_recipients_from_activity(%Activity{recipients: to}) do Repo.all(query) end - def search(query, resolve \\ false, for_user \\ nil, limit \\ 20) do + def search(term, options \\ %{}) do # Strip the beginning @ off if there is a query - query = String.trim_leading(query, "@") + term = String.trim_leading(term, "@") + query = options[:query] || User - if resolve, do: get_or_fetch(query) + if options[:resolve], do: get_or_fetch(term) - fts_results = do_search(fts_search_subquery(query), for_user, %{limit: limit}) + fts_results = + do_search(fts_search_subquery(term, query), options[:for_user], %{ + limit: options[:limit] + }) {:ok, trigram_results} = Repo.transaction(fn -> Ecto.Adapters.SQL.query(Repo, "select set_limit(0.25)", []) - do_search(trigram_search_subquery(query), for_user, %{limit: limit}) + + do_search(trigram_search_subquery(term, query), options[:for_user], %{ + limit: options[:limit] + }) end) Enum.uniq_by(fts_results ++ trigram_results, & &1.id) @@ -809,9 +816,9 @@ defp do_search(subquery, for_user, options) do boost_search_results(results, for_user) end - defp fts_search_subquery(query) do + defp fts_search_subquery(term, query) do processed_query = - query + term |> String.replace(~r/\W+/, " ") |> String.trim() |> String.split() @@ -819,7 +826,7 @@ defp fts_search_subquery(query) do |> Enum.join(" | ") from( - u in User, + u in query, select_merge: %{ search_rank: fragment( @@ -849,19 +856,19 @@ defp fts_search_subquery(query) do ) end - defp trigram_search_subquery(query) do + defp trigram_search_subquery(term, query) do from( - u in User, + u in query, select_merge: %{ search_rank: fragment( "similarity(?, trim(? || ' ' || coalesce(?, '')))", - ^query, + ^term, u.nickname, u.name ) }, - where: fragment("trim(? || ' ' || coalesce(?, '')) % ?", u.nickname, u.name, ^query) + where: fragment("trim(? || ' ' || coalesce(?, '')) % ?", u.nickname, u.name, ^term) ) end @@ -1018,6 +1025,14 @@ def unblock_domain(user, domain) do update_and_set_cache(cng) end + def maybe_local_user_query(local) when local == true do + local_user_query() + end + + def maybe_local_user_query(local) when local == false do + User + end + def local_user_query do from( u in User, diff --git a/lib/pleroma/web/admin_api/admin_api_controller.ex b/lib/pleroma/web/admin_api/admin_api_controller.ex index 37159cd40..a8f9e5012 100644 --- a/lib/pleroma/web/admin_api/admin_api_controller.ex +++ b/lib/pleroma/web/admin_api/admin_api_controller.ex @@ -78,8 +78,14 @@ def list_users(%{assigns: %{user: admin}} = conn, %{"page" => page_string}) do ) end - def search_users(%{assigns: %{user: admin}} = conn, %{"query" => query}) do - users = User.search(query, true, admin, @users_page_size) + def search_users(%{assigns: %{user: admin}} = conn, %{"query" => term} = params) do + users = + User.search(term, + query: User.maybe_local_user_query(params["local"] == "true"), + resolve: true, + for_user: admin, + limit: @users_page_size + ) conn |> json( diff --git a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex index 12987442a..056be49b0 100644 --- a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex +++ b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex @@ -894,7 +894,7 @@ def status_search(user, query) do end def search2(%{assigns: %{user: user}} = conn, %{"q" => query} = params) do - accounts = User.search(query, params["resolve"] == "true", user) + accounts = User.search(query, resolve: params["resolve"] == "true", for_user: user) statuses = status_search(user, query) @@ -919,7 +919,7 @@ def search2(%{assigns: %{user: user}} = conn, %{"q" => query} = params) do end def search(%{assigns: %{user: user}} = conn, %{"q" => query} = params) do - accounts = User.search(query, params["resolve"] == "true", user) + accounts = User.search(query, resolve: params["resolve"] == "true", for_user: user) statuses = status_search(user, query) @@ -941,7 +941,7 @@ def search(%{assigns: %{user: user}} = conn, %{"q" => query} = params) do end def account_search(%{assigns: %{user: user}} = conn, %{"q" => query} = params) do - accounts = User.search(query, params["resolve"] == "true", user) + accounts = User.search(query, resolve: params["resolve"] == "true", for_user: user) res = AccountView.render("accounts.json", users: accounts, for: user, as: :user) diff --git a/lib/pleroma/web/twitter_api/twitter_api_controller.ex b/lib/pleroma/web/twitter_api/twitter_api_controller.ex index 0d74c30c3..19264a93f 100644 --- a/lib/pleroma/web/twitter_api/twitter_api_controller.ex +++ b/lib/pleroma/web/twitter_api/twitter_api_controller.ex @@ -702,7 +702,7 @@ def search(%{assigns: %{user: user}} = conn, %{"q" => _query} = params) do end def search_user(%{assigns: %{user: user}} = conn, %{"query" => query}) do - users = User.search(query, true, user) + users = User.search(query, resolve: true, for_user: user) conn |> put_view(UserView) diff --git a/test/web/admin_api/admin_api_controller_test.exs b/test/web/admin_api/admin_api_controller_test.exs index 14625af32..460f2a6bd 100644 --- a/test/web/admin_api/admin_api_controller_test.exs +++ b/test/web/admin_api/admin_api_controller_test.exs @@ -388,25 +388,51 @@ test "PATCH /api/pleroma/admin/users/:nickname/toggle_activation" do } end - test "GET /api/pleroma/admin/users/search" do - admin = insert(:user, info: %{is_admin: true}) - user = insert(:user, nickname: "bob") + describe "GET /api/pleroma/admin/users/search" do + test "regular search" do + admin = insert(:user, info: %{is_admin: true}) + user = insert(:user, nickname: "bob") - conn = - build_conn() - |> assign(:user, admin) - |> get("/api/pleroma/admin/users/search?query=bo") + conn = + build_conn() + |> assign(:user, admin) + |> get("/api/pleroma/admin/users/search?query=bo") - assert json_response(conn, 200) == %{ - "count" => 1, - "page_size" => 50, - "users" => [ - %{ - "deactivated" => user.info.deactivated, - "id" => user.id, - "nickname" => user.nickname - } - ] - } + assert json_response(conn, 200) == %{ + "count" => 1, + "page_size" => 50, + "users" => [ + %{ + "deactivated" => user.info.deactivated, + "id" => user.id, + "nickname" => user.nickname + } + ] + } + end + + test "only local users" do + admin = insert(:user, info: %{is_admin: true}) + user = insert(:user, nickname: "bob") + + insert(:user, nickname: "bobb", local: false) + + conn = + build_conn() + |> assign(:user, admin) + |> get("/api/pleroma/admin/users/search?query=bo&local=true") + + assert json_response(conn, 200) == %{ + "count" => 1, + "page_size" => 50, + "users" => [ + %{ + "deactivated" => user.info.deactivated, + "id" => user.id, + "nickname" => user.nickname + } + ] + } + end end end From f1a4c3163b18692a7a8bd9874a45e75a6535dd5a Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Fri, 1 Mar 2019 20:23:03 +0300 Subject: [PATCH 07/14] Show current user in users list as well --- lib/pleroma/user.ex | 3 +- .../web/admin_api/admin_api_controller.ex | 2 +- .../admin_api/admin_api_controller_test.exs | 45 ++++++++++--------- 3 files changed, 27 insertions(+), 23 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index af3ce705d..37f8da892 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -779,10 +779,9 @@ def search(term, options \\ %{}) do Enum.uniq_by(fts_results ++ trigram_results, & &1.id) end - def all_except_one(user, page, page_size) do + def all(page, page_size) do from( u in User, - where: u.id != ^user.id, limit: ^page_size, offset: ^((page - 1) * page_size), order_by: u.id diff --git a/lib/pleroma/web/admin_api/admin_api_controller.ex b/lib/pleroma/web/admin_api/admin_api_controller.ex index a8f9e5012..270097d35 100644 --- a/lib/pleroma/web/admin_api/admin_api_controller.ex +++ b/lib/pleroma/web/admin_api/admin_api_controller.ex @@ -65,7 +65,7 @@ def untag_users(conn, %{"nicknames" => nicknames, "tags" => tags}) do def list_users(%{assigns: %{user: admin}} = conn, %{"page" => page_string}) do with {page, _} <- Integer.parse(page_string), - users <- User.all_except_one(admin, page, @users_page_size), + users <- User.all(page, @users_page_size), count <- User.count_all_except_one(admin), do: conn diff --git a/test/web/admin_api/admin_api_controller_test.exs b/test/web/admin_api/admin_api_controller_test.exs index 460f2a6bd..0679f5dfe 100644 --- a/test/web/admin_api/admin_api_controller_test.exs +++ b/test/web/admin_api/admin_api_controller_test.exs @@ -345,6 +345,11 @@ test "renders users array for the first page" do "count" => 1, "page_size" => 50, "users" => [ + %{ + "deactivated" => admin.info.deactivated, + "id" => admin.id, + "nickname" => admin.nickname + }, %{ "deactivated" => user.info.deactivated, "id" => user.id, @@ -399,16 +404,16 @@ test "regular search" do |> get("/api/pleroma/admin/users/search?query=bo") assert json_response(conn, 200) == %{ - "count" => 1, - "page_size" => 50, - "users" => [ - %{ - "deactivated" => user.info.deactivated, - "id" => user.id, - "nickname" => user.nickname - } - ] - } + "count" => 1, + "page_size" => 50, + "users" => [ + %{ + "deactivated" => user.info.deactivated, + "id" => user.id, + "nickname" => user.nickname + } + ] + } end test "only local users" do @@ -423,16 +428,16 @@ test "only local users" do |> get("/api/pleroma/admin/users/search?query=bo&local=true") assert json_response(conn, 200) == %{ - "count" => 1, - "page_size" => 50, - "users" => [ - %{ - "deactivated" => user.info.deactivated, - "id" => user.id, - "nickname" => user.nickname - } - ] - } + "count" => 1, + "page_size" => 50, + "users" => [ + %{ + "deactivated" => user.info.deactivated, + "id" => user.id, + "nickname" => user.nickname + } + ] + } end end end From f384a9a2563d2fb6161ebb824534fda08cf67c64 Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Fri, 1 Mar 2019 20:23:19 +0300 Subject: [PATCH 08/14] Format --- test/web/admin_api/admin_api_controller_test.exs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/web/admin_api/admin_api_controller_test.exs b/test/web/admin_api/admin_api_controller_test.exs index 0679f5dfe..a3042fa05 100644 --- a/test/web/admin_api/admin_api_controller_test.exs +++ b/test/web/admin_api/admin_api_controller_test.exs @@ -345,11 +345,11 @@ test "renders users array for the first page" do "count" => 1, "page_size" => 50, "users" => [ - %{ - "deactivated" => admin.info.deactivated, - "id" => admin.id, - "nickname" => admin.nickname - }, + %{ + "deactivated" => admin.info.deactivated, + "id" => admin.id, + "nickname" => admin.nickname + }, %{ "deactivated" => user.info.deactivated, "id" => user.id, From aaa9fed1ca196b55d8c05af838d6947959548bf0 Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Fri, 1 Mar 2019 20:58:47 +0300 Subject: [PATCH 09/14] Fix user_test --- test/user_test.exs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/user_test.exs b/test/user_test.exs index 0b1c39ecf..27137fc35 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -901,7 +901,7 @@ test "finds users, boosting ranks of friends and followers" do {:ok, follower} = User.follow(follower, u1) {:ok, u1} = User.follow(u1, friend) - assert [friend.id, follower.id, u2.id] == Enum.map(User.search("doe", false, u1), & &1.id) + assert [friend.id, follower.id, u2.id] == Enum.map(User.search("doe", resolve: false, for_user: u1), & &1.id) end test "finds a user whose name is nil" do @@ -923,7 +923,7 @@ test "does not yield false-positive matches" do end test "works with URIs" do - results = User.search("http://mastodon.example.org/users/admin", true) + results = User.search("http://mastodon.example.org/users/admin", resolve: true) result = results |> List.first() user = User.get_by_ap_id("http://mastodon.example.org/users/admin") From a25c1313aee2f5f3d2b858b9802698f29acf4043 Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Fri, 1 Mar 2019 21:07:05 +0300 Subject: [PATCH 10/14] Format --- test/user_test.exs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/user_test.exs b/test/user_test.exs index 27137fc35..4f9de4e31 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -901,7 +901,8 @@ test "finds users, boosting ranks of friends and followers" do {:ok, follower} = User.follow(follower, u1) {:ok, u1} = User.follow(u1, friend) - assert [friend.id, follower.id, u2.id] == Enum.map(User.search("doe", resolve: false, for_user: u1), & &1.id) + assert [friend.id, follower.id, u2.id] == + Enum.map(User.search("doe", resolve: false, for_user: u1), & &1.id) end test "finds a user whose name is nil" do From f635b675b2cc0bc10b395cd71ae1720b0696d364 Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Fri, 1 Mar 2019 21:17:20 +0300 Subject: [PATCH 11/14] Refactor a little bit --- lib/pleroma/user.ex | 16 ++++------------ .../web/admin_api/admin_api_controller.ex | 8 ++++---- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 37f8da892..3c6fb4f9b 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -763,17 +763,13 @@ def search(term, options \\ %{}) do if options[:resolve], do: get_or_fetch(term) fts_results = - do_search(fts_search_subquery(term, query), options[:for_user], %{ - limit: options[:limit] - }) + do_search(fts_search_subquery(term, query), options[:for_user], limit: options[:limit]) {:ok, trigram_results} = Repo.transaction(fn -> Ecto.Adapters.SQL.query(Repo, "select set_limit(0.25)", []) - do_search(trigram_search_subquery(term, query), options[:for_user], %{ - limit: options[:limit] - }) + do_search(trigram_search_subquery(term, query), options[:for_user], limit: options[:limit]) end) Enum.uniq_by(fts_results ++ trigram_results, & &1.id) @@ -1024,12 +1020,8 @@ def unblock_domain(user, domain) do update_and_set_cache(cng) end - def maybe_local_user_query(local) when local == true do - local_user_query() - end - - def maybe_local_user_query(local) when local == false do - User + def maybe_local_user_query(local) do + if local, do: local_user_query(), else: User end def local_user_query do diff --git a/lib/pleroma/web/admin_api/admin_api_controller.ex b/lib/pleroma/web/admin_api/admin_api_controller.ex index 270097d35..33f9689cd 100644 --- a/lib/pleroma/web/admin_api/admin_api_controller.ex +++ b/lib/pleroma/web/admin_api/admin_api_controller.ex @@ -70,11 +70,11 @@ def list_users(%{assigns: %{user: admin}} = conn, %{"page" => page_string}) do do: conn |> json( - AccountView.render("index.json", %{ + AccountView.render("index.json", users: users, count: count, page_size: @users_page_size - }) + ) ) end @@ -89,11 +89,11 @@ def search_users(%{assigns: %{user: admin}} = conn, %{"query" => term} = params) conn |> json( - AccountView.render("index.json", %{ + AccountView.render("index.json", users: users, count: length(users), page_size: @users_page_size - }) + ) ) end From 2ec8cf566569912b767e15ab467cadd04fd1fd1c Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Sat, 2 Mar 2019 17:21:18 +0300 Subject: [PATCH 12/14] Add pagination to search --- lib/pleroma/user.ex | 110 ++++++++++-------- .../web/admin_api/admin_api_controller.ex | 66 +++++++---- .../admin_api/admin_api_controller_test.exs | 46 +++++++- 3 files changed, 152 insertions(+), 70 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 3c6fb4f9b..230155c33 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -547,11 +547,8 @@ def get_followers_query(%User{id: id, follower_address: follower_address}, nil) end def get_followers_query(user, page) do - from( - u in get_followers_query(user, nil), - limit: 20, - offset: ^((page - 1) * 20) - ) + from(u in get_followers_query(user, nil)) + |> paginate(page, 20) end def get_followers_query(user), do: get_followers_query(user, nil) @@ -577,11 +574,8 @@ def get_friends_query(%User{id: id, following: following}, nil) do end def get_friends_query(user, page) do - from( - u in get_friends_query(user, nil), - limit: 20, - offset: ^((page - 1) * 20) - ) + from(u in get_friends_query(user, nil)) + |> paginate(page, 20) end def get_friends_query(user), do: get_friends_query(user, nil) @@ -755,47 +749,64 @@ def get_recipients_from_activity(%Activity{recipients: to}) do Repo.all(query) end - def search(term, options \\ %{}) do - # Strip the beginning @ off if there is a query + @spec search_for_admin(binary(), %{ + admin: Pleroma.User.t(), + local: boolean(), + page: number(), + page_size: number() + }) :: {:ok, [Pleroma.User.t()], number()} + def search_for_admin(term, %{admin: admin, local: local, page: page, page_size: page_size}) do term = String.trim_leading(term, "@") - query = options[:query] || User - if options[:resolve], do: get_or_fetch(term) + local_paginated_query = + User + |> maybe_local_user_query(local) + |> paginate(page, page_size) - fts_results = - do_search(fts_search_subquery(term, query), options[:for_user], limit: options[:limit]) + search_query = fts_search_subquery(term, local_paginated_query) + + count = + term + |> fts_search_subquery() + |> maybe_local_user_query(local) + |> Repo.aggregate(:count, :id) + + {:ok, do_search(search_query, admin), count} + end + + @spec all_for_admin(number(), number()) :: {:ok, [Pleroma.User.t()], number()} + def all_for_admin(page, page_size) do + query = from(u in User, order_by: u.id) + + paginated_query = + query + |> paginate(page, page_size) + + count = + query + |> Repo.aggregate(:count, :id) + + {:ok, Repo.all(paginated_query), count} + end + + def search(query, resolve \\ false, for_user \\ nil) do + # Strip the beginning @ off if there is a query + query = String.trim_leading(query, "@") + + if resolve, do: get_or_fetch(query) + + fts_results = do_search(fts_search_subquery(query), for_user) {:ok, trigram_results} = Repo.transaction(fn -> Ecto.Adapters.SQL.query(Repo, "select set_limit(0.25)", []) - - do_search(trigram_search_subquery(term, query), options[:for_user], limit: options[:limit]) + do_search(trigram_search_subquery(query), for_user) end) Enum.uniq_by(fts_results ++ trigram_results, & &1.id) end - def all(page, page_size) do - from( - u in User, - limit: ^page_size, - offset: ^((page - 1) * page_size), - order_by: u.id - ) - |> Repo.all() - end - - def count_all_except_one(user) do - query = - from( - u in User, - where: u.id != ^user.id - ) - - Repo.aggregate(query, :count, :id) - end - - defp do_search(subquery, for_user, options) do + defp do_search(subquery, for_user, options \\ []) do q = from( s in subquery(subquery), @@ -811,7 +822,7 @@ defp do_search(subquery, for_user, options) do boost_search_results(results, for_user) end - defp fts_search_subquery(term, query) do + defp fts_search_subquery(term, query \\ User) do processed_query = term |> String.replace(~r/\W+/, " ") @@ -851,9 +862,9 @@ defp fts_search_subquery(term, query) do ) end - defp trigram_search_subquery(term, query) do + defp trigram_search_subquery(term) do from( - u in query, + u in User, select_merge: %{ search_rank: fragment( @@ -1020,13 +1031,13 @@ def unblock_domain(user, domain) do update_and_set_cache(cng) end - def maybe_local_user_query(local) do - if local, do: local_user_query(), else: User + def maybe_local_user_query(query, local) do + if local, do: local_user_query(query), else: query end - def local_user_query do + def local_user_query(query \\ User) do from( - u in User, + u in query, where: u.local == true, where: not is_nil(u.nickname) ) @@ -1318,4 +1329,11 @@ def all_superusers do ) |> Repo.all() end + + defp paginate(query, page, page_size) do + from(u in query, + limit: ^page_size, + offset: ^((page - 1) * page_size) + ) + end end diff --git a/lib/pleroma/web/admin_api/admin_api_controller.ex b/lib/pleroma/web/admin_api/admin_api_controller.ex index 33f9689cd..aae02cab8 100644 --- a/lib/pleroma/web/admin_api/admin_api_controller.ex +++ b/lib/pleroma/web/admin_api/admin_api_controller.ex @@ -63,38 +63,40 @@ def untag_users(conn, %{"nicknames" => nicknames, "tags" => tags}) do do: json_response(conn, :no_content, "") end - def list_users(%{assigns: %{user: admin}} = conn, %{"page" => page_string}) do - with {page, _} <- Integer.parse(page_string), - users <- User.all(page, @users_page_size), - count <- User.count_all_except_one(admin), + def list_users(conn, params) do + {page, page_size} = page_params(params) + + with {:ok, users, count} <- User.all_for_admin(page, page_size), do: conn |> json( AccountView.render("index.json", users: users, count: count, - page_size: @users_page_size + page_size: page_size ) ) end - def search_users(%{assigns: %{user: admin}} = conn, %{"query" => term} = params) do - users = - User.search(term, - query: User.maybe_local_user_query(params["local"] == "true"), - resolve: true, - for_user: admin, - limit: @users_page_size - ) + def search_users(%{assigns: %{user: admin}} = conn, %{"query" => query} = params) do + {page, page_size} = page_params(params) - conn - |> json( - AccountView.render("index.json", - users: users, - count: length(users), - page_size: @users_page_size - ) - ) + with {:ok, users, count} <- + User.search_for_admin(query, %{ + admin: admin, + local: params["local"] == "true", + page: page, + page_size: page_size + }), + do: + conn + |> json( + AccountView.render("index.json", + users: users, + count: count, + page_size: page_size + ) + ) end def right_add(conn, %{"permission_group" => permission_group, "nickname" => nickname}) @@ -240,4 +242,26 @@ def errors(conn, _) do |> put_status(500) |> json("Something went wrong") end + + defp page_params(params) do + {get_page(params["page"]), get_page_size(params["page_size"])} + end + + defp get_page(page_string) when is_nil(page_string), do: 1 + + defp get_page(page_string) do + case Integer.parse(page_string) do + {page, _} -> page + :error -> 1 + end + end + + defp get_page_size(page_size_string) when is_nil(page_size_string), do: @users_page_size + + defp get_page_size(page_size_string) do + case Integer.parse(page_size_string) do + {page_size, _} -> page_size + :error -> @users_page_size + end + end end diff --git a/test/web/admin_api/admin_api_controller_test.exs b/test/web/admin_api/admin_api_controller_test.exs index a3042fa05..42e0daf8e 100644 --- a/test/web/admin_api/admin_api_controller_test.exs +++ b/test/web/admin_api/admin_api_controller_test.exs @@ -342,7 +342,7 @@ test "renders users array for the first page" do |> get("/api/pleroma/admin/users?page=1") assert json_response(conn, 200) == %{ - "count" => 1, + "count" => 2, "page_size" => 50, "users" => [ %{ @@ -369,7 +369,7 @@ test "renders empty array for the second page" do |> get("/api/pleroma/admin/users?page=2") assert json_response(conn, 200) == %{ - "count" => 1, + "count" => 2, "page_size" => 50, "users" => [] } @@ -416,9 +416,49 @@ test "regular search" do } end - test "only local users" do + test "regular search with page size" do admin = insert(:user, info: %{is_admin: true}) user = insert(:user, nickname: "bob") + user2 = insert(:user, nickname: "bo") + + conn = + build_conn() + |> assign(:user, admin) + |> get("/api/pleroma/admin/users/search?query=bo&page_size=1&page=1") + + assert json_response(conn, 200) == %{ + "count" => 2, + "page_size" => 1, + "users" => [ + %{ + "deactivated" => user.info.deactivated, + "id" => user.id, + "nickname" => user.nickname + } + ] + } + + conn = + build_conn() + |> assign(:user, admin) + |> get("/api/pleroma/admin/users/search?query=bo&page_size=1&page=2") + + assert json_response(conn, 200) == %{ + "count" => 2, + "page_size" => 1, + "users" => [ + %{ + "deactivated" => user2.info.deactivated, + "id" => user2.id, + "nickname" => user2.nickname + } + ] + } + end + + test "only local users" do + admin = insert(:user, info: %{is_admin: true}, nickname: "john") + user = insert(:user, nickname: "bob") insert(:user, nickname: "bobb", local: false) From bf30df99cb9a08870f7c6a25e0f9f3d2e03d5de7 Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Sat, 2 Mar 2019 17:32:40 +0300 Subject: [PATCH 13/14] We do not guarantee the order of elements when we search --- test/user_test.exs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/user_test.exs b/test/user_test.exs index 4f9de4e31..188ab1a5c 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -901,8 +901,8 @@ test "finds users, boosting ranks of friends and followers" do {:ok, follower} = User.follow(follower, u1) {:ok, u1} = User.follow(u1, friend) - assert [friend.id, follower.id, u2.id] == - Enum.map(User.search("doe", resolve: false, for_user: u1), & &1.id) + assert [friend.id, follower.id, u2.id] -- + Enum.map(User.search("doe", resolve: false, for_user: u1), & &1.id) == [] end test "finds a user whose name is nil" do From 08c6aeeed7ff5db242050d06e707f99b6d75684d Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Sat, 2 Mar 2019 17:32:46 +0300 Subject: [PATCH 14/14] Add docs --- docs/Admin-API.md | 49 ++++++++++++++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 18 deletions(-) diff --git a/docs/Admin-API.md b/docs/Admin-API.md index 4403620bf..407647645 100644 --- a/docs/Admin-API.md +++ b/docs/Admin-API.md @@ -7,20 +7,27 @@ Authentication is required and the user must be an admin. ### List users - Method `GET` +- Params: + - `page`: **integer** page number + - `page_size`: **integer** number of users per page (default is `50`) - Response: ```JSON -[ +{ + "page_size": integer, + "count": integer, + "users": [ { - "deactivated": bool, - "id": integer, - "nickname": string + "deactivated": bool, + "id": integer, + "nickname": string }, ... -] + ] +} ``` -## `/api/pleroma/admin/users/search?query={query}&local={local}` +## `/api/pleroma/admin/users/search?query={query}&local={local}&page={page}&page_size={page_size}` ### Search users by name or nickname @@ -28,17 +35,23 @@ Authentication is required and the user must be an admin. - Params: - `query`: **string** search term - `local`: **bool** whether to return only local users + - `page`: **integer** page number + - `page_size`: **integer** number of users per page (default is `50`) - Response: ```JSON -[ +{ + "page_size": integer, + "count": integer, + "users": [ { - "deactivated": bool, - "id": integer, - "nickname": string + "deactivated": bool, + "id": integer, + "nickname": string }, ... -] + ] +} ``` ## `/api/pleroma/admin/user` @@ -70,9 +83,9 @@ Authentication is required and the user must be an admin. ```JSON { - "deactivated": bool, - "id": integer, - "nickname": string + "deactivated": bool, + "id": integer, + "nickname": string } ``` @@ -102,8 +115,8 @@ Authentication is required and the user must be an admin. ```JSON { - "is_moderator": bool, - "is_admin": bool + "is_moderator": bool, + "is_admin": bool } ``` @@ -119,8 +132,8 @@ Note: Available `:permission_group` is currently moderator and admin. 404 is ret ```JSON { - "is_moderator": bool, - "is_admin": bool + "is_moderator": bool, + "is_admin": bool } ```