Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow imported views per visit #3792

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions assets/js/dashboard/stats/graph/top-stats.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<div>
Expand All @@ -84,7 +83,6 @@ export default class TopStats extends React.Component {
</div>}

{stat.name === 'Current visitors' && <p className="font-normal text-xs">Last updated <SecondsSinceLastLoad lastLoadTimestamp={lastLoadTimestamp}/>s ago</p>}
{stat.name === 'Views per visit' && showingImported && <p className="font-normal text-xs whitespace-nowrap">Based only on native data</p>}
</div>
)
}
Expand Down
2 changes: 1 addition & 1 deletion lib/plausible/stats/aggregate.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions lib/plausible/stats/base.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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))"),
ukutaht marked this conversation as resolved.
Show resolved Hide resolved
__internal_visits: fragment("toUInt32(sum(sign))"),
views_per_visit:
fragment("ifNotFinite(round(sum(? * ?) / sum(?), 2), 0)", s.sign, s.pageviews, s.sign)
}
Expand Down
2 changes: 1 addition & 1 deletion lib/plausible/stats/breakdown.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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: []
Expand Down
17 changes: 15 additions & 2 deletions lib/plausible/stats/imported.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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], %{
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lib/plausible/stats/timeseries.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 13 additions & 7 deletions lib/plausible/stats/util.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion test/plausible/imported/imported_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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}
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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}
]
Expand Down