Remove toggle_confirmation; require explicit state change
authorMark Felder <feld@FreeBSD.org>
Tue, 13 Oct 2020 21:07:36 +0000 (16:07 -0500)
committerMark Felder <feld@feld.me>
Fri, 15 Jan 2021 19:11:51 +0000 (13:11 -0600)
Also cosmetic changes to make the code clearer

lib/mix/tasks/pleroma/user.ex
lib/pleroma/user.ex
lib/pleroma/web/auth/pleroma_authenticator.ex
priv/repo/migrations/20201231185546_confirm_logged_in_users.exs
test/mix/tasks/pleroma/user_test.exs
test/pleroma/repo/migrations/confirm_logged_in_users_test.exs
test/pleroma/user_test.exs
test/pleroma/web/admin_api/controllers/admin_api_controller_test.exs
test/pleroma/web/o_auth/o_auth_controller_test.exs
test/pleroma/web/pleroma_api/controllers/account_controller_test.exs
test/pleroma/web/twitter_api/controller_test.exs

index a397d17488d8ab48c3a95e370b6e17e358970706..e87f1c271d157e0051be05dd98d75e970694dfba 100644 (file)
@@ -213,7 +213,7 @@ defmodule Mix.Tasks.Pleroma.User do
       user =
         case Keyword.get(options, :confirmed) do
           nil -> user
-          value -> set_confirmed(user, value)
+          value -> set_confirmation(user, value)
         end
 
       user =
@@ -373,7 +373,7 @@ defmodule Mix.Tasks.Pleroma.User do
     |> Pleroma.Repo.chunk_stream(500, :batches)
     |> Stream.each(fn users ->
       users
-      |> Enum.each(fn user -> User.need_confirmation(user, false) end)
+      |> Enum.each(fn user -> User.set_confirmation(user, true) end)
     end)
     |> Stream.run()
   end
@@ -391,7 +391,7 @@ defmodule Mix.Tasks.Pleroma.User do
     |> Pleroma.Repo.chunk_stream(500, :batches)
     |> Stream.each(fn users ->
       users
-      |> Enum.each(fn user -> User.need_confirmation(user, true) end)
+      |> Enum.each(fn user -> User.set_confirmation(user, false) end)
     end)
     |> Stream.run()
   end
@@ -454,8 +454,8 @@ defmodule Mix.Tasks.Pleroma.User do
     user
   end
 
-  defp set_confirmed(user, value) do
-    {:ok, user} = User.need_confirmation(user, !value)
+  defp set_confirmation(user, value) do
+    {:ok, user} = User.set_confirmation(user, value)
 
     shell_info("Confirmation status of #{user.nickname}: #{user.is_confirmed}")
     user
index 04ce1768d0a707db8209b41d47646caa400871c0..9efc278876a57ed016cf93bfce6eaa3c6c95eb38 100644 (file)
@@ -704,11 +704,11 @@ defmodule Pleroma.User do
     reason_limit = Config.get([:instance, :registration_reason_length], 500)
     params = Map.put_new(params, :accepts_chat_messages, true)
 
-    need_confirmation? =
-      if is_nil(opts[:need_confirmation]) do
-        Config.get([:instance, :account_activation_required])
+    confirmed? =
+      if is_nil(opts[:confirmed]) do
+        !Config.get([:instance, :account_activation_required])
       else
-        opts[:need_confirmation]
+        opts[:confirmed]
       end
 
     need_approval? =
@@ -719,7 +719,7 @@ defmodule Pleroma.User do
       end
 
     struct
-    |> confirmation_changeset(need_confirmation: need_confirmation?)
+    |> confirmation_changeset(set_confirmation: confirmed?)
     |> approval_changeset(need_approval: need_approval?)
     |> cast(params, [
       :bio,
@@ -1643,7 +1643,7 @@ defmodule Pleroma.User do
   end
 
   def confirm(%User{is_confirmed: false} = user) do
-    with chg <- confirmation_changeset(user, need_confirmation: false),
+    with chg <- confirmation_changeset(user, set_confirmation: true),
          {:ok, user} <- update_and_set_cache(chg) do
       post_register_action(user)
       {:ok, user}
@@ -2138,10 +2138,10 @@ defmodule Pleroma.User do
     updated_user
   end
 
-  @spec need_confirmation(User.t(), boolean()) :: {:ok, User.t()} | {:error, Changeset.t()}
-  def need_confirmation(%User{} = user, bool) do
+  @spec set_confirmation(User.t(), boolean()) :: {:ok, User.t()} | {:error, Changeset.t()}
+  def set_confirmation(%User{} = user, bool) do
     user
-    |> confirmation_changeset(need_confirmation: bool)
+    |> confirmation_changeset(set_confirmation: bool)
     |> update_and_set_cache()
   end
 
@@ -2309,17 +2309,17 @@ defmodule Pleroma.User do
   end
 
   @spec confirmation_changeset(User.t(), keyword()) :: Changeset.t()
-  def confirmation_changeset(user, need_confirmation: need_confirmation?) do
+  def confirmation_changeset(user, set_confirmation: confirmed?) do
     params =
-      if need_confirmation? do
+      if confirmed? do
         %{
-          is_confirmed: false,
-          confirmation_token: :crypto.strong_rand_bytes(32) |> Base.url_encode64()
+          is_confirmed: true,
+          confirmation_token: nil
         }
       else
         %{
-          is_confirmed: true,
-          confirmation_token: nil
+          is_confirmed: false,
+          confirmation_token: :crypto.strong_rand_bytes(32) |> Base.url_encode64()
         }
       end
 
index a2121e6a791fe7858837952427c160920bc4786c..401f23c9f683f2e0617eff976e865e2e0a88b8bd 100644 (file)
@@ -84,7 +84,7 @@ defmodule Pleroma.Web.Auth.PleromaAuthenticator do
                password_confirmation: random_password
              },
              external: true,
-             need_confirmation: false
+             confirmed: true
            )
            |> Repo.insert(),
          {:ok, _} <-
