Spec fixes

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

lib/federation/activity_pub/actions/reject.ex

@@ -30,16 +30,10 @@ defmodule Mobilizon.Federation.ActivityPub.Actions.Reject do
         :invite -> reject_invite(entity, additional)
       end
 
-    with {:ok, activity} <- create_activity(update_data, local),
+    {:ok, activity} = create_activity(update_data, local)
-         :ok <- maybe_federate(activity),
+    maybe_federate(activity)
-         :ok <- maybe_relay_if_group_activity(activity) do
+    maybe_relay_if_group_activity(activity)
     {:ok, activity, entity}
-    else
-      err ->
-        Logger.error("Something went wrong while creating an activity")
-        Logger.debug(inspect(err))
-        err
-    end
   end
 
   @spec reject_join(Participant.t(), map()) :: {:ok, Participant.t(), Activity.t()} | any()

lib/federation/activity_pub/transmogrifier.ex

@@ -1016,6 +1016,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
   end
 
   # Comment and conversations have different attributes for actor and groups
+  @spec transform_object_data_for_discussion(map()) :: map()
   defp transform_object_data_for_discussion(object_data) do
     # Basic comment
     if is_data_a_discussion_initialization?(object_data) do

lib/federation/activity_pub/utils.ex

@@ -176,7 +176,8 @@ defmodule Mobilizon.Federation.ActivityPub.Utils do
     :ok
   end
 
-  @spec do_maybe_relay_if_group_activity(map(), list(String.t()) | String.t()) :: :ok
+  # TODO: Is this a map or a String?
+  @spec do_maybe_relay_if_group_activity(map() | String.t(), list(String.t()) | String.t()) :: :ok
   defp do_maybe_relay_if_group_activity(object, attributed_to) when is_list(attributed_to),
     do: do_maybe_relay_if_group_activity(object, hd(attributed_to))
 

lib/federation/activity_stream/converter/discussion.ex

@@ -68,6 +68,8 @@ defmodule Mobilizon.Federation.ActivityStream.Converter.Discussion do
       %{actor_id: actor_id, creator_id: creator_id}
     else
       {:error, error} -> {:error, error}
+      {:ok, %Actor{url: ^creator_url}} -> {:error, :creator_suspended}
+      {:ok, %Actor{url: ^actor_url}} -> {:error, :actor_suspended}
     end
   end
 end

lib/graphql/api/comments.ex

@@ -11,7 +11,9 @@ defmodule Mobilizon.GraphQL.API.Comments do
   @doc """
   Create a comment
   """
-  @spec create_comment(map) :: {:ok, Activity.t(), Comment.t()} | any
+  @spec create_comment(map) ::
+          {:ok, Activity.t(), Comment.t()}
+          | {:error, :entity_tombstoned | atom() | Ecto.Changeset.t()}
   def create_comment(args) do
     args = extract_pictures_from_comment_body(args)
     Actions.Create.create(:comment, args, true)
@@ -20,7 +22,8 @@ defmodule Mobilizon.GraphQL.API.Comments do
   @doc """
   Updates a comment
   """
-  @spec update_comment(Comment.t(), map()) :: {:ok, Activity.t(), Comment.t()} | any
+  @spec update_comment(Comment.t(), map()) ::
+          {:ok, Activity.t(), Comment.t()} | {:error, atom() | Ecto.Changeset.t()}
   def update_comment(%Comment{} = comment, args) do
     args = extract_pictures_from_comment_body(args)
     Actions.Update.update(comment, args, true)
@@ -37,7 +40,9 @@ defmodule Mobilizon.GraphQL.API.Comments do
   @doc """
   Creates a discussion (or reply to a discussion)
   """
-  @spec create_discussion(map()) :: map()
+  @spec create_discussion(map()) ::
+          {:ok, Activity.t(), Discussion.t()}
+          | {:error, :entity_tombstoned | atom | Ecto.Changeset.t()}
   def create_discussion(args) do
     args = extract_pictures_from_comment_body(args)
 

lib/graphql/error.ex

@@ -20,7 +20,7 @@ defmodule Mobilizon.GraphQL.Error do
   # Error Tuples
   # ------------
   # Regular errors
