From f316f0a940da291af62a775ca3f6f2d97eced9bb Mon Sep 17 00:00:00 2001 From: miffy Date: Sat, 7 Sep 2019 19:54:11 +0200 Subject: [PATCH] Refactoring of Users context --- lib/mobilizon/crypto.ex | 15 + lib/mobilizon/users/user.ex | 170 ++++---- lib/mobilizon/users/users.ex | 412 +++++++++--------- lib/mobilizon_web/api/groups.ex | 15 +- lib/mobilizon_web/resolvers/event.ex | 122 ++---- lib/mobilizon_web/resolvers/feed_token.ex | 7 +- lib/mobilizon_web/resolvers/group.ex | 82 +--- lib/mobilizon_web/resolvers/person.ex | 36 +- lib/mobilizon_web/resolvers/picture.ex | 37 +- lib/mobilizon_web/resolvers/report.ex | 45 +- lib/mobilizon_web/resolvers/user.ex | 10 +- mix.exs | 2 +- .../20190307125009_move_user_role_to_enum.exs | 8 +- .../resolvers/report_resolver_test.exs | 3 +- 14 files changed, 421 insertions(+), 543 deletions(-) create mode 100644 lib/mobilizon/crypto.ex diff --git a/lib/mobilizon/crypto.ex b/lib/mobilizon/crypto.ex new file mode 100644 index 000000000..9068c8787 --- /dev/null +++ b/lib/mobilizon/crypto.ex @@ -0,0 +1,15 @@ +defmodule Mobilizon.Crypto do + @moduledoc """ + Utility module which contains cryptography related functions. + """ + + @doc """ + Returns random byte sequence of the length encoded to Base64. + """ + @spec random_string(integer) :: String.t() + def random_string(length) do + length + |> :crypto.strong_rand_bytes() + |> Base.url_encode64() + end +end diff --git a/lib/mobilizon/users/user.ex b/lib/mobilizon/users/user.ex index 449f4b1cb..eb8895108 100644 --- a/lib/mobilizon/users/user.ex +++ b/lib/mobilizon/users/user.ex @@ -1,63 +1,80 @@ -import EctoEnum - -defenum(Mobilizon.Users.UserRoleEnum, :user_role_type, [ - :administrator, - :moderator, - :user -]) - defmodule Mobilizon.Users.User do @moduledoc """ - Represents a local user + Represents a local user. """ + use Ecto.Schema + import Ecto.Changeset + alias Mobilizon.Actors.Actor - alias Mobilizon.Users.User - alias Mobilizon.Service.EmailChecker + alias Mobilizon.Crypto alias Mobilizon.Events.FeedToken + alias Mobilizon.Service.EmailChecker + alias Mobilizon.Users.{User, UserRole} + + @type t :: %__MODULE__{ + email: String.t(), + password_hash: String.t(), + password: String.t(), + role: UserRole.t(), + confirmed_at: DateTime.t(), + confirmation_sent_at: DateTime.t(), + confirmation_token: String.t(), + reset_password_sent_at: DateTime.t(), + reset_password_token: String.t(), + default_actor: Actor.t(), + actors: [Actor.t()], + feed_tokens: [FeedToken.t()] + } + + @required_attrs [:email] + @optional_attrs [ + :role, + :password, + :password_hash, + :confirmed_at, + :confirmation_sent_at, + :confirmation_token, + :reset_password_sent_at, + :reset_password_token + ] + @attrs @required_attrs ++ @optional_attrs + + @registration_required_attrs [:email, :password] + + @password_reset_required_attrs [:password, :reset_password_token, :reset_password_sent_at] + + @confirmation_token_length 30 schema "users" do field(:email, :string) field(:password_hash, :string) field(:password, :string, virtual: true) - field(:role, Mobilizon.Users.UserRoleEnum, default: :user) - has_many(:actors, Actor) - belongs_to(:default_actor, Actor) + field(:role, UserRole, default: :user) field(:confirmed_at, :utc_datetime) field(:confirmation_sent_at, :utc_datetime) field(:confirmation_token, :string) field(:reset_password_sent_at, :utc_datetime) field(:reset_password_token, :string) + + belongs_to(:default_actor, Actor) + has_many(:actors, Actor) has_many(:feed_tokens, FeedToken, foreign_key: :user_id) timestamps() end @doc false + @spec changeset(t | Ecto.Changeset.t(), map) :: Ecto.Changeset.t() def changeset(%User{} = user, attrs) do changeset = user - |> cast(attrs, [ - :email, - :role, - :password, - :password_hash, - :confirmed_at, - :confirmation_sent_at, - :confirmation_token, - :reset_password_sent_at, - :reset_password_token - ]) - |> validate_required([:email]) + |> cast(attrs, @attrs) + |> validate_required(@required_attrs) |> unique_constraint(:email, message: "This email is already used.") |> validate_email() - |> validate_length( - :password, - min: 6, - max: 100, - message: "The chosen password is too short." - ) + |> validate_length(:password, min: 6, max: 100, message: "The chosen password is too short.") if Map.has_key?(attrs, :default_actor) do put_assoc(changeset, :default_actor, attrs.default_actor) @@ -66,11 +83,13 @@ defmodule Mobilizon.Users.User do end end - def registration_changeset(struct, params) do - struct - |> changeset(params) + @doc false + @spec registration_changeset(User.t(), map) :: Ecto.Changeset.t() + def registration_changeset(%User{} = user, attrs) do + user + |> changeset(attrs) |> cast_assoc(:default_actor) - |> validate_required([:email, :password]) + |> validate_required(@registration_required_attrs) |> hash_password() |> save_confirmation_token() |> unique_constraint( @@ -79,16 +98,18 @@ defmodule Mobilizon.Users.User do ) end + @doc false + @spec send_password_reset_changeset(User.t(), map) :: Ecto.Changeset.t() def send_password_reset_changeset(%User{} = user, attrs) do - user - |> cast(attrs, [:reset_password_token, :reset_password_sent_at]) + cast(user, attrs, [:reset_password_token, :reset_password_sent_at]) end + @doc false + @spec password_reset_changeset(User.t(), map) :: Ecto.Changeset.t() def password_reset_changeset(%User{} = user, attrs) do user - |> cast(attrs, [:password, :reset_password_token, :reset_password_sent_at]) - |> validate_length( - :password, + |> cast(attrs, @password_reset_required_attrs) + |> validate_length(:password, min: 6, max: 100, message: "registration.error.password_too_short" @@ -96,28 +117,45 @@ defmodule Mobilizon.Users.User do |> hash_password() end + @doc """ + Checks whether an user is confirmed. + """ + @spec is_confirmed(User.t()) :: boolean + def is_confirmed(%User{confirmed_at: nil}), do: false + def is_confirmed(%User{}), do: true + + @doc """ + Returns whether an user owns an actor. + """ + @spec owns_actor(User.t(), integer | String.t()) :: {:is_owned, Actor.t() | nil} + def owns_actor(%User{actors: actors}, actor_id) do + user_actor = Enum.find(actors, fn actor -> "#{actor.id}" == "#{actor_id}" end) + + {:is_owned, user_actor} + end + + @spec save_confirmation_token(Ecto.Changeset.t()) :: Ecto.Changeset.t() defp save_confirmation_token(changeset) do case changeset do %Ecto.Changeset{valid?: true, changes: %{email: _email}} -> - changeset = put_change(changeset, :confirmation_token, random_string(30)) + now = DateTime.utc_now() - put_change( - changeset, - :confirmation_sent_at, - DateTime.utc_now() |> DateTime.truncate(:second) - ) + changeset + |> put_change(:confirmation_token, Crypto.random_string(@confirmation_token_length)) + |> put_change(:confirmation_sent_at, DateTime.truncate(now, :second)) _ -> changeset end end + @spec validate_email(Ecto.Changeset.t()) :: Ecto.Changeset.t() defp validate_email(changeset) do case changeset do %Ecto.Changeset{valid?: true, changes: %{email: email}} -> case EmailChecker.valid?(email) do false -> add_error(changeset, :email, "Email doesn't fit required format") - _ -> changeset + true -> changeset end _ -> @@ -125,46 +163,14 @@ defmodule Mobilizon.Users.User do end end - defp random_string(length) do - length - |> :crypto.strong_rand_bytes() - |> Base.url_encode64() - end - - # Hash password when it's changed + @spec hash_password(Ecto.Changeset.t()) :: Ecto.Changeset.t() defp hash_password(changeset) do case changeset do %Ecto.Changeset{valid?: true, changes: %{password: password}} -> - put_change( - changeset, - :password_hash, - Argon2.hash_pwd_salt(password) - ) + put_change(changeset, :password_hash, Argon2.hash_pwd_salt(password)) _ -> changeset end end - - def is_confirmed(%User{confirmed_at: nil} = _user), do: {:error, :unconfirmed} - def is_confirmed(%User{} = user), do: {:ok, user} - - @doc """ - Returns whether an user owns an actor - """ - @spec owns_actor(struct(), String.t()) :: {:is_owned, false} | {:is_owned, true, Actor.t()} - def owns_actor(%User{} = user, actor_id) when is_binary(actor_id) do - case Integer.parse(actor_id) do - {actor_id, ""} -> owns_actor(user, actor_id) - _ -> {:is_owned, false} - end - end - - @spec owns_actor(struct(), integer()) :: {:is_owned, false} | {:is_owned, true, Actor.t()} - def owns_actor(%User{actors: actors}, actor_id) do - case Enum.find(actors, fn a -> a.id == actor_id end) do - nil -> {:is_owned, false} - actor -> {:is_owned, true, actor} - end - end end diff --git a/lib/mobilizon/users/users.ex b/lib/mobilizon/users/users.ex index 92697d931..af515c2cf 100644 --- a/lib/mobilizon/users/users.ex +++ b/lib/mobilizon/users/users.ex @@ -3,69 +3,60 @@ defmodule Mobilizon.Users do The Users context. """ - import Ecto.Query, warn: false + import Ecto.Query + import EctoEnum - alias Mobilizon.Repo import Mobilizon.Ecto alias Mobilizon.Actors.Actor + alias Mobilizon.Events + alias Mobilizon.{Page, Repo} alias Mobilizon.Users.User - @doc false - def data() do - Dataloader.Ecto.new(Repo, query: &query/2) - end + @type tokens :: %{ + required(:access_token) => String.t(), + required(:refresh_token) => String.t() + } + + defenum(UserRole, :user_role, [:administrator, :moderator, :user]) @doc false - def query(queryable, _params) do - queryable - end + @spec data :: Dataloader.Ecto.t() + def data, do: Dataloader.Ecto.new(Mobilizon.Repo, query: &query/2) + + @doc false + @spec query(Ecto.Query.t(), map) :: Ecto.Query.t() + def query(queryable, _params), do: queryable @doc """ - Register user + Registers an user. """ - @spec register(map()) :: {:ok, User.t()} | {:error, String.t()} + @spec register(map) :: {:ok, User.t()} | {:error, Ecto.Changeset.t()} def register(%{email: _email, password: _password} = args) do with {:ok, %User{} = user} <- %User{} |> User.registration_changeset(args) - |> Mobilizon.Repo.insert() do - Mobilizon.Events.create_feed_token(%{"user_id" => user.id}) + |> Repo.insert() do + Events.create_feed_token(%{"user_id" => user.id}) + {:ok, user} end end @doc """ - Gets an user by it's email - - ## Examples - - iex> get_user_by_email("test@test.tld", true) - {:ok, %Mobilizon.Users.User{}} - - iex> get_user_by_email("test@notfound.tld", false) - {:error, :user_not_found} + Gets a single user. + Raises `Ecto.NoResultsError` if the user does not exist. """ + @spec get_user!(integer | String.t()) :: User.t() + def get_user!(id), do: Repo.get!(User, id) + + @doc """ + Gets an user by its email. + """ + @spec get_user_by_email(String.t(), boolean | nil) :: + {:ok, User.t()} | {:error, :user_not_found} def get_user_by_email(email, activated \\ nil) do - query = - case activated do - nil -> - from(u in User, where: u.email == ^email, preload: :default_actor) - - true -> - from( - u in User, - where: u.email == ^email and not is_nil(u.confirmed_at), - preload: :default_actor - ) - - false -> - from( - u in User, - where: u.email == ^email and is_nil(u.confirmed_at), - preload: :default_actor - ) - end + query = user_by_email_query(email, activated) case Repo.one(query) do nil -> {:error, :user_not_found} @@ -74,45 +65,29 @@ defmodule Mobilizon.Users do end @doc """ - Get an user by it's activation token + Get an user by its activation token. """ - @spec get_user_by_activation_token(String.t()) :: Actor.t() + @spec get_user_by_activation_token(String.t()) :: Actor.t() | nil def get_user_by_activation_token(token) do - Repo.one( - from( - u in User, - where: u.confirmation_token == ^token, - preload: [:default_actor] - ) - ) + token + |> user_by_activation_token_query() + |> Repo.one() end @doc """ - Get an user by it's reset password token + Get an user by its reset password token. """ @spec get_user_by_reset_password_token(String.t()) :: Actor.t() def get_user_by_reset_password_token(token) do - Repo.one( - from( - u in User, - where: u.reset_password_token == ^token, - preload: [:default_actor] - ) - ) + token + |> user_by_reset_password_token_query() + |> Repo.one() end @doc """ - Updates a user. - - ## Examples - - iex> update_user(User{}, %{password: "coucou"}) - {:ok, %Mobilizon.Users.User{}} - - iex> update_user(User{}, %{password: nil}) - {:error, %Ecto.Changeset{}} - + Updates an user. """ + @spec update_user(User.t(), map) :: {:ok, User.t()} | {:error, Ecto.Changeset.t()} def update_user(%User{} = user, attrs) do with {:ok, %User{} = user} <- user @@ -123,65 +98,28 @@ defmodule Mobilizon.Users do end @doc """ - Deletes a User. - - ## Examples - - iex> delete_user(%User{email: "test@test.tld"}) - {:ok, %Mobilizon.Users.User{}} - - iex> delete_user(%User{}) - {:error, %Ecto.Changeset{}} - + Deletes an user. """ + @spec delete_user(User.t()) :: {:ok, User.t()} | {:error, Ecto.Changeset.t()} def delete_user(%User{} = user) do Repo.delete(user) end - # @doc """ - # Returns an `%Ecto.Changeset{}` for tracking user changes. - - # ## Examples - - # iex> change_user(%Mobilizon.Users.User{}) - # %Ecto.Changeset{data: %Mobilizon.Users.User{}} - - # """ - # def change_user(%User{} = user) do - # User.changeset(user, %{}) - # end - @doc """ - Gets a single user. - - Raises `Ecto.NoResultsError` if the User does not exist. - - ## Examples - - iex> get_user!(123) - %Mobilizon.Users.User{} - - iex> get_user!(456) - ** (Ecto.NoResultsError) - + Get an user with its actors + Raises `Ecto.NoResultsError` if the user does not exist. """ - def get_user!(id), do: Repo.get!(User, id) - - @doc """ - Get an user with it's actors - - Raises `Ecto.NoResultsError` if the User does not exist. - """ - @spec get_user_with_actors!(integer()) :: User.t() + @spec get_user_with_actors!(integer | String.t()) :: User.t() def get_user_with_actors!(id) do - user = Repo.get!(User, id) - Repo.preload(user, [:actors, :default_actor]) + id + |> get_user!() + |> Repo.preload([:actors, :default_actor]) end @doc """ - Get user with it's actors by ID + Get user with its actors. """ - @spec get_user_with_actors(integer()) :: User.t() + @spec get_user_with_actors(integer()) :: {:ok, User.t()} | {:error, String.t()} def get_user_with_actors(id) do case Repo.get(User, id) do nil -> @@ -198,21 +136,19 @@ defmodule Mobilizon.Users do end @doc """ - Returns the associated actor for an user, either the default set one or the first found + Gets the associated actor for an user, either the default set one or the first + found. """ - @spec get_actor_for_user(Mobilizon.Users.User.t()) :: Mobilizon.Actors.Actor.t() - def get_actor_for_user(%Mobilizon.Users.User{} = user) do - case Repo.one( - from( - a in Actor, - join: u in User, - on: u.default_actor_id == a.id, - where: u.id == ^user.id - ) - ) do + @spec get_actor_for_user(User.t()) :: Actor.t() | nil + def get_actor_for_user(%User{} = user) do + actor = + user + |> actor_for_user_query() + |> Repo.one() + + case actor do nil -> - case user - |> get_actors_for_user() do + case get_actors_for_user(user) do [] -> nil actors -> hd(actors) end @@ -222,94 +158,48 @@ defmodule Mobilizon.Users do end end - def get_actors_for_user(%User{id: user_id}) do - Repo.all(from(a in Actor, where: a.user_id == ^user_id)) + @doc """ + Gets actors for an user. + """ + @spec get_actors_for_user(User.t()) :: [Actor.t()] + def get_actors_for_user(%User{} = user) do + user + |> actors_for_user_query() + |> Repo.all() end @doc """ - Authenticate user + Updates user's default actor. + Raises `Ecto.NoResultsError` if the user does not exist. """ - def authenticate(%{user: user, password: password}) do - # Does password match the one stored in the database? - with true <- Argon2.verify_pass(password, user.password_hash), - # Yes, create and return the token - {:ok, tokens} <- generate_tokens(user) do - {:ok, tokens} - else - _ -> - # No, return an error - {:error, :unauthorized} - end - end - - @doc """ - Generate access token and refresh token - """ - def generate_tokens(user) do - with {:ok, access_token} <- generate_access_token(user), - {:ok, refresh_token} <- generate_refresh_token(user) do - {:ok, %{access_token: access_token, refresh_token: refresh_token}} - end - end - - defp generate_access_token(user) do - with {:ok, access_token, _claims} <- - MobilizonWeb.Guardian.encode_and_sign(user, %{}, token_type: "access") do - {:ok, access_token} - end - end - - def generate_refresh_token(user) do - with {:ok, refresh_token, _claims} <- - MobilizonWeb.Guardian.encode_and_sign(user, %{}, token_type: "refresh") do - {:ok, refresh_token} - end - end - + @spec update_user_default_actor(integer | String.t(), integer | String.t()) :: User.t() def update_user_default_actor(user_id, actor_id) do with _ <- - from( - u in User, - where: u.id == ^user_id, - update: [ - set: [ - default_actor_id: ^actor_id - ] - ] - ) + user_id + |> update_user_default_actor_query(actor_id) |> Repo.update_all([]) do - Repo.get!(User, user_id) + user_id + |> get_user!() |> Repo.preload([:default_actor]) end end @doc """ Returns the list of users. - - ## Examples - - iex> list_users() - [%Mobilizon.Users.User{}] - """ + @spec list_users(integer | nil, integer | nil, atom | nil, atom | nil) :: [User.t()] def list_users(page \\ nil, limit \\ nil, sort \\ nil, direction \\ nil) do - Repo.all( - User - |> paginate(page, limit) - |> sort(sort, direction) - ) + User + |> Page.paginate(page, limit) + |> sort(sort, direction) + |> Repo.all() end @doc """ Returns the list of administrators. - - ## Examples - - iex> list_admins() - [%Mobilizon.Users.User{role: :administrator}] - """ - def list_admins() do + @spec list_admins :: [User.t()] + def list_admins do User |> where([u], u.role == ^:administrator) |> Repo.all() @@ -317,25 +207,129 @@ defmodule Mobilizon.Users do @doc """ Returns the list of moderators. - - ## Examples - - iex> list_moderators() - [%Mobilizon.Users.User{role: :moderator}, %Mobilizon.Users.User{role: :administrator}] - """ - def list_moderators() do + @spec list_moderators :: [User.t()] + def list_moderators do User |> where([u], u.role in ^[:administrator, :moderator]) |> Repo.all() end - def count_users() do - Repo.one( - from( - u in User, - select: count(u.id) - ) + @doc """ + Counts users. + """ + @spec count_users :: integer + def count_users do + Repo.one(from(u in User, select: count(u.id))) + end + + @doc """ + Authenticate an user. + """ + @spec authenticate(User.t()) :: {:ok, tokens} | {:error, :unauthorized} + def authenticate(%{user: %User{password_hash: password_hash} = user, password: password}) do + # Does password match the one stored in the database? + if Argon2.verify_pass(password, password_hash) do + {:ok, _tokens} = generate_tokens(user) + else + {:error, :unauthorized} + end + end + + @doc """ + Generates access token and refresh token for an user. + """ + @spec generate_tokens(User.t()) :: {:ok, tokens} + def generate_tokens(user) do + with {:ok, access_token} <- generate_access_token(user), + {:ok, refresh_token} <- generate_refresh_token(user) do + {:ok, %{access_token: access_token, refresh_token: refresh_token}} + end + end + + @doc """ + Generates access token for an user. + """ + @spec generate_access_token(User.t()) :: {:ok, String.t()} + def generate_access_token(user) do + with {:ok, access_token, _claims} <- + MobilizonWeb.Guardian.encode_and_sign(user, %{}, token_type: "access") do + {:ok, access_token} + end + end + + @doc """ + Generates refresh token for an user. + """ + @spec generate_refresh_token(User.t()) :: {:ok, String.t()} + def generate_refresh_token(user) do + with {:ok, refresh_token, _claims} <- + MobilizonWeb.Guardian.encode_and_sign(user, %{}, token_type: "refresh") do + {:ok, refresh_token} + end + end + + @spec user_by_email_query(String.t(), boolean | nil) :: Ecto.Query.t() + defp user_by_email_query(email, nil) do + from(u in User, where: u.email == ^email, preload: :default_actor) + end + + defp user_by_email_query(email, true) do + from( + u in User, + where: u.email == ^email and not is_nil(u.confirmed_at), + preload: :default_actor + ) + end + + defp user_by_email_query(email, false) do + from( + u in User, + where: u.email == ^email and is_nil(u.confirmed_at), + preload: :default_actor + ) + end + + @spec user_by_activation_token_query(String.t()) :: Ecto.Query.t() + defp user_by_activation_token_query(token) do + from( + u in User, + where: u.confirmation_token == ^token, + preload: [:default_actor] + ) + end + + @spec user_by_reset_password_token_query(String.t()) :: Ecto.Query.t() + defp user_by_reset_password_token_query(token) do + from( + u in User, + where: u.reset_password_token == ^token, + preload: [:default_actor] + ) + end + + @spec actor_for_user_query(User.t()) :: Ecto.Query.t() + defp actor_for_user_query(%User{id: user_id}) do + from( + a in Actor, + join: u in User, + on: u.default_actor_id == a.id, + where: u.id == ^user_id + ) + end + + @spec actors_for_user_query(User.t()) :: Ecto.Query.t() + defp actors_for_user_query(%User{id: user_id}) do + from(a in Actor, where: a.user_id == ^user_id) + end + + @spec update_user_default_actor_query(integer | String.t(), integer | String.t()) :: + Ecto.Query.t() + defp update_user_default_actor_query(user_id, actor_id) do + from( + u in User, + where: u.id == ^user_id, + update: [set: [default_actor_id: ^actor_id]] ) end end diff --git a/lib/mobilizon_web/api/groups.ex b/lib/mobilizon_web/api/groups.ex index 7ff870670..aaac7a6b1 100644 --- a/lib/mobilizon_web/api/groups.ex +++ b/lib/mobilizon_web/api/groups.ex @@ -3,6 +3,7 @@ defmodule MobilizonWeb.API.Groups do API for Events """ alias Mobilizon.Actors + alias Mobilizon.Actors.Actor alias Mobilizon.Users.User alias Mobilizon.Service.ActivityPub alias Mobilizon.Service.ActivityPub.Utils, as: ActivityPubUtils @@ -22,21 +23,13 @@ defmodule MobilizonWeb.API.Groups do banner: _banner } = args ) do - with {:is_owned, true, actor} <- User.owns_actor(user, creator_actor_id), + with {:is_owned, %Actor{} = actor} <- User.owns_actor(user, creator_actor_id), title <- String.trim(title), {:existing_group, nil} <- {:existing_group, Actors.get_group_by_title(title)}, visibility <- Map.get(args, :visibility, :public), {content_html, tags, to, cc} <- Utils.prepare_content(actor, summary, visibility, [], nil), - group <- - ActivityPubUtils.make_group_data( - actor.url, - to, - title, - content_html, - tags, - cc - ) do + group <- ActivityPubUtils.make_group_data(actor.url, to, title, content_html, tags, cc) do ActivityPub.create(%{ to: ["https://www.w3.org/ns/activitystreams#Public"], actor: actor, @@ -47,7 +40,7 @@ defmodule MobilizonWeb.API.Groups do {:existing_group, _} -> {:error, "A group with this name already exists"} - {:is_owned, _} -> + {:is_owned, nil} -> {:error, "Actor id is not owned by authenticated user"} end end diff --git a/lib/mobilizon_web/resolvers/event.ex b/lib/mobilizon_web/resolvers/event.ex index 8eadf44a6..fee135bf5 100644 --- a/lib/mobilizon_web/resolvers/event.ex +++ b/lib/mobilizon_web/resolvers/event.ex @@ -3,6 +3,7 @@ defmodule MobilizonWeb.Resolvers.Event do Handles the event-related GraphQL calls """ alias Mobilizon.Activity + alias Mobilizon.Actors.Actor alias Mobilizon.Addresses alias Mobilizon.Addresses.Address alias Mobilizon.Events @@ -64,11 +65,8 @@ defmodule MobilizonWeb.Resolvers.Event do # We find similar events with the same tags # uniq_by : It's possible event_from_same_actor is inside events_from_tags events = - (events ++ - Events.find_similar_events_by_common_tags( - tags, - @number_of_related_events - )) + events + |> Enum.concat(Events.find_similar_events_by_common_tags(tags, @number_of_related_events)) |> uniq_events() # TODO: We should use tag_relations to find more appropriate events @@ -76,8 +74,10 @@ defmodule MobilizonWeb.Resolvers.Event do # We've considered all recommended events, so we fetch the latest events events = if @number_of_related_events - length(events) > 0 do - (events ++ - Events.list_events(1, @number_of_related_events, :begins_on, :asc, true, true)) + events + |> Enum.concat( + Events.list_events(1, @number_of_related_events, :begins_on, :asc, true, true) + ) |> uniq_events() else events @@ -101,26 +101,23 @@ defmodule MobilizonWeb.Resolvers.Event do def actor_join_event( _parent, %{actor_id: actor_id, event_id: event_id}, - %{ - context: %{ - current_user: user - } - } + %{context: %{current_user: user}} ) do - with {:is_owned, true, actor} <- User.owns_actor(user, actor_id), + with {:is_owned, %Actor{} = actor} <- User.owns_actor(user, actor_id), {:has_event, {:ok, %Event{} = event}} <- {:has_event, Mobilizon.Events.get_event_full(event_id)}, {:error, :participant_not_found} <- Mobilizon.Events.get_participant(event_id, actor_id), {:ok, _activity, participant} <- MobilizonWeb.API.Participations.join(event, actor), participant <- - Map.put(participant, :event, event) + participant + |> Map.put(:event, event) |> Map.put(:actor, Person.proxify_pictures(actor)) do {:ok, participant} else {:has_event, _} -> {:error, "Event with this ID #{inspect(event_id)} doesn't exist"} - {:is_owned, false} -> + {:is_owned, nil} -> {:error, "Actor id is not owned by authenticated user"} {:error, :event_not_found} -> @@ -141,32 +138,18 @@ defmodule MobilizonWeb.Resolvers.Event do def actor_leave_event( _parent, %{actor_id: actor_id, event_id: event_id}, - %{ - context: %{ - current_user: user - } - } + %{context: %{current_user: user}} ) do - with {:is_owned, true, actor} <- User.owns_actor(user, actor_id), + with {:is_owned, %Actor{} = actor} <- User.owns_actor(user, actor_id), {:has_event, {:ok, %Event{} = event}} <- {:has_event, Mobilizon.Events.get_event_full(event_id)}, {:ok, _activity, _participant} <- MobilizonWeb.API.Participations.leave(event, actor) do - { - :ok, - %{ - event: %{ - id: event_id - }, - actor: %{ - id: actor_id - } - } - } + {:ok, %{event: %{id: event_id}, actor: %{id: actor_id}}} else {:has_event, _} -> {:error, "Event with this ID #{inspect(event_id)} doesn't exist"} - {:is_owned, false} -> + {:is_owned, nil} -> {:error, "Actor id is not owned by authenticated user"} {:only_organizer, true} -> @@ -187,31 +170,19 @@ defmodule MobilizonWeb.Resolvers.Event do def create_event( _parent, %{organizer_actor_id: organizer_actor_id} = args, - %{ - context: %{ - current_user: user - } - } = _resolution + %{context: %{current_user: user}} = _resolution ) do # See https://github.com/absinthe-graphql/absinthe/issues/490 with args <- Map.put(args, :options, args[:options] || %{}), - {:is_owned, true, organizer_actor} <- User.owns_actor(user, organizer_actor_id), + {:is_owned, %Actor{} = organizer_actor} <- User.owns_actor(user, organizer_actor_id), {:ok, args} <- save_attached_picture(args), {:ok, args} <- save_physical_address(args), args_with_organizer <- Map.put(args, :organizer_actor, organizer_actor), - { - :ok, - %Activity{ - data: %{ - "object" => %{"type" => "Event"} = _object - } - }, - %Event{} = event - } <- + {:ok, %Activity{data: %{"object" => %{"type" => "Event"} = _object}}, %Event{} = event} <- MobilizonWeb.API.Events.create_event(args_with_organizer) do {:ok, event} else - {:is_owned, false} -> + {:is_owned, nil} -> {:error, "Organizer actor id is not owned by the user"} end end @@ -226,35 +197,24 @@ defmodule MobilizonWeb.Resolvers.Event do def update_event( _parent, %{event_id: event_id} = args, - %{ - context: %{ - current_user: user - } - } = _resolution + %{context: %{current_user: user}} = _resolution ) do # See https://github.com/absinthe-graphql/absinthe/issues/490 with args <- Map.put(args, :options, args[:options] || %{}), {:ok, %Event{} = event} <- Mobilizon.Events.get_event_full(event_id), - {:is_owned, true, organizer_actor} <- User.owns_actor(user, event.organizer_actor_id), + {:is_owned, %Actor{} = organizer_actor} <- + User.owns_actor(user, event.organizer_actor_id), {:ok, args} <- save_attached_picture(args), {:ok, args} <- save_physical_address(args), args <- Map.put(args, :organizer_actor, organizer_actor), - { - :ok, - %Activity{ - data: %{ - "object" => %{"type" => "Event"} = _object - } - }, - %Event{} = event - } <- + {:ok, %Activity{data: %{"object" => %{"type" => "Event"} = _object}}, %Event{} = event} <- MobilizonWeb.API.Events.update_event(args, event) do {:ok, event} else {:error, :event_not_found} -> {:error, "Event not found"} - {:is_owned, _} -> + {:is_owned, nil} -> {:error, "User doesn't own actor"} end end @@ -268,24 +228,14 @@ defmodule MobilizonWeb.Resolvers.Event do # However, we need to pass it's actor ID @spec save_attached_picture(map()) :: {:ok, map()} defp save_attached_picture( - %{ - picture: %{ - picture: %{file: %Plug.Upload{} = _picture} = all_pic - } - } = args + %{picture: %{picture: %{file: %Plug.Upload{} = _picture} = all_pic}} = args ) do {:ok, Map.put(args, :picture, Map.put(all_pic, :actor_id, args.organizer_actor_id))} end # Otherwise if we use a previously uploaded picture we need to fetch it from database @spec save_attached_picture(map()) :: {:ok, map()} - defp save_attached_picture( - %{ - picture: %{ - picture_id: picture_id - } - } = args - ) do + defp save_attached_picture(%{picture: %{picture_id: picture_id}} = args) do with %Picture{} = picture <- Mobilizon.Media.get_picture(picture_id) do {:ok, Map.put(args, :picture, picture)} end @@ -295,13 +245,7 @@ defmodule MobilizonWeb.Resolvers.Event do defp save_attached_picture(args), do: {:ok, args} @spec save_physical_address(map()) :: {:ok, map()} - defp save_physical_address( - %{ - physical_address: %{ - url: physical_address_url - } - } = args - ) + defp save_physical_address(%{physical_address: %{url: physical_address_url}} = args) when not is_nil(physical_address_url) do with %Address{} = address <- Addresses.get_address_by_url(physical_address_url), args <- Map.put(args, :physical_address, address.url) do @@ -326,14 +270,10 @@ defmodule MobilizonWeb.Resolvers.Event do def delete_event( _parent, %{event_id: event_id, actor_id: actor_id}, - %{ - context: %{ - current_user: user - } - } + %{context: %{current_user: user}} ) do with {:ok, %Event{} = event} <- Mobilizon.Events.get_event(event_id), - {:is_owned, true, _} <- User.owns_actor(user, actor_id), + {:is_owned, %Actor{}} <- User.owns_actor(user, actor_id), {:event_can_be_managed, true} <- Event.can_event_be_managed_by(event, actor_id), event <- Mobilizon.Events.delete_event!(event) do {:ok, %{id: event.id}} @@ -341,7 +281,7 @@ defmodule MobilizonWeb.Resolvers.Event do {:error, :event_not_found} -> {:error, "Event not found"} - {:is_owned, false} -> + {:is_owned, nil} -> {:error, "Actor id is not owned by authenticated user"} {:event_can_be_managed, false} -> diff --git a/lib/mobilizon_web/resolvers/feed_token.ex b/lib/mobilizon_web/resolvers/feed_token.ex index 02ad34bc7..3d8242a84 100644 --- a/lib/mobilizon_web/resolvers/feed_token.ex +++ b/lib/mobilizon_web/resolvers/feed_token.ex @@ -2,10 +2,11 @@ defmodule MobilizonWeb.Resolvers.FeedToken do @moduledoc """ Handles the feed tokens-related GraphQL calls """ - require Logger + alias Mobilizon.Actors.Actor alias Mobilizon.Users.User alias Mobilizon.Events alias Mobilizon.Events.FeedToken + require Logger @doc """ Create an feed token for an user and a defined actor @@ -14,11 +15,11 @@ defmodule MobilizonWeb.Resolvers.FeedToken do def create_feed_token(_parent, %{actor_id: actor_id}, %{ context: %{current_user: %User{id: id} = user} }) do - with {:is_owned, true, _actor} <- User.owns_actor(user, actor_id), + with {:is_owned, %Actor{}} <- User.owns_actor(user, actor_id), {:ok, feed_token} <- Events.create_feed_token(%{"user_id" => id, "actor_id" => actor_id}) do {:ok, feed_token} else - {:is_owned, false} -> + {:is_owned, nil} -> {:error, "Actor id is not owned by authenticated user"} end end diff --git a/lib/mobilizon_web/resolvers/group.ex b/lib/mobilizon_web/resolvers/group.ex index 84239788a..a8e378a2f 100644 --- a/lib/mobilizon_web/resolvers/group.ex +++ b/lib/mobilizon_web/resolvers/group.ex @@ -40,19 +40,11 @@ defmodule MobilizonWeb.Resolvers.Group do def create_group( _parent, args, - %{ - context: %{ - current_user: user - } - } + %{context: %{current_user: user}} ) do with { :ok, - %Activity{ - data: %{ - "object" => %{"type" => "Group"} = _object - } - }, + %Activity{data: %{"object" => %{"type" => "Group"} = _object}}, %Actor{} = group } <- MobilizonWeb.API.Groups.create_group( @@ -66,10 +58,7 @@ defmodule MobilizonWeb.Resolvers.Group do banner: Map.get(args, "banner") } ) do - { - :ok, - group - } + {:ok, group} end end @@ -83,14 +72,10 @@ defmodule MobilizonWeb.Resolvers.Group do def delete_group( _parent, %{group_id: group_id, actor_id: actor_id}, - %{ - context: %{ - current_user: user - } - } + %{context: %{current_user: user}} ) do with {:ok, %Actor{} = group} <- Actors.get_group_by_actor_id(group_id), - {:is_owned, true, _} <- User.owns_actor(user, actor_id), + {:is_owned, %Actor{}} <- User.owns_actor(user, actor_id), {:ok, %Member{} = member} <- Member.get_member(actor_id, group.id), {:is_admin, true} <- Member.is_administrator(member), group <- Actors.delete_group!(group) do @@ -99,7 +84,7 @@ defmodule MobilizonWeb.Resolvers.Group do {:error, :group_not_found} -> {:error, "Group not found"} - {:is_owned, false} -> + {:is_owned, nil} -> {:error, "Actor id is not owned by authenticated user"} {:error, :member_not_found} -> @@ -120,37 +105,24 @@ defmodule MobilizonWeb.Resolvers.Group do def join_group( _parent, %{group_id: group_id, actor_id: actor_id}, - %{ - context: %{ - current_user: user - } - } + %{context: %{current_user: user}} ) do - with {:is_owned, true, actor} <- User.owns_actor(user, actor_id), + with {:is_owned, %Actor{} = actor} <- User.owns_actor(user, actor_id), {:ok, %Actor{} = group} <- Actors.get_group_by_actor_id(group_id), {:error, :member_not_found} <- Member.get_member(actor.id, group.id), {:is_able_to_join, true} <- {:is_able_to_join, Member.can_be_joined(group)}, role <- Mobilizon.Actors.get_default_member_role(group), - {:ok, _} <- - Actors.create_member(%{ - parent_id: group.id, - actor_id: actor.id, - role: role - }) do + {:ok, _} <- Actors.create_member(%{parent_id: group.id, actor_id: actor.id, role: role}) do { :ok, %{ - parent: - group - |> Person.proxify_pictures(), - actor: - actor - |> Person.proxify_pictures(), + parent: Person.proxify_pictures(group), + actor: Person.proxify_pictures(actor), role: role } } else - {:is_owned, false} -> + {:is_owned, nil} -> {:error, "Actor id is not owned by authenticated user"} {:error, :group_not_found} -> @@ -174,31 +146,17 @@ defmodule MobilizonWeb.Resolvers.Group do def leave_group( _parent, %{group_id: group_id, actor_id: actor_id}, - %{ - context: %{ - current_user: user - } - } + %{context: %{current_user: user}} ) do - with {:is_owned, true, actor} <- User.owns_actor(user, actor_id), + with {:is_owned, %Actor{} = actor} <- User.owns_actor(user, actor_id), {:ok, %Member{} = member} <- Member.get_member(actor.id, group_id), {:only_administrator, false} <- {:only_administrator, check_that_member_is_not_last_administrator(group_id, actor_id)}, {:ok, _} <- Mobilizon.Actors.delete_member(member) do - { - :ok, - %{ - parent: %{ - id: group_id - }, - actor: %{ - id: actor_id - } - } - } + {:ok, %{parent: %{id: group_id}, actor: %{id: actor_id}}} else - {:is_owned, false} -> + {:is_owned, nil} -> {:error, "Actor id is not owned by authenticated user"} {:error, :member_not_found} -> @@ -219,13 +177,7 @@ defmodule MobilizonWeb.Resolvers.Group do @spec check_that_member_is_not_last_administrator(integer(), integer()) :: boolean() defp check_that_member_is_not_last_administrator(group_id, actor_id) do case Member.list_administrator_members_for_group(group_id) do - [ - %Member{ - actor: %Actor{ - id: member_actor_id - } - } - ] -> + [%Member{actor: %Actor{id: member_actor_id}}] -> actor_id == member_actor_id _ -> diff --git a/lib/mobilizon_web/resolvers/person.ex b/lib/mobilizon_web/resolvers/person.ex index 88092c65c..347b32131 100644 --- a/lib/mobilizon_web/resolvers/person.ex +++ b/lib/mobilizon_web/resolvers/person.ex @@ -50,9 +50,7 @@ defmodule MobilizonWeb.Resolvers.Person do def create_person( _parent, %{preferred_username: _preferred_username} = args, - %{ - context: %{current_user: user} - } = _resolution + %{context: %{current_user: user}} = _resolution ) do args = Map.put(args, :user_id, user.id) @@ -75,17 +73,13 @@ defmodule MobilizonWeb.Resolvers.Person do def update_person( _parent, %{preferred_username: preferred_username} = args, - %{ - context: %{ - current_user: user - } - } = _resolution + %{context: %{current_user: user}} = _resolution ) do args = Map.put(args, :user_id, user.id) with {:find_actor, %Actor{} = actor} <- {:find_actor, Actors.get_actor_by_name(preferred_username)}, - {:is_owned, true, _} <- User.owns_actor(user, actor.id), + {:is_owned, %Actor{}} <- User.owns_actor(user, actor.id), args <- save_attached_pictures(args), {:ok, actor} <- Actors.update_actor(actor, args) do {:ok, actor} @@ -93,7 +87,7 @@ defmodule MobilizonWeb.Resolvers.Person do {:find_actor, nil} -> {:error, "Actor not found"} - {:is_owned, false} -> + {:is_owned, nil} -> {:error, "Actor is not owned by authenticated user"} end end @@ -108,15 +102,11 @@ defmodule MobilizonWeb.Resolvers.Person do def delete_person( _parent, %{preferred_username: preferred_username} = _args, - %{ - context: %{ - current_user: user - } - } = _resolution + %{context: %{current_user: user}} = _resolution ) do with {:find_actor, %Actor{} = actor} <- {:find_actor, Actors.get_actor_by_name(preferred_username)}, - {:is_owned, true, _} <- User.owns_actor(user, actor.id), + {:is_owned, %Actor{}} <- User.owns_actor(user, actor.id), {:last_identity, false} <- {:last_identity, last_identity?(user)}, {:last_admin, false} <- {:last_admin, last_admin_of_a_group?(actor.id)}, {:ok, actor} <- Actors.delete_actor(actor) do @@ -131,7 +121,7 @@ defmodule MobilizonWeb.Resolvers.Person do {:last_admin, true} -> {:error, "Cannot remove the last administrator of a group"} - {:is_owned, false} -> + {:is_owned, nil} -> {:error, "Actor is not owned by authenticated user"} end end @@ -184,14 +174,12 @@ defmodule MobilizonWeb.Resolvers.Person do @doc """ Returns the list of events this person is going to """ - def person_going_to_events(%Actor{id: actor_id}, _args, %{ - context: %{current_user: user} - }) do - with {:is_owned, true, actor} <- User.owns_actor(user, actor_id), + def person_going_to_events(%Actor{id: actor_id}, _args, %{context: %{current_user: user}}) do + with {:is_owned, %Actor{} = actor} <- User.owns_actor(user, actor_id), events <- Events.list_event_participations_for_actor(actor) do {:ok, events} else - {:is_owned, false} -> + {:is_owned, nil} -> {:error, "Actor id is not owned by authenticated user"} end end @@ -199,9 +187,7 @@ defmodule MobilizonWeb.Resolvers.Person do @doc """ Returns the list of events this person is going to """ - def person_going_to_events(_parent, %{}, %{ - context: %{current_user: user} - }) do + def person_going_to_events(_parent, %{}, %{context: %{current_user: user}}) do with %Actor{} = actor <- Users.get_actor_for_user(user), events <- Events.list_event_participations_for_actor(actor) do {:ok, events} diff --git a/lib/mobilizon_web/resolvers/picture.ex b/lib/mobilizon_web/resolvers/picture.ex index 494ab6916..9043c2558 100644 --- a/lib/mobilizon_web/resolvers/picture.ex +++ b/lib/mobilizon_web/resolvers/picture.ex @@ -2,6 +2,7 @@ defmodule MobilizonWeb.Resolvers.Picture do @moduledoc """ Handles the picture-related GraphQL calls """ + alias Mobilizon.Actors.Actor alias Mobilizon.Media alias Mobilizon.Media.Picture alias Mobilizon.Users.User @@ -10,9 +11,7 @@ defmodule MobilizonWeb.Resolvers.Picture do Get picture for an event's pic """ def picture(%{picture_id: picture_id} = _parent, _args, _resolution) do - with {:ok, picture} <- do_fetch_picture(picture_id) do - {:ok, picture} - end + with {:ok, picture} <- do_fetch_picture(picture_id), do: {:ok, picture} end @doc """ @@ -20,15 +19,9 @@ defmodule MobilizonWeb.Resolvers.Picture do See MobilizonWeb.Resolvers.Event.create_event/3 """ - def picture(%{picture: picture} = _parent, _args, _resolution) do - {:ok, picture} - end - + def picture(%{picture: picture} = _parent, _args, _resolution), do: {:ok, picture} def picture(_parent, %{id: picture_id}, _resolution), do: do_fetch_picture(picture_id) - - def picture(_parent, _args, _resolution) do - {:ok, nil} - end + def picture(_parent, _args, _resolution), do: {:ok, nil} @spec do_fetch_picture(nil) :: {:error, nil} defp do_fetch_picture(nil), do: {:error, nil} @@ -36,7 +29,7 @@ defmodule MobilizonWeb.Resolvers.Picture do @spec do_fetch_picture(String.t()) :: {:ok, Picture.t()} | {:error, :not_found} defp do_fetch_picture(picture_id) do case Media.get_picture(picture_id) do - %Picture{id: id, file: file} = _pic -> + %Picture{id: id, file: file} -> {:ok, %{ name: file.name, @@ -46,18 +39,18 @@ defmodule MobilizonWeb.Resolvers.Picture do size: file.size }} - _err -> + _error -> {:error, "Picture with ID #{picture_id} was not found"} end end @spec upload_picture(map(), map(), map()) :: {:ok, Picture.t()} | {:error, any()} - def upload_picture(_parent, %{file: %Plug.Upload{} = file, actor_id: actor_id} = args, %{ - context: %{ - current_user: user - } - }) do - with {:is_owned, true, _actor} <- User.owns_actor(user, actor_id), + def upload_picture( + _parent, + %{file: %Plug.Upload{} = file, actor_id: actor_id} = args, + %{context: %{current_user: user}} + ) do + with {:is_owned, %Actor{}} <- User.owns_actor(user, actor_id), {:ok, %{"url" => [%{"href" => url, "mediaType" => content_type}], "size" => size}} <- MobilizonWeb.Upload.store(file), args <- @@ -76,11 +69,11 @@ defmodule MobilizonWeb.Resolvers.Picture do size: picture.file.size }} else - {:is_owned, false} -> + {:is_owned, nil} -> {:error, "Actor id is not owned by authenticated user"} - err -> - {:error, err} + error -> + {:error, error} end end diff --git a/lib/mobilizon_web/resolvers/report.ex b/lib/mobilizon_web/resolvers/report.ex index dd3655050..85796e158 100644 --- a/lib/mobilizon_web/resolvers/report.ex +++ b/lib/mobilizon_web/resolvers/report.ex @@ -10,9 +10,11 @@ defmodule MobilizonWeb.Resolvers.Report do alias MobilizonWeb.API.Reports, as: ReportsAPI import Mobilizon.Users.Guards - def list_reports(_parent, %{page: page, limit: limit}, %{ - context: %{current_user: %User{role: role}} - }) + def list_reports( + _parent, + %{page: page, limit: limit}, + %{context: %{current_user: %User{role: role}}} + ) when is_moderator(role) do {:ok, Mobilizon.Reports.list_reports(page, limit)} end @@ -21,9 +23,7 @@ defmodule MobilizonWeb.Resolvers.Report do {:error, "You need to be logged-in and a moderator to list reports"} end - def get_report(_parent, %{id: id}, %{ - context: %{current_user: %User{role: role}} - }) + def get_report(_parent, %{id: id}, %{context: %{current_user: %User{role: role}}}) when is_moderator(role) do {:ok, Mobilizon.Reports.get_report(id)} end @@ -40,14 +40,14 @@ defmodule MobilizonWeb.Resolvers.Report do %{reporter_actor_id: reporter_actor_id} = args, %{context: %{current_user: user}} = _resolution ) do - with {:is_owned, true, _} <- User.owns_actor(user, reporter_actor_id), + with {:is_owned, %Actor{}} <- User.owns_actor(user, reporter_actor_id), {:ok, _, %Report{} = report} <- ReportsAPI.report(args) do {:ok, report} else - {:is_owned, false} -> + {:is_owned, nil} -> {:error, "Reporter actor id is not owned by authenticated user"} - _err -> + _error -> {:error, "Error while saving report"} end end @@ -62,22 +62,19 @@ defmodule MobilizonWeb.Resolvers.Report do def update_report( _parent, %{report_id: report_id, moderator_id: moderator_id, status: status}, - %{ - context: %{current_user: %User{role: role} = user} - } + %{context: %{current_user: %User{role: role} = user}} ) when is_moderator(role) do - with {:is_owned, true, _} <- User.owns_actor(user, moderator_id), - %Actor{} = actor <- Actors.get_actor!(moderator_id), + with {:is_owned, %Actor{} = actor} <- User.owns_actor(user, moderator_id), %Report{} = report <- Mobilizon.Reports.get_report(report_id), {:ok, %Report{} = report} <- MobilizonWeb.API.Reports.update_report_status(actor, report, status) do {:ok, report} else - {:is_owned, false} -> + {:is_owned, nil} -> {:error, "Actor id is not owned by authenticated user"} - _err -> + _error -> {:error, "Error while updating report"} end end @@ -89,12 +86,10 @@ defmodule MobilizonWeb.Resolvers.Report do def create_report_note( _parent, %{report_id: report_id, moderator_id: moderator_id, content: content}, - %{ - context: %{current_user: %User{role: role} = user} - } + %{context: %{current_user: %User{role: role} = user}} ) when is_moderator(role) do - with {:is_owned, true, _} <- User.owns_actor(user, moderator_id), + with {:is_owned, %Actor{}} <- User.owns_actor(user, moderator_id), %Report{} = report <- Reports.get_report(report_id), %Actor{} = moderator <- Actors.get_local_actor_with_everything(moderator_id), {:ok, %Note{} = note} <- @@ -103,11 +98,13 @@ defmodule MobilizonWeb.Resolvers.Report do end end - def delete_report_note(_parent, %{note_id: note_id, moderator_id: moderator_id}, %{ - context: %{current_user: %User{role: role} = user} - }) + def delete_report_note( + _parent, + %{note_id: note_id, moderator_id: moderator_id}, + %{context: %{current_user: %User{role: role} = user}} + ) when is_moderator(role) do - with {:is_owned, true, _} <- User.owns_actor(user, moderator_id), + with {:is_owned, %Actor{}} <- User.owns_actor(user, moderator_id), %Note{} = note <- Reports.get_note(note_id), %Actor{} = moderator <- Actors.get_local_actor_with_everything(moderator_id), {:ok, %Note{} = note} <- diff --git a/lib/mobilizon_web/resolvers/user.ex b/lib/mobilizon_web/resolvers/user.ex index 500036fa3..d25b971f2 100644 --- a/lib/mobilizon_web/resolvers/user.ex +++ b/lib/mobilizon_web/resolvers/user.ex @@ -118,8 +118,8 @@ defmodule MobilizonWeb.Resolvers.User do {:registrations_open, false} -> {:error, "Registrations are not enabled"} - err -> - err + error -> + error end end @@ -139,9 +139,9 @@ defmodule MobilizonWeb.Resolvers.User do user: Map.put(user, :default_actor, actor) }} else - err -> + error -> Logger.info("Unable to validate user with token #{token}") - Logger.debug(inspect(err)) + Logger.debug(inspect(error)) {:error, "Unable to validate user"} end end @@ -213,7 +213,7 @@ defmodule MobilizonWeb.Resolvers.User do {:user_actor, _} -> {:error, :actor_not_from_user} - _err -> + _error -> {:error, :unable_to_change_default_actor} end end diff --git a/mix.exs b/mix.exs index 83d043e07..f54af6661 100644 --- a/mix.exs +++ b/mix.exs @@ -200,7 +200,7 @@ defmodule Mobilizon.Mixfile do Mobilizon.Events.TagRelation, Mobilizon.Users, Mobilizon.Users.User, - Mobilizon.Users.UserRoleEnum, + Mobilizon.Users.UserRole, Mobilizon.Users.Guards, Mobilizon.Activity, Mobilizon.Ecto, diff --git a/priv/repo/migrations/20190307125009_move_user_role_to_enum.exs b/priv/repo/migrations/20190307125009_move_user_role_to_enum.exs index 1508391ce..66f8ce5be 100644 --- a/priv/repo/migrations/20190307125009_move_user_role_to_enum.exs +++ b/priv/repo/migrations/20190307125009_move_user_role_to_enum.exs @@ -1,13 +1,13 @@ defmodule Mobilizon.Repo.Migrations.MoveUserRoleToEnum do use Ecto.Migration - alias Mobilizon.Users.UserRoleEnum + alias Mobilizon.Users.UserRole def up do - UserRoleEnum.create_type() + UserRole.create_type() alter table(:users) do - add(:role_tmp, UserRoleEnum.type(), default: "user") + add(:role_tmp, UserRole.type(), default: "user") end execute("UPDATE users set role_tmp = 'user' where role = 0") @@ -34,7 +34,7 @@ defmodule Mobilizon.Repo.Migrations.MoveUserRoleToEnum do remove(:role) end - UserRoleEnum.drop_type() + UserRole.drop_type() rename(table(:users), :role_tmp, to: :role) end diff --git a/test/mobilizon_web/resolvers/report_resolver_test.exs b/test/mobilizon_web/resolvers/report_resolver_test.exs index 915ccbbe0..cf369a4dd 100644 --- a/test/mobilizon_web/resolvers/report_resolver_test.exs +++ b/test/mobilizon_web/resolvers/report_resolver_test.exs @@ -195,7 +195,8 @@ defmodule MobilizonWeb.Resolvers.ReportResolverTest do assert json_response(res, 200)["errors"] == nil assert json_response(res, 200)["data"]["reports"] - |> Enum.map(fn report -> Map.get(report, "id") end) == + |> Enum.map(fn report -> Map.get(report, "id") end) + |> Enum.sort() == Enum.map([report_1_id, report_2_id, report_3_id], &to_string/1) query = """