Prevent email reports when consolidated view ineligible (#5882)

* do not send email reports if consolidated view not ok to display

* fix CE

* more expressive condition in ok_to_send?

* Map.get -> Map.fetch

Author: RobertJoonas

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

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?(notification.site) 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

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: ["[email protected]"])
+
+        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: ["[email protected]"])
+
+        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

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 =~ "<b>2</b> visitors on <b>/a</b>"
         assert html_body =~ "<b>1</b> visitor on <b>/b</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: ["[email protected]"]
+        )
+
+        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