From c04e9286dd9b6594dee14a53c7a43f20a5d999a6 Mon Sep 17 00:00:00 2001
From: hq1 <hq@mtod.org>
Date: Mon, 10 Apr 2023 10:51:36 +0200
Subject: [PATCH] Associate goals with sites, not domains (#2828)

* Revert "Rephrase error message"

This reverts commit f624443a96f27d5d448f3f5ee2bbb8d16f6371e0.

* Revert "Temporarily disable goal creation"

This reverts commit a091635b9d2dec65f482e461d04ca05bfd17ac4a.

* Update ecto schema

* Make sure goal operations are per site

* Update tests

* Split postgres migrations
---
 lib/plausible/goal/schema.ex                  | 13 +++-
 lib/plausible/goals.ex                        | 44 ++++++-----
 lib/plausible/sites.ex                        |  2 +-
 lib/plausible/stats/breakdown.ex              |  4 +-
 lib/plausible/stats/filter_suggestions.ex     |  4 +-
 .../api/external_sites_controller.ex          |  7 +-
 .../controllers/site_controller.ex            |  4 +-
 ...20230410070312_fixup_goals_sites_assoc.exs | 10 +++
 test/plausible/goals_test.exs                 | 30 ++++++--
 .../api/external_sites_controller_test.exs    |  9 +--
 .../breakdown_test.exs                        |  6 +-
 .../api/stats_controller/conversions_test.exs | 74 +++++++++----------
 .../controllers/site_controller_test.exs      | 10 +--
 .../controllers/stats_controller_test.exs     |  2 +-
 14 files changed, 127 insertions(+), 92 deletions(-)
 create mode 100644 priv/repo/migrations/20230410070312_fixup_goals_sites_assoc.exs

diff --git a/lib/plausible/goal/schema.ex b/lib/plausible/goal/schema.ex
index d074d4a8a8..c26954ac9c 100644
--- a/lib/plausible/goal/schema.ex
+++ b/lib/plausible/goal/schema.ex
@@ -6,9 +6,12 @@ defimpl Jason.Encoder, for: Plausible.Goal do
         value.page_path -> :page
       end
 
+    domain = value.site.domain
+
     value
     |> Map.put(:goal_type, goal_type)
-    |> Map.take([:id, :domain, :goal_type, :event_name, :page_path])
+    |> Map.take([:id, :goal_type, :event_name, :page_path])
+    |> Map.put(:domain, domain)
     |> Jason.Encode.map(opts)
   end
 end
@@ -18,17 +21,19 @@ defmodule Plausible.Goal do
   import Ecto.Changeset
 
   schema "goals" do
-    field :domain, :string
     field :event_name, :string
     field :page_path, :string
 
+    belongs_to :site, Plausible.Site
+
     timestamps()
   end
 
   def changeset(goal, attrs \\ %{}) do
     goal
-    |> cast(attrs, [:domain, :event_name, :page_path])
-    |> validate_required([:domain])
+    |> cast(attrs, [:site_id, :event_name, :page_path])
+    |> validate_required([:site_id])
+    |> cast_assoc(:site)
     |> validate_event_name_and_page_path()
     |> update_change(:event_name, &String.trim/1)
     |> update_change(:page_path, &String.trim/1)
diff --git a/lib/plausible/goals.ex b/lib/plausible/goals.ex
index 9c25c27109..44a43e21ec 100644
--- a/lib/plausible/goals.ex
+++ b/lib/plausible/goals.ex
@@ -3,22 +3,23 @@ defmodule Plausible.Goals do
   alias Plausible.Goal
 
   def create(site, params) do
-    params = Map.merge(params, %{"domain" => site.domain})
+    params = Map.merge(params, %{"site_id" => site.id})
 
-    Goal.changeset(%Goal{}, params)
-    |> Ecto.Changeset.add_error(
-      :event_name,
-      "Sorry! Due to the ongoing maintenance, adding new goals is currently paused. Please try again in one hour. Thanks for your patience!"
-    )
-    |> Ecto.Changeset.add_error(
-      :page_path,
-      "Sorry! Due to the ongoing maintenance, adding new goals is currently paused. Please try again in one hour. Thanks for your patience!"
-    )
-    |> Repo.insert()
+    case Repo.insert(Goal.changeset(%Goal{}, params)) do
+      {:ok, goal} -> {:ok, Repo.preload(goal, :site)}
+      error -> error
+    end
   end
 
   def find_or_create(site, %{"goal_type" => "event", "event_name" => event_name}) do
-    goal = Repo.get_by(Plausible.Goal, domain: site.domain, event_name: event_name)
+    query =
+      from g in Goal,
+        inner_join: assoc(g, :site),
+        where: g.site_id == ^site.id,
+        where: g.event_name == ^event_name,
+        preload: [:site]
+
+    goal = Repo.one(query)
 
     case goal do
       nil -> create(site, %{"event_name" => event_name})
@@ -29,7 +30,14 @@ defmodule Plausible.Goals do
   def find_or_create(_, %{"goal_type" => "event"}), do: {:missing, "event_name"}
 
   def find_or_create(site, %{"goal_type" => "page", "page_path" => page_path}) do
-    goal = Repo.get_by(Plausible.Goal, domain: site.domain, page_path: page_path)
+    query =
+      from g in Goal,
+        inner_join: assoc(g, :site),
+        where: g.site_id == ^site.id,
+        where: g.page_path == ^page_path,
+        preload: [:site]
+
+    goal = Repo.one(query)
 
     case goal do
       nil -> create(site, %{"page_path" => page_path})
@@ -39,21 +47,23 @@ defmodule Plausible.Goals do
 
   def find_or_create(_, %{"goal_type" => "page"}), do: {:missing, "page_path"}
 
-  def for_domain(domain) do
+  def for_site(site) do
     query =
       from g in Goal,
-        where: g.domain == ^domain
+        inner_join: assoc(g, :site),
+        where: g.site_id == ^site.id,
+        preload: [:site]
 
     query
     |> Repo.all()
     |> Enum.map(&maybe_trim/1)
   end
 
-  def delete(id, domain) do
+  def delete(id, site) do
     case Repo.delete_all(
            from g in Goal,
              where: g.id == ^id,
-             where: g.domain == ^domain
+             where: g.site_id == ^site.id
          ) do
       {1, _} -> :ok
       {0, _} -> {:error, :not_found}
diff --git a/lib/plausible/sites.ex b/lib/plausible/sites.ex
index c7743ebe6f..3508e1aacd 100644
--- a/lib/plausible/sites.ex
+++ b/lib/plausible/sites.ex
@@ -115,7 +115,7 @@ defmodule Plausible.Sites do
   def has_goals?(site) do
     Repo.exists?(
       from g in Plausible.Goal,
-        where: g.domain == ^site.domain
+        where: g.site_id == ^site.id
     )
   end
 
diff --git a/lib/plausible/stats/breakdown.ex b/lib/plausible/stats/breakdown.ex
index f06b44e9cd..ab52a80715 100644
--- a/lib/plausible/stats/breakdown.ex
+++ b/lib/plausible/stats/breakdown.ex
@@ -13,8 +13,8 @@ defmodule Plausible.Stats.Breakdown do
 
   def breakdown(site, query, "event:goal" = property, metrics, pagination) do
     {event_goals, pageview_goals} =
-      site.domain
-      |> Goals.for_domain()
+      site
+      |> Goals.for_site()
       |> Enum.split_with(fn goal -> goal.event_name end)
 
     events = Enum.map(event_goals, & &1.event_name)
diff --git a/lib/plausible/stats/filter_suggestions.ex b/lib/plausible/stats/filter_suggestions.ex
index 68c460406e..1efe570e6e 100644
--- a/lib/plausible/stats/filter_suggestions.ex
+++ b/lib/plausible/stats/filter_suggestions.ex
@@ -118,8 +118,8 @@ defmodule Plausible.Stats.FilterSuggestions do
   end
 
   def filter_suggestions(site, _query, "goal", filter_search) do
-    site.domain
-    |> Plausible.Goals.for_domain()
+    site
+    |> Plausible.Goals.for_site()
     |> Enum.map(fn x -> if x.event_name, do: x.event_name, else: "Visit #{x.page_path}" end)
     |> Enum.filter(fn goal ->
       String.contains?(
diff --git a/lib/plausible_web/controllers/api/external_sites_controller.ex b/lib/plausible_web/controllers/api/external_sites_controller.ex
index 78106a849a..96d1398ad5 100644
--- a/lib/plausible_web/controllers/api/external_sites_controller.ex
+++ b/lib/plausible_web/controllers/api/external_sites_controller.ex
@@ -124,11 +124,6 @@ defmodule PlausibleWeb.Api.ExternalSitesController do
       {:missing, param} ->
         H.bad_request(conn, "Parameter `#{param}` is required to create a goal")
 
-      {:error, changeset} ->
-        conn
-        |> put_status(400)
-        |> json(serialize_errors(changeset))
-
       e ->
         H.bad_request(conn, "Something went wrong: #{inspect(e)}")
     end
@@ -139,7 +134,7 @@ defmodule PlausibleWeb.Api.ExternalSitesController do
          {:ok, goal_id} <- expect_param_key(params, "goal_id"),
          site when not is_nil(site) <-
            Sites.get_for_user(conn.assigns[:current_user].id, site_id, [:owner, :admin]) do
-      case Goals.delete(goal_id, site.domain) do
+      case Goals.delete(goal_id, site) do
         :ok ->
           json(conn, %{"deleted" => true})
 
diff --git a/lib/plausible_web/controllers/site_controller.ex b/lib/plausible_web/controllers/site_controller.ex
index 8fa10af8c1..2fceccd4c8 100644
--- a/lib/plausible_web/controllers/site_controller.ex
+++ b/lib/plausible_web/controllers/site_controller.ex
@@ -158,7 +158,7 @@ defmodule PlausibleWeb.SiteController do
   end
 
   def delete_goal(conn, %{"id" => goal_id}) do
-    case Plausible.Goals.delete(goal_id, conn.assigns[:site].domain) do
+    case Plausible.Goals.delete(goal_id, conn.assigns[:site]) do
       :ok ->
         conn
         |> put_flash(:success, "Goal deleted successfully")
@@ -225,7 +225,7 @@ defmodule PlausibleWeb.SiteController do
 
   def settings_goals(conn, _params) do
     site = conn.assigns[:site] |> Repo.preload(:custom_domain)
-    goals = Goals.for_domain(site.domain)
+    goals = Goals.for_site(site)
 
     conn
     |> assign(:skip_plausible_tracking, true)
diff --git a/priv/repo/migrations/20230410070312_fixup_goals_sites_assoc.exs b/priv/repo/migrations/20230410070312_fixup_goals_sites_assoc.exs
new file mode 100644
index 0000000000..42f493c3d0
--- /dev/null
+++ b/priv/repo/migrations/20230410070312_fixup_goals_sites_assoc.exs
@@ -0,0 +1,10 @@
+defmodule Plausible.Repo.Migrations.FixupGoalsSitesAssoc do
+  use Ecto.Migration
+
+  def change do
+    alter table(:goals) do
+      modify :site_id, references(:sites, on_delete: :delete_all), null: false
+      remove :domain
+    end
+  end
+end
diff --git a/test/plausible/goals_test.exs b/test/plausible/goals_test.exs
index 18facbb2ba..28cc38a240 100644
--- a/test/plausible/goals_test.exs
+++ b/test/plausible/goals_test.exs
@@ -3,7 +3,6 @@ defmodule Plausible.GoalsTest do
 
   alias Plausible.Goals
 
-  @tag :skip
   test "create/2 trims input" do
     site = insert(:site)
     {:ok, goal} = Goals.create(site, %{"page_path" => "/foo bar "})
@@ -13,13 +12,34 @@ defmodule Plausible.GoalsTest do
     assert goal.event_name == "some event name"
   end
 
-  test "for_domain/2 returns trimmed input even if it was saved with trailing whitespace" do
+  test "for_site2 returns trimmed input even if it was saved with trailing whitespace" do
     site = insert(:site)
-    insert(:goal, %{domain: site.domain, event_name: " Signup "})
-    insert(:goal, %{domain: site.domain, page_path: " /Signup "})
+    insert(:goal, %{site: site, event_name: " Signup "})
+    insert(:goal, %{site: site, page_path: " /Signup "})
 
-    goals = Goals.for_domain(site.domain)
+    goals = Goals.for_site(site)
 
     assert [%{event_name: "Signup"}, %{page_path: "/Signup"}] = goals
   end
+
+  @tag :v2_only
+  test "goals are present after domain change" do
+    site = insert(:site)
+    insert(:goal, %{site: site, event_name: " Signup "})
+    insert(:goal, %{site: site, page_path: " /Signup "})
+
+    {:ok, site} = Plausible.Site.Domain.change(site, "goals.example.com")
+
+    assert [_, _] = Goals.for_site(site)
+  end
+
+  test "goals are removed when site is deleted" do
+    site = insert(:site)
+    insert(:goal, %{site: site, event_name: " Signup "})
+    insert(:goal, %{site: site, page_path: " /Signup "})
+
+    Plausible.Site.Removal.run(site.domain)
+
+    assert [] = Goals.for_site(site)
+  end
 end
diff --git a/test/plausible_web/controllers/api/external_sites_controller_test.exs b/test/plausible_web/controllers/api/external_sites_controller_test.exs
index 9a865a1aa0..0a73e8466d 100644
--- a/test/plausible_web/controllers/api/external_sites_controller_test.exs
+++ b/test/plausible_web/controllers/api/external_sites_controller_test.exs
@@ -245,7 +245,6 @@ defmodule PlausibleWeb.Api.ExternalSitesControllerTest do
   describe "PUT /api/v1/sites/goals" do
     setup :create_site
 
-    @tag :skip
     test "can add a goal as event to a site", %{conn: conn, site: site} do
       conn =
         put(conn, "/api/v1/sites/goals", %{
@@ -257,9 +256,9 @@ defmodule PlausibleWeb.Api.ExternalSitesControllerTest do
       res = json_response(conn, 200)
       assert res["goal_type"] == "event"
       assert res["event_name"] == "Signup"
+      assert res["domain"] == site.domain
     end
 
-    @tag :skip
     test "can add a goal as page to a site", %{conn: conn, site: site} do
       conn =
         put(conn, "/api/v1/sites/goals", %{
@@ -271,10 +270,10 @@ defmodule PlausibleWeb.Api.ExternalSitesControllerTest do
       res = json_response(conn, 200)
       assert res["goal_type"] == "page"
       assert res["page_path"] == "/signup"
+      assert res["domain"] == site.domain
     end
 
     @tag :v2_only
-    @tag :skip
     test "can add a goal using old site_id after domain change", %{conn: conn, site: site} do
       old_domain = site.domain
       new_domain = "new.example.com"
@@ -291,9 +290,9 @@ defmodule PlausibleWeb.Api.ExternalSitesControllerTest do
       res = json_response(conn, 200)
       assert res["goal_type"] == "event"
       assert res["event_name"] == "Signup"
+      assert res["domain"] == new_domain
     end
 
-    @tag :skip
     test "is idempotent find or create op", %{conn: conn, site: site} do
       conn =
         put(conn, "/api/v1/sites/goals", %{
@@ -394,7 +393,6 @@ defmodule PlausibleWeb.Api.ExternalSitesControllerTest do
     end
   end
 
-  @tag :skip
   describe "DELETE /api/v1/sites/goals/:goal_id" do
     setup :create_new_site
 
@@ -417,7 +415,6 @@ defmodule PlausibleWeb.Api.ExternalSitesControllerTest do
     end
 
     @tag :v2_only
-    @tag :skip
     test "delete a goal using old site_id after domain change", %{conn: conn, site: site} do
       old_domain = site.domain
       new_domain = "new.example.com"
diff --git a/test/plausible_web/controllers/api/external_stats_controller/breakdown_test.exs b/test/plausible_web/controllers/api/external_stats_controller/breakdown_test.exs
index ccd5a67172..f60d3d762b 100644
--- a/test/plausible_web/controllers/api/external_stats_controller/breakdown_test.exs
+++ b/test/plausible_web/controllers/api/external_stats_controller/breakdown_test.exs
@@ -1061,9 +1061,9 @@ defmodule PlausibleWeb.Api.ExternalStatsController.BreakdownTest do
 
   describe "breakdown by event:goal" do
     test "custom properties from custom events are returned", %{conn: conn, site: site} do
-      insert(:goal, %{domain: site.domain, event_name: "404"})
-      insert(:goal, %{domain: site.domain, event_name: "Purchase"})
-      insert(:goal, %{domain: site.domain, page_path: "/test"})
+      insert(:goal, %{site: site, event_name: "404"})
+      insert(:goal, %{site: site, event_name: "Purchase"})
+      insert(:goal, %{site: site, page_path: "/test"})
 
       populate_stats(site, [
         build(:pageview,
diff --git a/test/plausible_web/controllers/api/stats_controller/conversions_test.exs b/test/plausible_web/controllers/api/stats_controller/conversions_test.exs
index ef2d482b36..e6a0626c25 100644
--- a/test/plausible_web/controllers/api/stats_controller/conversions_test.exs
+++ b/test/plausible_web/controllers/api/stats_controller/conversions_test.exs
@@ -27,8 +27,8 @@ defmodule PlausibleWeb.Api.StatsController.ConversionsTest do
         )
       ])
 
-      insert(:goal, %{domain: site.domain, page_path: "/register"})
-      insert(:goal, %{domain: site.domain, event_name: "Signup"})
+      insert(:goal, %{site: site, page_path: "/register"})
+      insert(:goal, %{site: site, event_name: "Signup"})
 
       conn = get(conn, "/api/stats/#{site.domain}/conversions?period=day")
 
@@ -76,7 +76,7 @@ defmodule PlausibleWeb.Api.StatsController.ConversionsTest do
         )
       ])
 
-      insert(:goal, %{domain: site.domain, event_name: "Payment"})
+      insert(:goal, %{site: site, event_name: "Payment"})
 
       filters = Jason.encode!(%{props: %{"logged_in" => "true"}})
       conn = get(conn, "/api/stats/#{site.domain}/conversions?period=day&filters=#{filters}")
@@ -117,7 +117,7 @@ defmodule PlausibleWeb.Api.StatsController.ConversionsTest do
         build(:event, name: "Payment")
       ])
 
-      insert(:goal, %{domain: site.domain, event_name: "Payment"})
+      insert(:goal, %{site: site, event_name: "Payment"})
 
       filters = Jason.encode!(%{props: %{"logged_in" => "!true"}})
       conn = get(conn, "/api/stats/#{site.domain}/conversions?period=day&filters=#{filters}")
@@ -156,7 +156,7 @@ defmodule PlausibleWeb.Api.StatsController.ConversionsTest do
         build(:event, name: "Payment")
       ])
 
-      insert(:goal, %{domain: site.domain, event_name: "Payment"})
+      insert(:goal, %{site: site, event_name: "Payment"})
 
       filters = Jason.encode!(%{props: %{"logged_in" => "(none)"}})
       conn = get(conn, "/api/stats/#{site.domain}/conversions?period=day&filters=#{filters}")
@@ -197,7 +197,7 @@ defmodule PlausibleWeb.Api.StatsController.ConversionsTest do
         build(:event, name: "Payment")
       ])
 
