From 54f873cd2fc23dbc8810e5744acb1b58975551f4 Mon Sep 17 00:00:00 2001 From: Adrian Gruntkowski Date: Mon, 20 May 2024 16:14:34 +0200 Subject: [PATCH] Drop support for breaking down by `event:hostname` property for now --- lib/plausible/stats/breakdown.ex | 9 -- lib/plausible/stats/imported.ex | 31 ------ .../api/external_stats_controller.ex | 16 ++- .../breakdown_test.exs | 103 +++--------------- 4 files changed, 24 insertions(+), 135 deletions(-) diff --git a/lib/plausible/stats/breakdown.ex b/lib/plausible/stats/breakdown.ex index f5c4bcc82e10..305d80838b3d 100644 --- 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 index 56b728c3dcbc..0d246344591e 100644 --- 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 index a14abf6cd75d..01424f50a2a2 100644 --- 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 index eb962c8fe388..acc074480c92 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 @@ -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