Fix division by zero in imported queries (#3890)

* prevent division by 0 in merge_imported queries

* Revert "fix bounce_rate change bug (#3886)"

This reverts commit 6eef32a8ff.

After 02aa0b2, we can keep on assuming that bounce rate is always numeric.
This commit is contained in:
RobertJoonas 2024-03-13 10:37:14 +00:00 committed by GitHub
parent 4d7d88cfec
commit e8f3946dde
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 45 additions and 6 deletions

View File

@ -4,9 +4,7 @@ defmodule Plausible.Stats.Compare do
end
def calculate_change(:bounce_rate, old_count, new_count) do
if is_integer(old_count) && is_integer(new_count) && old_count > 0 do
new_count - old_count
end
if old_count > 0, do: new_count - old_count
end
def calculate_change(_metric, old_count, new_count) do

View File

@ -488,7 +488,15 @@ defmodule Plausible.Stats.Imported do
|> select_merge([s, i], %{
views_per_visit:
fragment(
"round((? + ? * coalesce(?, 0)) / (coalesce(?, 0) + coalesce(?, 0)), 2)",
"""
if(
coalesce(?, 0) + coalesce(?, 0) > 0,
round((? + ? * coalesce(?, 0)) / (coalesce(?, 0) + coalesce(?, 0)), 2),
0
)
""",
s.__internal_visits,
i.__internal_visits,
i.pageviews,
s.views_per_visit,
s.__internal_visits,
@ -504,7 +512,15 @@ defmodule Plausible.Stats.Imported do
|> select_merge([s, i], %{
bounce_rate:
fragment(
"round(100 * (coalesce(?, 0) + coalesce((? * ? / 100), 0)) / (coalesce(?, 0) + coalesce(?, 0)))",
"""
if(
coalesce(?, 0) + coalesce(?, 0) > 0,
round(100 * (coalesce(?, 0) + coalesce((? * ? / 100), 0)) / (coalesce(?, 0) + coalesce(?, 0))),
0
)
""",
s.__internal_visits,
i.__internal_visits,
i.bounces,
s.bounce_rate,
s.__internal_visits,
@ -520,7 +536,15 @@ defmodule Plausible.Stats.Imported do
|> select_merge([s, i], %{
visit_duration:
fragment(
"round((? + ? * ?) / (? + ?), 1)",
"""
if(
? + ? > 0,
round((? + ? * ?) / (? + ?), 1),
0
)
""",
s.__internal_visits,
i.__internal_visits,
i.visit_duration,
s.visit_duration,
s.__internal_visits,

View File

@ -423,6 +423,23 @@ 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 "returns divisible metrics as 0 when no stats exist", %{
site: site,
conn: conn
} do
conn =
get(
conn,
"/api/stats/#{site.domain}/top-stats?period=day&date=2021-01-01&with_imported=true"
)
res = json_response(conn, 200)
assert %{"name" => "Bounce rate", "value" => 0} in res["top_stats"]
assert %{"name" => "Views per visit", "value" => 0.0} in res["top_stats"]
assert %{"name" => "Visit duration", "value" => 0} in res["top_stats"]
end
test "merges imported data into all top stat metrics", %{
conn: conn,
site: site