diff --git a/lib/service/date_time/date_time.ex b/lib/service/date_time/date_time.ex index 102a4c504..bccf9e7d0 100644 --- a/lib/service/date_time/date_time.ex +++ b/lib/service/date_time/date_time.ex @@ -205,9 +205,12 @@ defmodule Mobilizon.Service.DateTime do is_first_day_of_week(compare_to_day, locale) && is_between_hours?(options) end - @spec is_delay_ok_since_last_notification_sent?(DateTime.t()) :: boolean() - def is_delay_ok_since_last_notification_sent?(%DateTime{} = last_notification_sent) do - DateTime.compare(DateTime.add(last_notification_sent, 3_600), DateTime.utc_now()) == + @spec is_delay_ok_since_last_notification_sent?(DateTime.t(), pos_integer()) :: boolean() + def is_delay_ok_since_last_notification_sent?( + %DateTime{} = last_notification_sent, + delay \\ 3_600 + ) do + DateTime.compare(DateTime.add(last_notification_sent, delay), DateTime.utc_now()) == :lt end diff --git a/lib/service/notifier/email.ex b/lib/service/notifier/email.ex index fb2b21131..6fa37cc70 100644 --- a/lib/service/notifier/email.ex +++ b/lib/service/notifier/email.ex @@ -12,7 +12,8 @@ defmodule Mobilizon.Service.Notifier.Email do import Mobilizon.Service.DateTime, only: [ - is_delay_ok_since_last_notification_sent?: 1 + is_delay_ok_since_last_notification_sent?: 1, + is_delay_ok_since_last_notification_sent?: 2 ] require Logger @@ -35,9 +36,10 @@ defmodule Mobilizon.Service.Notifier.Email do def send(%User{email: email, locale: locale} = user, activities, options) when is_list(activities) do activities = Enum.filter(activities, &can_send_activity?(&1, user, options)) + nb_activities = length(activities) - if length(activities) > 0 do - Logger.debug("Found some activities to send by email") + if nb_activities > 0 do + Logger.info("Sending email containing #{nb_activities} activities to #{email}") email |> EmailActivity.direct_activity(activities, Keyword.put(options, :locale, locale)) @@ -119,6 +121,27 @@ defmodule Mobilizon.Service.Notifier.Email do is_delay_ok_since_last_notification_sent?(last_notification_sent) end + # Delay ok since last notification + defp match_group_notifications_setting( + :one_day, + _, + %DateTime{} = last_notification_sent, + options + ) do + is_delay_ok_since_last_notification_sent?(last_notification_sent, 3_600 * 23) and + Keyword.get(options, :recap, false) != false + end + + defp match_group_notifications_setting( + :one_week, + _, + %DateTime{} = last_notification_sent, + options + ) do + is_delay_ok_since_last_notification_sent?(last_notification_sent, 3_600 * 24 * 6) and + Keyword.get(options, :recap, false) != false + end + # This is a recap defp match_group_notifications_setting( _group_notifications, @@ -154,7 +177,8 @@ defmodule Mobilizon.Service.Notifier.Email do end @spec save_last_notification_time(User.t()) :: {:ok, Setting.t()} | {:error, Ecto.Changeset.t()} - defp save_last_notification_time(%User{id: user_id}) do + defp save_last_notification_time(%User{id: user_id, email: email}) do + Logger.debug("Saving last notification time for user #{email}") attrs = %{user_id: user_id, last_notification_sent: DateTime.utc_now()} case Users.get_setting(user_id) do diff --git a/lib/service/workers/send_activity_recap_worker.ex b/lib/service/workers/send_activity_recap_worker.ex index ec8a7b824..62d957886 100644 --- a/lib/service/workers/send_activity_recap_worker.ex +++ b/lib/service/workers/send_activity_recap_worker.ex @@ -10,6 +10,7 @@ defmodule Mobilizon.Service.Workers.SendActivityRecapWorker do alias Mobilizon.Service.Notifier.Email alias Mobilizon.Storage.Repo alias Mobilizon.Users.{Setting, User} + require Logger import Mobilizon.Service.DateTime, only: [ @@ -20,6 +21,8 @@ defmodule Mobilizon.Service.Workers.SendActivityRecapWorker do @impl Oban.Worker def perform(%Job{}) do + Logger.info("Sending scheduled activity recap") + Repo.transaction( fn -> Users.stream_users_for_recap() diff --git a/lib/web/email/group.ex b/lib/web/email/group.ex index df5f49f1a..964028fcb 100644 --- a/lib/web/email/group.ex +++ b/lib/web/email/group.ex @@ -17,25 +17,27 @@ defmodule Mobilizon.Web.Email.Group do # TODO: When we have events restricted to members, don't send emails to followers group |> Actors.list_actors_to_notify_from_group_event() + |> Enum.reduce([], fn actor, users -> + # No emails for remote actors + if is_nil(actor.user_id) do + users + else + users ++ [Users.get_user_with_activity_settings!(actor.user_id)] + end + end) |> Enum.each(¬ify_follower(event, group, &1)) end def notify_of_new_event(%Event{}), do: :ok - defp notify_follower(%Event{} = _event, %Actor{}, %Actor{user_id: nil}), do: :ok - - defp notify_follower(%Event{} = event, %Actor{type: :Group} = group, %Actor{ - id: profile_id, - user_id: user_id + defp notify_follower(%Event{} = event, %Actor{type: :Group} = group, %User{ + email: email, + locale: locale, + settings: %Setting{timezone: timezone}, + activity_settings: activity_settings, + default_actor_id: default_actor_id }) do - %User{ - email: email, - locale: locale, - settings: %Setting{timezone: timezone}, - activity_settings: activity_settings - } = Users.get_user_with_activity_settings!(user_id) - - if profile_id != event.organizer_actor_id && + if default_actor_id != event.organizer_actor_id && accepts_new_events_notifications(activity_settings) do Gettext.put_locale(locale) diff --git a/lib/web/templates/email/group_membership_approval.html.heex b/lib/web/templates/email/group_membership_approval.html.heex index aafa72415..a681bbc55 100644 --- a/lib/web/templates/email/group_membership_approval.html.heex +++ b/lib/web/templates/email/group_membership_approval.html.heex @@ -48,7 +48,7 @@ "Your membership request for group %{link_start}%{group}%{link_end} has been approved.", group: Mobilizon.Actors.Actor.display_name(@group), link_start: - "", + " URI.decode()}\">", link_end: "" ) |> raw %> @@ -66,7 +66,7 @@ URI.decode()}" } target="_blank" style="font-size: 20px; font-family: Helvetica, Arial, sans-serif; color: #ffffff; text-decoration: none; padding: 15px 25px; border-radius: 2px; border: 1px solid #3C376E; display: inline-block;" diff --git a/lib/web/templates/email/group_membership_approval.text.eex b/lib/web/templates/email/group_membership_approval.text.eex index 423ba9d89..adf657fdf 100644 --- a/lib/web/templates/email/group_membership_approval.text.eex +++ b/lib/web/templates/email/group_membership_approval.text.eex @@ -2,4 +2,4 @@ == <%= gettext "Your membership request for group %{group} has been approved.", group: Mobilizon.Actors.Actor.display_name(@group) %> -<%= Routes.page_url(Mobilizon.Web.Endpoint, :actor, Mobilizon.Actors.Actor.preferred_username_and_domain(@group)) %> +<%= Routes.page_url(Mobilizon.Web.Endpoint, :actor, Mobilizon.Actors.Actor.preferred_username_and_domain(@group)) |> URI.decode() %> diff --git a/lib/web/templates/email/group_membership_rejection.html.heex b/lib/web/templates/email/group_membership_rejection.html.heex index 565e3bea3..29343d5ed 100644 --- a/lib/web/templates/email/group_membership_rejection.html.heex +++ b/lib/web/templates/email/group_membership_rejection.html.heex @@ -48,7 +48,7 @@ "Your membership request for group %{link_start}%{group}%{link_end} has been rejected.", group: Mobilizon.Actors.Actor.display_name(@group), link_start: - "", + " URI.decode()}\">", link_end: "" ) |> raw %> diff --git a/lib/web/templates/email/group_membership_rejection.text.eex b/lib/web/templates/email/group_membership_rejection.text.eex index 9c867eeb4..7ecf7c2b7 100644 --- a/lib/web/templates/email/group_membership_rejection.text.eex +++ b/lib/web/templates/email/group_membership_rejection.text.eex @@ -2,4 +2,4 @@ == <%= gettext "Your membership request for group %{group} has been rejected.", group: Mobilizon.Actors.Actor.display_name(@group) %> -<%= Routes.page_url(Mobilizon.Web.Endpoint, :actor, Mobilizon.Actors.Actor.preferred_username_and_domain(@group)) %> +<%= Routes.page_url(Mobilizon.Web.Endpoint, :actor, Mobilizon.Actors.Actor.preferred_username_and_domain(@group)) |> URI.decode() %> diff --git a/lib/web/templates/email/participation/card/_title.html.heex b/lib/web/templates/email/participation/card/_title.html.heex index df8ab978f..3d9f6fe0a 100644 --- a/lib/web/templates/email/participation/card/_title.html.heex +++ b/lib/web/templates/email/participation/card/_title.html.heex @@ -40,7 +40,7 @@ :actor, Mobilizon.Actors.Actor.preferred_username_and_domain(@event.attributed_to) ) - |> raw + |> URI.decode() } style="color: rgb(254,56,89); font-family: Helvetica,Arial,sans-serif; font-weight: normal; text-align: left; line-height: 1.3; text-decoration: none; vertical-align: baseline; margin: 0; padding: 0; border: 0;" target="_blank" diff --git a/lib/web/templates/email/participation/card/_title.text.eex b/lib/web/templates/email/participation/card/_title.text.eex index 9e9d81c38..0ffc0a4da 100644 --- a/lib/web/templates/email/participation/card/_title.text.eex +++ b/lib/web/templates/email/participation/card/_title.text.eex @@ -1,3 +1,3 @@ <%= gettext("Title: %{title}", title: @event.title) %> -<%= if @event.attributed_to do %><%= gettext("Organizer: %{organizer}", organizer: @event.attributed_to.name || @event.attributed_to.preferred_username) %> <%= Routes.page_url(Mobilizon.Web.Endpoint, :actor, Mobilizon.Actors.Actor.preferred_username_and_domain(@event.attributed_to)) %><% else %><%= gettext("Organizer: %{organizer}", organizer: @event.organizer_actor.name || @event.organizer_actor.preferred_username) %><% end %> \ No newline at end of file +<%= if @event.attributed_to do %><%= gettext("Organizer: %{organizer}", organizer: @event.attributed_to.name || @event.attributed_to.preferred_username) %> <%= Routes.page_url(Mobilizon.Web.Endpoint, :actor, Mobilizon.Actors.Actor.preferred_username_and_domain(@event.attributed_to)) |> URI.decode() %><% else %><%= gettext("Organizer: %{organizer}", organizer: @event.organizer_actor.name || @event.organizer_actor.preferred_username) %><% end %> \ No newline at end of file diff --git a/test/service/notifier/email_test.exs b/test/service/notifier/email_test.exs index 49a2e0a3b..20c99d836 100644 --- a/test/service/notifier/email_test.exs +++ b/test/service/notifier/email_test.exs @@ -4,7 +4,7 @@ defmodule Mobilizon.Service.Notifier.EmailTest do """ alias Mobilizon.Activities.Activity - alias Mobilizon.Config + alias Mobilizon.{Config, Users} alias Mobilizon.Service.Notifier.Email alias Mobilizon.Users.{ActivitySetting, Setting, User} @@ -101,12 +101,14 @@ defmodule Mobilizon.Service.Notifier.EmailTest do %Activity{} = activity = insert(:mobilizon_activity, inserted_at: DateTime.utc_now()) %User{} = user = insert(:user) + old = DateTime.add(DateTime.utc_now(), -3600 * 24 * 3) + %Setting{} = user_settings = insert(:settings, user_id: user.id, group_notifications: :one_day, - last_notification_sent: DateTime.add(DateTime.utc_now(), 3600) + last_notification_sent: old ) %ActivitySetting{} = @@ -114,7 +116,19 @@ defmodule Mobilizon.Service.Notifier.EmailTest do user = %User{user | settings: user_settings, activity_settings: [activity_setting]} - assert {:ok, :skipped} == Email.send(user, activity) + assert {:ok, :sent} == Email.send(user, activity, recap: :one_day) + + assert_email_sent(to: user.email) + + assert %{last_notification_sent: updated_last_notification_sent} = + user_settings = Users.get_setting(user.id) + + assert old != updated_last_notification_sent + assert DateTime.diff(DateTime.utc_now(), updated_last_notification_sent) < 5 + + user = %User{user | settings: user_settings, activity_settings: [activity_setting]} + + assert {:ok, :skipped} == Email.send(user, activity, recap: :one_day) refute_email_sent() end