diff --git a/lib/service/workers/send_activity_recap_worker.ex b/lib/service/workers/send_activity_recap_worker.ex index 0bdf4da34..b11b2f9dc 100644 --- a/lib/service/workers/send_activity_recap_worker.ex +++ b/lib/service/workers/send_activity_recap_worker.ex @@ -23,34 +23,41 @@ defmodule Mobilizon.Service.Workers.SendActivityRecapWorker do def perform(%Job{}) do Logger.info("Sending scheduled activity recap") - Repo.transaction( - fn -> - Users.stream_users_for_recap() - |> Enum.to_list() - |> Repo.preload([:settings, :activity_settings]) - |> Enum.filter(&filter_elegible_users/1) - |> Enum.map(fn %User{} = user -> - %{ - activities: activities_for_user(user), - user: user - } - end) - |> Enum.filter(fn %{activities: activities, user: _user} -> length(activities) > 0 end) - |> Enum.map(fn %{ - activities: activities, - user: - %User{settings: %Setting{group_notifications: group_notifications}} = - user - } -> + case Repo.transaction(&produce_notifications/0, timeout: :infinity) do + {:ok, res} -> + Logger.info("Processed #{length(res)} notifications to send") + + Enum.each(res, fn %{ + activities: activities, + user: + %User{settings: %Setting{group_notifications: group_notifications}} = + user + } -> Logger.info( "Asking to send email notification #{group_notifications} to user #{user.email} for #{length(activities)} activities" ) Email.send(user, activities, recap: group_notifications) end) - end, - timeout: :infinity - ) + + {:error, err} -> + Logger.error("Error producing notifications #{inspect(err)}") + {:error, err} + end + end + + defp produce_notifications do + Users.stream_users_for_recap() + |> Enum.to_list() + |> Repo.preload([:settings, :activity_settings]) + |> Enum.filter(&filter_elegible_users/1) + |> Enum.map(fn %User{} = user -> + %{ + activities: activities_for_user(user), + user: user + } + end) + |> Enum.filter(fn %{activities: activities, user: _user} -> length(activities) > 0 end) end defp activities_for_user( @@ -87,6 +94,7 @@ defmodule Mobilizon.Service.Workers.SendActivityRecapWorker do defp filter_elegible_users(%User{ settings: %Setting{last_notification_sent: nil, group_notifications: :one_hour} }) do + Logger.debug("Sending because never sent before, and we must do it at most once an hour") true end @@ -96,6 +104,10 @@ defmodule Mobilizon.Service.Workers.SendActivityRecapWorker do group_notifications: :one_hour } }) do + Logger.debug( + "Testing if it's less than an hour since the last time we sent an activity recap" + ) + is_delay_ok_since_last_notification_sent?(last_notification_sent) end @@ -106,6 +118,7 @@ defmodule Mobilizon.Service.Workers.SendActivityRecapWorker do timezone: timezone } }) do + Logger.debug("Testing if we're between daily sending hours") is_between_hours?(timezone: timezone || "Etc/UTC") end @@ -117,6 +130,7 @@ defmodule Mobilizon.Service.Workers.SendActivityRecapWorker do timezone: timezone } }) do + Logger.debug("Testing if we're between weekly sending day and hours") is_between_hours_on_first_day?(timezone: timezone || "Etc/UTC", locale: locale) end end diff --git a/test/service/workers/send_activity_recap_worker_test.exs b/test/service/workers/send_activity_recap_worker_test.exs new file mode 100644 index 000000000..0d56659ef --- /dev/null +++ b/test/service/workers/send_activity_recap_worker_test.exs @@ -0,0 +1,73 @@ +defmodule Mobilizon.Service.Workers.SendActivityRecapWorkerTest do + @moduledoc """ + Test the SendActivityRecapWorker module + """ + + alias Mobilizon.{Activities, Users} + alias Mobilizon.Activities.Activity + alias Mobilizon.Actors.Actor + alias Mobilizon.Service.Workers.SendActivityRecapWorker + alias Mobilizon.Storage.Page + alias Mobilizon.Users.{ActivitySetting, Setting, User} + + use Mobilizon.DataCase + import Swoosh.TestAssertions + import Mobilizon.Factory + + describe "Send activity recap" do + # Skipped because this depends on the test being run between @start_time and @end_time + @tag :skip + test "not if we already have sent notifications" do + %User{} = user = insert(:user) + %Actor{} = actor = insert(:actor, user: user) + %Actor{} = group = insert(:group) + + insert(:member, + parent: group, + actor: actor, + role: :administrator, + member_since: DateTime.add(DateTime.utc_now(), -3600) + ) + + %Activity{id: activity_id} = + insert(:mobilizon_activity, inserted_at: DateTime.utc_now(), group: group) + + assert %Page{elements: [%Activity{id: ^activity_id}], total: 1} = + Activities.list_group_activities(group.id) + + assert [%Activity{id: ^activity_id}] = + Activities.list_group_activities_for_recap(group.id, actor.id) + + old = DateTime.utc_now() |> DateTime.add(-3600 * 24 * 3) |> DateTime.truncate(:second) + + %Setting{} = + user_settings = + insert(:settings, + user: user, + user_id: user.id, + group_notifications: :one_day, + last_notification_sent: old + ) + + %ActivitySetting{} = + activity_setting = insert(:mobilizon_activity_setting, user_id: user.id, user: user) + + Users.update_user(user, %{settings: user_settings, activity_settings: [activity_setting]}) + assert old == Users.get_user_with_settings!(user.id).settings.last_notification_sent + + assert :ok == SendActivityRecapWorker.perform(%Oban.Job{}) + + assert_email_sent(to: user.email) + + assert %{last_notification_sent: updated_last_notification_sent} = + Users.get_setting(user.id) + + assert old != updated_last_notification_sent + assert DateTime.diff(DateTime.utc_now(), updated_last_notification_sent) < 5 + + assert :ok == SendActivityRecapWorker.perform(%Oban.Job{}) + + refute_email_sent() + end + end +end diff --git a/test/support/factory.ex b/test/support/factory.ex index 039bc3a6b..2dd093bf9 100644 --- a/test/support/factory.ex +++ b/test/support/factory.ex @@ -263,6 +263,7 @@ defmodule Mobilizon.Factory do parent: build(:actor), actor: build(:actor), role: :not_approved, + member_since: nil, id: uuid, url: "#{Endpoint.url()}/member/#{uuid}" }