Refactoring of Users context

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

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.Crypto
-  alias Mobilizon.Service.EmailChecker
   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)
+    field(:role, UserRole, default: :user)
-    has_many(:actors, Actor)
-    belongs_to(:default_actor, Actor)
     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, [
+      |> cast(attrs, @attrs)
-        :email,
+      |> validate_required(@required_attrs)
-        :role,
-        :password,
-        :password_hash,
-        :confirmed_at,
-        :confirmation_sent_at,
-        :confirmation_token,
-        :reset_password_sent_at,
-        :reset_password_token
-      ])
-      |> validate_required([:email])
       |> unique_constraint(:email, message: "This email is already used.")
       |> validate_email()
-      |> validate_length(
+      |> validate_length(:password, min: 6, max: 100, message: "The chosen password is too short.")
-        :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
+  @doc false
-    struct
+  @spec registration_changeset(User.t(), map) :: Ecto.Changeset.t()
-    |> changeset(params)
+  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(user, attrs, [:reset_password_token, :reset_password_sent_at])
-    |> cast(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])
+    |> cast(attrs, @password_reset_required_attrs)
-    |> validate_length(
+    |> validate_length(:password,
-      :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
-          changeset,
+        |> put_change(:confirmation_token, Crypto.random_string(@confirmation_token_length))
-          :confirmation_sent_at,
+        |> put_change(:confirmation_sent_at, DateTime.truncate(now, :second))
-          DateTime.utc_now() |> DateTime.truncate(: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
+  @spec hash_password(Ecto.Changeset.t()) :: Ecto.Changeset.t()
-    length
-    |> :crypto.strong_rand_bytes()
-    |> Base.url_encode64()
-  end
-
-  # Hash password when it's changed
   defp hash_password(changeset) do
     case changeset do
       %Ecto.Changeset{valid?: true, changes: %{password: password}} ->
-        put_change(
+        put_change(changeset, :password_hash, Argon2.hash_pwd_salt(password))
-          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

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
+  @type tokens :: %{
-  def data() do
+          required(:access_token) => String.t(),
-    Dataloader.Ecto.new(Repo, query: &query/2)
+          required(:refresh_token) => String.t()
-  end
+        }
+
+  defenum(UserRole, :user_role, [:administrator, :moderator, :user])
 
   @doc false
-  def query(queryable, _params) do
+  @spec data :: Dataloader.Ecto.t()
-    queryable
+  def data, do: Dataloader.Ecto.new(Mobilizon.Repo, query: &query/2)
-  end
+
+  @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
+           |> Repo.insert() do
-      Mobilizon.Events.create_feed_token(%{"user_id" => user.id})
+      Events.create_feed_token(%{"user_id" => user.id})
+
       {:ok, user}
     end
   end
 
   @doc """
-  Gets an user by it's email
+  Gets a single user.
-
+  Raises `Ecto.NoResultsError` if the user does not exist.
-  ## 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}
   """
+  @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 =
+    query = user_by_email_query(email, activated)
-      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
 
     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(
+    token
-      from(
+    |> user_by_activation_token_query()
-        u in User,
+    |> Repo.one()
-        where: u.confirmation_token == ^token,
-        preload: [:default_actor]
-      )
-    )
   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(
+    token
-      from(
+    |> user_by_reset_password_token_query()
-        u in User,
+    |> Repo.one()
-        where: u.reset_password_token == ^token,
-        preload: [:default_actor]
-      )
-    )
   end
 
   @doc """
-  Updates a user.
+  Updates an user.
-
-  ## Examples
-
-      iex> update_user(User{}, %{password: "coucou"})
-      {:ok, %Mobilizon.Users.User{}}
-
-      iex> update_user(User{}, %{password: nil})
-      {:error, %Ecto.Changeset{}}
-
   """
+  @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.
+  Deletes an user.
-
-  ## Examples
-
-      iex> delete_user(%User{email: "test@test.tld"})
-      {:ok, %Mobilizon.Users.User{}}
-
-      iex> delete_user(%User{})
-      {:error, %Ecto.Changeset{}}
-
   """
+  @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.
+  Get an user with its actors
-
+  Raises `Ecto.NoResultsError` if the user does not exist.
-  Raises `Ecto.NoResultsError` if the User does not exist.
-
-  ## Examples
-
-      iex> get_user!(123)
-      %Mobilizon.Users.User{}
-
-      iex> get_user!(456)
-      ** (Ecto.NoResultsError)
-
   """
-  def get_user!(id), do: Repo.get!(User, id)
+  @spec get_user_with_actors!(integer | String.t()) :: User.t()
-
-  @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()
   def get_user_with_actors!(id) do
-    user = Repo.get!(User, id)
+    id
-    Repo.preload(user, [:actors, :default_actor])
+    |> 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()
+  @spec get_actor_for_user(User.t()) :: Actor.t() | nil
-  def get_actor_for_user(%Mobilizon.Users.User{} = user) do
+  def get_actor_for_user(%User{} = user) do
-    case Repo.one(
+    actor =
-           from(
+      user
-             a in Actor,
+      |> actor_for_user_query()
-             join: u in User,
+      |> Repo.one()
-             on: u.default_actor_id == a.id,
+
-             where: u.id == ^user.id
+    case actor do
-           )
-         ) do
       nil ->
-        case user
+        case get_actors_for_user(user) do
-             |> get_actors_for_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
+  @doc """
-    Repo.all(from(a in Actor, where: a.user_id == ^user_id))
+  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
+  @spec update_user_default_actor(integer | String.t(), integer | String.t()) :: User.t()
-    # 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
-
   def update_user_default_actor(user_id, actor_id) do
     with _ <-
-           from(
+           user_id
-             u in User,
+           |> update_user_default_actor_query(actor_id)
-             where: u.id == ^user_id,
-             update: [
-               set: [
-                 default_actor_id: ^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
-      User
+    |> Page.paginate(page, limit)
-      |> paginate(page, limit)
+    |> sort(sort, direction)
-      |> 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
+  @doc """
-    Repo.one(
+  Counts users.
-      from(
+  """
-        u in User,
+  @spec count_users :: integer
-        select: count(u.id)
+  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

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 <-
+         group <- ActivityPubUtils.make_group_data(actor.url, to, title, content_html, tags, cc) do
-           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

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
-         Events.find_similar_events_by_common_tags(
+      |> Enum.concat(Events.find_similar_events_by_common_tags(tags, @number_of_related_events))
-           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
-           Events.list_events(1, @number_of_related_events, :begins_on, :asc, true, true))
+        |> 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(
+  defp save_attached_picture(%{picture: %{picture_id: picture_id}} = args) do
-         %{
-           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(
+  defp save_physical_address(%{physical_address: %{url: physical_address_url}} = args)
-         %{
-           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} ->

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

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{
+           %Activity{data: %{"object" => %{"type" => "Group"} = _object}},
-             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, _} <-
+         {:ok, _} <- Actors.create_member(%{parent_id: group.id, actor_id: actor.id, role: role}) do
-           Actors.create_member(%{
-             parent_id: group.id,
-             actor_id: actor.id,
-             role: role
-           }) do
       {
         :ok,
         %{
-          parent:
+          parent: Person.proxify_pictures(group),
-            group
+          actor: Person.proxify_pictures(actor),
-            |> Person.proxify_pictures(),
-          actor:
-            actor
-            |> Person.proxify_pictures(),
           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
 
       _ ->

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, %{
+  def person_going_to_events(%Actor{id: actor_id}, _args, %{context: %{current_user: user}}) do
-        context: %{current_user: user}
+    with {:is_owned, %Actor{} = actor} <- User.owns_actor(user, actor_id),
-      }) do
-    with {:is_owned, true, 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, %{}, %{
+  def person_going_to_events(_parent, %{}, %{context: %{current_user: user}}) do
-        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}

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
+    with {:ok, picture} <- do_fetch_picture(picture_id), do: {:ok, picture}
-      {:ok, picture}
-    end
   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
+  def picture(%{picture: picture} = _parent, _args, _resolution), do: {:ok, picture}
-    {:ok, picture}
-  end
-
   def picture(_parent, %{id: picture_id}, _resolution), do: do_fetch_picture(picture_id)
-
+  def picture(_parent, _args, _resolution), do: {:ok, nil}
-  def picture(_parent, _args, _resolution) do
-    {:ok, nil}
-  end
 
   @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, %{
+  def upload_picture(
-        context: %{
+        _parent,
-          current_user: user
+        %{file: %Plug.Upload{} = file, actor_id: actor_id} = args,
-        }
+        %{context: %{current_user: user}}
-      }) do
+      ) do
-    with {:is_owned, true, _actor} <- User.owns_actor(user, actor_id),
+    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 ->
-        {:error, err}
+        {:error, error}
     end
   end
 

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}, %{
+  def list_reports(
-        context: %{current_user: %User{role: role}}
+        _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}, %{
+  def get_report(_parent, %{id: id}, %{context: %{current_user: %User{role: role}}})
-        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),
+    with {:is_owned, %Actor{} = actor} <- User.owns_actor(user, moderator_id),
-         %Actor{} = actor <- Actors.get_actor!(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}, %{
+  def delete_report_note(
-        context: %{current_user: %User{role: role} = user}
+        _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} <-

lib/mobilizon_web/resolvers/user.ex

@@ -118,8 +118,8 @@ defmodule MobilizonWeb.Resolvers.User do
       {:registrations_open, false} ->
         {:error, "Registrations are not enabled"}
 
-      err ->
+      error ->
-        err
+        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

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,

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

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 = """