Make sure activity notification recaps can't be sent multiple times

Signed-off-by: Thomas Citharel <tcit@tcit.fr>
This commit is contained in:
Thomas Citharel 2022-05-10 13:13:48 +02:00
parent 3fea2d0395
commit 1eb111f52f
No known key found for this signature in database
GPG Key ID: A061B9DDE0CA0773
3 changed files with 48 additions and 7 deletions

View File

@ -12,7 +12,8 @@ defmodule Mobilizon.Service.Notifier.Email do
import Mobilizon.Service.DateTime, import Mobilizon.Service.DateTime,
only: [ 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 require Logger
@ -35,9 +36,10 @@ defmodule Mobilizon.Service.Notifier.Email do
def send(%User{email: email, locale: locale} = user, activities, options) def send(%User{email: email, locale: locale} = user, activities, options)
when is_list(activities) do when is_list(activities) do
activities = Enum.filter(activities, &can_send_activity?(&1, user, options)) activities = Enum.filter(activities, &can_send_activity?(&1, user, options))
nb_activities = length(activities)
if length(activities) > 0 do if nb_activities > 0 do
Logger.debug("Found some activities to send by email") Logger.info("Sending email containing #{nb_activities} activities to #{email}")
email email
|> EmailActivity.direct_activity(activities, Keyword.put(options, :locale, locale)) |> 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) is_delay_ok_since_last_notification_sent?(last_notification_sent)
end 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 # This is a recap
defp match_group_notifications_setting( defp match_group_notifications_setting(
_group_notifications, _group_notifications,
@ -154,7 +177,8 @@ defmodule Mobilizon.Service.Notifier.Email do
end end
@spec save_last_notification_time(User.t()) :: {:ok, Setting.t()} | {:error, Ecto.Changeset.t()} @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()} attrs = %{user_id: user_id, last_notification_sent: DateTime.utc_now()}
case Users.get_setting(user_id) do case Users.get_setting(user_id) do

View File

@ -10,6 +10,7 @@ defmodule Mobilizon.Service.Workers.SendActivityRecapWorker do
alias Mobilizon.Service.Notifier.Email alias Mobilizon.Service.Notifier.Email
alias Mobilizon.Storage.Repo alias Mobilizon.Storage.Repo
alias Mobilizon.Users.{Setting, User} alias Mobilizon.Users.{Setting, User}
require Logger
import Mobilizon.Service.DateTime, import Mobilizon.Service.DateTime,
only: [ only: [
@ -20,6 +21,8 @@ defmodule Mobilizon.Service.Workers.SendActivityRecapWorker do
@impl Oban.Worker @impl Oban.Worker
def perform(%Job{}) do def perform(%Job{}) do
Logger.info("Sending scheduled activity recap")
Repo.transaction( Repo.transaction(
fn -> fn ->
Users.stream_users_for_recap() Users.stream_users_for_recap()

View File

@ -4,7 +4,7 @@ defmodule Mobilizon.Service.Notifier.EmailTest do
""" """
alias Mobilizon.Activities.Activity alias Mobilizon.Activities.Activity
alias Mobilizon.Config alias Mobilizon.{Config, Users}
alias Mobilizon.Service.Notifier.Email alias Mobilizon.Service.Notifier.Email
alias Mobilizon.Users.{ActivitySetting, Setting, User} 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()) %Activity{} = activity = insert(:mobilizon_activity, inserted_at: DateTime.utc_now())
%User{} = user = insert(:user) %User{} = user = insert(:user)
old = DateTime.add(DateTime.utc_now(), -3600 * 24 * 3)
%Setting{} = %Setting{} =
user_settings = user_settings =
insert(:settings, insert(:settings,
user_id: user.id, user_id: user.id,
group_notifications: :one_day, group_notifications: :one_day,
last_notification_sent: DateTime.add(DateTime.utc_now(), 3600) last_notification_sent: old
) )
%ActivitySetting{} = %ActivitySetting{} =
@ -114,7 +116,19 @@ defmodule Mobilizon.Service.Notifier.EmailTest do
user = %User{user | settings: user_settings, activity_settings: [activity_setting]} 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() refute_email_sent()
end end