From 868bff68e2729ef34a6470ad8c23b4847a31d37d Mon Sep 17 00:00:00 2001 From: Uku Taht Date: Wed, 10 Jun 2026 13:43:42 +0300 Subject: [PATCH 1/2] fix: ignore imported data when querying goals with custom props --- lib/plausible/stats/goals.ex | 13 ++- lib/plausible/stats/imported/base.ex | 23 +++-- .../query_goal_custom_props_test.exs | 90 +++++++++++++++++++ 3 files changed, 118 insertions(+), 8 deletions(-) diff --git a/lib/plausible/stats/goals.ex b/lib/plausible/stats/goals.ex index 806635f3509e..0183627ebbc7 100644 --- a/lib/plausible/stats/goals.ex +++ b/lib/plausible/stats/goals.ex @@ -98,11 +98,20 @@ defmodule Plausible.Stats.Goals do goal_type = Plausible.Goal.type(goal) {prop_keys, prop_values} = Enum.unzip(goal.custom_props) + # `types` and `event_names_imports` are only used for matching goals + # against imported data. Imported tables are aggregated without custom + # properties, so goals with custom props must never match - "" ensures + # that. + queryable_from_imports? = not Plausible.Goal.has_custom_props?(goal) + %{ indices: [idx | acc.indices], - types: [to_string(goal_type) | acc.types], + types: [if(queryable_from_imports?, do: to_string(goal_type), else: "") | acc.types], # This will contain "" for non-event goals - event_names_imports: [to_string(goal.event_name) | acc.event_names_imports], + event_names_imports: [ + if(queryable_from_imports?, do: to_string(goal.event_name), else: "") + | acc.event_names_imports + ], event_names_by_type: [event_name_by_type(goal_type, goal) | acc.event_names_by_type], # Event goals are considered to match everything for the sake of efficient queries in query_builder.ex # See also Plausible.Stats.SQL.Expression.event_goal_join/1 diff --git a/lib/plausible/stats/imported/base.ex b/lib/plausible/stats/imported/base.ex index 1580b9e42df3..d810541b4594 100644 --- a/lib/plausible/stats/imported/base.ex +++ b/lib/plausible/stats/imported/base.ex @@ -142,13 +142,21 @@ defmodule Plausible.Stats.Imported.Base do |> Enum.any?(&(&1 not in [property, "event:name", "event:goal"])) if has_required_event_or_goal_name_filter? and - not has_unsupported_filters? do + not has_unsupported_filters? and + not any_custom_prop_goal_filters?(query) do ["imported_custom_events"] else [] end end + # Goals with custom props cannot be queried from imported data, as imported + # tables are aggregated without custom properties. + defp any_custom_prop_goal_filters?(query) do + query.preloaded_goals.matching_toplevel_filters + |> Enum.any?(&Plausible.Goal.has_custom_props?/1) + end + defp do_decide_tables(%Query{filters: [], dimensions: []}), do: ["imported_visitors"] defp do_decide_tables(%Query{filters: [], dimensions: ["event:goal"]}) do @@ -172,6 +180,7 @@ defmodule Plausible.Stats.Imported.Base do Enum.any?(filter_dimensions, &(&1 not in ["event:page", "event:name", "event:goal"])) cond do + any_custom_prop_goal_filters?(query) -> [] any_other_filters? -> [] any_event_name_filters? and not any_page_filters? -> ["imported_custom_events"] any_page_filters? and not any_event_name_filters? -> ["imported_pages"] @@ -192,11 +201,13 @@ defmodule Plausible.Stats.Imported.Base do filter_goal_table_candidates = query.preloaded_goals.matching_toplevel_filters - |> Enum.map(&Plausible.Goal.type/1) - |> Enum.map(fn - :event -> "imported_custom_events" - :page -> "imported_pages" - :scroll -> nil + |> Enum.map(fn goal -> + case {Plausible.Goal.has_custom_props?(goal), Plausible.Goal.type(goal)} do + {true, _} -> nil + {false, :event} -> "imported_custom_events" + {false, :page} -> "imported_pages" + {false, :scroll} -> nil + end end) case Enum.uniq(table_candidates ++ filter_goal_table_candidates) do diff --git a/test/plausible_web/controllers/api/external_stats_controller/query_goal_custom_props_test.exs b/test/plausible_web/controllers/api/external_stats_controller/query_goal_custom_props_test.exs index 360e00a64249..34abe526faf2 100644 --- a/test/plausible_web/controllers/api/external_stats_controller/query_goal_custom_props_test.exs +++ b/test/plausible_web/controllers/api/external_stats_controller/query_goal_custom_props_test.exs @@ -367,4 +367,94 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryGoalCustomPropsTest do refute Enum.find(results, &(&1["dimensions"] == ["Button Click"])) end end + + describe "goals with custom props and imported data" do + setup :create_site_import + + test "filtering by a goal with custom props excludes imported data instead of crashing", %{ + conn: conn, + site: site, + site_import: site_import + } do + {:ok, _goal} = + Goals.create(site, %{ + "event_name" => "Purchase", + "custom_props" => %{"variant" => "A"} + }) + + populate_stats(site, site_import.id, [ + build(:event, + name: "Purchase", + "meta.key": ["variant"], + "meta.value": ["A"], + timestamp: ~N[2023-01-01 00:00:00] + ), + build(:imported_custom_events, + name: "Purchase", + visitors: 3, + events: 5, + date: ~D[2023-01-01] + ) + ]) + + conn = + post(conn, "/api/v2/query", %{ + "site_id" => site.domain, + "date_range" => "all", + "metrics" => ["visitors", "events"], + "filters" => [["is", "event:goal", ["Purchase"]]], + "include" => %{"imports" => true} + }) + + resp = json_response(conn, 200) + + # Imported data cannot be filtered by the goal's custom props, + # so it must be excluded from the query. + assert resp["results"] == [%{"dimensions" => [], "metrics" => [1, 1]}] + refute resp["meta"]["imports_included"] + end + + test "breakdown by event:goal does not attribute imported events to a goal with custom props", + %{ + conn: conn, + site: site, + site_import: site_import + } do + {:ok, _goal} = + Goals.create(site, %{ + "event_name" => "Purchase", + "custom_props" => %{"variant" => "A"} + }) + + populate_stats(site, site_import.id, [ + build(:event, + name: "Purchase", + "meta.key": ["variant"], + "meta.value": ["A"], + timestamp: ~N[2023-01-01 00:00:00] + ), + build(:imported_custom_events, + name: "Purchase", + visitors: 3, + events: 5, + date: ~D[2023-01-01] + ) + ]) + + conn = + post(conn, "/api/v2/query", %{ + "site_id" => site.domain, + "date_range" => "all", + "metrics" => ["visitors", "events"], + "dimensions" => ["event:goal"], + "include" => %{"imports" => true} + }) + + resp = json_response(conn, 200) + + # Imported events are aggregated by name only and cannot be checked + # against the goal's custom props, so they must not be counted. + assert resp["results"] == [%{"dimensions" => ["Purchase"], "metrics" => [1, 1]}] + end + end end From 44918ec792b913ff0738f0a1b661b2dbaaddf4f6 Mon Sep 17 00:00:00 2001 From: Uku Taht Date: Wed, 10 Jun 2026 15:27:43 +0300 Subject: [PATCH 2/2] Add tests --- .../query_goal_custom_props_test.exs | 132 ++++++++++++++++++ 1 file changed, 132 insertions(+) diff --git a/test/plausible_web/controllers/api/external_stats_controller/query_goal_custom_props_test.exs b/test/plausible_web/controllers/api/external_stats_controller/query_goal_custom_props_test.exs index 34abe526faf2..6b849c88521f 100644 --- a/test/plausible_web/controllers/api/external_stats_controller/query_goal_custom_props_test.exs +++ b/test/plausible_web/controllers/api/external_stats_controller/query_goal_custom_props_test.exs @@ -456,5 +456,137 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryGoalCustomPropsTest do # against the goal's custom props, so they must not be counted. assert resp["results"] == [%{"dimensions" => ["Purchase"], "metrics" => [1, 1]}] end + + test "breakdown by event:goal filtered by a goal with custom props excludes imported data instead of crashing", + %{ + conn: conn, + site: site, + site_import: site_import + } do + {:ok, _goal} = + Goals.create(site, %{ + "event_name" => "Purchase", + "custom_props" => %{"variant" => "A"} + }) + + populate_stats(site, site_import.id, [ + build(:event, + name: "Purchase", + "meta.key": ["variant"], + "meta.value": ["A"], + timestamp: ~N[2023-01-01 00:00:00] + ), + build(:imported_custom_events, + name: "Purchase", + visitors: 3, + events: 5, + date: ~D[2023-01-01] + ) + ]) + + conn = + post(conn, "/api/v2/query", %{ + "site_id" => site.domain, + "date_range" => "all", + "metrics" => ["visitors", "events"], + "dimensions" => ["event:goal"], + "filters" => [["is", "event:goal", ["Purchase"]]], + "include" => %{"imports" => true} + }) + + resp = json_response(conn, 200) + + assert resp["results"] == [%{"dimensions" => ["Purchase"], "metrics" => [1, 1]}] + refute resp["meta"]["imports_included"] + end + + test "filtering by a custom property and a special goal with custom props excludes imported data instead of crashing", + %{ + conn: conn, + site: site, + site_import: site_import + } do + {:ok, _goal} = + Goals.create(site, %{ + "event_name" => "Outbound Link: Click", + "custom_props" => %{"page_theme" => "dark"} + }) + + populate_stats(site, site_import.id, [ + build(:event, + name: "Outbound Link: Click", + "meta.key": ["url", "page_theme"], + "meta.value": ["https://example.com", "dark"], + timestamp: ~N[2023-01-01 00:00:00] + ), + build(:imported_custom_events, + name: "Outbound Link: Click", + link_url: "https://example.com", + visitors: 3, + events: 5, + date: ~D[2023-01-01] + ) + ]) + + conn = + post(conn, "/api/v2/query", %{ + "site_id" => site.domain, + "date_range" => "all", + "metrics" => ["visitors", "events"], + "filters" => [ + ["is", "event:goal", ["Outbound Link: Click"]], + ["is", "event:props:url", ["https://example.com"]] + ], + "include" => %{"imports" => true} + }) + + resp = json_response(conn, 200) + + assert resp["results"] == [%{"dimensions" => [], "metrics" => [1, 1]}] + refute resp["meta"]["imports_included"] + end + + test "breakdown by event:goal does not attribute imported pageviews to a page goal with custom props", + %{ + conn: conn, + site: site, + site_import: site_import + } do + {:ok, _goal} = + Goals.create(site, %{ + "page_path" => "/checkout", + "custom_props" => %{"variant" => "A"} + }) + + populate_stats(site, site_import.id, [ + build(:pageview, + pathname: "/checkout", + "meta.key": ["variant"], + "meta.value": ["A"], + timestamp: ~N[2023-01-01 00:00:00] + ), + build(:imported_pages, + page: "/checkout", + visitors: 3, + pageviews: 5, + date: ~D[2023-01-01] + ) + ]) + + conn = + post(conn, "/api/v2/query", %{ + "site_id" => site.domain, + "date_range" => "all", + "metrics" => ["visitors"], + "dimensions" => ["event:goal"], + "include" => %{"imports" => true} + }) + + resp = json_response(conn, 200) + + # Imported pageviews are aggregated by page only and cannot be checked + # against the goal's custom props, so they must not be counted. + assert resp["results"] == [%{"dimensions" => ["Visit /checkout"], "metrics" => [1]}] + end end end