Merge branch 'fixes' into 'master'

Couple of fixes

See merge request framasoft/mobilizon!957
This commit is contained in:
Thomas Citharel 2021-06-27 14:00:17 +00:00
commit 073c84b699
25 changed files with 276 additions and 93 deletions

View File

@ -4,3 +4,6 @@
752C0E897CA81ACD81F4BB215FA5F8E4
23412CF16549E4E88366DC9DECF39071
81C1F600C5809C7029EE32DE4818CD7D
155A1FB53DE39EC8EFCFD7FB94EA823D
73B351E4CB3AF715AD450A085F5E6304
BBACD7F0BACD4A6D3010C26604671692

View File

@ -113,7 +113,7 @@ config :mobilizon, Mobilizon.Web.Email.Mailer,
# Configures Elixir's Logger
config :logger, :console,
backends: [:console, Sentry.LoggerBackend],
backends: [:console],
format: "$time $metadata[$level] $message\n",
metadata: [:request_id]

View File

@ -9,6 +9,7 @@ defmodule Mobilizon.Federation.ActivityPub.Fetcher do
alias Mobilizon.Federation.HTTPSignatures.Signature
alias Mobilizon.Federation.ActivityPub.{Relay, Transmogrifier}
alias Mobilizon.Federation.ActivityStream.Converter.Actor, as: ActorConverter
alias Mobilizon.Service.ErrorReporting.Sentry
alias Mobilizon.Service.HTTP.ActivityPub, as: ActivityPubClient
import Mobilizon.Federation.ActivityPub.Utils,

View File