-      insert(:goal, %{domain: site.domain, event_name: "Payment"})
+      insert(:goal, %{site: site, event_name: "Payment"})
 
       filters = Jason.encode!(%{props: %{"logged_in" => "!(none)"}})
       conn = get(conn, "/api/stats/#{site.domain}/conversions?period=day&filters=#{filters}")
@@ -227,8 +227,8 @@ defmodule PlausibleWeb.Api.StatsController.ConversionsTest do
         build(:event, name: "Signup", "meta.key": ["variant"], "meta.value": ["B"])
       ])
 
-      insert(:goal, %{domain: site.domain, page_path: "/register"})
-      insert(:goal, %{domain: site.domain, event_name: "Signup"})
+      insert(:goal, %{site: site, page_path: "/register"})
+      insert(:goal, %{site: site, event_name: "Signup"})
 
       filters = Jason.encode!(%{goal: "Signup"})
 
@@ -259,8 +259,8 @@ defmodule PlausibleWeb.Api.StatsController.ConversionsTest do
         build(:event, name: "Signup")
       ])
 
-      insert(:goal, %{domain: site.domain, page_path: "/register"})
-      insert(:goal, %{domain: site.domain, event_name: "Signup"})
+      insert(:goal, %{site: site, page_path: "/register"})
+      insert(:goal, %{site: site, event_name: "Signup"})
 
       filters = Jason.encode!(%{goal: "Signup|Visit /register"})
 
