diff --git a/lib/federation/activity_pub/actions/reject.ex b/lib/federation/activity_pub/actions/reject.ex index cdc0ae50e..00320ad72 100644 --- a/lib/federation/activity_pub/actions/reject.ex +++ b/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 <- maybe_federate(activity), - :ok <- maybe_relay_if_group_activity(activity) do - {:ok, activity, entity} - else - err -> - Logger.error("Something went wrong while creating an activity") - Logger.debug(inspect(err)) - err - end + {:ok, activity} = create_activity(update_data, local) + maybe_federate(activity) + maybe_relay_if_group_activity(activity) + {:ok, activity, entity} end @spec reject_join(Participant.t(), map()) :: {:ok, Participant.t(), Activity.t()} | any() diff --git a/lib/federation/activity_pub/transmogrifier.ex b/lib/federation/activity_pub/transmogrifier.ex index 128bf4d73..ca14b42d0 100644 --- a/lib/federation/activity_pub/transmogrifier.ex +++ b/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 diff --git a/lib/federation/activity_pub/utils.ex b/lib/federation/activity_pub/utils.ex index 77a3e5f78..9c6f28443 100644 --- a/lib/federation/activity_pub/utils.ex +++ b/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)) diff --git a/lib/federation/activity_stream/converter/discussion.ex b/lib/federation/activity_stream/converter/discussion.ex index 3d611d1db..4989871a2 100644 --- a/lib/federation/activity_stream/converter/discussion.ex +++ b/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 diff --git a/lib/graphql/api/comments.ex b/lib/graphql/api/comments.ex index f8dd17b5b..0c7fabf1a 100644 --- a/lib/graphql/api/comments.ex +++ b/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) diff --git a/lib/graphql/error.ex b/lib/graphql/error.ex index 561567c5b..e412d6465 100644 --- a/lib/graphql/error.ex +++ b/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 diff --git a/lib/graphql/resolvers/comment.ex b/lib/graphql/resolvers/comment.ex index 247138f83..766749f93 100644 --- a/lib/graphql/resolvers/comment.ex +++ b/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] -> diff --git a/lib/graphql/resolvers/resource.ex b/lib/graphql/resolvers/resource.ex index f10f35c84..386eb470e 100644 --- a/lib/graphql/resolvers/resource.ex +++ b/lib/graphql/resolvers/resource.ex @@ -107,28 +107,29 @@ defmodule Mobilizon.GraphQL.Resolvers.Resource do } } = _resolution ) do - with {:member, true} <- {:member, Actors.is_member?(actor_id, group_id)}, - parent <- get_eventual_parent(args), - {:own_check, true} <- {:own_check, check_resource_owned_by_group(parent, group_id)}, - {:ok, _, %Resource{} = resource} <- - Actions.Create.create( - :resource, - args - |> Map.put(:actor_id, group_id) - |> Map.put(:creator_id, actor_id), - true, - %{} - ) do - {:ok, resource} - else - {:error, _} -> - {:error, dgettext("errors", "Error while creating resource")} + if Actors.is_member?(actor_id, group_id) do + parent = get_eventual_parent(args) - {:own_check, _} -> + if check_resource_owned_by_group(parent, group_id) do + case Actions.Create.create( + :resource, + args + |> Map.put(:actor_id, group_id) + |> Map.put(:creator_id, actor_id), + true, + %{} + ) do + {:ok, _, %Resource{} = resource} -> + {:ok, resource} + + {:error, _err} -> + {:error, dgettext("errors", "Error while creating resource")} + end + else {:error, dgettext("errors", "Parent resource doesn't belong to this group")} - - {:member, _} -> - {:error, dgettext("errors", "Profile is not member of group")} + end + else + {:error, dgettext("errors", "Profile is not member of group")} end end diff --git a/lib/graphql/resolvers/tag.ex b/lib/graphql/resolvers/tag.ex index 8257187e1..2fd96608e 100644 --- a/lib/graphql/resolvers/tag.ex +++ b/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 diff --git a/lib/graphql/resolvers/user.ex b/lib/graphql/resolvers/user.ex index d24cfdab3..6300028ca 100644 --- a/lib/graphql/resolvers/user.ex +++ b/lib/graphql/resolvers/user.ex @@ -425,35 +425,39 @@ 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} <- - {:can_change_password, Authenticator.can_change_email?(user)}, - {:current_password, {:ok, %User{}}} <- - {:current_password, Authenticator.login(user.email, password)}, - {:same_email, false} <- {:same_email, new_email == old_email}, - {:email_valid, true} <- {:email_valid, Email.Checker.valid?(new_email)}, - {:ok, %User{} = user} <- Users.update_user_email(user, new_email) do - user - |> Email.User.send_email_reset_old_email() - |> Email.Mailer.send_email_later() + if Authenticator.can_change_email?(user) do + case Authenticator.login(old_email, password) do + {:ok, %User{}} -> + if new_email != old_email do + if Email.Checker.valid?(new_email) do + case Users.update_user_email(user, new_email) do + {:ok, %User{} = user} -> + user + |> Email.User.send_email_reset_old_email() + |> Email.Mailer.send_email_later() - user - |> Email.User.send_email_reset_new_email() - |> Email.Mailer.send_email_later() + user + |> Email.User.send_email_reset_new_email() + |> Email.Mailer.send_email_later() - {:ok, user} + {:ok, user} + + {: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 - {: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")} + {:error, dgettext("errors", "User cannot change email")} end end @@ -635,14 +639,14 @@ defmodule Mobilizon.GraphQL.Resolvers.User do user, context ) do - with current_ip <- Map.get(context, :ip), - now <- DateTime.utc_now() do - Users.update_user(user, %{ - last_sign_in_at: current_sign_in_at || now, - last_sign_in_ip: current_sign_in_ip || current_ip, - current_sign_in_ip: current_ip, - current_sign_in_at: now - }) - end + current_ip = Map.get(context, :ip) + 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, + current_sign_in_ip: current_ip, + current_sign_in_at: now + }) end end diff --git a/lib/mix/tasks/mobilizon/actors/refresh.ex b/lib/mix/tasks/mobilizon/actors/refresh.ex index 9c1b20d06..aafbfe22d 100644 --- a/lib/mix/tasks/mobilizon/actors/refresh.ex +++ b/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) -> - shell_error(err) - - _err -> + {:error, _err} -> shell_error("Error while refreshing actor #{preferred_username}") end end diff --git a/lib/mix/tasks/mobilizon/instance.ex b/lib/mix/tasks/mobilizon/instance.ex index c2529b117..0bbbd0c69 100644 --- a/lib/mix/tasks/mobilizon/instance.ex +++ b/lib/mix/tasks/mobilizon/instance.ex @@ -187,35 +187,29 @@ 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 - :ok -> - :ok + with {:error, err} <- File.write(config_path, result_config) do + 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" + ) - {: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" - ) - - {:error, err} + {:error, err} 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 - :ok -> - :ok + with {:error, err} <- File.write(psql_path, result_psql) do + 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" + ) - {: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" - ) - - {:error, err} + {:error, err} end end end diff --git a/lib/mix/tasks/mobilizon/media/clean_orphan.ex b/lib/mix/tasks/mobilizon/media/clean_orphan.ex index 09ba6a37f..42f954ab9 100644 --- a/lib/mix/tasks/mobilizon/media/clean_orphan.ex +++ b/lib/mix/tasks/mobilizon/media/clean_orphan.ex @@ -34,22 +34,16 @@ defmodule Mix.Tasks.Mobilizon.Media.CleanOrphan do start_mobilizon() - case CleanOrphanMedia.clean(dry_run: dry_run, grace_period: grace_period) do - {:ok, medias} -> - if length(medias) > 0 do - if dry_run or verbose do - details(medias, dry_run, verbose) - end + {:ok, medias} = CleanOrphanMedia.clean(dry_run: dry_run, grace_period: grace_period) - result(dry_run, length(medias)) - else - empty_result(dry_run) - end + if length(medias) > 0 do + if dry_run or verbose do + details(medias, dry_run, verbose) + end - :ok - - _err -> - shell_error("Error while cleaning orphan media files") + result(dry_run, length(medias)) + else + empty_result(dry_run) end end @@ -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") diff --git a/lib/mix/tasks/mobilizon/relay/accept.ex b/lib/mix/tasks/mobilizon/relay/accept.ex index 88d594ff8..e2401324e 100644 --- a/lib/mix/tasks/mobilizon/relay/accept.ex +++ b/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) diff --git a/lib/mix/tasks/mobilizon/users/clean.ex b/lib/mix/tasks/mobilizon/users/clean.ex index 336af5ab6..43af9116e 100644 --- a/lib/mix/tasks/mobilizon/users/clean.ex +++ b/lib/mix/tasks/mobilizon/users/clean.ex @@ -35,23 +35,20 @@ defmodule Mix.Tasks.Mobilizon.Users.Clean do start_mobilizon() - case CleanUnconfirmedUsers.clean(dry_run: dry_run, grace_period: grace_period) do - {:ok, deleted_users} -> - if length(deleted_users) > 0 do - if dry_run or verbose do - details(deleted_users, dry_run, verbose) - end + {:ok, deleted_users} = + CleanUnconfirmedUsers.clean(dry_run: dry_run, grace_period: grace_period) - result(dry_run, length(deleted_users)) - else - empty_result(dry_run) - end + if length(deleted_users) > 0 do + if dry_run or verbose do + details(deleted_users, dry_run, verbose) + end - :ok - - _err -> - shell_error("Error while cleaning unconfirmed users") + result(dry_run, length(deleted_users)) + else + empty_result(dry_run) end + + :ok end @spec details(list(Media.t()), boolean(), boolean()) :: :ok diff --git a/lib/mobilizon/admin/admin.ex b/lib/mobilizon/admin/admin.ex index 24924e481..de267042d 100644 --- a/lib/mobilizon/admin/admin.ex +++ b/lib/mobilizon/admin/admin.ex @@ -48,20 +48,18 @@ 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), - {:role, true} <- {:role, role in [:administrator, :moderator]}, - {:ok, %ActionLog{} = create_action_log} <- - 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} + %User{role: role} = Users.get_user!(user_id) + + 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) + }) else - {:role, false} -> - {:error, :user_not_moderator} + {:error, :user_not_moderator} end end diff --git a/lib/mobilizon/discussions/comment.ex b/lib/mobilizon/discussions/comment.ex index 6ae10d27e..42915189f 100644 --- a/lib/mobilizon/discussions/comment.ex +++ b/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 - def can_be_managed_by(%__MODULE__{actor_id: creator_actor_id}, actor_id) + @spec can_be_managed_by?(t, integer | String.t()) :: boolean() + 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 diff --git a/lib/mobilizon/events/events.ex b/lib/mobilizon/events/events.ex index 25982f68a..a1f9aeef2 100644 --- a/lib/mobilizon/events/events.ex +++ b/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( diff --git a/lib/mobilizon/posts/posts.ex b/lib/mobilizon/posts/posts.ex index 2f4d7897b..716379bf9 100644 --- a/lib/mobilizon/posts/posts.ex +++ b/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, diff --git a/lib/mobilizon/reports/reports.ex b/lib/mobilizon/reports/reports.ex index 419dd3b9d..c9b59f9cc 100644 --- a/lib/mobilizon/reports/reports.ex +++ b/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, diff --git a/lib/mobilizon/storage/repo.ex b/lib/mobilizon/storage/repo.ex index 67796d3d4..545b9e6c0 100644 --- a/lib/mobilizon/storage/repo.ex +++ b/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 diff --git a/lib/mobilizon/users/user.ex b/lib/mobilizon/users/user.ex index c0e50b6a9..cd3a951d7 100644 --- a/lib/mobilizon/users/user.ex +++ b/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_at: DateTime.t(), + current_sign_in_ip: String.t() | nil, + current_sign_in_at: DateTime.t() | nil, activity_settings: [ActivitySetting.t()], settings: Setting.t(), unconfirmed_email: String.t() | nil diff --git a/lib/service/activity/activity.ex b/lib/service/activity/activity.ex index 6287f86b4..9c436fecf 100644 --- a/lib/service/activity/activity.ex +++ b/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()) :: - {:ok, Oban.Job.t()} | {:ok, any()} + @callback insert_activity(entity :: struct(), options :: Keyword.t()) :: + {:ok, Oban.Job.t()} | {:ok, any()} | {:error, Ecto.Changeset.t()} @callback get_object(object_id :: String.t() | integer()) :: struct() diff --git a/lib/service/activity/member.ex b/lib/service/activity/member.ex index 704a55e6e..72d0f55f8 100644 --- a/lib/service/activity/member.ex +++ b/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) diff --git a/lib/service/auth/ldap_authenticator.ex b/lib/service/auth/ldap_authenticator.ex index d4816a30b..6dba56edd 100644 --- a/lib/service/auth/ldap_authenticator.ex +++ b/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 - {:ok, %User{} = user} -> - user - - error -> - error + with {:ok, %User{} = user} <- Users.create_external(email, "ldap") do + user end end diff --git a/lib/service/clean_orphan_media.ex b/lib/service/clean_orphan_media.ex index 27c674aa5..b02d2953f 100644 --- a/lib/service/clean_orphan_media.ex +++ b/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) diff --git a/lib/service/clean_unconfirmed_users.ex b/lib/service/clean_unconfirmed_users.ex index 28c77d39e..c78db8fc9 100644 --- a/lib/service/clean_unconfirmed_users.ex +++ b/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) diff --git a/lib/service/export/feed.ex b/lib/service/export/feed.ex index 6af40bada..106d0a320 100644 --- a/lib/service/export/feed.ex +++ b/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} -> - {:commit, res} - - err -> - {:ignore, err} - end + {:ok, res} = fetch_instance_feed() + {:commit, res} 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} -> - {:ok, build_instance_feed(events, posts)} - - err -> - {:error, err} - end + {:ok, events, posts} = Common.fetch_instance_public_content(@item_limit) + {:ok, build_instance_feed(events, posts)} 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} -> diff --git a/lib/service/geospatial/addok.ex b/lib/service/geospatial/addok.ex index bf3d1f253..d9c196dbf 100644 --- a/lib/service/geospatial/addok.ex +++ b/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) :: - String.t() - defp do_add_parameter(url, :coords, coords), - do: "#{url}&lat=#{coords.lat}&lon=#{coords.lon}" - + @spec do_add_parameter(String.t(), :type, :administrative | atom()) :: String.t() defp do_add_parameter(url, :type, :administrative), do: "#{url}&type=municipality" diff --git a/lib/service/geospatial/google_maps.ex b/lib/service/geospatial/google_maps.ex index 0b50c5991..936e000f5 100644 --- a/lib/service/geospatial/google_maps.ex +++ b/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), - %{"results" => results, "status" => "OK"} <- body do - Enum.map(results, fn entry -> process_data(entry, options) end) - else + %Tesla.Env{status: 200, body: body} = GeospatialClient.get!(url) + + case body do + %{"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), - %{"results" => results, "status" => "OK"} <- body do - results |> Enum.map(fn entry -> process_data(entry, options) end) - else + %Tesla.Env{status: 200, body: body} = GeospatialClient.get!(url) + + case body do + %{"results" => results, "status" => "OK"} -> + results |> Enum.map(fn entry -> process_data(entry, options) end) + %{"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), - %{"result" => %{"name" => name}, "status" => "OK"} <- body do - name - else + %Tesla.Env{status: 200, body: body} = GeospatialClient.get!(url) + + case body do + %{"result" => %{"name" => name}, "status" => "OK"} -> + name + %{"status" => "REQUEST_DENIED", "error_message" => error_message} -> raise ArgumentError, message: to_string(error_message) diff --git a/lib/service/geospatial/provider.ex b/lib/service/geospatial/provider.ex index 47d92f7ba..8c20712c6 100644 --- a/lib/service/geospatial/provider.ex +++ b/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 - def coordinates(coords, srid \\ 4326) - - def coordinates([x, y], srid) when is_number(x) and is_number(y) do - %Geo.Point{coordinates: {x, y}, srid: srid} + @spec coordinates([number | String.t()]) :: Geo.Point.t() | nil + def coordinates([x, y]) when is_number(x) and is_number(y) do + %Geo.Point{coordinates: {x, y}, srid: 4326} end - def coordinates([x, y], srid) when is_binary(x) and is_binary(y) do - %Geo.Point{coordinates: {String.to_float(x), String.to_float(y)}, srid: srid} + 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: 4326} end - def coordinates(_, _), do: nil + def coordinates(_), do: nil @spec endpoint(atom()) :: String.t() def endpoint(provider) do diff --git a/mix.exs b/mix.exs index c4693fc4e..127b768d3 100644 --- a/mix.exs +++ b/mix.exs @@ -9,7 +9,7 @@ defmodule Mobilizon.Mixfile do version: @version, elixir: "~> 1.11", elixirc_paths: elixirc_paths(Mix.env()), - compilers: [:gettext, :unused] ++ Mix.compilers(), + compilers: [:gettext] ++ Mix.compilers(), xref: [exclude: [:eldap]], start_permanent: Mix.env() == :prod, aliases: aliases(), @@ -221,8 +221,7 @@ defmodule Mobilizon.Mixfile do {:mox, "~> 1.0", only: :test}, {:junit_formatter, "~> 3.1", only: [:test]}, {:sobelow, "~> 0.8", only: [:dev, :test]}, - {:doctor, "~> 0.18.0", only: :dev}, - {:mix_unused, "~> 0.2.0"} + {:doctor, "~> 0.18.0", only: :dev} ] ++ oauth_deps() end diff --git a/mix.lock b/mix.lock index 6b3969f44..e0792c07b 100644 --- a/mix.lock +++ b/mix.lock @@ -19,7 +19,6 @@ "cowboy_telemetry": {:hex, :cowboy_telemetry, "0.4.0", "f239f68b588efa7707abce16a84d0d2acf3a0f50571f8bb7f56a15865aae820c", [:rebar3], [{:cowboy, "~> 2.7", [hex: :cowboy, repo: "hexpm", optional: false]}, {:telemetry, "~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "7d98bac1ee4565d31b62d59f8823dfd8356a169e7fcbb83831b8a5397404c9de"}, "cowlib": {:hex, :cowlib, "2.11.0", "0b9ff9c346629256c42ebe1eeb769a83c6cb771a6ee5960bd110ab0b9b872063", [:make, :rebar3], [], "hexpm", "2b3e9da0b21c4565751a6d4901c20d1b4cc25cbb7fd50d91d2ab6dd287bc86a9"}, "credo": {:hex, :credo, "1.5.6", "e04cc0fdc236fefbb578e0c04bd01a471081616e741d386909e527ac146016c6", [:mix], [{:bunt, "~> 0.2.0", [hex: :bunt, repo: "hexpm", optional: false]}, {:file_system, "~> 0.2.8", [hex: :file_system, repo: "hexpm", optional: false]}, {:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: false]}], "hexpm", "4b52a3e558bd64e30de62a648518a5ea2b6e3e5d2b164ef5296244753fc7eb17"}, - "csv": {:hex, :csv, "2.4.1", "50e32749953b6bf9818dbfed81cf1190e38cdf24f95891303108087486c5925e", [:mix], [{:parallel_stream, "~> 1.0.4", [hex: :parallel_stream, repo: "hexpm", optional: false]}], "hexpm", "54508938ac67e27966b10ef49606e3ad5995d665d7fc2688efb3eab1307c9079"}, "dataloader": {:hex, :dataloader, "1.0.7", "58351b335673cf40601429bfed6c11fece6ce7ad169b2ac0f0fe83e716587391", [:mix], [{:ecto, ">= 0.0.0", [hex: :ecto, repo: "hexpm", optional: true]}], "hexpm", "12bf66478e4a5085d09dc96932d058c206ee8c219cc7691d12a40dc35c8cefaa"}, "db_connection": {:hex, :db_connection, "2.4.0", "d04b1b73795dae60cead94189f1b8a51cc9e1f911c234cc23074017c43c031e5", [:mix], [{:connection, "~> 1.0", [hex: :connection, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "ad416c21ad9f61b3103d254a71b63696ecadb6a917b36f563921e0de00d7d7c8"}, "decimal": {:hex, :decimal, "2.0.0", "a78296e617b0f5dd4c6caf57c714431347912ffb1d0842e998e9792b5642d697", [:mix], [], "hexpm", "34666e9c55dea81013e77d9d87370fe6cb6291d1ef32f46a1600230b1d44f577"}, @@ -94,7 +93,6 @@ "mimerl": {:hex, :mimerl, "1.2.0", "67e2d3f571088d5cfd3e550c383094b47159f3eee8ffa08e64106cdf5e981be3", [:rebar3], [], "hexpm", "f278585650aa581986264638ebf698f8bb19df297f66ad91b18910dfc6e19323"}, "mimetype_parser": {:hex, :mimetype_parser, "0.1.3", "628ac9fe56aa7edcedb534d68397dd66674ab82493c8ebe39acb9a19b666099d", [:mix], [], "hexpm", "7d8f80c567807ce78cd93c938e7f4b0a20b1aaaaab914bf286f68457d9f7a852"}, "mix_test_watch": {:hex, :mix_test_watch, "1.1.0", "330bb91c8ed271fe408c42d07e0773340a7938d8a0d281d57a14243eae9dc8c3", [:mix], [{:file_system, "~> 0.2.1 or ~> 0.3", [hex: :file_system, repo: "hexpm", optional: false]}], "hexpm", "52b6b1c476cbb70fd899ca5394506482f12e5f6b0d6acff9df95c7f1e0812ec3"}, - "mix_unused": {:hex, :mix_unused, "0.2.0", "430d011f7c86eec8e4824c66fb69127f192ca3b2b2dd0e0f8f58bf535d319167", [:mix], [], "hexpm", "f3eb53bf416d89be1db801865b3f2a6c8c084e138ad290c82b5136a31e5622ad"}, "mmdb2_decoder": {:hex, :mmdb2_decoder, "3.0.0", "54828676a36e75e9a25bc9a0bb0598d4c7fcc767bf0b40674850b22e05b7b6cc", [:mix], [], "hexpm", "359dc9242915538d1dceb9f6d96c72201dca76ce62e49d22e2ed1e86f20bea8e"}, "mock": {:hex, :mock, "0.3.7", "75b3bbf1466d7e486ea2052a73c6e062c6256fb429d6797999ab02fa32f29e03", [:mix], [{:meck, "~> 0.9.2", [hex: :meck, repo: "hexpm", optional: false]}], "hexpm", "4da49a4609e41fd99b7836945c26f373623ea968cfb6282742bcb94440cf7e5c"}, "mogrify": {:hex, :mogrify, "0.9.1", "a26f107c4987477769f272bd0f7e3ac4b7b75b11ba597fd001b877beffa9c068", [:mix], [], "hexpm", "134edf189337d2125c0948bf0c228fdeef975c594317452d536224069a5b7f05"}, @@ -106,7 +104,6 @@ "oauther": {:hex, :oauther, "1.3.0", "82b399607f0ca9d01c640438b34d74ebd9e4acd716508f868e864537ecdb1f76", [:mix], [], "hexpm", "78eb888ea875c72ca27b0864a6f550bc6ee84f2eeca37b093d3d833fbcaec04e"}, "oban": {:hex, :oban, "2.9.2", "5504c1c28d0b04e326c1075bd5f0f9c0fbe93850f581d9b201e2e2ad86ef8cc8", [:mix], [{:ecto_sql, ">= 3.4.3", [hex: :ecto_sql, repo: "hexpm", optional: false]}, {:jason, "~> 1.1", [hex: :jason, repo: "hexpm", optional: false]}, {:postgrex, "~> 0.14", [hex: :postgrex, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "f05a042c6611c013a21717dd78cc1e64690b6b66885e6e237c3a2af1b9e9cff8"}, "paasaa": {:hex, :paasaa, "0.5.1", "58d8bf61902adfd1d04815a115f0eb3b996845c0360f1831854e21073411e822", [:mix], [], "hexpm", "571f1a33b8e184396a93fc18ee5331f2655c96ba9a6fc383dc675e4bc8fc7596"}, - "parallel_stream": {:hex, :parallel_stream, "1.0.6", "b967be2b23f0f6787fab7ed681b4c45a215a81481fb62b01a5b750fa8f30f76c", [:mix], [], "hexpm", "639b2e8749e11b87b9eb42f2ad325d161c170b39b288ac8d04c4f31f8f0823eb"}, "parse_trans": {:hex, :parse_trans, "3.3.1", "16328ab840cc09919bd10dab29e431da3af9e9e7e7e6f0089dd5a2d2820011d8", [:rebar3], [], "hexpm", "07cd9577885f56362d414e8c4c4e6bdf10d43a8767abb92d24cbe8b24c54888b"}, "phoenix": {:hex, :phoenix, "1.6.0", "7b85023f7ddef9a5c70909a51cc37c8b868b474d853f90f4280efd26b0e7cce5", [:mix], [{:jason, "~> 1.0", [hex: :jason, repo: "hexpm", optional: true]}, {:phoenix_pubsub, "~> 2.0", [hex: :phoenix_pubsub, repo: "hexpm", optional: false]}, {:phoenix_view, "~> 1.0", [hex: :phoenix_view, repo: "hexpm", optional: false]}, {:plug, "~> 1.10", [hex: :plug, repo: "hexpm", optional: false]}, {:plug_cowboy, "~> 2.2", [hex: :plug_cowboy, repo: "hexpm", optional: true]}, {:plug_crypto, "~> 1.2", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4 or ~> 1.0", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "52ffdd31f2daeb399b2e1eb57d468f99a1ad6eee5d8ea19d2353492f06c9fc96"}, "phoenix_ecto": {:hex, :phoenix_ecto, "4.4.0", "0672ed4e4808b3fbed494dded89958e22fb882de47a97634c0b13e7b0b5f7720", [:mix], [{:ecto, "~> 3.3", [hex: :ecto, repo: "hexpm", optional: false]}, {:phoenix_html, "~> 2.14.2 or ~> 3.0", [hex: :phoenix_html, repo: "hexpm", optional: true]}, {:plug, "~> 1.9", [hex: :plug, repo: "hexpm", optional: false]}], "hexpm", "09864e558ed31ee00bd48fcc1d4fc58ae9678c9e81649075431e69dbabb43cc1"}, diff --git a/test/tasks/media/clean_orphan_test.exs b/test/tasks/media/clean_orphan_test.exs index 83440dcf2..073dc1ded 100644 --- a/test/tasks/media/clean_orphan_test.exs +++ b/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") diff --git a/test/tasks/users/clean_unconfirmed_users_test.exs b/test/tasks/users/clean_unconfirmed_users_test.exs index 10d56c573..165ac8709 100644 --- a/test/tasks/users/clean_unconfirmed_users_test.exs +++ b/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