@ -8,6 +8,7 @@ defmodule Mobilizon.Federation.ActivityPub.Refresher do
alias Mobilizon.Federation.ActivityPub
alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
alias Mobilizon.Federation.ActivityPub.{Fetcher, Relay, Transmogrifier, Utils}
alias Mobilizon.Service.ErrorReporting.Sentry
require Logger
@doc """

View File

@ -21,6 +21,7 @@ defmodule Mobilizon.Federation.ActivityPub.Transmogrifier do
alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
alias Mobilizon.Federation.ActivityPub.Types.Ownable
alias Mobilizon.Federation.ActivityStream.{Converter, Convertible}
alias Mobilizon.Service.ErrorReporting.Sentry
alias Mobilizon.Service.Workers.Background
alias Mobilizon.Tombstone
alias Mobilizon.Web.Email.Participation

View File

@ -13,6 +13,7 @@ defmodule Mobilizon.Federation.HTTPSignatures.Signature do
alias Mobilizon.Actors.Actor
alias Mobilizon.Federation.ActivityPub.Actor, as: ActivityPubActor
alias Mobilizon.Service.ErrorReporting.Sentry
require Logger

View File

@ -16,7 +16,7 @@ defmodule Mobilizon do
alias Mobilizon.{Config, Storage, Web}
alias Mobilizon.Federation.ActivityPub
alias Mobilizon.Service.{ErrorPage, ErrorReporter}
alias Mobilizon.Service.{ErrorPage, ErrorReporting}
alias Mobilizon.Service.Export.{Feed, ICalendar}
@name Mix.Project.config()[:name]
@ -66,13 +66,14 @@ defmodule Mobilizon do
] ++
task_children(@env)
Logger.add_backend(Sentry.LoggerBackend)
ErrorReporting.configure()
:ok = Oban.Telemetry.attach_default_logger()
:telemetry.attach_many(
"oban-errors",
[[:oban, :job, :exception], [:oban, :circuit, :trip]],
&ErrorReporter.handle_event/4,
&ErrorReporting.handle_event/4,
%{}
)

View File

@ -16,6 +16,7 @@ defmodule Mobilizon.Actors do
alias Mobilizon.Events.FeedToken
alias Mobilizon.Federation.ActivityPub
alias Mobilizon.Medias.File
alias Mobilizon.Service.ErrorReporting.Sentry
alias Mobilizon.Service.Workers
alias Mobilizon.Storage.{Page, Repo}
alias Mobilizon.Users
@ -360,6 +361,9 @@ defmodule Mobilizon.Actors do
Multi.run(multi, :reset_default_actor_id, fn _, _ ->
reset_default_actor_id(actor)
end)
_ ->
multi
end
multi =

View File

@ -214,6 +214,17 @@ defmodule Mobilizon.Events do
|> Repo.exists?()
end
@doc """
Gets an event by its UUID, with all associations loaded.
"""
@spec get_event_by_uuid_with_preload(String.t()) :: Event.t() | nil
def get_event_by_uuid_with_preload(uuid) do
uuid
|> event_by_uuid_query()
|> preload_for_event()
|> Repo.one()
end
@doc """
Gets an event by its UUID, with all associations loaded.
"""

View File

@ -159,10 +159,11 @@ defmodule Mobilizon.Service.Activity.Comment do
%Comment{
actor_id: actor_id,
in_reply_to_comment_id: in_reply_to_comment_id,
id: comment_id
},
id: comment_id,
uuid: comment_uuid
} = comment,
%Event{
uuid: uuid,
uuid: event_uuid,
title: title,
attributed_to: nil,
organizer_actor_id: organizer_actor_id
@ -174,8 +175,10 @@ defmodule Mobilizon.Service.Activity.Comment do
"subject" => :event_new_comment,
"subject_params" => %{
event_title: title,
event_uuid: uuid,
comment_reply_to: !is_nil(in_reply_to_comment_id)
event_uuid: event_uuid,
comment_reply_to: !is_nil(in_reply_to_comment_id),
comment_uuid: comment_uuid,
comment_reply_to_uuid: reply_to_comment_uuid(comment)
},
"author_id" => actor_id,
"object_id" => to_string(comment_id)
@ -185,4 +188,10 @@ defmodule Mobilizon.Service.Activity.Comment do
end
defp notify(_, _, _, _), do: {:ok, :skipped}
@spec reply_to_comment_uuid(Comment.t()) :: String.t() | nil
defp reply_to_comment_uuid(%Comment{in_reply_to_comment: %Comment{uuid: comment_reply_to_uuid}}),
do: comment_reply_to_uuid
defp reply_to_comment_uuid(%Comment{in_reply_to_comment: _}), do: nil
end

View File

@ -45,6 +45,35 @@ defmodule Mobilizon.Service.Activity.Renderer.Comment do
),
url: event_url(activity)
}
:event_new_comment ->
if activity.subject_params["comment_reply_to"] do
%{
body:
dgettext(
"activity",
"%{profile} has posted a new reply under your event %{event}.",
%{
profile: profile,
event: event_title(activity)
}
),
url: event_comment_url(activity)
}
else
%{
body:
dgettext(
"activity",
"%{profile} has posted a new comment under your event %{event}.",
%{
profile: profile,
event: event_title(activity)
}
),
url: event_comment_url(activity)
}
end
end
end
@ -56,6 +85,18 @@ defmodule Mobilizon.Service.Activity.Renderer.Comment do
)
end
defp event_comment_url(activity) do
"#{event_url(activity)}#comment-#{comment_uuid(activity)}"
end
defp comment_uuid(activity) do
if activity.subject_params["comment_reply_to"] do
"#{activity.subject_params["reply_to_comment_uuid"]}-#{activity.subject_params["comment_uuid"]}"
else
activity.subject_params["comment_uuid"]
end
end
defp profile(activity), do: Actor.display_name_and_username(activity.author)
defp event_title(activity), do: activity.subject_params["event_title"]
end

View File

@ -1,17 +0,0 @@
defmodule Mobilizon.Service.ErrorReporter do
@moduledoc """
Module to delegate all exceptions to Sentry
"""
def handle_event([:oban, :job, :exception], measure, %{job: job} = meta, _) do
extra =
job
|> Map.take([:id, :args, :meta, :queue, :worker])
|> Map.merge(measure)
Sentry.capture_exception(meta.error, stacktrace: meta.stacktrace, extra: extra)
end
def handle_event([:oban, :circuit, :trip], _measure, meta, _) do
Sentry.capture_exception(meta.error, stacktrace: meta.stacktrace, extra: meta)
end
end

View File

@ -0,0 +1,35 @@
defmodule Mobilizon.Service.ErrorReporting do
@moduledoc """
Mpdule to load and configure error reporting adapters
"""
@callback enabled? :: boolean()
@callback configure :: any()
@callback handle_event(list(atom()), map(), map(), any()) :: any()
@spec adapter :: module() | nil
def adapter do
adapter = Mobilizon.Config.get([__MODULE__, :adapter])
if adapter && adapter.enabled?(), do: adapter, else: nil
end
@spec handle_event(list(atom()), map(), map(), any()) :: any()
def handle_event(event_name, event_measurements, event_metadata, handler_config) do
adapter = adapter()
if adapter do
adapter.handle_event(event_name, event_measurements, event_metadata, handler_config)
end
end
@spec configure :: any()
def configure do
adapter = adapter()
if adapter do
adapter.configure()
end
end
end

View File

@ -0,0 +1,45 @@
defmodule Mobilizon.Service.ErrorReporting.Sentry do
@moduledoc """
Sentry adapter for error reporting
"""
alias Mobilizon.Service.ErrorReporting
@behaviour ErrorReporting
@impl ErrorReporting
def enabled? do
!is_nil(Application.get_env(:sentry, :dsn))
end
@impl ErrorReporting
def configure do
Logger.add_backend(Sentry.LoggerBackend)
end
def capture_message(message, opts \\ []) when is_binary(message) do
if enabled?() do
Sentry.capture_message(message, opts)
end
end
def capture_exception(exception, opts \\ []) do
if enabled?() do
Sentry.capture_exception(exception, opts)
end
end
@impl ErrorReporting
def handle_event([:oban, :job, :exception], measure, %{job: job} = meta, _) do
extra =
job
|> Map.take([:id, :args, :meta, :queue, :worker])
|> Map.merge(measure)
Sentry.capture_exception(meta.error, stacktrace: meta.stacktrace, extra: extra)
end
@impl ErrorReporting
def handle_event([:oban, :circuit, :trip], _measure, meta, _) do
Sentry.capture_exception(meta.error, stacktrace: meta.stacktrace, extra: meta)
end
end

View File

@ -56,7 +56,8 @@ defmodule Mobilizon.Service.Notifier.Email do
@always_direct_subjects [
:participation_event_comment,
:event_comment_mention,
:discussion_mention
:discussion_mention,
:event_new_comment
]
@spec can_send_activity?(Activity.t(), User.t(), Keyword.t()) :: boolean()
@ -96,30 +97,24 @@ defmodule Mobilizon.Service.Notifier.Email do
when subject in @always_direct_subjects,
do: true
defp match_group_notifications_setting(
group_notifications,
_subject,
last_notification_sent,
options
) do
cond do
# This is a recap
Keyword.get(options, :recap, false) != false ->
true
# First notification EVER!
group_notifications == :one_hour && is_nil(last_notification_sent) ->
true
defp match_group_notifications_setting(:one_hour, _, last_notification_sent, _)
when is_nil(last_notification_sent),
do: true
# Delay ok since last notification
group_notifications == :one_hour &&
is_delay_ok_since_last_notification_sent(last_notification_sent) ->
true
# Otherwise, no thanks
true ->
false
defp match_group_notifications_setting(:one_hour, _, %DateTime{} = last_notification_sent, _) do
is_delay_ok_since_last_notification_sent(last_notification_sent)
end
# This is a recap
defp match_group_notifications_setting(
_group_notifications,
_subject,
_last_notification_sent,
options
) do
Keyword.get(options, :recap, false) != false
end
@default_behavior %{

View File

@ -23,10 +23,13 @@ defmodule Mobilizon.Service.Notifier.Filter do
defp enabled?(nil, activity_setting, get_default), do: get_default.(activity_setting)
defp enabled?(%ActivitySetting{enabled: enabled}, _activity_setting, _get_default), do: enabled
# Comment mention
# Mention
defp map_activity_to_activity_setting(%Activity{subject: :event_comment_mention}),
do: "event_comment_mention"
defp map_activity_to_activity_setting(%Activity{subject: :discussion_mention}),
do: "discussion_mention"
# Participation
@spec map_activity_to_activity_setting(Activity.t()) :: String.t() | false
defp map_activity_to_activity_setting(%Activity{subject: :participation_event_updated}),
@ -42,6 +45,9 @@ defmodule Mobilizon.Service.Notifier.Filter do
defp map_activity_to_activity_setting(%Activity{subject: :event_new_participation}),
do: "event_new_participation"
defp map_activity_to_activity_setting(%Activity{subject: :event_new_comment}),
do: "event_new_comment"
# Event
defp map_activity_to_activity_setting(%Activity{subject: :event_created}), do: "event_created"
defp map_activity_to_activity_setting(%Activity{type: :event}), do: "event_updated"
@ -60,7 +66,7 @@ defmodule Mobilizon.Service.Notifier.Filter do
defp map_activity_to_activity_setting(%Activity{subject: :member_request}),
do: "member_request"
defp map_activity_to_activity_setting(%Activity{type: :member}), do: "member"
defp map_activity_to_activity_setting(%Activity{type: :member}), do: "member_updated"
defp map_activity_to_activity_setting(_), do: false
end

View File

@ -5,6 +5,8 @@ defmodule Mobilizon.Service.Workers.LegacyNotifierBuilder do
alias Mobilizon.Activities.Activity
alias Mobilizon.{Actors, Events, Users}
alias Mobilizon.Actors.Actor
alias Mobilizon.Events.Event
alias Mobilizon.Service.Notifier
use Mobilizon.Service.Workers.Helper, queue: "activity"
@ -66,6 +68,18 @@ defmodule Mobilizon.Service.Workers.LegacyNotifierBuilder do
|> users_from_actor_ids(Keyword.fetch!(options, :author_id))
end
defp users_to_notify(
%{"subject" => "event_new_comment", "subject_params" => %{"event_uuid" => event_uuid}},
options
) do
event_uuid
|> Events.get_event_by_uuid_with_preload()
|> (fn %Event{organizer_actor: %Actor{id: actor_id}} -> [actor_id] end).()
|> users_from_actor_ids(Keyword.fetch!(options, :author_id))
end
defp users_to_notify(_, _), do: []
@spec users_from_actor_ids(list(), integer() | String.t()) :: list(Users.t())
defp users_from_actor_ids(actor_ids, author_id) do
actor_ids

View File

@ -96,7 +96,7 @@ defmodule Mobilizon.Service.Workers.SendActivityRecapWorker do
timezone: timezone
}
}) do
is_between_hours(timezone: timezone)
is_between_hours(timezone: timezone || "Etc/UTC")
end
# If we're on the first day of the week between notification hours
@ -107,6 +107,6 @@ defmodule Mobilizon.Service.Workers.SendActivityRecapWorker do
timezone: timezone
}
}) do
is_between_hours_on_first_day(timezone: timezone, locale: locale)
is_between_hours_on_first_day(timezone: timezone || "Etc/UTC", locale: locale)
end
end

View File

@ -6,6 +6,7 @@ defmodule Mobilizon.Web.Auth.Context do
import Plug.Conn
alias Mobilizon.Service.ErrorReporting.Sentry, as: SentryAdapter
alias Mobilizon.Users.User
def init(opts) do
@ -24,7 +25,10 @@ defmodule Mobilizon.Web.Auth.Context do
context =
case Guardian.Plug.current_resource(conn) do
%User{id: user_id, email: user_email} = user ->
if SentryAdapter.enabled?() do
Sentry.Context.set_user_context(%{id: user_id, name: user_email})
end
Map.put(context, :current_user, user)
nil ->

View File

@ -3,6 +3,7 @@ defmodule Mobilizon.Web.Email.Mailer do
Mobilizon Mailer.
"""
use Bamboo.Mailer, otp_app: :mobilizon
alias Mobilizon.Service.ErrorReporting.Sentry
def send_email_later(email) do
Mobilizon.Web.Email.Mailer.deliver_later!(email)

