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

@@ -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)

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

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, ","),

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}
              }

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