From 4d196036934a0d12ed0ae800487015f26283fa2c Mon Sep 17 00:00:00 2001 From: Karl-Aksel Puulmann Date: Wed, 10 Jul 2024 15:10:29 +0300 Subject: [PATCH] fix: Calculate revenue metrics correctly when including imports (#4335) Previously, revenue metrics were reported as 0.0$ when imports were included with APIv2. This is because the metrics were not properly included/forwarded by the imports query. Not sure how long this bug has persisted, but eh. --- lib/plausible/stats/imported/sql/builder.ex | 8 +++++--- .../controllers/api/stats_controller/conversions_test.exs | 6 ++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/plausible/stats/imported/sql/builder.ex b/lib/plausible/stats/imported/sql/builder.ex index cf81959d3..557b95b27 100644 --- a/lib/plausible/stats/imported/sql/builder.ex +++ b/lib/plausible/stats/imported/sql/builder.ex @@ -435,14 +435,16 @@ defmodule Plausible.Stats.Imported.SQL.Builder do |> select_joined_metrics(rest) end - def select_joined_metrics(q, [:sample_percent | rest]) do + # Ignored as it's calculated separately + def select_joined_metrics(q, [metric | rest]) + when metric in [:conversion_rate, :group_conversion_rate, :percentage] do q - |> select_merge_as([s, i], %{sample_percent: s.sample_percent}) |> select_joined_metrics(rest) end - def select_joined_metrics(q, [_ | rest]) do + def select_joined_metrics(q, [metric | rest]) do q + |> select_merge_as([s, i], %{metric => field(s, ^metric)}) |> select_joined_metrics(rest) end diff --git a/test/plausible_web/controllers/api/stats_controller/conversions_test.exs b/test/plausible_web/controllers/api/stats_controller/conversions_test.exs index a5bcdb38b..63484907c 100644 --- a/test/plausible_web/controllers/api/stats_controller/conversions_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/conversions_test.exs @@ -272,7 +272,9 @@ defmodule PlausibleWeb.Api.StatsController.ConversionsTest do conn: conn, site: site } do - populate_stats(site, [ + site_import = insert(:site_import, site: site) + + populate_stats(site, site_import.id, [ build(:event, name: "Payment", revenue_reporting_amount: Decimal.new("200100300.123"), @@ -294,7 +296,7 @@ defmodule PlausibleWeb.Api.StatsController.ConversionsTest do insert(:goal, %{site: site, event_name: "Payment", currency: :EUR}) - conn = get(conn, "/api/stats/#{site.domain}/conversions?period=day") + conn = get(conn, "/api/stats/#{site.domain}/conversions?period=day&with_imported=true") assert json_response(conn, 200)["results"] == [ %{