View File

@ -2,7 +2,10 @@ defmodule Mobilizon.Web.Endpoint do
@moduledoc """
Endpoint for Mobilizon app
"""
if Application.fetch_env!(:mobilizon, :env) !== :test do
alias Mobilizon.Service.ErrorReporting.Sentry, as: SentryAdapter
if Application.fetch_env!(:mobilizon, :env) !== :test &&
SentryAdapter.enabled?() do
use Sentry.PlugCapture
end
@ -92,7 +95,8 @@ defmodule Mobilizon.Web.Endpoint do
String.replace_leading(url(), "http", "ws")
end
if Application.fetch_env!(:mobilizon, :env) !== :test do
if Application.fetch_env!(:mobilizon, :env) !== :test &&
SentryAdapter.enabled?() do
plug(Sentry.PlugContext)
end
end

View File

@ -37,7 +37,8 @@ defmodule Mobilizon.Web.Router do
plug(:put_secure_browser_headers)
plug(Cldr.Plug.AcceptLanguage,
cldr_backend: Mobilizon.Cldr
cldr_backend: Mobilizon.Cldr,
no_match_log_level: :debug
)
end
@ -50,7 +51,8 @@ defmodule Mobilizon.Web.Router do
plug(Plug.Static, at: "/", from: "priv/static")
plug(Cldr.Plug.AcceptLanguage,
cldr_backend: Mobilizon.Cldr
cldr_backend: Mobilizon.Cldr,
no_match_log_level: :debug
)
plug(:accepts, ["html"])