-  @spec normalize(error | list(error) | String.t() | any()) :: t()
+  @spec normalize(any()) :: t() | list(t())
   def normalize({:error, reason}) do
     handle(reason)
   end

lib/graphql/resolvers/comment.ex

@@ -106,7 +106,8 @@ defmodule Mobilizon.GraphQL.Resolvers.Comment do
     case Discussions.get_comment_with_preload(comment_id) do
       %CommentModel{deleted_at: nil} = comment ->
         cond do
-          {:comment_can_be_managed, true} == CommentModel.can_be_managed_by(comment, actor_id) ->
+          {:comment_can_be_managed, true} ==
+              {:comment_can_be_managed, CommentModel.can_be_managed_by?(comment, actor_id)} ->
             do_delete_comment(comment, actor)
 
           role in [:moderator, :administrator] ->

lib/graphql/resolvers/resource.ex

@@ -107,11 +107,11 @@ defmodule Mobilizon.GraphQL.Resolvers.Resource do
           }
         } = _resolution
       ) do
-    with {:member, true} <- {:member, Actors.is_member?(actor_id, group_id)},
+    if Actors.is_member?(actor_id, group_id) do
-         parent <- get_eventual_parent(args),
+      parent = get_eventual_parent(args)
-         {:own_check, true} <- {:own_check, check_resource_owned_by_group(parent, group_id)},
+
-         {:ok, _, %Resource{} = resource} <-
+      if check_resource_owned_by_group(parent, group_id) do
-           Actions.Create.create(
+        case Actions.Create.create(
                :resource,
                args
                |> Map.put(:actor_id, group_id)
@@ -119,15 +119,16 @@ defmodule Mobilizon.GraphQL.Resolvers.Resource do
                true,
                %{}
              ) do
+          {:ok, _, %Resource{} = resource} ->
             {:ok, resource}
-    else
+
-      {:error, _} ->
+          {:error, _err} ->
             {:error, dgettext("errors", "Error while creating resource")}
-
+        end
-      {:own_check, _} ->
+      else
         {:error, dgettext("errors", "Parent resource doesn't belong to this group")}
-
+      end
-      {:member, _} ->
+    else
       {:error, dgettext("errors", "Profile is not member of group")}
     end
   end

lib/graphql/resolvers/tag.ex

@@ -54,7 +54,7 @@ defmodule Mobilizon.GraphQL.Resolvers.Tag do
   @doc """
   Retrieve the list of related tags for a parent tag
   """
-  @spec list_tags_for_post(Tag.t(), map(), Absinthe.Resolution.t()) :: {:ok, list(Tag.t())}
+  @spec get_related_tags(Tag.t(), map(), Absinthe.Resolution.t()) :: {:ok, list(Tag.t())}
   def get_related_tags(%Tag{} = tag, _args, _resolution) do
     {:ok, Events.list_tag_neighbors(tag)}
   end

lib/graphql/resolvers/user.ex

@@ -425,13 +425,13 @@ defmodule Mobilizon.GraphQL.Resolvers.User do
   def change_email(_parent, %{email: new_email, password: password}, %{
         context: %{current_user: %User{email: old_email} = user}
       }) do
-    with {:can_change_password, true} <-
+    if Authenticator.can_change_email?(user) do
-           {:can_change_password, Authenticator.can_change_email?(user)},
+      case Authenticator.login(old_email, password) do
-         {:current_password, {:ok, %User{}}} <-
+        {:ok, %User{}} ->
-           {:current_password, Authenticator.login(user.email, password)},
+          if new_email != old_email do
-         {:same_email, false} <- {:same_email, new_email == old_email},
+            if Email.Checker.valid?(new_email) do
-         {:email_valid, true} <- {:email_valid, Email.Checker.valid?(new_email)},
+              case Users.update_user_email(user, new_email) do
-         {:ok, %User{} = user} <- Users.update_user_email(user, new_email) do
+                {:ok, %User{} = user} ->
                   user
                   |> Email.User.send_email_reset_old_email()
                   |> Email.Mailer.send_email_later()
@@ -441,20 +441,24 @@ defmodule Mobilizon.GraphQL.Resolvers.User do
                   |> Email.Mailer.send_email_later()
 
                   {:ok, user}
-    else
-      {:current_password, {:error, _}} ->
-        {:error, dgettext("errors", "The password provided is invalid")}
-
-      {:same_email, true} ->
-        {:error, dgettext("errors", "The new email must be different")}
-
-      {:email_valid, false} ->
-        {:error, dgettext("errors", "The new email doesn't seem to be valid")}
 
                 {:error, %Ecto.Changeset{} = err} ->
                   Logger.debug(inspect(err))
                   {:error, dgettext("errors", "Failed to update user email")}
               end
+            else
+              {:error, dgettext("errors", "The new email doesn't seem to be valid")}
+            end
+          else
+            {:error, dgettext("errors", "The new email must be different")}
+          end
+
+        {:error, _} ->
+          {:error, dgettext("errors", "The password provided is invalid")}
+      end
+    else
+      {:error, dgettext("errors", "User cannot change email")}
+    end
   end
 
   def change_email(_parent, _args, _resolution) do
@@ -635,8 +639,9 @@ defmodule Mobilizon.GraphQL.Resolvers.User do
            user,
          context
        ) do
