Typespec fixes and refactoring

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

lib/federation/activity_pub/refresher.ex

@@ -191,8 +191,8 @@ defmodule Mobilizon.Federation.ActivityPub.Refresher do
   defp process_collection(_, _), do: :error
 
   # If we're handling an activity
-  @spec handling_element(map()) :: {:ok, any, struct} | :error
+  @spec handling_element(map() | String.t()) ::
-  @spec handling_element(String.t()) :: {:ok, struct} | {:ok, atom, struct} | {:error, any()}
+          {:ok, any, struct} | {:ok, struct} | {:ok, atom, struct} | {:error, any()} | :error
   defp handling_element(%{"type" => activity_type} = data)
        when activity_type in ["Create", "Update", "Delete"] do
     object = get_in(data, ["object"])

lib/federation/activity_stream/converter/actor.ex

@@ -192,5 +192,5 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Actor do
     Map.put(res, "location", AddressConverter.model_to_as(physical_address))
   end
 
-  defp maybe_add_physical_address(res, %ActorModel{physical_address: _}), do: res
+  defp maybe_add_physical_address(res, _), do: res
 end

lib/federation/activity_stream/converter/discussion.ex

@@ -48,12 +48,8 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Discussion do
   @impl Converter
   @spec as_to_model_data(map) :: map() | {:error, atom()}
   def as_to_model_data(%{"type" => "Note", "name" => name} = object) when is_valid_string(name) do
-    case extract_actors(object) do
+    with %{actor_id: actor_id, creator_id: creator_id} <- extract_actors(object) do
-      %{actor_id: actor_id, creator_id: creator_id} ->
+      %{actor_id: actor_id, creator_id: creator_id, title: name, url: object["id"]}
-        %{actor_id: actor_id, creator_id: creator_id, title: name, url: object["id"]}
-
-      {:error, error} ->
-        {:error, error}
     end
   end
 

lib/federation/activity_stream/converter/media.ex

@@ -46,22 +46,18 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Media do
         actor_id
       )
       when is_binary(media_url) do
-    case upload_media(media_url, name) do
+    with {:ok, %{url: url} = uploaded} <- upload_media(media_url, name) do
-      {:error, err} ->
+      case Medias.get_media_by_url(url) do
-        {:error, err}
+        %MediaModel{file: _file} = media ->
+          {:ok, media}
 
-      {:ok, %{url: url} = uploaded} ->
+        nil ->
-        case Medias.get_media_by_url(url) do
+          Medias.create_media(%{
-          %MediaModel{file: _file} = media ->
+            file: Map.take(uploaded, [:url, :name, :content_type, :size]),
-            {:ok, media}
+            metadata: Map.take(uploaded, [:width, :height, :blurhash]),
-
+            actor_id: actor_id
-          nil ->
+          })
-            Medias.create_media(%{
+      end
-              file: Map.take(uploaded, [:url, :name, :content_type, :size]),
-              metadata: Map.take(uploaded, [:width, :height, :blurhash]),
-              actor_id: actor_id
-            })
-        end
     end
   end
 

lib/mobilizon/actors/actors.ex

@@ -848,7 +848,13 @@ defmodule Mobilizon.Actors do
   @doc """
   Returns a paginated list of members for a group.
   """
-  @spec list_members_for_group(Actor.t(), list(atom()), integer | nil, integer | nil) ::
+  @spec list_members_for_group(
+          Actor.t(),
+          String.t() | nil,
+          list(atom()),
+          integer | nil,
+          integer | nil
+        ) ::
           Page.t(Member.t())
   def list_members_for_group(
         %Actor{id: group_id, type: :Group},

lib/service/activity/renderer/renderer.ex

@@ -29,7 +29,7 @@ defmodule Mobilizon.Service.Activity.Renderer do
 
   @type common_render :: %{body: String.t(), url: String.t()}
 
-  @callback render(entity :: Activity.t(), Keyword.t()) :: common_render()
+  @callback render(activity :: Activity.t(), Keyword.t()) :: common_render()
 
   @spec render(Activity.t()) :: render()
   def render(%Activity{} = activity, options \\ []) do

lib/service/geospatial/google_maps.ex

@@ -72,7 +72,7 @@ defmodule Mobilizon.Service.Geospatial.GoogleMaps do
     end
   end
 
-  @spec build_url(:search | :geocode, map(), list()) :: String.t() | no_return
+  @spec build_url(:search | :geocode | :place_details, map(), list()) :: String.t() | no_return
   defp build_url(method, args, options) do
     limit = Keyword.get(options, :limit, 10)
     lang = Keyword.get(options, :lang, "en")

lib/service/notifications/scheduler.ex

@@ -289,7 +289,7 @@ defmodule Mobilizon.Service.Notifications.Scheduler do
         {:ok, nil}
 
       # Sending to calculated time
-      true ->
+      match?(%DateTime{}, send_at) ->
         Notification.enqueue(:pending_membership_notification, params, scheduled_at: send_at)
     end
   end

lib/service/notifier/push.ex

@@ -73,7 +73,7 @@ defmodule Mobilizon.Service.Notifier.Push do
     Map.get(@default_behavior, activity_setting, false)
   end
 
-  @spec send_subscription(Activity.t(), any, Keyword.t()) :: no_return
+  @spec send_subscription(Activity.t(), any, Keyword.t()) :: any()
   defp send_subscription(activity, subscription, options) do
     activity
     |> payload(options)

lib/service/workers/helper.ex

@@ -41,7 +41,7 @@ defmodule Mobilizon.Service.Workers.Helper do
 
       alias Oban.Job
 
-      @spec enqueue(String.t() | :atom, map(), Keyword.t()) ::
+      @spec enqueue(String.t() | atom(), map(), Keyword.t()) ::
               {:ok, Job.t()} | {:error, Ecto.Changeset.t()}
       def enqueue(operation, params, worker_args \\ []) do
         params = Map.merge(%{"op" => operation}, params)