Fix posts and rework graphql errors

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

lib/federation/activity_stream/converter/post.ex

@@ -35,7 +35,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Post do
       "name" => post.title,
       "content" => post.body,
       "attributedTo" => creator_url,
-      "published" => (post.publish_at || post.inserted_at) |> DateTime.to_iso8601()
+      "published" => (post.publish_at || post.inserted_at) |> to_date()
     }
   end
 
@@ -67,4 +67,7 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Post do
   @spec get_actor(String.t() | map() | nil) :: {:ok, Actor.t()} | {:error, String.t()}
   defp get_actor(nil), do: {:error, "nil property found for actor data"}
   defp get_actor(actor), do: actor |> Utils.get_url() |> ActivityPub.get_or_fetch_actor_by_url()
+
+  defp to_date(%DateTime{} = date), do: DateTime.to_iso8601(date)
+  defp to_date(%NaiveDateTime{} = date), do: NaiveDateTime.to_iso8601(date)
 end

lib/graphql/error.ex

@@ -0,0 +1,90 @@
+defmodule Mobilizon.GraphQL.Error do
+  @moduledoc """
+  Module to handle errors in GraphQL
+  """
+
+  require Logger
+  alias __MODULE__
+  import Mobilizon.Web.Gettext
+
+  defstruct [:code, :message, :status_code, :field]
+
+  # Error Tuples
+  # ------------
+  # Regular errors
+  def normalize({:error, reason}) do
+    handle(reason)
+  end
+
+  # Ecto transaction errors
+  def normalize({:error, _operation, reason, _changes}) do
+    handle(reason)
+  end
+
+  # Unhandled errors
+  def normalize(other) do
+    handle(other)
+  end
+
+  # Handle Different Errors
+  # -----------------------
+  defp handle(code) when is_atom(code) do
+    {status, message} = metadata(code)
+
+    %Error{
+      code: code,
+      message: message,
+      status_code: status
+    }
+  end
+
+  defp handle(errors) when is_list(errors) do
+    Enum.map(errors, &handle/1)
+  end
+
+  defp handle(%Ecto.Changeset{} = changeset) do
+    changeset
+    |> Ecto.Changeset.traverse_errors(fn {err, _opts} -> err end)
+    |> Enum.map(fn {k, v} ->
+      %Error{
+        code: :validation,
+        message: v,
+        field: k,
+        status_code: 422
+      }
+    end)
+  end
+
+  defp handle(reason) when is_binary(reason) do
+    %Error{
+      code: :unknown_error,
+      message: reason,
+      status_code: 500
+    }
+  end
+
+  # ... Handle other error types here ...
+  defp handle(other) do
+    Logger.error("Unhandled error term:\n#{inspect(other)}")
+    handle(:unknown)
+  end
+
+  # Build Error Metadata
+  # --------------------
+  defp metadata(:unknown_resource), do: {400, "Unknown Resource"}
+  defp metadata(:invalid_argument), do: {400, "Invalid arguments passed"}
+  defp metadata(:unauthenticated), do: {401, "You need to be logged in"}
+  defp metadata(:password_hash_missing), do: {401, "Reset your password to login"}
+  defp metadata(:incorrect_password), do: {401, "Invalid credentials"}
+  defp metadata(:unauthorized), do: {403, "You don't have permission to do this"}
+  defp metadata(:not_found), do: {404, "Resource not found"}
+  defp metadata(:user_not_found), do: {404, "User not found"}
+  defp metadata(:post_not_found), do: {404, dgettext("errors", "Post not found")}
+  defp metadata(:event_not_found), do: {404, dgettext("errors", "Event not found")}
+  defp metadata(:unknown), do: {500, "Something went wrong"}
+
+  defp metadata(code) do
+    Logger.warn("Unhandled error code: #{inspect(code)}")
+    {422, to_string(code)}
+  end
+end

lib/graphql/helpers/error.ex