-    with current_ip <- Map.get(context, :ip),
+    current_ip = Map.get(context, :ip)
-         now <- DateTime.utc_now() do
+    now = DateTime.utc_now()
+
     Users.update_user(user, %{
       last_sign_in_at: current_sign_in_at || now,
       last_sign_in_ip: current_sign_in_ip || current_ip,
@@ -644,5 +649,4 @@ defmodule Mobilizon.GraphQL.Resolvers.User do
       current_sign_in_at: now
     })
   end
-  end
 end

lib/mix/tasks/mobilizon/actors/refresh.ex

@@ -71,10 +71,7 @@ defmodule Mix.Tasks.Mobilizon.Actors.Refresh do
         Actor #{preferred_username} refreshed
         """)
 
-      {:error, err} when is_binary(err) ->
+      {:error, _err} ->
-        shell_error(err)
-
-      _err ->
         shell_error("Error while refreshing actor #{preferred_username}")
     end
   end

lib/mix/tasks/mobilizon/instance.ex

@@ -187,14 +187,11 @@ defmodule Mix.Tasks.Mobilizon.Instance do
     end
   end
 
+  @spec write_config(String.t(), String.t()) :: :ok | {:error, atom()}
   defp write_config(config_path, result_config) do
     shell_info("Writing config to #{config_path}.")
 
-    case File.write(config_path, result_config) do
+    with {:error, err} <- File.write(config_path, result_config) do
-      :ok ->
-        :ok
-
-      {:error, err} ->
       shell_error(
         "\nERROR: Unable to write config file to #{config_path}. Make sure you have permissions on the destination and that the parent path exists.\n"
       )
@@ -203,14 +200,11 @@ defmodule Mix.Tasks.Mobilizon.Instance do
     end
   end
 
+  @spec write_psql(String.t(), String.t()) :: :ok | {:error, atom()}
   defp write_psql(psql_path, result_psql) do
     shell_info("Writing #{psql_path}.")
 
-    case File.write(psql_path, result_psql) do
+    with {:error, err} <- File.write(psql_path, result_psql) do
-      :ok ->
-        :ok
-
-      {:error, err} ->
       shell_error(
         "\nERROR: Unable to write psql file to #{psql_path}. Make sure you have permissions on the destination and that the parent path exists.\n"
       )

lib/mix/tasks/mobilizon/media/clean_orphan.ex

@@ -34,8 +34,8 @@ defmodule Mix.Tasks.Mobilizon.Media.CleanOrphan do
 
     start_mobilizon()
 
-    case CleanOrphanMedia.clean(dry_run: dry_run, grace_period: grace_period) do
+    {:ok, medias} = CleanOrphanMedia.clean(dry_run: dry_run, grace_period: grace_period)
-      {:ok, medias} ->
+
     if length(medias) > 0 do
       if dry_run or verbose do
         details(medias, dry_run, verbose)
@@ -45,12 +45,6 @@ defmodule Mix.Tasks.Mobilizon.Media.CleanOrphan do
     else
       empty_result(dry_run)
     end
-
-        :ok
-
-      _err ->
-        shell_error("Error while cleaning orphan media files")
-    end
   end
 
   @spec details(list(Media.t()), boolean(), boolean()) :: :ok
@@ -70,7 +64,7 @@ defmodule Mix.Tasks.Mobilizon.Media.CleanOrphan do
     end)
   end
 
-  @spec result(boolean(), boolean()) :: :ok
+  @spec result(boolean(), non_neg_integer()) :: :ok
   defp result(dry_run, nb_medias) do
     if dry_run do
       shell_info("#{nb_medias} files would have been deleted")

lib/mix/tasks/mobilizon/relay/accept.ex

@@ -13,7 +13,7 @@ defmodule Mix.Tasks.Mobilizon.Relay.Accept do
     start_mobilizon()
 
     case Relay.accept(target) do
-      {:ok, _activity} ->
+      {:ok, _activity, _follow} ->
         # put this task to sleep to allow the genserver to push out the messages
         :timer.sleep(500)
 

lib/mix/tasks/mobilizon/users/clean.ex

@@ -35,8 +35,9 @@ defmodule Mix.Tasks.Mobilizon.Users.Clean do
 
     start_mobilizon()
 
-    case CleanUnconfirmedUsers.clean(dry_run: dry_run, grace_period: grace_period) do
+    {:ok, deleted_users} =
-      {:ok, deleted_users} ->
+      CleanUnconfirmedUsers.clean(dry_run: dry_run, grace_period: grace_period)
+
     if length(deleted_users) > 0 do
       if dry_run or verbose do
         details(deleted_users, dry_run, verbose)
@@ -48,10 +49,6 @@ defmodule Mix.Tasks.Mobilizon.Users.Clean do
     end
 
     :ok
-
-      _err ->
-        shell_error("Error while cleaning unconfirmed users")
-    end
   end
 
   @spec details(list(Media.t()), boolean(), boolean()) :: :ok

lib/mobilizon/admin/admin.ex

@@ -48,19 +48,17 @@ defmodule Mobilizon.Admin do
   @spec log_action(Actor.t(), String.t(), struct()) ::
           {:ok, ActionLog.t()} | {:error, Ecto.Changeset.t() | :user_not_moderator}
   def log_action(%Actor{user_id: user_id, id: actor_id}, action, target) do
-    with %User{role: role} <- Users.get_user!(user_id),
+    %User{role: role} = Users.get_user!(user_id)
-         {:role, true} <- {:role, role in [:administrator, :moderator]},
+
-         {:ok, %ActionLog{} = create_action_log} <-
+    if role in [:administrator, :moderator] do
       Admin.create_action_log(%{
         "actor_id" => actor_id,
         "target_type" => to_string(target.__struct__),
         "target_id" => target.id,
         "action" => action,
         "changes" => stringify_struct(target)
-           }) do
+      })
-      {:ok, create_action_log}
     else
-      {:role, false} ->
       {:error, :user_not_moderator}
     end
   end

lib/mobilizon/discussions/comment.ex

@@ -115,13 +115,13 @@ defmodule Mobilizon.Discussions.Comment do
   @doc """
   Checks whether an comment can be managed.
   """
-  @spec can_be_managed_by(t, integer | String.t()) :: boolean
+  @spec can_be_managed_by?(t, integer | String.t()) :: boolean()
-  def can_be_managed_by(%__MODULE__{actor_id: creator_actor_id}, actor_id)
+  def can_be_managed_by?(%__MODULE__{actor_id: creator_actor_id}, actor_id)
       when creator_actor_id == actor_id do
-    {:comment_can_be_managed, true}
+    creator_actor_id == actor_id
   end
 
-  def can_be_managed_by(_comment, _actor), do: {:comment_can_be_managed, false}
+  def can_be_managed_by?(_comment, _actor), do: false
 
   defp common_changeset(%__MODULE__{} = comment, attrs) do
     comment

lib/mobilizon/events/events.ex

@@ -572,7 +572,7 @@ defmodule Mobilizon.Events do
   @doc """
   Returns the list of tags.
   """
-  @spec list_tags(String.t() | nil, integer | nil, integer | nil) :: [Tag.t()]
+  @spec list_tags(String.t() | nil, integer | nil, integer | nil) :: Page.t(Tag.t())
   def list_tags(filter \\ nil, page \\ nil, limit \\ nil) do
     Tag
     |> tag_filter(filter)
@@ -608,7 +608,7 @@ defmodule Mobilizon.Events do
   Creates a relation between two tags.
   """
   @spec create_tag_relation(map) :: {:ok, TagRelation.t()} | {:error, Changeset.t()}
