Prevent special goal renames

lib/plausible/goal.ex

@@ -54,6 +54,23 @@ defmodule Plausible.Goal do
 
   def max_custom_props_per_goal(), do: @max_custom_props_per_goal
 
+  @special_goals [
+    "404",
+    "Outbound Link: Click",
+    "Cloaked Link: Click",
+    "File Download",
+    "Form: Submission",
+    "WP Search Queries",
+    "WP Form Completions"
+  ]
+
+  def special_goals(), do: @special_goals
+
+  @spec special_goal?(t() | String.t() | nil) :: boolean()
+  def special_goal?(%__MODULE__{event_name: event_name}), do: special_goal?(event_name)
+  def special_goal?(event_name) when is_binary(event_name), do: event_name in @special_goals
+  def special_goal?(_), do: false
+
   def changeset(goal, attrs \\ %{}) do
     goal
     |> cast(attrs, @fields)
@@ -79,6 +96,7 @@ defmodule Plausible.Goal do
     )
     |> maybe_drop_currency()
     |> prevent_currency_change()
+    |> prevent_special_goal_renames()
   end
 
   @spec display_name(t()) :: String.t()
@@ -235,6 +253,35 @@ defmodule Plausible.Goal do
     end
   end
 
+  defp prevent_special_goal_renames(changeset) do
+    if changeset.data.id && special_goal?(changeset.data.event_name) do
+      changeset
+      |> reject_special_goal_field_change(:event_name)
+      |> reject_special_goal_field_change(:display_name)
+    else
+      changeset
+    end
+  end
+
+  defp reject_special_goal_field_change(changeset, :event_name) do
+    if Map.has_key?(changeset.changes, :event_name) do
+      add_error(changeset, :event_name, "cannot be changed for an automated goal")
+    else
+      changeset
+    end
+  end
+
+  defp reject_special_goal_field_change(changeset, :display_name) do
+    new_value = get_change(changeset, :display_name)
+    canonical = changeset.data.event_name
+
+    if new_value && new_value != canonical do
+      add_error(changeset, :display_name, "cannot be changed for an automated goal")
+    else
+      changeset
+    end
+  end
+
   on_ee do
     defp journey_end_event?(name) do
       name == @journey_end_event

test/plausible/goals_test.exs

@@ -520,6 +520,79 @@ defmodule Plausible.GoalsTest do
            }
   end
 
+  test "update/2 prevents renaming event_name of a special goal" do
+    site = new_site()
+
+    for event_name <- Plausible.Goal.special_goals() do
+      {:ok, goal} = Goals.create(site, %{"event_name" => event_name})
+
+      assert {:error, changeset} =
+               Goals.update(goal, %{"event_name" => "Renamed #{event_name}"})
+
+      assert {"cannot be changed for an automated goal", _} = changeset.errors[:event_name]
+    end
+  end
+
+  test "update/2 prevents renaming display_name of a special goal to a non-canonical value" do
+    site = new_site()
+
+    for event_name <- Plausible.Goal.special_goals() do
+      {:ok, goal} = Goals.create(site, %{"event_name" => event_name})
+
+      assert {:error, changeset} =
+               Goals.update(goal, %{"display_name" => "Renamed #{event_name}"})
+
+      assert {"cannot be changed for an automated goal", _} = changeset.errors[:display_name]
+    end
+  end
+
+  test "update/2 allows restoring display_name of a special goal to its canonical value" do
+    site = new_site()
+    {:ok, goal} = Goals.create(site, %{"event_name" => "File Download"})
+
+    # Simulate a previously broken goal by directly updating the DB
+    Plausible.Repo.update_all(
+      Ecto.Query.where(Plausible.Goal, id: ^goal.id),
+      set: [display_name: "My File Downloads"]
+    )
+
+    broken_goal = Plausible.Repo.reload!(goal)
+    assert broken_goal.display_name == "My File Downloads"
+
+    assert {:ok, fixed} = Goals.update(broken_goal, %{"display_name" => "File Download"})
+    assert fixed.display_name == "File Download"
+  end
+
+  test "update/2 allows updating non-name fields of a special goal" do
+    site = new_site()
+    {:ok, goal} = Goals.create(site, %{"event_name" => "File Download"})
+
+    assert {:ok, updated} =
+             Goals.update(goal, %{
+               "event_name" => "File Download",
+               "display_name" => "File Download",
+               "custom_props" => %{"path" => "/file.jpg"}
+             })
+
+    assert updated.custom_props == %{"path" => "/file.jpg"}
+  end
+
+  test "update/2 allows renaming event_name of a regular goal" do
+    site = new_site()
+    {:ok, goal} = Goals.create(site, %{"event_name" => "Signup"})
+
+    assert {:ok, updated} = Goals.update(goal, %{"event_name" => "Register"})
+    assert updated.event_name == "Register"
+  end
+
+  test "update/2 allows renaming display_name of a regular goal" do
+    site = new_site()
+    {:ok, goal} = Goals.create(site, %{"event_name" => "Signup"})
+
+    assert {:ok, updated} = Goals.update(goal, %{"display_name" => "User Registration"})
+    assert updated.display_name == "User Registration"
+  end
+
   on_ee do
     test "update/2 prevents changing currency of existing revenue goal" do
       site = new_site()

test/plausible_web/live/goal_settings/form_test.exs

@@ -332,6 +332,38 @@ defmodule PlausibleWeb.Live.GoalSettings.FormTest do
       assert updated.id == g.id
     end
 
+    test "renders error when trying to rename event_name of an automated goal", %{
+      conn: conn,
+      site: site
+    } do
+      {:ok, goal} = Plausible.Goals.create(site, %{"event_name" => "File Download"})
+      lv = get_liveview(conn, site)
+
+      lv |> element(~s/button#edit-goal-#{goal.id}/) |> render_click()
+
+      lv
+      |> element("#goals-form-modalseq0 form")
+      |> render_submit(%{goal: %{event_name: "My File Download"}})
+
+      assert render(lv) =~ "cannot be changed for an automated goal"
+    end
+
+    test "renders error when trying to rename display_name of an automated goal", %{
+      conn: conn,
+      site: site
+    } do
+      {:ok, goal} = Plausible.Goals.create(site, %{"event_name" => "File Download"})
+      lv = get_liveview(conn, site)
+
+      lv |> element(~s/button#edit-goal-#{goal.id}/) |> render_click()
+
+      lv
+      |> element("#goals-form-modalseq0 form")
+      |> render_submit(%{goal: %{event_name: "File Download", display_name: "My Downloads"}})
+
+      assert render(lv) =~ "cannot be changed for an automated goal"
+    end
+
     test "renders error when goal is invalid", %{conn: conn, site: site} do
       {:ok, [g, g2, _]} = setup_goals(site)
       lv = get_liveview(conn, site)