@@ -298,10 +298,10 @@ defmodule PlausibleWeb.Api.StatsController.ConversionsTest do
         build(:event, name: "Signup")
       ])
 
-      insert(:goal, %{domain: site.domain, page_path: "/register"})
-      insert(:goal, %{domain: site.domain, page_path: "/another"})
-      insert(:goal, %{domain: site.domain, event_name: "CTA"})
-      insert(:goal, %{domain: site.domain, event_name: "Signup"})
+      insert(:goal, %{site: site, page_path: "/register"})
+      insert(:goal, %{site: site, page_path: "/another"})
+      insert(:goal, %{site: site, event_name: "CTA"})
+      insert(:goal, %{site: site, event_name: "Signup"})
 
       filters = Jason.encode!(%{goal: "!Signup|Visit /another"})
 
@@ -339,9 +339,9 @@ defmodule PlausibleWeb.Api.StatsController.ConversionsTest do
         build(:event, name: "Signup")
       ])
 
-      insert(:goal, %{domain: site.domain, page_path: "/blog**"})
-      insert(:goal, %{domain: site.domain, event_name: "CTA"})
-      insert(:goal, %{domain: site.domain, event_name: "Signup"})
+      insert(:goal, %{site: site, page_path: "/blog**"})
+      insert(:goal, %{site: site, event_name: "CTA"})
+      insert(:goal, %{site: site, event_name: "Signup"})
 
       filters = Jason.encode!(%{goal: "Signup|Visit /blog**"})
 