-  def create_tag_relation(attrs \\ {}) do
+  def create_tag_relation(attrs) do
     %TagRelation{}
     |> TagRelation.changeset(attrs)
     |> Repo.insert(

lib/mobilizon/posts/posts.ex

@@ -131,7 +131,7 @@ defmodule Mobilizon.Posts do
     |> Repo.all()
   end
 
-  @spec tags_for_post_query(integer) :: Ecto.Query.t()
+  @spec tags_for_post_query(String.t()) :: Ecto.Query.t()
   defp tags_for_post_query(post_id) do
     from(
       t in Tag,

lib/mobilizon/reports/reports.ex

@@ -49,7 +49,8 @@ defmodule Mobilizon.Reports do
   @doc """
   Returns the list of reports.
   """
-  @spec list_reports(integer | nil, integer | nil, atom, atom, ReportStatus) :: Page.t()
+  @spec list_reports(integer | nil, integer | nil, atom, atom, ReportStatus.t()) ::
+          Page.t(Report.t())
   def list_reports(
         page \\ nil,
         limit \\ nil,

lib/mobilizon/storage/repo.ex

@@ -10,7 +10,7 @@ defmodule Mobilizon.Storage.Repo do
   @doc """
   Dynamically loads the repository url from the DATABASE_URL environment variable.
   """
-  @spec init(any(), any()) :: any()
+  @spec init(any(), any()) :: {:ok, Keyword.t()}
   def init(_, opts) do
     {:ok, opts}
   end

lib/mobilizon/users/user.ex

@@ -31,8 +31,8 @@ defmodule Mobilizon.Users.User do
           feed_tokens: [FeedToken.t()],
           last_sign_in_at: DateTime.t(),
           last_sign_in_ip: String.t(),
-          current_sign_in_ip: String.t(),
+          current_sign_in_ip: String.t() | nil,
-          current_sign_in_at: DateTime.t(),
+          current_sign_in_at: DateTime.t() | nil,
           activity_settings: [ActivitySetting.t()],
           settings: Setting.t(),
           unconfirmed_email: String.t() | nil

lib/service/activity/activity.ex

@@ -6,8 +6,8 @@ defmodule Mobilizon.Service.Activity do
   alias Mobilizon.Activities.Activity
   alias Mobilizon.Service.Activity.{Comment, Discussion, Event, Group, Member, Post, Resource}
 
-  @callback insert_activity(entity :: struct(), options :: map()) ::
+  @callback insert_activity(entity :: struct(), options :: Keyword.t()) ::
-              {:ok, Oban.Job.t()} | {:ok, any()}
+              {:ok, Oban.Job.t()} | {:ok, any()} | {:error, Ecto.Changeset.t()}
 
   @callback get_object(object_id :: String.t() | integer()) :: struct()
 

lib/service/activity/member.ex

@@ -40,7 +40,7 @@ defmodule Mobilizon.Service.Activity.Member do
     Actors.get_member(member_id)
   end
 
-  @spec get_author(Member.t(), Member.t() | nil) :: integer()
+  @spec get_author(Member.t(), Keyword.t()) :: integer()
   defp get_author(%Member{actor_id: actor_id}, options) do
     moderator = Keyword.get(options, :moderator)
 

lib/service/auth/ldap_authenticator.ex

@@ -191,14 +191,10 @@ defmodule Mobilizon.Service.Auth.LDAPAuthenticator do
     ])
   end
 
-  @spec register_user(String.t()) :: User.t() | any()
+  @spec register_user(String.t()) :: User.t() | {:error, Ecto.Changeset.t()}
   defp register_user(email) do
-    case Users.create_external(email, "ldap") do
+    with {:ok, %User{} = user} <- Users.create_external(email, "ldap") do
-      {:ok, %User{} = user} ->
       user
-
-      error ->
-        error
     end
   end
 

lib/service/clean_orphan_media.ex

@@ -19,7 +19,7 @@ defmodule Mobilizon.Service.CleanOrphanMedia do
    * `grace_period` how old in hours can the media be before it's taken into account for deletion
    * `dry_run` just return the media that would have been deleted, don't actually delete it
   """
-  @spec clean(Keyword.t()) :: {:ok, list(Media.t())} | {:error, String.t()}
+  @spec clean(Keyword.t()) :: {:ok, list(Media.t())}
   def clean(opts \\ []) do
     medias = find_media(opts)
 

lib/service/clean_unconfirmed_users.ex

@@ -15,7 +15,7 @@ defmodule Mobilizon.Service.CleanUnconfirmedUsers do
 
   Remove media that is not attached to an entity, such as media uploads that were never used in entities.
   """
-  @spec clean(Keyword.t()) :: {:ok, list(Media.t())} | {:error, String.t()}
+  @spec clean(Keyword.t()) :: {:ok, list(Media.t())}
   def clean(opts \\ []) do
     users_to_delete = find_unconfirmed_users_to_clean(opts)
 

lib/service/export/feed.ex

@@ -21,49 +21,41 @@ defmodule Mobilizon.Service.Export.Feed do
 
   @item_limit 500
 
-  def version, do: Config.instance_version()
+  @spec version :: String.t()
+  defp version, do: Config.instance_version()
 
-  @spec create_cache(String.t()) :: {:commit, String.t()} | {:ignore, any()}
+  @spec create_cache(String.t()) ::
+          {:commit, String.t()}
+          | {:ignore, :actor_not_found | :actor_not_public | :bad_token | :token_not_found}
   def create_cache("actor_" <> name) do
     case fetch_actor_event_feed(name) do
       {:ok, res} ->
         {:commit, res}
 
-      err ->
+      {:error, err} ->
         {:ignore, err}
     end
   end
 
-  @spec create_cache(String.t()) :: {:commit, String.t()} | {:ignore, any()}
   def create_cache("token_" <> token) do
     case fetch_events_from_token(token) do
       {:ok, res} ->
         {:commit, res}
 
-      err ->
+      {:error, err} ->
         {:ignore, err}
     end
   end
 
   def create_cache("instance") do
-    case fetch_instance_feed() do
+    {:ok, res} = fetch_instance_feed()
-      {:ok, res} ->
     {:commit, res}
-
-      err ->
-        {:ignore, err}
-    end
   end
 
   @spec fetch_instance_feed :: {:ok, String.t()}
   defp fetch_instance_feed do
-    case Common.fetch_instance_public_content(@item_limit) do
+    {:ok, events, posts} = Common.fetch_instance_public_content(@item_limit)
-      {:ok, events, posts} ->
     {:ok, build_instance_feed(events, posts)}
-
-      err ->
-        {:error, err}
-    end
   end
 
   # Build an atom feed from the whole instance and its public events and posts
@@ -90,7 +82,8 @@ defmodule Mobilizon.Service.Export.Feed do
     |> Atomex.generate_document()
   end
 
-  @spec fetch_actor_event_feed(String.t(), integer()) :: String.t()
+  @spec fetch_actor_event_feed(String.t(), integer()) ::
+          {:ok, String.t()} | {:error, :actor_not_found | :actor_not_public}
   defp fetch_actor_event_feed(name, limit \\ @item_limit) do
     case Common.fetch_actor_event_feed(name, limit) do
       {:ok, actor, events, posts} ->
@@ -199,7 +192,8 @@ defmodule Mobilizon.Service.Export.Feed do
   end
 
   # Only events, not posts
-  @spec fetch_events_from_token(String.t(), integer()) :: {:ok, String.t()} | {:error, atom()}
+  @spec fetch_events_from_token(String.t(), integer()) ::
+          {:ok, String.t()} | {:error, :bad_token | :token_not_found}
   defp fetch_events_from_token(token, limit \\ @item_limit) do
     case Common.fetch_events_from_token(token, limit) do
       %{events: events, token: token, user: user, actor: actor, type: type} ->

lib/service/geospatial/addok.ex

@@ -93,11 +93,7 @@ defmodule Mobilizon.Service.Geospatial.Addok do
     if is_nil(value), do: url, else: do_add_parameter(url, key, value)
   end
 
-  @spec do_add_parameter(String.t(), :coords | :type, %{lat: float, lon: float} | :administrative) ::
+  @spec do_add_parameter(String.t(), :type, :administrative | atom()) :: String.t()
-          String.t()
-  defp do_add_parameter(url, :coords, coords),
-    do: "#{url}&lat=#{coords.lat}&lon=#{coords.lon}"
-
   defp do_add_parameter(url, :type, :administrative),
     do: "#{url}&type=municipality"
 

lib/service/geospatial/google_maps.ex

@@ -31,16 +31,18 @@ defmodule Mobilizon.Service.Geospatial.GoogleMaps do
   @doc """
   Google Maps implementation for `c:Mobilizon.Service.Geospatial.Provider.geocode/3`.
   """
-  @spec geocode(String.t(), keyword()) :: list(Address.t()) | no_return
+  @spec geocode(float(), float(), keyword()) :: list(Address.t())
   def geocode(lon, lat, options \\ []) do
     url = build_url(:geocode, %{lon: lon, lat: lat}, options)
 
     Logger.debug("Asking Google Maps for reverse geocode with #{url}")
 
-    with {:ok, %{status: 200, body: body}} <- GeospatialClient.get(url),
+    %Tesla.Env{status: 200, body: body} = GeospatialClient.get!(url)
-         %{"results" => results, "status" => "OK"} <- body do
+
-      Enum.map(results, fn entry -> process_data(entry, options) end)
+    case body do
-    else
+      %{"results" => results, "status" => "OK"} ->
+        Enum.map(results, &process_data(&1, options))
+
       %{"status" => "REQUEST_DENIED", "error_message" => error_message} ->
         raise ArgumentError, message: to_string(error_message)
     end
@@ -50,16 +52,18 @@ defmodule Mobilizon.Service.Geospatial.GoogleMaps do
   @doc """
   Google Maps implementation for `c:Mobilizon.Service.Geospatial.Provider.search/2`.
   """
-  @spec search(String.t(), keyword()) :: list(Address.t()) | no_return
+  @spec search(String.t(), keyword()) :: list(Address.t())
   def search(q, options \\ []) do
     url = build_url(:search, %{q: q}, options)
 
     Logger.debug("Asking Google Maps for addresses with #{url}")
 
-    with {:ok, %{status: 200, body: body}} <- GeospatialClient.get(url),
+    %Tesla.Env{status: 200, body: body} = GeospatialClient.get!(url)
-         %{"results" => results, "status" => "OK"} <- body do
+
+    case body do
+      %{"results" => results, "status" => "OK"} ->
         results |> Enum.map(fn entry -> process_data(entry, options) end)
-    else
+
       %{"status" => "REQUEST_DENIED", "error_message" => error_message} ->
         raise ArgumentError, message: to_string(error_message)
 
@@ -68,7 +72,7 @@ defmodule Mobilizon.Service.Geospatial.GoogleMaps do
     end
   end
 
-  @spec build_url(atom(), map(), list()) :: String.t() | no_return
+  @spec build_url(:search | :geocode, 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")
@@ -97,6 +101,7 @@ defmodule Mobilizon.Service.Geospatial.GoogleMaps do
     URI.encode(uri)
   end
 
+  @spec process_data(map(), Keyword.t()) :: Address.t()
   defp process_data(
          %{
            "formatted_address" => description,
@@ -148,16 +153,18 @@ defmodule Mobilizon.Service.Geospatial.GoogleMaps do
     end
   end
 
-  @spec do_fetch_place_details(String.t() | nil, Keyword.t()) :: String.t() | no_return
+  @spec do_fetch_place_details(String.t() | nil, Keyword.t()) :: String.t() | nil
   defp do_fetch_place_details(place_id, options) do
     url = build_url(:place_details, %{place_id: place_id}, options)
 
     Logger.debug("Asking Google Maps for details with #{url}")
 
-    with {:ok, %{status: 200, body: body}} <- GeospatialClient.get(url),
+    %Tesla.Env{status: 200, body: body} = GeospatialClient.get!(url)
-         %{"result" => %{"name" => name}, "status" => "OK"} <- body do
+
+    case body do
+      %{"result" => %{"name" => name}, "status" => "OK"} ->
         name
-    else
+
       %{"status" => "REQUEST_DENIED", "error_message" => error_message} ->
         raise ArgumentError, message: to_string(error_message)
 

lib/service/geospatial/provider.ex

@@ -43,7 +43,7 @@ defmodule Mobilizon.Service.Geospatial.Provider do
       %Address{}
   """
   @callback geocode(longitude :: number, latitude :: number, options :: keyword) ::
-              [Address.t()] | {:error, atom()}
+              [Address.t()]
 
   @doc """
   Search for an address
@@ -63,23 +63,21 @@ defmodule Mobilizon.Service.Geospatial.Provider do
       iex> search("10 rue Jangot")
       %Address{}
   """
-  @callback search(address :: String.t(), options :: keyword) :: [Address.t()] | {:error, atom()}
+  @callback search(address :: String.t(), options :: keyword) :: [Address.t()]
 
   @doc """
   Returns a `Geo.Point` for given coordinates
   """
-  @spec coordinates([number | String.t()], number) :: Geo.Point.t() | nil
+  @spec coordinates([number | String.t()]) :: Geo.Point.t() | nil
-  def coordinates(coords, srid \\ 4326)
+  def coordinates([x, y]) when is_number(x) and is_number(y) do
-
+    %Geo.Point{coordinates: {x, y}, srid: 4326}
-  def coordinates([x, y], srid) when is_number(x) and is_number(y) do
-    %Geo.Point{coordinates: {x, y}, srid: srid}
   end
 
-  def coordinates([x, y], srid) when is_binary(x) and is_binary(y) do
+  def coordinates([x, y]) when is_binary(x) and is_binary(y) do
-    %Geo.Point{coordinates: {String.to_float(x), String.to_float(y)}, srid: srid}
+    %Geo.Point{coordinates: {String.to_float(x), String.to_float(y)}, srid: 4326}
   end
 
-  def coordinates(_, _), do: nil
+  def coordinates(_), do: nil
 
   @spec endpoint(atom()) :: String.t()
   def endpoint(provider) do

test/tasks/media/clean_orphan_test.exs

@@ -113,17 +113,6 @@ defmodule Mix.Tasks.Mobilizon.Media.CleanOrphanTest do
     end
   end
 
-  describe "returns an error" do
-    test "for some reason" do
-      with_mock CleanOrphanMedia,
-        clean: fn [dry_run: false, grace_period: 48] -> {:error, "Some error"} end do
-        CleanOrphan.run([])
-        assert_received {:mix_shell, :error, [output_received]}
-        assert output_received == "Error while cleaning orphan media files"
-      end
-    end
-  end
-
   defp create_file do
     File.cp!("test/fixtures/picture.png", "test/fixtures/picture_tmp.png")
 

test/tasks/users/clean_unconfirmed_users_test.exs

@@ -117,15 +117,4 @@ defmodule Mix.Tasks.Mobilizon.Media.CleanUnconfirmedUsersTest do
       end
     end
   end
-
-  describe "returns an error" do
-    test "for some reason" do
-      with_mock CleanUnconfirmedUsers,
-        clean: fn [dry_run: false, grace_period: 48] -> {:error, "Some error"} end do
-        Clean.run([])
-        assert_received {:mix_shell, :error, [output_received]}
-        assert output_received == "Error while cleaning unconfirmed users"
-      end
-    end
-  end
 end