@@ -1,45 +0,0 @@
-defmodule Mobilizon.GraphQL.Helpers.Error do
-  @moduledoc """
-  Helper functions for Mobilizon.GraphQL
-  """
-  alias Ecto.Changeset
-
-  def handle_errors(fun) do
-    fn source, args, info ->
-      case Absinthe.Resolution.call(fun, source, args, info) do
-        {:error, %Changeset{} = changeset} ->
-          format_changeset(changeset)
-
-        {:error, _, %Changeset{} = changeset} ->
-          format_changeset(changeset)
-
-        val ->
-          val
-      end
-    end
-  end
-
-  def format_changeset(%Changeset{changes: changes} = changeset) do
-    # {:error, [email: {"has already been taken", []}]}
-
-    errors =
-      Enum.reduce(changes, [], fn {_key, value}, acc ->
-        case value do
-          %Changeset{} ->
-            {:error, errors} = format_changeset(value)
-            acc ++ errors
-
-          _ ->
-            acc
-        end
-      end)
-
-    errors = errors ++ Enum.map(changeset.errors, &transform_error/1)
-
-    {:error, errors}
-  end
-
-  defp transform_error({key, {value, _context}}) do
-    [message: "#{value}", details: key]
-  end
-end

lib/graphql/middleware/error_handler.ex

@@ -0,0 +1,21 @@
+defmodule Mobilizon.GraphQL.Middleware.ErrorHandler do
+  @moduledoc """
+  Absinthe Error Handler
+  """
+  alias Mobilizon.GraphQL.Error
+
+  @behaviour Absinthe.Middleware
+  @impl true
+  def call(resolution, _config) do
+    errors =
+      resolution.errors
+      |> Enum.map(&Error.normalize/1)
+      |> List.flatten()
+      |> Enum.map(&to_absinthe_format/1)
+
+    %{resolution | errors: errors}
+  end
+
+  defp to_absinthe_format(%Error{} = error), do: Map.from_struct(error)
+  defp to_absinthe_format(error), do: error
+end

lib/graphql/resolvers/event.ex

@@ -38,12 +38,12 @@ defmodule Mobilizon.GraphQL.Resolvers.Event do
         {:ok, Map.put(event, :organizer_actor, Person.proxify_pictures(event.organizer_actor))}
 
       {:has_event, _} ->
-        {:error, dgettext("errors", "Event with UUID %{uuid} not found", uuid: uuid)}
+        {:error, :event_not_found}
     end
   end
 
-  defp find_private_event(_parent, %{uuid: uuid}, _resolution) do
-    {:error, dgettext("errors", "Event with UUID %{uuid} not found", uuid: uuid)}
+  defp find_private_event(_parent, _args, _resolution) do
+    {:error, :event_not_found}
   end
 
   def find_event(parent, %{uuid: uuid} = args, %{context: context} = resolution) do
@@ -57,7 +57,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Event do
         find_private_event(parent, args, resolution)
 
       {:access_valid, _} ->
-        {:error, dgettext("errors", "Event with UUID %{uuid} not found", uuid)}
+        {:error, :event_not_found}
     end
   end
 

lib/graphql/resolvers/post.ex

@@ -7,6 +7,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Post do
   alias Mobilizon.{Actors, Posts, Users}
   alias Mobilizon.Actors.Actor
   alias Mobilizon.Federation.ActivityPub
+  alias Mobilizon.Federation.ActivityPub.Utils
   alias Mobilizon.Posts.Post
   alias Mobilizon.Storage.Page
   alias Mobilizon.Users.User
@@ -76,7 +77,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Post do
       {:ok, post}
     else
       {:member, false} -> get_post(parent, %{slug: slug}, nil)
-      {:post, _} -> {:error, dgettext("errors", "No such post")}
+      {:post, _} -> {:error, :post_not_found}
     end
   end
 
@@ -91,12 +92,12 @@ defmodule Mobilizon.GraphQL.Resolvers.Post do
         {:ok, post}
 
       {:post, _} ->
-        {:error, dgettext("errors", "No such post")}
+        {:error, :post_not_found}
     end
   end
 
   def get_post(_parent, _args, _resolution) do
-    {:error, dgettext("errors", "No such post")}
+    {:error, :post_not_found}
   end
 
   def create_post(
@@ -110,6 +111,11 @@ defmodule Mobilizon.GraphQL.Resolvers.Post do
       ) do
     with %Actor{id: actor_id} <- Users.get_actor_for_user(user),
          {:member, true} <- {:member, Actors.is_member?(actor_id, group_id)},