index 4372d093f7bb37107c5eb42cff93e3473bb0fd78..b9656c17bf39f80d3946d86495a6a5a240d4a451 100644 (file)
@@ -11,9 +11,9 @@ defmodule Pleroma.Repo.Migrations.ConfirmLoggedInUsers do
 
   def up do
     User
-    |> where([u], u.confirmation_pending == true)
+    |> where([u], u.is_confirmed == false)
     |> join(:inner, [u], t in Token, on: t.user_id == u.id)
-    |> Repo.update_all(set: [confirmation_pending: false])
+    |> Repo.update_all(set: [is_confirmed: true])
   end
 
   def down do
index a620e5960253dc47a291e4a8662d7bb4affd0401..2b52322835dc9b9e684b82829b703426ed181c5d 100644 (file)
@@ -436,13 +436,6 @@ defmodule Mix.Tasks.Pleroma.UserTest do
       assert_received {:mix_shell, :info, [message]}
       assert message =~ "Invite for token #{invite.token} was revoked."
     end
-
-    test "it prints an error message when invite is not exist" do
-      Mix.Tasks.Pleroma.User.run(["revoke_invite", "foo"])
-
-      assert_received {:mix_shell, :error, [message]}
-      assert message =~ "No invite found"
-    end
   end
 
   describe "running delete_activities" do
index b30faa257d724768fc4f0e42a44724bf78775410..99d17f62a5253301748a7289a7f1e3fd3c732479 100644 (file)
@@ -14,12 +14,12 @@ defmodule Pleroma.Repo.Migrations.ConfirmLoggedInUsersTest do
 
   test "up/0 confirms unconfirmed but previously-logged-in users", %{migration: migration} do
     insert_list(25, :oauth_token)
-    Repo.update_all(User, set: [confirmation_pending: true])
-    insert_list(5, :user, confirmation_pending: true)
+    Repo.update_all(User, set: [is_confirmed: false])
+    insert_list(5, :user, is_confirmed: false)
 
     count =
       User
-      |> where(confirmation_pending: true)
+      |> where(is_confirmed: false)
       |> Repo.aggregate(:count)
 
     assert count == 30
@@ -28,7 +28,7 @@ defmodule Pleroma.Repo.Migrations.ConfirmLoggedInUsersTest do
 
     count =
       User
-      |> where(confirmation_pending: true)
+      |> where(is_confirmed: false)
       |> Repo.aggregate(:count)
 
     assert count == 5
index 7e50d53d35565efd03c46e5d7bf3f47a70b61dda..ac9db9ffb53ffd62b4477ec0c0a1a0a70db35635 100644 (file)
@@ -666,7 +666,7 @@ defmodule Pleroma.UserTest do
     end
 
     test "it creates confirmed user if :confirmed option is given" do
-      changeset = User.register_changeset(%User{}, @full_user_data, need_confirmation: false)
+      changeset = User.register_changeset(%User{}, @full_user_data, confirmed: true)
       assert changeset.valid?
 
       {:ok, user} = Repo.insert(changeset)
index 65f2a124f8e0b436809f93741aa6ffe75d2b7db2..23e4bc3af585ea12076ba0254e71d9a84f717bf0 100644 (file)
@@ -906,8 +906,11 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do
 
       assert ret_conn.status == 200
 
-      assert User.get_by_id(first_user.id).is_confirmed
-      assert User.get_by_id(second_user.id).is_confirmed
+      first_user = User.get_by_id(first_user.id)
+      second_user = User.get_by_id(second_user.id)
+
+      assert first_user.is_confirmed
+      assert second_user.is_confirmed
 
       log_entry = Repo.one(ModerationLog)
 
index 254d4e9a4fdd0756959789febe1329a31193a72b..337d2650ce89b4b71f82a690d0ab6b04f28d1765 100644 (file)
@@ -928,7 +928,7 @@ defmodule Pleroma.Web.OAuth.OAuthControllerTest do
 
       {:ok, user} =
         insert(:user, password_hash: Pleroma.Password.Pbkdf2.hash_pwd_salt(password))
-        |> User.confirmation_changeset(need_confirmation: true)
+        |> User.confirmation_changeset(set_confirmation: false)
         |> User.update_and_set_cache()
 
       refute Pleroma.User.account_status(user) == :active
index 668bb46fb3f6d05e836d24ad395283a2d7b7faab..9f14c55770b38c3d54a6919f047848051fe48bef 100644 (file)
@@ -17,7 +17,7 @@ defmodule Pleroma.Web.PleromaAPI.AccountControllerTest do
     setup do
       {:ok, user} =
         insert(:user)
-        |> User.confirmation_changeset(need_confirmation: true)
+        |> User.confirmation_changeset(set_confirmation: false)
         |> User.update_and_set_cache()
 
       refute user.is_confirmed
index 8f29a4f63862b884c4e68c9af243facfb579ba57..583c904b2e18ac1fb059dd4c2db4a24dff0fb197 100644 (file)
@@ -64,7 +64,7 @@ defmodule Pleroma.Web.TwitterAPI.ControllerTest do
     setup do
       {:ok, user} =
         insert(:user)
-        |> User.confirmation_changeset(need_confirmation: true)
+        |> User.confirmation_changeset(set_confirmation: false)
         |> Repo.update()
 
       refute user.is_confirmed