@@ -379,10 +379,10 @@ defmodule PlausibleWeb.Api.StatsController.ConversionsTest do
         build(:event, name: "Signup")
       ])
 
-      insert(:goal, %{domain: site.domain, page_path: "/blog**"})
-      insert(:goal, %{domain: site.domain, page_path: "/ano**"})
-      insert(:goal, %{domain: site.domain, event_name: "CTA"})
-      insert(:goal, %{domain: site.domain, event_name: "Signup"})
+      insert(:goal, %{site: site, page_path: "/blog**"})
+      insert(:goal, %{site: site, page_path: "/ano**"})
+      insert(:goal, %{site: site, event_name: "CTA"})
+      insert(:goal, %{site: site, event_name: "Signup"})
 
       filters = Jason.encode!(%{goal: "!Signup|Visit /blog**"})
 
@@ -422,7 +422,7 @@ defmodule PlausibleWeb.Api.StatsController.ConversionsTest do
         build(:event, name: "Signup", user_id: 2)
       ])
 
-      insert(:goal, %{domain: site.domain, event_name: "Signup"})
+      insert(:goal, %{site: site, event_name: "Signup"})
 
       filters = Jason.encode!(%{goal: "Signup", props: %{variant: "(none)"}})
 
@@ -457,7 +457,7 @@ defmodule PlausibleWeb.Api.StatsController.ConversionsTest do
         build(:event, name: "Signup", "meta.key": ["variant"], "meta.value": ["B"])
       ])
 
