From c3aa145148fbb5019e6c89e09f372d1b9fb12802 Mon Sep 17 00:00:00 2001
From: Thomas Citharel <tcit@tcit.fr>
Date: Thu, 4 Jan 2024 12:19:28 +0100
Subject: [PATCH] refactor(backend): extract convert_ecto_errors in the
 Mobilizon.Storage.Ecto module

And use it to log refreshing instance errors

Signed-off-by: Thomas Citharel <tcit@tcit.fr>
---
 lib/graphql/error.ex                     | 27 ++----------------------
 lib/mobilizon/storage/ecto.ex            | 27 ++++++++++++++++++++++++
 lib/service/workers/refresh_instances.ex |  5 +++++
 3 files changed, 34 insertions(+), 25 deletions(-)

diff --git a/lib/graphql/error.ex b/lib/graphql/error.ex
index 6223fd509..ddcf0df1d 100644
--- a/lib/graphql/error.ex
+++ b/lib/graphql/error.ex
@@ -5,8 +5,8 @@ defmodule Mobilizon.GraphQL.Error do
 
   require Logger
   alias __MODULE__
-  alias Mobilizon.Web.Gettext, as: GettextBackend
   import Mobilizon.Web.Gettext, only: [dgettext: 2]
+  import Mobilizon.Storage.Ecto, only: [convert_ecto_errors: 1]
 
   @type t :: %{code: atom(), message: String.t(), status_code: pos_integer(), field: atom()}
 
@@ -64,7 +64,7 @@ defmodule Mobilizon.GraphQL.Error do
 
   defp handle(%Ecto.Changeset{} = changeset) do
     changeset
-    |> Ecto.Changeset.traverse_errors(&translate_error/1)
+    |> convert_ecto_errors()
     |> Enum.map(fn {k, v} ->
       %Error{
         code: :validation,
@@ -126,27 +126,4 @@ defmodule Mobilizon.GraphQL.Error do
     Logger.warning("Unhandled error code: #{inspect(code)}")
     {422, to_string(code)}
   end
-
-  # Translates an error message using gettext.
-  defp translate_error({msg, opts}) do
-    # Because error messages were defined within Ecto, we must
-    # call the Gettext module passing our Gettext backend. We
-    # also use the "errors" domain as translations are placed
-    # in the errors.po file.
-    # Ecto will pass the :count keyword if the error message is
-    # meant to be pluralized.
-    # On your own code and templates, depending on whether you
-    # need the message to be pluralized or not, this could be
-    # written simply as:
-    #
-    #     dngettext "errors", "1 file", "%{count} files", count
-    #     dgettext "errors", "is invalid"
-    #
-
-    if count = opts[:count] do
-      Gettext.dngettext(GettextBackend, "errors", msg, msg, count, opts)
-    else
-      Gettext.dgettext(GettextBackend, "errors", msg, opts)
-    end
-  end
 end
diff --git a/lib/mobilizon/storage/ecto.ex b/lib/mobilizon/storage/ecto.ex
index 16421cc0c..43f42f3ad 100644
--- a/lib/mobilizon/storage/ecto.ex
+++ b/lib/mobilizon/storage/ecto.ex
@@ -7,6 +7,7 @@ defmodule Mobilizon.Storage.Ecto do
   import Ecto.Changeset, only: [fetch_change: 2, put_change: 3, get_field: 2]
   alias Ecto.{Changeset, Query}
   alias Mobilizon.Web.Endpoint
+  alias Mobilizon.Web.Gettext, as: GettextBackend
   alias Mobilizon.Web.Router.Helpers, as: Routes
 
   @doc """
@@ -56,4 +57,30 @@ defmodule Mobilizon.Storage.Ecto do
       changeset
     end
   end
+
+  def convert_ecto_errors(%Ecto.Changeset{} = changeset),
+    do: Ecto.Changeset.traverse_errors(changeset, &translate_error/1)
+
+  # Translates an error message using gettext.
+  defp translate_error({msg, opts}) do
+    # Because error messages were defined within Ecto, we must
+    # call the Gettext module passing our Gettext backend. We
+    # also use the "errors" domain as translations are placed
+    # in the errors.po file.
+    # Ecto will pass the :count keyword if the error message is
+    # meant to be pluralized.
+    # On your own code and templates, depending on whether you
+    # need the message to be pluralized or not, this could be
+    # written simply as:
+    #
+    #     dngettext "errors", "1 file", "%{count} files", count
+    #     dgettext "errors", "is invalid"
+    #
+
+    if count = opts[:count] do
+      Gettext.dngettext(GettextBackend, "errors", msg, msg, count, opts)
+    else
+      Gettext.dgettext(GettextBackend, "errors", msg, opts)
+    end
+  end
 end
diff --git a/lib/service/workers/refresh_instances.ex b/lib/service/workers/refresh_instances.ex
index a1b93cf36..1edf15a29 100644
--- a/lib/service/workers/refresh_instances.ex
+++ b/lib/service/workers/refresh_instances.ex
@@ -13,6 +13,7 @@ defmodule Mobilizon.Service.Workers.RefreshInstances do
   alias Mobilizon.Instances.{Instance, InstanceActor}
   alias Oban.Job
   require Logger
+  import Mobilizon.Storage.Ecto, only: [convert_ecto_errors: 1]
 
   @impl Oban.Worker
   @spec perform(Oban.Job.t()) :: :ok
@@ -56,6 +57,10 @@ defmodule Mobilizon.Service.Workers.RefreshInstances do
              Instances.create_instance_actor(args) do
         Logger.info("Saved instance actor details for domain #{host}")
       else
+        {:error, %Ecto.Changeset{} = changeset} ->
+          Logger.error("Unable to save instance \"#{domain}\" metadata")
+          Logger.debug(convert_ecto_errors(changeset))
+
         err ->
           Logger.error(inspect(err))
       end