View File

@ -29,4 +29,37 @@
</a>"
}
) |> raw %>
<% :event_new_comment -> %>
<%= if @activity.subject_params["comment_reply_to"] do %>
<%=
dgettext("activity", "%{profile} has posted a new reply under your event %{event}.",
%{
profile: "<b>#{Mobilizon.Actors.Actor.display_name_and_username(@activity.author)}</b>",
event: "<a href=\"#{
page_url(
Mobilizon.Web.Endpoint,
:event,
@activity.subject_params["event_uuid"]
) |> URI.decode()}#comment-#{@activity.subject_params["comment_reply_to_uuid"]}-#{@activity.subject_params["comment_uuid"]}\">
#{@activity.subject_params["event_title"]}
</a>"
}
) |> raw %>
<% else %>
<%=
dgettext("activity", "%{profile} has posted a new comment under your event %{event}.",
%{
profile: "<b>#{Mobilizon.Actors.Actor.display_name_and_username(@activity.author)}</b>",
event: "<a href=\"#{
page_url(
Mobilizon.Web.Endpoint,
:event,
@activity.subject_params["event_uuid"]
) |> URI.decode()}#comment-#{@activity.subject_params["comment_uuid"]}\">
#{@activity.subject_params["event_title"]}
</a>"
}
) |> raw %>
<% end %>
<% end %>

