More test stuff

Signed-off-by: Thomas Citharel <tcit@tcit.fr>

lib/mobilizon/actors/actors.ex

@@ -9,6 +9,7 @@ defmodule Mobilizon.Actors do
   alias Mobilizon.Actors.{Actor, Bot, Member, Follower, User}
 
   alias Mobilizon.Service.ActivityPub
+  import Exgravatar
 
   @doc false
   def data() do
@@ -58,19 +59,12 @@ defmodule Mobilizon.Actors do
   """
   @spec get_actor_for_user(Mobilizon.Actors.User.t()) :: Mobilizon.Actors.Actor.t()
   def get_actor_for_user(%Mobilizon.Actors.User{} = user) do
-    case user.default_actor_id do
-      nil -> get_first_actor_for_user(user)
-      actor_id -> get_actor!(actor_id)
+    with %User{default_actor: actor} = user when not is_nil(user) and not is_nil(actor) <-
+           Repo.preload(user, [:default_actor]) do
+      actor
     end
   end
 
-  # Returns the first actor found for an user
-  # Useful when the user has not defined default actor
-  # Raises `Ecto.NoResultsError` if no Actor is found for this ID
-  defp get_first_actor_for_user(%Mobilizon.Actors.User{id: id} = _user) do
-    Repo.one!(from(a in Actor, where: a.user_id == ^id))
-  end
-
   def get_actor_with_everything!(id) do
     actor = Repo.get!(Actor, id)
     Repo.preload(actor, [:organized_events, :followers, :followings])
@@ -496,37 +490,26 @@ defmodule Mobilizon.Actors do
   @doc """
   Register user
   """
+  @spec register(map()) :: {:ok, Actor.t()} | {:error, String.t()}
   def register(%{email: email, password: password, username: username}) do
     key = :public_key.generate_key({:rsa, 2048, 65_537})
     entry = :public_key.pem_entry_encode(:RSAPrivateKey, key)
     pem = [entry] |> :public_key.pem_encode() |> String.trim_trailing()
 
-    import Exgravatar
-
-    avatar_url = gravatar_url(email, default: "404")
-
-    avatar =
-      case HTTPoison.get(avatar_url) do
-        {:ok, %HTTPoison.Response{status_code: 200}} ->
-          avatar_url
-
-        _ ->
-          nil
-      end
-
-    with actor_changeset <-
+    with avatar <- gravatar(email),
+         actor_changeset <-
            Mobilizon.Actors.Actor.registration_changeset(%Mobilizon.Actors.Actor{}, %{
              preferred_username: username,
              domain: nil,
              keys: pem,
              avatar_url: avatar
            }),
-         {:ok, %Mobilizon.Actors.Actor{id: id} = actor} <- Mobilizon.Repo.insert(actor_changeset),
+         {:ok, %Mobilizon.Actors.Actor{} = actor} <- Mobilizon.Repo.insert(actor_changeset),
          user_changeset <-
            Mobilizon.Actors.User.registration_changeset(%Mobilizon.Actors.User{}, %{
              email: email,
              password: password,
-             default_actor_id: id
+             default_actor: actor
            }),
          {:ok, %Mobilizon.Actors.User{} = user} <- Mobilizon.Repo.insert(user_changeset) do
       {:ok, Map.put(actor, :user, user)}
@@ -536,6 +519,22 @@ defmodule Mobilizon.Actors do
     end
   end
 
+  @spec gravatar(String.t()) :: String.t() | nil
+  defp gravatar(nil), do: nil
+
+  defp gravatar(email) do
+    avatar_url = gravatar_url(email, default: "404")
+
+    case HTTPoison.get(avatar_url) do
+      {:ok, %HTTPoison.Response{status_code: 200}} ->
+        avatar_url
+
+      _ ->
+        nil
+    end
+  end
+
+  @spec handle_actor_user_changeset(Ecto.Changeset.t()) :: {:error, String.t()}
   defp handle_actor_user_changeset(changeset) do
     changeset =
       Ecto.Changeset.traverse_errors(changeset, fn
@@ -543,7 +542,8 @@ defmodule Mobilizon.Actors do
         msg -> msg
       end)
 
-    {:error, hd(Map.get(changeset, :email))}
+    email_msg = Map.get(changeset, :email) || [:empty_email]
+    {:error, hd(email_msg)}
   end
 
   def register_bot_account(%{name: name, summary: summary}) do
@@ -611,6 +611,16 @@ defmodule Mobilizon.Actors do
     end
   end
 
+  @spec get_user_by_activation_token(String.t()) :: Actor.t()
+  def get_user_by_activation_token(token) do
+    Repo.one(
+      from(u in User,
+        where: u.confirmation_token == ^token,
+        preload: [:default_actor]
+      )
+    )
+  end
+
   @doc """
   Updates a user.
 
