Re-apply Move conversion_rate logic from elixir to clickhouse (#3924)

* Revert "Revert "Move conversion_rate logic from elixir to clickhouse (#3887)"…"

This reverts commit 253fb5d67d.

* Fix issue with missing columns

The issue came from refactoring event:goal UNION ALL logic and trying to move
name select from first to last. If any other tables were joined, the incorrect
item would be used as an array index, causing this issue.

Added a relevant test.
This commit is contained in:
Karl-Aksel Puulmann 2024-03-21 10:48:41 +02:00 committed by GitHub
parent 253fb5d67d
commit c219652dae
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 390 additions and 343 deletions

View File

@ -10,21 +10,17 @@ defmodule Plausible.Stats.Goal.Revenue do
@revenue_metrics
end
def total_revenue_query(query) do
from(e in query,
select_merge: %{
total_revenue:
fragment("toDecimal64(sum(?) * any(_sample_factor), 3)", e.revenue_reporting_amount)
}
def total_revenue_query() do
dynamic(
[e],
fragment("toDecimal64(sum(?) * any(_sample_factor), 3)", e.revenue_reporting_amount)
)
end
def average_revenue_query(query) do
from(e in query,
select_merge: %{
average_revenue:
fragment("toDecimal64(avg(?) * any(_sample_factor), 3)", e.revenue_reporting_amount)
}
def average_revenue_query() do
dynamic(
[e],
fragment("toDecimal64(avg(?) * any(_sample_factor), 3)", e.revenue_reporting_amount)
)
end

View File

@ -11,7 +11,9 @@ defmodule Plausible.Stats.Aggregate do
:visitors,
:pageviews,
:events,
:sample_percent
:sample_percent,
:conversion_rate,
:total_visitors
] ++ @revenue_metrics
@session_metrics [:visits, :bounce_rate, :visit_duration, :views_per_visit, :sample_percent]
@ -45,7 +47,6 @@ defmodule Plausible.Stats.Aggregate do
Plausible.ClickhouseRepo.parallel_tasks([session_task, event_task, time_on_page_task])
|> Enum.reduce(%{}, fn aggregate, task_result -> Map.merge(aggregate, task_result) end)
|> maybe_put_cr(site, query, metrics)
|> Util.keep_requested_metrics(metrics)
|> cast_revenue_metrics_to_money(currency)
|> Enum.map(&maybe_round_value/1)
@ -53,45 +54,20 @@ defmodule Plausible.Stats.Aggregate do
|> Enum.into(%{})
end
defp maybe_put_cr(aggregate_result, site, query, metrics) do
if :conversion_rate in metrics do
all =
query
|> Query.remove_event_filters([:goal, :props])
|> then(fn query -> aggregate_events(site, query, [:visitors]) end)
|> Map.fetch!(:visitors)
converted = aggregate_result.visitors
cr = Util.calculate_cr(all, converted)
aggregate_result = Map.put(aggregate_result, :conversion_rate, cr)
if :total_visitors in metrics do
Map.put(aggregate_result, :total_visitors, all)
else
aggregate_result
end
else
aggregate_result
end
end
defp aggregate_events(_, _, []), do: %{}
defp aggregate_events(site, query, metrics) do
from(e in base_event_query(site, query), select: %{})
|> select_event_metrics(metrics)
from(e in base_event_query(site, query), select: ^select_event_metrics(metrics))
|> merge_imported(site, query, :aggregate, metrics)
|> maybe_add_conversion_rate(site, query, metrics, include_imported: query.include_imported)
|> ClickhouseRepo.one()
end
defp aggregate_sessions(_, _, []), do: %{}
defp aggregate_sessions(site, query, metrics) do
from(e in query_sessions(site, query), select: %{})
from(e in query_sessions(site, query), select: ^select_session_metrics(metrics, query))
|> filter_converted_sessions(site, query)
|> select_session_metrics(metrics, query)
|> merge_imported(site, query, :aggregate, metrics)
|> ClickhouseRepo.one()
|> Util.keep_requested_metrics(metrics)

View File

@ -209,163 +209,152 @@ defmodule Plausible.Stats.Base do
def apply_entry_prop_filter(sessions_q, _, _), do: sessions_q
def select_event_metrics(q, []), do: q
def select_event_metrics(metrics) do
metrics
|> Enum.map(&select_event_metric/1)
|> Enum.reduce(%{}, &Map.merge/2)
end
def select_event_metrics(q, [:pageviews | rest]) do
from(e in q,
select_merge: %{
pageviews:
defp select_event_metric(:pageviews) do
%{
pageviews:
dynamic(
[e],
fragment("toUInt64(round(countIf(? = 'pageview') * any(_sample_factor)))", e.name)
}
)
|> select_event_metrics(rest)
)
}
end
def select_event_metrics(q, [:events | rest]) do
from(e in q,
select_merge: %{events: fragment("toUInt64(round(count(*) * any(_sample_factor)))")}
)
|> select_event_metrics(rest)
defp select_event_metric(:events) do
%{
events: dynamic([], fragment("toUInt64(round(count(*) * any(_sample_factor)))"))
}
end
def select_event_metrics(q, [:visitors | rest]) do
from(e in q,
select_merge: %{
visitors: selected_as(fragment(@uniq_users_expression, e.user_id), :visitors)
}
)
|> select_event_metrics(rest)
defp select_event_metric(:visitors) do
%{
visitors: dynamic([e], selected_as(fragment(@uniq_users_expression, e.user_id), :visitors))
}
end
on_full_build do
def select_event_metrics(q, [:total_revenue | rest]) do
q
|> Plausible.Stats.Goal.Revenue.total_revenue_query()
|> select_event_metrics(rest)
defp select_event_metric(:total_revenue) do
%{total_revenue: Plausible.Stats.Goal.Revenue.total_revenue_query()}
end
def select_event_metrics(q, [:average_revenue | rest]) do
q
|> Plausible.Stats.Goal.Revenue.average_revenue_query()
|> select_event_metrics(rest)
defp select_event_metric(:average_revenue) do
%{average_revenue: Plausible.Stats.Goal.Revenue.average_revenue_query()}
end
end
def select_event_metrics(q, [:sample_percent | rest]) do
from(e in q,
select_merge: %{
sample_percent:
defp select_event_metric(:sample_percent) do
%{
sample_percent:
dynamic(
[],
fragment("if(any(_sample_factor) > 1, round(100 / any(_sample_factor)), 100)")
}
)
|> select_event_metrics(rest)
)
}
end
def select_event_metrics(q, [:percentage | rest]) do
q |> select_event_metrics(rest)
defp select_event_metric(:percentage), do: %{}
defp select_event_metric(:conversion_rate), do: %{}
defp select_event_metric(:total_visitors), do: %{}
defp select_event_metric(unknown), do: raise("Unknown metric: #{unknown}")
def select_session_metrics(metrics, query) do
metrics
|> Enum.map(&select_session_metric(&1, query))
|> Enum.reduce(%{}, &Map.merge/2)
end
def select_event_metrics(_, [unknown | _]), do: raise("Unknown metric: #{unknown}")
def select_session_metrics(q, [], _query), do: q
def select_session_metrics(q, [:bounce_rate | rest], query) do
defp select_session_metric(:bounce_rate, query) do
condition = dynamic_filter_condition(query, "event:page", :entry_page)
from(s in q,
select_merge:
^%{
bounce_rate:
dynamic(
[],
fragment(
"toUInt32(ifNotFinite(round(sumIf(is_bounce * sign, ?) / sumIf(sign, ?) * 100), 0))",
^condition,
^condition
)
),
__internal_visits: dynamic([], fragment("toUInt32(sum(sign))"))
}
)
|> select_session_metrics(rest, query)
%{
bounce_rate:
dynamic(
[],
fragment(
"toUInt32(ifNotFinite(round(sumIf(is_bounce * sign, ?) / sumIf(sign, ?) * 100), 0))",
^condition,
^condition
)
),
__internal_visits: dynamic([], fragment("toUInt32(sum(sign))"))
}
end
def select_session_metrics(q, [:visits | rest], query) do
from(s in q,
select_merge: %{
visits: fragment("toUInt64(round(sum(?) * any(_sample_factor)))", s.sign)
}
)
|> select_session_metrics(rest, query)
defp select_session_metric(:visits, _query) do
%{
visits: dynamic([s], fragment("toUInt64(round(sum(?) * any(_sample_factor)))", s.sign))
}
end
def select_session_metrics(q, [:pageviews | rest], query) do
from(s in q,
select_merge: %{
pageviews:
defp select_session_metric(:pageviews, _query) do
%{
pageviews:
dynamic(
[s],
fragment("toUInt64(round(sum(? * ?) * any(_sample_factor)))", s.sign, s.pageviews)
}
)
|> select_session_metrics(rest, query)
)
}
end
def select_session_metrics(q, [:events | rest], query) do
from(s in q,
select_merge: %{
events: fragment("toUInt64(round(sum(? * ?) * any(_sample_factor)))", s.sign, s.events)
}
)
|> select_session_metrics(rest, query)
defp select_session_metric(:events, _query) do
%{
events:
dynamic(
[s],
fragment("toUInt64(round(sum(? * ?) * any(_sample_factor)))", s.sign, s.events)
)
}
end
def select_session_metrics(q, [:visitors | rest], query) do
from(s in q,
select_merge: %{
visitors:
defp select_session_metric(:visitors, _query) do
%{
visitors:
dynamic(
[s],
selected_as(
fragment("toUInt64(round(uniq(?) * any(_sample_factor)))", s.user_id),
:visitors
)
}
)
|> select_session_metrics(rest, query)
)
}
end
def select_session_metrics(q, [:visit_duration | rest], query) do
from(s in q,
select_merge: %{
:visit_duration =>
fragment("toUInt32(ifNotFinite(round(sum(duration * sign) / sum(sign)), 0))"),
__internal_visits: fragment("toUInt32(sum(sign))")
}
)
|> select_session_metrics(rest, query)
defp select_session_metric(:visit_duration, _query) do
%{
visit_duration:
dynamic([], fragment("toUInt32(ifNotFinite(round(sum(duration * sign) / sum(sign)), 0))")),
__internal_visits: dynamic([], fragment("toUInt32(sum(sign))"))
}
end
def select_session_metrics(q, [:views_per_visit | rest], query) do
from(s in q,
select_merge: %{
views_per_visit:
fragment("ifNotFinite(round(sum(? * ?) / sum(?), 2), 0)", s.sign, s.pageviews, s.sign),
__internal_visits: fragment("toUInt32(sum(sign))")
}
)
|> select_session_metrics(rest, query)
defp select_session_metric(:views_per_visit, _query) do
%{
views_per_visit:
dynamic(
[s],
fragment("ifNotFinite(round(sum(? * ?) / sum(?), 2), 0)", s.sign, s.pageviews, s.sign)
),
__internal_visits: dynamic([], fragment("toUInt32(sum(sign))"))
}
end
def select_session_metrics(q, [:sample_percent | rest], query) do
from(e in q,
select_merge: %{
sample_percent:
defp select_session_metric(:sample_percent, _query) do
%{
sample_percent:
dynamic(
[],
fragment("if(any(_sample_factor) > 1, round(100 / any(_sample_factor)), 100)")
}
)
|> select_session_metrics(rest, query)
)
}
end
def select_session_metrics(q, [:percentage | rest], query) do
q |> select_session_metrics(rest, query)
end
defp select_session_metric(:percentage, _query), do: %{}
def dynamic_filter_condition(query, filter_key, db_field) do
case query && query.filters && query.filters[filter_key] do
@ -586,7 +575,7 @@ defmodule Plausible.Stats.Base do
|> select([e], total_visitors: fragment(@uniq_users_expression, e.user_id))
end
defp total_visitors_subquery(site, %Query{include_imported: true} = query) do
defp total_visitors_subquery(site, query, true) do
dynamic(
[e],
selected_as(
@ -597,14 +586,16 @@ defmodule Plausible.Stats.Base do
)
end
defp total_visitors_subquery(site, query) do
defp total_visitors_subquery(site, query, false) do
dynamic([e], selected_as(subquery(total_visitors(site, query)), :__total_visitors))
end
def add_percentage_metric(q, site, query, metrics) do
if :percentage in metrics do
q
|> select_merge(^%{__total_visitors: total_visitors_subquery(site, query)})
|> select_merge(
^%{__total_visitors: total_visitors_subquery(site, query, query.include_imported)}
)
|> select_merge(%{
percentage:
fragment(
@ -618,4 +609,32 @@ defmodule Plausible.Stats.Base do
q
end
end
# Adds conversion_rate metric to query, calculated as
# X / Y where Y is the same breakdown value without goal or props
# filters.
def maybe_add_conversion_rate(q, site, query, metrics, opts) do
if :conversion_rate in metrics do
include_imported = Keyword.fetch!(opts, :include_imported)
total_query = query |> Query.remove_event_filters([:goal, :props])
# :TRICKY: Subquery is used due to event:goal breakdown above doing an UNION ALL
subquery(q)
|> select_merge(
^%{total_visitors: total_visitors_subquery(site, total_query, include_imported)}
)
|> select_merge([e], %{
conversion_rate:
fragment(
"if(? > 0, round(? / ? * 100, 1), 0)",
selected_as(:__total_visitors),
e.visitors,
selected_as(:__total_visitors)
)
})
else
q
end
end
end

View File

@ -4,6 +4,7 @@ defmodule Plausible.Stats.Breakdown do
use Plausible.Stats.Fragments
import Plausible.Stats.{Base, Imported}
import Ecto.Query
require OpenTelemetry.Tracer, as: Tracer
alias Plausible.Stats.{Query, Util}
@ -52,50 +53,72 @@ defmodule Plausible.Stats.Breakdown do
metrics_to_select = Util.maybe_add_visitors_metric(metrics) -- @computed_metrics
event_results =
event_q =
if Enum.any?(event_goals) do
site
|> breakdown(event_query, "event:name", metrics_to_select, pagination, skip_tracing: true)
|> transform_keys(%{name: :goal})
|> cast_revenue_metrics_to_money(revenue_goals)
|> breakdown_events(event_query, "event:name", metrics_to_select)
|> apply_pagination(pagination)
else
[]
nil
end
{limit, page} = pagination
offset = (page - 1) * limit
page_results =
page_q =
if Enum.any?(pageview_goals) do
page_exprs = Enum.map(pageview_goals, & &1.page_path)
page_regexes = Enum.map(page_exprs, &page_regex/1)
select_columns = metrics_to_select |> select_event_metrics |> mark_revenue_as_nil
from(e in base_event_query(site, query),
order_by: [desc: fragment("uniq(?)", e.user_id)],
limit: ^limit,
offset: ^offset,
where:
fragment(
"notEmpty(multiMatchAllIndices(?, ?) as indices)",
e.pathname,
^page_regexes
) and e.name == "pageview",
group_by: fragment("index"),
array_join: index in fragment("indices"),
group_by: index,
select: %{
index: fragment("arrayJoin(indices) as index"),
goal: fragment("concat('Visit ', ?[index])", ^page_exprs)
name: fragment("concat('Visit ', ?[?])", ^page_exprs, index)
}
)
|> select_event_metrics(metrics_to_select -- @revenue_metrics)
|> ClickhouseRepo.all()
|> Enum.map(fn row -> Map.delete(row, :index) end)
|> select_merge(^select_columns)
|> apply_pagination(pagination)
else
[]
nil
end
zip_results(event_results, page_results, :goal, metrics_to_select)
|> maybe_add_cr(site, query, nil, metrics)
|> Util.keep_requested_metrics(metrics)
full_q =
case {event_q, page_q} do
{nil, nil} ->
nil
{event_q, nil} ->
event_q
{nil, page_q} ->
page_q
{event_q, page_q} ->
from(
e in subquery(union_all(event_q, ^page_q)),
# :TODO: Handle other orderings
order_by: [desc: e.visitors]
)
|> apply_pagination(pagination)
end
if full_q do
full_q
|> maybe_add_conversion_rate(site, query, metrics, include_imported: false)
|> ClickhouseRepo.all()
|> transform_keys(%{name: :goal})
|> cast_revenue_metrics_to_money(revenue_goals)
|> Util.keep_requested_metrics(metrics)
else
[]
end
end
def breakdown(site, query, "event:props:" <> custom_prop = property, metrics, pagination, opts) do
@ -110,11 +133,11 @@ defmodule Plausible.Stats.Breakdown do
if !Keyword.get(opts, :skip_tracing), do: trace(query, property, metrics)
breakdown_events(site, query, "event:props:" <> custom_prop, metrics_to_select, pagination)
breakdown_events(site, query, "event:props:" <> custom_prop, metrics_to_select)
|> maybe_add_conversion_rate(site, query, metrics, include_imported: false)
|> paginate_and_execute(metrics, pagination)
|> transform_keys(%{name: custom_prop})
|> Enum.map(&cast_revenue_metrics_to_money(&1, currency))
|> sort_results(metrics_to_select)
|> maybe_add_cr(site, query, nil, metrics)
|> Util.keep_requested_metrics(metrics)
end
def breakdown(site, query, "event:page" = property, metrics, pagination, opts) do
@ -125,10 +148,10 @@ defmodule Plausible.Stats.Breakdown do
event_result =
site
|> breakdown_events(query, "event:page", event_metrics, pagination)
|> breakdown_events(query, property, event_metrics)
|> maybe_add_group_conversion_rate(&breakdown_events/4, site, query, property, metrics)
|> paginate_and_execute(metrics, pagination)
|> maybe_add_time_on_page(site, query, metrics)
|> maybe_add_cr(site, query, property, metrics, pagination)
|> Util.keep_requested_metrics(metrics)
session_metrics = Enum.filter(metrics, &(&1 in @session_metrics))
@ -149,7 +172,8 @@ defmodule Plausible.Stats.Breakdown do
{limit, _page} = pagination
session_result =
breakdown_sessions(site, new_query, "visit:entry_page", session_metrics, {limit, 1})
breakdown_sessions(site, new_query, "visit:entry_page", session_metrics)
|> paginate_and_execute(session_metrics, {limit, 1})
|> transform_keys(%{entry_page: :page})
metrics = metrics ++ [:page]
@ -166,7 +190,9 @@ defmodule Plausible.Stats.Breakdown do
def breakdown(site, query, "event:name" = property, metrics, pagination, opts) do
if !Keyword.get(opts, :skip_tracing), do: trace(query, property, metrics)
breakdown_events(site, query, property, metrics, pagination)
breakdown_events(site, query, property, metrics)
|> paginate_and_execute(metrics, pagination)
end
def breakdown(site, query, property, metrics, pagination, opts) do
@ -174,9 +200,9 @@ defmodule Plausible.Stats.Breakdown do
metrics_to_select = Util.maybe_add_visitors_metric(metrics) -- @computed_metrics
breakdown_sessions(site, query, property, metrics_to_select, pagination)
|> maybe_add_cr(site, query, property, metrics, pagination)
|> Util.keep_requested_metrics(metrics)
breakdown_sessions(site, query, property, metrics_to_select)
|> maybe_add_group_conversion_rate(&breakdown_sessions/4, site, query, property, metrics)
|> paginate_and_execute(metrics, pagination)
end
defp zip_results(event_result, session_result, property, metrics) do
@ -197,35 +223,32 @@ defmodule Plausible.Stats.Breakdown do
|> sort_results(metrics)
end
defp breakdown_sessions(_, _, _, [], _), do: []
defp breakdown_sessions(site, query, property, metrics, pagination) do
defp breakdown_sessions(site, query, property, metrics) do
from(s in query_sessions(site, query),
order_by: [desc: fragment("uniq(?)", s.user_id)],
select: %{}
select: ^select_session_metrics(metrics, query)
)
|> filter_converted_sessions(site, query)
|> do_group_by(property)
|> select_session_metrics(metrics, query)
|> merge_imported(site, query, property, metrics)
|> add_percentage_metric(site, query, metrics)
|> apply_pagination(pagination)
|> ClickhouseRepo.all()
|> transform_keys(%{operating_system: :os})
|> Util.keep_requested_metrics(metrics)
end
defp breakdown_events(_, _, _, [], _), do: []
defp breakdown_events(site, query, property, metrics, pagination) do
defp breakdown_events(site, query, property, metrics) do
from(e in base_event_query(site, query),
order_by: [desc: fragment("uniq(?)", e.user_id)],
select: %{}
)
|> do_group_by(property)
|> select_event_metrics(metrics)
|> select_merge(^select_event_metrics(metrics))
|> merge_imported(site, query, property, metrics)
|> add_percentage_metric(site, query, metrics)
end
defp paginate_and_execute(_, [], _), do: []
defp paginate_and_execute(q, metrics, pagination) do
q
|> apply_pagination(pagination)
|> ClickhouseRepo.all()
|> transform_keys(%{operating_system: :os})
@ -423,18 +446,18 @@ defmodule Plausible.Stats.Breakdown do
from(
e in q,
select_merge: %{
^prop =>
name:
selected_as(
fragment(
"if(not empty(?), ?, '(none)')",
get_by_key(e, :meta, ^prop),
get_by_key(e, :meta, ^prop)
),
:breakdown_prop_value
:name
)
},
group_by: selected_as(:breakdown_prop_value),
order_by: {:asc, selected_as(:breakdown_prop_value)}
group_by: selected_as(:name),
order_by: {:asc, selected_as(:name)}
)
end
@ -652,130 +675,26 @@ defmodule Plausible.Stats.Breakdown do
)
end
defp maybe_add_cr(breakdown_results, site, query, property, metrics, pagination \\ nil) do
cond do
:conversion_rate not in metrics -> breakdown_results
Enum.empty?(breakdown_results) -> breakdown_results
is_nil(property) -> add_absolute_cr(breakdown_results, site, query)
true -> add_cr(breakdown_results, site, query, property, metrics, pagination)
end
defp group_by_field_names("event:props:" <> _prop), do: [:name]
defp group_by_field_names("visit:os"), do: [:operating_system]
defp group_by_field_names("visit:os_version"), do: [:os, :os_version]
defp group_by_field_names("visit:browser_version"), do: [:browser, :browser_version]
defp group_by_field_names(property), do: [Plausible.Stats.Filters.without_prefix(property)]
defp on_matches_group_by(fields) do
Enum.reduce(fields, nil, &fields_equal/2)
end
# This function injects a conversion_rate metric into every
# breakdown result map. It is calculated as X / Y, where:
#
# * X is the number of conversions for a breakdown
# result (conversion = number of visitors who
# completed the filtered goal with the filtered
# custom properties).
#
# * Y is the number of all visitors for this breakdown
# result without the `event:goal` and `event:props:*`
# filters.
defp add_cr(breakdown_results, site, query, property, metrics, pagination) do
property_atom = Plausible.Stats.Filters.without_prefix(property)
items =
Enum.map(breakdown_results, fn item -> Map.fetch!(item, property_atom) end)
query_without_goal =
query
|> Query.put_filter(property, {:member, items})
|> Query.remove_event_filters([:goal, :props])
# Here, we're always only interested in the first page of results
# - the :member filter makes sure that the results always match with
# the items in the given breakdown_results list
page = 1
# For browser/os versions we need to fetch a lot more entries than the
# pagination limit. This is because many entries can correspond to a
# single version number and we need to make sure that the results
# without goal filter will include all those combinations of browsers/os-s
# and their versions that were present in the `breakdown_results` array.
{pagination_limit, find_match_fn} =
case property_atom do
:browser_version ->
pagination_limit = min(elem(pagination, 0) * 10, 3_000)
find_match_fn = fn total, conversion ->
total[:browser_version] == conversion[:browser_version] &&
total[:browser] == conversion[:browser]
end
{pagination_limit, find_match_fn}
:os_version ->
pagination_limit = min(elem(pagination, 0) * 5, 3_000)
find_match_fn = fn total, conversion ->
total[:os_version] == conversion[:os_version] &&
total[:os] == conversion[:os]
end
{pagination_limit, find_match_fn}
_ ->
{elem(pagination, 0),
fn total, conversion ->
total[property_atom] == conversion[property_atom]
end}
end
pagination = {pagination_limit, page}
res_without_goal = breakdown(site, query_without_goal, property, [:visitors], pagination)
Enum.map(breakdown_results, fn item ->
without_goal = Enum.find(res_without_goal, &find_match_fn.(&1, item))
{conversion_rate, total_visitors} =
if without_goal do
{Util.calculate_cr(without_goal.visitors, item.visitors), without_goal.visitors}
else
Sentry.capture_message(
"Unable to find a conversion_rate divisor from a breakdown response",
extra: %{
domain: site.domain,
query: inspect(query),
property: property,
pagination: inspect(pagination),
item_not_found: inspect(item)
}
)
{"N/A", "N/A"}
end
if :total_visitors in metrics do
item
|> Map.put(:conversion_rate, conversion_rate)
|> Map.put(:total_visitors, total_visitors)
else
Map.put(item, :conversion_rate, conversion_rate)
end
end)
defp outer_order_by(fields) do
Enum.map(fields, fn field_name -> {:asc, dynamic([q], field(q, ^field_name))} end)
end
# Similar to `add_cr/5`, injects a conversion_rate metric into
# every breakdown result. However, a single divisor is used in
# the CR calculation across all breakdown results. That is the
# number of visitors without `event:goal` and `event:props:*`
# filters.
#
# This is useful when we're only interested in the conversions
# themselves - not how well a certain property such as browser
# or page converted.
defp add_absolute_cr(breakdown_results, site, query) do
total_q = Query.remove_event_filters(query, [:goal, :props])
defp fields_equal(field_name, nil),
do: dynamic([a, b], field(a, ^field_name) == field(b, ^field_name))
%{visitors: %{value: total_visitors}} = Plausible.Stats.aggregate(site, total_q, [:visitors])
breakdown_results
|> Enum.map(fn goal ->
Map.put(goal, :conversion_rate, Util.calculate_cr(total_visitors, goal[:visitors]))
end)
end
defp fields_equal(field_name, condition),
do: dynamic([a, b], field(a, ^field_name) == field(b, ^field_name) and ^condition)
defp sort_results(results, metrics) do
Enum.sort_by(
@ -790,6 +709,54 @@ defmodule Plausible.Stats.Breakdown do
)
end
# This function injects a conversion_rate metric into
# a breakdown query. It is calculated as X / Y, where:
#
# * X is the number of conversions for a breakdown
# result (conversion = number of visitors who
# completed the filtered goal with the filtered
# custom properties).
#
# * Y is the number of all visitors for this breakdown
# result without the `event:goal` and `event:props:*`
# filters.
defp maybe_add_group_conversion_rate(q, breakdown_fn, site, query, property, metrics) do
if :conversion_rate in metrics do
breakdown_total_visitors_query = query |> Query.remove_event_filters([:goal, :props])
breakdown_total_visitors_q =
breakdown_fn.(site, breakdown_total_visitors_query, property, [:visitors])
from(e in subquery(q),
left_join: c in subquery(breakdown_total_visitors_q),
on: ^on_matches_group_by(group_by_field_names(property)),
select_merge: %{
total_visitors: c.visitors,
conversion_rate:
fragment(
"if(? > 0, round(? / ? * 100, 1), 0)",
c.visitors,
e.visitors,
c.visitors
)
},
order_by: [desc: e.visitors],
order_by: ^outer_order_by(group_by_field_names(property))
)
else
q
end
end
# When querying custom event goals and pageviewgoals together, UNION ALL is used
# so the same fields must be present on both sides of the union. This change to the
# query will ensure that we don't unnecessarily read revenue column for pageview goals
defp mark_revenue_as_nil(select_columns) do
select_columns
|> Map.replace(:total_revenue, nil)
|> Map.replace(:average_revenue, nil)
end
defp sorting_key(metrics) do
if Enum.member?(metrics, :visitors), do: :visitors, else: List.first(metrics)
end
@ -807,8 +774,8 @@ defmodule Plausible.Stats.Breakdown do
offset = (page - 1) * limit
q
|> Ecto.Query.limit(^limit)
|> Ecto.Query.offset(^offset)
|> limit(^limit)
|> offset(^offset)
end
defp trace(query, property, metrics) do

View File

@ -53,9 +53,8 @@ defmodule Plausible.Stats.Timeseries do
defp events_timeseries(_, _, []), do: []
defp events_timeseries(site, query, metrics) do
from(e in base_event_query(site, query), select: %{})
from(e in base_event_query(site, query), select: ^select_event_metrics(metrics))
|> select_bucket(site, query)
|> select_event_metrics(metrics)
|> Plausible.Stats.Imported.merge_imported_timeseries(site, query, metrics)
|> ClickhouseRepo.all()
end
@ -63,10 +62,9 @@ defmodule Plausible.Stats.Timeseries do
defp sessions_timeseries(_, _, []), do: []
defp sessions_timeseries(site, query, metrics) do
from(e in query_sessions(site, query), select: %{})
from(e in query_sessions(site, query), select: ^select_session_metrics(metrics, query))
|> filter_converted_sessions(site, query)
|> select_bucket(site, query)
|> select_session_metrics(metrics, query)
|> Plausible.Stats.Imported.merge_imported_timeseries(site, query, metrics)
|> ClickhouseRepo.all()
|> Util.keep_requested_metrics(metrics)

View File

@ -7,7 +7,8 @@ defmodule Plausible.Stats.Util do
:__internal_visits,
:visitors,
:__total_visitors,
:__breakdown_value
:__breakdown_value,
:total_visitors
]
@doc """
@ -48,12 +49,4 @@ defmodule Plausible.Stats.Util do
metrics
end
end
def calculate_cr(nil, _converted_visitors), do: nil
def calculate_cr(unique_visitors, converted_visitors) do
if unique_visitors > 0,
do: Float.round(converted_visitors / unique_visitors * 100, 1),
else: 0.0
end
end

View File

@ -1569,5 +1569,21 @@ defmodule PlausibleWeb.Api.ExternalStatsController.AggregateTest do
} =
json_response(conn, 200)["results"]
end
test "conversion_rate for the filtered goal is 0 when no stats exist", %{
conn: conn,
site: site
} do
insert(:goal, %{site: site, event_name: "Signup"})
conn =
get(conn, "/api/v1/stats/aggregate", %{
"site_id" => site.domain,
"metrics" => "conversion_rate",
"filters" => "event:goal==Signup"
})
assert json_response(conn, 200)["results"] == %{"conversion_rate" => %{"value" => 0}}
end
end
end

View File

@ -219,6 +219,52 @@ defmodule PlausibleWeb.Api.StatsController.ConversionsTest do
assert resp == []
end
test "filtering by session attribute and multiple goals", %{
conn: conn,
site: site
} do
populate_stats(site, [
build(:event,
user_id: @user_id,
name: "Payment",
browser: "Firefox"
),
build(:event,
user_id: @user_id,
name: "Payment",
browser: "Firefox"
),
build(:event,
name: "Payment",
browser: "Chrome"
),
build(:event, name: "Payment"),
build(:pageview, browser: "Firefox", pathname: "/"),
build(:pageview, browser: "Firefox", pathname: "/register")
])
insert(:goal, %{site: site, event_name: "Payment"})
insert(:goal, %{site: site, page_path: "/register"})
filters = Jason.encode!(%{browser: "Firefox"})
conn = get(conn, "/api/stats/#{site.domain}/conversions?period=day&filters=#{filters}")
assert json_response(conn, 200) == [
%{
"name" => "Payment",
"visitors" => 1,
"events" => 2,
"conversion_rate" => 33.3
},
%{
"name" => "Visit /register",
"visitors" => 1,
"events" => 1,
"conversion_rate" => 33.3
}
]
end
@tag :full_build_only
test "returns formatted average and total values for a conversion with revenue value", %{
conn: conn,
@ -580,6 +626,42 @@ defmodule PlausibleWeb.Api.StatsController.ConversionsTest do
}
]
end
test "conversion_rate for goals should not be calculated with imported data", %{
conn: conn,
site: site
} do
site =
site
|> Plausible.Site.start_import(~D[2005-01-01], Timex.today(), "Google Analytics", "ok")
|> Plausible.Repo.update!()
populate_stats(site, [
build(:pageview, pathname: "/"),
build(:pageview, pathname: "/another"),
build(:pageview, pathname: "/blog/post-1"),
build(:pageview, pathname: "/blog/post-2"),
build(:imported_pages, page: "/blog/post-1"),
build(:imported_visitors)
])
insert(:goal, %{site: site, page_path: "/blog**"})
conn =
get(
conn,
"/api/stats/#{site.domain}/conversions?period=day"
)
assert json_response(conn, 200) == [
%{
"name" => "Visit /blog**",
"visitors" => 2,
"events" => 2,
"conversion_rate" => 50
}
]
end
end
describe "GET /api/stats/:domain/conversions - with goal and prop=(none) filter" do