+         %Actor{} = group <- Actors.get_actor(group_id),
+         args <-
+           Map.update(args, :picture, nil, fn picture ->
+             process_picture(picture, group)
+           end),
          {:ok, _, %Post{} = post} <-
            ActivityPub.create(
              :post,
@@ -123,6 +129,9 @@ defmodule Mobilizon.GraphQL.Resolvers.Post do
     else
       {:member, _} ->
         {:error, dgettext("errors", "Profile is not member of group")}
+
+      {:error, error} ->
+        {:error, error}
     end
   end
 
@@ -141,8 +150,12 @@ defmodule Mobilizon.GraphQL.Resolvers.Post do
       ) do
     with {:uuid, {:ok, _uuid}} <- {:uuid, Ecto.UUID.cast(id)},
          %Actor{id: actor_id} <- Users.get_actor_for_user(user),
-         {:post, %Post{attributed_to: %Actor{id: group_id}} = post} <-
+         {:post, %Post{attributed_to: %Actor{id: group_id} = group} = post} <-
            {:post, Posts.get_post_with_preloads(id)},
+         args <-
+           Map.update(args, :picture, nil, fn picture ->
+             process_picture(picture, group)
+           end),
          {:member, true} <- {:member, Actors.is_member?(actor_id, group_id)},
          {:ok, _, %Post{} = post} <-
            ActivityPub.update(post, args, true, %{}) do
@@ -195,4 +208,17 @@ defmodule Mobilizon.GraphQL.Resolvers.Post do
   def delete_post(_parent, _args, _resolution) do
     {:error, dgettext("errors", "You need to be logged-in to delete posts")}
   end
+
+  defp process_picture(nil, _), do: nil
+  defp process_picture(%{picture_id: _picture_id} = args, _), do: args
+
+  defp process_picture(%{picture: picture}, %Actor{id: actor_id}) do
+    %{
+      file:
+        picture
+        |> Map.get(:file)
+        |> Utils.make_picture_data(description: Map.get(picture, :name)),
+      actor_id: actor_id
+    }
+  end
 end

lib/graphql/resolvers/user.ex

@@ -127,8 +127,8 @@ defmodule Mobilizon.GraphQL.Resolvers.User do
       :not_allowlisted ->
         {:error, dgettext("errors", "Your email is not on the allowlist")}
 
-      error ->
-        error
+      {:error, error} ->
+        {:error, error}
     end
   end
 

lib/graphql/schema.ex

@@ -20,6 +20,7 @@ defmodule Mobilizon.GraphQL.Schema do
   alias Mobilizon.Actors.{Actor, Follower, Member}
   alias Mobilizon.Discussions.Comment
   alias Mobilizon.Events.{Event, Participant}
+  alias Mobilizon.GraphQL.Middleware.ErrorHandler
   alias Mobilizon.GraphQL.Schema
   alias Mobilizon.Storage.Repo
 
@@ -185,4 +186,12 @@ defmodule Mobilizon.GraphQL.Schema do
     import_fields(:person_subscriptions)
     import_fields(:discussion_subscriptions)
   end
+
+  def middleware(middleware, _field, %{identifier: type}) when type in [:query, :mutation] do
+    middleware ++ [ErrorHandler]
+  end
+
+  def middleware(middleware, _field, _object) do
+    middleware
+  end
 end

lib/graphql/schema/actors/person.ex

@@ -5,7 +5,6 @@ defmodule Mobilizon.GraphQL.Schema.Actors.PersonType do
   use Absinthe.Schema.Notation
 
   import Absinthe.Resolution.Helpers, only: [dataloader: 1]
-  import Mobilizon.GraphQL.Helpers.Error
 
   alias Mobilizon.Events
   alias Mobilizon.GraphQL.Resolvers.Person
@@ -136,7 +135,7 @@ defmodule Mobilizon.GraphQL.Schema.Actors.PersonType do
           "The banner for the profile, either as an object or directly the ID of an existing Picture"
       )
 
-      resolve(handle_errors(&Person.create_person/3))
+      resolve(&Person.create_person/3)
     end
 
     @desc "Update an identity"
@@ -157,14 +156,14 @@ defmodule Mobilizon.GraphQL.Schema.Actors.PersonType do
           "The banner for the profile, either as an object or directly the ID of an existing Picture"
       )
 
-      resolve(handle_errors(&Person.update_person/3))
+      resolve(&Person.update_person/3)
     end
 
     @desc "Delete an identity"
     field :delete_person, :person do
       arg(:id, non_null(:id))
 
-      resolve(handle_errors(&Person.delete_person/3))
+      resolve(&Person.delete_person/3)
     end
 
     @desc "Register a first profile on registration"
