Send activity digests

Signed-off-by: Thomas Citharel <tcit@tcit.fr>

lib/graphql/schema/user.ex

@@ -190,6 +190,10 @@ defmodule Mobilizon.GraphQL.Schema.UserType do
         "When does the user receives a notification about a new pending membership in one of the group they're admin for"
     )
 
+    field(:group_notifications, :notification_pending_enum,
+      description: "When does the user receives a notification about new activity"
+    )
+
     field(:location, :location,
       description: "The user's preferred location, where they want to be suggested events"
     )
@@ -213,6 +217,11 @@ defmodule Mobilizon.GraphQL.Schema.UserType do
       as: :one_day,
       description: "One day. Notifications will be sent at most each day"
     )
+
+    value(:one_week,
+      as: :one_week,
+      description: "One Week. Notifications will be sent at most each week"
+    )
   end
 
   object :location do
@@ -384,6 +393,10 @@ defmodule Mobilizon.GraphQL.Schema.UserType do
           "When does the user receives a notification about a new pending membership in one of the group they're admin for"
       )
 
+      arg(:group_notifications, :notification_pending_enum,
+        description: "When does the user receives a notification about new activity"
+      )
+
       arg(:location, :location_input,
         description: "A geohash of the user's preferred location, where they want to see events"
       )

lib/mobilizon/activities/activities.ex

@@ -121,6 +121,32 @@ defmodule Mobilizon.Activities do
     |> Page.build_page(page, limit)
   end
 