-      insert(:goal, %{domain: site.domain, event_name: "Signup"})
+      insert(:goal, %{site: site, event_name: "Signup"})
       filters = Jason.encode!(%{goal: "Signup"})
       prop_key = "variant"
 
@@ -493,7 +493,7 @@ defmodule PlausibleWeb.Api.StatsController.ConversionsTest do
         build(:event, name: "Signup", "meta.key": ["variant"], "meta.value": ["A"])
       ])
 
-      insert(:goal, %{domain: site.domain, event_name: "Signup"})
+      insert(:goal, %{site: site, event_name: "Signup"})
       filters = Jason.encode!(%{goal: "Signup"})
       prop_key = "variant"
 
@@ -527,7 +527,7 @@ defmodule PlausibleWeb.Api.StatsController.ConversionsTest do
         build(:event, user_id: 2, name: "Signup", "meta.key": ["variant"], "meta.value": ["B"])
       ])
 
-      insert(:goal, %{domain: site.domain, event_name: "Signup"})
+      insert(:goal, %{site: site, event_name: "Signup"})
       filters = Jason.encode!(%{goal: "Signup", props: %{"variant" => "B"}})
       prop_key = "variant"
 
@@ -565,7 +565,7 @@ defmodule PlausibleWeb.Api.StatsController.ConversionsTest do
         )
       ])
 