@@ -186,7 +185,7 @@ defmodule Mobilizon.GraphQL.Schema.Actors.PersonType do
           "The banner for the profile, either as an object or directly the ID of an existing Picture"
       )
 
-      resolve(handle_errors(&Person.register_person/3))
+      resolve(&Person.register_person/3)
     end
   end
 

lib/graphql/schema/event.ex

@@ -6,7 +6,6 @@ defmodule Mobilizon.GraphQL.Schema.EventType do
   use Absinthe.Schema.Notation
 
   import Absinthe.Resolution.Helpers, only: [dataloader: 1]
-  import Mobilizon.GraphQL.Helpers.Error
 
   alias Mobilizon.{Actors, Addresses, Discussions}
   alias Mobilizon.GraphQL.Resolvers.{Event, Picture, Tag}
@@ -311,7 +310,7 @@ defmodule Mobilizon.GraphQL.Schema.EventType do
       arg(:draft, :boolean, default_value: false)
       arg(:contacts, list_of(:contact), default_value: [])
 
-      resolve(handle_errors(&Event.create_event/3))
+      resolve(&Event.create_event/3)
     end
 
     @desc "Update an event"
@@ -343,7 +342,7 @@ defmodule Mobilizon.GraphQL.Schema.EventType do
       arg(:draft, :boolean)
       arg(:contacts, list_of(:contact), default_value: [])
 
-      resolve(handle_errors(&Event.update_event/3))
+      resolve(&Event.update_event/3)
     end
 
     @desc "Delete an event"

lib/graphql/schema/post.ex

@@ -3,7 +3,7 @@ defmodule Mobilizon.GraphQL.Schema.PostType do
   Schema representation for Posts
   """
   use Absinthe.Schema.Notation
-  alias Mobilizon.GraphQL.Resolvers.{Post, Tag}
+  alias Mobilizon.GraphQL.Resolvers.{Picture, Post, Tag}
 
   @desc "A post"
   object :post do
@@ -24,6 +24,11 @@ defmodule Mobilizon.GraphQL.Schema.PostType do
       resolve: &Tag.list_tags_for_post/3,
       description: "The post's tags"
     )
+
+    field(:picture, :picture,
+      description: "The event's picture",
+      resolve: &Picture.picture/3
+    )
   end
 
   object :paginated_post_list do
@@ -55,7 +60,7 @@ defmodule Mobilizon.GraphQL.Schema.PostType do
     field :create_post, :post do
       arg(:attributed_to_id, non_null(:id))
       arg(:title, non_null(:string))
-      arg(:body, :string)
+      arg(:body, non_null(:string))
       arg(:draft, :boolean, default_value: false)
       arg(:visibility, :post_visibility)
       arg(:publish_at, :datetime)
@@ -65,6 +70,11 @@ defmodule Mobilizon.GraphQL.Schema.PostType do
         description: "The list of tags associated to the post"
       )
 
+      arg(:picture, :picture_input,
+        description:
+          "The banner for the post, either as an object or directly the ID of an existing Picture"
+      )
+
       resolve(&Post.create_post/3)
     end
 
@@ -79,6 +89,11 @@ defmodule Mobilizon.GraphQL.Schema.PostType do
       arg(:publish_at, :datetime)
       arg(:tags, list_of(:string), description: "The list of tags associated to the post")
 
+      arg(:picture, :picture_input,
+        description:
+          "The banner for the post, either as an object or directly the ID of an existing Picture"
+      )
+
       resolve(&Post.update_post/3)
     end
 

lib/graphql/schema/user.ex

@@ -5,7 +5,6 @@ defmodule Mobilizon.GraphQL.Schema.UserType do
   use Absinthe.Schema.Notation
 
   import Absinthe.Resolution.Helpers, only: [dataloader: 1]
-  import Mobilizon.GraphQL.Helpers.Error
 
   alias Mobilizon.Events
   alias Mobilizon.GraphQL.Resolvers.User
@@ -177,7 +176,7 @@ defmodule Mobilizon.GraphQL.Schema.UserType do
       arg(:password, non_null(:string))
       arg(:locale, :string)
 
-      resolve(handle_errors(&User.create_user/3))
+      resolve(&User.create_user/3)
     end
 
     @desc "Validate an user after registration"

lib/mobilizon/posts/post.ex

@@ -4,13 +4,13 @@ defmodule Mobilizon.Posts.Post.TitleSlug do
   """
   use EctoAutoslugField.Slug, from: [:title, :id], to: :slug
 