+  @spec list_group_activities_for_recap(
+          integer() | String.t(),
+          integer() | String.t(),
+          DateTime.t() | nil
+        ) :: [Activity.t()]
+  def list_group_activities_for_recap(
+        group_id,
+        actor_asking_id,
+        last_sent_at \\ nil
+      ) do
+    query =
+      Activity
+      |> where([a], a.group_id == ^group_id)
+      |> join(:inner, [a], m in Member,
+        on: m.parent_id == a.group_id and m.actor_id == ^actor_asking_id
+      )
+      |> where([a, m], a.inserted_at >= m.member_since)
+      |> order_by(desc: :inserted_at)
+      |> preload([:author, :group])
+
+    query =
+      if is_nil(last_sent_at), do: query, else: where(query, [a], a.inserted_at >= ^last_sent_at)
+
+    Repo.all(query)
+  end
+
   @doc """
   Gets a single activity.
 

lib/mobilizon/actors/actors.ex

@@ -1474,12 +1474,9 @@ defmodule Mobilizon.Actors do
 
   @spec groups_member_of_query(integer | String.t()) :: Ecto.Query.t()
   defp groups_member_of_query(actor_id) do
-    from(
-      a in Actor,
-      join: m in Member,
-      on: a.id == m.parent_id,
-      where: m.actor_id == ^actor_id
-    )
+    Actor
+    |> join(:inner, [a], m in Member, on: a.id == m.parent_id)
+    |> where([a, m], m.actor_id == ^actor_id and m.role in ^@member_roles)
   end
 
   @spec groups_query :: Ecto.Query.t()

lib/mobilizon/users/users.ex

@@ -17,7 +17,13 @@ defmodule Mobilizon.Users do
 
   defenum(UserRole, :user_role, [:administrator, :moderator, :user])
 
-  defenum(NotificationPendingNotificationDelay, none: 0, direct: 1, one_hour: 5, one_day: 10)
+  defenum(NotificationPendingNotificationDelay,
+    none: 0,
+    direct: 1,
+    one_hour: 5,
+    one_day: 10,
+    one_week: 15
+  )
 
   @confirmation_token_length 30
 
@@ -520,6 +526,18 @@ defmodule Mobilizon.Users do
     |> Repo.insert(on_conflict: :replace_all, conflict_target: [:user_id, :key, :method])
   end
 
+  @doc """
+  Returns a stream of users which want to have a scheduled recap
+  """
+  @spec stream_users_for_recap :: Enum.t()
+  def stream_users_for_recap do
+    User
+    |> filter_activated(true)
+    |> join(:inner, [u], s in Setting, on: s.user_id == u.id)
+    |> where([_u, s], s.group_notifications in [:one_hour, :one_day, :one_week])
+    |> Repo.stream()
+  end
+
   @spec user_by_email_query(String.t(), boolean | nil, boolean()) :: Ecto.Query.t()
   defp user_by_email_query(email, activated, unconfirmed) do
     User

lib/service/date_time/date_time.ex

@@ -15,8 +15,8 @@ defmodule Mobilizon.Service.DateTime do
   @spec datetime_tz_convert(DateTime.t(), String.t()) :: DateTime.t()
   def datetime_tz_convert(%DateTime{} = datetime, timezone) do
     case DateTime.shift_zone(datetime, timezone) do
-      {:ok, datetime_with_user_tz} ->
-        datetime_with_user_tz
+      {:ok, datetime_with_tz} ->
+        datetime_with_tz
 
       _ ->
         datetime
@@ -38,4 +38,156 @@ defmodule Mobilizon.Service.DateTime do
       "en"
     end
   end
+
+  def is_first_day_of_week(%Date{} = date, locale \\ "en") do
+    Date.day_of_week(date) == Cldr.Calendar.first_day_for_locale(locale)
+  end
+
+  @spec calculate_first_day_of_week(Date.t(), String.t()) :: Date.t()
+  def calculate_first_day_of_week(%Date{} = date, locale \\ "en") do
+    if is_first_day_of_week(date, locale),
+      do: date,
+      else: calculate_first_day_of_week(Date.add(date, -1), locale)
+  end
+
+  @doc """
+  Calculate the time when a notification should be sent, based on a daily schedule
+
+  ## Parameters
+   * `compare_to` When to compare to. Defaults to the current datetime
+   * `notification_time` The time when the notification is being sent. Defaults to `~T[08:00:00]`
+   * `timezone` The user's timezone. Needed to convert the time in the user's local timezone. Defaults to `"Etc/UTC"`
+  """
+  @spec calculate_next_day_notification(Date.t(), Keyword.t()) :: DateTime.t()
+  def calculate_next_day_notification(%Date{} = day, options \\ []) do
+    compare_to = Keyword.get(options, :compare_to, DateTime.utc_now())
+    notification_time = Keyword.get(options, :notification_time, ~T[18:00:00])
+    timezone = Keyword.get(options, :timezone, "Etc/UTC")
+
+    send_at = DateTime.new!(day, notification_time, timezone)
+
+    if DateTime.compare(send_at, compare_to) == :lt do
+      day
+      |> Date.add(1)
+      |> DateTime.new!(notification_time, timezone)
+    else
+      send_at
+    end
+  end
+
+  @doc """
+  Calculate the time when a notification should be sent, based on a weekly schedule
+
+  ## Parameters
+   * `compare_to` When to compare to. Defaults to the current datetime
+   * `notification_time` The time when the notification is being sent. Defaults to `~T[08:00:00]`
+   * `timezone` The user's timezone. Needed to convert the time in the user's local timezone. Defaults to `"Etc/UTC"`
+   * `locale` The user's locale. Allows to get the first day of the week to send the notification on the beginning of the week. Defaults to `"en"`.
+  """
+  @spec calculate_next_week_notification(DateTime.t(), Keyword.t()) :: DateTime.t() | nil
+  def calculate_next_week_notification(begins_on, options \\ []) do
+    # That's now, but we allow to override it for tests
+    compare_to = Keyword.get(options, :compare_to, DateTime.utc_now())
+
+    # If the event is in the future
+    if DateTime.compare(begins_on, compare_to) == :gt do
+      # We get the day of the scheduled notification next week
+      notification_date = appropriate_first_day_of_week(begins_on, options)
+
+      if is_nil(notification_date) do
+        nil
+      else
+        # This is the datetime when the notification should be sent
+        if DateTime.compare(notification_date, compare_to) == :gt do
+          notification_date
+        else
+          nil
+        end
+      end
+    else
+      # In the past, don't send anything
+      nil
+    end
+  end
+
+  @spec next_first_day_of_week(DateTime.t(), Keyword.t()) :: Date.t() | nil
+  def next_first_day_of_week(%DateTime{} = datetime, options) do
+    locale = Keyword.get(options, :locale, "en")
+    compare_to = Keyword.get(options, :compare_to, DateTime.utc_now())
+
+    next_first_day_of_week =
+      compare_to
+      |> DateTime.to_date()
+      |> calculate_first_day_of_week(locale)
+      |> Timex.add(Timex.Duration.from_weeks(1))
+      |> build_notification_datetime(options)
+
+    if Date.compare(datetime, next_first_day_of_week) == :gt do
+      next_first_day_of_week
+    else
+      nil
+    end
+  end
+
+  def appropriate_first_day_of_week(%DateTime{} = datetime, options) do
+    locale = Keyword.get(options, :locale, "en")
+    timezone = Keyword.get(options, :timezone, "Etc/UTC")
+
+    local_datetime = datetime_tz_convert(datetime, timezone)
+
+    first_day = local_datetime |> DateTime.to_date() |> calculate_first_day_of_week(locale)
+    first_datetime = build_notification_datetime(first_day, options)
+
+    if DateTime.compare(local_datetime, first_datetime) == :gt do
+      first_datetime
+    else
+      next_first_day_of_week(local_datetime, options)
+    end
+  end
+
+  @spec build_notification_datetime(Date.t(), Keyword.t()) :: DateTime.t()
+  def build_notification_datetime(
+        %Date{} = date,
+        options
+      ) do
+    notification_time = Keyword.get(options, :notification_time, ~T[08:00:00])
+    timezone = Keyword.get(options, :timezone, "Etc/UTC")
+    DateTime.new!(date, notification_time, timezone)
+  end
+
+  @start_time ~T[08:00:00]
+  @end_time ~T[09:00:00]
+
+  @spec is_between_hours(Keyword.t()) :: boolean()
+  def is_between_hours(options \\ []) when is_list(options) do
+    compare_to_day = Keyword.get(options, :compare_to_day, Date.utc_today())
+    compare_to = Keyword.get(options, :compare_to_datetime, DateTime.utc_now())
+    start_time = Keyword.get(options, :start_time, @start_time)
+    timezone = Keyword.get(options, :timezone, "Etc/UTC")
+    end_time = Keyword.get(options, :end_time, @end_time)
+
+    DateTime.compare(compare_to, DateTime.new!(compare_to_day, start_time, timezone)) in [
+      :gt,
+      :eq
+    ] &&
+      DateTime.compare(
+        compare_to,
+        DateTime.new!(compare_to_day, end_time, timezone)
+      ) == :lt
+  end
+
+  @spec is_between_hours_on_first_day(Keyword.t()) :: boolean()
+  def is_between_hours_on_first_day(options) when is_list(options) do
+    compare_to_day = Keyword.get(options, :compare_to_day, Date.utc_today())
+    locale = Keyword.get(options, :locale, "en")
+
+    Mobilizon.Service.DateTime.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()) ==
+      :lt
+  end
 end

lib/service/notifications/scheduler.ex

@@ -8,6 +8,15 @@ defmodule Mobilizon.Service.Notifications.Scheduler do
   alias Mobilizon.Events.{Event, Participant}
   alias Mobilizon.Service.Workers.Notification
   alias Mobilizon.Users.{Setting, User}
+
+  import Mobilizon.Service.DateTime,
+    only: [
+      datetime_tz_convert: 2,
+      calculate_first_day_of_week: 2,
+      calculate_next_day_notification: 2,
+      calculate_next_week_notification: 2
+    ]
+
   require Logger
 
   @spec trigger_notifications_for_participant(Participant.t()) :: {:ok, nil}
@@ -44,7 +53,7 @@ defmodule Mobilizon.Service.Notifications.Scheduler do
       when not is_nil(user_id) do
     case Users.get_setting(user_id) do
       %Setting{notification_on_day: true, timezone: timezone} ->
-        %DateTime{hour: hour} = begins_on_shifted = shift_zone(begins_on, timezone)
+        %DateTime{hour: hour} = begins_on_shifted = datetime_tz_convert(begins_on, timezone)
         Logger.debug("Participation event start at #{inspect(begins_on_shifted)} (user timezone)")
 
         send_date =
@@ -90,7 +99,7 @@ defmodule Mobilizon.Service.Notifications.Scheduler do
 
     case settings do
       %Setting{notification_each_week: true, timezone: timezone} ->
-        %DateTime{} = begins_on_shifted = shift_zone(begins_on, timezone)
+        %DateTime{} = begins_on_shifted = datetime_tz_convert(begins_on, timezone)
 
         Logger.debug(
           "Participation event start at #{inspect(begins_on_shifted)} (user timezone is #{timezone})"
@@ -143,6 +152,7 @@ defmodule Mobilizon.Service.Notifications.Scheduler do
     with %Actor{user_id: user_id} when not is_nil(user_id) <-
            Actors.get_actor(organizer_actor_id),
          %User{
+           locale: locale,
            settings: %Setting{
              notification_pending_participation: notification_pending_participation,
              timezone: timezone
@@ -157,7 +167,13 @@ defmodule Mobilizon.Service.Notifications.Scheduler do
             :direct
 
           :one_day ->
-            calculate_next_day_notification(Date.utc_today(), timezone)
+            calculate_next_day_notification(Date.utc_today(), timezone: timezone)
+
+          :one_week ->
+            calculate_next_week_notification(DateTime.utc_now(),
+              timezone: timezone,
+              locale: locale
+            )
 
           :one_hour ->
             DateTime.utc_now()
@@ -259,40 +275,4 @@ defmodule Mobilizon.Service.Notifications.Scheduler do
         Notification.enqueue(:pending_membership_notification, params, scheduled_at: send_at)
     end
   end
-
-  defp shift_zone(datetime, timezone) do
-    case DateTime.shift_zone(datetime, timezone) do
-      {:ok, shift_datetime} -> shift_datetime
-      {:error, _} -> datetime
-    end
-  end
-
-  defp calculate_first_day_of_week(%Date{} = date, locale) do
-    day_number = Date.day_of_week(date)
-    first_day_number = Cldr.Calendar.first_day_for_locale(locale)
-
-    if day_number == first_day_number,
-      do: date,
-      else: calculate_first_day_of_week(Date.add(date, -1), locale)
-  end
-
-  defp calculate_next_day_notification(%Date{} = day, timezone) do
-    send_at = date_to_datetime(day, ~T[18:00:00], timezone)
-
-    if DateTime.compare(send_at, DateTime.utc_now()) == :lt do
-      day
-      |> Date.add(1)
-      |> date_to_datetime(~T[18:00:00], timezone)
-    else
-      send_at
-    end
-  end
-
-  defp date_to_datetime(%Date{} = day, time, timezone) do
-    # Just in case
-    timezone = timezone || "Etc/UTC"
-    {:ok, datetime} = NaiveDateTime.new(day, time)
-    {:ok, datetime} = DateTime.from_naive(datetime, timezone)
-    datetime
-  end
 end

lib/service/notifier/email.ex

@@ -6,10 +6,17 @@ defmodule Mobilizon.Service.Notifier.Email do
   alias Mobilizon.{Config, Users}
   alias Mobilizon.Service.Notifier
   alias Mobilizon.Service.Notifier.{Email, Filter}
-  alias Mobilizon.Users.{NotificationPendingNotificationDelay, Setting, User}
+  alias Mobilizon.Users.{Setting, User}
   alias Mobilizon.Web.Email.Activity, as: EmailActivity
   alias Mobilizon.Web.Email.Mailer
 
+  import Mobilizon.Service.DateTime,
+    only: [
+      is_delay_ok_since_last_notification_sent: 1
+    ]
+
+  require Logger
+
   @behaviour Notifier
 
   @impl Notifier
@@ -27,9 +34,11 @@ defmodule Mobilizon.Service.Notifier.Email do
   @impl Notifier
   def send(%User{email: email, locale: locale} = user, activities, options)
       when is_list(activities) do
-    activities = Enum.filter(activities, &can_send_activity?(&1, user))
+    activities = Enum.filter(activities, &can_send_activity?(&1, user, options))
+
+    if length(activities) > 0 do
+      Logger.debug("Found some activities to send by email")
 
-    if can_send?(user) && length(activities) > 0 do
       email
       |> EmailActivity.direct_activity(activities, Keyword.put(options, :locale, locale))
       |> Mailer.send_email()
@@ -37,13 +46,80 @@ defmodule Mobilizon.Service.Notifier.Email do
       save_last_notification_time(user)
       {:ok, :sent}
     else
+      Logger.debug("No activities to send by email")
       {:ok, :skipped}
     end
   end
 
-  @spec can_send_activity?(Activity.t(), User.t()) :: boolean()
-  defp can_send_activity?(%Activity{} = activity, %User{} = user) do
-    Filter.can_send_activity?(activity, "email", user, &default_activity_behavior/1)
+  # These notifications are using LegacyNotifierBuilder and don't have any history,
+  # so we always send them directly, as long as the setting isn't none
+  @always_direct_subjects [
+    :participation_event_comment,
+    :event_comment_mention,
+    :discussion_mention
+  ]
+
+  @spec can_send_activity?(Activity.t(), User.t(), Keyword.t()) :: boolean()
+  defp can_send_activity?(
+         %Activity{subject: subject} = activity,
+         %User{
+           settings: %Setting{
+             group_notifications: group_notifications,
+             last_notification_sent: last_notification_sent
+           }
+         } = user,
+         options
+       ) do
+    Filter.can_send_activity?(activity, "email", user, &default_activity_behavior/1) &&
+      match_group_notifications_setting(
+        group_notifications,
+        subject,
+        last_notification_sent,
+        options
+      )
+  end
+
+  @spec match_group_notifications_setting(
+          NotificationPendingNotificationDelay.t(),
+          String.t(),
+          DateTime.t() | nil,
+          Keyword.t()
+        ) :: boolean()
+  # No notifications at all
+  defp match_group_notifications_setting(:none, _, _, _), do: false
+
+  # Every notification
+  defp match_group_notifications_setting(:direct, _, _, _), do: true
+
+  # Direct notifications
+  defp match_group_notifications_setting(_, subject, _, _)
+       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
+
+      # 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
+    end
   end
 
   @default_behavior %{
@@ -70,32 +146,6 @@ defmodule Mobilizon.Service.Notifier.Email do
     Map.get(@default_behavior, activity_setting, false)
   end
 
-  @type notification_type ::
-          :group_notifications
-          | :notification_pending_participation
-          | :notification_pending_membership
-
-  @spec user_notification_delay(User.t(), notification_type()) ::
-          NotificationPendingNotificationDelay.t()
-  defp user_notification_delay(%User{} = user, type \\ :group_notifications) do
-    Map.from_struct(user.settings)[type]
-  end
-
-  @spec can_send?(User.t()) :: boolean()
-  defp can_send?(%User{settings: %Setting{last_notification_sent: last_notification_sent}} = user) do
-    last_notification_sent_or_default = last_notification_sent || DateTime.utc_now()
-    notification_delay = user_notification_delay(user)
-    diff = DateTime.diff(DateTime.utc_now(), last_notification_sent_or_default)
-
-    cond do
-      notification_delay == :none -> false
-      is_nil(last_notification_sent) -> true
-      notification_delay == :direct -> true
-      notification_delay == :one_hour -> diff >= 60 * 60
-      notification_delay == :one_day -> diff >= 24 * 60 * 60
-    end
-  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
     attrs = %{user_id: user_id, last_notification_sent: DateTime.utc_now()}

lib/service/workers/digest_notifier_worker.ex

@@ -1,14 +0,0 @@
-defmodule Mobilizon.Service.Workers.DigestNotifierWorker do
-  @moduledoc """
-  Worker to send notifications
-  """
-
-  use Mobilizon.Service.Workers.Helper, queue: "notifications"
-
-  @impl Oban.Worker
-  def perform(%Job{}) do
-    # Get last time activities were send
-    # List activities to send
-    # Send activites
-  end
-end

lib/service/workers/notification.ex

@@ -11,6 +11,11 @@ defmodule Mobilizon.Service.Workers.Notification do
   alias Mobilizon.Users.{Setting, User}
   alias Mobilizon.Web.Email.{Mailer, Notification}
 
+  import Mobilizon.Service.DateTime,
+    only: [
+      datetime_tz_convert: 2
+    ]
+
   use Mobilizon.Service.Workers.Helper, queue: "mailers"
 
   @impl Oban.Worker
@@ -109,16 +114,9 @@ defmodule Mobilizon.Service.Workers.Notification do
     end
   end
 
-  defp shift_zone(datetime, timezone) do
-    case DateTime.shift_zone(datetime, timezone) do
-      {:ok, shift_datetime} -> shift_datetime
-      {:error, _} -> datetime
-    end
-  end
-
   defp calculate_start_end(days, timezone) do
     now = DateTime.utc_now()
-    %DateTime{} = now_shifted = shift_zone(now, timezone)
+    %DateTime{} = now_shifted = datetime_tz_convert(now, timezone)
     start = %{now_shifted | hour: 8, minute: 0, second: 0, microsecond: {0, 0}}
 
     {:ok, %NaiveDateTime{} = tomorrow} =

lib/service/workers/send_activity_recap_worker.ex

@@ -0,0 +1,112 @@
+defmodule Mobilizon.Service.Workers.SendActivityRecapWorker do
+  @moduledoc """
+  Worker to send activity recaps
+  """
+
+  use Oban.Worker, queue: "notifications"
+  alias Mobilizon.{Activities, Actors, Users}
+  alias Mobilizon.Activities.Activity
+  alias Mobilizon.Actors.Actor
+  alias Mobilizon.Service.Notifier.Email
+  alias Mobilizon.Storage.Repo
+  alias Mobilizon.Users.{Setting, User}
+
+  import Mobilizon.Service.DateTime,
+    only: [
+      is_between_hours: 1,
+      is_between_hours_on_first_day: 1,
+      is_delay_ok_since_last_notification_sent: 1
+    ]
+
+  @impl Oban.Worker
+  def perform(%Job{}) do
+    Repo.transaction(fn ->
+      Users.stream_users_for_recap()
+      |> Enum.to_list()
+      |> Repo.preload([: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
+                     } ->
+        Email.send(user, activities, recap: group_notifications)
+      end)
+    end)
+  end
+
+  defp activities_for_user(
+         %User{settings: %Setting{last_notification_sent: last_notification_sent}} = user
+       ) do
+    user
+    |> Users.get_actors_for_user()
+    |> Enum.flat_map(&group_memberships(&1, last_notification_sent))
+    |> Enum.uniq()
+  end
+
+  defp group_memberships(%Actor{id: actor_id} = actor, last_notification_sent) do
+    actor
+    |> group_memberships_for_actor()
+    |> Enum.uniq()
+    |> Enum.flat_map(&activities_for_group(&1, actor_id, last_notification_sent))
+  end
+
+  defp group_memberships_for_actor(%Actor{} = actor) do
+    Actors.list_groups_member_of(actor)
+  end
+
+  defp activities_for_group(
+         %Actor{id: group_id, type: :Group},
+         actor_asking_id,
+         last_notification_sent
+       ) do
+    group_id
+    |> Activities.list_group_activities_for_recap(actor_asking_id, last_notification_sent)
+    # Don't send my own activities
+    |> Enum.filter(fn %Activity{author: %Actor{id: author_id}} -> author_id != actor_asking_id end)
+  end
+
+  defp filter_elegible_users(%User{
+         settings: %Setting{last_notification_sent: nil, group_notifications: :one_hour}
+       }) do
+    true
+  end
+
+  defp filter_elegible_users(%User{
+         settings: %Setting{
+           last_notification_sent: %DateTime{} = last_notification_sent,
+           group_notifications: :one_hour
+         }
+       }) do
+    is_delay_ok_since_last_notification_sent(last_notification_sent)
+  end
+
+  # If we're between notification hours
+  defp filter_elegible_users(%User{
+         settings: %Setting{
+           group_notifications: :one_day,
+           timezone: timezone
+         }
+       }) do
+    is_between_hours(timezone: timezone)
+  end
+
+  # If we're on the first day of the week between notification hours
+  defp filter_elegible_users(%User{
+         locale: locale,
+         settings: %Setting{
+           group_notifications: :one_week,
+           timezone: timezone
+         }
+       }) do
+    is_between_hours_on_first_day(timezone: timezone, locale: locale)
+  end
+end

lib/web/email/activity.ex

@@ -21,13 +21,10 @@ defmodule Mobilizon.Web.Email.Activity do
       ) do
     locale = Keyword.get(options, :locale, "en")
     single_activity = Keyword.get(options, :single_activity, false)
+    recap = Keyword.get(options, :recap, false)
     Gettext.put_locale(locale)
 
-    subject =
-      gettext(
-        "Activity notification for %{instance}",
-        instance: Config.instance_name()
-      )
+    subject = get_subject(recap)
 
     chunked_activities = chunk_activities(activities)
 
@@ -37,6 +34,7 @@ defmodule Mobilizon.Web.Email.Activity do
     |> assign(:activities, chunked_activities)
     |> assign(:total_number_activities, length(activities))
     |> assign(:single_activity, single_activity)
+    |> assign(:recap, recap)
     |> render(:email_direct_activity)
   end
 
@@ -94,4 +92,38 @@ defmodule Mobilizon.Web.Email.Activity do
       end
     end)
   end
+
+  @spec get_subject(atom() | false) :: String.t()
+  defp get_subject(recap) do
+    if recap do
+      case recap do
+        :one_hour ->
+          dgettext(
+            "activity",
+            "Activity notification for %{instance}",
+            instance: Config.instance_name()
+          )
+
+        :one_day ->
+          dgettext(
+            "activity",
+            "Daily activity recap for %{instance}",
+            instance: Config.instance_name()
+          )
+
+        :one_week ->
+          dgettext(
+            "activity",
+            "Weekly activity recap for %{instance}",
+            instance: Config.instance_name()
+          )
+      end
+    else
+      dgettext(
+        "activity",
+        "Activity notification for %{instance}",
+        instance: Config.instance_name()
+      )
+    end
+  end
 end

lib/web/templates/email/activity/_comment_activity_item.html.eex

@@ -1,73 +1,4 @@
 <%= case @activity.subject do %>
-    <% :discussion_created -> %>
-        <%= 
-            dgettext("activity", "%{profile} created the discussion %{discussion}.", 
-                %{
-                    profile: "<b>#{Mobilizon.Actors.Actor.display_name_and_username(@activity.author)}</b>",
-                    discussion: "<a href=\"#{
-                        page_url(
-                            Mobilizon.Web.Endpoint,
-                            :discussion,
-                            Mobilizon.Actors.Actor.preferred_username_and_domain(@activity.group), @activity.subject_params["discussion_slug"]
-                        ) |> URI.decode()}\">
-                            #{@activity.subject_params["discussion_title"]}
-                        </a>"
-                }
-            ) |> raw %>
-    <% :discussion_replied -> %>
-    <%= 
-            dgettext("activity", "%{profile} replied to the discussion %{discussion}.", 
-                %{
-                    profile: "<b>#{Mobilizon.Actors.Actor.display_name_and_username(@activity.author)}</b>",
-                    discussion: "<a href=\"#{
-                        page_url(
-                            Mobilizon.Web.Endpoint,
-                            :discussion,
-                            Mobilizon.Actors.Actor.preferred_username_and_domain(@activity.group), @activity.subject_params["discussion_slug"]
-                        ) |> URI.decode()}\">
-                            #{@activity.subject_params["discussion_title"]}
-                        </a>"
-                }
-            ) |> raw %>
-    <% :discussion_renamed -> %>
-    <%= 
-            dgettext("activity", "%{profile} renamed the discussion %{discussion}.", 
-                %{
-                    profile: "<b>#{Mobilizon.Actors.Actor.display_name_and_username(@activity.author)}</b>",
-                    discussion: "<a href=\"#{
-                        page_url(
-                            Mobilizon.Web.Endpoint,
-                            :discussion,
-                            Mobilizon.Actors.Actor.preferred_username_and_domain(@activity.group), @activity.subject_params["discussion_slug"]
-                        ) |> URI.decode()}\">
-                            #{@activity.subject_params["discussion_title"]}
-                        </a>"
-                }
-            ) |> raw %>
-    <% :discussion_archived -> %>
-    <%= 
-            dgettext("activity", "%{profile} archived the discussion %{discussion}.", 
-                %{
-                    profile: "<b>#{Mobilizon.Actors.Actor.display_name_and_username(@activity.author)}</b>",
-                    discussion: "<a href=\"#{
-                        page_url(
-                            Mobilizon.Web.Endpoint,
-                            :discussion,
-                            Mobilizon.Actors.Actor.preferred_username_and_domain(@activity.group), @activity.subject_params["discussion_slug"]
-                        ) |> URI.decode()}\">
-                            #{@activity.subject_params["discussion_title"]}
-                        </a>"
-                }
-            ) |> raw %>
-    <% :discussion_deleted -> %>
-    <%= 
-            dgettext("activity", "%{profile} deleted the discussion %{discussion}.", 
-                %{
-                    profile: "<b>#{Mobilizon.Actors.Actor.display_name_and_username(@activity.author)}</b>",
-                    discussion: "<b>#{@activity.subject_params["discussion_title"]}</b>"
-                }
-            ) |> raw %>
-
     <% :event_comment_mention -> %>
     <%= 
             dgettext("activity", "%{profile} mentionned you in a comment under event %{event}.", 

lib/web/templates/email/activity/_comment_activity_item.text.eex

@@ -22,19 +22,19 @@
         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}.", 
+<%= 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}.", 
+<%= 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}.",
     %{
         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() %><% :participation_event_comment -> %><%= dgettext("activity", "%{profile} has posted an announcement under event %{event}.", 
+<%= page_url(Mobilizon.Web.Endpoint, :event, @activity.subject_params["event_uuid"]) |> URI.decode() %><% :participation_event_comment -> %><%= dgettext("activity", "%{profile} has posted an announcement under event %{event}.",
     %{
         profile: Mobilizon.Actors.Actor.display_name_and_username(@activity.author),
         event: @activity.subject_params["event_title"]

lib/web/templates/email/email.html.eex

@@ -110,11 +110,11 @@
           <table border="0" cellpadding="0" cellspacing="0" width="100%" style="max-width: 600px;" >
             <!-- HEADLINE -->
             <tr>
-              <td bgcolor="#757199" align="center" style="padding: 30px 30px 30px 30px; border-radius: 4px 4px 4px 4px; color: #3A384C; font-family: 'Roboto', Helvetica, Arial, sans-serif; font-size: 18px; font-weight: 400; line-height: 25px;" >
-                <h2 style="font-size: 20px; font-weight: 400; color: #3A384C; margin: 0;">
+              <td bgcolor="#474467" align="center" style="padding: 30px 30px 30px 30px; border-radius: 4px 4px 4px 4px; color: #3A384C; font-family: 'Roboto', Helvetica, Arial, sans-serif; font-size: 18px; font-weight: 400; line-height: 25px;" >
+                <h2 style="font-size: 20px; font-weight: 400; color: #FFFFFF; margin: 0;">
                   <%= gettext "Need help? Is something not working as expected?" %>
                 </h2>
-                <p style="margin: 0;"><a href="https://framacolibri.org/c/mobilizon/test-mobilizon" target="_blank" style="color: #474467;">
+                <p style="margin: 0;"><a href="https://framacolibri.org/c/mobilizon/test-mobilizon" target="_blank" style="color: #FFFFFF;">
                     <%= gettext "Ask the community on Framacolibri" %>
                   </a></p>
               </td>
@@ -140,7 +140,7 @@
             <tr>
               <td bgcolor="#ECEBF2" align="center" style="padding: 30px 30px 30px 30px; color: #474467; font-family: 'Roboto', Helvetica, Arial, sans-serif; font-size: 14px; font-weight: 400; line-height: 18px;" >
                 <p style="margin: 0;">
-                  <%= gettext("<b>%{instance}</b> is powered by Mobilizon.", instance: @instance[:name]) |> raw %>
+                  <%= gettext("<b>%{instance}</b> is powered by Mobilizon.", instance: @instance[:name]) |> raw %><br />
                   <a href="https://joinmobilizon.org"><%= gettext "Learn more about Mobilizon here!" %></a>
                 </p>
               </td>

lib/web/templates/email/email_direct_activity.html.eex

@@ -35,7 +35,16 @@
       <tr>
         <td bgcolor="#ffffff" align="left" style="padding: 20px 5% 0px; color: #474467; font-family: 'Roboto', Helvetica, Arial, sans-serif; font-size: 18px; font-weight: 400; line-height: 25px;" >
           <p style="margin: 0;">
-            <%= dngettext("activity", "There has been an activity!", "There has been some activity!", @total_number_activities) %>
+            <%= case @recap do %>
+              <% :one_day -> %>
+                <% dgettext("activity", "Here's your daily activity recap") %>
+              <% :one_week -> %>
+                <% dgettext("activity", "Here's your weekly activity recap") %>
+              <% :one_hour -> %>
+                <%= dngettext("activity", "There has been an activity!", "There has been some activity!", @total_number_activities) %>
+              <% false -> %>
+                <%= dngettext("activity", "There has been an activity!", "There has been some activity!", @total_number_activities) %>
+            <% end %>
           </p>
         </td>
       </tr>
@@ -108,7 +117,9 @@
                           <% end %>
                         </p>
                         <%= unless @single_activity do %>
-                        <em><%= datetime_relative(activity.inserted_at, @locale) %></em>
+                        <em>
+                          <%= datetime_to_string(activity.inserted_at, @locale, :short) %>
+                        </em>
                         <% end %>
                       </li>
                       <% end %>
@@ -141,7 +152,7 @@
       <tr>
         <td bgcolor="#ffffff" align="left" style="padding: 20px 30px 40px 30px; color: #474467; font-family: 'Roboto', Helvetica, Arial, sans-serif; font-size: 14px; font-weight: 400; line-height: 20px;" >
           <p style="margin: 0">
-            <%= dgettext "activity", "Don't want to receive activity notifications? You may change frequency or disable them in your settings." %>
+            <%= dgettext("activity", "Don't want to receive activity notifications? You may change frequency or disable them in %{tag_start}your settings%{tag_end}.", %{tag_start: "<a href=\"#{Mobilizon.Web.Endpoint.url()}/settings/notifications\">", tag_end: "</a>"}) |> raw %>
           </p>
         </td>
       </tr>

lib/web/templates/email/email_direct_activity.text.eex

@@ -1,7 +1,11 @@
 <%= @subject %>
 
 ==
-<%= dngettext("activity", "There has been an activity!", "There has been some activity!", @total_number_activities) %>
+<%= case @recap do %><% :one_day -> %><% dgettext("activity", "Here's your daily activity recap") %>
+<% :one_week -> %><% dgettext("activity", "Here's your weekly activity recap") %>
+<% :one_hour -> %><%= dngettext("activity", "There has been an activity!", "There has been some activity!", @total_number_activities) %>
+<% false -> %><%= dngettext("activity", "There has been an activity!", "There has been some activity!", @total_number_activities) %>
+<% end %>
 
 <%= for {_, group_activities} <- @activities do %>
 
@@ -13,11 +17,12 @@
 <%= for activity <- Enum.take(group_activities, 5) do %>
 * <%= case activity.type do %><% :discussion -> %><%= render("activity/_discussion_activity_item.text", activity: activity) %><% :event -> %><%= render("activity/_event_activity_item.text", activity: activity) %><% :group -> %><%= render("activity/_group_activity_item.text", activity: activity) %>
 <% :member -> %><%= render("activity/_member_activity_item.text", activity: activity) %><% :post -> %><%= render("activity/_post_activity_item.text", activity: activity) %><% :resource -> %><%= render("activity/_resource_activity_item.text", activity: activity) %><% :comment -> %><%= render("activity/_comment_activity_item.text", activity: activity) %><% end %>
-<%= unless @single_activity do %><%= datetime_relative(activity.inserted_at, @locale) %><% end %>
+<%= unless @single_activity do %><%= datetime_to_string(activity.inserted_at, @locale, :short) %><% end %>
 <% end %>
 <%= if length(group_activities) > 5 do %>
 <%= dngettext "activity", "View one more activity", "View %{count} more activities", length(group_activities) - 5, %{count: length(group_activities) - 5} %>
 <%= page_url(Mobilizon.Web.Endpoint, :actor, Mobilizon.Actors.Actor.preferred_username_and_domain(hd(group_activities).group)) |> URI.decode() %>/timeline
 <% end %>
 <% end %>
-<%= dgettext("activity", "Don't want to receive activity notifications? You may change frequency or disable them in your settings.") %>
+<%= dgettext("activity", "Don't want to receive activity notifications? You may change frequency or disable them in your settings.") %>
+<% "#{Mobilizon.Web.Endpoint.url()}/settings/notifications" %>