Fix email report sentry errors (#4381)

* Fix missing email param on email unsubscribe

* Guard against email report being deleted

lib/plausible_web/controllers/unsubscribe_controller.ex

@@ -22,6 +22,10 @@ defmodule PlausibleWeb.UnsubscribeController do
     )
   end
 
+  def weekly_report(conn, _) do
+    render_error(conn, 400)
+  end
+
   def monthly_report(conn, %{"website" => website, "email" => email}) do
     site = Repo.get_by(Plausible.Site, domain: website)
     monthly_report = site && Repo.get_by(MonthlyReport, site_id: site.id)
@@ -40,4 +44,8 @@ defmodule PlausibleWeb.UnsubscribeController do
       layout: {PlausibleWeb.LayoutView, "focus.html"}
     )
   end
+
+  def monthly_report(conn, _) do
+    render_error(conn, 400)
+  end
 end

lib/workers/send_email_report.ex

@@ -7,7 +7,7 @@ defmodule Plausible.Workers.SendEmailReport do
   def perform(%Oban.Job{args: %{"interval" => "weekly", "site_id" => site_id}}) do
     site = Repo.get(Plausible.Site, site_id) |> Repo.preload(:weekly_report)
 
-    if site do
+    if site && site.weekly_report do
       %{site: site}
       |> Map.put(:type, :weekly)
       |> Map.put(:name, "Weekly")
@@ -23,7 +23,7 @@ defmodule Plausible.Workers.SendEmailReport do
   def perform(%Oban.Job{args: %{"interval" => "monthly", "site_id" => site_id}}) do
     site = Repo.get(Plausible.Site, site_id) |> Repo.preload(:monthly_report)
 
-    if site do
+    if site && site.monthly_report do
       %{site: site}
       |> Map.put(:type, :monthly)
       |> put_last_month_query()

test/plausible_web/controllers/unsubscribe_controller_test.exs

@@ -24,6 +24,13 @@ defmodule PlausibleWeb.UnsubscribeControllerTest do
 
       assert html_response(conn, 200) =~ "Unsubscribe successful"
     end
+
+    test "renders failure if email parameter not provided", %{conn: conn} do
+      conn =
+        get(conn, "/sites/nonexistent.com/weekly-report/unsubscribe")
+
+      assert html_response(conn, 400) =~ "Bad Request"
+    end
   end
 
   describe "GET /sites/:website/monthly-report/unsubscribe" do
@@ -46,5 +53,12 @@ defmodule PlausibleWeb.UnsubscribeControllerTest do
 
       assert html_response(conn, 200) =~ "Unsubscribe successful"
     end
+
+    test "renders failure if email parameter not provided", %{conn: conn} do
+      conn =
+        get(conn, "/sites/nonexistent.com/monthly-report/unsubscribe")
+
+      assert html_response(conn, 400) =~ "Bad Request"
+    end
   end
 end

test/workers/send_email_report_test.exs

@@ -32,6 +32,13 @@ defmodule Plausible.Workers.SendEmailReportTest do
                perform_job(SendEmailReport, %{"site_id" => 28_378_237, "interval" => "weekly"})
     end
 
+    test "does not crash when weekly report has been deleted since scheduling job" do
+      site = insert(:site, domain: "test-site.com", timezone: "US/Eastern")
+
+      assert :discard =
+               perform_job(SendEmailReport, %{"site_id" => site.id, "interval" => "weekly"})
+    end
+
     test "calculates timezone correctly" do
       site =
         insert(:site,
@@ -229,5 +236,12 @@ defmodule Plausible.Workers.SendEmailReportTest do
         to: [nil: "user2@email.com"]
       )
     end
+
+    test "does not crash when monthly report has been deleted since scheduling job" do
+      site = insert(:site, domain: "test-site.com", timezone: "US/Eastern")
+
+      assert :discard =
+               perform_job(SendEmailReport, %{"site_id" => site.id, "interval" => "monthly"})
+    end
   end
 end