Fix unique constraint validation on weekly/monthly reports (#3200)

* Fix unique constraint validation on weekly/monthly reports

* Undo this annoying lsp formatter race condition

lib/plausible/site/monthly_report.ex

@@ -13,7 +13,7 @@ defmodule Plausible.Site.MonthlyReport do
     settings
     |> cast(attrs, [:site_id, :recipients])
     |> validate_required([:site_id, :recipients])
-    |> unique_constraint(:site)
+    |> unique_constraint(:site_id)
   end
 
   def add_recipient(report, recipient) do

lib/plausible/site/weekly_report.ex

@@ -13,7 +13,7 @@ defmodule Plausible.Site.WeeklyReport do
     settings
     |> cast(attrs, [:site_id, :recipients])
     |> validate_required([:site_id, :recipients])
-    |> unique_constraint(:site)
+    |> unique_constraint(:site_id)
   end
 
   def add_recipient(report, recipient) do

lib/plausible_web/controllers/site_controller.ex

@@ -452,11 +452,14 @@ defmodule PlausibleWeb.SiteController do
   def enable_weekly_report(conn, _params) do
     site = conn.assigns[:site]
 
-    Plausible.Site.WeeklyReport.changeset(%Plausible.Site.WeeklyReport{}, %{
-      site_id: site.id,
-      recipients: [conn.assigns[:current_user].email]
-    })
-    |> Repo.insert!()
+    result =
+      Plausible.Site.WeeklyReport.changeset(%Plausible.Site.WeeklyReport{}, %{
+        site_id: site.id,
+        recipients: [conn.assigns[:current_user].email]
+      })
+      |> Repo.insert()
+
+    :ok = tolerate_unique_contraint_violation(result, "weekly_reports_site_id_index")
 
     conn
     |> put_flash(:success, "You will receive an email report every Monday going forward")
@@ -502,11 +505,15 @@ defmodule PlausibleWeb.SiteController do
   def enable_monthly_report(conn, _params) do
     site = conn.assigns[:site]
 
-    Plausible.Site.MonthlyReport.changeset(%Plausible.Site.MonthlyReport{}, %{
-      site_id: site.id,
-      recipients: [conn.assigns[:current_user].email]
-    })
-    |> Repo.insert!()
+    result =
+      %Plausible.Site.MonthlyReport{}
+      |> Plausible.Site.MonthlyReport.changeset(%{
+        site_id: site.id,
+        recipients: [conn.assigns[:current_user].email]
+      })
+      |> Repo.insert()
+
+    :ok = tolerate_unique_contraint_violation(result, "monthly_reports_site_id_index")
 
     conn
     |> put_flash(:success, "You will receive an email report every month going forward")
@@ -966,4 +973,22 @@ defmodule PlausibleWeb.SiteController do
       layout: {PlausibleWeb.LayoutView, "focus.html"}
     )
   end
+
+  defp tolerate_unique_contraint_violation(result, name) do
+    case result do
+      {:ok, _} ->
+        :ok
+
+      {:error,
+       %{
+         errors: [
+           site_id: {_, [constraint: :unique, constraint_name: ^name]}
+         ]
+       }} ->
+        :ok
+
+      other ->
+        other
+    end
+  end
 end

test/plausible_web/controllers/site_controller_test.exs

@@ -852,8 +852,22 @@ defmodule PlausibleWeb.SiteControllerTest do
       conn: conn,
       site: site,
       user: user
+    } do
+      conn = post(conn, "/sites/#{site.domain}/weekly-report/enable")
+      assert Phoenix.Flash.get(conn.assigns.flash, :success) =~ "You will receive an email report"
+
+      report = Repo.get_by(Plausible.Site.WeeklyReport, site_id: site.id)
+      assert report.recipients == [user.email]
+    end
+
+    test "creates a weekly report record twice (e.g. from a second tab)", %{
+      conn: conn,
+      site: site,
+      user: user
     } do
       post(conn, "/sites/#{site.domain}/weekly-report/enable")
+      conn = post(conn, "/sites/#{site.domain}/weekly-report/enable")
+      assert Phoenix.Flash.get(conn.assigns.flash, :success) =~ "You will receive an email report"
 
       report = Repo.get_by(Plausible.Site.WeeklyReport, site_id: site.id)
       assert report.recipients == [user.email]
@@ -929,10 +943,24 @@ defmodule PlausibleWeb.SiteControllerTest do
       site: site,
       user: user
     } do
-      post(conn, "/sites/#{site.domain}/monthly-report/enable")
+      conn = post(conn, "/sites/#{site.domain}/monthly-report/enable")
 
       report = Repo.get_by(Plausible.Site.MonthlyReport, site_id: site.id)
       assert report.recipients == [user.email]
+      assert Phoenix.Flash.get(conn.assigns.flash, :success) =~ "You will receive an email report"
+    end
+
+    test "enable monthly report twice (e.g. from a second tab)", %{
+      conn: conn,
+      site: site,
+      user: user
+    } do
+      post(conn, "/sites/#{site.domain}/monthly-report/enable")
+      conn = post(conn, "/sites/#{site.domain}/monthly-report/enable")
+
+      assert Phoenix.Flash.get(conn.assigns.flash, :success) =~ "You will receive an email report"
+      report = Repo.get_by(Plausible.Site.MonthlyReport, site_id: site.id)
+      assert report.recipients == [user.email]
     end
   end