@@ -645,18 +655,18 @@ defmodule Mobilizon.Actors do
     Repo.delete(user)
   end
 
-  @doc """
-  Returns an `%Ecto.Changeset{}` for tracking user changes.
+  # @doc """
+  # Returns an `%Ecto.Changeset{}` for tracking user changes.
 
-  ## Examples
+  # ## Examples
 
-      iex> change_user(%Mobilizon.Actors.User{})
-      %Ecto.Changeset{data: %Mobilizon.Actors.User{}}
+  #     iex> change_user(%Mobilizon.Actors.User{})
+  #     %Ecto.Changeset{data: %Mobilizon.Actors.User{}}
 
-  """
-  def change_user(%User{} = user) do
-    User.changeset(user, %{})
-  end
+  # """
+  # def change_user(%User{} = user) do
+  #   User.changeset(user, %{})
+  # end
 
   alias Mobilizon.Actors.Member
 

lib/mobilizon/actors/service/activation.ex

@@ -9,7 +9,7 @@ defmodule Mobilizon.Actors.Service.Activation do
 
   @doc false
   def check_confirmation_token(token) when is_binary(token) do
-    with %User{} = user <- Repo.get_by(User, confirmation_token: token),
+    with %User{} = user <- Actors.get_user_by_activation_token(token),
          {:ok, %User{} = user} <-
            Actors.update_user(user, %{
              "confirmed_at" => DateTime.utc_now(),
@@ -20,7 +20,7 @@ defmodule Mobilizon.Actors.Service.Activation do
       {:ok, user}
     else
       _err ->
-        {:error, "Invalid token"}
+        {:error, :invalid_token}
     end
   end
 

lib/mobilizon/actors/service/reset_password.ex

@@ -26,7 +26,7 @@ defmodule Mobilizon.Actors.Service.ResetPassword do
       {:error, %Ecto.Changeset{errors: [password: {"registration.error.password_too_short", _}]}} ->
         {:error, :password_too_short}
 
-      err ->
+      _err ->
         {:error, :invalid_token}
     end
   end

lib/mobilizon/actors/user.ex

@@ -13,7 +13,7 @@ defmodule Mobilizon.Actors.User do
     field(:password, :string, virtual: true)
     field(:role, :integer, default: 0)
     has_many(:actors, Actor)
-    field(:default_actor_id, :integer)
+    has_one(:default_actor, Actor)
     field(:confirmed_at, :utc_datetime)
     field(:confirmation_sent_at, :utc_datetime)
     field(:confirmation_token, :string)
@@ -29,7 +29,7 @@ defmodule Mobilizon.Actors.User do
     |> cast(attrs, [
       :email,
       :role,
-      :default_actor_id,
+      # :default_actor,
       :password_hash,
       :confirmed_at,
       :confirmation_sent_at,
@@ -52,7 +52,8 @@ defmodule Mobilizon.Actors.User do
     struct
     |> changeset(params)
     |> cast(params, ~w(password)a, [])
-    |> validate_required([:email, :password, :default_actor_id])
+    |> put_assoc(:default_actor, params.default_actor)
+    |> validate_required([:email, :password])
     |> validate_email()
     |> validate_length(
       :password,

lib/mobilizon_web/resolvers/user.ex

@@ -1,6 +1,7 @@
 defmodule MobilizonWeb.Resolvers.User do
   alias Mobilizon.Actors.{User, Actor}
   alias Mobilizon.Actors
+  require Logger
 
   @doc """
   Find an user by it's ID
@@ -56,11 +57,18 @@ defmodule MobilizonWeb.Resolvers.User do
   Validate an user, get it's actor and a token
   """
   def validate_user(_parent, %{token: token}, _resolution) do
-    with {:ok, %User{} = user} <-
-           Mobilizon.Actors.Service.Activation.check_confirmation_token(token),
-         %Actor{} = actor <- Actors.get_actor_for_user(user),
-         {:ok, token, _} <- MobilizonWeb.Guardian.encode_and_sign(user) do
+    with {:check_confirmation_token, {:ok, %User{} = user}} <-
+           {:check_confirmation_token,
+            Mobilizon.Actors.Service.Activation.check_confirmation_token(token)},
+         {:get_actor, %Actor{} = actor} <- {:get_actor, Actors.get_actor_for_user(user)},
+         {:guardian_encode_and_sign, {:ok, token, _}} <-
+           {:guardian_encode_and_sign, MobilizonWeb.Guardian.encode_and_sign(user)} do
       {:ok, %{token: token, user: user, person: actor}}
+    else
+      err ->
+        Logger.info("Unable to validate user with token #{token}")
+        Logger.debug(inspect(err))
+        {:error, :validation_failed}
     end
   end
 
@@ -116,8 +124,8 @@ defmodule MobilizonWeb.Resolvers.User do
   def change_default_actor(_parent, %{preferred_username: username}, %{
         context: %{current_user: user}
       }) do
-    with %Actor{id: id} <- Actors.get_local_actor_by_name(username) do
-      Actors.update_user(user, %{default_actor_id: id})
+    with %Actor{} = actor <- Actors.get_local_actor_by_name(username) do
+      Actors.update_user(user, %{default_actor: actor})
     end
   end
 end

lib/mobilizon_web/schema.ex

@@ -177,8 +177,7 @@ defmodule MobilizonWeb.Schema do
       description: "The user's list of profiles (identities)"
     )
 
-    # TODO: This shouldn't be an ID, but the actor itself
-    field(:default_actor_id, non_null(:integer), description: "The user's default actor")
+    field(:default_actor, non_null(:person), description: "The user's default actor")
 
     field(:confirmed_at, :datetime,
       description: "The datetime when the user was confirmed/activated"

test/mobilizon/actors/actors_test.exs

@@ -289,19 +289,9 @@ defmodule Mobilizon.ActorsTest do
   describe "users" do
     alias Mobilizon.Actors.{User, Actor}
 
-    @actor_valid_attrs %{
-      description: "some description",
-      display_name: "some display_name",
-      domain: "some domain",
-      keys: "some keys",
-      suspended: true,
-      uri: "some uri",
-      url: "some url",
-      preferred_username: "some username"
-    }
-    @valid_attrs %{email: "foo@bar.tld", password: "some password", role: 42}
-    @update_attrs %{email: "foo@fighters.tld", password: "some updated password", role: 43}
-    @invalid_attrs %{email: nil, password_hash: nil, role: nil}
+    @valid_attrs %{email: "foo@bar.tld", password: "some password", username: "foo"}
+    @update_attrs %{email: "foo@fighters.tld", password: "some updated password"}
+    @invalid_attrs %{email: nil, password: nil, username: nil}
 
     test "list_users/0 returns all users" do
       user = insert(:user)
@@ -314,24 +304,23 @@ defmodule Mobilizon.ActorsTest do
       assert user = Actors.get_user!(user.id)
     end
 
-    test "create_user/1 with valid data creates a user" do
-      {:ok, %Actor{} = actor} = Actors.create_actor(@actor_valid_attrs)
-      attrs = @valid_attrs |> Map.put(:actor_id, actor.id) |> Map.put(:default_actor_id, actor.id)
-      assert {:ok, %User{} = user} = Actors.create_user(attrs)
-      assert user.email == "foo@bar.tld"
-      assert user.role == 42
+    # There's no create_user/1, just register/1
+    test "register/1 with valid data creates a user" do
+      assert {:ok, %Actor{preferred_username: username, user: %User{email: email}}} =
+               Actors.register(@valid_attrs)
+
+      assert email == @valid_attrs.email
+      assert username == @valid_attrs.username
     end
 
     test "create_user/1 with invalid data returns error changeset" do
-      assert {:error, %Ecto.Changeset{}} = Actors.create_user(@invalid_attrs)
+      assert {:error, :empty_email} = Actors.register(@invalid_attrs)
     end
 
     test "update_user/2 with valid data updates the user" do
       user = insert(:user)
-      assert {:ok, user} = Actors.update_user(user, @update_attrs)
-      assert %User{} = user
-      assert user.email == "foo@fighters.tld"
-      assert user.role == 43
+      assert {:ok, %User{email: email}} = Actors.update_user(user, @update_attrs)
+      assert email == "foo@fighters.tld"
     end
 
     test "update_user/2 with invalid data returns error changeset" do
@@ -346,10 +335,10 @@ defmodule Mobilizon.ActorsTest do
       assert_raise Ecto.NoResultsError, fn -> Actors.get_user!(user.id) end
     end
 
-    test "change_user/1 returns a user changeset" do
-      user = insert(:user)
-      assert %Ecto.Changeset{} = Actors.change_user(user)
-    end
+    # test "change_user/1 returns a user changeset" do
+    #   user = insert(:user)
+    #   assert %Ecto.Changeset{} = Actors.change_user(user)
+    # end
 
     @email "email@domain.tld"
     @password "password"

test/mobilizon_web/resolvers/user_resolver_test.exs

@@ -190,7 +190,7 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do
         context.conn
         |> post("/api", AbsintheHelpers.mutation_skeleton(mutation))
 
-      assert hd(json_response(res, 200)["errors"])["message"] == "Invalid token"
+      assert hd(json_response(res, 200)["errors"])["message"] == "validation_failed"
     end
   end