diff --git a/lib/plausible/stats/imported/imported.ex b/lib/plausible/stats/imported/imported.ex index 902d2c0e93fb..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 @@ -360,7 +365,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/lib/plausible/stats/parsed_query_params.ex b/lib/plausible/stats/parsed_query_params.ex index 3a7405e1bda6..86689ec8638d 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{} |> Map.from_struct() |> Map.keys() + 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 40dc2a6ae121..ddcdf7598e00 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), @@ -150,49 +150,60 @@ 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) + parsed_query_params + |> ParsedQueryParams.to_query!() + |> set_now() + |> set_utc_time_range(site, Map.get(parsed_query_params, :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 + ) + |> maybe_drop_revenue_metrics() + 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 +230,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 @@ -241,8 +238,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, @@ -252,10 +248,32 @@ defmodule Plausible.Stats.QueryBuilder do :ok end 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.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, %{}} 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 5093a4fd5f3a..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] @@ -2205,6 +2266,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 +2485,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