From 3c4931598b8449e17efbf3f838b7353010f25279 Mon Sep 17 00:00:00 2001
From: Uku Taht <uku.taht@gmail.com>
Date: Mon, 22 Feb 2021 14:55:42 +0200
Subject: [PATCH] Fix breakdown call

---
 lib/plausible/stats/breakdown.ex              | 34 ++++++++++------
 .../breakdown_test.exs                        | 39 -------------------
 2 files changed, 22 insertions(+), 51 deletions(-)

diff --git a/lib/plausible/stats/breakdown.ex b/lib/plausible/stats/breakdown.ex
index b1235902d..2044d61f9 100644
--- a/lib/plausible/stats/breakdown.ex
+++ b/lib/plausible/stats/breakdown.ex
@@ -5,24 +5,34 @@ defmodule Plausible.Stats.Breakdown do
   @event_metrics ["visitors", "pageviews"]
   @session_metrics ["bounce_rate", "visit_duration"]
 
+  # use join once this is solved: https://github.com/ClickHouse/ClickHouse/issues/10276
+  # https://github.com/ClickHouse/ClickHouse/issues/17319
   def breakdown(site, query, "event:page", metrics, pagination) do
     event_metrics = Enum.filter(metrics, &(&1 in @event_metrics))
     session_metrics = Enum.filter(metrics, &(&1 in @session_metrics))
 
-    event_result =
-      breakdown_events(site, query, event_metrics, pagination)
-      |> Enum.map(fn row -> {row[:page], Map.delete(row, :page)} end)
-      |> Enum.into(%{})
+    event_result = breakdown_events(site, query, event_metrics, pagination)
+    pages = Enum.map(event_result, fn r -> r[:page] end)
 
-    session_result =
-      breakdown(site, query, "visit:entry_page", session_metrics, pagination)
-      |> Enum.map(fn row -> {row[:entry_page], Map.delete(row, :entry_page)} end)
-      |> Enum.into(%{})
+    if Enum.any?(session_metrics) do
+      session_result =
+        from(s in query_sessions(site, query),
+          group_by: s.entry_page,
+          where: s.entry_page in ^pages,
+          select: %{entry_page: s.entry_page}
+        )
+        |> select_metrics(session_metrics)
+        |> ClickhouseRepo.all()
 
-    Map.merge(event_result, session_result, fn _page, v1, v2 ->
-      Map.merge(v1, v2)
-    end)
-    |> Enum.map(fn {k, v} -> Map.put(v, :page, k) end)
+      session_metrics_atoms = Enum.map(session_metrics, &String.to_atom/1)
+
+      Enum.map(event_result, fn row ->
+        session_row = Enum.find(session_result, fn row2 -> row2[:entry_page] == row[:page] end)
+        Map.merge(row, Map.take(session_row, session_metrics_atoms))
+      end)
+    else
+      event_result
+    end
   end
 
   def breakdown(_, _, _, [], _), do: %{}
diff --git a/test/plausible_web/controllers/api/external_stats_controller/breakdown_test.exs b/test/plausible_web/controllers/api/external_stats_controller/breakdown_test.exs
index e3a3e6b5b..48bfac5c1 100644
--- a/test/plausible_web/controllers/api/external_stats_controller/breakdown_test.exs
+++ b/test/plausible_web/controllers/api/external_stats_controller/breakdown_test.exs
@@ -568,44 +568,5 @@ defmodule PlausibleWeb.Api.ExternalStatsController.BreakdownTest do
                ]
              }
     end
-
-    test "just bounce rate for event:page", %{conn: conn, site: site} do
-      populate_stats([
-        build(:pageview,
-          user_id: 1,
-          pathname: "/",
-          domain: site.domain,
-          timestamp: ~N[2021-01-01 00:00:00]
-        ),
-        build(:pageview,
-          user_id: 1,
-          pathname: "/plausible.io",
-          domain: site.domain,
-          timestamp: ~N[2021-01-01 00:10:00]
-        ),
-        build(:pageview, pathname: "/", domain: site.domain, timestamp: ~N[2021-01-01 00:25:00]),
-        build(:pageview,
-          pathname: "/plausible.io",
-          domain: site.domain,
-          timestamp: ~N[2021-01-01 00:00:00]
-        )
-      ])
-
-      conn =
-        get(conn, "/api/v1/stats/breakdown", %{
-          "site_id" => site.domain,
-          "period" => "day",
-          "date" => "2021-01-01",
-          "property" => "event:page",
-          "metrics" => "bounce_rate"
-        })
-
-      assert json_response(conn, 200) == %{
-               "results" => [
-                 %{"page" => "/", "bounce_rate" => 50},
-                 %{"page" => "/plausible.io", "bounce_rate" => 100}
-               ]
-             }
-    end
   end
 end