From 08883b08fa59f75ef41366cd532d1f3983d57714 Mon Sep 17 00:00:00 2001 From: Robert Joonas Date: Thu, 31 Oct 2024 12:09:21 +0100 Subject: [PATCH 1/5] migration: add scroll_threshold to goals --- ...28161815_add_scroll_threshold_to_goals.exs | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) create mode 100644 priv/repo/migrations/20250128161815_add_scroll_threshold_to_goals.exs diff --git a/priv/repo/migrations/20250128161815_add_scroll_threshold_to_goals.exs b/priv/repo/migrations/20250128161815_add_scroll_threshold_to_goals.exs new file mode 100644 index 000000000000..935f2919e8db --- /dev/null +++ b/priv/repo/migrations/20250128161815_add_scroll_threshold_to_goals.exs @@ -0,0 +1,39 @@ +defmodule Plausible.Repo.Migrations.AddScrollThresholdToGoals do + use Ecto.Migration + + @disable_ddl_transaction true + @disable_migration_lock true + + # Plausible.Repo.Migrations.GoalsUnique + @old_index unique_index( + :goals, + [:site_id, :page_path], + where: "page_path IS NOT NULL", + name: :goals_page_path_unique + ) + + @new_index unique_index( + :goals, + [:site_id, :page_path, :scroll_threshold], + where: "page_path IS NOT NULL", + name: :goals_page_path_and_scroll_threshold_unique + ) + + def up do + alter table(:goals) do + add :scroll_threshold, :smallint, null: false, default: -1 + end + + drop(@old_index) + create(@new_index) + end + + def down do + drop(@new_index) + create(@old_index) + + alter table(:goals) do + remove :scroll_threshold + end + end +end From 04397934a7ab7004b7721861db45fc8dfdf9110f Mon Sep 17 00:00:00 2001 From: Robert Joonas Date: Thu, 30 Jan 2025 12:13:23 +0100 Subject: [PATCH 2/5] do not drop the old index yet --- .../20250128161815_add_scroll_threshold_to_goals.exs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/priv/repo/migrations/20250128161815_add_scroll_threshold_to_goals.exs b/priv/repo/migrations/20250128161815_add_scroll_threshold_to_goals.exs index 935f2919e8db..3392a11d1fde 100644 --- a/priv/repo/migrations/20250128161815_add_scroll_threshold_to_goals.exs +++ b/priv/repo/migrations/20250128161815_add_scroll_threshold_to_goals.exs @@ -4,14 +4,6 @@ defmodule Plausible.Repo.Migrations.AddScrollThresholdToGoals do @disable_ddl_transaction true @disable_migration_lock true - # Plausible.Repo.Migrations.GoalsUnique - @old_index unique_index( - :goals, - [:site_id, :page_path], - where: "page_path IS NOT NULL", - name: :goals_page_path_unique - ) - @new_index unique_index( :goals, [:site_id, :page_path, :scroll_threshold], @@ -24,13 +16,11 @@ defmodule Plausible.Repo.Migrations.AddScrollThresholdToGoals do add :scroll_threshold, :smallint, null: false, default: -1 end - drop(@old_index) create(@new_index) end def down do drop(@new_index) - create(@old_index) alter table(:goals) do remove :scroll_threshold From 82e70dc907a6bbbf140c2e82b418b42adb78eb5b Mon Sep 17 00:00:00 2001 From: Robert Joonas Date: Thu, 30 Jan 2025 14:15:56 +0100 Subject: [PATCH 3/5] add scroll_threshold field to goal schema + new unique constraint --- lib/plausible/goal/schema.ex | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/plausible/goal/schema.ex b/lib/plausible/goal/schema.ex index 8f2e29febe01..84d76cb991ff 100644 --- a/lib/plausible/goal/schema.ex +++ b/lib/plausible/goal/schema.ex @@ -8,6 +8,7 @@ defmodule Plausible.Goal do schema "goals" do field :event_name, :string field :page_path, :string + field :scroll_threshold, :integer, default: -1 field :display_name, :string on_ee do @@ -23,7 +24,7 @@ defmodule Plausible.Goal do timestamps() end - @fields [:id, :site_id, :event_name, :page_path, :display_name] ++ + @fields [:id, :site_id, :event_name, :page_path, :scroll_threshold, :display_name] ++ on_ee(do: [:currency], else: []) @max_event_name_length 120 @@ -40,6 +41,9 @@ defmodule Plausible.Goal do |> maybe_put_display_name() |> unique_constraint(:event_name, name: :goals_event_name_unique) |> unique_constraint(:page_path, name: :goals_page_path_unique) + |> unique_constraint([:page_path, :scroll_threshold], + name: :goals_page_path_and_scroll_threshold_unique + ) |> unique_constraint(:display_name, name: :goals_site_id_display_name_index) |> validate_length(:event_name, max: @max_event_name_length) |> check_constraint(:event_name, From 3d0a13170221328e5d7a65f4d30d3dfb82fae5c3 Mon Sep 17 00:00:00 2001 From: Robert Joonas Date: Thu, 30 Jan 2025 14:29:39 +0100 Subject: [PATCH 4/5] adjust test to test what it claims to --- test/plausible/goals_test.exs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/plausible/goals_test.exs b/test/plausible/goals_test.exs index ede9287ba3a6..d553573271e3 100644 --- a/test/plausible/goals_test.exs +++ b/test/plausible/goals_test.exs @@ -34,8 +34,8 @@ defmodule Plausible.GoalsTest do test "create/2 fails to create the same pageview goal twice" do site = new_site() - {:ok, _} = Goals.create(site, %{"page_path" => "foo bar"}) - assert {:error, changeset} = Goals.create(site, %{"page_path" => "foo bar"}) + {:ok, _} = Goals.create(site, %{"page_path" => "foo bar", "display_name" => "one"}) + assert {:error, changeset} = Goals.create(site, %{"page_path" => "foo bar", "display_name" => "two"}) assert {"has already been taken", _} = changeset.errors[:page_path] end From 72e2934974a406cd2e9f3ad798172def7f4ce64a Mon Sep 17 00:00:00 2001 From: Robert Joonas Date: Thu, 30 Jan 2025 14:41:21 +0100 Subject: [PATCH 5/5] mix format --- test/plausible/goals_test.exs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/plausible/goals_test.exs b/test/plausible/goals_test.exs index d553573271e3..d57cedfceafc 100644 --- a/test/plausible/goals_test.exs +++ b/test/plausible/goals_test.exs @@ -35,7 +35,10 @@ defmodule Plausible.GoalsTest do test "create/2 fails to create the same pageview goal twice" do site = new_site() {:ok, _} = Goals.create(site, %{"page_path" => "foo bar", "display_name" => "one"}) - assert {:error, changeset} = Goals.create(site, %{"page_path" => "foo bar", "display_name" => "two"}) + + assert {:error, changeset} = + Goals.create(site, %{"page_path" => "foo bar", "display_name" => "two"}) + assert {"has already been taken", _} = changeset.errors[:page_path] end