From 7bc558900203b587016fb962deb23374117acc11 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Fri, 25 Jan 2019 13:59:58 +0100 Subject: [PATCH 1/4] Make register/1 only create an user Signed-off-by: Thomas Citharel Credo fix Signed-off-by: Thomas Citharel Fix rebase Signed-off-by: Thomas Citharel --- lib/mobilizon/actors/actors.ex | 111 +++++++++++------- lib/mobilizon_web/resolvers/user.ex | 10 +- lib/mobilizon_web/schema.ex | 31 +++-- lib/mobilizon_web/schema/actors/group.ex | 38 ++++++ lib/mobilizon_web/schema/user.ex | 2 +- test/mobilizon/actors/actors_test.exs | 28 ++--- .../resolvers/category_resolver_test.exs | 6 +- .../resolvers/comment_resolver_test.exs | 7 +- .../resolvers/event_resolver_test.exs | 9 +- .../resolvers/group_resolver_test.exs | 6 +- .../resolvers/person_resolver_test.exs | 10 +- .../resolvers/user_resolver_test.exs | 87 ++++---------- 12 files changed, 184 insertions(+), 161 deletions(-) diff --git a/lib/mobilizon/actors/actors.ex b/lib/mobilizon/actors/actors.ex index cb08885c8..084f4be6c 100644 --- a/lib/mobilizon/actors/actors.ex +++ b/lib/mobilizon/actors/actors.ex @@ -11,7 +11,7 @@ defmodule Mobilizon.Actors do alias Mobilizon.Actors.{Actor, Bot, Member, Follower, User} alias Mobilizon.Service.ActivityPub - import Exgravatar + # import Exgravatar @doc false def data() do @@ -68,8 +68,14 @@ defmodule Mobilizon.Actors do where: u.id == ^user.id ) ) do - nil -> user |> get_actors_for_user() |> hd - actor -> actor + nil -> + case user |> get_actors_for_user() do + [] -> nil + actors -> hd(actors) + end + + actor -> + actor end end @@ -597,56 +603,71 @@ 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 - with avatar <- gravatar(email), - user_changeset <- - User.registration_changeset(%User{}, %{ - email: email, - password: password, - default_actor: %{ - preferred_username: username, - domain: nil, - keys: create_keys(), - avatar_url: avatar - } - }), - {:ok, %User{default_actor: %Actor{} = actor, id: user_id} = user} <- - Mobilizon.Repo.insert(user_changeset), - {:ok, %Actor{} = _actor} <- update_actor(actor, %{user_id: user_id}) do - {:ok, Repo.preload(user, [:actors])} - else - {:error, %Ecto.Changeset{} = changeset} -> - handle_actor_user_changeset(changeset) + @spec register(map()) :: {:ok, User.t()} | {:error, String.t()} + def register(%{email: _email, password: _password} = args) do + with {:ok, %User{} = user} <- + %User{} |> User.registration_changeset(args) |> Mobilizon.Repo.insert() do + {:ok, user} + # else + # {:error, %Ecto.Changeset{} = changeset} -> + # {:error, Ecto.Changeset.traverse_errors(changeset, fn {msg, opts} -> + # Enum.reduce(opts, msg, fn {key, value}, acc -> + # String.replace(acc, "%{#{key}}", to_string(value)) + # end) + # end)} end end - @spec gravatar(String.t()) :: String.t() | nil - defp gravatar(nil), do: nil + # @spec register(map()) :: {:ok, Actor.t()} | {:error, String.t()} + # def register(%{email: email, password: password, username: username}) do + # with avatar <- gravatar(email), + # user_changeset <- + # User.registration_changeset(%User{}, %{ + # email: email, + # password: password, + # default_actor: %{ + # preferred_username: username, + # domain: nil, + # keys: create_keys(), + # avatar_url: avatar + # } + # }), + # {:ok, %User{default_actor: %Actor{} = actor, id: user_id} = user} <- + # Mobilizon.Repo.insert(user_changeset), + # {:ok, %Actor{} = _actor} <- update_actor(actor, %{user_id: user_id}) do + # {:ok, Repo.preload(user, [:actors])} + # else + # {:error, %Ecto.Changeset{} = changeset} -> + # handle_actor_user_changeset(changeset) + # end + # end - defp gravatar(email) do - avatar_url = gravatar_url(email, default: "404") + # @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 + # {msg, _opts} -> msg + # msg -> msg + # end) - case HTTPoison.get(avatar_url) do - {:ok, %HTTPoison.Response{status_code: 200}} -> - avatar_url + # email_msg = Map.get(changeset, :email) || [:empty_email] + # {:error, hd(email_msg)} + # end - _ -> - nil - end - end + # @spec gravatar(String.t()) :: String.t() | nil + # defp gravatar(nil), do: nil - @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 - {msg, _opts} -> msg - msg -> msg - end) + # defp gravatar(email) do + # avatar_url = gravatar_url(email, default: "404") - email_msg = Map.get(changeset, :email) || [:empty_email] - {:error, hd(email_msg)} - end + # case HTTPoison.get(avatar_url) do + # {:ok, %HTTPoison.Response{status_code: 200}} -> + # avatar_url + + # _ -> + # nil + # end + # end @doc """ Create a new person actor diff --git a/lib/mobilizon_web/resolvers/user.ex b/lib/mobilizon_web/resolvers/user.ex index 108059228..f8d45fbb7 100644 --- a/lib/mobilizon_web/resolvers/user.ex +++ b/lib/mobilizon_web/resolvers/user.ex @@ -43,12 +43,10 @@ defmodule MobilizonWeb.Resolvers.User do @doc """ Register an user : - create the user - - create the actor - - set the user's default_actor to the newly created actor - send a validation email to the user """ - @spec create_user_actor(any(), map(), any()) :: tuple() - def create_user_actor(_parent, args, _resolution) do + @spec create_user(any(), map(), any()) :: tuple() + def create_user(_parent, args, _resolution) do with {:ok, %User{} = user} <- Actors.register(args) do Mobilizon.Actors.Service.Activation.send_confirmation_email(user) {:ok, user} @@ -62,7 +60,7 @@ defmodule MobilizonWeb.Resolvers.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)}, + {:get_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}} @@ -70,7 +68,7 @@ defmodule MobilizonWeb.Resolvers.User do err -> Logger.info("Unable to validate user with token #{token}") Logger.debug(inspect(err)) - {:error, :validation_failed} + {:error, "Unable to validate user"} end end diff --git a/lib/mobilizon_web/schema.ex b/lib/mobilizon_web/schema.ex index 48edd79da..b0a0d16e5 100644 --- a/lib/mobilizon_web/schema.ex +++ b/lib/mobilizon_web/schema.ex @@ -245,9 +245,8 @@ defmodule MobilizonWeb.Schema do field :create_user, type: :user do arg(:email, non_null(:string)) arg(:password, non_null(:string)) - arg(:username, non_null(:string)) - resolve(&Resolvers.User.create_user_actor/3) + resolve(handle_errors(&Resolvers.User.create_user/3)) end @desc "Validate an user after registration" @@ -314,18 +313,30 @@ defmodule MobilizonWeb.Schema do resolve(&Resolvers.Group.create_group/3) end - @desc "Delete a group" - field :delete_group, :deleted_object do - arg(:group_id, non_null(:integer)) - arg(:actor_id, non_null(:integer)) - - resolve(&Resolvers.Group.delete_group/3) - end - # @desc "Upload a picture" # field :upload_picture, :picture do # arg(:file, non_null(:upload)) # resolve(&Resolvers.Upload.upload_picture/3) # end end + + def handle_errors(fun) do + fn source, args, info -> + case Absinthe.Resolution.call(fun, source, args, info) do + {:error, %Ecto.Changeset{} = changeset} -> format_changeset(changeset) + val -> val + end + end + end + + def format_changeset(changeset) do + # {:error, [email: {"has already been taken", []}]} + errors = + changeset.errors + |> Enum.map(fn {_key, {value, context}} -> + [message: "#{value}", details: context] + end) + + {:error, errors} + end end diff --git a/lib/mobilizon_web/schema/actors/group.ex b/lib/mobilizon_web/schema/actors/group.ex index cf8316236..120655939 100644 --- a/lib/mobilizon_web/schema/actors/group.ex +++ b/lib/mobilizon_web/schema/actors/group.ex @@ -69,4 +69,42 @@ defmodule MobilizonWeb.Schema.Actors.GroupType do value(:open, description: "The actor is open to followings") end + + object :group_queries do + @desc "Get all groups" + field :groups, list_of(:group) do + arg(:page, :integer, default_value: 1) + arg(:limit, :integer, default_value: 10) + resolve(&Resolvers.Group.list_groups/3) + end + + @desc "Get a group by it's preferred username" + field :group, :group do + arg(:preferred_username, non_null(:string)) + resolve(&Resolvers.Group.find_group/3) + end + end + + object :group_mutations do + @desc "Create a group" + field :create_group, :group do + arg(:preferred_username, non_null(:string), description: "The name for the group") + arg(:name, :string, description: "The displayed name for the group") + arg(:description, :string, description: "The summary for the group", default_value: "") + + arg(:admin_actor_username, :string, + description: "The actor's username which will be the admin (otherwise user's default one)" + ) + + resolve(&Resolvers.Group.create_group/3) + end + + @desc "Delete a group" + field :delete_group, :deleted_object do + arg(:group_id, non_null(:integer)) + arg(:actor_id, non_null(:integer)) + + resolve(&Resolvers.Group.delete_group/3) + end + end end diff --git a/lib/mobilizon_web/schema/user.ex b/lib/mobilizon_web/schema/user.ex index b1106de93..fa34d5aa4 100644 --- a/lib/mobilizon_web/schema/user.ex +++ b/lib/mobilizon_web/schema/user.ex @@ -13,7 +13,7 @@ defmodule MobilizonWeb.Schema.UserType do description: "The user's list of profiles (identities)" ) - field(:default_actor, non_null(:person), description: "The user's default actor") + field(:default_actor, :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 53819cf63..7e67d79d3 100644 --- a/test/mobilizon/actors/actors_test.exs +++ b/test/mobilizon/actors/actors_test.exs @@ -289,9 +289,9 @@ defmodule Mobilizon.ActorsTest do describe "users" do alias Mobilizon.Actors.{User, Actor} - @valid_attrs %{email: "foo@bar.tld", password: "some password", username: "foo"} + @valid_attrs %{email: "foo@bar.tld", password: "some password"} @update_attrs %{email: "foo@fighters.tld", password: "some updated password"} - @invalid_attrs %{email: nil, password: nil, username: nil} + @invalid_attrs %{email: nil, password: nil} test "list_users/0 returns all users" do user = insert(:user) @@ -306,17 +306,20 @@ defmodule Mobilizon.ActorsTest do # There's no create_user/1, just register/1 test "register/1 with valid data creates a user" do - assert {:ok, - %User{email: email, default_actor: %Actor{preferred_username: username} = actor} = - user} = Actors.register(@valid_attrs) + assert {:ok, %User{email: email} = user} = Actors.register(@valid_attrs) assert email == @valid_attrs.email - assert username == @valid_attrs.username - assert [actor.id] == Actors.get_actors_for_user(user) |> Enum.map(& &1.id) end test "create_user/1 with invalid data returns error changeset" do - assert {:error, "can't be blank"} = Actors.register(@invalid_attrs) + assert {:error, + %Ecto.Changeset{ + errors: [ + password: {"can't be blank", [validation: :required]}, + email: {"can't be blank", [validation: :required]} + ], + valid?: false + }} = Actors.register(@invalid_attrs) end test "update_user/2 with valid data updates the user" do @@ -345,8 +348,7 @@ defmodule Mobilizon.ActorsTest do @email "email@domain.tld" @password "password" test "authenticate/1 checks the user's password" do - {:ok, %User{} = user} = - Actors.register(%{email: @email, password: @password, username: "yolo"}) + {:ok, %User{} = user} = Actors.register(%{email: @email, password: @password}) assert {:ok, _, _} = Actors.authenticate(%{user: user, password: @password}) @@ -355,8 +357,7 @@ defmodule Mobilizon.ActorsTest do end test "get_user_by_email/1 finds an user by it's email" do - {:ok, %User{email: email} = user} = - Actors.register(%{email: @email, password: @password, username: "yolo"}) + {:ok, %User{email: email} = user} = Actors.register(%{email: @email, password: @password}) assert email == @email {:ok, %User{id: id}} = Actors.get_user_by_email(@email) @@ -365,8 +366,7 @@ defmodule Mobilizon.ActorsTest do end test "get_user_by_email/1 finds an activated user by it's email" do - {:ok, %User{} = user} = - Actors.register(%{email: @email, password: @password, username: "yolo"}) + {:ok, %User{} = user} = Actors.register(%{email: @email, password: @password}) {:ok, %User{id: id}} = Actors.get_user_by_email(@email, false) assert id == user.id diff --git a/test/mobilizon_web/resolvers/category_resolver_test.exs b/test/mobilizon_web/resolvers/category_resolver_test.exs index 7bf7d8a01..bfc82b7a7 100644 --- a/test/mobilizon_web/resolvers/category_resolver_test.exs +++ b/test/mobilizon_web/resolvers/category_resolver_test.exs @@ -1,13 +1,11 @@ defmodule MobilizonWeb.Resolvers.CategoryResolverTest do use MobilizonWeb.ConnCase - alias Mobilizon.Actors - alias Mobilizon.Actors.{Actor, User} alias MobilizonWeb.AbsintheHelpers import Mobilizon.Factory setup %{conn: conn} do - {:ok, %User{default_actor: %Actor{} = actor} = user} = - Actors.register(%{email: "test@test.tld", password: "testest", username: "test"}) + user = insert(:user) + actor = insert(:actor, user: user) {:ok, conn: conn, actor: actor, user: user} end diff --git a/test/mobilizon_web/resolvers/comment_resolver_test.exs b/test/mobilizon_web/resolvers/comment_resolver_test.exs index 7ddc3b612..618473b53 100644 --- a/test/mobilizon_web/resolvers/comment_resolver_test.exs +++ b/test/mobilizon_web/resolvers/comment_resolver_test.exs @@ -1,14 +1,13 @@ defmodule MobilizonWeb.Resolvers.CommentResolverTest do use MobilizonWeb.ConnCase - alias Mobilizon.Actors - alias Mobilizon.Actors.{Actor, User} alias MobilizonWeb.AbsintheHelpers + import Mobilizon.Factory @comment %{text: "I love this event"} setup %{conn: conn} do - {:ok, %User{default_actor: %Actor{} = actor} = user} = - Actors.register(%{email: "test@test.tld", password: "testest", username: "test"}) + user = insert(:user) + actor = insert(:actor, user: user) {:ok, conn: conn, actor: actor, user: user} end diff --git a/test/mobilizon_web/resolvers/event_resolver_test.exs b/test/mobilizon_web/resolvers/event_resolver_test.exs index 2659cefb7..13181ff23 100644 --- a/test/mobilizon_web/resolvers/event_resolver_test.exs +++ b/test/mobilizon_web/resolvers/event_resolver_test.exs @@ -1,7 +1,6 @@ defmodule MobilizonWeb.Resolvers.EventResolverTest do use MobilizonWeb.ConnCase - alias Mobilizon.{Events, Actors} - alias Mobilizon.Actors.{Actor, User} + alias Mobilizon.Events alias MobilizonWeb.AbsintheHelpers import Mobilizon.Factory @@ -14,8 +13,8 @@ defmodule MobilizonWeb.Resolvers.EventResolverTest do } setup %{conn: conn} do - {:ok, %User{default_actor: %Actor{} = actor} = user} = - Actors.register(%{email: "test@test.tld", password: "testest", username: "test"}) + user = insert(:user) + actor = insert(:actor, user: user, preferred_username: "test") {:ok, conn: conn, actor: actor, user: user} end @@ -137,8 +136,6 @@ defmodule MobilizonWeb.Resolvers.EventResolverTest do |> auth_conn(user) |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) - require Logger - Logger.error(inspect(json_response(res, 200))) assert json_response(res, 200)["data"]["createEvent"]["title"] == "come to my event" end diff --git a/test/mobilizon_web/resolvers/group_resolver_test.exs b/test/mobilizon_web/resolvers/group_resolver_test.exs index 64f0cde8f..6c5557d9f 100644 --- a/test/mobilizon_web/resolvers/group_resolver_test.exs +++ b/test/mobilizon_web/resolvers/group_resolver_test.exs @@ -1,7 +1,5 @@ defmodule MobilizonWeb.Resolvers.GroupResolverTest do use MobilizonWeb.ConnCase - alias Mobilizon.Actors - alias Mobilizon.Actors.{User, Actor} alias MobilizonWeb.AbsintheHelpers import Mobilizon.Factory require Logger @@ -10,8 +8,8 @@ defmodule MobilizonWeb.Resolvers.GroupResolverTest do @new_group_params %{groupname: "new group"} setup %{conn: conn} do - {:ok, %User{default_actor: %Actor{} = actor} = user} = - Actors.register(%{email: "test2@test.tld", password: "testest", username: "test"}) + user = insert(:user) + actor = insert(:actor, user: user) {:ok, conn: conn, actor: actor, user: user} end diff --git a/test/mobilizon_web/resolvers/person_resolver_test.exs b/test/mobilizon_web/resolvers/person_resolver_test.exs index 6f0475ab0..de89869a4 100644 --- a/test/mobilizon_web/resolvers/person_resolver_test.exs +++ b/test/mobilizon_web/resolvers/person_resolver_test.exs @@ -1,15 +1,14 @@ defmodule MobilizonWeb.Resolvers.PersonResolverTest do use MobilizonWeb.ConnCase - alias Mobilizon.Actors - alias Mobilizon.Actors.{User, Actor} alias MobilizonWeb.AbsintheHelpers + import Mobilizon.Factory - @valid_actor_params %{email: "test@test.tld", password: "testest", username: "test"} @non_existent_username "nonexistent" describe "Person Resolver" do test "find_person/3 returns a person by it's username", context do - {:ok, %User{default_actor: %Actor{} = actor} = _user} = Actors.register(@valid_actor_params) + user = insert(:user) + actor = insert(:actor, user: user) query = """ { @@ -45,7 +44,8 @@ defmodule MobilizonWeb.Resolvers.PersonResolverTest do end test "get_current_person/3 returns the current logged-in actor", context do - {:ok, %User{default_actor: %Actor{} = actor} = user} = Actors.register(@valid_actor_params) + user = insert(:user) + actor = insert(:actor, user: user) query = """ { diff --git a/test/mobilizon_web/resolvers/user_resolver_test.exs b/test/mobilizon_web/resolvers/user_resolver_test.exs index d28d669fc..de969f41e 100644 --- a/test/mobilizon_web/resolvers/user_resolver_test.exs +++ b/test/mobilizon_web/resolvers/user_resolver_test.exs @@ -75,26 +75,21 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do describe "Resolver: Create an user & actor" do @account_creation %{ email: "test@demo.tld", - password: "long password", - username: "test_account" + password: "long password" } @account_creation_bad_email %{ email: "y@l@", - password: "long password", - username: "test_account" + password: "long password" } - test "test create_user_actor/3 creates an user", context do + test "test create_user/3 creates an user", context do mutation = """ mutation { createUser( email: "#{@account_creation.email}", password: "#{@account_creation.password}", - username: "#{@account_creation.username}" ) { - default_actor { - preferred_username, - }, + id, email } } @@ -104,24 +99,18 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do context.conn |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) - assert json_response(res, 200)["data"]["createUser"]["default_actor"]["preferred_username"] == - @account_creation.username - assert json_response(res, 200)["data"]["createUser"]["email"] == @account_creation.email end - test "test create_user_actor/3 doesn't create an user with bad email", context do + test "test create_user/3 doesn't create an user with bad email", context do mutation = """ mutation { createUser( email: "#{@account_creation_bad_email.email}", password: "#{@account_creation.password}", - username: "#{@account_creation.username}" ) { - default_actor { - preferred_username, - }, - email, + id, + email } } """ @@ -136,9 +125,9 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do end describe "Resolver: Validate an user" do - @valid_actor_params %{email: "test@test.tld", password: "testest", username: "test"} + @valid_actor_params %{email: "test@test.tld", password: "testest"} test "test validate_user/3 validates an user", context do - {:ok, %User{default_actor: %Actor{} = _actor} = user} = Actors.register(@valid_actor_params) + {:ok, %User{} = user} = Actors.register(@valid_actor_params) mutation = """ mutation { @@ -148,9 +137,6 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do token, user { id, - default_actor { - preferredUsername - } }, } } @@ -160,16 +146,11 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do context.conn |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) - assert json_response(res, 200)["data"]["validateUser"]["user"]["default_actor"][ - "preferredUsername" - ] == @valid_actor_params.username - assert json_response(res, 200)["data"]["validateUser"]["user"]["id"] == to_string(user.id) end test "test validate_user/3 with invalid token doesn't validate an user", context do - {:ok, %User{default_actor: %Actor{} = _actor} = _user} = - Actors.register(@valid_actor_params) + insert(:user, confirmation_token: "t0t0") mutation = """ mutation { @@ -178,10 +159,7 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do ) { token, user { - id, - default_actor { - preferredUsername - } + id }, } } @@ -191,14 +169,14 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do context.conn |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) - assert hd(json_response(res, 200)["errors"])["message"] == "validation_failed" + assert hd(json_response(res, 200)["errors"])["message"] == "Unable to validate user" end end describe "Resolver: Resend confirmation emails" do test "test resend_confirmation_email/3 with valid email resends an validation email", context do - {:ok, %User{default_actor: %Actor{} = _actor} = user} = Actors.register(@valid_actor_params) + {:ok, %User{} = user} = Actors.register(%{email: "toto@tata.tld", password: "p4ssw0rd"}) mutation = """ mutation { @@ -230,9 +208,6 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do test "test resend_confirmation_email/3 with invalid email resends an validation email", context do - {:ok, %User{default_actor: %Actor{} = _actor} = _user} = - Actors.register(@valid_actor_params) - mutation = """ mutation { resendConfirmationEmail( @@ -289,7 +264,7 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do describe "Resolver: Reset user's password" do test "test reset_password/3 with valid email", context do - %User{} = user = insert(:user) + {:ok, %User{} = user} = Actors.register(%{email: "toto@tata.tld", password: "p4ssw0rd"}) %Actor{} = insert(:actor, user: user) {:ok, _email_sent} = Mobilizon.Actors.Service.ResetPassword.send_password_reset_email(user) %User{reset_password_token: reset_password_token} = Mobilizon.Actors.get_user!(user.id) @@ -369,7 +344,7 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do describe "Resolver: Login an user" do test "test login_user/3 with valid credentials", context do - {:ok, %User{} = user} = Actors.register(@valid_actor_params) + {:ok, %User{} = user} = Actors.register(%{email: "toto@tata.tld", password: "p4ssw0rd"}) {:ok, %User{} = _user} = Actors.update_user(user, %{ @@ -381,14 +356,12 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do mutation = """ mutation { login( - email: "#{@valid_actor_params.email}", - password: "#{@valid_actor_params.password}", + email: "#{user.email}", + password: "#{user.password}", ) { token, user { - default_actor { - preferred_username, - } + id } } } @@ -400,11 +373,10 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do assert login = json_response(res, 200)["data"]["login"] assert Map.has_key?(login, "token") && not is_nil(login["token"]) - assert login["user"]["default_actor"]["preferred_username"] == @valid_actor_params.username end test "test login_user/3 with invalid password", context do - {:ok, %User{} = user} = Actors.register(@valid_actor_params) + {:ok, %User{} = user} = Actors.register(%{email: "toto@tata.tld", password: "p4ssw0rd"}) {:ok, %User{} = _user} = Actors.update_user(user, %{ @@ -416,7 +388,7 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do mutation = """ mutation { login( - email: "#{@valid_actor_params.email}", + email: "#{user.email}", password: "bad password", ) { token, @@ -438,15 +410,6 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do end test "test login_user/3 with invalid email", context do - {:ok, %User{} = user} = Actors.register(@valid_actor_params) - - {:ok, %User{} = _user} = - Actors.update_user(user, %{ - "confirmed_at" => DateTime.utc_now(), - "confirmation_sent_at" => nil, - "confirmation_token" => nil - }) - mutation = """ mutation { login( @@ -474,15 +437,15 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do describe "Resolver: change default actor for user" do test "test change_default_actor/3 with valid actor", context do # Prepare user with two actors - assert {:ok, %User{id: user_id, default_actor: %Actor{} = actor} = user} = - Actors.register(@valid_actor_params) + user = insert(:user) + insert(:actor, user: user) - assert {:ok, %User{actors: actors}} = Actors.get_user_with_actors(user_id) + assert {:ok, %User{actors: actors}} = Actors.get_user_with_actors(user.id) - actor_params = @valid_single_actor_params |> Map.put(:user_id, user_id) + actor_params = @valid_single_actor_params |> Map.put(:user_id, user.id) assert {:ok, %Actor{} = actor2} = Actors.create_actor(actor_params) - assert {:ok, %User{actors: actors}} = Actors.get_user_with_actors(user_id) + assert {:ok, %User{actors: actors}} = Actors.get_user_with_actors(user.id) assert length(actors) == 2 mutation = """ From 0988ff390cc9fc37fe6cf68271ac5fcae14e622c Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Fri, 25 Jan 2019 15:41:10 +0100 Subject: [PATCH 2/4] Move queries and mutations to submodules Signed-off-by: Thomas Citharel --- lib/mobilizon_web/schema.ex | 215 ++---------------- lib/mobilizon_web/schema/actors/group.ex | 1 + lib/mobilizon_web/schema/actors/person.ex | 32 ++- lib/mobilizon_web/schema/comment.ex | 11 + lib/mobilizon_web/schema/event.ex | 46 ++++ lib/mobilizon_web/schema/events/category.ex | 20 ++ .../schema/events/participant.ex | 11 + lib/mobilizon_web/schema/user.ex | 66 ++++++ lib/mobilizon_web/schema/utils.ex | 21 ++ 9 files changed, 220 insertions(+), 203 deletions(-) create mode 100644 lib/mobilizon_web/schema/utils.ex diff --git a/lib/mobilizon_web/schema.ex b/lib/mobilizon_web/schema.ex index b0a0d16e5..564522fe3 100644 --- a/lib/mobilizon_web/schema.ex +++ b/lib/mobilizon_web/schema.ex @@ -13,6 +13,7 @@ defmodule MobilizonWeb.Schema do import_types(Absinthe.Type.Custom) import_types(Absinthe.Plug.Types) + import_types(MobilizonWeb.Schema.UserType) import_types(MobilizonWeb.Schema.ActorInterface) import_types(MobilizonWeb.Schema.Actors.PersonType) import_types(MobilizonWeb.Schema.Actors.GroupType) @@ -116,20 +117,6 @@ defmodule MobilizonWeb.Schema do Root Query """ query do - @desc "Get all events" - field :events, list_of(:event) do - arg(:page, :integer, default_value: 1) - arg(:limit, :integer, default_value: 10) - resolve(&Resolvers.Event.list_events/3) - end - - @desc "Get all groups" - field :groups, list_of(:group) do - arg(:page, :integer, default_value: 1) - arg(:limit, :integer, default_value: 10) - resolve(&Resolvers.Group.list_groups/3) - end - @desc "Search through events, persons and groups" field :search, list_of(:search_result) do arg(:search, non_null(:string)) @@ -138,180 +125,24 @@ defmodule MobilizonWeb.Schema do resolve(&Resolvers.Event.search_events_and_actors/3) end - @desc "Get an event by uuid" - field :event, :event do - arg(:uuid, non_null(:uuid)) - resolve(&Resolvers.Event.find_event/3) - end - - @desc "Get all participants for an event uuid" - field :participants, list_of(:participant) do - arg(:uuid, non_null(:uuid)) - arg(:page, :integer, default_value: 1) - arg(:limit, :integer, default_value: 10) - resolve(&Resolvers.Event.list_participants_for_event/3) - end - - @desc "Get a group by it's preferred username" - field :group, :group do - arg(:preferred_username, non_null(:string)) - resolve(&Resolvers.Group.find_group/3) - end - - @desc "Get an user" - field :user, :user do - arg(:id, non_null(:id)) - resolve(&Resolvers.User.find_user/3) - end - - @desc "Get the current user" - field :logged_user, :user do - resolve(&Resolvers.User.get_current_user/3) - end - - @desc "Get the current actor for the logged-in user" - field :logged_person, :person do - resolve(&Resolvers.Person.get_current_person/3) - end - - @desc "Get a person by it's preferred username" - field :person, :person do - arg(:preferred_username, non_null(:string)) - resolve(&Resolvers.Person.find_person/3) - end - - @desc "Get the persons for an user" - field :identities, list_of(:person) do - resolve(&Resolvers.Person.identities/3) - end - - @desc "Get the list of categories" - field :categories, non_null(list_of(:category)) do - arg(:page, :integer, default_value: 1) - arg(:limit, :integer, default_value: 10) - resolve(&Resolvers.Category.list_categories/3) - end + import_fields(:user_queries) + import_fields(:person_queries) + import_fields(:group_queries) + import_fields(:event_queries) + import_fields(:participant_queries) + import_fields(:category_queries) end @desc """ Root Mutation """ mutation do - @desc "Create an event" - field :create_event, type: :event do - arg(:title, non_null(:string)) - arg(:description, non_null(:string)) - arg(:begins_on, non_null(:datetime)) - arg(:ends_on, :datetime) - arg(:state, :integer) - arg(:status, :integer) - arg(:public, :boolean) - arg(:thumbnail, :string) - arg(:large_image, :string) - arg(:publish_at, :datetime) - arg(:online_address, :string) - arg(:phone_address, :string) - arg(:organizer_actor_id, non_null(:id)) - arg(:category, non_null(:string)) - - resolve(&Resolvers.Event.create_event/3) - end - - @desc "Delete an event" - field :delete_event, :deleted_object do - arg(:event_id, non_null(:integer)) - arg(:actor_id, non_null(:integer)) - - resolve(&Resolvers.Event.delete_event/3) - end - - @desc "Create a comment" - field :create_comment, type: :comment do - arg(:text, non_null(:string)) - arg(:actor_username, non_null(:string)) - - resolve(&Resolvers.Comment.create_comment/3) - end - - @desc "Create a category with a title, description and picture" - field :create_category, type: :category do - arg(:title, non_null(:string)) - arg(:description, non_null(:string)) - arg(:picture, non_null(:upload)) - resolve(&Resolvers.Category.create_category/3) - end - - @desc "Create an user" - field :create_user, type: :user do - arg(:email, non_null(:string)) - arg(:password, non_null(:string)) - - resolve(handle_errors(&Resolvers.User.create_user/3)) - end - - @desc "Validate an user after registration" - field :validate_user, type: :login do - arg(:token, non_null(:string)) - resolve(&Resolvers.User.validate_user/3) - end - - @desc "Resend registration confirmation token" - field :resend_confirmation_email, type: :string do - arg(:email, non_null(:string)) - arg(:locale, :string, default_value: "en") - resolve(&Resolvers.User.resend_confirmation_email/3) - end - - @desc "Send a link through email to reset user password" - field :send_reset_password, type: :string do - arg(:email, non_null(:string)) - arg(:locale, :string, default_value: "en") - resolve(&Resolvers.User.send_reset_password/3) - end - - @desc "Reset user password" - field :reset_password, type: :login do - arg(:token, non_null(:string)) - arg(:password, non_null(:string)) - arg(:locale, :string, default_value: "en") - resolve(&Resolvers.User.reset_password/3) - end - - @desc "Login an user" - field :login, :login do - arg(:email, non_null(:string)) - arg(:password, non_null(:string)) - resolve(&Resolvers.User.login_user/3) - end - - @desc "Change default actor for user" - field :change_default_actor, :user do - arg(:preferred_username, non_null(:string)) - resolve(&Resolvers.User.change_default_actor/3) - end - - @desc "Create a new person for user" - field :create_person, :person do - arg(:preferred_username, non_null(:string)) - arg(:name, :string, description: "The displayed name for the new profile") - - arg(:description, :string, description: "The summary for the new profile", default_value: "") - - resolve(&Resolvers.Person.create_person/3) - end - - @desc "Create a group" - field :create_group, :group do - arg(:preferred_username, non_null(:string), description: "The name for the group") - arg(:name, :string, description: "The displayed name for the group") - arg(:description, :string, description: "The summary for the group", default_value: "") - - arg(:admin_actor_username, :string, - description: "The actor's username which will be the admin (otherwise user's default one)" - ) - - resolve(&Resolvers.Group.create_group/3) - end + import_fields(:user_mutations) + import_fields(:person_mutations) + import_fields(:group_mutations) + import_fields(:event_mutations) + import_fields(:category_mutations) + import_fields(:comment_mutations) # @desc "Upload a picture" # field :upload_picture, :picture do @@ -319,24 +150,4 @@ defmodule MobilizonWeb.Schema do # resolve(&Resolvers.Upload.upload_picture/3) # end end - - def handle_errors(fun) do - fn source, args, info -> - case Absinthe.Resolution.call(fun, source, args, info) do - {:error, %Ecto.Changeset{} = changeset} -> format_changeset(changeset) - val -> val - end - end - end - - def format_changeset(changeset) do - # {:error, [email: {"has already been taken", []}]} - errors = - changeset.errors - |> Enum.map(fn {_key, {value, context}} -> - [message: "#{value}", details: context] - end) - - {:error, errors} - end end diff --git a/lib/mobilizon_web/schema/actors/group.ex b/lib/mobilizon_web/schema/actors/group.ex index 120655939..7a24382a9 100644 --- a/lib/mobilizon_web/schema/actors/group.ex +++ b/lib/mobilizon_web/schema/actors/group.ex @@ -5,6 +5,7 @@ defmodule MobilizonWeb.Schema.Actors.GroupType do use Absinthe.Schema.Notation import Absinthe.Resolution.Helpers, only: [dataloader: 1] import_types(MobilizonWeb.Schema.Actors.MemberType) + alias MobilizonWeb.Resolvers @desc """ Represents a group of actors diff --git a/lib/mobilizon_web/schema/actors/person.ex b/lib/mobilizon_web/schema/actors/person.ex index 24acaba5e..70610a324 100644 --- a/lib/mobilizon_web/schema/actors/person.ex +++ b/lib/mobilizon_web/schema/actors/person.ex @@ -4,8 +4,8 @@ defmodule MobilizonWeb.Schema.Actors.PersonType do """ use Absinthe.Schema.Notation import Absinthe.Resolution.Helpers, only: [dataloader: 1] - import_types(MobilizonWeb.Schema.UserType) alias Mobilizon.Events + alias MobilizonWeb.Resolvers @desc """ Represents a person identity @@ -46,4 +46,34 @@ defmodule MobilizonWeb.Schema.Actors.PersonType do description: "A list of the events this actor has organized" ) end + + object :person_queries do + @desc "Get the current actor for the logged-in user" + field :logged_person, :person do + resolve(&Resolvers.Person.get_current_person/3) + end + + @desc "Get a person by it's preferred username" + field :person, :person do + arg(:preferred_username, non_null(:string)) + resolve(&Resolvers.Person.find_person/3) + end + + @desc "Get the persons for an user" + field :identities, list_of(:person) do + resolve(&Resolvers.Person.identities/3) + end + end + + object :person_mutations do + @desc "Create a new person for user" + field :create_person, :person do + arg(:preferred_username, non_null(:string)) + arg(:name, :string, description: "The displayed name for the new profile") + + arg(:description, :string, description: "The summary for the new profile", default_value: "") + + resolve(&Resolvers.Person.create_person/3) + end + end end diff --git a/lib/mobilizon_web/schema/comment.ex b/lib/mobilizon_web/schema/comment.ex index 61a4569fa..196347782 100644 --- a/lib/mobilizon_web/schema/comment.ex +++ b/lib/mobilizon_web/schema/comment.ex @@ -3,6 +3,7 @@ defmodule MobilizonWeb.Schema.CommentType do Schema representation for Comment """ use Absinthe.Schema.Notation + alias MobilizonWeb.Resolvers.Comment @desc "A comment" object :comment do @@ -29,4 +30,14 @@ defmodule MobilizonWeb.Schema.CommentType do value(:moderated, description: "Visible only after a moderator accepted") value(:invite, description: "visible only to people invited") end + + object :comment_mutations do + @desc "Create a comment" + field :create_comment, type: :comment do + arg(:text, non_null(:string)) + arg(:actor_username, non_null(:string)) + + resolve(&Comment.create_comment/3) + end + end end diff --git a/lib/mobilizon_web/schema/event.ex b/lib/mobilizon_web/schema/event.ex index a06409243..003dee4bf 100644 --- a/lib/mobilizon_web/schema/event.ex +++ b/lib/mobilizon_web/schema/event.ex @@ -8,6 +8,7 @@ defmodule MobilizonWeb.Schema.EventType do import_types(MobilizonWeb.Schema.AddressType) import_types(MobilizonWeb.Schema.Events.ParticipantType) import_types(MobilizonWeb.Schema.Events.CategoryType) + alias MobilizonWeb.Resolvers @desc "An event" object :event do @@ -70,4 +71,49 @@ defmodule MobilizonWeb.Schema.EventType do value(:confirmed, description: "The event is confirmed") value(:cancelled, description: "The event is cancelled") end + + object :event_queries do + @desc "Get all events" + field :events, list_of(:event) do + arg(:page, :integer, default_value: 1) + arg(:limit, :integer, default_value: 10) + resolve(&Resolvers.Event.list_events/3) + end + + @desc "Get an event by uuid" + field :event, :event do + arg(:uuid, non_null(:uuid)) + resolve(&Resolvers.Event.find_event/3) + end + end + + object :event_mutations do + @desc "Create an event" + field :create_event, type: :event do + arg(:title, non_null(:string)) + arg(:description, non_null(:string)) + arg(:begins_on, non_null(:datetime)) + arg(:ends_on, :datetime) + arg(:state, :integer) + arg(:status, :integer) + arg(:public, :boolean) + arg(:thumbnail, :string) + arg(:large_image, :string) + arg(:publish_at, :datetime) + arg(:online_address, :string) + arg(:phone_address, :string) + arg(:organizer_actor_id, non_null(:id)) + arg(:category, non_null(:string)) + + resolve(&Resolvers.Event.create_event/3) + end + + @desc "Delete an event" + field :delete_event, :deleted_object do + arg(:event_id, non_null(:integer)) + arg(:actor_id, non_null(:integer)) + + resolve(&Resolvers.Event.delete_event/3) + end + end end diff --git a/lib/mobilizon_web/schema/events/category.ex b/lib/mobilizon_web/schema/events/category.ex index 25e263fe6..521000ac7 100644 --- a/lib/mobilizon_web/schema/events/category.ex +++ b/lib/mobilizon_web/schema/events/category.ex @@ -3,6 +3,7 @@ defmodule MobilizonWeb.Schema.Events.CategoryType do Schema representation for Category """ use Absinthe.Schema.Notation + alias MobilizonWeb.Resolvers @desc "A category" object :category do @@ -11,4 +12,23 @@ defmodule MobilizonWeb.Schema.Events.CategoryType do field(:picture, :picture, description: "The category's picture") field(:title, :string, description: "The category's title") end + + object :category_queries do + @desc "Get the list of categories" + field :categories, non_null(list_of(:category)) do + arg(:page, :integer, default_value: 1) + arg(:limit, :integer, default_value: 10) + resolve(&Resolvers.Category.list_categories/3) + end + end + + object :category_mutations do + @desc "Create a category with a title, description and picture" + field :create_category, type: :category do + arg(:title, non_null(:string)) + arg(:description, non_null(:string)) + arg(:picture, non_null(:upload)) + resolve(&Resolvers.Category.create_category/3) + end + end end diff --git a/lib/mobilizon_web/schema/events/participant.ex b/lib/mobilizon_web/schema/events/participant.ex index 47a6b36b9..f811299af 100644 --- a/lib/mobilizon_web/schema/events/participant.ex +++ b/lib/mobilizon_web/schema/events/participant.ex @@ -4,6 +4,7 @@ defmodule MobilizonWeb.Schema.Events.ParticipantType do """ use Absinthe.Schema.Notation import Absinthe.Resolution.Helpers, only: [dataloader: 1] + alias MobilizonWeb.Resolvers @desc "Represents a participant to an event" object :participant do @@ -15,4 +16,14 @@ defmodule MobilizonWeb.Schema.Events.ParticipantType do field(:actor, :actor, description: "The actor that participates to the event") field(:role, :integer, description: "The role of this actor at this event") end + + object :participant_queries do + @desc "Get all participants for an event uuid" + field :participants, list_of(:participant) do + arg(:uuid, non_null(:uuid)) + arg(:page, :integer, default_value: 1) + arg(:limit, :integer, default_value: 10) + resolve(&Resolvers.Event.list_participants_for_event/3) + end + end end diff --git a/lib/mobilizon_web/schema/user.ex b/lib/mobilizon_web/schema/user.ex index fa34d5aa4..c0eba4562 100644 --- a/lib/mobilizon_web/schema/user.ex +++ b/lib/mobilizon_web/schema/user.ex @@ -3,6 +3,8 @@ defmodule MobilizonWeb.Schema.UserType do Schema representation for User """ use Absinthe.Schema.Notation + alias MobilizonWeb.Resolvers.User + import MobilizonWeb.Schema.Utils @desc "A local user of Mobilizon" object :user do @@ -33,4 +35,68 @@ defmodule MobilizonWeb.Schema.UserType do description: "The token sent when requesting password token" ) end + + object :user_queries do + @desc "Get an user" + field :user, :user do + arg(:id, non_null(:id)) + resolve(&User.find_user/3) + end + + @desc "Get the current user" + field :logged_user, :user do + resolve(&User.get_current_user/3) + end + end + + object :user_mutations do + @desc "Create an user" + field :create_user, type: :user do + arg(:email, non_null(:string)) + arg(:password, non_null(:string)) + + resolve(handle_errors(&User.create_user/3)) + end + + @desc "Validate an user after registration" + field :validate_user, type: :login do + arg(:token, non_null(:string)) + resolve(&User.validate_user/3) + end + + @desc "Resend registration confirmation token" + field :resend_confirmation_email, type: :string do + arg(:email, non_null(:string)) + arg(:locale, :string, default_value: "en") + resolve(&User.resend_confirmation_email/3) + end + + @desc "Send a link through email to reset user password" + field :send_reset_password, type: :string do + arg(:email, non_null(:string)) + arg(:locale, :string, default_value: "en") + resolve(&User.send_reset_password/3) + end + + @desc "Reset user password" + field :reset_password, type: :login do + arg(:token, non_null(:string)) + arg(:password, non_null(:string)) + arg(:locale, :string, default_value: "en") + resolve(&User.reset_password/3) + end + + @desc "Login an user" + field :login, :login do + arg(:email, non_null(:string)) + arg(:password, non_null(:string)) + resolve(&User.login_user/3) + end + + @desc "Change default actor for user" + field :change_default_actor, :user do + arg(:preferred_username, non_null(:string)) + resolve(&User.change_default_actor/3) + end + end end diff --git a/lib/mobilizon_web/schema/utils.ex b/lib/mobilizon_web/schema/utils.ex new file mode 100644 index 000000000..a2bf553df --- /dev/null +++ b/lib/mobilizon_web/schema/utils.ex @@ -0,0 +1,21 @@ +defmodule MobilizonWeb.Schema.Utils do + def handle_errors(fun) do + fn source, args, info -> + case Absinthe.Resolution.call(fun, source, args, info) do + {:error, %Ecto.Changeset{} = changeset} -> format_changeset(changeset) + val -> val + end + end + end + + def format_changeset(changeset) do + # {:error, [email: {"has already been taken", []}]} + errors = + changeset.errors + |> Enum.map(fn {_key, {value, context}} -> + [message: "#{value}", details: context] + end) + + {:error, errors} + end +end From 640bb878e8472b648d0c88cd4790b567fbef0dc5 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Tue, 29 Jan 2019 11:02:32 +0100 Subject: [PATCH 3/4] Introduce registerPerson mutation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To register a profile from an unactivated user Signed-off-by: Thomas Citharel 👤 Fix Person interface use Signed-off-by: Thomas Citharel Change host function for data property Signed-off-by: Thomas Citharel --- js/src/graphql/actor.ts | 28 ++- js/src/graphql/user.ts | 4 +- js/src/router/index.ts | 50 +---- js/src/router/user.ts | 59 ++++++ js/src/types/actor.model.ts | 2 +- js/src/views/Account/Register.vue | 147 ++++++--------- js/src/views/User/Register.vue | 177 ++++++++++++++++++ lib/mobilizon/actors/actor.ex | 12 ++ lib/mobilizon/actors/user.ex | 17 +- lib/mobilizon_web/resolvers/person.ex | 23 ++- lib/mobilizon_web/schema/actors/person.ex | 20 +- lib/mobilizon_web/schema/utils.ex | 4 +- .../resolvers/user_resolver_test.exs | 148 ++++++++++++++- 13 files changed, 519 insertions(+), 172 deletions(-) create mode 100644 js/src/router/user.ts create mode 100644 js/src/views/User/Register.vue diff --git a/js/src/graphql/actor.ts b/js/src/graphql/actor.ts index 6fa5a65a2..5167e7ff6 100644 --- a/js/src/graphql/actor.ts +++ b/js/src/graphql/actor.ts @@ -39,10 +39,34 @@ query { export const CREATE_PERSON = gql` mutation CreatePerson($preferredUsername: String!) { - createPerson(preferredUsername: $preferredUsername) { + createPerson( + preferredUsername: $preferredUsername, + name: $name, + summary: $summary + ) { preferredUsername, name, + summary, avatarUrl } } -` \ No newline at end of file +`; + +/** + * This one is used only to register the first account. Prefer CREATE_PERSON when creating another identity + */ +export const REGISTER_PERSON = gql` +mutation ($preferredUsername: String!, $name: String!, $summary: String!, $email: String!) { + registerPerson( + preferredUsername: $preferredUsername, + name: $name, + summary: $summary, + email: $email + ) { + preferredUsername, + name, + summary, + avatarUrl, + } +} +`; diff --git a/js/src/graphql/user.ts b/js/src/graphql/user.ts index 9031370e5..0db4b5486 100644 --- a/js/src/graphql/user.ts +++ b/js/src/graphql/user.ts @@ -1,8 +1,8 @@ import gql from 'graphql-tag'; export const CREATE_USER = gql` -mutation CreateUser($email: String!, $username: String!, $password: String!) { - createUser(email: $email, username: $username, password: $password) { +mutation CreateUser($email: String!, $password: String!) { + createUser(email: $email, password: $password) { email, confirmationSentAt } diff --git a/js/src/router/index.ts b/js/src/router/index.ts index 389cffbf5..817b50dd2 100644 --- a/js/src/router/index.ts +++ b/js/src/router/index.ts @@ -8,17 +8,12 @@ import Location from '@/views/Location.vue'; import CreateEvent from '@/views/Event/Create.vue'; import CategoryList from '@/views/Category/List.vue'; import CreateCategory from '@/views/Category/Create.vue'; -import Register from '@/views/Account/Register.vue'; -import Login from '@/views/User/Login.vue'; -import Validate from '@/views/User/Validate.vue'; -import ResendConfirmation from '@/views/User/ResendConfirmation.vue'; -import SendPasswordReset from '@/views/User/SendPasswordReset.vue'; -import PasswordReset from '@/views/User/PasswordReset.vue'; import Profile from '@/views/Account/Profile.vue'; import CreateGroup from '@/views/Group/Create.vue'; import Group from '@/views/Group/Group.vue'; import GroupList from '@/views/Group/GroupList.vue'; import Identities from '@/views/Account/Identities.vue'; +import userRoutes from './user'; Vue.use(Router); @@ -26,6 +21,7 @@ const router = new Router({ mode: 'history', base: '/', routes: [ + ...userRoutes, { path: '/', name: 'Home', @@ -69,48 +65,6 @@ const router = new Router({ component: CreateCategory, meta: { requiredAuth: true }, }, - { - path: '/register', - name: 'Register', - component: Register, - props: true, - meta: { requiredAuth: false }, - }, - { - path: '/resend-instructions', - name: 'ResendConfirmation', - component: ResendConfirmation, - props: true, - meta: { requiresAuth: false }, - }, - { - path: '/password-reset/send', - name: 'SendPasswordReset', - component: SendPasswordReset, - props: true, - meta: { requiresAuth: false }, - }, - { - path: '/password-reset/:token', - name: 'PasswordReset', - component: PasswordReset, - meta: { requiresAuth: false }, - props: true, - }, - { - path: '/validate/:token', - name: 'Validate', - component: Validate, - props: true, - meta: { requiresAuth: false }, - }, - { - path: '/login', - name: 'Login', - component: Login, - props: true, - meta: { requiredAuth: false }, - }, { path: '/identities', name: 'Identities', diff --git a/js/src/router/user.ts b/js/src/router/user.ts new file mode 100644 index 000000000..bdec67529 --- /dev/null +++ b/js/src/router/user.ts @@ -0,0 +1,59 @@ +import RegisterUser from '@/views/User/Register.vue'; +import RegisterProfile from '@/views/Account/Register.vue'; +import Login from '@/views/User/Login.vue'; +import Validate from '@/views/User/Validate.vue'; +import ResendConfirmation from '@/views/User/ResendConfirmation.vue'; +import SendPasswordReset from '@/views/User/SendPasswordReset.vue'; +import PasswordReset from '@/views/User/PasswordReset.vue'; + +export default [ + { + path: '/register/user', + name: 'Register', + component: RegisterUser, + props: true, + meta: { requiredAuth: false }, + }, + { + path: '/register/profile', + name: 'RegisterProfile', + component: RegisterProfile, + props: true, + meta: { requiredAuth: false }, + }, + { + path: '/resend-instructions', + name: 'ResendConfirmation', + component: ResendConfirmation, + props: true, + meta: { requiresAuth: false }, + }, + { + path: '/password-reset/send', + name: 'SendPasswordReset', + component: SendPasswordReset, + props: true, + meta: { requiresAuth: false }, + }, + { + path: '/password-reset/:token', + name: 'PasswordReset', + component: PasswordReset, + meta: { requiresAuth: false }, + props: true, + }, + { + path: '/validate/:token', + name: 'Validate', + component: Validate, + props: true, + meta: { requiresAuth: false }, + }, + { + path: '/login', + name: 'Login', + component: Login, + props: true, + meta: { requiredAuth: false }, + }, +]; \ No newline at end of file diff --git a/js/src/types/actor.model.ts b/js/src/types/actor.model.ts index 78891e493..a5803c0b2 100644 --- a/js/src/types/actor.model.ts +++ b/js/src/types/actor.model.ts @@ -2,7 +2,7 @@ export interface IActor { id: string; url: string; name: string; - domain: string; + domain: string|null; summary: string; preferredUsername: string; suspended: boolean; diff --git a/js/src/views/Account/Register.vue b/js/src/views/Account/Register.vue index 543c23ff6..a644049f0 100644 --- a/js/src/views/Account/Register.vue +++ b/js/src/views/Account/Register.vue @@ -10,102 +10,63 @@
-
-
-

