From 9ef46ce4103f338d5bb75278013a63d3ae418d7e Mon Sep 17 00:00:00 2001
From: Maksim Pechnikov <parallel588@gmail.com>
Date: Mon, 21 Sep 2020 09:33:51 +0300
Subject: [PATCH] added 'unconfirmed' filter to admin/users

---
 lib/pleroma/user/query.ex                     |   5 +
 .../controllers/admin_api_controller.ex       |  55 +--
 .../controllers/admin_api_controller_test.exs | 399 +++++-------------
 test/pleroma/web/admin_api/search_test.exs    |  11 +
 4 files changed, 131 insertions(+), 339 deletions(-)

diff --git a/lib/pleroma/user/query.ex b/lib/pleroma/user/query.ex
index 2440bf890..2933e7fb4 100644
--- a/lib/pleroma/user/query.ex
+++ b/lib/pleroma/user/query.ex
@@ -43,6 +43,7 @@ defmodule Pleroma.User.Query do
             active: boolean(),
             deactivated: boolean(),
             need_approval: boolean(),
+            need_confirmed: boolean(),
             is_admin: boolean(),
             is_moderator: boolean(),
             super_users: boolean(),
@@ -156,6 +157,10 @@ defp compose_query({:need_approval, _}, query) do
     where(query, [u], u.approval_pending)
   end
 
+  defp compose_query({:need_confirmed, _}, query) do
+    where(query, [u], u.confirmation_pending)
+  end
+
   defp compose_query({:followers, %User{id: id}}, query) do
     query
     |> where([u], u.id != ^id)
diff --git a/lib/pleroma/web/admin_api/controllers/admin_api_controller.ex b/lib/pleroma/web/admin_api/controllers/admin_api_controller.ex
index bdd3e195d..acfbeb0c8 100644
--- a/lib/pleroma/web/admin_api/controllers/admin_api_controller.ex
+++ b/lib/pleroma/web/admin_api/controllers/admin_api_controller.ex
@@ -5,7 +5,8 @@
 defmodule Pleroma.Web.AdminAPI.AdminAPIController do
   use Pleroma.Web, :controller
 
-  import Pleroma.Web.ControllerHelper, only: [json_response: 3]
+  import Pleroma.Web.ControllerHelper,
+    only: [json_response: 3, fetch_integer_param: 3]
 
   alias Pleroma.Config
   alias Pleroma.MFA
@@ -100,12 +101,9 @@ def user_delete(conn, %{"nickname" => nickname}) do
   end
 
   def user_delete(%{assigns: %{user: admin}} = conn, %{"nicknames" => nicknames}) do
-    users =
-      nicknames
-      |> Enum.map(&User.get_cached_by_nickname/1)
+    users = Enum.map(nicknames, &User.get_cached_by_nickname/1)
 
