From 2f7dcae99138be5a1173a3e3ed04ebbf55d19388 Mon Sep 17 00:00:00 2001 From: hq1 Date: Tue, 11 Jun 2024 16:42:07 +0200 Subject: [PATCH] Revert "Multiple filters on the frontend (#4174)" (#4218) This reverts commit e1f0002d2ebcfae2145001b1c3957158f39e4792. --- .../stats/modals/filter-modal-group.js | 7 +- assets/js/dashboard/util/filters.js | 37 +- extra/lib/plausible/stats/goal/revenue.ex | 3 +- .../google/search_console/filters.ex | 27 +- lib/plausible/stats/base.ex | 4 +- lib/plausible/stats/breakdown.ex | 48 +-- lib/plausible/stats/email_report.ex | 6 +- ...r_parser.ex => dashboard_filter_parser.ex} | 26 +- lib/plausible/stats/filters/filters.ex | 26 +- lib/plausible/stats/filters/query_parser.ex | 203 ---------- .../stats/filters/stats_api_filter_parser.ex | 18 +- lib/plausible/stats/filters/utils.ex | 2 +- lib/plausible/stats/filters/where_builder.ex | 90 ++++- lib/plausible/stats/imported/base.ex | 82 ++-- lib/plausible/stats/imported/imported.ex | 8 +- lib/plausible/stats/query.ex | 20 +- .../api/external_stats_controller.ex | 16 +- .../controllers/api/stats_controller.ex | 10 +- .../controllers/stats_controller.ex | 7 +- .../google/search_console/filters_test.exs | 56 +-- .../stats/dashboard_filter_parser_test.exs | 207 ++++++++++ test/plausible/stats/filters_test.exs | 42 +- .../legacy_dashboard_filter_parser_test.exs | 201 ---------- test/plausible/stats/query_parser_test.exs | 373 ------------------ test/plausible/stats/query_test.exs | 4 +- 25 files changed, 503 insertions(+), 1020 deletions(-) rename lib/plausible/stats/filters/{legacy_dashboard_filter_parser.ex => dashboard_filter_parser.ex} (82%) delete mode 100644 lib/plausible/stats/filters/query_parser.ex create mode 100644 test/plausible/stats/dashboard_filter_parser_test.exs delete mode 100644 test/plausible/stats/legacy_dashboard_filter_parser_test.exs delete mode 100644 test/plausible/stats/query_parser_test.exs diff --git a/assets/js/dashboard/stats/modals/filter-modal-group.js b/assets/js/dashboard/stats/modals/filter-modal-group.js index 8cfea5ebbbcb..b8eee128e4ba 100644 --- a/assets/js/dashboard/stats/modals/filter-modal-group.js +++ b/assets/js/dashboard/stats/modals/filter-modal-group.js @@ -17,17 +17,18 @@ export default function FilterModalGroup({ () => Object.entries(filterState).filter(([_, filter]) => getFilterGroup(filter) == filterGroup).map(([id, filter]) => ({ id, filter })), [filterGroup, filterState] ) + const disabledOptions = useMemo( () => (filterGroup == 'props') ? rows.map(({ filter }) => ({ value: getPropertyKeyFromFilterKey(filter[1]) })) : null, [filterGroup, rows] ) - const showAddRow = site.flags.multiple_filters ? !['goal', 'hostname'].includes(filterGroup) : filterGroup == 'props' + const showAddRow = filterGroup == 'props' const showTitle = filterGroup != 'props' return ( <> -
+
{showTitle && (
{formattedFilters[filterGroup]}
)} {rows.map(({ id, filter }) => filterGroup === 'props' ? ( @@ -54,7 +55,7 @@ export default function FilterModalGroup({ )}
{showAddRow && ( -
+
onAddRow(filterGroup)}> + Add another diff --git a/assets/js/dashboard/util/filters.js b/assets/js/dashboard/util/filters.js index d41693db70c4..27367e260da6 100644 --- a/assets/js/dashboard/util/filters.js +++ b/assets/js/dashboard/util/filters.js @@ -34,12 +34,6 @@ export const OPERATION_PREFIX = { [FILTER_OPERATIONS.is]: '' }; -export const BACKEND_OPERATION = { - [FILTER_OPERATIONS.is]: 'is', - [FILTER_OPERATIONS.isNot]: 'is_not', - [FILTER_OPERATIONS.contains]: 'matches' -} - export function supportsIsNot(filterName) { return !['goal', 'prop_key'].includes(filterName) } @@ -59,6 +53,17 @@ try { const ESCAPED_PIPE = '\\|' +function escapeFilterValue(value) { + return value.replaceAll(NON_ESCAPED_PIPE_REGEX, ESCAPED_PIPE) +} + +function toFilterQuery(type, clauses) { + const prefix = OPERATION_PREFIX[type]; + const result = clauses.map(clause => escapeFilterValue(clause.toString().trim())).join('|') + return prefix + result; +} + + export function getLabel(labels, filterKey, value) { if (['country', 'region', 'city'].includes(filterKey)) { return labels[value] @@ -136,21 +141,19 @@ export function cleanLabels(filters, labels, mergedFilterKey, mergedLabels) { return result } -const EVENT_FILTER_KEYS = new Set(["name", "page", "goal", "hostname"]) +// :TODO: New schema for filters in the BE export function serializeApiFilters(filters) { - const apiFilters = filters.map(([operation, filterKey, clauses]) => { - let apiFilterKey = `visit:${filterKey}` - if (filterKey.startsWith(EVENT_PROPS_PREFIX) || EVENT_FILTER_KEYS.has(filterKey)) { - apiFilterKey = `event:${filterKey}` + const cleaned = {} + filters.forEach(([operation, filterKey, clauses]) => { + if (filterKey.startsWith(EVENT_PROPS_PREFIX)) { + cleaned.props ||= {} + cleaned.props[getPropertyKeyFromFilterKey(filterKey)] = toFilterQuery(operation, clauses) + } else { + cleaned[filterKey] = toFilterQuery(operation, clauses) } - if (operation == FILTER_OPERATIONS.contains) { - clauses = clauses.map((value) => value.includes('*') ? value : `**${value}**`) - } - return [BACKEND_OPERATION[operation], apiFilterKey, clauses] }) - - return JSON.stringify(apiFilters) + return JSON.stringify(cleaned) } export function fetchSuggestions(apiPath, query, input, additionalFilter) { diff --git a/extra/lib/plausible/stats/goal/revenue.ex b/extra/lib/plausible/stats/goal/revenue.ex index d531b5d63817..8c4115c0aae3 100644 --- a/extra/lib/plausible/stats/goal/revenue.ex +++ b/extra/lib/plausible/stats/goal/revenue.ex @@ -40,7 +40,8 @@ defmodule Plausible.Stats.Goal.Revenue do def get_revenue_tracking_currency(site, query, metrics) do goal_filters = case Query.get_filter(query, "event:goal") do - [:is, "event:goal", list] -> Enum.map(list, fn {_, goal_name} -> goal_name end) + [:is, "event:goal", {_, goal_name}] -> [goal_name] + [:member, "event:goal", list] -> Enum.map(list, fn {_, goal_name} -> goal_name end) _ -> [] end diff --git a/lib/plausible/google/search_console/filters.ex b/lib/plausible/google/search_console/filters.ex index ee9bc2157866..484ed843909b 100644 --- a/lib/plausible/google/search_console/filters.ex +++ b/lib/plausible/google/search_console/filters.ex @@ -23,14 +23,23 @@ defmodule Plausible.Google.SearchConsole.Filters do transform_filter(property, [op, "visit:entry_page" | rest]) end - defp transform_filter(property, [:is, "visit:entry_page", pages]) when is_list(pages) do + defp transform_filter(property, [:is, "visit:entry_page", page]) when is_binary(page) do + %{dimension: "page", expression: property_url(property, page)} + end + + defp transform_filter(property, [:member, "visit:entry_page", pages]) when is_list(pages) do expression = Enum.map_join(pages, "|", fn page -> property_url(property, Regex.escape(page)) end) %{dimension: "page", operator: "includingRegex", expression: expression} end - defp transform_filter(property, [:matches, "visit:entry_page", pages]) + defp transform_filter(property, [:matches, "visit:entry_page", page]) when is_binary(page) do + page = page_regex(property_url(property, page)) + %{dimension: "page", operator: "includingRegex", expression: page} + end + + defp transform_filter(property, [:matches_member, "visit:entry_page", pages]) when is_list(pages) do expression = Enum.map_join(pages, "|", fn page -> page_regex(property_url(property, page)) end) @@ -38,12 +47,20 @@ defmodule Plausible.Google.SearchConsole.Filters do %{dimension: "page", operator: "includingRegex", expression: expression} end - defp transform_filter(_property, [:is, "visit:screen", devices]) when is_list(devices) do - expression = Enum.map_join(devices, "|", &search_console_device/1) + defp transform_filter(_property, [:is, "visit:screen", device]) when is_binary(device) do + %{dimension: "device", expression: search_console_device(device)} + end + + defp transform_filter(_property, [:member, "visit:screen", devices]) when is_list(devices) do + expression = devices |> Enum.join("|") %{dimension: "device", operator: "includingRegex", expression: expression} end - defp transform_filter(_property, [:is, "visit:country", countries]) + defp transform_filter(_property, [:is, "visit:country", country]) when is_binary(country) do + %{dimension: "country", expression: search_console_country(country)} + end + + defp transform_filter(_property, [:member, "visit:country", countries]) when is_list(countries) do expression = Enum.map_join(countries, "|", &search_console_country/1) %{dimension: "country", operator: "includingRegex", expression: expression} diff --git a/lib/plausible/stats/base.ex b/lib/plausible/stats/base.ex index a449d0fd30b3..85ffa7cd4ce3 100644 --- a/lib/plausible/stats/base.ex +++ b/lib/plausible/stats/base.ex @@ -320,7 +320,7 @@ defmodule Plausible.Stats.Base do def add_percentage_metric(q, site, query, metrics) do if :percentage in metrics do - total_query = Query.set_dimensions(query, []) + total_query = Query.set_property(query, nil) q |> select_merge( @@ -348,7 +348,7 @@ defmodule Plausible.Stats.Base do total_query = query |> Query.remove_filters(["event:goal", "event:props"]) - |> Query.set_dimensions([]) + |> Query.set_property(nil) # :TRICKY: Subquery is used due to event:goal breakdown above doing an UNION ALL subquery(q) diff --git a/lib/plausible/stats/breakdown.ex b/lib/plausible/stats/breakdown.ex index 6f3799e831cd..23db74a9e04c 100644 --- a/lib/plausible/stats/breakdown.ex +++ b/lib/plausible/stats/breakdown.ex @@ -27,7 +27,7 @@ defmodule Plausible.Stats.Breakdown do def breakdown( site, - %Query{dimensions: ["event:goal"]} = query, + %Query{property: "event:goal"} = query, metrics, pagination, opts @@ -39,8 +39,8 @@ defmodule Plausible.Stats.Breakdown do event_query = query - |> Query.put_filter([:is, "event:name", events]) - |> Query.set_dimensions(["event:name"]) + |> Query.put_filter([:member, "event:name", events]) + |> Query.set_property("event:name") if !Keyword.get(opts, :skip_tracing), do: Query.trace(query, metrics) @@ -73,7 +73,7 @@ defmodule Plausible.Stats.Breakdown do page_q = if Enum.any?(pageview_goals) do - page_query = Query.set_dimensions(query, ["event:page"]) + page_query = Query.set_property(query, "event:page") page_exprs = Enum.map(pageview_goals, & &1.page_path) page_regexes = Enum.map(page_exprs, &page_regex/1) @@ -134,7 +134,7 @@ defmodule Plausible.Stats.Breakdown do def breakdown( site, - %Query{dimensions: ["event:props:" <> custom_prop]} = query, + %Query{property: "event:props:" <> custom_prop} = query, metrics, pagination, opts @@ -157,7 +157,7 @@ defmodule Plausible.Stats.Breakdown do |> Enum.map(&cast_revenue_metrics_to_money(&1, currency)) end - def breakdown(site, %Query{dimensions: ["event:page"]} = query, metrics, pagination, opts) do + def breakdown(site, %Query{property: "event:page"} = query, metrics, pagination, opts) do event_metrics = metrics |> Util.maybe_add_visitors_metric() @@ -182,8 +182,8 @@ defmodule Plausible.Stats.Breakdown do pages -> query |> Query.remove_filters(["event:page"]) - |> Query.put_filter([:is, "visit:entry_page", Enum.map(pages, & &1[:page])]) - |> Query.set_dimensions(["visit:entry_page"]) + |> Query.put_filter([:member, "visit:entry_page", Enum.map(pages, & &1[:page])]) + |> Query.set_property("visit:entry_page") end if Enum.any?(event_metrics) && Enum.empty?(event_result) do @@ -208,7 +208,7 @@ defmodule Plausible.Stats.Breakdown do end end - def breakdown(site, %Query{dimensions: ["event:name"]} = query, metrics, pagination, opts) do + def breakdown(site, %Query{property: "event:name"} = query, metrics, pagination, opts) do if !Keyword.get(opts, :skip_tracing), do: Query.trace(query, metrics) breakdown_events(site, query, metrics) @@ -234,7 +234,7 @@ defmodule Plausible.Stats.Breakdown do end end - defp maybe_update_breakdown_filters(%Query{dimensions: [visit_entry_prop]} = query) + defp maybe_update_breakdown_filters(%Query{property: visit_entry_prop} = query) when visit_entry_prop in [ "visit:source", "visit:entry_page", @@ -249,7 +249,7 @@ defmodule Plausible.Stats.Breakdown do update_hostname_filter_prop(query, "visit:entry_page_hostname") end - defp maybe_update_breakdown_filters(%Query{dimensions: ["visit:exit_page"]} = query) do + defp maybe_update_breakdown_filters(%Query{property: "visit:exit_page"} = query) do update_hostname_filter_prop(query, "visit:exit_page_hostname") end @@ -271,13 +271,13 @@ defmodule Plausible.Stats.Breakdown do # Backwards compatibility defp breakdown_table(%Query{experimental_reduced_joins?: false}, _), do: :session - defp breakdown_table(%Query{dimensions: ["visit:entry_page"]}, _metrics), do: :session - defp breakdown_table(%Query{dimensions: ["visit:entry_page_hostname"]}, _metrics), do: :session - defp breakdown_table(%Query{dimensions: ["visit:exit_page"]}, _metrics), do: :session - defp breakdown_table(%Query{dimensions: ["visit:exit_page_hostname"]}, _metrics), do: :session + defp breakdown_table(%Query{property: "visit:entry_page"}, _metrics), do: :session + defp breakdown_table(%Query{property: "visit:entry_page_hostname"}, _metrics), do: :session + defp breakdown_table(%Query{property: "visit:exit_page"}, _metrics), do: :session + defp breakdown_table(%Query{property: "visit:exit_page_hostname"}, _metrics), do: :session - defp breakdown_table(%Query{dimensions: [dimension]} = query, metrics) do - {_, session_metrics, _} = TableDecider.partition_metrics(metrics, query, dimension) + defp breakdown_table(%Query{property: property} = query, metrics) do + {_, session_metrics, _} = TableDecider.partition_metrics(metrics, query, property) if not Enum.empty?(session_metrics) do :session @@ -304,23 +304,23 @@ defmodule Plausible.Stats.Breakdown do |> sort_results(metrics) end - defp breakdown_sessions(site, %Query{dimensions: [dimension]} = query, metrics) do + defp breakdown_sessions(site, %Query{property: property} = query, metrics) do from(s in query_sessions(site, query), order_by: [desc: fragment("uniq(?)", s.user_id)], select: ^select_session_metrics(metrics, query) ) |> filter_converted_sessions(site, query) - |> do_group_by(dimension) + |> do_group_by(property) |> merge_imported(site, query, metrics) |> add_percentage_metric(site, query, metrics) end - defp breakdown_events(site, %Query{dimensions: [dimension]} = query, metrics) do + defp breakdown_events(site, %Query{property: property} = query, metrics) do from(e in base_event_query(site, query), order_by: [desc: fragment("uniq(?)", e.user_id)], select: %{} ) - |> do_group_by(dimension) + |> do_group_by(property) |> select_merge(^select_event_metrics(metrics)) |> merge_imported(site, query, metrics) |> add_percentage_metric(site, query, metrics) @@ -718,7 +718,7 @@ defmodule Plausible.Stats.Breakdown do q, breakdown_fn, site, - %Query{dimensions: [dimension]} = query, + %Query{property: property} = query, metrics ) do if :conversion_rate in metrics do @@ -730,7 +730,7 @@ defmodule Plausible.Stats.Breakdown do from(e in subquery(q), left_join: c in subquery(breakdown_total_visitors_q), - on: ^on_matches_group_by(group_by_field_names(dimension)), + on: ^on_matches_group_by(group_by_field_names(property)), select_merge: %{ total_visitors: c.visitors, conversion_rate: @@ -742,7 +742,7 @@ defmodule Plausible.Stats.Breakdown do ) }, order_by: [desc: e.visitors], - order_by: ^outer_order_by(group_by_field_names(dimension)) + order_by: ^outer_order_by(group_by_field_names(property)) ) else q diff --git a/lib/plausible/stats/email_report.ex b/lib/plausible/stats/email_report.ex index 1dc26465bb1b..675597731b34 100644 --- a/lib/plausible/stats/email_report.ex +++ b/lib/plausible/stats/email_report.ex @@ -40,7 +40,7 @@ defmodule Plausible.Stats.EmailReport do end defp put_top_5_pages(stats, site, query) do - query = Query.set_dimensions(query, ["event:page"]) + query = Query.set_property(query, "event:page") pages = Stats.breakdown(site, query, [:visitors], {5, 1}) Map.put(stats, :pages, pages) end @@ -48,8 +48,8 @@ defmodule Plausible.Stats.EmailReport do defp put_top_5_sources(stats, site, query) do query = query - |> Query.put_filter([:is_not, "visit:source", ["Direct / None"]]) - |> Query.set_dimensions(["visit:source"]) + |> Query.put_filter([:is_not, "visit:source", "Direct / None"]) + |> Query.set_property("visit:source") sources = Stats.breakdown(site, query, [:visitors], {5, 1}) diff --git a/lib/plausible/stats/filters/legacy_dashboard_filter_parser.ex b/lib/plausible/stats/filters/dashboard_filter_parser.ex similarity index 82% rename from lib/plausible/stats/filters/legacy_dashboard_filter_parser.ex rename to lib/plausible/stats/filters/dashboard_filter_parser.ex index 67e8c8cd0f89..926642eac4e5 100644 --- a/lib/plausible/stats/filters/legacy_dashboard_filter_parser.ex +++ b/lib/plausible/stats/filters/dashboard_filter_parser.ex @@ -1,4 +1,4 @@ -defmodule Plausible.Stats.Filters.LegacyDashboardFilterParser do +defmodule Plausible.Stats.Filters.DashboardFilterParser do @moduledoc false import Plausible.Stats.Filters.Utils @@ -36,40 +36,40 @@ defmodule Plausible.Stats.Filters.LegacyDashboardFilterParser do cond do is_negated && is_wildcard && is_list -> - [:does_not_match, key, val] + [:not_matches_member, key, val] is_negated && is_contains && is_list -> - [:does_not_match, key, Enum.map(val, &"**#{&1}**")] + [:not_matches_member, key, Enum.map(val, &"**#{&1}**")] is_wildcard && is_list -> - [:matches, key, val] + [:matches_member, key, val] is_negated && is_wildcard -> - [:does_not_match, key, [val]] + [:does_not_match, key, val] is_negated && is_list -> - [:is_not, key, val] + [:not_member, key, val] is_negated && is_contains -> - [:does_not_match, key, ["**" <> val <> "**"]] + [:does_not_match, key, "**" <> val <> "**"] is_contains && is_list -> - [:matches, key, Enum.map(val, &"**#{&1}**")] + [:matches_member, key, Enum.map(val, &"**#{&1}**")] is_negated -> - [:is_not, key, [val]] + [:is_not, key, val] is_list -> - [:is, key, val] + [:member, key, val] is_contains -> - [:matches, key, ["**" <> val <> "**"]] + [:matches, key, "**" <> val <> "**"] is_wildcard -> - [:matches, key, [val]] + [:matches, key, val] true -> - [:is, key, [val]] + [:is, key, val] end end diff --git a/lib/plausible/stats/filters/filters.ex b/lib/plausible/stats/filters/filters.ex index b6760965edf7..3775e734359f 100644 --- a/lib/plausible/stats/filters/filters.ex +++ b/lib/plausible/stats/filters/filters.ex @@ -3,8 +3,7 @@ defmodule Plausible.Stats.Filters do A module for parsing filters used in stat queries. """ - alias Plausible.Stats.Filters.QueryParser - alias Plausible.Stats.Filters.{LegacyDashboardFilterParser, StatsAPIFilterParser} + alias Plausible.Stats.Filters.{DashboardFilterParser, StatsAPIFilterParser} @visit_props [ :source, @@ -48,39 +47,32 @@ defmodule Plausible.Stats.Filters do Depending on the format and type of the `filters` argument, returns: - * a decoded list, when `filters` is encoded JSON - * a parsed filter list, when `filters` is a filter expression string - * the same list, when `filters` is a map + * a decoded map, when `filters` is encoded JSON + * a parsed filter map, when `filters` is a filter expression string + * the same map, when `filters` is a map - Returns an empty list when argument type is unexpected (e.g. `nil`). + Returns an empty map when argument type is unexpected (e.g. `nil`). ### Examples: iex> Filters.parse("{\\"page\\":\\"/blog/**\\"}") - [[:matches, "event:page", ["/blog/**"]]] + [[:matches, "event:page", "/blog/**"]] iex> Filters.parse("visit:browser!=Chrome") - [[:is_not, "visit:browser", ["Chrome"]]] + [[:is_not, "visit:browser", "Chrome"]] iex> Filters.parse(nil) [] """ def parse(filters) when is_binary(filters) do case Jason.decode(filters) do - {:ok, filters} when is_map(filters) or is_list(filters) -> parse(filters) + {:ok, filters} when is_map(filters) -> DashboardFilterParser.parse_and_prefix(filters) {:ok, _} -> [] {:error, err} -> StatsAPIFilterParser.parse_filter_expression(err.data) end end - def parse(filters) when is_map(filters), - do: LegacyDashboardFilterParser.parse_and_prefix(filters) - - def parse(filters) when is_list(filters) do - {:ok, parsed_filters} = QueryParser.parse_filters(filters) - parsed_filters - end - + def parse(filters) when is_map(filters), do: DashboardFilterParser.parse_and_prefix(filters) def parse(_), do: [] def without_prefix(property) do diff --git a/lib/plausible/stats/filters/query_parser.ex b/lib/plausible/stats/filters/query_parser.ex deleted file mode 100644 index d9825a2ccc0d..000000000000 --- a/lib/plausible/stats/filters/query_parser.ex +++ /dev/null @@ -1,203 +0,0 @@ -defmodule Plausible.Stats.Filters.QueryParser do - @moduledoc false - - alias Plausible.Stats.Filters - - def parse(params) when is_map(params) do - with {:ok, metrics} <- parse_metrics(Map.get(params, "metrics", [])), - {:ok, filters} <- parse_filters(Map.get(params, "filters", [])), - {:ok, date_range} <- parse_date_range(Map.get(params, "date_range")), - {:ok, dimensions} <- parse_dimensions(Map.get(params, "dimensions", [])), - {:ok, order_by} <- parse_order_by(Map.get(params, "order_by")), - query = %{ - metrics: metrics, - filters: filters, - date_range: date_range, - dimensions: dimensions, - order_by: order_by - }, - :ok <- validate_order_by(query) do - {:ok, query} - end - end - - defp parse_metrics([]), do: {:error, "No valid metrics passed"} - - defp parse_metrics(metrics) when is_list(metrics) do - if length(metrics) == length(Enum.uniq(metrics)) do - parse_list(metrics, &parse_metric/1) - else - {:error, "Metrics cannot be queried multiple times"} - end - end - - defp parse_metrics(_invalid_metrics), do: {:error, "Invalid metrics passed"} - - defp parse_metric("time_on_page"), do: {:ok, :time_on_page} - defp parse_metric("conversion_rate"), do: {:ok, :conversion_rate} - defp parse_metric("visitors"), do: {:ok, :visitors} - defp parse_metric("pageviews"), do: {:ok, :pageviews} - defp parse_metric("events"), do: {:ok, :events} - defp parse_metric("visits"), do: {:ok, :visits} - defp parse_metric("bounce_rate"), do: {:ok, :bounce_rate} - defp parse_metric("visit_duration"), do: {:ok, :visit_duration} - defp parse_metric(unknown_metric), do: {:error, "Unknown metric '#{inspect(unknown_metric)}'"} - - def parse_filters(filters) when is_list(filters) do - parse_list(filters, &parse_filter/1) - end - - def parse_filters(_invalid_metrics), do: {:error, "Invalid filters passed"} - - defp parse_filter(filter) do - with {:ok, operator} <- parse_operator(filter), - {:ok, filter_key} <- parse_filter_key(filter), - {:ok, rest} <- parse_filter_rest(operator, filter) do - {:ok, [operator, filter_key | rest]} - end - end - - defp parse_operator(["is" | _rest]), do: {:ok, :is} - defp parse_operator(["is_not" | _rest]), do: {:ok, :is_not} - defp parse_operator(["matches" | _rest]), do: {:ok, :matches} - defp parse_operator(["does_not_match" | _rest]), do: {:ok, :does_not_match} - defp parse_operator(filter), do: {:error, "Unknown operator for filter '#{inspect(filter)}'"} - - defp parse_filter_key([_operator, filter_key | _rest] = filter) do - parse_filter_key_string(filter_key, "Invalid filter '#{inspect(filter)}") - end - - defp parse_filter_key(filter), do: {:error, "Invalid filter '#{inspect(filter)}'"} - - defp parse_filter_rest(:is, filter), do: parse_clauses_list(filter) - defp parse_filter_rest(:is_not, filter), do: parse_clauses_list(filter) - defp parse_filter_rest(:matches, filter), do: parse_clauses_list(filter) - defp parse_filter_rest(:does_not_match, filter), do: parse_clauses_list(filter) - - defp parse_clauses_list([_operation, filter_key, list] = filter) when is_list(list) do - all_strings? = Enum.all?(list, &is_bitstring/1) - - cond do - filter_key == "event:goal" && all_strings? -> {:ok, [Filters.Utils.wrap_goal_value(list)]} - filter_key != "event:goal" && all_strings? -> {:ok, [list]} - true -> {:error, "Invalid filter '#{inspect(filter)}'"} - end - end - - defp parse_clauses_list(filter), do: {:error, "Invalid filter '#{inspect(filter)}'"} - - defp parse_date_range("day"), do: {:ok, "day"} - defp parse_date_range("7d"), do: {:ok, "7d"} - defp parse_date_range("30d"), do: {:ok, "30d"} - defp parse_date_range("month"), do: {:ok, "month"} - defp parse_date_range("6mo"), do: {:ok, "6mo"} - defp parse_date_range("12mo"), do: {:ok, "6mo"} - defp parse_date_range("year"), do: {:ok, "year"} - defp parse_date_range("all"), do: {:ok, "all"} - - defp parse_date_range([from_date_string, to_date_string]) - when is_bitstring(from_date_string) and is_bitstring(to_date_string) do - with {:ok, from_date} <- Date.from_iso8601(from_date_string), - {:ok, to_date} <- Date.from_iso8601(to_date_string) do - {:ok, Date.range(from_date, to_date)} - else - _ -> {:error, "Invalid date_range '#{inspect([from_date_string, to_date_string])}'"} - end - end - - defp parse_date_range(unknown), do: {:error, "Invalid date range '#{inspect(unknown)}'"} - - defp parse_dimensions(dimensions) when is_list(dimensions) do - if length(dimensions) == length(Enum.uniq(dimensions)) do - parse_list( - dimensions, - &parse_filter_key_string(&1, "Invalid dimensions '#{inspect(dimensions)}'") - ) - else - {:error, "Some dimensions are listed multiple times"} - end - end - - defp parse_dimensions(dimensions), do: {:error, "Invalid dimensions '#{inspect(dimensions)}'"} - - def parse_order_by(order_by) when is_list(order_by) do - parse_list(order_by, &parse_order_by_entry/1) - end - - def parse_order_by(nil), do: {:ok, nil} - def parse_order_by(order_by), do: {:error, "Invalid order_by '#{inspect(order_by)}'"} - - def parse_order_by_entry(entry) do - with {:ok, metric_or_dimension} <- parse_metric_or_dimension(entry), - {:ok, order_direction} <- parse_order_direction(entry) do - {:ok, {metric_or_dimension, order_direction}} - end - end - - def parse_metric_or_dimension([metric_or_dimension, _] = entry) do - case {parse_metric(metric_or_dimension), parse_filter_key_string(metric_or_dimension)} do - {{:ok, metric}, _} -> {:ok, metric} - {_, {:ok, dimension}} -> {:ok, dimension} - _ -> {:error, "Invalid order_by entry '#{inspect(entry)}'"} - end - end - - def parse_order_direction([_, "asc"]), do: {:ok, :asc} - def parse_order_direction([_, "desc"]), do: {:ok, :desc} - def parse_order_direction(entry), do: {:error, "Invalid order_by entry '#{inspect(entry)}'"} - - defp parse_filter_key_string(filter_key, error_message \\ "") do - case filter_key do - "event:props:" <> _property_name -> - {:ok, filter_key} - - "event:" <> key -> - if key in Filters.event_props() do - {:ok, filter_key} - else - {:error, error_message} - end - - "visit:" <> key -> - if key in Filters.visit_props() do - {:ok, filter_key} - else - {:error, error_message} - end - - _ -> - {:error, error_message} - end - end - - def validate_order_by(query) do - if query.order_by do - valid_values = query.metrics ++ query.dimensions - - invalid_entry = - Enum.find(query.order_by, fn {value, _direction} -> - not Enum.member?(valid_values, value) - end) - - case invalid_entry do - nil -> - :ok - - _ -> - {:error, - "Invalid order_by entry '#{inspect(invalid_entry)}'. Entry is not a queried metric or dimension"} - end - else - :ok - end - end - - defp parse_list(list, parser_function) do - Enum.reduce_while(list, {:ok, []}, fn value, {:ok, results} -> - case parser_function.(value) do - {:ok, result} -> {:cont, {:ok, results ++ [result]}} - {:error, _} = error -> {:halt, error} - end - end) - end -end diff --git a/lib/plausible/stats/filters/stats_api_filter_parser.ex b/lib/plausible/stats/filters/stats_api_filter_parser.ex index 5f02b6c8c633..fc58168993fa 100644 --- a/lib/plausible/stats/filters/stats_api_filter_parser.ex +++ b/lib/plausible/stats/filters/stats_api_filter_parser.ex @@ -27,11 +27,11 @@ defmodule Plausible.Stats.Filters.StatsAPIFilterParser do final_value = remove_escape_chars(raw_value) cond do - is_wildcard? && is_negated? -> [:does_not_match, key, [raw_value]] - is_wildcard? -> [:matches, key, [raw_value]] - is_list? -> [:is, key, parse_member_list(raw_value)] - is_negated? -> [:is_not, key, [final_value]] - true -> [:is, key, [final_value]] + is_wildcard? && is_negated? -> [:does_not_match, key, raw_value] + is_wildcard? -> [:matches, key, raw_value] + is_list? -> [:member, key, parse_member_list(raw_value)] + is_negated? -> [:is_not, key, final_value] + true -> [:is, key, final_value] end |> reject_invalid_country_codes() @@ -71,10 +71,10 @@ defmodule Plausible.Stats.Filters.StatsAPIFilterParser do |> wrap_goal_value() cond do - is_list? && is_wildcard? -> [:matches, key, value] - is_list? -> [:is, key, value] - is_wildcard? -> [:matches, key, [value]] - true -> [:is, key, [value]] + is_list? && is_wildcard? -> [:matches_member, key, value] + is_list? -> [:member, key, value] + is_wildcard? -> [:matches, key, value] + true -> [:is, key, value] end end end diff --git a/lib/plausible/stats/filters/utils.ex b/lib/plausible/stats/filters/utils.ex index 396b62e13e65..5bb612b62e3c 100644 --- a/lib/plausible/stats/filters/utils.ex +++ b/lib/plausible/stats/filters/utils.ex @@ -1,6 +1,6 @@ defmodule Plausible.Stats.Filters.Utils do @moduledoc """ - Contains utility functions shared between `LegacyDashboardFilterParser` + Contains utility functions shared between `DashboardFilterParser` and `StatsAPIFilterParser`. """ diff --git a/lib/plausible/stats/filters/where_builder.ex b/lib/plausible/stats/filters/where_builder.ex index 77a02f91366e..04f66c836344 100644 --- a/lib/plausible/stats/filters/where_builder.ex +++ b/lib/plausible/stats/filters/where_builder.ex @@ -68,17 +68,35 @@ defmodule Plausible.Stats.Filters.WhereBuilder do ) end - defp add_filter(:events, _query, [:is, "event:name", list]) do + defp add_filter(:events, _query, [:is, "event:name", name]) do + dynamic([e], e.name == ^name) + end + + defp add_filter(:events, _query, [:member, "event:name", list]) do dynamic([e], e.name in ^list) end - defp add_filter(:events, _query, [:is, "event:goal", clauses]) do + defp add_filter(:events, _query, [:is, "event:goal", {:page, path}]) do + dynamic([e], e.pathname == ^path and e.name == "pageview") + end + + defp add_filter(:events, _query, [:matches, "event:goal", {:page, expr}]) do + regex = page_regex(expr) + + dynamic([e], fragment("match(?, ?)", e.pathname, ^regex) and e.name == "pageview") + end + + defp add_filter(:events, _query, [:is, "event:goal", {:event, event}]) do + dynamic([e], e.name == ^event) + end + + defp add_filter(:events, _query, [:member, "event:goal", clauses]) do {events, pages} = split_goals(clauses) dynamic([e], (e.pathname in ^pages and e.name == "pageview") or e.name in ^events) end - defp add_filter(:events, _query, [:matches, "event:goal", clauses]) do + defp add_filter(:events, _query, [:matches_member, "event:goal", clauses]) do {events, pages} = split_goals(clauses, &page_regex/1) event_clause = @@ -151,7 +169,39 @@ defmodule Plausible.Stats.Filters.WhereBuilder do true end - defp filter_custom_prop(prop_name, column_name, [:is, _, values]) do + defp filter_custom_prop(prop_name, column_name, [:is, _, "(none)"]) do + dynamic([t], not has_key(t, column_name, ^prop_name)) + end + + defp filter_custom_prop(prop_name, column_name, [:is, _, value]) do + dynamic( + [t], + has_key(t, column_name, ^prop_name) and get_by_key(t, column_name, ^prop_name) == ^value + ) + end + + defp filter_custom_prop(prop_name, column_name, [:is_not, _, "(none)"]) do + dynamic([t], has_key(t, column_name, ^prop_name)) + end + + defp filter_custom_prop(prop_name, column_name, [:is_not, _, value]) do + dynamic( + [t], + not has_key(t, column_name, ^prop_name) or get_by_key(t, column_name, ^prop_name) != ^value + ) + end + + defp filter_custom_prop(prop_name, column_name, [:matches, _, value]) do + regex = page_regex(value) + + dynamic( + [t], + has_key(t, column_name, ^prop_name) and + fragment("match(?, ?)", get_by_key(t, column_name, ^prop_name), ^regex) + ) + end + + defp filter_custom_prop(prop_name, column_name, [:member, _, values]) do none_value_included = Enum.member?(values, "(none)") dynamic( @@ -161,7 +211,7 @@ defmodule Plausible.Stats.Filters.WhereBuilder do ) end - defp filter_custom_prop(prop_name, column_name, [:is_not, _, values]) do + defp filter_custom_prop(prop_name, column_name, [:not_member, _, values]) do none_value_included = Enum.member?(values, "(none)") dynamic( @@ -175,7 +225,7 @@ defmodule Plausible.Stats.Filters.WhereBuilder do ) end - defp filter_custom_prop(prop_name, column_name, [:matches, _, clauses]) do + defp filter_custom_prop(prop_name, column_name, [:matches_member, _, clauses]) do regexes = Enum.map(clauses, &page_regex/1) dynamic( @@ -189,22 +239,42 @@ defmodule Plausible.Stats.Filters.WhereBuilder do ) end - defp filter_field(db_field, [:matches, _key, glob_exprs]) do + defp filter_field(db_field, [:is, _key, value]) do + value = db_field_val(db_field, value) + dynamic([x], field(x, ^db_field) == ^value) + end + + defp filter_field(db_field, [:is_not, _key, value]) do + value = db_field_val(db_field, value) + dynamic([x], field(x, ^db_field) != ^value) + end + + defp filter_field(db_field, [:matches_member, _key, glob_exprs]) do page_regexes = Enum.map(glob_exprs, &page_regex/1) dynamic([x], fragment("multiMatchAny(?, ?)", field(x, ^db_field), ^page_regexes)) end - defp filter_field(db_field, [:does_not_match, _key, glob_exprs]) do + defp filter_field(db_field, [:not_matches_member, _key, glob_exprs]) do page_regexes = Enum.map(glob_exprs, &page_regex/1) dynamic([x], fragment("not(multiMatchAny(?, ?))", field(x, ^db_field), ^page_regexes)) end - defp filter_field(db_field, [:is, _key, list]) do + defp filter_field(db_field, [:matches, _key, glob_expr]) do + regex = page_regex(glob_expr) + dynamic([x], fragment("match(?, ?)", field(x, ^db_field), ^regex)) + end + + defp filter_field(db_field, [:does_not_match, _key, glob_expr]) do + regex = page_regex(glob_expr) + dynamic([x], fragment("not(match(?, ?))", field(x, ^db_field), ^regex)) + end + + defp filter_field(db_field, [:member, _key, list]) do list = Enum.map(list, &db_field_val(db_field, &1)) dynamic([x], field(x, ^db_field) in ^list) end - defp filter_field(db_field, [:is_not, _key, list]) do + defp filter_field(db_field, [:not_member, _key, list]) do list = Enum.map(list, &db_field_val(db_field, &1)) dynamic([x], field(x, ^db_field) not in ^list) end diff --git a/lib/plausible/stats/imported/base.ex b/lib/plausible/stats/imported/base.ex index a446817a37bc..539f3fb83cac 100644 --- a/lib/plausible/stats/imported/base.ex +++ b/lib/plausible/stats/imported/base.ex @@ -89,11 +89,17 @@ defmodule Plausible.Stats.Imported.Base do new_filters = query.filters |> Enum.reject(fn - [:is, "event:name", ["pageview"]] -> true + [:is, "event:name", "pageview"] -> true _ -> false end) |> Enum.flat_map(fn filter -> case filter do + [op, "event:goal", {:event, name}] -> + [[op, "event:name", name]] + + [op, "event:goal", {:page, page}] -> + [[op, "event:page", page]] + [op, "event:goal", events] -> events |> Enum.group_by(&elem(&1, 0), &elem(&1, 1)) @@ -113,48 +119,44 @@ defmodule Plausible.Stats.Imported.Base do defp custom_prop_query?(query) do query.filters |> Enum.map(&Enum.at(&1, 1)) - |> Enum.concat(query.dimensions) + |> Enum.concat([query.property]) |> Enum.any?(&(&1 in @imported_custom_props)) end - defp do_decide_custom_prop_table(%{dimensions: [dimension]} = query) - when dimension in @imported_custom_props do - do_decide_custom_prop_table(query, dimension) + defp do_decide_custom_prop_table(%{property: property} = query) + when property in @imported_custom_props do + do_decide_custom_prop_table(query, property) end - defp do_decide_custom_prop_table(%{dimensions: dimensions} = query) do - if dimensions == [] or - (length(dimensions) == 1 and hd(dimensions) in ["event:goal", "event:name"]) do - custom_prop_filters = - query.filters - |> Enum.map(&Enum.at(&1, 1)) - |> Enum.filter(&(&1 in @imported_custom_props)) - |> Enum.uniq() - - case custom_prop_filters do - [custom_prop_filter] -> - do_decide_custom_prop_table(query, custom_prop_filter) - - _ -> - nil - end - else - nil + defp do_decide_custom_prop_table(%{property: property} = query) + when property in [nil, "event:goal", "event:name"] do + custom_prop_filters = + query.filters + |> Enum.map(&Enum.at(&1, 1)) + |> Enum.filter(&(&1 in @imported_custom_props)) + |> Enum.uniq() + + case custom_prop_filters do + [custom_prop_filter] -> + do_decide_custom_prop_table(query, custom_prop_filter) + + _ -> + nil end end + defp do_decide_custom_prop_table(_query), do: nil + defp do_decide_custom_prop_table(query, property) do has_required_name_filter? = - query.filters - |> Enum.flat_map(fn - [:is, "event:name", names] -> names - _ -> [] + Enum.any?(query.filters, fn + [:is, "event:name", name] -> name in special_goals_for(property) + _ -> false end) - |> Enum.any?(&(&1 in special_goals_for(property))) has_unsupported_filters? = - Enum.any?(query.filters, fn [_, filter_key | _] -> - filter_key not in [property, "event:name"] + Enum.any?(query.filters, fn [_, filtered_prop | _] -> + filtered_prop not in [property, "event:name"] end) if has_required_name_filter? and not has_unsupported_filters? do @@ -164,17 +166,17 @@ defmodule Plausible.Stats.Imported.Base do end end - defp do_decide_table(%Query{filters: [], dimensions: []}), do: "imported_visitors" + defp do_decide_table(%Query{filters: [], property: nil}), do: "imported_visitors" - defp do_decide_table(%Query{filters: [], dimensions: ["event:goal"]}) do + defp do_decide_table(%Query{filters: [], property: "event:goal"}) do "imported_custom_events" end - defp do_decide_table(%Query{filters: [], dimensions: [dimension]}) do - @property_to_table_mappings[dimension] + defp do_decide_table(%Query{filters: [], property: property}) do + @property_to_table_mappings[property] end - defp do_decide_table(%Query{filters: filters, dimensions: ["event:goal"]}) do + defp do_decide_table(%Query{filters: filters, property: "event:goal"}) do filter_props = Enum.map(filters, &Enum.at(&1, 1)) any_event_name_filters? = "event:name" in filter_props @@ -189,11 +191,11 @@ defmodule Plausible.Stats.Imported.Base do end end - defp do_decide_table(%Query{filters: filters, dimensions: dimensions}) do + defp do_decide_table(%Query{filters: filters, property: property}) do table_candidates = filters - |> Enum.map(fn [_, filter_key | _] -> filter_key end) - |> Enum.concat(dimensions) + |> Enum.map(fn [_, prop | _] -> prop end) + |> Enum.concat(if property, do: [property], else: []) |> Enum.map(fn "visit:screen" -> "visit:device" prop -> prop @@ -207,8 +209,8 @@ defmodule Plausible.Stats.Imported.Base do end defp apply_filter(q, %Query{filters: filters}) do - Enum.reduce(filters, q, fn [_, filter_key | _] = filter, q -> - db_field = Plausible.Stats.Filters.without_prefix(filter_key) + Enum.reduce(filters, q, fn [_, filtered_prop | _] = filter, q -> + db_field = Plausible.Stats.Filters.without_prefix(filtered_prop) mapped_db_field = Map.get(@db_field_mappings, db_field, db_field) condition = Filters.WhereBuilder.build_condition(mapped_db_field, filter) diff --git a/lib/plausible/stats/imported/imported.ex b/lib/plausible/stats/imported/imported.ex index 2e680f824f1d..a387ab1a24f5 100644 --- a/lib/plausible/stats/imported/imported.ex +++ b/lib/plausible/stats/imported/imported.ex @@ -266,9 +266,9 @@ defmodule Plausible.Stats.Imported do def merge_imported(q, _, %Query{include_imported: false}, _), do: q - def merge_imported(q, site, %Query{dimensions: [dimension]} = query, metrics) - when dimension in @imported_properties do - dim = Plausible.Stats.Filters.without_prefix(dimension) + def merge_imported(q, site, %Query{property: property} = query, metrics) + when property in @imported_properties do + dim = Plausible.Stats.Filters.without_prefix(property) imported_q = site @@ -302,7 +302,7 @@ defmodule Plausible.Stats.Imported do |> apply_order_by(metrics) end - def merge_imported(q, site, %Query{dimensions: []} = query, metrics) do + def merge_imported(q, site, %Query{property: nil} = query, metrics) do imported_q = site |> Imported.Base.query_imported(query) diff --git a/lib/plausible/stats/query.ex b/lib/plausible/stats/query.ex index 737b3ba0bfdb..76041690222d 100644 --- a/lib/plausible/stats/query.ex +++ b/lib/plausible/stats/query.ex @@ -4,7 +4,7 @@ defmodule Plausible.Stats.Query do defstruct date_range: nil, interval: nil, period: nil, - dimensions: [], + property: nil, filters: [], sample_threshold: 20_000_000, imported_data_requested: false, @@ -30,7 +30,7 @@ defmodule Plausible.Stats.Query do |> put_experimental_session_count(site, params) |> put_experimental_reduced_joins(site, params) |> put_period(site, params) - |> put_dimensions(params) + |> put_breakdown_property(params) |> put_interval(params) |> put_parsed_filters(params) |> put_imported_opts(site, params) @@ -185,12 +185,8 @@ defmodule Plausible.Stats.Query do put_period(query, site, Map.merge(params, %{"period" => "30d"})) end - defp put_dimensions(query, params) do - if not is_nil(params["property"]) do - struct!(query, dimensions: [params["property"]]) - else - struct!(query, dimensions: Map.get(params, "dimensions", [])) - end + defp put_breakdown_property(query, params) do + struct!(query, property: params["property"]) end defp put_interval(%{:period => "all"} = query, params) do @@ -207,10 +203,10 @@ defmodule Plausible.Stats.Query do struct!(query, filters: Filters.parse(params["filters"])) end - @spec set_dimensions(t(), list(String.t())) :: t() - def set_dimensions(query, dimensions) do + @spec set_property(t(), String.t() | nil) :: t() + def set_property(query, property) do query - |> struct!(dimensions: dimensions) + |> struct!(property: property) |> refresh_imported_opts() end @@ -326,7 +322,7 @@ defmodule Plausible.Stats.Query do Tracer.set_attributes([ {"plausible.query.interval", query.interval}, {"plausible.query.period", query.period}, - {"plausible.query.dimensions", query.dimensions |> Enum.join(";")}, + {"plausible.query.breakdown_property", query.property}, {"plausible.query.include_imported", query.include_imported}, {"plausible.query.filter_keys", filter_keys}, {"plausible.query.metrics", metrics} diff --git a/lib/plausible_web/controllers/api/external_stats_controller.ex b/lib/plausible_web/controllers/api/external_stats_controller.ex index 248f709baf13..f8145b8ccfa0 100644 --- a/lib/plausible_web/controllers/api/external_stats_controller.ex +++ b/lib/plausible_web/controllers/api/external_stats_controller.ex @@ -124,14 +124,14 @@ defmodule PlausibleWeb.Api.ExternalStatsController do prop_filter = Query.get_filter_by_prefix(query, "event:props:") query_allowed? = - case {prop_filter, query.dimensions, allowed_props} do + case {prop_filter, query.property, allowed_props} do {_, _, :all} -> true {[_, "event:props:" <> prop | _], _property, allowed_props} -> prop in allowed_props - {_filter, ["event:props:" <> prop], allowed_props} -> + {_filter, "event:props:" <> prop, allowed_props} -> prop in allowed_props _ -> @@ -171,10 +171,10 @@ defmodule PlausibleWeb.Api.ExternalStatsController do Query.get_filter(query, "event:name") -> {:error, "Metric `#{metric}` cannot be queried when filtering by `event:name`"} - query.dimensions == ["event:page"] -> + query.property == "event:page" -> {:ok, metric} - not Enum.empty?(query.dimensions) -> + not is_nil(query.property) -> {:error, "Metric `#{metric}` is not supported in breakdown queries (except `event:page` breakdown)"} @@ -189,7 +189,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController do defp validate_metric("conversion_rate" = metric, query) do cond do - query.dimensions == ["event:goal"] -> + query.property == "event:goal" -> {:ok, metric} Query.get_filter(query, "event:goal") -> @@ -210,7 +210,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController do Query.get_filter(query, "event:page") -> {:error, "Metric `#{metric}` cannot be queried with a filter on `event:page`."} - not Enum.empty?(query.dimensions) -> + query.property != nil -> {:error, "Metric `#{metric}` is not supported in breakdown queries."} true -> @@ -230,9 +230,9 @@ defmodule PlausibleWeb.Api.ExternalStatsController do defp validate_session_metric(metric, query) do cond do - length(query.dimensions) == 1 and event_only_property?(hd(query.dimensions)) -> + event_only_property?(query.property) -> {:error, - "Session metric `#{metric}` cannot be queried for breakdown by `#{query.dimensions}`."} + "Session metric `#{metric}` cannot be queried for breakdown by `#{query.property}`."} event_only_filter = find_event_only_filter(query) -> {:error, diff --git a/lib/plausible_web/controllers/api/stats_controller.ex b/lib/plausible_web/controllers/api/stats_controller.ex index 5dcf7800cf63..6e0907147720 100644 --- a/lib/plausible_web/controllers/api/stats_controller.ex +++ b/lib/plausible_web/controllers/api/stats_controller.ex @@ -6,7 +6,7 @@ defmodule PlausibleWeb.Api.StatsController do alias Plausible.Stats alias Plausible.Stats.{Query, Comparisons} - alias Plausible.Stats.Filters.LegacyDashboardFilterParser + alias Plausible.Stats.Filters.DashboardFilterParser alias PlausibleWeb.Api.Helpers, as: H require Logger @@ -769,7 +769,7 @@ defmodule PlausibleWeb.Api.StatsController do site = conn.assigns[:site] params = Map.put(params, "property", "visit:referrer") - referrer_filter = LegacyDashboardFilterParser.filter_value("visit:source", referrer) + referrer_filter = DashboardFilterParser.filter_value("visit:source", referrer) query = Query.from(site, params) @@ -903,9 +903,9 @@ defmodule PlausibleWeb.Api.StatsController do total_pageviews_query = query |> Query.remove_filters(["visit:exit_page"]) - |> Query.put_filter([:is, "event:page", pages]) - |> Query.put_filter([:is, "event:name", ["pageview"]]) - |> Query.set_dimensions(["event:page"]) + |> Query.put_filter([:member, "event:page", pages]) + |> Query.put_filter([:is, "event:name", "pageview"]) + |> Query.set_property("event:page") total_pageviews = Stats.breakdown(site, total_pageviews_query, [:pageviews], {limit, 1}) diff --git a/lib/plausible_web/controllers/stats_controller.ex b/lib/plausible_web/controllers/stats_controller.ex index d92c934472cc..1325fe642a93 100644 --- a/lib/plausible_web/controllers/stats_controller.ex +++ b/lib/plausible_web/controllers/stats_controller.ex @@ -367,12 +367,7 @@ defmodule PlausibleWeb.StatsController do defp shared_link_cookie_name(slug), do: "shared-link-" <> slug - defp get_flags(user, site), - do: %{ - multiple_filters: - FunWithFlags.enabled?(:multiple_filters, for: user) || - FunWithFlags.enabled?(:multiple_filters, for: site) - } + defp get_flags(_user, _site), do: %{} defp is_dbip() do on_ee do diff --git a/test/plausible/google/search_console/filters_test.exs b/test/plausible/google/search_console/filters_test.exs index 265df747620b..c6efffa069a1 100644 --- a/test/plausible/google/search_console/filters_test.exs +++ b/test/plausible/google/search_console/filters_test.exs @@ -4,27 +4,19 @@ defmodule Plausible.Google.SearchConsole.FiltersTest do test "transforms simple page filter" do filters = [ - [:is, "visit:entry_page", ["/page"]] + [:is, "visit:entry_page", "/page"] ] {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters) assert transformed == [ - %{ - filters: [ - %{ - dimension: "page", - operator: "includingRegex", - expression: "https://plausible.io/page" - } - ] - } + %{filters: [%{dimension: "page", expression: "https://plausible.io/page"}]} ] end test "transforms matches page filter" do filters = [ - [:matches, "visit:entry_page", ["*page*"]] + [:matches, "visit:entry_page", "*page*"] ] {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters) @@ -42,9 +34,9 @@ defmodule Plausible.Google.SearchConsole.FiltersTest do ] end - test "transforms is page filter" do + test "transforms member page filter" do filters = [ - [:is, "visit:entry_page", ["/pageA", "/pageB"]] + [:member, "visit:entry_page", ["/pageA", "/pageB"]] ] {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters) @@ -62,9 +54,9 @@ defmodule Plausible.Google.SearchConsole.FiltersTest do ] end - test "transforms matches multiple page filter" do + test "transforms matches_member page filter" do filters = [ - [:matches, "visit:entry_page", ["/pageA*", "/pageB*"]] + [:matches_member, "visit:entry_page", ["/pageA*", "/pageB*"]] ] {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters) @@ -84,7 +76,7 @@ defmodule Plausible.Google.SearchConsole.FiltersTest do test "transforms event:page exactly like visit:entry_page" do filters = [ - [:matches, "event:page", ["/pageA*", "/pageB*"]] + [:matches_member, "event:page", ["/pageA*", "/pageB*"]] ] {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters) @@ -104,23 +96,17 @@ defmodule Plausible.Google.SearchConsole.FiltersTest do test "transforms simple visit:screen filter" do filters = [ - [:is, "visit:screen", ["Desktop"]] + [:is, "visit:screen", "Desktop"] ] {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters) - assert transformed == [ - %{ - filters: [ - %{dimension: "device", operator: "includingRegex", expression: "DESKTOP"} - ] - } - ] + assert transformed == [%{filters: [%{dimension: "device", expression: "DESKTOP"}]}] end - test "transforms is visit:screen filter" do + test "transforms member visit:screen filter" do filters = [ - [:is, "visit:screen", ["Mobile", "Tablet"]] + [:member, "visit:screen", ["Mobile", "Tablet"]] ] {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters) @@ -128,7 +114,7 @@ defmodule Plausible.Google.SearchConsole.FiltersTest do assert transformed == [ %{ filters: [ - %{dimension: "device", operator: "includingRegex", expression: "MOBILE|TABLET"} + %{dimension: "device", operator: "includingRegex", expression: "Mobile|Tablet"} ] } ] @@ -136,19 +122,17 @@ defmodule Plausible.Google.SearchConsole.FiltersTest do test "transforms simple visit:country filter to alpha3" do filters = [ - [:is, "visit:country", ["EE"]] + [:is, "visit:country", "EE"] ] {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters) - assert transformed == [ - %{filters: [%{dimension: "country", operator: "includingRegex", expression: "EST"}]} - ] + assert transformed == [%{filters: [%{dimension: "country", expression: "EST"}]}] end test "transforms member visit:country filter" do filters = [ - [:is, "visit:country", ["EE", "PL"]] + [:member, "visit:country", ["EE", "PL"]] ] {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters) @@ -164,9 +148,9 @@ defmodule Plausible.Google.SearchConsole.FiltersTest do test "filters can be combined" do filters = [ - [:is, "visit:country", ["EE", "PL"]], - [:matches, "visit:entry_page", ["*web-analytics*"]], - [:is, "visit:screen", ["Desktop"]] + [:member, "visit:country", ["EE", "PL"]], + [:matches, "visit:entry_page", "*web-analytics*"], + [:is, "visit:screen", "Desktop"] ] {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters) @@ -174,7 +158,7 @@ defmodule Plausible.Google.SearchConsole.FiltersTest do assert transformed == [ %{ filters: [ - %{dimension: "device", operator: "includingRegex", expression: "DESKTOP"}, + %{dimension: "device", expression: "DESKTOP"}, %{ dimension: "page", operator: "includingRegex", diff --git a/test/plausible/stats/dashboard_filter_parser_test.exs b/test/plausible/stats/dashboard_filter_parser_test.exs new file mode 100644 index 000000000000..1602380f3780 --- /dev/null +++ b/test/plausible/stats/dashboard_filter_parser_test.exs @@ -0,0 +1,207 @@ +defmodule Plausible.Stats.DashboardFilterParserTest do + use ExUnit.Case, async: true + alias Plausible.Stats.Filters.DashboardFilterParser + + def assert_parsed(filters, expected_output) do + assert DashboardFilterParser.parse_and_prefix(filters) == expected_output + end + + describe "adding prefix" do + test "adds appropriate prefix to filter" do + %{"page" => "/"} + |> assert_parsed([[:is, "event:page", "/"]]) + + %{"goal" => "Signup"} + |> assert_parsed([[:is, "event:goal", {:event, "Signup"}]]) + + %{"goal" => "Visit /blog"} + |> assert_parsed([[:is, "event:goal", {:page, "/blog"}]]) + + %{"source" => "Google"} + |> assert_parsed([[:is, "visit:source", "Google"]]) + + %{"referrer" => "cnn.com"} + |> assert_parsed([[:is, "visit:referrer", "cnn.com"]]) + + %{"utm_medium" => "search"} + |> assert_parsed([[:is, "visit:utm_medium", "search"]]) + + %{"utm_source" => "bing"} + |> assert_parsed([[:is, "visit:utm_source", "bing"]]) + + %{"utm_content" => "content"} + |> assert_parsed([[:is, "visit:utm_content", "content"]]) + + %{"utm_term" => "term"} + |> assert_parsed([[:is, "visit:utm_term", "term"]]) + + %{"screen" => "Desktop"} + |> assert_parsed([[:is, "visit:screen", "Desktop"]]) + + %{"browser" => "Opera"} + |> assert_parsed([[:is, "visit:browser", "Opera"]]) + + %{"browser_version" => "10.1"} + |> assert_parsed([[:is, "visit:browser_version", "10.1"]]) + + %{"os" => "Linux"} + |> assert_parsed([[:is, "visit:os", "Linux"]]) + + %{"os_version" => "13.0"} + |> assert_parsed([[:is, "visit:os_version", "13.0"]]) + + %{"country" => "EE"} + |> assert_parsed([[:is, "visit:country", "EE"]]) + + %{"region" => "EE-12"} + |> assert_parsed([[:is, "visit:region", "EE-12"]]) + + %{"city" => "123"} + |> assert_parsed([[:is, "visit:city", "123"]]) + + %{"entry_page" => "/blog"} + |> assert_parsed([[:is, "visit:entry_page", "/blog"]]) + + %{"exit_page" => "/blog"} + |> assert_parsed([[:is, "visit:exit_page", "/blog"]]) + + %{"props" => %{"cta" => "Top"}} + |> assert_parsed([[:is, "event:props:cta", "Top"]]) + + %{"hostname" => "dummy.site"} + |> assert_parsed([[:is, "event:hostname", "dummy.site"]]) + end + end + + describe "escaping pipe character" do + test "in simple is filter" do + %{"goal" => ~S(Foo \| Bar)} + |> assert_parsed([[:is, "event:goal", {:event, "Foo | Bar"}]]) + end + + test "in member filter" do + %{"page" => ~S(/|\|)} + |> assert_parsed([[:member, "event:page", ["/", "|"]]]) + end + end + + describe "is not filter type" do + test "simple is not filter" do + %{"page" => "!/"} + |> assert_parsed([[:is_not, "event:page", "/"]]) + + %{"props" => %{"cta" => "!Top"}} + |> assert_parsed([[:is_not, "event:props:cta", "Top"]]) + end + end + + describe "member filter type" do + test "simple member filter" do + %{"page" => "/|/blog"} + |> assert_parsed([[:member, "event:page", ["/", "/blog"]]]) + end + + test "escaping pipe character" do + %{"page" => "/|\\|"} + |> assert_parsed([[:member, "event:page", ["/", "|"]]]) + end + + test "mixed goals" do + %{"goal" => "Signup|Visit /thank-you"} + |> assert_parsed([[:member, "event:goal", [{:event, "Signup"}, {:page, "/thank-you"}]]]) + + %{"goal" => "Visit /thank-you|Signup"} + |> assert_parsed([[:member, "event:goal", [{:page, "/thank-you"}, {:event, "Signup"}]]]) + end + end + + describe "matches_member filter type" do + test "parses matches_member filter type" do + %{"page" => "/|/blog**"} + |> assert_parsed([[:matches_member, "event:page", ["/", "/blog**"]]]) + end + + test "parses not_matches_member filter type" do + %{"page" => "!/|/blog**"} + |> assert_parsed([[:not_matches_member, "event:page", ["/", "/blog**"]]]) + end + end + + describe "contains filter type" do + test "single contains" do + %{"page" => "~blog"} + |> assert_parsed([[:matches, "event:page", "**blog**"]]) + end + + test "negated contains" do + %{"page" => "!~articles"} + |> assert_parsed([[:does_not_match, "event:page", "**articles**"]]) + end + + test "contains member" do + %{"page" => "~articles|blog"} + |> assert_parsed([[:matches_member, "event:page", ["**articles**", "**blog**"]]]) + end + + test "not contains member" do + %{"page" => "!~articles|blog"} + |> assert_parsed([[:not_matches_member, "event:page", ["**articles**", "**blog**"]]]) + end + end + + describe "not_member filter type" do + test "simple not_member filter" do + %{"page" => "!/|/blog"} + |> assert_parsed([[:not_member, "event:page", ["/", "/blog"]]]) + end + + test "mixed goals" do + %{"goal" => "!Signup|Visit /thank-you"} + |> assert_parsed([ + [:not_member, "event:goal", [{:event, "Signup"}, {:page, "/thank-you"}]] + ]) + + %{"goal" => "!Visit /thank-you|Signup"} + |> assert_parsed([ + [:not_member, "event:goal", [{:page, "/thank-you"}, {:event, "Signup"}]] + ]) + end + end + + describe "matches filter type" do + test "can be used with `goal` or `page` filters" do + %{"page" => "/blog/post-*"} + |> assert_parsed([[:matches, "event:page", "/blog/post-*"]]) + + %{"goal" => "Visit /blog/post-*"} + |> assert_parsed([[:matches, "event:goal", {:page, "/blog/post-*"}]]) + end + + test "other filters default to `is` even when wildcard is present" do + %{"country" => "Germa**"} + |> assert_parsed([[:is, "visit:country", "Germa**"]]) + end + end + + describe "does_not_match filter type" do + test "can be used with `page` filter" do + %{"page" => "!/blog/post-*"} + |> assert_parsed([[:does_not_match, "event:page", "/blog/post-*"]]) + end + + test "other filters default to is_not even when wildcard is present" do + %{"country" => "!Germa**"} + |> assert_parsed([[:is_not, "visit:country", "Germa**"]]) + end + end + + describe "contains prefix filter type" do + test "can be used with any filter" do + %{"page" => "~/blog/post"} + |> assert_parsed([[:matches, "event:page", "**/blog/post**"]]) + + %{"source" => "~facebook"} + |> assert_parsed([[:matches, "visit:source", "**facebook**"]]) + end + end +end diff --git a/test/plausible/stats/filters_test.exs b/test/plausible/stats/filters_test.exs index a99423cd54ec..aa5b77412f62 100644 --- a/test/plausible/stats/filters_test.exs +++ b/test/plausible/stats/filters_test.exs @@ -12,70 +12,70 @@ defmodule Plausible.Stats.FiltersTest do describe "parses filter expression" do test "simple positive" do "event:name==pageview" - |> assert_parsed([[:is, "event:name", ["pageview"]]]) + |> assert_parsed([[:is, "event:name", "pageview"]]) end test "simple negative" do "event:name!=pageview" - |> assert_parsed([[:is_not, "event:name", ["pageview"]]]) + |> assert_parsed([[:is_not, "event:name", "pageview"]]) end test "whitespace is trimmed" do " event:name == pageview " - |> assert_parsed([[:is, "event:name", ["pageview"]]]) + |> assert_parsed([[:is, "event:name", "pageview"]]) end test "wildcard" do "event:page==/blog/post-*" - |> assert_parsed([[:matches, "event:page", ["/blog/post-*"]]]) + |> assert_parsed([[:matches, "event:page", "/blog/post-*"]]) end test "negative wildcard" do "event:page!=/blog/post-*" - |> assert_parsed([[:does_not_match, "event:page", ["/blog/post-*"]]]) + |> assert_parsed([[:does_not_match, "event:page", "/blog/post-*"]]) end test "custom event goal" do "event:goal==Signup" - |> assert_parsed([[:is, "event:goal", [{:event, "Signup"}]]]) + |> assert_parsed([[:is, "event:goal", {:event, "Signup"}]]) end test "pageview goal" do "event:goal==Visit /blog" - |> assert_parsed([[:is, "event:goal", [{:page, "/blog"}]]]) + |> assert_parsed([[:is, "event:goal", {:page, "/blog"}]]) end - test "is" do + test "member" do "visit:country==FR|GB|DE" - |> assert_parsed([[:is, "visit:country", ["FR", "GB", "DE"]]]) + |> assert_parsed([[:member, "visit:country", ["FR", "GB", "DE"]]]) end test "member + wildcard" do "event:page==/blog**|/newsletter|/*/" - |> assert_parsed([[:matches, "event:page", ["/blog**|/newsletter|/*/"]]]) + |> assert_parsed([[:matches, "event:page", "/blog**|/newsletter|/*/"]]) end test "combined with \";\"" do "event:page==/blog**|/newsletter|/*/ ; visit:country==FR|GB|DE" |> assert_parsed([ - [:matches, "event:page", ["/blog**|/newsletter|/*/"]], - [:is, "visit:country", ["FR", "GB", "DE"]] + [:matches, "event:page", "/blog**|/newsletter|/*/"], + [:member, "visit:country", ["FR", "GB", "DE"]] ]) end test "escaping pipe character" do "utm_campaign==campaign \\| 1" - |> assert_parsed([[:is, "utm_campaign", ["campaign | 1"]]]) + |> assert_parsed([[:is, "utm_campaign", "campaign | 1"]]) end - test "escaping pipe character in is filter" do + test "escaping pipe character in member filter" do "utm_campaign==campaign \\| 1|campaign \\| 2" - |> assert_parsed([[:is, "utm_campaign", ["campaign | 1", "campaign | 2"]]]) + |> assert_parsed([[:member, "utm_campaign", ["campaign | 1", "campaign | 2"]]]) end - test "keeps escape characters in is + wildcard filter" do + test "keeps escape characters in member + wildcard filter" do "event:page==/**\\|page|/other/page" - |> assert_parsed([[:matches, "event:page", ["/**\\|page|/other/page"]]]) + |> assert_parsed([[:matches, "event:page", "/**\\|page|/other/page"]]) end test "gracefully fails to parse garbage" do @@ -98,12 +98,4 @@ defmodule Plausible.Stats.FiltersTest do |> assert_parsed([]) end end - - describe "parses filters list" do - test "simple" do - [["is", "event:name", ["pageview"]]] - |> Jason.encode!() - |> assert_parsed([[:is, "event:name", ["pageview"]]]) - end - end end diff --git a/test/plausible/stats/legacy_dashboard_filter_parser_test.exs b/test/plausible/stats/legacy_dashboard_filter_parser_test.exs deleted file mode 100644 index 8a3e94053a32..000000000000 --- a/test/plausible/stats/legacy_dashboard_filter_parser_test.exs +++ /dev/null @@ -1,201 +0,0 @@ -defmodule Plausible.Stats.LegacyDashboardFilterParserTest do - use ExUnit.Case, async: true - alias Plausible.Stats.Filters.LegacyDashboardFilterParser - - def assert_parsed(filters, expected_output) do - assert LegacyDashboardFilterParser.parse_and_prefix(filters) == expected_output - end - - describe "adding prefix" do - test "adds appropriate prefix to filter" do - %{"page" => "/"} - |> assert_parsed([[:is, "event:page", ["/"]]]) - - %{"goal" => "Signup"} - |> assert_parsed([[:is, "event:goal", [{:event, "Signup"}]]]) - - %{"goal" => "Visit /blog"} - |> assert_parsed([[:is, "event:goal", [{:page, "/blog"}]]]) - - %{"source" => "Google"} - |> assert_parsed([[:is, "visit:source", ["Google"]]]) - - %{"referrer" => "cnn.com"} - |> assert_parsed([[:is, "visit:referrer", ["cnn.com"]]]) - - %{"utm_medium" => "search"} - |> assert_parsed([[:is, "visit:utm_medium", ["search"]]]) - - %{"utm_source" => "bing"} - |> assert_parsed([[:is, "visit:utm_source", ["bing"]]]) - - %{"utm_content" => "content"} - |> assert_parsed([[:is, "visit:utm_content", ["content"]]]) - - %{"utm_term" => "term"} - |> assert_parsed([[:is, "visit:utm_term", ["term"]]]) - - %{"screen" => "Desktop"} - |> assert_parsed([[:is, "visit:screen", ["Desktop"]]]) - - %{"browser" => "Opera"} - |> assert_parsed([[:is, "visit:browser", ["Opera"]]]) - - %{"browser_version" => "10.1"} - |> assert_parsed([[:is, "visit:browser_version", ["10.1"]]]) - - %{"os" => "Linux"} - |> assert_parsed([[:is, "visit:os", ["Linux"]]]) - - %{"os_version" => "13.0"} - |> assert_parsed([[:is, "visit:os_version", ["13.0"]]]) - - %{"country" => "EE"} - |> assert_parsed([[:is, "visit:country", ["EE"]]]) - - %{"region" => "EE-12"} - |> assert_parsed([[:is, "visit:region", ["EE-12"]]]) - - %{"city" => "123"} - |> assert_parsed([[:is, "visit:city", ["123"]]]) - - %{"entry_page" => "/blog"} - |> assert_parsed([[:is, "visit:entry_page", ["/blog"]]]) - - %{"exit_page" => "/blog"} - |> assert_parsed([[:is, "visit:exit_page", ["/blog"]]]) - - %{"props" => %{"cta" => "Top"}} - |> assert_parsed([[:is, "event:props:cta", ["Top"]]]) - - %{"hostname" => "dummy.site"} - |> assert_parsed([[:is, "event:hostname", ["dummy.site"]]]) - end - end - - describe "escaping pipe character" do - test "in simple is filter" do - %{"goal" => ~S(Foo \| Bar)} - |> assert_parsed([[:is, "event:goal", [{:event, "Foo | Bar"}]]]) - end - - test "in member filter" do - %{"page" => ~S(/|\|)} - |> assert_parsed([[:is, "event:page", ["/", "|"]]]) - end - end - - describe "is not filter type" do - test "simple is not filter" do - %{"page" => "!/"} - |> assert_parsed([[:is_not, "event:page", ["/"]]]) - - %{"props" => %{"cta" => "!Top"}} - |> assert_parsed([[:is_not, "event:props:cta", ["Top"]]]) - end - end - - describe "is filter type" do - test "simple is filter" do - %{"page" => "/|/blog"} - |> assert_parsed([[:is, "event:page", ["/", "/blog"]]]) - end - - test "escaping pipe character" do - %{"page" => "/|\\|"} - |> assert_parsed([[:is, "event:page", ["/", "|"]]]) - end - - test "mixed goals" do - %{"goal" => "Signup|Visit /thank-you"} - |> assert_parsed([[:is, "event:goal", [{:event, "Signup"}, {:page, "/thank-you"}]]]) - - %{"goal" => "Visit /thank-you|Signup"} - |> assert_parsed([[:is, "event:goal", [{:page, "/thank-you"}, {:event, "Signup"}]]]) - end - end - - describe "matches filter type" do - test "parses matches filter type" do - %{"page" => "/|/blog**"} - |> assert_parsed([[:matches, "event:page", ["/", "/blog**"]]]) - end - - test "parses not_matches filter type" do - %{"page" => "!/|/blog**"} - |> assert_parsed([[:does_not_match, "event:page", ["/", "/blog**"]]]) - end - - test "single matches" do - %{"page" => "~blog"} - |> assert_parsed([[:matches, "event:page", ["**blog**"]]]) - end - - test "negated matches" do - %{"page" => "!~articles"} - |> assert_parsed([[:does_not_match, "event:page", ["**articles**"]]]) - end - - test "matches member" do - %{"page" => "~articles|blog"} - |> assert_parsed([[:matches, "event:page", ["**articles**", "**blog**"]]]) - end - - test "not matches member" do - %{"page" => "!~articles|blog"} - |> assert_parsed([[:does_not_match, "event:page", ["**articles**", "**blog**"]]]) - end - - test "can be used with `goal` or `page` filters" do - %{"page" => "/blog/post-*"} - |> assert_parsed([[:matches, "event:page", ["/blog/post-*"]]]) - - %{"goal" => "Visit /blog/post-*"} - |> assert_parsed([[:matches, "event:goal", [{:page, "/blog/post-*"}]]]) - end - - test "other filters default to `is` even when wildcard is present" do - %{"country" => "Germa**"} - |> assert_parsed([[:is, "visit:country", ["Germa**"]]]) - end - - test "can be used with `page` filter" do - %{"page" => "!/blog/post-*"} - |> assert_parsed([[:does_not_match, "event:page", ["/blog/post-*"]]]) - end - - test "other filters default to is_not even when wildcard is present" do - %{"country" => "!Germa**"} - |> assert_parsed([[:is_not, "visit:country", ["Germa**"]]]) - end - end - - describe "is_not filter type" do - test "simple is_not filter" do - %{"page" => "!/|/blog"} - |> assert_parsed([[:is_not, "event:page", ["/", "/blog"]]]) - end - - test "mixed goals" do - %{"goal" => "!Signup|Visit /thank-you"} - |> assert_parsed([ - [:is_not, "event:goal", [{:event, "Signup"}, {:page, "/thank-you"}]] - ]) - - %{"goal" => "!Visit /thank-you|Signup"} - |> assert_parsed([ - [:is_not, "event:goal", [{:page, "/thank-you"}, {:event, "Signup"}]] - ]) - end - end - - describe "contains prefix filter type" do - test "can be used with any filter" do - %{"page" => "~/blog/post"} - |> assert_parsed([[:matches, "event:page", ["**/blog/post**"]]]) - - %{"source" => "~facebook"} - |> assert_parsed([[:matches, "visit:source", ["**facebook**"]]]) - end - end -end diff --git a/test/plausible/stats/query_parser_test.exs b/test/plausible/stats/query_parser_test.exs deleted file mode 100644 index 39baead3f344..000000000000 --- a/test/plausible/stats/query_parser_test.exs +++ /dev/null @@ -1,373 +0,0 @@ -defmodule Plausible.Stats.Filters.QueryParserTest do - use ExUnit.Case, async: true - alias Plausible.Stats.Filters - import Plausible.Stats.Filters.QueryParser - - def check_success(params, expected_result) do - assert parse(params) == {:ok, expected_result} - end - - def check_error(params, expected_error_message) do - {:error, message} = parse(params) - assert message =~ expected_error_message - end - - test "parsing empty map fails" do - %{} - |> check_error("No valid metrics passed") - end - - describe "metrics validation" do - test "valid metrics passed" do - %{"metrics" => ["visitors", "events"], "date_range" => "all"} - |> check_success(%{ - metrics: [:visitors, :events], - date_range: "all", - filters: [], - dimensions: [], - order_by: nil - }) - end - - test "invalid metric passed" do - %{"metrics" => ["visitors", "event:name"], "date_range" => "all"} - |> check_error("Unknown metric '\"event:name\"'") - end - - test "fuller list of metrics" do - %{ - "metrics" => [ - "time_on_page", - "conversion_rate", - "visitors", - "pageviews", - "visits", - "events", - "bounce_rate", - "visit_duration" - ], - "date_range" => "all" - } - |> check_success(%{ - metrics: [ - :time_on_page, - :conversion_rate, - :visitors, - :pageviews, - :visits, - :events, - :bounce_rate, - :visit_duration - ], - date_range: "all", - filters: [], - dimensions: [], - order_by: nil - }) - end - - test "same metric queried multiple times" do - %{"metrics" => ["events", "visitors", "visitors"], "date_range" => "all"} - |> check_error(~r/Metrics cannot be queried multiple times/) - end - end - - describe "filters validation" do - for operation <- [:is, :is_not, :matches, :does_not_match] do - test "#{operation} filter" do - %{ - "metrics" => ["visitors"], - "date_range" => "all", - "filters" => [ - [Atom.to_string(unquote(operation)), "event:name", ["foo"]] - ] - } - |> check_success(%{ - metrics: [:visitors], - date_range: "all", - filters: [ - [unquote(operation), "event:name", ["foo"]] - ], - dimensions: [], - order_by: nil - }) - end - - test "#{operation} filter with invalid clause" do - %{ - "metrics" => ["visitors"], - "date_range" => "all", - "filters" => [ - [Atom.to_string(unquote(operation)), "event:name", "foo"] - ] - } - |> check_error(~r/Invalid filter/) - end - end - - test "filtering by invalid operation" do - %{ - "metrics" => ["visitors"], - "date_range" => "all", - "filters" => [ - ["exists?", "event:name", ["foo"]] - ] - } - |> check_error(~r/Unknown operator for filter/) - end - - test "filtering by custom properties" do - %{ - "metrics" => ["visitors"], - "date_range" => "all", - "filters" => [ - ["is", "event:props:foobar", ["value"]] - ] - } - |> check_success(%{ - metrics: [:visitors], - date_range: "all", - filters: [ - [:is, "event:props:foobar", ["value"]] - ], - dimensions: [], - order_by: nil - }) - end - - for dimension <- Filters.event_props() do - if dimension != "goal" do - test "filtering by event:#{dimension} filter" do - %{ - "metrics" => ["visitors"], - "date_range" => "all", - "filters" => [ - ["is", "event:#{unquote(dimension)}", ["foo"]] - ] - } - |> check_success(%{ - metrics: [:visitors], - date_range: "all", - filters: [ - [:is, "event:#{unquote(dimension)}", ["foo"]] - ], - dimensions: [], - order_by: nil - }) - end - end - end - - for dimension <- Filters.visit_props() do - test "filtering by visit:#{dimension} filter" do - %{ - "metrics" => ["visitors"], - "date_range" => "all", - "filters" => [ - ["is", "visit:#{unquote(dimension)}", ["foo"]] - ] - } - |> check_success(%{ - metrics: [:visitors], - date_range: "all", - filters: [ - [:is, "visit:#{unquote(dimension)}", ["foo"]] - ], - dimensions: [], - order_by: nil - }) - end - end - - test "filtering by event:goal" do - %{ - "metrics" => ["visitors"], - "date_range" => "all", - "filters" => [ - ["is", "event:goal", ["Signup", "Visit /thank-you"]] - ] - } - |> check_success(%{ - metrics: [:visitors], - date_range: "all", - filters: [ - [:is, "event:goal", [{:event, "Signup"}, {:page, "/thank-you"}]] - ], - dimensions: [], - order_by: nil - }) - end - - test "invalid event filter" do - %{ - "metrics" => ["visitors"], - "date_range" => "all", - "filters" => [ - ["is", "event:device", ["foo"]] - ] - } - |> check_error(~r/Invalid filter /) - end - - test "invalid visit filter" do - %{ - "metrics" => ["visitors"], - "date_range" => "all", - "filters" => [ - ["is", "visit:name", ["foo"]] - ] - } - |> check_error(~r/Invalid filter /) - end - - test "invalid filter" do - %{ - "metrics" => ["visitors"], - "date_range" => "all", - "filters" => "foobar" - } - |> check_error(~r/Invalid filters passed/) - end - end - - describe "date range validation" do - end - - describe "dimensions validation" do - for dimension <- Filters.event_props() do - test "event:#{dimension} dimension" do - %{ - "metrics" => ["visitors"], - "date_range" => "all", - "dimensions" => ["event:#{unquote(dimension)}"] - } - |> check_success(%{ - metrics: [:visitors], - date_range: "all", - filters: [], - dimensions: ["event:#{unquote(dimension)}"], - order_by: nil - }) - end - end - - for dimension <- Filters.visit_props() do - test "visit:#{dimension} dimension" do - %{ - "metrics" => ["visitors"], - "date_range" => "all", - "dimensions" => ["visit:#{unquote(dimension)}"] - } - |> check_success(%{ - metrics: [:visitors], - date_range: "all", - filters: [], - dimensions: ["visit:#{unquote(dimension)}"], - order_by: nil - }) - end - end - - test "custom properties dimension" do - %{ - "metrics" => ["visitors"], - "date_range" => "all", - "dimensions" => ["event:props:foobar"] - } - |> check_success(%{ - metrics: [:visitors], - date_range: "all", - filters: [], - dimensions: ["event:props:foobar"], - order_by: nil - }) - end - - test "invalid dimension name passed" do - %{ - "metrics" => ["visitors"], - "date_range" => "all", - "dimensions" => ["visitors"] - } - |> check_error(~r/Invalid dimensions/) - end - - test "invalid dimension" do - %{ - "metrics" => ["visitors"], - "date_range" => "all", - "dimensions" => "foobar" - } - |> check_error(~r/Invalid dimensions/) - end - - test "dimensions are not unique" do - %{ - "metrics" => ["visitors"], - "date_range" => "all", - "dimensions" => ["event:name", "event:name"] - } - |> check_error(~r/Some dimensions are listed multiple times/) - end - end - - describe "order_by validation" do - test "ordering by metric" do - %{ - "metrics" => ["visitors", "events"], - "date_range" => "all", - "order_by" => [["events", "desc"], ["visitors", "asc"]] - } - |> check_success(%{ - metrics: [:visitors, :events], - date_range: "all", - filters: [], - dimensions: [], - order_by: [{:events, :desc}, {:visitors, :asc}] - }) - end - - test "ordering by dimension" do - %{ - "metrics" => ["visitors"], - "date_range" => "all", - "dimensions" => ["event:name"], - "order_by" => [["event:name", "desc"]] - } - |> check_success(%{ - metrics: [:visitors], - date_range: "all", - filters: [], - dimensions: ["event:name"], - order_by: [{"event:name", :desc}] - }) - end - - test "ordering by invalid value" do - %{ - "metrics" => ["visitors"], - "date_range" => "all", - "order_by" => [["visssss", "desc"]] - } - |> check_error(~r/Invalid order_by entry/) - end - - test "ordering by not queried metric" do - %{ - "metrics" => ["visitors"], - "date_range" => "all", - "order_by" => [["events", "desc"]] - } - |> check_error(~r/Entry is not a queried metric or dimension/) - end - - test "ordering by not queried dimension" do - %{ - "metrics" => ["visitors"], - "date_range" => "all", - "order_by" => [["event:name", "desc"]] - } - |> check_error(~r/Entry is not a queried metric or dimension/) - end - end -end diff --git a/test/plausible/stats/query_test.exs b/test/plausible/stats/query_test.exs index 62c919319fab..629ea0344974 100644 --- a/test/plausible/stats/query_test.exs +++ b/test/plausible/stats/query_test.exs @@ -194,14 +194,14 @@ defmodule Plausible.Stats.QueryTest do filters = Jason.encode!(%{"goal" => "Signup"}) q = Query.from(site, %{"period" => "6mo", "filters" => filters}) - assert q.filters == [[:is, "event:goal", [{:event, "Signup"}]]] + assert q.filters == [[:is, "event:goal", {:event, "Signup"}]] end test "parses source filter", %{site: site} do filters = Jason.encode!(%{"source" => "Twitter"}) q = Query.from(site, %{"period" => "6mo", "filters" => filters}) - assert q.filters == [[:is, "visit:source", ["Twitter"]]] + assert q.filters == [[:is, "visit:source", "Twitter"]] end end