Features

-
    -
  • Create your communities and your events
  • -
  • Other stuff…
  • -
-
-

- Learn more on - joinmobilizon.org -

-
-
-

About this instance

-

- Your local administrator resumed it's policy: -

-
    -
  • Please be nice to each other
  • -
  • meditate a bit
  • -
-

- Please read the full rules -

-
-
- +
- - + + + +

+ @{{ host }} +

+
- - + + - - + +
-
- - Didn't receive the instructions ? - -
-
- - Login - -
-

- A validation email was sent to %{email} +

+ Your account is nearly ready, %{username}

+

+ A validation email was sent to %{email} +

Before you can login, you need to click on the link inside it to validate your account

@@ -120,8 +81,9 @@ + + diff --git a/lib/mobilizon/actors/actor.ex b/lib/mobilizon/actors/actor.ex index 8dacdfa5c..b264eee4a 100644 --- a/lib/mobilizon/actors/actor.ex +++ b/lib/mobilizon/actors/actor.ex @@ -103,6 +103,8 @@ defmodule Mobilizon.Actors.Actor do :user_id ]) |> build_urls() + # Needed because following constraint can't work for domain null values (local) + |> unique_username_validator() |> unique_constraint(:preferred_username, name: :actors_preferred_username_domain_type_index) |> unique_constraint(:url, name: :actors_url_index) |> validate_required([:preferred_username, :keys, :suspended, :url, :type]) @@ -177,6 +179,16 @@ defmodule Mobilizon.Actors.Actor do |> put_change(:local, true) end + def unique_username_validator( + %Ecto.Changeset{changes: %{preferred_username: username}} = changeset + ) do + if Actors.get_local_actor_by_name(username) do + changeset |> add_error(:preferred_username, "Username is already taken") + else + changeset + end + end + @spec build_urls(Ecto.Changeset.t(), atom()) :: Ecto.Changeset.t() defp build_urls(changeset, type \\ :Person) diff --git a/lib/mobilizon/actors/user.ex b/lib/mobilizon/actors/user.ex index 9fc1c6792..d69be537b 100644 --- a/lib/mobilizon/actors/user.ex +++ b/lib/mobilizon/actors/user.ex @@ -30,6 +30,7 @@ defmodule Mobilizon.Actors.User do |> cast(attrs, [ :email, :role, + :password, :password_hash, :confirmed_at, :confirmation_sent_at, @@ -38,13 +39,13 @@ defmodule Mobilizon.Actors.User do :reset_password_token ]) |> validate_required([:email]) - |> unique_constraint(:email, message: "registration.error.email_already_used") - |> validate_format(:email, ~r/@/) + |> unique_constraint(:email, message: "This email is already used.") + |> validate_email() |> validate_length( :password, min: 6, max: 100, - message: "registration.error.password_too_short" + message: "The choosen password is too short." ) if Map.has_key?(attrs, :default_actor) do @@ -57,21 +58,13 @@ defmodule Mobilizon.Actors.User do def registration_changeset(struct, params) do struct |> changeset(params) - |> cast(params, ~w(password)a, []) |> cast_assoc(:default_actor) |> validate_required([:email, :password]) - |> validate_email() - |> validate_length( - :password, - min: 6, - max: 100, - message: "registration.error.password_too_short" - ) |> hash_password() |> save_confirmation_token() |> unique_constraint( :confirmation_token, - message: "regisration.error.confirmation_token_already_in_use" + message: "The registration is already in use, this looks like an issue on our side." ) end diff --git a/lib/mobilizon_web/resolvers/person.ex b/lib/mobilizon_web/resolvers/person.ex index 347a70a7e..2258d49d7 100644 --- a/lib/mobilizon_web/resolvers/person.ex +++ b/lib/mobilizon_web/resolvers/person.ex @@ -3,7 +3,7 @@ defmodule MobilizonWeb.Resolvers.Person do Handles the person-related GraphQL calls """ alias Mobilizon.Actors - alias Mobilizon.Actors.Actor + alias Mobilizon.Actors.{Actor, User} alias Mobilizon.Service.ActivityPub @deprecated "Use find_person/3 or find_group/3 instead" @@ -52,6 +52,9 @@ defmodule MobilizonWeb.Resolvers.Person do {:error, "You need to be logged-in to view your list of identities"} end + @doc """ + This function is used to create more identities from an existing user + """ def create_person(_parent, %{preferred_username: _preferred_username} = args, %{ context: %{current_user: user} }) do @@ -59,9 +62,23 @@ defmodule MobilizonWeb.Resolvers.Person do with {:ok, %Actor{} = new_person} <- Actors.new_person(args) do {:ok, new_person} + end + end + + @doc """ + This function is used to register a person afterwards the user has been created (but not activated) + """ + def register_person(_parent, args, _resolution) do + with {:ok, %User{} = user} <- Actors.get_user_by_email(args.email), + {:no_actor, nil} <- {:no_actor, Actors.get_actor_for_user(user)}, + args <- Map.put(args, :user_id, user.id), + {:ok, %Actor{} = new_person} <- Actors.new_person(args) do + {:ok, new_person} else - {:error, %Ecto.Changeset{} = _e} -> - {:error, "Unable to create a profile with this username"} + {:error, :user_not_found} -> + {:error, "User with email not found"} + {:no_actor, _} -> + {:error, "You already have a profile for this user"} end end end diff --git a/lib/mobilizon_web/schema/actors/person.ex b/lib/mobilizon_web/schema/actors/person.ex index 70610a324..4397caf6e 100644 --- a/lib/mobilizon_web/schema/actors/person.ex +++ b/lib/mobilizon_web/schema/actors/person.ex @@ -6,6 +6,7 @@ defmodule MobilizonWeb.Schema.Actors.PersonType do import Absinthe.Resolution.Helpers, only: [dataloader: 1] alias Mobilizon.Events alias MobilizonWeb.Resolvers + import MobilizonWeb.Schema.Utils @desc """ Represents a person identity @@ -69,11 +70,24 @@ defmodule MobilizonWeb.Schema.Actors.PersonType do @desc "Create a new person for user" field :create_person, :person do arg(:preferred_username, non_null(:string)) - arg(:name, :string, description: "The displayed name for the new profile") - arg(:description, :string, description: "The summary for the new profile", default_value: "") + arg(:name, :string, description: "The displayed name for the new profile", default_value: "") - resolve(&Resolvers.Person.create_person/3) + arg(:summary, :string, description: "The summary for the new profile", default_value: "") + + resolve(handle_errors(&Resolvers.Person.create_person/3)) + end + + @desc "Register a first profile on registration" + field :register_person, :person do + arg(:preferred_username, non_null(:string)) + + arg(:name, :string, description: "The displayed name for the new profile", default_value: "") + + arg(:summary, :string, description: "The summary for the new profile", default_value: "") + arg(:email, non_null(:string), description: "The email from the user previously created") + + resolve(handle_errors(&Resolvers.Person.register_person/3)) end end end diff --git a/lib/mobilizon_web/schema/utils.ex b/lib/mobilizon_web/schema/utils.ex index a2bf553df..f7371ff8f 100644 --- a/lib/mobilizon_web/schema/utils.ex +++ b/lib/mobilizon_web/schema/utils.ex @@ -12,8 +12,8 @@ defmodule MobilizonWeb.Schema.Utils do # {:error, [email: {"has already been taken", []}]} errors = changeset.errors - |> Enum.map(fn {_key, {value, context}} -> - [message: "#{value}", details: context] + |> Enum.map(fn {key, {value, _context}} -> + [message: "#{value}", details: key] end) {:error, errors} diff --git a/test/mobilizon_web/resolvers/user_resolver_test.exs b/test/mobilizon_web/resolvers/user_resolver_test.exs index de969f41e..9381296bb 100644 --- a/test/mobilizon_web/resolvers/user_resolver_test.exs +++ b/test/mobilizon_web/resolvers/user_resolver_test.exs @@ -73,21 +73,25 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do end describe "Resolver: Create an user & actor" do - @account_creation %{ + @user_creation %{ email: "test@demo.tld", - password: "long password" + password: "long password", + username: "toto", + name: "Sir Toto", + summary: "Sir Toto, prince of the functional tests" } - @account_creation_bad_email %{ + @user_creation_bad_email %{ email: "y@l@", password: "long password" } - test "test create_user/3 creates an user", context do + test "test create_user/3 creates an user and register_person/3 registers a profile", + context do mutation = """ mutation { createUser( - email: "#{@account_creation.email}", - password: "#{@account_creation.password}", + email: "#{@user_creation.email}", + password: "#{@user_creation.password}", ) { id, email @@ -99,15 +103,141 @@ defmodule MobilizonWeb.Resolvers.UserResolverTest do context.conn |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) - assert json_response(res, 200)["data"]["createUser"]["email"] == @account_creation.email + assert json_response(res, 200)["data"]["createUser"]["email"] == @user_creation.email + + mutation = """ + mutation { + registerPerson( + preferredUsername: "#{@user_creation.username}", + name: "#{@user_creation.name}", + summary: "#{@user_creation.summary}", + email: "#{@user_creation.email}", + ) { + preferredUsername, + name, + summary, + avatarUrl, + } + } + """ + + res = + context.conn + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + assert json_response(res, 200)["data"]["registerPerson"]["preferredUsername"] == + @user_creation.username + end + + test "register_person/3 doesn't register a profile from an unknown email", context do + mutation = """ + mutation { + createUser( + email: "#{@user_creation.email}", + password: "#{@user_creation.password}", + ) { + id, + email + } + } + """ + + context.conn + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + mutation = """ + mutation { + registerPerson( + preferredUsername: "#{@user_creation.username}", + name: "#{@user_creation.name}", + summary: "#{@user_creation.summary}", + email: "random", + ) { + preferredUsername, + name, + summary, + avatarUrl, + } + } + """ + + res = + context.conn + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + assert hd(json_response(res, 200)["errors"])["message"] == + "User with email not found" + end + + test "register_person/3 can't be called with an existing profile", context do + mutation = """ + mutation { + createUser( + email: "#{@user_creation.email}", + password: "#{@user_creation.password}", + ) { + id, + email + } + } + """ + + context.conn + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + mutation = """ + mutation { + registerPerson( + preferredUsername: "#{@user_creation.username}", + name: "#{@user_creation.name}", + summary: "#{@user_creation.summary}", + email: "#{@user_creation.email}", + ) { + preferredUsername, + name, + summary, + avatarUrl, + } + } + """ + + res = + context.conn + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + assert json_response(res, 200)["data"]["registerPerson"]["preferredUsername"] == + @user_creation.username + + mutation = """ + mutation { + registerPerson( + preferredUsername: "#{@user_creation.username}", + name: "#{@user_creation.name}", + summary: "#{@user_creation.summary}", + email: "#{@user_creation.email}", + ) { + preferredUsername, + name, + summary, + avatarUrl, + } + } + """ + + res = + context.conn + |> post("/api", AbsintheHelpers.mutation_skeleton(mutation)) + + assert hd(json_response(res, 200)["errors"])["message"] == + "You already have a profile for this user" end test "test create_user/3 doesn't create an user with bad email", context do mutation = """ mutation { createUser( - email: "#{@account_creation_bad_email.email}", - password: "#{@account_creation.password}", + email: "#{@user_creation_bad_email.email}", + password: "#{@user_creation_bad_email.password}", ) { id, email From 1b470f3f044eec43bbc636722147171cdead30c3 Mon Sep 17 00:00:00 2001 From: Thomas Citharel Date: Wed, 30 Jan 2019 15:54:21 +0100 Subject: [PATCH 4/4] Handle validated users without profiles Signed-off-by: Thomas Citharel Format Signed-off-by: Thomas Citharel --- js/src/graphql/user.ts | 6 +++++- js/src/router/user.ts | 3 ++- js/src/views/Account/Register.vue | 11 ++++++++--- js/src/views/User/Validate.vue | 21 ++++++++++++++------- lib/mobilizon_web/resolvers/person.ex | 4 ++++ lib/mobilizon_web/resolvers/user.ex | 2 +- 6 files changed, 34 insertions(+), 13 deletions(-) diff --git a/js/src/graphql/user.ts b/js/src/graphql/user.ts index 0db4b5486..07a16aba0 100644 --- a/js/src/graphql/user.ts +++ b/js/src/graphql/user.ts @@ -15,6 +15,10 @@ mutation ValidateUser($token: String!) { token, user { id, + email, + defaultActor { + id + } } } } @@ -33,4 +37,4 @@ export const UPDATE_CURRENT_USER_CLIENT = gql` mutation UpdateCurrentUser($id: Int!, $email: String!) { updateCurrentUser(id: $id, email: $email) @client } -` +`; diff --git a/js/src/router/user.ts b/js/src/router/user.ts index bdec67529..811723730 100644 --- a/js/src/router/user.ts +++ b/js/src/router/user.ts @@ -46,7 +46,8 @@ export default [ path: '/validate/:token', name: 'Validate', component: Validate, - props: true, + // We can only pass string values through params, therefore + props: (route) => ({ email: route.params.email, userAlreadyActivated: route.params.userAlreadyActivated === 'true'}), meta: { requiresAuth: false }, }, { diff --git a/js/src/views/Account/Register.vue b/js/src/views/Account/Register.vue index a644049f0..0e9b2a727 100644 --- a/js/src/views/Account/Register.vue +++ b/js/src/views/Account/Register.vue @@ -57,7 +57,7 @@ -
+