View File

@ -1,34 +1,4 @@
<%= case @activity.subject do %><% :discussion_created -> %><%= dgettext("activity", "%{profile} created the discussion %{discussion}.",
%{
profile: Mobilizon.Actors.Actor.display_name_and_username(@activity.author),
discussion: @activity.subject_params["discussion_title"]
}
) %>
<%= page_url(Mobilizon.Web.Endpoint, :discussion, Mobilizon.Actors.Actor.preferred_username_and_domain(@activity.group), @activity.subject_params["discussion_slug"]) |> URI.decode() %><% :discussion_replied -> %><%= dgettext("activity", "%{profile} replied to the discussion %{discussion}.",
%{
profile: Mobilizon.Actors.Actor.display_name_and_username(@activity.author),
discussion: @activity.subject_params["discussion_title"]
}
) %>
<%= page_url(Mobilizon.Web.Endpoint, :discussion, Mobilizon.Actors.Actor.preferred_username_and_domain(@activity.group), @activity.subject_params["discussion_slug"]) |> URI.decode() %><% :discussion_renamed -> %><%= dgettext("activity", "%{profile} renamed the discussion %{discussion}.",
%{
profile: Mobilizon.Actors.Actor.display_name_and_username(@activity.author),
discussion: @activity.subject_params["discussion_title"]
}
) %>
<%= page_url(Mobilizon.Web.Endpoint, :discussion, Mobilizon.Actors.Actor.preferred_username_and_domain(@activity.group), @activity.subject_params["discussion_slug"]) |> URI.decode() %><% :discussion_archived -> %><%= dgettext("activity", "%{profile} archived the discussion %{discussion}.",
%{
profile: Mobilizon.Actors.Actor.display_name_and_username(@activity.author),
discussion: @activity.subject_params["discussion_title"]
}
) %>
<%= page_url(Mobilizon.Web.Endpoint, :discussion, Mobilizon.Actors.Actor.preferred_username_and_domain(@activity.group), @activity.subject_params["discussion_slug"]) |> URI.decode() %><% :discussion_deleted -> %><%= dgettext("activity", "%{profile} deleted the discussion %{discussion}.",
%{
profile: Mobilizon.Actors.Actor.display_name_and_username(@activity.author),
discussion: @activity.subject_params["discussion_title"]
}
) %>
<%= page_url(Mobilizon.Web.Endpoint, :discussion, Mobilizon.Actors.Actor.preferred_username_and_domain(@activity.group), @activity.subject_params["discussion_slug"]) |> URI.decode() %><% :event_comment_mention -> %><%= dgettext("activity", "%{profile} mentionned you in a comment under event %{event}.",
<%= case @activity.subject do %><% :event_comment_mention -> %><%= dgettext("activity", "%{profile} mentionned you in a comment under event %{event}.",
%{
profile: Mobilizon.Actors.Actor.display_name_and_username(@activity.author),
event: @activity.subject_params["event_title"]
@ -40,4 +10,16 @@
event: @activity.subject_params["event_title"]
}
) %>
<%= page_url(Mobilizon.Web.Endpoint, :event, @activity.subject_params["event_uuid"]) |> URI.decode() %><% end %>
<%= page_url(Mobilizon.Web.Endpoint, :event, @activity.subject_params["event_uuid"]) |> URI.decode() %><% :event_new_comment -> %><%= if @activity.subject_params["comment_reply_to"] do %><%=dgettext("activity", "%{profile} has posted a new reply under your event %{event}.",
%{
profile: Mobilizon.Actors.Actor.display_name_and_username(@activity.author),
event: @activity.subject_params["event_title"]
}
) %>
<%= "#{page_url(Mobilizon.Web.Endpoint, :event, @activity.subject_params["event_uuid"]) |> URI.decode()}#comment-#{@activity.subject_params["comment_reply_to_uuid"]}-#{@activity.subject_params["comment_uuid"]}" %><% else %><%= dgettext("activity", "%{profile} has posted a new comment under your event %{event}.",
%{
profile: Mobilizon.Actors.Actor.display_name_and_username(@activity.author),
event: @activity.subject_params["event_title"]
}
) %>
<%= "#{page_url(Mobilizon.Web.Endpoint, :event, @activity.subject_params["event_uuid"]) |> URI.decode()}#comment-#{@activity.subject_params["comment_uuid"]}"%><% end %><% end %>

View File

@ -18,7 +18,9 @@ defmodule Mobilizon.Service.Activity.CommentTest do
describe "handle comment with mentions" do
test "with no mentions" do
%Event{title: event_title, uuid: event_uuid} = event = insert(:event)
%Comment{id: comment_id, actor_id: author_id} = comment = insert(:comment, event: event)
%Comment{id: comment_id, actor_id: author_id, uuid: comment_uuid} =
comment = insert(:comment, event: event)
assert {:ok, [organizer: :enqueued, announcement: :skipped, mentionned: :skipped]} ==
CommentActivity.insert_activity(comment)
@ -37,9 +39,11 @@ defmodule Mobilizon.Service.Activity.CommentTest do
"op" => "legacy_notify",
"subject" => "event_new_comment",
"subject_params" => %{
"comment_reply_to_uuid" => nil,
"event_title" => event_title,
"event_uuid" => event_uuid,
"comment_reply_to" => false
"comment_reply_to" => false,
"comment_uuid" => comment_uuid
},
"type" => "comment"
}
@ -51,7 +55,7 @@ defmodule Mobilizon.Service.Activity.CommentTest do
%Actor{id: actor_id} = actor = insert(:actor, user: user)
%Event{uuid: event_uuid, title: event_title} = event = insert(:event)
%Comment{id: comment_id, actor_id: author_id} =
%Comment{id: comment_id, actor_id: author_id, uuid: comment_uuid} =
comment = insert(:comment, text: "Hey @you", event: event)
comment = %Comment{
@ -90,9 +94,11 @@ defmodule Mobilizon.Service.Activity.CommentTest do
"op" => "legacy_notify",
"subject" => "event_new_comment",
"subject_params" => %{
"comment_reply_to_uuid" => nil,
"event_title" => event_title,
"event_uuid" => event_uuid,
"comment_reply_to" => false
"comment_reply_to" => false,
"comment_uuid" => comment_uuid
},
"type" => "comment"
}