-    users
-    |> Enum.each(fn user ->
+    Enum.each(users, fn user ->
       {:ok, delete_data, _} = Builder.delete(admin, user.ap_id)
       Pipeline.common_pipeline(delete_data, local: true)
     end)
@@ -367,16 +365,18 @@ def list_users(conn, params) do
     {page, page_size} = page_params(params)
     filters = maybe_parse_filters(params["filters"])
 
-    search_params = %{
-      query: params["query"],
-      page: page,
-      page_size: page_size,
-      tags: params["tags"],
-      name: params["name"],
-      email: params["email"]
-    }
+    search_params =
+      %{
+        query: params["query"],
+        page: page,
+        page_size: page_size,
+        tags: params["tags"],
+        name: params["name"],
+        email: params["email"]
+      }
+      |> Map.merge(filters)
 
-    with {:ok, users, count} <- Search.user(Map.merge(search_params, filters)) do
+    with {:ok, users, count} <- Search.user(search_params) do
       json(
         conn,
         AccountView.render("index.json",
@@ -388,7 +388,7 @@ def list_users(conn, params) do
     end
   end
 
-  @filters ~w(local external active deactivated need_approval is_admin is_moderator)
+  @filters ~w(local external active deactivated need_approval need_confirmed is_admin is_moderator)
 
   @spec maybe_parse_filters(String.t()) :: %{required(String.t()) => true} | %{}
   defp maybe_parse_filters(filters) when is_nil(filters) or filters == "", do: %{}
@@ -682,24 +682,9 @@ def stats(conn, params) do
   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
+    {
+      fetch_integer_param(params, "page", 1),
+      fetch_integer_param(params, "page_size", @users_page_size)
+    }
   end
 end
diff --git a/test/pleroma/web/admin_api/controllers/admin_api_controller_test.exs b/test/pleroma/web/admin_api/controllers/admin_api_controller_test.exs
index cba6b43d3..686b53a80 100644
--- a/test/pleroma/web/admin_api/controllers/admin_api_controller_test.exs
+++ b/test/pleroma/web/admin_api/controllers/admin_api_controller_test.exs
@@ -369,23 +369,7 @@ test "Show", %{conn: conn} do
 
       conn = get(conn, "/api/pleroma/admin/users/#{user.nickname}")
 
-      expected = %{
-        "deactivated" => false,
-        "id" => to_string(user.id),
-        "local" => true,
-        "nickname" => user.nickname,
-        "roles" => %{"admin" => false, "moderator" => false},
-        "tags" => [],
-        "avatar" => User.avatar_url(user) |> MediaProxy.url(),
-        "display_name" => HTML.strip_tags(user.name || user.nickname),
-        "confirmation_pending" => false,
-        "approval_pending" => false,
-        "url" => user.ap_id,
-        "registration_reason" => nil,
-        "actor_type" => "Person"
-      }
-
-      assert expected == json_response(conn, 200)
+      assert user_response(user) == json_response(conn, 200)
     end
 
     test "when the user doesn't exist", %{conn: conn} do
@@ -652,51 +636,20 @@ test "renders users array for the first page", %{conn: conn, admin: admin} do
 
       users =
         [
-          %{
-            "deactivated" => admin.deactivated,
-            "id" => admin.id,
-            "nickname" => admin.nickname,
-            "roles" => %{"admin" => true, "moderator" => false},
-            "local" => true,
-            "tags" => [],
-            "avatar" => User.avatar_url(admin) |> MediaProxy.url(),
-            "display_name" => HTML.strip_tags(admin.name || admin.nickname),
-            "confirmation_pending" => false,
-            "approval_pending" => false,
-            "url" => admin.ap_id,
-            "registration_reason" => nil,
-            "actor_type" => "Person"
-          },
-          %{
-            "deactivated" => user.deactivated,
-            "id" => user.id,
-            "nickname" => user.nickname,
-            "roles" => %{"admin" => false, "moderator" => false},
-            "local" => false,
-            "tags" => ["foo", "bar"],
-            "avatar" => User.avatar_url(user) |> MediaProxy.url(),
-            "display_name" => HTML.strip_tags(user.name || user.nickname),
-            "confirmation_pending" => false,
-            "approval_pending" => false,
-            "url" => user.ap_id,
-            "registration_reason" => nil,
-            "actor_type" => "Person"
-          },
-          %{
-            "deactivated" => user2.deactivated,
-            "id" => user2.id,
-            "nickname" => user2.nickname,
-            "roles" => %{"admin" => false, "moderator" => false},
-            "local" => true,
-            "tags" => [],
-            "avatar" => User.avatar_url(user2) |> MediaProxy.url(),
-            "display_name" => HTML.strip_tags(user2.name || user2.nickname),
-            "confirmation_pending" => false,
-            "approval_pending" => true,
-            "url" => user2.ap_id,
-            "registration_reason" => "I'm a chill dude",
-            "actor_type" => "Person"
-          }
+          user_response(
+            admin,
+            %{"roles" => %{"admin" => true, "moderator" => false}}
+          ),
+          user_response(user, %{"local" => false, "tags" => ["foo", "bar"]}),
+          user_response(
+            user2,
+            %{
+              "local" => true,
+              "approval_pending" => true,
+              "registration_reason" => "I'm a chill dude",
+              "actor_type" => "Person"
+            }
+          )
         ]
         |> Enum.sort_by(& &1["nickname"])
 
@@ -757,23 +710,7 @@ test "regular search", %{conn: conn} do
       assert json_response(conn, 200) == %{
                "count" => 1,
                "page_size" => 50,
-               "users" => [
-                 %{
-                   "deactivated" => user.deactivated,
-                   "id" => user.id,
-                   "nickname" => user.nickname,
-                   "roles" => %{"admin" => false, "moderator" => false},
-                   "local" => true,
-                   "tags" => [],
-                   "avatar" => User.avatar_url(user) |> MediaProxy.url(),
-                   "display_name" => HTML.strip_tags(user.name || user.nickname),
-                   "confirmation_pending" => false,
-                   "approval_pending" => false,
-                   "url" => user.ap_id,
-                   "registration_reason" => nil,
-                   "actor_type" => "Person"
-                 }
-               ]
+               "users" => [user_response(user, %{"local" => true})]
              }
     end
 
@@ -786,23 +723,7 @@ test "search by domain", %{conn: conn} do
       assert json_response(conn, 200) == %{
                "count" => 1,
                "page_size" => 50,
-               "users" => [
-                 %{
-                   "deactivated" => user.deactivated,
-                   "id" => user.id,
-                   "nickname" => user.nickname,
-                   "roles" => %{"admin" => false, "moderator" => false},
-                   "local" => true,
-                   "tags" => [],
-                   "avatar" => User.avatar_url(user) |> MediaProxy.url(),
-                   "display_name" => HTML.strip_tags(user.name || user.nickname),
-                   "confirmation_pending" => false,
-                   "approval_pending" => false,
-                   "url" => user.ap_id,
-                   "registration_reason" => nil,
-                   "actor_type" => "Person"
-                 }
-               ]
+               "users" => [user_response(user)]
              }
     end
 
@@ -815,23 +736,7 @@ test "search by full nickname", %{conn: conn} do
       assert json_response(conn, 200) == %{
                "count" => 1,
                "page_size" => 50,
-               "users" => [
-                 %{
-                   "deactivated" => user.deactivated,
-                   "id" => user.id,
-                   "nickname" => user.nickname,
-                   "roles" => %{"admin" => false, "moderator" => false},
-                   "local" => true,
-                   "tags" => [],
-                   "avatar" => User.avatar_url(user) |> MediaProxy.url(),
-                   "display_name" => HTML.strip_tags(user.name || user.nickname),
-                   "confirmation_pending" => false,
-                   "approval_pending" => false,
-                   "url" => user.ap_id,
-                   "registration_reason" => nil,
-                   "actor_type" => "Person"
-                 }
-               ]
+               "users" => [user_response(user)]
              }
     end
 
@@ -844,23 +749,7 @@ test "search by display name", %{conn: conn} do
       assert json_response(conn, 200) == %{
                "count" => 1,
                "page_size" => 50,
-               "users" => [
-                 %{
-                   "deactivated" => user.deactivated,
-                   "id" => user.id,
-                   "nickname" => user.nickname,
-                   "roles" => %{"admin" => false, "moderator" => false},
-                   "local" => true,
-                   "tags" => [],
-                   "avatar" => User.avatar_url(user) |> MediaProxy.url(),
-                   "display_name" => HTML.strip_tags(user.name || user.nickname),
-                   "confirmation_pending" => false,
-                   "approval_pending" => false,
-                   "url" => user.ap_id,
-                   "registration_reason" => nil,
-                   "actor_type" => "Person"
-                 }
-               ]
+               "users" => [user_response(user)]
              }
     end
 
@@ -873,23 +762,7 @@ test "search by email", %{conn: conn} do
       assert json_response(conn, 200) == %{
                "count" => 1,
                "page_size" => 50,
-               "users" => [
-                 %{
-                   "deactivated" => user.deactivated,
-                   "id" => user.id,
-                   "nickname" => user.nickname,
-                   "roles" => %{"admin" => false, "moderator" => false},
-                   "local" => true,
-                   "tags" => [],
-                   "avatar" => User.avatar_url(user) |> MediaProxy.url(),
-                   "display_name" => HTML.strip_tags(user.name || user.nickname),
-                   "confirmation_pending" => false,
-                   "approval_pending" => false,
-                   "url" => user.ap_id,
-                   "registration_reason" => nil,
-                   "actor_type" => "Person"
-                 }
-               ]
+               "users" => [user_response(user)]
              }
     end
 
@@ -902,23 +775,7 @@ test "regular search with page size", %{conn: conn} do
       assert json_response(conn1, 200) == %{
                "count" => 2,
                "page_size" => 1,
-               "users" => [
-                 %{
-                   "deactivated" => user.deactivated,
-                   "id" => user.id,
-                   "nickname" => user.nickname,
-                   "roles" => %{"admin" => false, "moderator" => false},
-                   "local" => true,
-                   "tags" => [],
-                   "avatar" => User.avatar_url(user) |> MediaProxy.url(),
-                   "display_name" => HTML.strip_tags(user.name || user.nickname),
-                   "confirmation_pending" => false,
-                   "approval_pending" => false,
-                   "url" => user.ap_id,
-                   "registration_reason" => nil,
-                   "actor_type" => "Person"
-                 }
-               ]
+               "users" => [user_response(user)]
              }
 
       conn2 = get(conn, "/api/pleroma/admin/users?query=a&page_size=1&page=2")
@@ -926,23 +783,7 @@ test "regular search with page size", %{conn: conn} do
       assert json_response(conn2, 200) == %{
                "count" => 2,
                "page_size" => 1,
-               "users" => [
-                 %{
-                   "deactivated" => user2.deactivated,
-                   "id" => user2.id,
-                   "nickname" => user2.nickname,
-                   "roles" => %{"admin" => false, "moderator" => false},
-                   "local" => true,
-                   "tags" => [],
-                   "avatar" => User.avatar_url(user2) |> MediaProxy.url(),
-                   "display_name" => HTML.strip_tags(user2.name || user2.nickname),
-                   "confirmation_pending" => false,
-                   "approval_pending" => false,
-                   "url" => user2.ap_id,
-                   "registration_reason" => nil,
-                   "actor_type" => "Person"
-                 }
-               ]
+               "users" => [user_response(user2)]
              }
     end
 
@@ -962,23 +803,7 @@ test "only local users" do
       assert json_response(conn, 200) == %{
                "count" => 1,
                "page_size" => 50,
-               "users" => [
-                 %{
-                   "deactivated" => user.deactivated,
-                   "id" => user.id,
-                   "nickname" => user.nickname,
-                   "roles" => %{"admin" => false, "moderator" => false},
-                   "local" => true,
-                   "tags" => [],
-                   "avatar" => User.avatar_url(user) |> MediaProxy.url(),
-                   "display_name" => HTML.strip_tags(user.name || user.nickname),
-                   "confirmation_pending" => false,
-                   "approval_pending" => false,
-                   "url" => user.ap_id,
-                   "registration_reason" => nil,
-                   "actor_type" => "Person"
-                 }
-               ]
+               "users" => [user_response(user)]
              }
     end
 
@@ -992,51 +817,14 @@ test "only local users with no query", %{conn: conn, admin: old_admin} do
 
       users =
         [
-          %{
-            "deactivated" => user.deactivated,
-            "id" => user.id,
-            "nickname" => user.nickname,
-            "roles" => %{"admin" => false, "moderator" => false},
-            "local" => true,
-            "tags" => [],
-            "avatar" => User.avatar_url(user) |> MediaProxy.url(),
-            "display_name" => HTML.strip_tags(user.name || user.nickname),
-            "confirmation_pending" => false,
-            "approval_pending" => false,
-            "url" => user.ap_id,
-            "registration_reason" => nil,
-            "actor_type" => "Person"
-          },
-          %{
-            "deactivated" => admin.deactivated,
-            "id" => admin.id,
-            "nickname" => admin.nickname,
-            "roles" => %{"admin" => true, "moderator" => false},
-            "local" => true,
-            "tags" => [],
-            "avatar" => User.avatar_url(admin) |> MediaProxy.url(),
-            "display_name" => HTML.strip_tags(admin.name || admin.nickname),
-            "confirmation_pending" => false,
-            "approval_pending" => false,
-            "url" => admin.ap_id,
-            "registration_reason" => nil,
-            "actor_type" => "Person"
-          },
-          %{
+          user_response(user),
+          user_response(admin, %{
+            "roles" => %{"admin" => true, "moderator" => false}
+          }),
+          user_response(old_admin, %{
             "deactivated" => false,
-            "id" => old_admin.id,
-            "local" => true,
-            "nickname" => old_admin.nickname,
-            "roles" => %{"admin" => true, "moderator" => false},
-            "tags" => [],
-            "avatar" => User.avatar_url(old_admin) |> MediaProxy.url(),
-            "display_name" => HTML.strip_tags(old_admin.name || old_admin.nickname),
-            "confirmation_pending" => false,
-            "approval_pending" => false,
-            "url" => old_admin.ap_id,
-            "registration_reason" => nil,
-            "actor_type" => "Person"
-          }
+            "roles" => %{"admin" => true, "moderator" => false}
+          })
         ]
         |> Enum.sort_by(& &1["nickname"])
 
@@ -1047,6 +835,30 @@ test "only local users with no query", %{conn: conn, admin: old_admin} do
              }
     end
 
+    test "only unconfirmed users", %{conn: conn} do
+      sad_user = insert(:user, nickname: "sadboy", confirmation_pending: true)
+      old_user = insert(:user, nickname: "oldboy", confirmation_pending: true)
+
+      insert(:user, nickname: "happyboy", approval_pending: false)
+      insert(:user, confirmation_pending: false)
+
+      result =
+        conn
+        |> get("/api/pleroma/admin/users?filters=need_confirmed")
+        |> json_response(200)
+
+      users =
+        Enum.map([old_user, sad_user], fn user ->
+          user_response(user, %{
+            "confirmation_pending" => true,
+            "approval_pending" => false
+          })
+        end)
+        |> Enum.sort_by(& &1["nickname"])
+
+      assert result == %{"count" => 2, "page_size" => 50, "users" => users}
+    end
+
     test "only unapproved users", %{conn: conn} do
       user =
         insert(:user,
@@ -1175,21 +987,22 @@ test "load users with tags list", %{conn: conn} do
 
       users =
         [
-          %{
-            "deactivated" => false,
-            "id" => user1.id,
-            "nickname" => user1.nickname,
-            "roles" => %{"admin" => false, "moderator" => false},
-            "local" => user1.local,
-            "tags" => ["first"],
-            "avatar" => User.avatar_url(user1) |> MediaProxy.url(),
-            "display_name" => HTML.strip_tags(user1.name || user1.nickname),
-            "confirmation_pending" => false,
-            "approval_pending" => false,
-            "url" => user1.ap_id,
-            "registration_reason" => nil,
-            "actor_type" => "Person"
-          },
+          user_response(
+            user1,
+            %{
+              "deactivated" => false,
+              "roles" => %{"admin" => false, "moderator" => false},
+              "local" => user1.local,
+              "tags" => ["first"],
+              "avatar" => User.avatar_url(user1) |> MediaProxy.url(),
+              "display_name" => HTML.strip_tags(user1.name || user1.nickname),
+              "confirmation_pending" => false,
+              "approval_pending" => false,
+              "url" => user1.ap_id,
+              "registration_reason" => nil,
+              "actor_type" => "Person"
+            }
+          ),
           %{
             "deactivated" => false,
             "id" => user2.id,
@@ -1253,23 +1066,7 @@ test "it works with multiple filters" do
       assert json_response(conn, 200) == %{
                "count" => 1,
                "page_size" => 50,
-               "users" => [
-                 %{
-                   "deactivated" => user.deactivated,
-                   "id" => user.id,
-                   "nickname" => user.nickname,
-                   "roles" => %{"admin" => false, "moderator" => false},
-                   "local" => user.local,
-                   "tags" => [],
-                   "avatar" => User.avatar_url(user) |> MediaProxy.url(),
-                   "display_name" => HTML.strip_tags(user.name || user.nickname),
-                   "confirmation_pending" => false,
-                   "approval_pending" => false,
-                   "url" => user.ap_id,
-                   "registration_reason" => nil,
-                   "actor_type" => "Person"
-                 }
-               ]
+               "users" => [user_response(user)]
              }
     end
 
@@ -1282,21 +1079,7 @@ test "it omits relay user", %{admin: admin, conn: conn} do
                "count" => 1,
                "page_size" => 50,
                "users" => [
-                 %{
-                   "deactivated" => admin.deactivated,
-                   "id" => admin.id,
-                   "nickname" => admin.nickname,
-                   "roles" => %{"admin" => true, "moderator" => false},
-                   "local" => true,
-                   "tags" => [],
-                   "avatar" => User.avatar_url(admin) |> MediaProxy.url(),
-                   "display_name" => HTML.strip_tags(admin.name || admin.nickname),
-                   "confirmation_pending" => false,
-                   "approval_pending" => false,
-                   "url" => admin.ap_id,
-                   "registration_reason" => nil,
-                   "actor_type" => "Person"
-                 }
+                 user_response(admin, %{"roles" => %{"admin" => true, "moderator" => false}})
                ]
              }
     end
@@ -1368,21 +1151,10 @@ test "PATCH /api/pleroma/admin/users/:nickname/toggle_activation", %{admin: admi
     conn = patch(conn, "/api/pleroma/admin/users/#{user.nickname}/toggle_activation")
 
     assert json_response(conn, 200) ==
-             %{
-               "deactivated" => !user.deactivated,
-               "id" => user.id,
-               "nickname" => user.nickname,
-               "roles" => %{"admin" => false, "moderator" => false},
-               "local" => true,
-               "tags" => [],
-               "avatar" => User.avatar_url(user) |> MediaProxy.url(),
-               "display_name" => HTML.strip_tags(user.name || user.nickname),
-               "confirmation_pending" => false,
-               "approval_pending" => false,
-               "url" => user.ap_id,
-               "registration_reason" => nil,
-               "actor_type" => "Person"
-             }
+             user_response(
+               user,
+               %{"deactivated" => !user.deactivated}
+             )
 
     log_entry = Repo.one(ModerationLog)
 
@@ -2024,6 +1796,25 @@ test "by instance", %{conn: conn} do
                response["status_visibility"]
     end
   end
+
+  defp user_response(user, attrs \\ %{}) do
+    %{
+      "deactivated" => user.deactivated,
+      "id" => user.id,
+      "nickname" => user.nickname,
+      "roles" => %{"admin" => false, "moderator" => false},
+      "local" => user.local,
+      "tags" => [],
+      "avatar" => User.avatar_url(user) |> MediaProxy.url(),
+      "display_name" => HTML.strip_tags(user.name || user.nickname),
+      "confirmation_pending" => false,
+      "approval_pending" => false,
+      "url" => user.ap_id,
+      "registration_reason" => nil,
+      "actor_type" => "Person"
+    }
+    |> Map.merge(attrs)
+  end
 end
 
 # Needed for testing
diff --git a/test/pleroma/web/admin_api/search_test.exs b/test/pleroma/web/admin_api/search_test.exs
index ceec64f1e..27ca396e6 100644
--- a/test/pleroma/web/admin_api/search_test.exs
+++ b/test/pleroma/web/admin_api/search_test.exs
@@ -178,6 +178,17 @@ test "it returns unapproved user" do
       assert count == 1
     end
 
+    test "it returns unconfirmed user" do
+      unconfirmed = insert(:user, confirmation_pending: true)
+      insert(:user)
+      insert(:user)
+
+      {:ok, _results, total} = Search.user()
+      {:ok, [^unconfirmed], count} = Search.user(%{need_confirmed: true})
+      assert total == 3
+      assert count == 1
+    end
+
     test "it returns non-discoverable users" do
       insert(:user)
       insert(:user, is_discoverable: false)