From a6b1a6ebc79dbfd826e805490ae17157e78eaa33 Mon Sep 17 00:00:00 2001 From: RobertJoonas <56999674+RobertJoonas@users.noreply.github.com> Date: Thu, 15 Feb 2024 12:10:08 +0000 Subject: [PATCH] Unify percentage change for CR and bounce_rate (#3781) * Fix conversion rate change calculation The change in conversion rate should be calculated similar to bounce rate. For example, an increase of 25% -> 50% should not be a 100% change, but a 25% change instead. * Use the same comparison function in Stats API and dashboard API This commit fixes a bug where the percentage change reported by the Stats API is different from the one returned by the internal dashboard API. * changelog update --- CHANGELOG.md | 2 ++ lib/plausible/stats/compare.ex | 4 ++++ .../api/external_stats_controller.ex | 24 ++++--------------- .../aggregate_test.exs | 6 ++--- .../api/stats_controller/top_stats_test.exs | 2 +- 5 files changed, 14 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 36c1dce9e..7011460ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,6 +40,8 @@ All notable changes to this project will be documented in this file. - Replace `CLICKHOUSE_MAX_BUFFER_SIZE` with `CLICKHOUSE_MAX_BUFFER_SIZE_BYTES` ### Fixed +- Calculate `conversion_rate` percentage change in the same way like `bounce_rate` (subtraction instead of division) +- Calculate `bounce_rate` percentage change in the Stats API in the same way as it's done in the dashboard - Stop returning custom events in goal breakdown with a pageview goal filter and vice versa - Only return `(none)` values in custom property breakdown for the first page (pagination) of results - Fixed weekly/monthly e-mail report [rendering issues](https://github.com/plausible/analytics/issues/284) diff --git a/lib/plausible/stats/compare.ex b/lib/plausible/stats/compare.ex index 2424118b2..d234d9aad 100644 --- a/lib/plausible/stats/compare.ex +++ b/lib/plausible/stats/compare.ex @@ -1,4 +1,8 @@ defmodule Plausible.Stats.Compare do + def calculate_change(:conversion_rate, old_value, new_value) do + Float.round(new_value - old_value, 1) + end + def calculate_change(:bounce_rate, old_count, new_count) do if old_count > 0, do: new_count - old_count end diff --git a/lib/plausible_web/controllers/api/external_stats_controller.ex b/lib/plausible_web/controllers/api/external_stats_controller.ex index 186a5abf7..c9cc9af83 100644 --- a/lib/plausible_web/controllers/api/external_stats_controller.ex +++ b/lib/plausible_web/controllers/api/external_stats_controller.ex @@ -2,7 +2,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController do use PlausibleWeb, :controller use Plausible.Repo use PlausibleWeb.Plugs.ErrorHandler - alias Plausible.Stats.Query + alias Plausible.Stats.{Query, Compare, Comparisons} @metrics [ :visitors, @@ -34,7 +34,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController do :ok <- ensure_custom_props_access(site, query) do results = if params["compare"] == "previous_period" do - {:ok, prev_query} = Plausible.Stats.Comparisons.compare(site, query, "previous_period") + {:ok, prev_query} = Comparisons.compare(site, query, "previous_period") [prev_result, curr_result] = Plausible.ClickhouseRepo.parallel_tasks([ @@ -44,12 +44,9 @@ defmodule PlausibleWeb.Api.ExternalStatsController do Enum.map(curr_result, fn {metric, %{value: current_val}} -> %{value: prev_val} = prev_result[metric] + change = Compare.calculate_change(metric, prev_val, current_val) - {metric, - %{ - value: current_val, - change: percent_change(prev_val, current_val) - }} + {metric, %{value: current_val, change: change}} end) |> Enum.into(%{}) else @@ -251,19 +248,6 @@ defmodule PlausibleWeb.Api.ExternalStatsController do end end - defp percent_change(old_count, new_count) do - cond do - old_count == 0 and new_count > 0 -> - 100 - - old_count == 0 and new_count == 0 -> - 0 - - true -> - round((new_count - old_count) / old_count * 100) - end - end - defp validate_date(%{"period" => "custom"} = params) do with {:ok, date} <- Map.fetch(params, "date"), [from, to] <- String.split(date, ","), 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 d14218635..40e6f7a26 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 @@ -322,7 +322,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController.AggregateTest do assert json_response(conn, 200)["results"] == %{ "pageviews" => %{"value" => 4, "change" => 100}, "visitors" => %{"value" => 3, "change" => 100}, - "bounce_rate" => %{"value" => 100, "change" => 100}, + "bounce_rate" => %{"value" => 100, "change" => nil}, "visit_duration" => %{"value" => 0, "change" => 0} } end @@ -352,7 +352,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController.AggregateTest do }) assert json_response(conn, 200)["results"] == %{ - "conversion_rate" => %{"value" => 50.0, "change" => 50.0} + "conversion_rate" => %{"value" => 50.0, "change" => 16.7} } end end @@ -419,7 +419,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController.AggregateTest do "visitors" => %{"value" => 2, "change" => 100}, "visits" => %{"value" => 5, "change" => 150}, "pageviews" => %{"value" => 9, "change" => -10}, - "bounce_rate" => %{"value" => 40, "change" => -20}, + "bounce_rate" => %{"value" => 40, "change" => -10}, "views_per_visit" => %{"value" => 1.0, "change" => 100}, "visit_duration" => %{"value" => 20, "change" => -80} } 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 66b078b44..c2dd91541 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 @@ -1094,7 +1094,7 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do res = json_response(conn, 200) assert %{ - "change" => -50, + "change" => -33.4, "comparison_value" => 66.7, "name" => "Conversion rate", "value" => 33.3