From f7e59c28ed2d4693ce177737e3878b606f1b5848 Mon Sep 17 00:00:00 2001 From: Mark Felder Date: Fri, 16 Oct 2020 21:44:25 +0000 Subject: [PATCH] Change user.approval_pending field to user.is_approved --- lib/pleroma/user.ex | 30 ++++++++-------- lib/pleroma/user/query.ex | 4 +-- .../web/admin_api/views/account_view.ex | 2 +- ...0_refactor_approval_pending_user_field.exs | 20 +++++++++++ test/pleroma/user_test.exs | 36 +++++++++---------- .../controllers/user_controller_test.exs | 26 +++++++------- test/pleroma/web/admin_api/search_test.exs | 2 +- .../controllers/account_controller_test.exs | 4 +-- .../web/o_auth/o_auth_controller_test.exs | 2 +- .../web/twitter_api/twitter_api_test.exs | 2 +- 10 files changed, 74 insertions(+), 54 deletions(-) create mode 100644 priv/repo/migrations/20201016205220_refactor_approval_pending_user_field.exs diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 6a81adfd6..83a37890a 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -112,7 +112,7 @@ defmodule Pleroma.User do field(:is_locked, :boolean, default: false) field(:confirmation_pending, :boolean, default: false) field(:password_reset_pending, :boolean, default: false) - field(:approval_pending, :boolean, default: false) + field(:is_approved, :boolean, default: true) field(:registration_reason, :string, default: nil) field(:confirmation_token, :string, default: nil) field(:default_scope, :string, default: "public") @@ -288,7 +288,7 @@ def binary_id(%User{} = user), do: binary_id(user.id) @spec account_status(User.t()) :: account_status() def account_status(%User{deactivated: true}), do: :deactivated def account_status(%User{password_reset_pending: true}), do: :password_reset_pending - def account_status(%User{local: true, approval_pending: true}), do: :approval_pending + def account_status(%User{local: true, is_approved: false}), do: :approval_pending def account_status(%User{local: true, confirmation_pending: true}) do if Config.get([:instance, :account_activation_required]) do @@ -711,16 +711,16 @@ def register_changeset(struct, params \\ %{}, opts \\ []) do opts[:need_confirmation] end - need_approval? = - if is_nil(opts[:need_approval]) do - Config.get([:instance, :account_approval_required]) + approved? = + if is_nil(opts[:approved]) do + !Config.get([:instance, :account_approval_required]) else - opts[:need_approval] + opts[:approved] end struct |> confirmation_changeset(need_confirmation: need_confirmation?) - |> approval_changeset(need_approval: need_approval?) + |> approval_changeset(set_approval: approved?) |> cast(params, [ :bio, :raw_bio, @@ -814,14 +814,14 @@ def post_register_action(%User{confirmation_pending: true} = user) do end end - def post_register_action(%User{approval_pending: true} = user) do + def post_register_action(%User{is_approved: false} = user) do with {:ok, _} <- send_user_approval_email(user), {:ok, _} <- send_admin_approval_emails(user) do {:ok, user} end end - def post_register_action(%User{approval_pending: false, confirmation_pending: false} = user) do + def post_register_action(%User{is_approved: true, confirmation_pending: false} = user) do with {:ok, user} <- autofollow_users(user), {:ok, _} <- autofollowing_users(user), {:ok, user} <- set_cache(user), @@ -1624,8 +1624,8 @@ def approve(users) when is_list(users) do end) end - def approve(%User{approval_pending: true} = user) do - with chg <- change(user, approval_pending: false), + def approve(%User{is_approved: false} = user) do + with chg <- change(user, is_approved: true), {:ok, user} <- update_and_set_cache(chg) do post_register_action(user) {:ok, user} @@ -1684,7 +1684,7 @@ def purge_user_changeset(user) do is_locked: false, confirmation_pending: false, password_reset_pending: false, - approval_pending: false, + is_approved: true, registration_reason: nil, confirmation_token: nil, domain_blocks: [], @@ -2327,9 +2327,9 @@ def confirmation_changeset(user, need_confirmation: need_confirmation?) do end @spec approval_changeset(User.t(), keyword()) :: Changeset.t() - def approval_changeset(user, need_approval: need_approval?) do - params = if need_approval?, do: %{approval_pending: true}, else: %{approval_pending: false} - cast(user, params, [:approval_pending]) + def approval_changeset(user, set_approval: approved?) do + params = if approved?, do: %{is_approved: true}, else: %{is_approved: false} + cast(user, params, [:is_approved]) end def add_pinnned_activity(user, %Pleroma.Activity{id: id}) do diff --git a/lib/pleroma/user/query.ex b/lib/pleroma/user/query.ex index ab9554bd2..90548677f 100644 --- a/lib/pleroma/user/query.ex +++ b/lib/pleroma/user/query.ex @@ -138,7 +138,7 @@ defp compose_query({:external, _}, query), do: location_query(query, false) defp compose_query({:active, _}, query) do User.restrict_deactivated(query) - |> where([u], u.approval_pending == false) + |> where([u], u.is_approved == true) end defp compose_query({:legacy_active, _}, query) do @@ -159,7 +159,7 @@ defp compose_query({:confirmation_pending, bool}, query) do end defp compose_query({:need_approval, _}, query) do - where(query, [u], u.approval_pending) + where(query, [u], u.is_approved == false) end defp compose_query({:unconfirmed, _}, query) do diff --git a/lib/pleroma/web/admin_api/views/account_view.ex b/lib/pleroma/web/admin_api/views/account_view.ex index 37188bfeb..1a876d272 100644 --- a/lib/pleroma/web/admin_api/views/account_view.ex +++ b/lib/pleroma/web/admin_api/views/account_view.ex @@ -78,7 +78,7 @@ def render("show.json", %{user: user}) do "roles" => User.roles(user), "tags" => user.tags || [], "confirmation_pending" => user.confirmation_pending, - "approval_pending" => user.approval_pending, + "is_approved" => user.is_approved, "url" => user.uri || user.ap_id, "registration_reason" => user.registration_reason, "actor_type" => user.actor_type diff --git a/priv/repo/migrations/20201016205220_refactor_approval_pending_user_field.exs b/priv/repo/migrations/20201016205220_refactor_approval_pending_user_field.exs new file mode 100644 index 000000000..944dcf8de --- /dev/null +++ b/priv/repo/migrations/20201016205220_refactor_approval_pending_user_field.exs @@ -0,0 +1,20 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2020 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Repo.Migrations.RefactorApprovalPendingUserField do + use Ecto.Migration + + def up do + # Flip the values before we change the meaning of the column + execute("UPDATE users SET approval_pending = NOT approval_pending;") + execute("ALTER TABLE users RENAME COLUMN approval_pending TO is_approved;") + execute("ALTER TABLE users ALTER COLUMN is_approved SET DEFAULT true;") + end + + def down do + execute("UPDATE users SET is_approved = NOT is_approved;") + execute("ALTER TABLE users RENAME COLUMN is_approved TO approval_pending;") + execute("ALTER TABLE users ALTER COLUMN approval_pending SET DEFAULT false;") + end +end diff --git a/test/pleroma/user_test.exs b/test/pleroma/user_test.exs index bdf17e96a..c6d8e1463 100644 --- a/test/pleroma/user_test.exs +++ b/test/pleroma/user_test.exs @@ -694,7 +694,7 @@ test "it creates unapproved user" do {:ok, user} = Repo.insert(changeset) - assert user.approval_pending + refute user.is_approved assert user.registration_reason == "I'm a cool guy :)" end @@ -1388,17 +1388,17 @@ test "hide a user's statuses from timelines and notifications" do describe "approve" do test "approves a user" do - user = insert(:user, approval_pending: true) - assert true == user.approval_pending + user = insert(:user, is_approved: false) + refute user.is_approved {:ok, user} = User.approve(user) - assert false == user.approval_pending + assert user.is_approved end test "approves a list of users" do unapproved_users = [ - insert(:user, approval_pending: true), - insert(:user, approval_pending: true), - insert(:user, approval_pending: true) + insert(:user, is_approved: false), + insert(:user, is_approved: false), + insert(:user, is_approved: false) ] {:ok, users} = User.approve(unapproved_users) @@ -1406,7 +1406,7 @@ test "approves a list of users" do assert Enum.count(users) == 3 Enum.each(users, fn user -> - assert false == user.approval_pending + assert user.is_approved end) end @@ -1414,7 +1414,7 @@ test "it sends welcome email if it is set" do clear_config([:welcome, :email, :enabled], true) clear_config([:welcome, :email, :sender], "tester@test.me") - user = insert(:user, approval_pending: true) + user = insert(:user, is_approved: false) welcome_user = insert(:user, email: "tester@test.me") instance_name = Pleroma.Config.get([:instance, :name]) @@ -1432,7 +1432,7 @@ test "it sends welcome email if it is set" do test "approving an approved user does not trigger post-register actions" do clear_config([:welcome, :email, :enabled], true) - user = insert(:user, approval_pending: false) + user = insert(:user, is_approved: true) User.approve(user) ObanHelpers.perform_all() @@ -1465,9 +1465,9 @@ test "confirms a list of users" do end) end - test "sends approval emails when `approval_pending: true`" do + test "sends approval emails when `is_approved: false`" do admin = insert(:user, is_admin: true) - user = insert(:user, confirmation_pending: true, approval_pending: true) + user = insert(:user, confirmation_pending: true, is_approved: false) User.confirm(user) ObanHelpers.perform_all() @@ -1494,7 +1494,7 @@ test "sends approval emails when `approval_pending: true`" do end test "confirming a confirmed user does not trigger post-register actions" do - user = insert(:user, confirmation_pending: false, approval_pending: true) + user = insert(:user, confirmation_pending: false, is_approved: false) User.confirm(user) ObanHelpers.perform_all() @@ -1591,7 +1591,7 @@ test "deactivates user when activation is not required", %{user: user} do end test "delete/1 when approval is pending deletes the user" do - user = insert(:user, approval_pending: true) + user = insert(:user, is_approved: false) {:ok, job} = User.delete(user) {:ok, _} = ObanHelpers.perform(job) @@ -1618,7 +1618,7 @@ test "delete/1 purges a user when they wouldn't be fully deleted" do is_locked: true, confirmation_pending: true, password_reset_pending: true, - approval_pending: true, + is_approved: false, registration_reason: "ahhhhh", confirmation_token: "qqqq", domain_blocks: ["lain.com"], @@ -1660,7 +1660,7 @@ test "delete/1 purges a user when they wouldn't be fully deleted" do is_locked: false, confirmation_pending: false, password_reset_pending: false, - approval_pending: false, + is_approved: true, registration_reason: nil, confirmation_token: nil, domain_blocks: [], @@ -1755,10 +1755,10 @@ test "returns :deactivated for deactivated user" do end test "returns :approval_pending for unapproved user" do - user = insert(:user, local: true, approval_pending: true) + user = insert(:user, local: true, is_approved: false) assert User.account_status(user) == :approval_pending - user = insert(:user, local: true, confirmation_pending: true, approval_pending: true) + user = insert(:user, local: true, confirmation_pending: true, is_approved: false) assert User.account_status(user) == :approval_pending end end diff --git a/test/pleroma/web/admin_api/controllers/user_controller_test.exs b/test/pleroma/web/admin_api/controllers/user_controller_test.exs index 40d39aae7..ed094823b 100644 --- a/test/pleroma/web/admin_api/controllers/user_controller_test.exs +++ b/test/pleroma/web/admin_api/controllers/user_controller_test.exs @@ -429,7 +429,7 @@ test "allows to force-unfollow another user", %{admin: admin, conn: conn} do describe "GET /api/pleroma/admin/users" do test "renders users array for the first page", %{conn: conn, admin: admin} do user = insert(:user, local: false, tags: ["foo", "bar"]) - user2 = insert(:user, approval_pending: true, registration_reason: "I'm a chill dude") + user2 = insert(:user, is_approved: false, registration_reason: "I'm a chill dude") conn = get(conn, "/api/pleroma/admin/users?page=1") @@ -444,7 +444,7 @@ test "renders users array for the first page", %{conn: conn, admin: admin} do user2, %{ "local" => true, - "approval_pending" => true, + "is_approved" => false, "registration_reason" => "I'm a chill dude", "actor_type" => "Person" } @@ -638,7 +638,7 @@ 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, nickname: "happyboy", is_approved: true) insert(:user, confirmation_pending: false) result = @@ -650,7 +650,7 @@ test "only unconfirmed users", %{conn: conn} do Enum.map([old_user, sad_user], fn user -> user_response(user, %{ "confirmation_pending" => true, - "approval_pending" => false + "is_approved" => true }) end) |> Enum.sort_by(& &1["nickname"]) @@ -662,18 +662,18 @@ test "only unapproved users", %{conn: conn} do user = insert(:user, nickname: "sadboy", - approval_pending: true, + is_approved: false, registration_reason: "Plz let me in!" ) - insert(:user, nickname: "happyboy", approval_pending: false) + insert(:user, nickname: "happyboy", is_approved: true) conn = get(conn, "/api/pleroma/admin/users?filters=need_approval") users = [ user_response( user, - %{"approval_pending" => true, "registration_reason" => "Plz let me in!"} + %{"is_approved" => false, "registration_reason" => "Plz let me in!"} ) ] @@ -816,8 +816,8 @@ test "load users with tags list", %{conn: conn} do end test "`active` filters out users pending approval", %{token: token} do - insert(:user, approval_pending: true) - %{id: user_id} = insert(:user, approval_pending: false) + insert(:user, is_approved: false) + %{id: user_id} = insert(:user, is_approved: true) %{id: admin_id} = token.user conn = @@ -913,8 +913,8 @@ test "PATCH /api/pleroma/admin/users/deactivate", %{admin: admin, conn: conn} do end test "PATCH /api/pleroma/admin/users/approve", %{admin: admin, conn: conn} do - user_one = insert(:user, approval_pending: true) - user_two = insert(:user, approval_pending: true) + user_one = insert(:user, is_approved: false) + user_two = insert(:user, is_approved: false) conn = patch( @@ -924,7 +924,7 @@ test "PATCH /api/pleroma/admin/users/approve", %{admin: admin, conn: conn} do ) response = json_response(conn, 200) - assert Enum.map(response["users"], & &1["approval_pending"]) == [false, false] + assert Enum.map(response["users"], & &1["is_approved"]) == [true, true] log_entry = Repo.one(ModerationLog) @@ -961,7 +961,7 @@ defp user_response(user, attrs \\ %{}) do "avatar" => User.avatar_url(user) |> MediaProxy.url(), "display_name" => HTML.strip_tags(user.name || user.nickname), "confirmation_pending" => false, - "approval_pending" => false, + "is_approved" => true, "url" => user.ap_id, "registration_reason" => nil, "actor_type" => "Person" diff --git a/test/pleroma/web/admin_api/search_test.exs b/test/pleroma/web/admin_api/search_test.exs index 307578ae0..10ec227d6 100644 --- a/test/pleroma/web/admin_api/search_test.exs +++ b/test/pleroma/web/admin_api/search_test.exs @@ -182,7 +182,7 @@ test "it returns user by email" do end test "it returns unapproved user" do - unapproved = insert(:user, approval_pending: true) + unapproved = insert(:user, is_approved: false) insert(:user) insert(:user) diff --git a/test/pleroma/web/mastodon_api/controllers/account_controller_test.exs b/test/pleroma/web/mastodon_api/controllers/account_controller_test.exs index 7b3cc7344..1c310bb07 100644 --- a/test/pleroma/web/mastodon_api/controllers/account_controller_test.exs +++ b/test/pleroma/web/mastodon_api/controllers/account_controller_test.exs @@ -1028,7 +1028,7 @@ test "registers and logs in without :account_activation_required / :account_appr assert user refute user.confirmation_pending - refute user.approval_pending + assert user.is_approved end test "registers but does not log in with :account_activation_required", %{conn: conn} do @@ -1150,7 +1150,7 @@ test "registers but does not log in with :account_approval_required", %{conn: co user = Repo.get_by(User, email: "lain@example.org") - assert user.approval_pending + refute user.is_approved assert user.registration_reason == "I'm a cool dude, bro" end diff --git a/test/pleroma/web/o_auth/o_auth_controller_test.exs b/test/pleroma/web/o_auth/o_auth_controller_test.exs index bf47afed8..646d62da4 100644 --- a/test/pleroma/web/o_auth/o_auth_controller_test.exs +++ b/test/pleroma/web/o_auth/o_auth_controller_test.exs @@ -1041,7 +1041,7 @@ test "rejects token exchange for valid credentials belonging to an unapproved us user = insert(:user, password_hash: Pleroma.Password.Pbkdf2.hash_pwd_salt(password), - approval_pending: true + is_approved: false ) refute Pleroma.User.account_status(user) == :active diff --git a/test/pleroma/web/twitter_api/twitter_api_test.exs b/test/pleroma/web/twitter_api/twitter_api_test.exs index 3be4812d3..05152ee21 100644 --- a/test/pleroma/web/twitter_api/twitter_api_test.exs +++ b/test/pleroma/web/twitter_api/twitter_api_test.exs @@ -97,7 +97,7 @@ test "it sends an admin email if :account_approval_required is specified in inst {:ok, user} = TwitterAPI.register_user(data) ObanHelpers.perform_all() - assert user.approval_pending + refute user.is_approved user_email = Pleroma.Emails.UserEmail.approval_pending_email(user) admin_email = Pleroma.Emails.AdminEmail.new_unapproved_registration(admin, user)