-  def build_slug([title, id], %Ecto.Changeset{valid?: true}) do
+  def build_slug([title, id], _changeset) do
     [title, ShortUUID.encode!(id)]
     |> Enum.join("-")
     |> Slugger.slugify()
   end
 
-  def build_slug(_sources, %Ecto.Changeset{valid?: false}), do: ""
+  def build_slug(_, _), do: nil
 end
 
 defmodule Mobilizon.Posts.Post do
@@ -22,11 +22,13 @@ defmodule Mobilizon.Posts.Post do
   alias Ecto.Changeset
   alias Mobilizon.Actors.Actor
   alias Mobilizon.Events.Tag
+  alias Mobilizon.Media
   alias Mobilizon.Media.Picture
   alias Mobilizon.Posts.Post.TitleSlug
   alias Mobilizon.Posts.PostVisibility
   alias Mobilizon.Web.Endpoint
   alias Mobilizon.Web.Router.Helpers, as: Routes
+  import Mobilizon.Web.Gettext
 
   @type t :: %__MODULE__{
           url: String.t(),
@@ -82,12 +84,15 @@ defmodule Mobilizon.Posts.Post do
     |> maybe_generate_id()
     |> put_tags(attrs)
     |> maybe_put_publish_date()
+    |> put_picture(attrs)
     # Validate ID and title here because they're needed for slug
-    |> validate_required([:id, :title])
+    |> validate_required(:id)
+    |> validate_required(:title, message: gettext("A title is required for the post"))
+    |> validate_required(:body, message: gettext("A text is required for the post"))
     |> TitleSlug.maybe_generate_slug()
     |> TitleSlug.unique_constraint()
     |> maybe_generate_url()
-    |> validate_required(@required_attrs)
+    |> validate_required(@required_attrs -- [:slug, :url])
   end
 
   defp maybe_generate_id(%Ecto.Changeset{} = changeset) do
@@ -105,7 +110,7 @@ defmodule Mobilizon.Posts.Post do
     with res when res in [:error, {:data, nil}] <- fetch_field(changeset, :url),
          {changes, id_and_slug} when changes in [:changes, :data] <-
            fetch_field(changeset, :slug),
-         url <- generate_url(id_and_slug) do
+         url when is_binary(url) <- generate_url(id_and_slug) do
       put_change(changeset, :url, url)
     else
       _ -> changeset
@@ -113,7 +118,10 @@ defmodule Mobilizon.Posts.Post do
   end
 
   @spec generate_url(String.t()) :: String.t()
-  defp generate_url(id_and_slug), do: Routes.page_url(Endpoint, :post, id_and_slug)
+  defp generate_url(id_and_slug) when is_binary(id_and_slug),
+    do: Routes.page_url(Endpoint, :post, id_and_slug)
+
+  defp generate_url(_), do: nil
 
   @spec put_tags(Ecto.Changeset.t(), map) :: Ecto.Changeset.t()
   defp put_tags(changeset, %{"tags" => tags}),
@@ -134,4 +142,21 @@ defmodule Mobilizon.Posts.Post do
 
     put_change(changeset, :publish_at, publish_at)
   end
+
+  # In case the provided picture is an existing one
+  @spec put_picture(Changeset.t(), map) :: Changeset.t()
+  defp put_picture(%Changeset{} = changeset, %{picture: %{picture_id: id} = _picture}) do
+    case Media.get_picture!(id) do
+      %Picture{} = picture ->
+        put_assoc(changeset, :picture, picture)
+
+      _ ->
+        changeset
+    end
+  end
+
+  # In case it's a new picture
+  defp put_picture(%Changeset{} = changeset, _attrs) do
+    cast_assoc(changeset, :picture)
+  end
 end

lib/mobilizon/posts/posts.ex

@@ -89,9 +89,12 @@ defmodule Mobilizon.Posts do
   """
   @spec create_post(map) :: {:ok, Post.t()} | {:error, Ecto.Changeset.t()}
   def create_post(attrs \\ %{}) do
-    %Post{}
-    |> Post.changeset(attrs)
-    |> Repo.insert()
+    with {:ok, %Post{} = post} <-
+           %Post{}
+           |> Post.changeset(attrs)
+           |> Repo.insert() do
+      {:ok, Repo.preload(post, @post_preloads)}
+    end
   end
 
   @doc """