-      insert(:goal, %{domain: site.domain, event_name: "ButtonClick"})
+      insert(:goal, %{site: site, event_name: "ButtonClick"})
 
       filters =
         Jason.encode!(%{
@@ -611,7 +611,7 @@ defmodule PlausibleWeb.Api.StatsController.ConversionsTest do
         )
       ])
 
-      insert(:goal, %{domain: site.domain, event_name: "ButtonClick"})
+      insert(:goal, %{site: site, event_name: "ButtonClick"})
 
       filters =
         Jason.encode!(%{
@@ -642,16 +642,16 @@ defmodule PlausibleWeb.Api.StatsController.ConversionsTest do
     setup [:create_user, :log_in, :create_site]
 
     test "returns correct and sorted glob goal counts", %{conn: conn, site: site} do
-      insert(:goal, %{domain: site.domain, page_path: "/register"})
-      insert(:goal, %{domain: site.domain, page_path: "/reg*"})
-      insert(:goal, %{domain: site.domain, page_path: "/*/register"})
-      insert(:goal, %{domain: site.domain, page_path: "/billing**/success"})
-      insert(:goal, %{domain: site.domain, page_path: "/billing*/success"})
-      insert(:goal, %{domain: site.domain, page_path: "/signup"})
-      insert(:goal, %{domain: site.domain, page_path: "/signup/*"})
-      insert(:goal, %{domain: site.domain, page_path: "/signup/**"})
-      insert(:goal, %{domain: site.domain, page_path: "/*"})
-      insert(:goal, %{domain: site.domain, page_path: "/**"})
+      insert(:goal, %{site: site, page_path: "/register"})
+      insert(:goal, %{site: site, page_path: "/reg*"})
+      insert(:goal, %{site: site, page_path: "/*/register"})
+      insert(:goal, %{site: site, page_path: "/billing**/success"})
+      insert(:goal, %{site: site, page_path: "/billing*/success"})
+      insert(:goal, %{site: site, page_path: "/signup"})
+      insert(:goal, %{site: site, page_path: "/signup/*"})
+      insert(:goal, %{site: site, page_path: "/signup/**"})
+      insert(:goal, %{site: site, page_path: "/*"})
+      insert(:goal, %{site: site, page_path: "/**"})
 
       populate_stats(site, [
         build(:pageview,
diff --git a/test/plausible_web/controllers/site_controller_test.exs b/test/plausible_web/controllers/site_controller_test.exs
index 460fa0ad35..97d647993a 100644
--- a/test/plausible_web/controllers/site_controller_test.exs
+++ b/test/plausible_web/controllers/site_controller_test.exs
@@ -334,8 +334,8 @@ defmodule PlausibleWeb.SiteControllerTest do
     setup [:create_user, :log_in, :create_site]
 
     test "lists goals for the site", %{conn: conn, site: site} do
-      insert(:goal, domain: site.domain, event_name: "Custom event")
-      insert(:goal, domain: site.domain, page_path: "/register")
+      insert(:goal, site: site, event_name: "Custom event")
+      insert(:goal, site: site, page_path: "/register")
 
       conn = get(conn, "/#{site.domain}/settings/goals")
 
@@ -620,7 +620,6 @@ defmodule PlausibleWeb.SiteControllerTest do
     end
   end
 
-  @tag :skip
   describe "POST /:website/goals" do
     setup [:create_user, :log_in, :create_site]
 
@@ -640,7 +639,6 @@ defmodule PlausibleWeb.SiteControllerTest do
       assert redirected_to(conn, 302) == "/#{site.domain}/settings/goals"
     end
 
-    @tag :skip
     test "creates a custom event goal for the website", %{conn: conn, site: site} do
       conn =
         post(conn, "/#{site.domain}/goals", %{
@@ -662,7 +660,7 @@ defmodule PlausibleWeb.SiteControllerTest do
     setup [:create_user, :log_in, :create_site]
 
     test "deletes goal", %{conn: conn, site: site} do
-      goal = insert(:goal, domain: site.domain, event_name: "Custom event")
+      goal = insert(:goal, site: site, event_name: "Custom event")
 
       conn = delete(conn, "/#{site.domain}/goals/#{goal.id}")
 
@@ -672,7 +670,7 @@ defmodule PlausibleWeb.SiteControllerTest do
 
     test "fails to delete goal for a foreign site", %{conn: conn, site: site} do
       another_site = insert(:site)
-      goal = insert(:goal, domain: another_site.domain, event_name: "Custom event")
+      goal = insert(:goal, site: another_site, event_name: "Custom event")
 
       conn = delete(conn, "/#{site.domain}/goals/#{goal.id}")
 
diff --git a/test/plausible_web/controllers/stats_controller_test.exs b/test/plausible_web/controllers/stats_controller_test.exs
index 0c7d32e83b..7529d6367e 100644
--- a/test/plausible_web/controllers/stats_controller_test.exs
+++ b/test/plausible_web/controllers/stats_controller_test.exs
@@ -250,7 +250,7 @@ defmodule PlausibleWeb.StatsControllerTest do
       )
     ])
 
-    insert(:goal, %{domain: site.domain, event_name: "Signup"})
+    insert(:goal, %{site: site, event_name: "Signup"})
   end
 
   describe "GET /:website/export - with goal filter" do