From 534138f775d46a38fab1306b077a5035cdf76f87 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Wed, 28 Nov 2018 17:16:23 +0100 Subject: [PATCH] More test stuff Signed-off-by: Thomas Citharel --- lib/mobilizon/actors/actors.ex | 82 +++++++++++-------- lib/mobilizon/actors/service/activation.ex | 4 +- .../actors/service/reset_password.ex | 2 +- lib/mobilizon/actors/user.ex | 7 +- lib/mobilizon_web/resolvers/user.ex | 20 +++-- lib/mobilizon_web/schema.ex | 3 +- test/mobilizon/actors/actors_test.exs | 45 ++++------ .../resolvers/user_resolver_test.exs | 2 +- 8 files changed, 86 insertions(+), 79 deletions(-) diff --git a/lib/mobilizon/actors/actors.ex b/lib/mobilizon/actors/actors.ex index 762c79b0e..297f237d4 100644 --- a/lib/mobilizon/actors/actors.ex +++ b/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 diff --git a/lib/mobilizon/actors/service/activation.ex b/lib/mobilizon/actors/service/activation.ex index 24ac0d2c7..4b71a8f8f 100644 --- a/lib/mobilizon/actors/service/activation.ex +++ b/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 diff --git a/lib/mobilizon/actors/service/reset_password.ex b/lib/mobilizon/actors/service/reset_password.ex index ba5e1e0d1..db09c17dc 100644 --- a/lib/mobilizon/actors/service/reset_password.ex +++ b/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 diff --git a/lib/mobilizon/actors/user.ex b/lib/mobilizon/actors/user.ex index aa15f9a69..5ce95229d 100644 --- a/lib/mobilizon/actors/user.ex +++ b/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, diff --git a/lib/mobilizon_web/resolvers/user.ex b/lib/mobilizon_web/resolvers/user.ex index 2a208e9cf..f23eb9bc1 100644 --- a/lib/mobilizon_web/resolvers/user.ex +++ b/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 diff --git a/lib/mobilizon_web/schema.ex b/lib/mobilizon_web/schema.ex index 6a97f687b..1dfe494df 100644 --- a/lib/mobilizon_web/schema.ex +++ b/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" diff --git a/test/mobilizon/actors/actors_test.exs b/test/mobilizon/actors/actors_test.exs index 223a862ba..d2ccdb9e0 100644 --- a/test/mobilizon/actors/actors_test.exs +++ b/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" diff --git a/test/mobilizon_web/resolvers/user_resolver_test.exs b/test/mobilizon_web/resolvers/user_resolver_test.exs index 19cae6d24..adef2d4e2 100644 --- a/test/mobilizon_web/resolvers/user_resolver_test.exs +++ b/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