Drop support for breaking down by event:hostname property for now

plausible · May 20, 2024 · 54f873c · 54f873c
1 parent 7070578
commit 54f873c
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 135 deletions.
diff --git a/lib/plausible/stats/breakdown.ex b/lib/plausible/stats/breakdown.ex
@@ -474,15 +474,6 @@ defmodule Plausible.Stats.Breakdown do
     )
   end
 
-  defp do_group_by(q, "event:hostname") do
-    from(
-      s in q,
-      group_by: s.hostname,
-      select_merge: %{hostname: s.hostname},
-      order_by: {:asc, s.hostname}
-    )
-  end
-
   defp do_group_by(q, "visit:source") do
     from(
       s in q,

diff --git a/lib/plausible/stats/imported.ex b/lib/plausible/stats/imported.ex
@@ -29,7 +29,6 @@ defmodule Plausible.Stats.Imported do
     "visit:os_version" => "imported_operating_systems",
     "event:page" => "imported_pages",
     "event:name" => "imported_custom_events",
-    "event:hostname" => "imported_pages",
     "event:props:url" => "imported_custom_events",
     "event:props:path" => "imported_custom_events"
   }
@@ -321,18 +320,6 @@ defmodule Plausible.Stats.Imported do
     |> select_imported_metrics(rest)
   end
 
-  defp select_imported_metrics(
-         %Ecto.Query{from: %Ecto.Query.FromExpr{source: {"imported_pages", _}}} = q,
-         [:bounce_rate | rest]
-       ) do
-    q
-    |> select_merge([i], %{
-      bounces: 0,
-      __internal_visits: 0
-    })
-    |> select_imported_metrics(rest)
-  end
-
   defp select_imported_metrics(
          %Ecto.Query{from: %Ecto.Query.FromExpr{source: {"imported_entry_pages", _}}} = q,
          [:bounce_rate | rest]
@@ -366,18 +353,6 @@ defmodule Plausible.Stats.Imported do
     |> select_imported_metrics(rest)
   end
 
-  defp select_imported_metrics(
-         %Ecto.Query{from: %Ecto.Query.FromExpr{source: {"imported_pages", _}}} = q,
-         [:visit_duration | rest]
-       ) do
-    q
-    |> select_merge([i], %{
-      visit_duration: 0,
-      __internal_visits: 0
-    })
-    |> select_imported_metrics(rest)
-  end
-
   defp select_imported_metrics(
          %Ecto.Query{from: %Ecto.Query.FromExpr{source: {"imported_entry_pages", _}}} = q,
          [:visit_duration | rest]
@@ -527,12 +502,6 @@ defmodule Plausible.Stats.Imported do
     |> select_merge([i], %{name: i.name})
   end
 
-  defp group_imported_by(q, :hostname) do
-    q
-    |> group_by([i], i.hostname)
-    |> select_merge([i], %{hostname: i.hostname})
-  end
-
   defp group_imported_by(q, :url) do
     q
     |> group_by([i], i.link_url)

diff --git a/lib/plausible_web/controllers/api/external_stats_controller.ex b/lib/plausible_web/controllers/api/external_stats_controller.ex
@@ -68,11 +68,17 @@ defmodule PlausibleWeb.Api.ExternalStatsController do
   end
 
   defp validate_property(%{"property" => property}) do
-    if Plausible.Stats.Props.valid_prop?(property) do
-      :ok
-    else
-      {:error,
-       "Invalid property '#{property}'. Please provide a valid property for the breakdown endpoint: https://plausible.io/docs/stats-api#properties"}
+    cond do
+      property == "event:hostname" ->
+        {:error,
+         "Property 'event:hostname' is currently not supported for breakdowns.  Please provide a valid property for the breakdown endpoint: https://plausible.io/docs/stats-api#properties"}
+
+      Plausible.Stats.Props.valid_prop?(property) ->
+        :ok
+
+      true ->
+        {:error,
+         "Invalid property '#{property}'. Please provide a valid property for the breakdown endpoint: https://plausible.io/docs/stats-api#properties"}
     end
   end
 

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
@@ -949,98 +949,21 @@ defmodule PlausibleWeb.Api.ExternalStatsController.BreakdownTest do
     assert json_response(conn, 200) == %{"results" => []}
   end
 
-  describe "breakdown by event:hostname" do
-    setup %{site: site} do
-      site_import = insert(:site_import, site: site)
-
-      populate_stats(site, site_import.id, [
-        build(:pageview,
-          hostname: "alice.example.com",
-          pathname: "/a",
-          timestamp: ~N[2021-01-01 00:00:00]
-        ),
-        build(:pageview,
-          hostname: "bob.example.com",
-          pathname: "/a",
-          timestamp: ~N[2021-01-01 00:25:00]
-        ),
-        build(:pageview,
-          hostname: "alice.example.com",
-          pathname: "/b",
-          timestamp: ~N[2021-01-01 00:35:00]
-        ),
-        build(:imported_pages,
-          hostname: "alice.example.com",
-          page: "/a",
-          date: ~D[2021-01-01],
-          visitors: 3,
-          pageviews: 20
-        ),
-        build(:imported_pages,
-          hostname: "bob.example.com",
-          page: "/a",
-          date: ~D[2021-01-01],
-          visitors: 10,
-          pageviews: 18
-        )
-      ])
-    end
-
-    test "can breakdown by event:hostname", %{conn: conn, site: site} do
-      conn =
-        get(conn, "/api/v1/stats/breakdown", %{
-          "site_id" => site.domain,
-          "period" => "day",
-          "date" => "2021-01-01",
-          "property" => "event:hostname",
-          "with_imported" => "true"
-        })
-
-      assert json_response(conn, 200) == %{
-               "results" => [
-                 %{"hostname" => "bob.example.com", "visitors" => 11},
-                 %{"hostname" => "alice.example.com", "visitors" => 5}
-               ]
-             }
-    end
+  test "attempting to breakdown by event:hostname returns an error", %{conn: conn, site: site} do
+    conn =
+      get(conn, "/api/v1/stats/breakdown", %{
+        "site_id" => site.domain,
+        "period" => "day",
+        "date" => "2021-01-01",
+        "property" => "event:hostname",
+        "with_imported" => "true"
+      })
 
-    test "handles all applicable metrics with graceful fallback for imported data when needed", %{
-      conn: conn,
-      site: site
-    } do
-      conn =
-        get(conn, "/api/v1/stats/breakdown", %{
-          "site_id" => site.domain,
-          "period" => "day",
-          "date" => "2021-01-01",
-          "property" => "event:hostname",
-          "metrics" => "visitors,visits,pageviews,bounce_rate,visit_duration,events",
-          "with_imported" => "true"
-        })
+    assert %{
+             "error" => error
+           } = json_response(conn, 400)
 
-      assert json_response(conn, 200) == %{
-               "results" => [
-                 %{
-                   "hostname" => "bob.example.com",
-                   "visitors" => 11,
-                   "bounce_rate" => 100.0,
-                   "events" => 19,
-                   "pageviews" => 19,
-                   "visit_duration" => 0.0,
-                   "visits" => 1
-                 },
-                 %{
-                   "hostname" => "alice.example.com",
-                   "visitors" => 5,
-                   "bounce_rate" => 100.0,
-                   "events" => 22,
-                   "pageviews" => 22,
-                   "visit_duration" => 0.0,
-                   "visits" => 2
-                 }
-               ]
-             }
-    end
+    assert error =~ "Property 'event:hostname' is currently not supported for breakdowns."
   end
 
   describe "breakdown by visit:exit_page" do