From a3555e7542e158526c0758f78a93290a88bef7f7 Mon Sep 17 00:00:00 2001 From: Robert Joonas Date: Thu, 7 May 2026 23:51:28 +0300 Subject: [PATCH 1/8] organize QueryBuilder.do_build function better --- lib/plausible/stats/query_builder.ex | 86 ++++++++++++++-------------- 1 file changed, 42 insertions(+), 44 deletions(-) diff --git a/lib/plausible/stats/query_builder.ex b/lib/plausible/stats/query_builder.ex index 40dc2a6ae121..021b4488bef2 100644 --- a/lib/plausible/stats/query_builder.ex +++ b/lib/plausible/stats/query_builder.ex @@ -150,49 +150,61 @@ defmodule Plausible.Stats.QueryBuilder do end defp do_build(parsed_query_params, site, debug_metadata) do - now = parsed_query_params.now || DateTime.utc_now(:second) + {%{relative_date: relative_date}, query_fields} = + Map.split(parsed_query_params, [:relative_date, :skip_goal_existence_check]) + + struct!(%Query{}, Map.to_list(query_fields)) + |> set_now() + |> set_utc_time_range(site, relative_date) + |> set_preloaded_goals_and_revenue(site) + |> Query.set( + site_id: site.id, + site_native_stats_start_at: site.native_stats_start_at, + timezone: site.timezone, + consolidated_site_ids: get_consolidated_site_ids(site), + debug_metadata: debug_metadata + ) + end - %ParsedQueryParams{ - input_date_range: input_date_range, - relative_date: relative_date, - metrics: metrics, - filters: filters, - dimensions: dimensions - } = parsed_query_params + defp set_now(%Query{now: nil} = query), do: Query.set(query, now: DateTime.utc_now(:second)) + defp set_now(query), do: query - relative_date = relative_date || Times.to_date(now, site.timezone) + defp set_utc_time_range(query, site, relative_date) do + relative_date = relative_date || Times.to_date(query.now, site.timezone) utc_time_range = - input_date_range - |> build_datetime_range(site, relative_date, now) + query.input_date_range + |> build_datetime_range(site, relative_date, query.now) |> DateTimeRange.to_timezone("Etc/UTC") - {preloaded_goals, revenue_warning, revenue_currencies} = - preload_goals_and_revenue(site, metrics, filters, dimensions) + Query.set(query, utc_time_range: utc_time_range) + end - consolidated_site_ids = get_consolidated_site_ids(site) + defp set_preloaded_goals_and_revenue(query, site) do + {preloaded_goals, revenue_warning, revenue_currencies} = + preload_goals_and_revenue(site, query.metrics, query.filters, query.dimensions) - struct!(%Query{}, - now: now, - input_date_range: input_date_range, - utc_time_range: utc_time_range, - site_id: site.id, - metrics: metrics, - dimensions: dimensions, - filters: filters, - order_by: parsed_query_params.order_by, - pagination: parsed_query_params.pagination, - include: parsed_query_params.include, - site_native_stats_start_at: site.native_stats_start_at, - consolidated_site_ids: consolidated_site_ids, - timezone: site.timezone, + Query.set(query, preloaded_goals: preloaded_goals, revenue_warning: revenue_warning, - revenue_currencies: revenue_currencies, - debug_metadata: debug_metadata + revenue_currencies: revenue_currencies ) end + def preload_goals_and_revenue(site, metrics, filters, dimensions) do + preloaded_goals = + Plausible.Stats.Goals.preload_needed_goals(site, dimensions, filters) + + {revenue_warning, revenue_currencies} = + preload_revenue(site, preloaded_goals, metrics, dimensions) + + { + preloaded_goals, + revenue_warning, + revenue_currencies + } + end + on_ee do def get_consolidated_site_ids(%Plausible.Site{} = site) do if Plausible.Sites.consolidated?(site) do @@ -219,20 +231,6 @@ defmodule Plausible.Stats.QueryBuilder do struct!(query, comparison_utc_time_range: datetime_range) end - def preload_goals_and_revenue(site, metrics, filters, dimensions) do - preloaded_goals = - Plausible.Stats.Goals.preload_needed_goals(site, dimensions, filters) - - {revenue_warning, revenue_currencies} = - preload_revenue(site, preloaded_goals, metrics, dimensions) - - { - preloaded_goals, - revenue_warning, - revenue_currencies - } - end - on_ee do alias Plausible.Stats.Goal.Revenue From 8461eda23179612f84ea46482749668a9b34769b Mon Sep 17 00:00:00 2001 From: Robert Joonas Date: Fri, 8 May 2026 13:17:43 +0300 Subject: [PATCH 2/8] fix querying revenue metrics when including imports --- lib/plausible/stats/imported/imported.ex | 4 +- .../api/stats_controller/main_graph_test.exs | 86 +++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/lib/plausible/stats/imported/imported.ex b/lib/plausible/stats/imported/imported.ex index 902d2c0e93fb..4658d74219be 100644 --- a/lib/plausible/stats/imported/imported.ex +++ b/lib/plausible/stats/imported/imported.ex @@ -360,7 +360,9 @@ defmodule Plausible.Stats.Imported do :bounce_rate, :conversion_rate, :group_conversion_rate, - :time_on_page + :time_on_page, + :total_revenue, + :average_revenue ] defp can_order_by?(query) do diff --git a/test/plausible_web/controllers/api/stats_controller/main_graph_test.exs b/test/plausible_web/controllers/api/stats_controller/main_graph_test.exs index 5093a4fd5f3a..de0d05db5fc7 100644 --- a/test/plausible_web/controllers/api/stats_controller/main_graph_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/main_graph_test.exs @@ -2205,6 +2205,49 @@ defmodule PlausibleWeb.Api.StatsController.MainGraphTest do } ] end + + test "silently ignores imported data", %{conn: conn, site: site} do + insert(:goal, site: site, event_name: "Payment", currency: "USD") + + populate_stats(site, [ + build(:event, + name: "Payment", + revenue_reporting_amount: Decimal.new("13.29"), + revenue_reporting_currency: "USD", + timestamp: ~N[2021-01-01 00:00:00] + ), + build(:event, + name: "Payment", + revenue_reporting_amount: Decimal.new("13.21"), + revenue_reporting_currency: "USD", + timestamp: ~N[2021-01-01 00:00:00] + ), + build(:imported_visitors, date: ~D[2021-01-01]) + ]) + + response = + do_query(conn, site, %{ + "date_range" => "all", + "metrics" => ["total_revenue"], + "dimensions" => ["time:month"], + "filters" => [["is", "event:goal", ["Payment"]]], + "include" => %{"imports" => true} + }) + + assert response["results"] == [ + %{ + "dimensions" => ["2021-01-01"], + "metrics" => [ + %{ + "currency" => "USD", + "long" => "$26.50", + "short" => "$26.5", + "value" => 26.5 + } + ] + } + ] + end end describe "average_revenue plot" do @@ -2381,6 +2424,49 @@ defmodule PlausibleWeb.Api.StatsController.MainGraphTest do } ] end + + test "silently ignores imported data", %{conn: conn, site: site} do + insert(:goal, site: site, event_name: "Payment", currency: "USD") + + populate_stats(site, [ + build(:event, + name: "Payment", + revenue_reporting_amount: Decimal.new("3.3"), + revenue_reporting_currency: "USD", + timestamp: ~N[2021-01-01 00:00:00] + ), + build(:event, + name: "Payment", + revenue_reporting_amount: Decimal.new("1.1"), + revenue_reporting_currency: "USD", + timestamp: ~N[2021-01-01 00:00:00] + ), + build(:imported_visitors, date: ~D[2021-01-01]) + ]) + + response = + do_query(conn, site, %{ + "date_range" => "all", + "metrics" => ["average_revenue"], + "dimensions" => ["time:month"], + "filters" => [["is", "event:goal", ["Payment"]]], + "include" => %{"imports" => true} + }) + + assert response["results"] == [ + %{ + "dimensions" => ["2021-01-01"], + "metrics" => [ + %{ + "currency" => "USD", + "long" => "$2.20", + "short" => "$2.2", + "value" => 2.2 + } + ] + } + ] + end end describe "present_index" do From a37e0172280a0578b5cf447a244bd32cae66b448 Mon Sep 17 00:00:00 2001 From: Robert Joonas Date: Fri, 8 May 2026 13:42:05 +0300 Subject: [PATCH 3/8] Invalidate queries where the only (revenue) metric is dropped This fixes a 500 error where we're trying to optimistically fetch the main graph with a stored revenue metric, while the goal filter has been removed. Rather than dropping revenue metrics in QueryOptimizer (which does not validate anything), we will do it in QueryBuilder instead. Query building will fail when dropping revenue metrics leaves query.metrics empty. --- lib/plausible/stats/query_builder.ex | 25 +++++++- lib/plausible/stats/query_optimizer.ex | 2 +- .../api/stats_controller/main_graph_test.exs | 61 +++++++++++++++++++ 3 files changed, 84 insertions(+), 4 deletions(-) diff --git a/lib/plausible/stats/query_builder.ex b/lib/plausible/stats/query_builder.ex index 021b4488bef2..15736a93b6b2 100644 --- a/lib/plausible/stats/query_builder.ex +++ b/lib/plausible/stats/query_builder.ex @@ -30,7 +30,7 @@ defmodule Plausible.Stats.QueryBuilder do def build(site, %ParsedQueryParams{} = parsed_query_params, debug_metadata) do with {:ok, parsed_query_params} <- resolve_segments_in_filters(parsed_query_params, site), - query = do_build(parsed_query_params, site, debug_metadata), + {:ok, query} <- do_build(parsed_query_params, site, debug_metadata), :ok <- validate_order_by(query), :ok <- validate_custom_props_access(site, query), :ok <- validate_case_sensitive_filter_modifier(query), @@ -164,6 +164,7 @@ defmodule Plausible.Stats.QueryBuilder do consolidated_site_ids: get_consolidated_site_ids(site), debug_metadata: debug_metadata ) + |> maybe_drop_revenue_metrics() end defp set_now(%Query{now: nil} = query), do: Query.set(query, now: DateTime.utc_now(:second)) @@ -239,8 +240,7 @@ defmodule Plausible.Stats.QueryBuilder do end defp validate_revenue_metrics_access(site, query) do - if Revenue.requested?(query.metrics) and not Revenue.available?(site) and - not query.include.drop_unavailable_revenue_metrics do + if Revenue.requested?(query.metrics) and not Revenue.available?(site) do {:error, %QueryError{ code: :feature_access, @@ -250,10 +250,29 @@ defmodule Plausible.Stats.QueryBuilder do :ok end end + + defp maybe_drop_revenue_metrics(query) do + if query.include.drop_unavailable_revenue_metrics and + map_size(query.revenue_currencies) == 0 do + if Enum.all?(query.metrics, &(&1 in Revenue.revenue_metrics())) do + {:error, + %QueryError{ + code: :all_metrics_dropped, + message: "Revenue metrics were dropped and no other metrics were left to query." + }} + else + {:ok, Query.set(query, metrics: query.metrics -- Revenue.revenue_metrics())} + end + else + {:ok, query} + end + end else defp preload_revenue(_site, _preloaded_goals, _metrics, _dimensions), do: {nil, %{}} defp validate_revenue_metrics_access(_site, _query), do: :ok + + defp maybe_drop_revenue_metrics(query), do: {:ok, query} end defp validate_order_by(query) do diff --git a/lib/plausible/stats/query_optimizer.ex b/lib/plausible/stats/query_optimizer.ex index 27a9452c45eb..24b41402ba6e 100644 --- a/lib/plausible/stats/query_optimizer.ex +++ b/lib/plausible/stats/query_optimizer.ex @@ -24,7 +24,7 @@ defmodule Plausible.Stats.QueryOptimizer do 2. Adds a missing order_by clause to a query 3. Updating "time" dimension in order_by to the right granularity 4. Updates event:hostname filters to also apply on visit level for sane results. - 5. Removes revenue metrics from dashboard queries if not requested, present or unavailable for the site. + 5. [DEPRECATED AND WILL BE REMOVED] Removes revenue metrics from legacy queries if ineligible 6. Trims the date range to the current time if query.include.trim_relative_date_range is true. 7. Sets the join_type for the query based on the query. diff --git a/test/plausible_web/controllers/api/stats_controller/main_graph_test.exs b/test/plausible_web/controllers/api/stats_controller/main_graph_test.exs index de0d05db5fc7..2a51ddd1d6ae 100644 --- a/test/plausible_web/controllers/api/stats_controller/main_graph_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/main_graph_test.exs @@ -2037,6 +2037,67 @@ defmodule PlausibleWeb.Api.StatsController.MainGraphTest do end end + describe "revenue metric dropped" do + @describetag :ee_only + setup [:create_user, :log_in, :create_site] + + for revenue_metric <- ["total_revenue", "average_revenue"] do + test "#{revenue_metric} without goal filter", %{ + conn: conn, + site: site + } do + response = + do_query_fail(conn, site, %{ + "date_range" => "day", + "metrics" => [unquote(revenue_metric)], + "dimensions" => ["time:hour"] + }) + + assert %{"error" => error} = json_response(response, 400) + assert error =~ "Revenue metrics were dropped" + end + + test "#{revenue_metric} with filtered goals mixing currencies", %{ + conn: conn, + site: site + } do + insert(:goal, site: site, event_name: "PurchaseEUR", currency: "EUR") + insert(:goal, site: site, event_name: "PurchaseUSD", currency: "USD") + + response = + do_query_fail(conn, site, %{ + "date_range" => "day", + "metrics" => [unquote(revenue_metric)], + "filters" => [["is", "event:goal", ["PurchaseUSD", "PurchaseEUR"]]], + "dimensions" => ["time:hour"] + }) + + assert %{"error" => error} = json_response(response, 400) + assert error =~ "Revenue metrics were dropped" + end + + test "#{revenue_metric} with insufficient subscription", %{ + conn: conn, + user: user, + site: site + } do + insert(:goal, site: site, event_name: "PurchaseEUR", currency: "EUR") + subscribe_to_growth_plan(user) + + response = + do_query_fail(conn, site, %{ + "date_range" => "day", + "metrics" => [unquote(revenue_metric)], + "filters" => [["is", "event:goal", ["PurchaseEUR"]]], + "dimensions" => ["time:hour"] + }) + + assert %{"error" => error} = json_response(response, 400) + assert error =~ "Revenue metrics were dropped" + end + end + end + describe "total_revenue plot" do @describetag :ee_only setup [:create_user, :log_in, :create_site, :create_legacy_site_import] From 2df67ea78da89d8405bb2c06262857f1e1e14989 Mon Sep 17 00:00:00 2001 From: Robert Joonas Date: Fri, 8 May 2026 17:34:27 +0300 Subject: [PATCH 4/8] add moduledoc to silence credo --- lib/plausible/stats/imported/imported.ex | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/plausible/stats/imported/imported.ex b/lib/plausible/stats/imported/imported.ex index 4658d74219be..88a923e565b4 100644 --- a/lib/plausible/stats/imported/imported.ex +++ b/lib/plausible/stats/imported/imported.ex @@ -1,4 +1,9 @@ defmodule Plausible.Stats.Imported do + @moduledoc """ + Module defining functions that merge imported query into a native one, + and also functions that decide whether imported data is eligible for + a given query. + """ use Plausible.ClickhouseRepo use Plausible.Stats.SQL.Fragments From ac9d0a557c4f21a356cbb793317865b96dc25617 Mon Sep 17 00:00:00 2001 From: Robert Joonas Date: Mon, 11 May 2026 14:54:34 +0300 Subject: [PATCH 5/8] pattern match + guard to avoid nested if --- lib/plausible/stats/query_builder.ex | 29 +++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/lib/plausible/stats/query_builder.ex b/lib/plausible/stats/query_builder.ex index 15736a93b6b2..ef75643330ef 100644 --- a/lib/plausible/stats/query_builder.ex +++ b/lib/plausible/stats/query_builder.ex @@ -251,22 +251,25 @@ defmodule Plausible.Stats.QueryBuilder do end end - defp maybe_drop_revenue_metrics(query) do - if query.include.drop_unavailable_revenue_metrics and - map_size(query.revenue_currencies) == 0 do - if Enum.all?(query.metrics, &(&1 in Revenue.revenue_metrics())) do - {:error, - %QueryError{ - code: :all_metrics_dropped, - message: "Revenue metrics were dropped and no other metrics were left to query." - }} - else - {:ok, Query.set(query, metrics: query.metrics -- Revenue.revenue_metrics())} - end + defp maybe_drop_revenue_metrics( + %Query{ + include: %QueryInclude{drop_unavailable_revenue_metrics: true}, + revenue_currencies: revenue_currencies + } = query + ) + when map_size(revenue_currencies) == 0 do + if Enum.all?(query.metrics, &(&1 in Revenue.revenue_metrics())) do + {:error, + %QueryError{ + code: :all_metrics_dropped, + message: "Revenue metrics were dropped and no other metrics were left to query." + }} else - {:ok, query} + {:ok, Query.set(query, metrics: query.metrics -- Revenue.revenue_metrics())} end end + + defp maybe_drop_revenue_metrics(%Query{} = query), do: {:ok, query} else defp preload_revenue(_site, _preloaded_goals, _metrics, _dimensions), do: {nil, %{}} From de142b1bf101d5f9c04df266afc1bf9c11a9ccf5 Mon Sep 17 00:00:00 2001 From: Robert Joonas Date: Mon, 11 May 2026 15:45:56 +0300 Subject: [PATCH 6/8] ParsedQueryParams.to_query! --- lib/plausible/stats/parsed_query_params.ex | 7 +++++++ lib/plausible/stats/query_builder.ex | 8 +++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/plausible/stats/parsed_query_params.ex b/lib/plausible/stats/parsed_query_params.ex index 3a7405e1bda6..1dbfd30c375a 100644 --- a/lib/plausible/stats/parsed_query_params.ex +++ b/lib/plausible/stats/parsed_query_params.ex @@ -1,6 +1,8 @@ defmodule Plausible.Stats.ParsedQueryParams do @moduledoc false + alias Plausible.Stats.Query + defstruct input_date_range: nil, # `relative_date` is a convenience currently exclusive to the internal # dashboard API for constructing datetime ranges. It adds the ability @@ -20,6 +22,11 @@ defmodule Plausible.Stats.ParsedQueryParams do # the behaviour of legacy endpoints like top_stats. skip_goal_existence_check: false + def to_query!(%__MODULE__{} = parsed_query_params) do + query_fields = (Query.__struct__() |> Map.keys()) -- [:__struct__] + struct!(%Query{}, Map.take(parsed_query_params, query_fields)) + end + def new!(params) when is_map(params) do struct!(__MODULE__, Map.to_list(params)) end diff --git a/lib/plausible/stats/query_builder.ex b/lib/plausible/stats/query_builder.ex index ef75643330ef..bee5778631e1 100644 --- a/lib/plausible/stats/query_builder.ex +++ b/lib/plausible/stats/query_builder.ex @@ -150,12 +150,10 @@ defmodule Plausible.Stats.QueryBuilder do end defp do_build(parsed_query_params, site, debug_metadata) do - {%{relative_date: relative_date}, query_fields} = - Map.split(parsed_query_params, [:relative_date, :skip_goal_existence_check]) - - struct!(%Query{}, Map.to_list(query_fields)) + parsed_query_params + |> ParsedQueryParams.to_query!() |> set_now() - |> set_utc_time_range(site, relative_date) + |> set_utc_time_range(site, parsed_query_params["relative_date"]) |> set_preloaded_goals_and_revenue(site) |> Query.set( site_id: site.id, From a30d265e475823a511a9ec02022f7da51aee4593 Mon Sep 17 00:00:00 2001 From: Robert Joonas Date: Mon, 11 May 2026 16:11:40 +0300 Subject: [PATCH 7/8] fix reading relative_date --- lib/plausible/stats/query_builder.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/plausible/stats/query_builder.ex b/lib/plausible/stats/query_builder.ex index bee5778631e1..ddcdf7598e00 100644 --- a/lib/plausible/stats/query_builder.ex +++ b/lib/plausible/stats/query_builder.ex @@ -153,7 +153,7 @@ defmodule Plausible.Stats.QueryBuilder do parsed_query_params |> ParsedQueryParams.to_query!() |> set_now() - |> set_utc_time_range(site, parsed_query_params["relative_date"]) + |> set_utc_time_range(site, Map.get(parsed_query_params, :relative_date)) |> set_preloaded_goals_and_revenue(site) |> Query.set( site_id: site.id, From d8c4bb5bd52c61720a7b4a4d2919df26a8be541a Mon Sep 17 00:00:00 2001 From: Robert Joonas Date: Thu, 14 May 2026 09:25:37 +0300 Subject: [PATCH 8/8] cleaner with Map.from_struct --- lib/plausible/stats/parsed_query_params.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/plausible/stats/parsed_query_params.ex b/lib/plausible/stats/parsed_query_params.ex index 1dbfd30c375a..86689ec8638d 100644 --- a/lib/plausible/stats/parsed_query_params.ex +++ b/lib/plausible/stats/parsed_query_params.ex @@ -23,7 +23,7 @@ defmodule Plausible.Stats.ParsedQueryParams do skip_goal_existence_check: false def to_query!(%__MODULE__{} = parsed_query_params) do - query_fields = (Query.__struct__() |> Map.keys()) -- [:__struct__] + query_fields = %Query{} |> Map.from_struct() |> Map.keys() struct!(%Query{}, Map.take(parsed_query_params, query_fields)) end