From 1ec60b26942ecd0ff1ac8562d99230590e2641cc Mon Sep 17 00:00:00 2001 From: ruslandoga <67764432+ruslandoga@users.noreply.github.com> Date: Sun, 18 Feb 2024 19:07:10 +0800 Subject: [PATCH] allow imported views per visit --- assets/js/dashboard/stats/graph/top-stats.js | 4 +--- lib/plausible/stats/aggregate.ex | 2 +- lib/plausible/stats/base.ex | 2 ++ lib/plausible/stats/breakdown.ex | 2 +- lib/plausible/stats/imported.ex | 17 ++++++++++++++-- lib/plausible/stats/timeseries.ex | 2 +- lib/plausible/stats/util.ex | 20 ++++++++++++------- test/plausible/imported/imported_test.exs | 2 +- .../aggregate_test.exs | 2 +- .../api/stats_controller/top_stats_test.exs | 7 ++----- 10 files changed, 38 insertions(+), 22 deletions(-) diff --git a/assets/js/dashboard/stats/graph/top-stats.js b/assets/js/dashboard/stats/graph/top-stats.js index 66a8374a54c0..a233885091a9 100644 --- a/assets/js/dashboard/stats/graph/top-stats.js +++ b/assets/js/dashboard/stats/graph/top-stats.js @@ -69,8 +69,7 @@ export default class TopStats extends React.Component { let statName = stat.name.toLowerCase() statName = stat.value === 1 ? statName.slice(0, -1) : statName - const { topStatData, lastLoadTimestamp } = this.props - const showingImported = topStatData?.imported_source && topStatData?.with_imported + const { lastLoadTimestamp } = this.props return (
@@ -84,7 +83,6 @@ export default class TopStats extends React.Component {
} {stat.name === 'Current visitors' &&

Last updated s ago

} - {stat.name === 'Views per visit' && showingImported &&

Based only on native data

} ) } diff --git a/lib/plausible/stats/aggregate.ex b/lib/plausible/stats/aggregate.ex index 1f14b87091d3..27865a26819d 100644 --- a/lib/plausible/stats/aggregate.ex +++ b/lib/plausible/stats/aggregate.ex @@ -63,7 +63,7 @@ defmodule Plausible.Stats.Aggregate do |> select_session_metrics(metrics, query) |> merge_imported(site, query, :aggregate, metrics) |> ClickhouseRepo.one() - |> remove_internal_visits_metric() + |> remove_internal_metrics() end defp aggregate_time_on_page(site, query) do diff --git a/lib/plausible/stats/base.ex b/lib/plausible/stats/base.ex index 84e1f8ca6a35..70f106a03ad7 100644 --- a/lib/plausible/stats/base.ex +++ b/lib/plausible/stats/base.ex @@ -335,6 +335,8 @@ defmodule Plausible.Stats.Base do def select_session_metrics(q, [:views_per_visit | rest], query) do from(s in q, select_merge: %{ + __internal_pageviews: fragment("toUInt32(sum(sign * pageviews))"), + __internal_visits: fragment("toUInt32(sum(sign))"), views_per_visit: fragment("ifNotFinite(round(sum(? * ?) / sum(?), 2), 0)", s.sign, s.pageviews, s.sign) } diff --git a/lib/plausible/stats/breakdown.ex b/lib/plausible/stats/breakdown.ex index 6586d6cfd661..9366916445f7 100644 --- a/lib/plausible/stats/breakdown.ex +++ b/lib/plausible/stats/breakdown.ex @@ -211,7 +211,7 @@ defmodule Plausible.Stats.Breakdown do |> apply_pagination(pagination) |> ClickhouseRepo.all() |> transform_keys(%{operating_system: :os}) - |> remove_internal_visits_metric(metrics) + |> remove_internal_metrics(metrics) end defp breakdown_events(_, _, _, [], _), do: [] diff --git a/lib/plausible/stats/imported.ex b/lib/plausible/stats/imported.ex index 8cfe02384842..a86a6f6dabe6 100644 --- a/lib/plausible/stats/imported.ex +++ b/lib/plausible/stats/imported.ex @@ -415,6 +415,15 @@ defmodule Plausible.Stats.Imported do |> select_imported_metrics(rest) end + defp select_imported_metrics(q, [:views_per_visit | rest]) do + q + |> select_merge([i], %{ + pageviews: sum(i.pageviews), + visits: sum(i.visits) + }) + |> select_imported_metrics(rest) + end + defp select_imported_metrics(q, [:visit_duration | rest]) do q |> select_merge([i], %{ @@ -462,8 +471,12 @@ defmodule Plausible.Stats.Imported do defp select_joined_metrics(q, [:views_per_visit | rest]) do q - |> select_merge([s, _i], %{ - views_per_visit: s.views_per_visit + |> select_merge([s, i], %{ + views_per_visit: + fragment( + "round(?,2)", + (s.__internal_pageviews + i.pageviews) / (s.__internal_visits + i.visits) + ) }) |> select_joined_metrics(rest) end diff --git a/lib/plausible/stats/timeseries.ex b/lib/plausible/stats/timeseries.ex index d87d354104a1..e0a6ddb41f6c 100644 --- a/lib/plausible/stats/timeseries.ex +++ b/lib/plausible/stats/timeseries.ex @@ -69,7 +69,7 @@ defmodule Plausible.Stats.Timeseries do |> select_session_metrics(metrics, query) |> Plausible.Stats.Imported.merge_imported_timeseries(site, query, metrics) |> ClickhouseRepo.all() - |> remove_internal_visits_metric(metrics) + |> remove_internal_metrics(metrics) end defp buckets(%Query{interval: "month"} = query) do diff --git a/lib/plausible/stats/util.ex b/lib/plausible/stats/util.ex index 046550660897..5db975665537 100644 --- a/lib/plausible/stats/util.ex +++ b/lib/plausible/stats/util.ex @@ -4,20 +4,26 @@ defmodule Plausible.Stats.Util do """ @doc """ - `__internal_visits` is fetched when querying bounce rate and visit duration, as it - is needed to calculate these from imported data. This function removes that metric + `__internal_visits` is fetched when querying bounce rate and visit duration and + `__internal_pageviews` is additionally fetched when querying views per visit + as they are needed to calculate these from imported data. This function removes these metrics from all entries in the results list. """ - def remove_internal_visits_metric(results, metrics) when is_list(results) do - if :bounce_rate in metrics or :visit_duration in metrics do + def remove_internal_metrics(results, metrics) when is_list(results) do + has_internal_metrics? = + Enum.any?(metrics, fn metric -> + metric in [:bounce_rate, :visit_duration, :views_per_visit] + end) + + if has_internal_metrics? do results - |> Enum.map(&remove_internal_visits_metric/1) + |> Enum.map(&remove_internal_metrics/1) else results end end - def remove_internal_visits_metric(result) when is_map(result) do - Map.delete(result, :__internal_visits) + def remove_internal_metrics(result) when is_map(result) do + Map.drop(result, [:__internal_visits, :__internal_pageviews]) end end diff --git a/test/plausible/imported/imported_test.exs b/test/plausible/imported/imported_test.exs index 03ccfb0c0d31..5084407a0820 100644 --- a/test/plausible/imported/imported_test.exs +++ b/test/plausible/imported/imported_test.exs @@ -1211,7 +1211,7 @@ defmodule Plausible.ImportedTest do %{"name" => "Unique visitors", "value" => 1}, %{"name" => "Total visits", "value" => 1}, %{"name" => "Total pageviews", "value" => 1}, - %{"name" => "Views per visit", "value" => +0.0}, + %{"name" => "Views per visit", "value" => 1.0}, %{"name" => "Bounce rate", "value" => 0}, %{"name" => "Visit duration", "value" => 60} ] diff --git a/test/plausible_web/controllers/api/external_stats_controller/aggregate_test.exs b/test/plausible_web/controllers/api/external_stats_controller/aggregate_test.exs index b7e84b7a5833..cf8bcfe0463e 100644 --- a/test/plausible_web/controllers/api/external_stats_controller/aggregate_test.exs +++ b/test/plausible_web/controllers/api/external_stats_controller/aggregate_test.exs @@ -358,7 +358,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController.AggregateTest do "visits" => %{"value" => 5, "change" => 150}, "pageviews" => %{"value" => 9, "change" => -10}, "bounce_rate" => %{"value" => 40, "change" => -20}, - "views_per_visit" => %{"value" => 1.0, "change" => 100}, + "views_per_visit" => %{"value" => 1.8, "change" => -64}, "visit_duration" => %{"value" => 20, "change" => -80} } end diff --git a/test/plausible_web/controllers/api/stats_controller/top_stats_test.exs b/test/plausible_web/controllers/api/stats_controller/top_stats_test.exs index 66b078b44cbf..c8fa18ecc626 100644 --- a/test/plausible_web/controllers/api/stats_controller/top_stats_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/top_stats_test.exs @@ -387,10 +387,7 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do describe "GET /api/stats/top-stats - with imported data" do setup [:create_user, :log_in, :create_new_site, :add_imported_data] - test "merges imported data into all top stat metrics except views_per_visit", %{ - conn: conn, - site: site - } do + test "merges imported data into all top stat metrics", %{conn: conn, site: site} do populate_stats(site, [ build(:pageview, user_id: @user_id, @@ -418,7 +415,7 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do %{"name" => "Unique visitors", "value" => 3}, %{"name" => "Total visits", "value" => 3}, %{"name" => "Total pageviews", "value" => 4}, - %{"name" => "Views per visit", "value" => 1.5}, + %{"name" => "Views per visit", "value" => 1.33}, %{"name" => "Bounce rate", "value" => 33}, %{"name" => "Visit duration", "value" => 303} ]