{ acc[error.details] = error.message; diff --git a/js/src/views/User/Validate.vue b/js/src/views/User/Validate.vue index 801a13a9a..9187aed12 100644 --- a/js/src/views/User/Validate.vue +++ b/js/src/views/User/Validate.vue @@ -5,8 +5,8 @@

- - Error while validating account + + Either the account is already validated, either the validation token is incorrect.

@@ -28,21 +28,28 @@ export default class Validate extends Vue { loading = true; failed = false; - created() { - this.validateAction(); + async created() { + await this.validateAction(); } async validateAction() { try { - const data = await this.$apollo.mutate({ + const { data } = await this.$apollo.mutate({ mutation: VALIDATE_USER, variables: { token: this.token } }); - this.saveUserData(data.data); - this.$router.push({ name: "Home" }); + this.saveUserData(data); + + const user = data.validateUser.user; + console.log(user); + if (user.defaultActor) { + this.$router.push({name: "Home"}); + } else { // If the user didn't register any profile yet, let's create one for them + this.$router.push({ name: 'RegisterProfile', params: {email: user.email, userAlreadyActivated: 'true'} }); + } } catch (err) { console.error(err); this.failed = true; diff --git a/lib/mobilizon_web/resolvers/person.ex b/lib/mobilizon_web/resolvers/person.ex index 2258d49d7..8a3d4ba75 100644 --- a/lib/mobilizon_web/resolvers/person.ex +++ b/lib/mobilizon_web/resolvers/person.ex @@ -77,8 +77,12 @@ defmodule MobilizonWeb.Resolvers.Person do else {:error, :user_not_found} -> {:error, "User with email not found"} + {:no_actor, _} -> {:error, "You already have a profile for this user"} + + {:error, %Ecto.Changeset{} = e} -> + {:error, e} end end end diff --git a/lib/mobilizon_web/resolvers/user.ex b/lib/mobilizon_web/resolvers/user.ex index f8d45fbb7..12594c6a6 100644 --- a/lib/mobilizon_web/resolvers/user.ex +++ b/lib/mobilizon_web/resolvers/user.ex @@ -63,7 +63,7 @@ defmodule MobilizonWeb.Resolvers.User do {:get_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}} + {:ok, %{token: token, user: Map.put(user, :default_actor, actor)}} else err -> Logger.info("Unable to validate user with token #{token}")