From f9f0407d68d3f9b4b21137794011ff7f0647b046 Mon Sep 17 00:00:00 2001 From: hq1 Date: Thu, 4 Apr 2024 17:20:16 +0200 Subject: [PATCH] Remove `experimtnal_hostname_filter` and keep it on by default (#3973) * Remove `experimental_hostname_filter` and keep it on by default * Catch up with changes done via e5b56dbe6 --- assets/js/dashboard/api.js | 1 - assets/js/dashboard/query.js | 1 - assets/js/dashboard/util/filters.js | 7 +- lib/plausible/stats/filter_suggestions.ex | 4 - lib/plausible/stats/query.ex | 14 +-- .../controllers/api/stats_controller.ex | 6 +- .../api/stats_controller/pages_test.exs | 106 +----------------- .../api/stats_controller/sources_test.exs | 56 ++------- .../api/stats_controller/top_stats_test.exs | 37 ++++-- 9 files changed, 46 insertions(+), 186 deletions(-) diff --git a/assets/js/dashboard/api.js b/assets/js/dashboard/api.js index e2e80f2d7..26fd29916 100644 --- a/assets/js/dashboard/api.js +++ b/assets/js/dashboard/api.js @@ -44,7 +44,6 @@ export function serializeQuery(query, extraQuery = []) { if (query.to) { queryObj.to = formatISO(query.to) } if (query.filters) { queryObj.filters = serializeFilters(query.filters) } if (query.experimental_session_count) { queryObj.experimental_session_count = query.experimental_session_count } - if (query.experimental_hostname_filter) { queryObj.experimental_hostname_filter = query.experimental_hostname_filter } if (query.with_imported) { queryObj.with_imported = query.with_imported } if (SHARED_LINK_AUTH) { queryObj.auth = SHARED_LINK_AUTH } diff --git a/assets/js/dashboard/query.js b/assets/js/dashboard/query.js index a726ba914..a44bc4352 100644 --- a/assets/js/dashboard/query.js +++ b/assets/js/dashboard/query.js @@ -40,7 +40,6 @@ export function parseQuery(querystring, site) { match_day_of_week: matchDayOfWeek == 'true', with_imported: q.get('with_imported') ? q.get('with_imported') === 'true' : true, experimental_session_count: q.get('experimental_session_count'), - experimental_hostname_filter: q.get('experimental_hostname_filter'), filters: { 'goal': q.get('goal'), 'props': JSON.parse(q.get('props')), diff --git a/assets/js/dashboard/util/filters.js b/assets/js/dashboard/util/filters.js index 766f0c5af..c7b8bf904 100644 --- a/assets/js/dashboard/util/filters.js +++ b/assets/js/dashboard/util/filters.js @@ -10,11 +10,11 @@ export const FILTER_GROUPS = { 'utm': ['utm_medium', 'utm_source', 'utm_campaign', 'utm_term', 'utm_content'], 'goal': ['goal'], 'props': ['prop_key', 'prop_value'], - ...(flags.hostname_filter ? { 'hostname': ['hostname', 'experimental_hostname_filter'] } : {}) + ...(flags.hostname_filter ? { 'hostname': ['hostname'] } : {}) } -export const NO_CONTAINS_OPERATOR = new Set(['experimental_hostname_filter', 'goal', 'screen'].concat(FILTER_GROUPS['location'])) +export const NO_CONTAINS_OPERATOR = new Set(['goal', 'screen'].concat(FILTER_GROUPS['location'])) export const FILTER_OPERATIONS = { isNot: 'is not', @@ -29,7 +29,7 @@ export const OPERATION_PREFIX = { }; export function supportsIsNot(filterName) { - return !['goal', 'prop_key', 'experimental_hostname_filter'].includes(filterName) + return !['goal', 'prop_key'].includes(filterName) } export function isFreeChoiceFilter(filterName) { @@ -157,7 +157,6 @@ export const formattedFilters = { 'city': 'City', 'page': 'Page', 'hostname': 'Hostname', - 'experimental_hostname_filter': 'Treat hostname as entry/exit hostname', 'entry_page': 'Entry Page', 'exit_page': 'Exit Page', } diff --git a/lib/plausible/stats/filter_suggestions.ex b/lib/plausible/stats/filter_suggestions.ex index 29b768506..fb95ac13a 100644 --- a/lib/plausible/stats/filter_suggestions.ex +++ b/lib/plausible/stats/filter_suggestions.ex @@ -118,10 +118,6 @@ defmodule Plausible.Stats.FilterSuggestions do end) end - def filter_suggestions(_site, _query, "experimental_hostname_filter", _filter_search) do - wrap_suggestions(["true", "false"]) - end - def filter_suggestions(site, _query, "goal", filter_search) do site |> Plausible.Goals.for_site() diff --git a/lib/plausible/stats/query.ex b/lib/plausible/stats/query.ex index 97b700ef0..852f4e372 100644 --- a/lib/plausible/stats/query.ex +++ b/lib/plausible/stats/query.ex @@ -10,8 +10,7 @@ defmodule Plausible.Stats.Query do include_imported: false, now: nil, experimental_session_count?: false, - experimental_reduced_joins?: false, - experimental_hostname_filter?: false + experimental_reduced_joins?: false require OpenTelemetry.Tracer, as: Tracer alias Plausible.Stats.{Filters, Interval} @@ -26,7 +25,6 @@ defmodule Plausible.Stats.Query do |> struct!(now: now) |> put_experimental_session_count(site, params) |> put_experimental_reduced_joins(site, params) - |> put_experimental_hostname_filter(params) |> put_period(site, params) |> put_interval(params) |> put_parsed_filters(params) @@ -64,16 +62,6 @@ defmodule Plausible.Stats.Query do end end - defp put_experimental_hostname_filter(query, params) do - if Map.has_key?(params, "experimental_hostname_filter") do - struct!(query, - experimental_hostname_filter?: Map.get(params, "experimental_hostname_filter") == "true" - ) - else - query - end - end - defp put_period(query, site, %{"period" => "realtime"}) do date = today(site.timezone) diff --git a/lib/plausible_web/controllers/api/stats_controller.ex b/lib/plausible_web/controllers/api/stats_controller.ex index 080142c0e..1a5735c8a 100644 --- a/lib/plausible_web/controllers/api/stats_controller.ex +++ b/lib/plausible_web/controllers/api/stats_controller.ex @@ -459,7 +459,7 @@ defmodule PlausibleWeb.Api.StatsController do pagination = parse_pagination(params) query = - if query.experimental_hostname_filter? and query.filters["event:hostname"] do + if query.filters["event:hostname"] do Query.put_filter(query, "visit:entry_page_hostname", query.filters["event:hostname"]) else query @@ -771,7 +771,7 @@ defmodule PlausibleWeb.Api.StatsController do metrics = breakdown_metrics(query, [:visits, :visit_duration]) query = - if query.experimental_hostname_filter? and query.filters["event:hostname"] do + if query.filters["event:hostname"] do Query.put_filter(query, "visit:entry_page_hostname", query.filters["event:hostname"]) else query @@ -808,7 +808,7 @@ defmodule PlausibleWeb.Api.StatsController do metrics = breakdown_metrics(query, [:visits]) query = - if query.experimental_hostname_filter? and query.filters["event:hostname"] do + if query.filters["event:hostname"] do Query.put_filter(query, "visit:exit_page_hostname", query.filters["event:hostname"]) else query diff --git a/test/plausible_web/controllers/api/stats_controller/pages_test.exs b/test/plausible_web/controllers/api/stats_controller/pages_test.exs index 2c4897f62..be2259ee8 100644 --- a/test/plausible_web/controllers/api/stats_controller/pages_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/pages_test.exs @@ -1384,58 +1384,7 @@ defmodule PlausibleWeb.Api.StatsController.PagesTest do ] end - test "returns top entry pages by visitors filtered by hostname", %{conn: conn, site: site} do - populate_stats(site, [ - build(:pageview, - pathname: "/page1", - hostname: "en.example.com", - timestamp: ~N[2021-01-01 00:00:00] - ), - build(:pageview, - pathname: "/page1", - hostname: "es.example.com", - timestamp: ~N[2021-01-01 00:00:00] - ), - build(:pageview, - pathname: "/page2", - hostname: "en.example.com", - user_id: @user_id, - timestamp: ~N[2021-01-01 00:00:00] - ), - build(:pageview, - pathname: "/page2", - hostname: "es.example.com", - user_id: @user_id, - timestamp: ~N[2021-01-01 00:15:00] - ), - build(:pageview, - pathname: "/exit", - hostname: "es.example.com", - user_id: @user_id, - timestamp: ~N[2021-01-01 00:16:00] - ), - build(:pageview, - pathname: "/page2", - hostname: "es.example.com", - timestamp: ~N[2021-01-01 23:15:00] - ) - ]) - - filters = Jason.encode!(%{"hostname" => "es.example.com"}) - - conn = - get( - conn, - "/api/stats/#{site.domain}/entry-pages?period=day&date=2021-01-01&filters=#{filters}" - ) - - assert json_response(conn, 200) == [ - %{"name" => "/page2", "visit_duration" => 480, "visitors" => 2, "visits" => 2}, - %{"name" => "/page1", "visit_duration" => 0, "visitors" => 1, "visits" => 1} - ] - end - - test "returns top entry pages by visitors filtered by hostname with experimental_hostname_filter", + test "returns top entry pages by visitors filtered by hostname", %{conn: conn, site: site} do populate_stats(site, [ build(:pageview, @@ -1478,7 +1427,7 @@ defmodule PlausibleWeb.Api.StatsController.PagesTest do conn = get( conn, - "/api/stats/#{site.domain}/entry-pages?period=day&date=2021-01-01&filters=#{filters}&experimental_hostname_filter=true" + "/api/stats/#{site.domain}/entry-pages?period=day&date=2021-01-01&filters=#{filters}" ) # We're going to only join sessions where the exit hostname matches the filter @@ -1638,54 +1587,7 @@ defmodule PlausibleWeb.Api.StatsController.PagesTest do ] end - test "returns top exit pages by visitors filtered by hostname", %{conn: conn, site: site} do - populate_stats(site, [ - build(:pageview, - pathname: "/page1", - hostname: "en.example.com", - timestamp: ~N[2021-01-01 00:00:00] - ), - build(:pageview, - pathname: "/page1", - hostname: "es.example.com", - timestamp: ~N[2021-01-01 00:00:00] - ), - build(:pageview, - pathname: "/page1", - hostname: "en.example.com", - user_id: @user_id, - timestamp: ~N[2021-01-01 00:00:00] - ), - build(:pageview, - pathname: "/page2", - hostname: "es.example.com", - user_id: @user_id, - timestamp: ~N[2021-01-01 00:15:00] - ), - build(:pageview, - pathname: "/exit", - hostname: "en.example.com", - user_id: @user_id, - timestamp: ~N[2021-01-01 00:16:00] - ) - ]) - - filters = Jason.encode!(%{hostname: "es.example.com"}) - - conn = - get( - conn, - "/api/stats/#{site.domain}/exit-pages?period=day&date=2021-01-01&filters=#{filters}" - ) - - assert json_response(conn, 200) == - [ - %{"name" => "/exit", "visitors" => 1, "visits" => 1}, - %{"name" => "/page1", "visitors" => 1, "visits" => 1} - ] - end - - test "returns top exit pages by visitors filtered by hostname with experimental_hostname_filter", + test "returns top exit pages by visitors filtered by hostname", %{conn: conn, site: site} do populate_stats(site, [ build(:pageview, @@ -1723,7 +1625,7 @@ defmodule PlausibleWeb.Api.StatsController.PagesTest do conn = get( conn, - "/api/stats/#{site.domain}/exit-pages?period=day&date=2021-01-01&filters=#{filters}&experimental_hostname_filter=true" + "/api/stats/#{site.domain}/exit-pages?period=day&date=2021-01-01&filters=#{filters}" ) # We're going to only join sessions where the entry hostname matches the filter diff --git a/test/plausible_web/controllers/api/stats_controller/sources_test.exs b/test/plausible_web/controllers/api/stats_controller/sources_test.exs index c1908d15d..50f8600f4 100644 --- a/test/plausible_web/controllers/api/stats_controller/sources_test.exs +++ b/test/plausible_web/controllers/api/stats_controller/sources_test.exs @@ -1221,10 +1221,11 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do ] end - test "returns top referrers for a custom goal and filtered by hostname", %{ - conn: conn, - site: site - } do + test "returns no top referrers for a custom goal and filtered by hostname", + %{ + conn: conn, + site: site + } do populate_stats(site, [ build(:pageview, hostname: "blog.example.com", @@ -1252,53 +1253,10 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do "/api/stats/#{site.domain}/sources?period=day&filters=#{filters}" ) - assert json_response(conn, 200) == - [ - %{ - "conversion_rate" => 100.0, - "name" => "Facebook", - "total_visitors" => 1, - "visitors" => 1 - } - ] - end - - test "returns no top referrers for a custom goal and filtered by hostname and experimental_hostname_filter", - %{ - conn: conn, - site: site - } do - populate_stats(site, [ - build(:pageview, - hostname: "blog.example.com", - referrer_source: "Facebook", - user_id: @user_id - ), - build(:pageview, - hostname: "app.example.com", - pathname: "/register", - user_id: @user_id - ), - build(:event, - name: "Signup", - hostname: "app.example.com", - pathname: "/register", - user_id: @user_id - ) - ]) - - filters = Jason.encode!(%{goal: "Signup", hostname: "app.example.com"}) - - conn = - get( - conn, - "/api/stats/#{site.domain}/sources?period=day&filters=#{filters}&experimental_hostname_filter=true" - ) - assert json_response(conn, 200) == [] end - test "returns top referrers for a custom goal and filtered by hostname and experimental_hostname_filter", + test "returns top referrers for a custom goal and filtered by hostname (2)", %{ conn: conn, site: site @@ -1323,7 +1281,7 @@ defmodule PlausibleWeb.Api.StatsController.SourcesTest do conn = get( conn, - "/api/stats/#{site.domain}/sources?period=day&filters=#{filters}&experimental_hostname_filter=true" + "/api/stats/#{site.domain}/sources?period=day&filters=#{filters}" ) assert json_response(conn, 200) == [ 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 720c138e8..911e099e8 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 @@ -803,8 +803,13 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do res = json_response(conn, 200) - assert %{"name" => "Unique visitors", "value" => 3} in res["top_stats"] - assert %{"name" => "Total visits", "value" => 3} in res["top_stats"] + assert %{"name" => "Unique visitors", "value" => 3, "graph_metric" => "visitors"} in res[ + "top_stats" + ] + + assert %{"name" => "Total visits", "value" => 3, "graph_metric" => "visits"} in res[ + "top_stats" + ] end test "returns only visitors with specific browser", %{conn: conn, site: site} do @@ -928,8 +933,13 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do res = json_response(conn, 200) - assert %{"name" => "Unique visitors", "value" => 2} in res["top_stats"] - assert %{"name" => "Total pageviews", "value" => 2} in res["top_stats"] + assert %{"name" => "Unique visitors", "value" => 2, "graph_metric" => "visitors"} in res[ + "top_stats" + ] + + assert %{"name" => "Total pageviews", "value" => 2, "graph_metric" => "pageviews"} in res[ + "top_stats" + ] end test "hostname glob filter", %{conn: conn, site: site} do @@ -953,8 +963,13 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do res = json_response(conn, 200) - assert %{"name" => "Unique visitors", "value" => 4} in res["top_stats"] - assert %{"name" => "Total pageviews", "value" => 6} in res["top_stats"] + assert %{"name" => "Unique visitors", "value" => 4, "graph_metric" => "visitors"} in res[ + "top_stats" + ] + + assert %{"name" => "Total pageviews", "value" => 6, "graph_metric" => "pageviews"} in res[ + "top_stats" + ] end test "hostname glob subdomain filter", %{conn: conn, site: site} do @@ -972,7 +987,6 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do ]) filters = Jason.encode!(%{hostname: "*.example.com"}) - # filters = Jason.encode!(%{page: "/blog/*"}) conn = get( @@ -982,8 +996,13 @@ defmodule PlausibleWeb.Api.StatsController.TopStatsTest do res = json_response(conn, 200) - assert %{"name" => "Unique visitors", "value" => 3} in res["top_stats"] - assert %{"name" => "Total pageviews", "value" => 4} in res["top_stats"] + assert %{"name" => "Unique visitors", "value" => 3, "graph_metric" => "visitors"} in res[ + "top_stats" + ] + + assert %{"name" => "Total pageviews", "value" => 4, "graph_metric" => "pageviews"} in res[ + "top_stats" + ] end end