From bbe92a02a60bcd7874e633da624ec7ba488b6cac Mon Sep 17 00:00:00 2001 From: Karl-Aksel Puulmann Date: Wed, 27 Nov 2024 15:35:19 +0200 Subject: [PATCH 01/15] WIP: Optional modifiers to queries --- lib/plausible/goals/filters.ex | 4 +- .../google/search_console/filters.ex | 9 +- lib/plausible/stats/breakdown.ex | 2 +- lib/plausible/stats/comparisons.ex | 2 +- lib/plausible/stats/email_report.ex | 2 +- lib/plausible/stats/filters/filters.ex | 8 +- .../filters/legacy_dashboard_filter_parser.ex | 25 +++--- lib/plausible/stats/filters/query_parser.ex | 25 ++++-- .../stats/filters/stats_api_filter_parser.ex | 14 +-- lib/plausible/stats/imported/base.ex | 6 +- .../stats/imported/sql/where_builder.ex | 2 +- lib/plausible/stats/query_result.ex | 9 +- lib/plausible/stats/sql/where_builder.ex | 50 +++++------ .../api/external_stats_controller.ex | 2 +- .../controllers/api/stats_controller.ex | 2 +- priv/json-schemas/query-api-schema.json | 20 +++-- .../google/search_console/filters_test.exs | 32 +++---- test/plausible/stats/comparisons_test.exs | 12 +-- test/plausible/stats/filters_test.exs | 30 +++---- .../legacy_dashboard_filter_parser_test.exs | 86 +++++++++--------- test/plausible/stats/query_parser_test.exs | 88 +++++++++++++++---- test/plausible/stats/query_test.exs | 4 +- 22 files changed, 255 insertions(+), 179 deletions(-) diff --git a/lib/plausible/goals/filters.ex b/lib/plausible/goals/filters.ex index 142095e0488e..d4b4bea9af34 100644 --- a/lib/plausible/goals/filters.ex +++ b/lib/plausible/goals/filters.ex @@ -20,7 +20,7 @@ defmodule Plausible.Goals.Filters do * `imported?` - when `true`, builds conditions on the `page` db field rather than `pathname`, and also skips the `e.name == "pageview"` check. """ - def add_filter(query, [operation, "event:goal", clauses], opts \\ []) + def add_filter(query, [operation, "event:goal", clauses, _modifiers], opts \\ []) when operation in [:is, :contains] do imported? = Keyword.get(opts, :imported?, false) @@ -38,7 +38,7 @@ defmodule Plausible.Goals.Filters do goals = Plausible.Goals.for_site(site) Enum.reduce(filters, goals, fn - [operation, "event:goal", clauses], goals -> + [operation, "event:goal", clauses | _rest], goals -> goals_matching_any_clause(goals, operation, clauses) _filter, goals -> diff --git a/lib/plausible/google/search_console/filters.ex b/lib/plausible/google/search_console/filters.ex index d010e013b5d4..5296b34bff93 100644 --- a/lib/plausible/google/search_console/filters.ex +++ b/lib/plausible/google/search_console/filters.ex @@ -24,14 +24,15 @@ 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 + # :TODO: Should also work case-insensitive + defp transform_filter(property, [:is, "visit:entry_page", pages, _modifiers]) 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_wildcard, "visit:entry_page", pages]) + defp transform_filter(property, [:matches_wildcard, "visit:entry_page", pages, _modifiers]) when is_list(pages) do expression = Enum.map_join(pages, "|", fn page -> page_regex(property_url(property, page)) end) @@ -39,12 +40,12 @@ 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 + defp transform_filter(_property, [:is, "visit:screen", devices, _modifiers]) when is_list(devices) do expression = Enum.map_join(devices, "|", &search_console_device/1) %{dimension: "device", operator: "includingRegex", expression: expression} end - defp transform_filter(_property, [:is, "visit:country", countries]) + defp transform_filter(_property, [:is, "visit:country", countries, _modifiers]) 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/breakdown.ex b/lib/plausible/stats/breakdown.ex index 3e0699b605e2..f4b9449e9ce2 100644 --- a/lib/plausible/stats/breakdown.ex +++ b/lib/plausible/stats/breakdown.ex @@ -151,7 +151,7 @@ defmodule Plausible.Stats.Breakdown do @extra_filter_dimensions Map.keys(@filter_dimensions_not) defp dimension_filters(dimension) when dimension in @extra_filter_dimensions do - [[:is_not, dimension, Map.get(@filter_dimensions_not, dimension)]] + [[:is_not, dimension, Map.get(@filter_dimensions_not, dimension), %{}]] end defp dimension_filters(_), do: [] diff --git a/lib/plausible/stats/comparisons.ex b/lib/plausible/stats/comparisons.ex index 647658855bfd..77f557a6eec2 100644 --- a/lib/plausible/stats/comparisons.ex +++ b/lib/plausible/stats/comparisons.ex @@ -87,7 +87,7 @@ defmodule Plausible.Stats.Comparisons do query.dimensions |> Enum.zip(dimension_labels) |> Enum.reject(fn {dimension, _label} -> Time.time_dimension?(dimension) end) - |> Enum.map(fn {dimension, label} -> [:is, dimension, [label]] end) + |> Enum.map(fn {dimension, label} -> [:is, dimension, [label], %{}] end) case query_filters do [] -> [] diff --git a/lib/plausible/stats/email_report.ex b/lib/plausible/stats/email_report.ex index 4f9a403ab8b7..e205bb066dcc 100644 --- a/lib/plausible/stats/email_report.ex +++ b/lib/plausible/stats/email_report.ex @@ -48,7 +48,7 @@ defmodule Plausible.Stats.EmailReport do defp put_top_5_sources(stats, site, query) do query = query - |> Query.add_filter([:is_not, "visit:source", ["Direct / None"]]) + |> Query.add_filter([:is_not, "visit:source", ["Direct / None"], %{}]) |> Query.set(dimensions: ["visit:source"]) sources = Stats.breakdown(site, query, [:visitors], {5, 1}) diff --git a/lib/plausible/stats/filters/filters.ex b/lib/plausible/stats/filters/filters.ex index c68e2b3f8651..14e658a0afdd 100644 --- a/lib/plausible/stats/filters/filters.ex +++ b/lib/plausible/stats/filters/filters.ex @@ -61,10 +61,10 @@ defmodule Plausible.Stats.Filters do ### Examples: iex> Filters.parse("{\\"page\\":\\"/blog/**\\"}") - [[:matches_wildcard, "event:page", ["/blog/**"]]] + [[:matches_wildcard, "event:page", ["/blog/**"], %{}]] iex> Filters.parse("visit:browser!=Chrome") - [[:is_not, "visit:browser", ["Chrome"]]] + [[:is_not, "visit:browser", ["Chrome"], %{}]] iex> Filters.parse(nil) [] @@ -121,8 +121,8 @@ defmodule Plausible.Stats.Filters do def rename_dimensions_used_in_filter(filters, renames) do transform_filters(filters, fn - [operation, dimension, clauses] -> - [[operation, Map.get(renames, dimension, dimension), clauses]] + [operation, dimension, clauses, modifiers] -> + [[operation, Map.get(renames, dimension, dimension), clauses, modifiers]] _subtree -> nil diff --git a/lib/plausible/stats/filters/legacy_dashboard_filter_parser.ex b/lib/plausible/stats/filters/legacy_dashboard_filter_parser.ex index 8e3ef1ccd27c..eae0728983bc 100644 --- a/lib/plausible/stats/filters/legacy_dashboard_filter_parser.ex +++ b/lib/plausible/stats/filters/legacy_dashboard_filter_parser.ex @@ -35,41 +35,40 @@ defmodule Plausible.Stats.Filters.LegacyDashboardFilterParser do cond do is_negated && is_wildcard && is_list -> - [:matches_wildcard_not, key, val] + [:matches_wildcard_not, key, val, %{}] - # TODO is_negated && is_contains && is_list -> - [:matches_wildcard_not, key, Enum.map(val, &"**#{&1}**")] + [:matches_wildcard_not, key, Enum.map(val, &"**#{&1}**"), %{}] is_wildcard && is_list -> - [:matches_wildcard, key, val] + [:matches_wildcard, key, val, %{}] is_negated && is_wildcard -> - [:matches_wildcard_not, key, [val]] + [:matches_wildcard_not, key, [val], %{}] is_negated && is_list -> - [:is_not, key, val] + [:is_not, key, val, %{}] is_negated && is_contains -> - [:matches_wildcard_not, key, ["**" <> val <> "**"]] + [:matches_wildcard_not, key, ["**" <> val <> "**"], %{}] is_negated -> - [:is_not, key, [val]] + [:is_not, key, [val], %{}] is_contains && is_list -> - [:contains, key, val] + [:contains, key, val, %{}] is_list -> - [:is, key, val] + [:is, key, val, %{}] is_contains -> - [:contains, key, [val]] + [:contains, key, [val], %{}] is_wildcard -> - [:matches_wildcard, key, [val]] + [:matches_wildcard, key, [val], %{}] true -> - [:is, key, [val]] + [:is, key, [val], %{}] end end diff --git a/lib/plausible/stats/filters/query_parser.ex b/lib/plausible/stats/filters/query_parser.ex index b7410964b854..a1e97cd9b83e 100644 --- a/lib/plausible/stats/filters/query_parser.ex +++ b/lib/plausible/stats/filters/query_parser.ex @@ -133,31 +133,36 @@ defmodule Plausible.Stats.Filters.QueryParser do :matches_wildcard_not, :contains, :contains_not - ], - do: parse_clauses_list(filter) + ] do + with {:ok, clauses} <- parse_clauses_list(filter), + {:ok, config} <- parse_filter_modifiers(Enum.at(filter, 3)) + do + {:ok, [clauses, config]} + end + end defp parse_filter_rest(operator, _filter) when operator in [:not, :and, :or], do: {:ok, []} - defp parse_clauses_list([operation, filter_key, list] = filter) when is_list(list) do + defp parse_clauses_list([operator, filter_key, list | _rest] = filter) when is_list(list) do all_strings? = Enum.all?(list, &is_binary/1) all_integers? = Enum.all?(list, &is_integer/1) case {filter_key, all_strings?} do {"visit:city", false} when all_integers? -> - {:ok, [list]} + {:ok, list} - {"visit:country", true} when operation in ["is", "is_not"] -> + {"visit:country", true} when operator in ["is", "is_not"] -> if Enum.all?(list, &(String.length(&1) == 2)) do - {:ok, [list]} + {:ok, list} else {:error, "Invalid visit:country filter, visit:country needs to be a valid 2-letter country code."} end {_, true} -> - {:ok, [list]} + {:ok, list} _ -> {:error, "Invalid filter '#{i(filter)}'."} @@ -166,6 +171,10 @@ defmodule Plausible.Stats.Filters.QueryParser do defp parse_clauses_list(filter), do: {:error, "Invalid filter '#{i(filter)}'"} + defp parse_filter_modifiers(config)do + {:ok, atomize_keys(config || %{})} + end + defp parse_date(_site, date_string, _date) when is_binary(date_string) do case Date.from_iso8601(date_string) do {:ok, date} -> {:ok, date} @@ -459,7 +468,7 @@ defmodule Plausible.Stats.Filters.QueryParser do # Note: Only works since event:goal is allowed as a top level filter goal_filter_clauses = Enum.flat_map(query.filters, fn - [:is, "event:goal", clauses] -> clauses + [:is, "event:goal", clauses | _rest] -> clauses _ -> [] end) diff --git a/lib/plausible/stats/filters/stats_api_filter_parser.ex b/lib/plausible/stats/filters/stats_api_filter_parser.ex index f7168a674194..43f693700e57 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? -> [:matches_wildcard_not, key, [raw_value]] - is_wildcard? -> [:matches_wildcard, 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? -> [:matches_wildcard_not, key, [raw_value], %{}] + is_wildcard? -> [:matches_wildcard, key, [raw_value], %{}] + is_list? -> [:is, key, parse_member_list(raw_value), %{}] + is_negated? -> [:is_not, key, [final_value], %{}] + true -> [:is, key, [final_value], %{}] end |> reject_invalid_country_codes() @@ -40,7 +40,7 @@ defmodule Plausible.Stats.Filters.StatsAPIFilterParser do end end - defp reject_invalid_country_codes([_op, "visit:country", code_or_codes] = filter) do + defp reject_invalid_country_codes([_op, "visit:country", code_or_codes, _modifier] = filter) do code_or_codes |> List.wrap() |> Enum.reduce_while(filter, fn @@ -68,6 +68,6 @@ defmodule Plausible.Stats.Filters.StatsAPIFilterParser do remove_escape_chars(value) end - [:is, key, List.wrap(value)] + [:is, key, List.wrap(value), %{}] end end diff --git a/lib/plausible/stats/imported/base.ex b/lib/plausible/stats/imported/base.ex index 831314e6dcb8..bcb441987395 100644 --- a/lib/plausible/stats/imported/base.ex +++ b/lib/plausible/stats/imported/base.ex @@ -117,8 +117,8 @@ defmodule Plausible.Stats.Imported.Base do has_required_name_filter? = query.filters |> Enum.flat_map(fn - [:is, "event:name", names] -> names - [:is, "event:goal", names] -> names + [:is, "event:name", names, _] -> names + [:is, "event:goal", names, _] -> names _ -> [] end) |> Enum.any?(&(&1 in special_goals_for(property))) @@ -196,7 +196,7 @@ defmodule Plausible.Stats.Imported.Base do defp get_filter_goals(query) do query.filters |> Enum.filter(fn [_, dimension | _rest] -> dimension == "event:goal" end) - |> Enum.flat_map(fn [operation, _dimension, clauses] -> + |> Enum.flat_map(fn [operation, _dimension, clauses, _modifiers] -> Enum.flat_map(clauses, fn clause -> query.preloaded_goals |> Plausible.Goals.Filters.filter_preloaded(operation, clause) diff --git a/lib/plausible/stats/imported/sql/where_builder.ex b/lib/plausible/stats/imported/sql/where_builder.ex index 638fdf270c2c..93d847621032 100644 --- a/lib/plausible/stats/imported/sql/where_builder.ex +++ b/lib/plausible/stats/imported/sql/where_builder.ex @@ -49,7 +49,7 @@ defmodule Plausible.Stats.Imported.SQL.WhereBuilder do |> Enum.reduce(fn condition, acc -> dynamic([], ^acc or ^condition) end) end - defp add_filter(query, [_operation, dimension, _clauses] = filter) do + defp add_filter(query, [_operation, dimension, _clauses | _rest] = filter) do db_field = Plausible.Stats.Filters.without_prefix(dimension) if db_field == :goal do diff --git a/lib/plausible/stats/query_result.ex b/lib/plausible/stats/query_result.ex index 82a21287315f..7db4dca6a172 100644 --- a/lib/plausible/stats/query_result.ex +++ b/lib/plausible/stats/query_result.ex @@ -31,7 +31,7 @@ defmodule Plausible.Stats.QueryResult do to_iso8601(query.utc_time_range.first, query.timezone), to_iso8601(query.utc_time_range.last, query.timezone) ], - filters: query.filters, + filters: query.filters |> remove_empty_modifiers(), dimensions: query.dimensions, order_by: query.order_by |> Enum.map(&Tuple.to_list/1), include: include(query) |> Map.filter(fn {_key, val} -> val end), @@ -85,6 +85,13 @@ defmodule Plausible.Stats.QueryResult do |> DateTime.shift_zone!(timezone) |> DateTime.to_iso8601(:extended) end + + defp remove_empty_modifiers(filters) do + Plausible.Stats.Filters.transform_filters(filters, fn + [operation, dimension, clauses, %{}] -> [operation, dimension, clauses] + _ -> nil + end) + end end defimpl Jason.Encoder, for: Plausible.Stats.QueryResult do diff --git a/lib/plausible/stats/sql/where_builder.ex b/lib/plausible/stats/sql/where_builder.ex index 2463a889e230..7bc6824d7885 100644 --- a/lib/plausible/stats/sql/where_builder.ex +++ b/lib/plausible/stats/sql/where_builder.ex @@ -93,11 +93,11 @@ defmodule Plausible.Stats.SQL.WhereBuilder do |> Enum.reduce(fn condition, acc -> dynamic([], ^acc or ^condition) end) end - defp add_filter(:events, _query, [:is, "event:name", list]) do - dynamic([e], e.name in ^list) + defp add_filter(:events, _query, [:is, "event:name", clauses, _modifiers]) do + dynamic([e], e.name in ^clauses) end - defp add_filter(:events, query, [_, "event:goal", _] = filter) do + defp add_filter(:events, query, [_, "event:goal" | _rest] = filter) do Plausible.Goals.Filters.add_filter(query, filter) end @@ -154,7 +154,7 @@ defmodule Plausible.Stats.SQL.WhereBuilder do false end - defp filter_custom_prop(prop_name, column_name, [:is, _, clauses]) do + defp filter_custom_prop(prop_name, column_name, [:is, _, clauses, _modifiers]) do none_value_included = Enum.member?(clauses, "(none)") dynamic( @@ -164,7 +164,7 @@ defmodule Plausible.Stats.SQL.WhereBuilder do ) end - defp filter_custom_prop(prop_name, column_name, [:is_not, _, clauses]) do + defp filter_custom_prop(prop_name, column_name, [:is_not, _, clauses, _modifiers]) do none_value_included = Enum.member?(clauses, "(none)") dynamic( @@ -178,19 +178,19 @@ defmodule Plausible.Stats.SQL.WhereBuilder do ) end - defp filter_custom_prop(prop_name, column_name, [:matches_wildcard, dimension, clauses]) do + defp filter_custom_prop(prop_name, column_name, [:matches_wildcard, dimension, clauses, modifiers]) do regexes = Enum.map(clauses, &page_regex/1) - filter_custom_prop(prop_name, column_name, [:matches, dimension, regexes]) + filter_custom_prop(prop_name, column_name, [:matches, dimension, regexes, modifiers]) end - defp filter_custom_prop(prop_name, column_name, [:matches_wildcard_not, dimension, clauses]) do + defp filter_custom_prop(prop_name, column_name, [:matches_wildcard_not, dimension, clauses, modifiers]) do regexes = Enum.map(clauses, &page_regex/1) - filter_custom_prop(prop_name, column_name, [:matches_not, dimension, regexes]) + filter_custom_prop(prop_name, column_name, [:matches_not, dimension, regexes, modifiers]) end - defp filter_custom_prop(prop_name, column_name, [:matches, _dimension, clauses]) do + defp filter_custom_prop(prop_name, column_name, [:matches, _dimension, clauses, _modifiers]) do dynamic( [t], has_key(t, column_name, ^prop_name) and @@ -202,7 +202,7 @@ defmodule Plausible.Stats.SQL.WhereBuilder do ) end - defp filter_custom_prop(prop_name, column_name, [:matches_not, _dimension, clauses]) do + defp filter_custom_prop(prop_name, column_name, [:matches_not, _dimension, clauses, _modifiers]) do dynamic( [t], has_key(t, column_name, ^prop_name) and @@ -214,7 +214,7 @@ defmodule Plausible.Stats.SQL.WhereBuilder do ) end - defp filter_custom_prop(prop_name, column_name, [:contains, _dimension, clauses]) do + defp filter_custom_prop(prop_name, column_name, [:contains, _dimension, clauses, _modifiers]) do dynamic( [t], has_key(t, column_name, ^prop_name) and @@ -226,7 +226,7 @@ defmodule Plausible.Stats.SQL.WhereBuilder do ) end - defp filter_custom_prop(prop_name, column_name, [:contains_not, _dimension, clauses]) do + defp filter_custom_prop(prop_name, column_name, [:contains_not, _dimension, clauses, _modifiers]) do dynamic( [t], has_key(t, column_name, ^prop_name) and @@ -238,7 +238,7 @@ defmodule Plausible.Stats.SQL.WhereBuilder do ) end - defp filter_field(db_field, [:matches_wildcard, _dimension, glob_exprs]) do + defp filter_field(db_field, [:matches_wildcard, _dimension, glob_exprs, _modifiers]) do page_regexes = Enum.map(glob_exprs, &page_regex/1) dynamic( @@ -247,36 +247,36 @@ defmodule Plausible.Stats.SQL.WhereBuilder do ) end - defp filter_field(db_field, [:matches_wildcard_not, dimension, clauses]) do - dynamic([], not (^filter_field(db_field, [:matches_wildcard, dimension, clauses]))) + defp filter_field(db_field, [:matches_wildcard_not, dimension, clauses, modifiers]) do + dynamic([], not (^filter_field(db_field, [:matches_wildcard, dimension, clauses, modifiers]))) end - defp filter_field(db_field, [:contains, _dimension, values]) do + defp filter_field(db_field, [:contains, _dimension, values, _modifiers]) do dynamic([x], fragment("multiSearchAny(?, ?)", type(field(x, ^db_field), :string), ^values)) end - defp filter_field(db_field, [:contains_not, dimension, clauses]) do - dynamic([], not (^filter_field(db_field, [:contains, dimension, clauses]))) + defp filter_field(db_field, [:contains_not, dimension, clauses, modifiers]) do + dynamic([], not (^filter_field(db_field, [:contains, dimension, clauses, modifiers]))) end - defp filter_field(db_field, [:matches, _dimension, clauses]) do + defp filter_field(db_field, [:matches, _dimension, clauses, _modifiers]) do dynamic( [x], fragment("multiMatchAny(?, ?)", type(field(x, ^db_field), :string), ^clauses) ) end - defp filter_field(db_field, [:matches_not, dimension, clauses]) do - dynamic([], not (^filter_field(db_field, [:matches, dimension, clauses]))) + defp filter_field(db_field, [:matches_not, dimension, clauses, modifiers]) do + dynamic([], not (^filter_field(db_field, [:matches, dimension, clauses, modifiers]))) end - defp filter_field(db_field, [:is, _dimension, clauses]) do + defp filter_field(db_field, [:is, _dimension, clauses, _modifiers]) do list = Enum.map(clauses, &db_field_val(db_field, &1)) dynamic([x], field(x, ^db_field) in ^list) end - defp filter_field(db_field, [:is_not, dimension, clauses]) do - dynamic([], not (^filter_field(db_field, [:is, dimension, clauses]))) + defp filter_field(db_field, [:is_not, dimension, clauses, modifiers]) do + dynamic([], not (^filter_field(db_field, [:is, dimension, clauses, modifiers]))) end @no_ref "Direct / None" diff --git a/lib/plausible_web/controllers/api/external_stats_controller.ex b/lib/plausible_web/controllers/api/external_stats_controller.ex index 0adebbd506d0..121206b8d184 100644 --- a/lib/plausible_web/controllers/api/external_stats_controller.ex +++ b/lib/plausible_web/controllers/api/external_stats_controller.ex @@ -351,7 +351,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController do end) end - defp validate_filter(site, [_type, "event:goal", goal_filter]) do + defp validate_filter(site, [_type, "event:goal", goal_filter, _modifiers]) do configured_goals = site |> Plausible.Goals.for_site() diff --git a/lib/plausible_web/controllers/api/stats_controller.ex b/lib/plausible_web/controllers/api/stats_controller.ex index 89b73d5caed6..88d4819725fc 100644 --- a/lib/plausible_web/controllers/api/stats_controller.ex +++ b/lib/plausible_web/controllers/api/stats_controller.ex @@ -951,7 +951,7 @@ defmodule PlausibleWeb.Api.StatsController do query |> struct!(order_by: []) |> Query.remove_top_level_filters(["visit:exit_page"]) - |> Query.add_filter([:is, "event:page", pages]) + |> Query.add_filter([:is, "event:page", pages, %{}]) |> Query.set(dimensions: ["event:page"]) total_pageviews = diff --git a/priv/json-schemas/query-api-schema.json b/priv/json-schemas/query-api-schema.json index e715c5957184..cdcc3bf03c8c 100644 --- a/priv/json-schemas/query-api-schema.json +++ b/priv/json-schemas/query-api-schema.json @@ -346,7 +346,7 @@ "enum": ["is_not", "contains_not", "matches", "matches_not"], "description": "filter operation" }, - "filter_operation_with_goals": { + "filter_operation_contains": { "type": "string", "enum": ["is", "contains"], "description": "filter operation" @@ -375,14 +375,14 @@ { "$ref": "#/definitions/clauses" } ] }, - "filter_with_goals": { + "filter_case_sensitive": { "type": "array", "additionalItems": false, "minItems": 3, - "maxItems": 3, + "maxItems": 4, "items": [ { - "$ref": "#/definitions/filter_operation_with_goals" + "$ref": "#/definitions/filter_operation_contains" }, { "oneOf": [ @@ -393,13 +393,23 @@ }, { "$ref": "#/definitions/clauses" + }, + { + "type": "object", + "additionalProperties": false, + "properties": { + "case_sensitive": { + "type": "boolean", + "default": true + } + } } ] }, "filter_entry": { "oneOf": [ { "$ref": "#/definitions/filter_without_goals" }, - { "$ref": "#/definitions/filter_with_goals" } + { "$ref": "#/definitions/filter_case_sensitive" } ] }, "filter_tree": { diff --git a/test/plausible/google/search_console/filters_test.exs b/test/plausible/google/search_console/filters_test.exs index d056dfb2e670..a37c60f06ff9 100644 --- a/test/plausible/google/search_console/filters_test.exs +++ b/test/plausible/google/search_console/filters_test.exs @@ -4,7 +4,7 @@ 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, "") @@ -24,7 +24,7 @@ defmodule Plausible.Google.SearchConsole.FiltersTest do test "transforms matches_wildcard page filter" do filters = [ - [:matches_wildcard, "visit:entry_page", ["*page*"]] + [:matches_wildcard, "visit:entry_page", ["*page*"], %{}] ] {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters, "") @@ -44,7 +44,7 @@ defmodule Plausible.Google.SearchConsole.FiltersTest do test "transforms is page filter" do filters = [ - [:is, "visit:entry_page", ["/pageA", "/pageB"]] + [:is, "visit:entry_page", ["/pageA", "/pageB"], %{}] ] {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters, "") @@ -64,7 +64,7 @@ defmodule Plausible.Google.SearchConsole.FiltersTest do test "transforms matches multiple page filter" do filters = [ - [:matches_wildcard, "visit:entry_page", ["/pageA*", "/pageB*"]] + [:matches_wildcard, "visit:entry_page", ["/pageA*", "/pageB*"], %{}] ] {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters, "") @@ -84,7 +84,7 @@ defmodule Plausible.Google.SearchConsole.FiltersTest do test "transforms event:page exactly like visit:entry_page" do filters = [ - [:matches_wildcard, "event:page", ["/pageA*", "/pageB*"]] + [:matches_wildcard, "event:page", ["/pageA*", "/pageB*"], %{}] ] {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters, "") @@ -104,7 +104,7 @@ 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, "") @@ -120,7 +120,7 @@ defmodule Plausible.Google.SearchConsole.FiltersTest do test "transforms is visit:screen filter" do filters = [ - [:is, "visit:screen", ["Mobile", "Tablet"]] + [:is, "visit:screen", ["Mobile", "Tablet"], %{}] ] {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters, "") @@ -136,7 +136,7 @@ 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, "") @@ -148,7 +148,7 @@ defmodule Plausible.Google.SearchConsole.FiltersTest do test "transforms member visit:country filter" do filters = [ - [:is, "visit:country", ["EE", "PL"]] + [:is, "visit:country", ["EE", "PL"], %{}] ] {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters, "") @@ -164,9 +164,9 @@ defmodule Plausible.Google.SearchConsole.FiltersTest do test "filters can be combined" do filters = [ - [:is, "visit:country", ["EE", "PL"]], - [:matches_wildcard, "visit:entry_page", ["*web-analytics*"]], - [:is, "visit:screen", ["Desktop"]] + [:is, "visit:country", ["EE", "PL"], %{}], + [:matches_wildcard, "visit:entry_page", ["*web-analytics*"], %{}], + [:is, "visit:screen", ["Desktop"], %{}] ] {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters, "") @@ -188,10 +188,10 @@ defmodule Plausible.Google.SearchConsole.FiltersTest do test "when unsupported filter is included the whole set becomes invalid" do filters = [ - [:matches_wildcard, "visit:entry_page", "*web-analytics*"], - [:is, "visit:screen", "Desktop"], - [:member, "visit:country", ["EE", "PL"]], - [:is, "visit:utm_medium", "facebook"] + [:matches_wildcard, "visit:entry_page", "*web-analytics*", %{}], + [:is, "visit:screen", "Desktop", %{}], + [:member, "visit:country", ["EE", "PL"], %{}], + [:is, "visit:utm_medium", "facebook", %{}] ] assert :unsupported_filters = Filters.transform("sc-domain:plausible.io", filters, "") diff --git a/test/plausible/stats/comparisons_test.exs b/test/plausible/stats/comparisons_test.exs index f291cfd6b071..f04356727a15 100644 --- a/test/plausible/stats/comparisons_test.exs +++ b/test/plausible/stats/comparisons_test.exs @@ -298,7 +298,7 @@ defmodule Plausible.Stats.ComparisonsTest do Comparisons.add_comparison_filters(query, [%{dimensions: ["Chrome"], metrics: [123]}]) assert result_query.filters == [ - [:ignore_in_totals_query, [:is, "visit:browser", ["Chrome"]]] + [:ignore_in_totals_query, [:is, "visit:browser", ["Chrome"], %{}]] ] end @@ -323,7 +323,7 @@ defmodule Plausible.Stats.ComparisonsTest do result_query = Comparisons.add_comparison_filters(query, main_query_results) assert result_query.filters == [ - [:is, "visit:country_name", ["Estonia"]], + [:is, "visit:country_name", ["Estonia"], %{}], [ :ignore_in_totals_query, [ @@ -332,15 +332,15 @@ defmodule Plausible.Stats.ComparisonsTest do [ :and, [ - [:is, "visit:browser", ["Chrome"]], - [:is, "visit:browser_version", ["99.9"]] + [:is, "visit:browser", ["Chrome"], %{}], + [:is, "visit:browser_version", ["99.9"], %{}] ] ], [ :and, [ - [:is, "visit:browser", ["Firefox"]], - [:is, "visit:browser_version", ["12.0"]] + [:is, "visit:browser", ["Firefox"], %{}], + [:is, "visit:browser_version", ["12.0"], %{}] ] ] ] diff --git a/test/plausible/stats/filters_test.exs b/test/plausible/stats/filters_test.exs index c239bfa156d0..ec7ee04a3b37 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_wildcard, "event:page", ["/blog/post-*"]]]) + |> assert_parsed([[:matches_wildcard, "event:page", ["/blog/post-*"], %{}]]) end test "negative wildcard" do "event:page!=/blog/post-*" - |> assert_parsed([[:matches_wildcard_not, "event:page", ["/blog/post-*"]]]) + |> assert_parsed([[:matches_wildcard_not, "event:page", ["/blog/post-*"], %{}]]) end test "custom event goal" do "event:goal==Signup" - |> assert_parsed([[:is, "event:goal", ["Signup"]]]) + |> assert_parsed([[:is, "event:goal", ["Signup"], %{}]]) end test "pageview goal" do "event:goal==Visit /blog" - |> assert_parsed([[:is, "event:goal", ["Visit /blog"]]]) + |> assert_parsed([[:is, "event:goal", ["Visit /blog"], %{}]]) end test "is" do "visit:country==FR|GB|DE" - |> assert_parsed([[:is, "visit:country", ["FR", "GB", "DE"]]]) + |> assert_parsed([[:is, "visit:country", ["FR", "GB", "DE"], %{}]]) end test "member + wildcard" do "event:page==/blog**|/newsletter|/*/" - |> assert_parsed([[:matches_wildcard, "event:page", ["/blog**|/newsletter|/*/"]]]) + |> assert_parsed([[:matches_wildcard, "event:page", ["/blog**|/newsletter|/*/"], %{}]]) end test "combined with \";\"" do "event:page==/blog**|/newsletter|/*/ ; visit:country==FR|GB|DE" |> assert_parsed([ - [:matches_wildcard, "event:page", ["/blog**|/newsletter|/*/"]], - [:is, "visit:country", ["FR", "GB", "DE"]] + [:matches_wildcard, "event:page", ["/blog**|/newsletter|/*/"], %{}], + [:is, "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 "utm_campaign==campaign \\| 1|campaign \\| 2" - |> assert_parsed([[:is, "utm_campaign", ["campaign | 1", "campaign | 2"]]]) + |> assert_parsed([[:is, "utm_campaign", ["campaign | 1", "campaign | 2"], %{}]]) end test "keeps escape characters in is + wildcard filter" do "event:page==/**\\|page|/other/page" - |> assert_parsed([[:matches_wildcard, "event:page", ["/**\\|page|/other/page"]]]) + |> assert_parsed([[:matches_wildcard, "event:page", ["/**\\|page|/other/page"], %{}]]) end test "gracefully fails to parse garbage" do @@ -103,7 +103,7 @@ defmodule Plausible.Stats.FiltersTest do test "simple" do [["is", "event:name", ["pageview"]]] |> Jason.encode!() - |> assert_parsed([[:is, "event:name", ["pageview"]]]) + |> 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 index 89002036f848..e06e2b120b8c 100644 --- a/test/plausible/stats/legacy_dashboard_filter_parser_test.exs +++ b/test/plausible/stats/legacy_dashboard_filter_parser_test.exs @@ -9,174 +9,174 @@ defmodule Plausible.Stats.Legacy.DashboardFilterParserTest do describe "adding prefix" do test "adds appropriate prefix to filter" do %{"page" => "/"} - |> assert_parsed([[:is, "event:page", ["/"]]]) + |> assert_parsed([[:is, "event:page", ["/"], %{}]]) %{"goal" => "Signup"} - |> assert_parsed([[:is, "event:goal", ["Signup"]]]) + |> assert_parsed([[:is, "event:goal", ["Signup"], %{}]]) %{"goal" => "Visit /blog"} - |> assert_parsed([[:is, "event:goal", ["Visit /blog"]]]) + |> assert_parsed([[:is, "event:goal", ["Visit /blog"], %{}]]) %{"source" => "Google"} - |> assert_parsed([[:is, "visit:source", ["Google"]]]) + |> assert_parsed([[:is, "visit:source", ["Google"], %{}]]) %{"referrer" => "cnn.com"} - |> assert_parsed([[:is, "visit:referrer", ["cnn.com"]]]) + |> assert_parsed([[:is, "visit:referrer", ["cnn.com"], %{}]]) %{"utm_medium" => "search"} - |> assert_parsed([[:is, "visit:utm_medium", ["search"]]]) + |> assert_parsed([[:is, "visit:utm_medium", ["search"], %{}]]) %{"utm_source" => "bing"} - |> assert_parsed([[:is, "visit:utm_source", ["bing"]]]) + |> assert_parsed([[:is, "visit:utm_source", ["bing"], %{}]]) %{"utm_content" => "content"} - |> assert_parsed([[:is, "visit:utm_content", ["content"]]]) + |> assert_parsed([[:is, "visit:utm_content", ["content"], %{}]]) %{"utm_term" => "term"} - |> assert_parsed([[:is, "visit:utm_term", ["term"]]]) + |> assert_parsed([[:is, "visit:utm_term", ["term"], %{}]]) %{"screen" => "Desktop"} - |> assert_parsed([[:is, "visit:screen", ["Desktop"]]]) + |> assert_parsed([[:is, "visit:screen", ["Desktop"], %{}]]) %{"browser" => "Opera"} - |> assert_parsed([[:is, "visit:browser", ["Opera"]]]) + |> assert_parsed([[:is, "visit:browser", ["Opera"], %{}]]) %{"browser_version" => "10.1"} - |> assert_parsed([[:is, "visit:browser_version", ["10.1"]]]) + |> assert_parsed([[:is, "visit:browser_version", ["10.1"], %{}]]) %{"os" => "Linux"} - |> assert_parsed([[:is, "visit:os", ["Linux"]]]) + |> assert_parsed([[:is, "visit:os", ["Linux"], %{}]]) %{"os_version" => "13.0"} - |> assert_parsed([[:is, "visit:os_version", ["13.0"]]]) + |> assert_parsed([[:is, "visit:os_version", ["13.0"], %{}]]) %{"country" => "EE"} - |> assert_parsed([[:is, "visit:country", ["EE"]]]) + |> assert_parsed([[:is, "visit:country", ["EE"], %{}]]) %{"region" => "EE-12"} - |> assert_parsed([[:is, "visit:region", ["EE-12"]]]) + |> assert_parsed([[:is, "visit:region", ["EE-12"], %{}]]) %{"city" => "123"} - |> assert_parsed([[:is, "visit:city", ["123"]]]) + |> assert_parsed([[:is, "visit:city", ["123"], %{}]]) %{"entry_page" => "/blog"} - |> assert_parsed([[:is, "visit:entry_page", ["/blog"]]]) + |> assert_parsed([[:is, "visit:entry_page", ["/blog"], %{}]]) %{"exit_page" => "/blog"} - |> assert_parsed([[:is, "visit:exit_page", ["/blog"]]]) + |> assert_parsed([[:is, "visit:exit_page", ["/blog"], %{}]]) %{"props" => %{"cta" => "Top"}} - |> assert_parsed([[:is, "event:props:cta", ["Top"]]]) + |> assert_parsed([[:is, "event:props:cta", ["Top"], %{}]]) %{"hostname" => "dummy.site"} - |> assert_parsed([[:is, "event: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", ["Foo | Bar"]]]) + |> assert_parsed([[:is, "event:goal", ["Foo | Bar"], %{}]]) end test "in member filter" do %{"page" => ~S(/|\|)} - |> assert_parsed([[:is, "event:page", ["/", "|"]]]) + |> 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", ["/"]]]) + |> assert_parsed([[:is_not, "event:page", ["/"], %{}]]) %{"props" => %{"cta" => "!Top"}} - |> assert_parsed([[:is_not, "event: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"]]]) + |> assert_parsed([[:is, "event:page", ["/", "/blog"], %{}]]) end test "escaping pipe character" do %{"page" => "/|\\|"} - |> assert_parsed([[:is, "event:page", ["/", "|"]]]) + |> assert_parsed([[:is, "event:page", ["/", "|"], %{}]]) end test "mixed goals" do %{"goal" => "Signup|Visit /thank-you"} - |> assert_parsed([[:is, "event:goal", ["Signup", "Visit /thank-you"]]]) + |> assert_parsed([[:is, "event:goal", ["Signup", "Visit /thank-you"], %{}]]) %{"goal" => "Visit /thank-you|Signup"} - |> assert_parsed([[:is, "event:goal", ["Visit /thank-you", "Signup"]]]) + |> assert_parsed([[:is, "event:goal", ["Visit /thank-you", "Signup"], %{}]]) end end describe "matches_wildcard filter type" do test "parses matches filter type" do %{"page" => "/|/blog**"} - |> assert_parsed([[:matches_wildcard, "event:page", ["/", "/blog**"]]]) + |> assert_parsed([[:matches_wildcard, "event:page", ["/", "/blog**"], %{}]]) end test "parses not_matches filter type" do %{"page" => "!/|/blog**"} - |> assert_parsed([[:matches_wildcard_not, "event:page", ["/", "/blog**"]]]) + |> assert_parsed([[:matches_wildcard_not, "event:page", ["/", "/blog**"], %{}]]) end test "single matches" do %{"page" => "~blog"} - |> assert_parsed([[:contains, "event:page", ["blog"]]]) + |> assert_parsed([[:contains, "event:page", ["blog"], %{}]]) end test "negated matches" do %{"page" => "!~articles"} - |> assert_parsed([[:matches_wildcard_not, "event:page", ["**articles**"]]]) + |> assert_parsed([[:matches_wildcard_not, "event:page", ["**articles**"], %{}]]) end test "matches member" do %{"page" => "~articles|blog"} - |> assert_parsed([[:contains, "event:page", ["articles", "blog"]]]) + |> assert_parsed([[:contains, "event:page", ["articles", "blog"], %{}]]) end test "not matches member" do %{"page" => "!~articles|blog"} - |> assert_parsed([[:matches_wildcard_not, "event:page", ["**articles**", "**blog**"]]]) + |> assert_parsed([[:matches_wildcard_not, "event:page", ["**articles**", "**blog**"], %{}]]) end test "other filters default to `is` even when wildcard is present" do %{"country" => "Germa**"} - |> assert_parsed([[:is, "visit:country", ["Germa**"]]]) + |> assert_parsed([[:is, "visit:country", ["Germa**"], %{}]]) end test "can be used with `page` filter" do %{"page" => "!/blog/post-*"} - |> assert_parsed([[:matches_wildcard_not, "event:page", ["/blog/post-*"]]]) + |> assert_parsed([[:matches_wildcard_not, "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**"]]]) + |> 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"]]]) + |> assert_parsed([[:is_not, "event:page", ["/", "/blog"], %{}]]) end test "mixed goals" do %{"goal" => "!Signup|Visit /thank-you"} |> assert_parsed([ - [:is_not, "event:goal", ["Signup", "Visit /thank-you"]] + [:is_not, "event:goal", ["Signup", "Visit /thank-you"], %{}] ]) %{"goal" => "!Visit /thank-you|Signup"} |> assert_parsed([ - [:is_not, "event:goal", ["Visit /thank-you", "Signup"]] + [:is_not, "event:goal", ["Visit /thank-you", "Signup"], %{}] ]) end end @@ -184,10 +184,10 @@ defmodule Plausible.Stats.Legacy.DashboardFilterParserTest do describe "contains prefix filter type" do test "can be used with any filter" do %{"page" => "~/blog/post"} - |> assert_parsed([[:contains, "event:page", ["/blog/post"]]]) + |> assert_parsed([[:contains, "event:page", ["/blog/post"], %{}]]) %{"source" => "~facebook"} - |> assert_parsed([[:contains, "visit:source", ["facebook"]]]) + |> assert_parsed([[:contains, "visit:source", ["facebook"], %{}]]) end end end diff --git a/test/plausible/stats/query_parser_test.exs b/test/plausible/stats/query_parser_test.exs index 53e998d23a99..9a40a189b423 100644 --- a/test/plausible/stats/query_parser_test.exs +++ b/test/plausible/stats/query_parser_test.exs @@ -208,7 +208,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do metrics: [:visitors], utc_time_range: @date_range_day, filters: [ - [unquote(operation), "event:name", ["foo"]] + [unquote(operation), "event:name", ["foo"], %{}] ], dimensions: [], order_by: nil, @@ -333,7 +333,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do metrics: [:visitors], utc_time_range: @date_range_day, filters: [ - [:is, "event:props:foobar", ["value"]] + [:is, "event:props:foobar", ["value"], %{}] ], dimensions: [], order_by: nil, @@ -358,7 +358,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do metrics: [:visitors], utc_time_range: @date_range_day, filters: [ - [:is, "event:#{unquote(dimension)}", ["foo"]] + [:is, "event:#{unquote(dimension)}", ["foo"], %{}] ], dimensions: [], order_by: nil, @@ -384,7 +384,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do metrics: [:visitors], utc_time_range: @date_range_day, filters: [ - [:is, "visit:#{unquote(dimension)}", ["ab"]] + [:is, "visit:#{unquote(dimension)}", ["ab"], %{}] ], dimensions: [], order_by: nil, @@ -450,7 +450,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do metrics: [:visitors], utc_time_range: @date_range_day, filters: [ - [:is, "visit:city", [123, 456]] + [:is, "visit:city", [123, 456], %{}] ], dimensions: [], order_by: nil, @@ -469,7 +469,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do metrics: [:visitors], utc_time_range: @date_range_day, filters: [ - [:is, "visit:city", ["123", "456"]] + [:is, "visit:city", ["123", "456"], %{}] ], dimensions: [], order_by: nil, @@ -523,11 +523,11 @@ defmodule Plausible.Stats.Filters.QueryParserTest do [ :and, [ - [:is, "visit:city_name", ["Tallinn"]], - [:is, "visit:country_name", ["Estonia"]] + [:is, "visit:city_name", ["Tallinn"], %{}], + [:is, "visit:country_name", ["Estonia"], %{}] ] ], - [:not, [:is, "visit:country_name", ["Estonia"]]] + [:not, [:is, "visit:country_name", ["Estonia"], %{}]] ] ] ], @@ -576,7 +576,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do metrics: [:visitors], utc_time_range: @date_range_day, filters: [ - [:is, "event:hostname", ["a.plausible.io"]] + [:is, "event:hostname", ["a.plausible.io"], %{}] ], dimensions: [], order_by: nil, @@ -598,6 +598,56 @@ defmodule Plausible.Stats.Filters.QueryParserTest do "Invalid filters. Dimension `event:hostname` can only be filtered at the top level." ) end + + for operation <- [:is, :contains] do + test "#{operation} allows case_sensitive modifier", %{site: site} do + %{ + "site_id" => site.domain, + "metrics" => ["visitors"], + "date_range" => "all", + "filters" => [ + [ + Atom.to_string(unquote(operation)), + "event:page", + ["/foo"], + %{"case_sensitive" => false} + ], + [ + Atom.to_string(unquote(operation)), + "event:name", + ["/foo"], + %{"case_sensitive" => true} + ] + ] + } + |> check_success(site, %{ + metrics: [:visitors], + utc_time_range: @date_range_day, + filters: [ + [unquote(operation), "event:page", ["/foo"], %{case_sensitive: false}], + [unquote(operation), "event:name", ["/foo"], %{case_sensitive: true}] + ], + dimensions: [], + order_by: nil, + timezone: site.timezone, + include: %{imports: false, time_labels: false, total_rows: false, comparisons: nil}, + pagination: %{limit: 10_000, offset: 0} + }) + end + end + + test "case_sensitive modifier is not valid for other operations", %{site: site} do + %{ + "site_id" => site.domain, + "metrics" => ["visitors"], + "date_range" => "all", + "filters" => [["is_not", "event:hostname", ["a.plausible.io"], %{"case_sensitive" => false}]] + } + |> check_error( + site, + "#/filters/0: Invalid filter [\"is_not\", \"event:hostname\", [\"a.plausible.io\"], %{\"case_sensitive\" => false}]" + ) + end end describe "include validation" do @@ -846,7 +896,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do metrics: [:visitors], utc_time_range: @date_range_day, filters: [ - [:is, "event:goal", ["Signup", "Visit /thank-you"]] + [:is, "event:goal", ["Signup", "Visit /thank-you"], %{}] ], dimensions: [], order_by: nil, @@ -1346,7 +1396,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do |> check_success(site, %{ metrics: [:conversion_rate], utc_time_range: @date_range_day, - filters: [[:is, "event:goal", ["Signup"]]], + filters: [[:is, "event:goal", ["Signup"], %{}]], dimensions: [], order_by: nil, timezone: site.timezone, @@ -1391,7 +1441,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do metrics: [:conversion_rate, :group_conversion_rate], utc_time_range: @date_range_day, filters: [ - [:is, "event:props:foo", ["bar"]] + [:is, "event:props:foo", ["bar"], %{}] ], dimensions: ["event:goal"], order_by: nil, @@ -1456,7 +1506,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do %{ metrics: [:scroll_depth], utc_time_range: @date_range_day, - filters: [[:is, "event:page", ["/"]]], + filters: [[:is, "event:page", ["/"], %{}]], dimensions: [], order_by: nil, timezone: site.timezone, @@ -1504,7 +1554,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do |> check_success(site, %{ metrics: [:views_per_visit], utc_time_range: @date_range_day, - filters: [[:is, "event:goal", ["Signup"]]], + filters: [[:is, "event:goal", ["Signup"], %{}]], dimensions: [], order_by: nil, timezone: site.timezone, @@ -1630,7 +1680,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do %{ metrics: [:total_revenue, :average_revenue], utc_time_range: @date_range_day, - filters: [[:is, "event:goal", ["PurchaseUSD", "Signup", "Subscription"]]], + filters: [[:is, "event:goal", ["PurchaseUSD", "Signup", "Subscription"], %{}]], dimensions: [], order_by: nil, timezone: site.timezone, @@ -1661,7 +1711,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do %{ metrics: [:total_revenue, :average_revenue], utc_time_range: @date_range_day, - filters: [[:is, "event:goal", ["Purchase", "Signup", "Subscription"]]], + filters: [[:is, "event:goal", ["Purchase", "Signup", "Subscription"], %{}]], dimensions: [], order_by: nil, timezone: site.timezone, @@ -1725,7 +1775,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do %{ metrics: [:total_revenue, :average_revenue], utc_time_range: @date_range_day, - filters: [[:is, "event:goal", ["Purchase", "Signup"]]], + filters: [[:is, "event:goal", ["Purchase", "Signup"], %{}]], dimensions: ["event:goal"], order_by: nil, timezone: site.timezone, @@ -1803,7 +1853,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do |> check_success(site, %{ metrics: [:bounce_rate], utc_time_range: @date_range_day, - filters: [[:is, "event:props:foo", ["(none)"]]], + filters: [[:is, "event:props:foo", ["(none)"], %{}]], dimensions: [], order_by: nil, timezone: site.timezone, diff --git a/test/plausible/stats/query_test.exs b/test/plausible/stats/query_test.exs index 8716128b7b73..88e10959560d 100644 --- a/test/plausible/stats/query_test.exs +++ b/test/plausible/stats/query_test.exs @@ -199,14 +199,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", ["Signup"]]] + assert q.filters == [[:is, "event:goal", ["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 From 20d61d5cb57fff3bf3cd11122d002dee8273f871 Mon Sep 17 00:00:00 2001 From: Karl-Aksel Puulmann Date: Thu, 28 Nov 2024 10:52:40 +0200 Subject: [PATCH 02/15] WIP: Modifiers v2 --- lib/plausible/goals/filters.ex | 2 +- .../google/search_console/filters.ex | 10 +-- lib/plausible/stats/breakdown.ex | 2 +- lib/plausible/stats/comparisons.ex | 2 +- lib/plausible/stats/email_report.ex | 2 +- lib/plausible/stats/filters/filters.ex | 8 +- .../filters/legacy_dashboard_filter_parser.ex | 24 +++--- lib/plausible/stats/filters/query_parser.ex | 13 +-- .../stats/filters/stats_api_filter_parser.ex | 14 +-- lib/plausible/stats/imported/base.ex | 6 +- lib/plausible/stats/query_result.ex | 9 +- lib/plausible/stats/sql/where_builder.ex | 50 ++++++----- .../api/external_stats_controller.ex | 2 +- .../controllers/api/stats_controller.ex | 2 +- .../google/search_console/filters_test.exs | 32 +++---- test/plausible/stats/comparisons_test.exs | 12 +-- test/plausible/stats/filters_test.exs | 30 +++---- .../legacy_dashboard_filter_parser_test.exs | 86 +++++++++---------- test/plausible/stats/query_parser_test.exs | 42 ++++----- test/plausible/stats/query_test.exs | 4 +- 20 files changed, 177 insertions(+), 175 deletions(-) diff --git a/lib/plausible/goals/filters.ex b/lib/plausible/goals/filters.ex index d4b4bea9af34..850575560e62 100644 --- a/lib/plausible/goals/filters.ex +++ b/lib/plausible/goals/filters.ex @@ -20,7 +20,7 @@ defmodule Plausible.Goals.Filters do * `imported?` - when `true`, builds conditions on the `page` db field rather than `pathname`, and also skips the `e.name == "pageview"` check. """ - def add_filter(query, [operation, "event:goal", clauses, _modifiers], opts \\ []) + def add_filter(query, [operation, "event:goal", clauses | _], opts \\ []) when operation in [:is, :contains] do imported? = Keyword.get(opts, :imported?, false) diff --git a/lib/plausible/google/search_console/filters.ex b/lib/plausible/google/search_console/filters.ex index 5296b34bff93..1874e1640351 100644 --- a/lib/plausible/google/search_console/filters.ex +++ b/lib/plausible/google/search_console/filters.ex @@ -24,15 +24,15 @@ defmodule Plausible.Google.SearchConsole.Filters do transform_filter(property, [op, "visit:entry_page" | rest]) end - # :TODO: Should also work case-insensitive - defp transform_filter(property, [:is, "visit:entry_page", pages, _modifiers]) when is_list(pages) do + # :TODO: Should also work case-insensitive, if not, blacklist. + defp transform_filter(property, [:is, "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_wildcard, "visit:entry_page", pages, _modifiers]) + defp transform_filter(property, [:matches_wildcard, "visit:entry_page", pages | _]) when is_list(pages) do expression = Enum.map_join(pages, "|", fn page -> page_regex(property_url(property, page)) end) @@ -40,12 +40,12 @@ defmodule Plausible.Google.SearchConsole.Filters do %{dimension: "page", operator: "includingRegex", expression: expression} end - defp transform_filter(_property, [:is, "visit:screen", devices, _modifiers]) when is_list(devices) do + defp transform_filter(_property, [:is, "visit:screen", devices | _]) when is_list(devices) do expression = Enum.map_join(devices, "|", &search_console_device/1) %{dimension: "device", operator: "includingRegex", expression: expression} end - defp transform_filter(_property, [:is, "visit:country", countries, _modifiers]) + defp transform_filter(_property, [:is, "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/breakdown.ex b/lib/plausible/stats/breakdown.ex index f4b9449e9ce2..3e0699b605e2 100644 --- a/lib/plausible/stats/breakdown.ex +++ b/lib/plausible/stats/breakdown.ex @@ -151,7 +151,7 @@ defmodule Plausible.Stats.Breakdown do @extra_filter_dimensions Map.keys(@filter_dimensions_not) defp dimension_filters(dimension) when dimension in @extra_filter_dimensions do - [[:is_not, dimension, Map.get(@filter_dimensions_not, dimension), %{}]] + [[:is_not, dimension, Map.get(@filter_dimensions_not, dimension)]] end defp dimension_filters(_), do: [] diff --git a/lib/plausible/stats/comparisons.ex b/lib/plausible/stats/comparisons.ex index 77f557a6eec2..647658855bfd 100644 --- a/lib/plausible/stats/comparisons.ex +++ b/lib/plausible/stats/comparisons.ex @@ -87,7 +87,7 @@ defmodule Plausible.Stats.Comparisons do query.dimensions |> Enum.zip(dimension_labels) |> Enum.reject(fn {dimension, _label} -> Time.time_dimension?(dimension) end) - |> Enum.map(fn {dimension, label} -> [:is, dimension, [label], %{}] end) + |> Enum.map(fn {dimension, label} -> [:is, dimension, [label]] end) case query_filters do [] -> [] diff --git a/lib/plausible/stats/email_report.ex b/lib/plausible/stats/email_report.ex index e205bb066dcc..4f9a403ab8b7 100644 --- a/lib/plausible/stats/email_report.ex +++ b/lib/plausible/stats/email_report.ex @@ -48,7 +48,7 @@ defmodule Plausible.Stats.EmailReport do defp put_top_5_sources(stats, site, query) do query = query - |> Query.add_filter([:is_not, "visit:source", ["Direct / None"], %{}]) + |> Query.add_filter([:is_not, "visit:source", ["Direct / None"]]) |> Query.set(dimensions: ["visit:source"]) sources = Stats.breakdown(site, query, [:visitors], {5, 1}) diff --git a/lib/plausible/stats/filters/filters.ex b/lib/plausible/stats/filters/filters.ex index 14e658a0afdd..0c16ccb3f21e 100644 --- a/lib/plausible/stats/filters/filters.ex +++ b/lib/plausible/stats/filters/filters.ex @@ -61,10 +61,10 @@ defmodule Plausible.Stats.Filters do ### Examples: iex> Filters.parse("{\\"page\\":\\"/blog/**\\"}") - [[:matches_wildcard, "event:page", ["/blog/**"], %{}]] + [[:matches_wildcard, "event:page", ["/blog/**"]]] iex> Filters.parse("visit:browser!=Chrome") - [[:is_not, "visit:browser", ["Chrome"], %{}]] + [[:is_not, "visit:browser", ["Chrome"]]] iex> Filters.parse(nil) [] @@ -121,8 +121,8 @@ defmodule Plausible.Stats.Filters do def rename_dimensions_used_in_filter(filters, renames) do transform_filters(filters, fn - [operation, dimension, clauses, modifiers] -> - [[operation, Map.get(renames, dimension, dimension), clauses, modifiers]] + [operation, dimension | rest] -> + [[operation, Map.get(renames, dimension, dimension) | rest]] _subtree -> nil diff --git a/lib/plausible/stats/filters/legacy_dashboard_filter_parser.ex b/lib/plausible/stats/filters/legacy_dashboard_filter_parser.ex index eae0728983bc..a0a6cf0401c6 100644 --- a/lib/plausible/stats/filters/legacy_dashboard_filter_parser.ex +++ b/lib/plausible/stats/filters/legacy_dashboard_filter_parser.ex @@ -35,40 +35,40 @@ defmodule Plausible.Stats.Filters.LegacyDashboardFilterParser do cond do is_negated && is_wildcard && is_list -> - [:matches_wildcard_not, key, val, %{}] + [:matches_wildcard_not, key, val] is_negated && is_contains && is_list -> - [:matches_wildcard_not, key, Enum.map(val, &"**#{&1}**"), %{}] + [:matches_wildcard_not, key, Enum.map(val, &"**#{&1}**")] is_wildcard && is_list -> - [:matches_wildcard, key, val, %{}] + [:matches_wildcard, key, val] is_negated && is_wildcard -> - [:matches_wildcard_not, key, [val], %{}] + [:matches_wildcard_not, key, [val]] is_negated && is_list -> - [:is_not, key, val, %{}] + [:is_not, key, val] is_negated && is_contains -> - [:matches_wildcard_not, key, ["**" <> val <> "**"], %{}] + [:matches_wildcard_not, key, ["**" <> val <> "**"]] is_negated -> - [:is_not, key, [val], %{}] + [:is_not, key, [val]] is_contains && is_list -> - [:contains, key, val, %{}] + [:contains, key, val] is_list -> - [:is, key, val, %{}] + [:is, key, val] is_contains -> - [:contains, key, [val], %{}] + [:contains, key, [val]] is_wildcard -> - [:matches_wildcard, key, [val], %{}] + [:matches_wildcard, key, [val]] true -> - [:is, key, [val], %{}] + [:is, key, [val]] end end diff --git a/lib/plausible/stats/filters/query_parser.ex b/lib/plausible/stats/filters/query_parser.ex index a1e97cd9b83e..ebd3ff9060dd 100644 --- a/lib/plausible/stats/filters/query_parser.ex +++ b/lib/plausible/stats/filters/query_parser.ex @@ -135,9 +135,8 @@ defmodule Plausible.Stats.Filters.QueryParser do :contains_not ] do with {:ok, clauses} <- parse_clauses_list(filter), - {:ok, config} <- parse_filter_modifiers(Enum.at(filter, 3)) - do - {:ok, [clauses, config]} + {:ok, modifiers} <- parse_filter_modifiers(Enum.at(filter, 3)) do + {:ok, [clauses | modifiers]} end end @@ -171,8 +170,12 @@ defmodule Plausible.Stats.Filters.QueryParser do defp parse_clauses_list(filter), do: {:error, "Invalid filter '#{i(filter)}'"} - defp parse_filter_modifiers(config)do - {:ok, atomize_keys(config || %{})} + defp parse_filter_modifiers(modifiers) when is_map(modifiers) do + {:ok, [atomize_keys(modifiers)]} + end + + defp parse_filter_modifiers(nil) do + {:ok, []} end defp parse_date(_site, date_string, _date) when is_binary(date_string) do diff --git a/lib/plausible/stats/filters/stats_api_filter_parser.ex b/lib/plausible/stats/filters/stats_api_filter_parser.ex index 43f693700e57..ef3b03cdaa01 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? -> [:matches_wildcard_not, key, [raw_value], %{}] - is_wildcard? -> [:matches_wildcard, 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? -> [:matches_wildcard_not, key, [raw_value]] + is_wildcard? -> [:matches_wildcard, key, [raw_value]] + is_list? -> [:is, key, parse_member_list(raw_value)] + is_negated? -> [:is_not, key, [final_value]] + true -> [:is, key, [final_value]] end |> reject_invalid_country_codes() @@ -40,7 +40,7 @@ defmodule Plausible.Stats.Filters.StatsAPIFilterParser do end end - defp reject_invalid_country_codes([_op, "visit:country", code_or_codes, _modifier] = filter) do + defp reject_invalid_country_codes([_op, "visit:country", code_or_codes | _rest] = filter) do code_or_codes |> List.wrap() |> Enum.reduce_while(filter, fn @@ -68,6 +68,6 @@ defmodule Plausible.Stats.Filters.StatsAPIFilterParser do remove_escape_chars(value) end - [:is, key, List.wrap(value), %{}] + [:is, key, List.wrap(value)] end end diff --git a/lib/plausible/stats/imported/base.ex b/lib/plausible/stats/imported/base.ex index bcb441987395..9469b4b7f32c 100644 --- a/lib/plausible/stats/imported/base.ex +++ b/lib/plausible/stats/imported/base.ex @@ -117,8 +117,8 @@ defmodule Plausible.Stats.Imported.Base do has_required_name_filter? = query.filters |> Enum.flat_map(fn - [:is, "event:name", names, _] -> names - [:is, "event:goal", names, _] -> names + [:is, "event:name", names | _rest] -> names + [:is, "event:goal", names | _rest] -> names _ -> [] end) |> Enum.any?(&(&1 in special_goals_for(property))) @@ -196,7 +196,7 @@ defmodule Plausible.Stats.Imported.Base do defp get_filter_goals(query) do query.filters |> Enum.filter(fn [_, dimension | _rest] -> dimension == "event:goal" end) - |> Enum.flat_map(fn [operation, _dimension, clauses, _modifiers] -> + |> Enum.flat_map(fn [operation, _dimension, clauses | _rest] -> Enum.flat_map(clauses, fn clause -> query.preloaded_goals |> Plausible.Goals.Filters.filter_preloaded(operation, clause) diff --git a/lib/plausible/stats/query_result.ex b/lib/plausible/stats/query_result.ex index 7db4dca6a172..82a21287315f 100644 --- a/lib/plausible/stats/query_result.ex +++ b/lib/plausible/stats/query_result.ex @@ -31,7 +31,7 @@ defmodule Plausible.Stats.QueryResult do to_iso8601(query.utc_time_range.first, query.timezone), to_iso8601(query.utc_time_range.last, query.timezone) ], - filters: query.filters |> remove_empty_modifiers(), + filters: query.filters, dimensions: query.dimensions, order_by: query.order_by |> Enum.map(&Tuple.to_list/1), include: include(query) |> Map.filter(fn {_key, val} -> val end), @@ -85,13 +85,6 @@ defmodule Plausible.Stats.QueryResult do |> DateTime.shift_zone!(timezone) |> DateTime.to_iso8601(:extended) end - - defp remove_empty_modifiers(filters) do - Plausible.Stats.Filters.transform_filters(filters, fn - [operation, dimension, clauses, %{}] -> [operation, dimension, clauses] - _ -> nil - end) - end end defimpl Jason.Encoder, for: Plausible.Stats.QueryResult do diff --git a/lib/plausible/stats/sql/where_builder.ex b/lib/plausible/stats/sql/where_builder.ex index 7bc6824d7885..df14a22485b1 100644 --- a/lib/plausible/stats/sql/where_builder.ex +++ b/lib/plausible/stats/sql/where_builder.ex @@ -93,7 +93,7 @@ defmodule Plausible.Stats.SQL.WhereBuilder do |> Enum.reduce(fn condition, acc -> dynamic([], ^acc or ^condition) end) end - defp add_filter(:events, _query, [:is, "event:name", clauses, _modifiers]) do + defp add_filter(:events, _query, [:is, "event:name", clauses | _rest]) do dynamic([e], e.name in ^clauses) end @@ -154,7 +154,7 @@ defmodule Plausible.Stats.SQL.WhereBuilder do false end - defp filter_custom_prop(prop_name, column_name, [:is, _, clauses, _modifiers]) do + defp filter_custom_prop(prop_name, column_name, [:is, _, clauses | _rest]) do none_value_included = Enum.member?(clauses, "(none)") dynamic( @@ -164,7 +164,7 @@ defmodule Plausible.Stats.SQL.WhereBuilder do ) end - defp filter_custom_prop(prop_name, column_name, [:is_not, _, clauses, _modifiers]) do + defp filter_custom_prop(prop_name, column_name, [:is_not, _, clauses | _rest]) do none_value_included = Enum.member?(clauses, "(none)") dynamic( @@ -178,19 +178,23 @@ defmodule Plausible.Stats.SQL.WhereBuilder do ) end - defp filter_custom_prop(prop_name, column_name, [:matches_wildcard, dimension, clauses, modifiers]) do + defp filter_custom_prop(prop_name, column_name, [:matches_wildcard, dimension, clauses | rest]) do regexes = Enum.map(clauses, &page_regex/1) - filter_custom_prop(prop_name, column_name, [:matches, dimension, regexes, modifiers]) + filter_custom_prop(prop_name, column_name, [:matches, dimension, regexes | rest]) end - defp filter_custom_prop(prop_name, column_name, [:matches_wildcard_not, dimension, clauses, modifiers]) do + defp filter_custom_prop(prop_name, column_name, [ + :matches_wildcard_not, + dimension, + clauses | rest + ]) do regexes = Enum.map(clauses, &page_regex/1) - filter_custom_prop(prop_name, column_name, [:matches_not, dimension, regexes, modifiers]) + filter_custom_prop(prop_name, column_name, [:matches_not, dimension, regexes | rest]) end - defp filter_custom_prop(prop_name, column_name, [:matches, _dimension, clauses, _modifiers]) do + defp filter_custom_prop(prop_name, column_name, [:matches, _dimension, clauses | _rest]) do dynamic( [t], has_key(t, column_name, ^prop_name) and @@ -202,7 +206,7 @@ defmodule Plausible.Stats.SQL.WhereBuilder do ) end - defp filter_custom_prop(prop_name, column_name, [:matches_not, _dimension, clauses, _modifiers]) do + defp filter_custom_prop(prop_name, column_name, [:matches_not, _dimension, clauses | _rest]) do dynamic( [t], has_key(t, column_name, ^prop_name) and @@ -214,7 +218,7 @@ defmodule Plausible.Stats.SQL.WhereBuilder do ) end - defp filter_custom_prop(prop_name, column_name, [:contains, _dimension, clauses, _modifiers]) do + defp filter_custom_prop(prop_name, column_name, [:contains, _dimension, clauses | _rest]) do dynamic( [t], has_key(t, column_name, ^prop_name) and @@ -226,7 +230,7 @@ defmodule Plausible.Stats.SQL.WhereBuilder do ) end - defp filter_custom_prop(prop_name, column_name, [:contains_not, _dimension, clauses, _modifiers]) do + defp filter_custom_prop(prop_name, column_name, [:contains_not, _dimension, clauses | _rest]) do dynamic( [t], has_key(t, column_name, ^prop_name) and @@ -238,7 +242,7 @@ defmodule Plausible.Stats.SQL.WhereBuilder do ) end - defp filter_field(db_field, [:matches_wildcard, _dimension, glob_exprs, _modifiers]) do + defp filter_field(db_field, [:matches_wildcard, _dimension, glob_exprs | _rest]) do page_regexes = Enum.map(glob_exprs, &page_regex/1) dynamic( @@ -247,36 +251,36 @@ defmodule Plausible.Stats.SQL.WhereBuilder do ) end - defp filter_field(db_field, [:matches_wildcard_not, dimension, clauses, modifiers]) do - dynamic([], not (^filter_field(db_field, [:matches_wildcard, dimension, clauses, modifiers]))) + defp filter_field(db_field, [:matches_wildcard_not | rest]) do + dynamic([], not (^filter_field(db_field, [:matches_wildcard | rest]))) end - defp filter_field(db_field, [:contains, _dimension, values, _modifiers]) do + defp filter_field(db_field, [:contains, _dimension, values | _rest]) do dynamic([x], fragment("multiSearchAny(?, ?)", type(field(x, ^db_field), :string), ^values)) end - defp filter_field(db_field, [:contains_not, dimension, clauses, modifiers]) do - dynamic([], not (^filter_field(db_field, [:contains, dimension, clauses, modifiers]))) + defp filter_field(db_field, [:contains_not | rest]) do + dynamic([], not (^filter_field(db_field, [:contains | rest]))) end - defp filter_field(db_field, [:matches, _dimension, clauses, _modifiers]) do + defp filter_field(db_field, [:matches, _dimension, clauses | _rest]) do dynamic( [x], fragment("multiMatchAny(?, ?)", type(field(x, ^db_field), :string), ^clauses) ) end - defp filter_field(db_field, [:matches_not, dimension, clauses, modifiers]) do - dynamic([], not (^filter_field(db_field, [:matches, dimension, clauses, modifiers]))) + defp filter_field(db_field, [:matches_not | rest]) do + dynamic([], not (^filter_field(db_field, [:matches | rest]))) end - defp filter_field(db_field, [:is, _dimension, clauses, _modifiers]) do + defp filter_field(db_field, [:is, _dimension, clauses | _rest]) do list = Enum.map(clauses, &db_field_val(db_field, &1)) dynamic([x], field(x, ^db_field) in ^list) end - defp filter_field(db_field, [:is_not, dimension, clauses, modifiers]) do - dynamic([], not (^filter_field(db_field, [:is, dimension, clauses, modifiers]))) + defp filter_field(db_field, [:is_not | rest]) do + dynamic([], not (^filter_field(db_field, [:is | rest]))) end @no_ref "Direct / None" diff --git a/lib/plausible_web/controllers/api/external_stats_controller.ex b/lib/plausible_web/controllers/api/external_stats_controller.ex index 121206b8d184..8e121b817557 100644 --- a/lib/plausible_web/controllers/api/external_stats_controller.ex +++ b/lib/plausible_web/controllers/api/external_stats_controller.ex @@ -351,7 +351,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController do end) end - defp validate_filter(site, [_type, "event:goal", goal_filter, _modifiers]) do + defp validate_filter(site, [_type, "event:goal", goal_filter | _rest]) do configured_goals = site |> Plausible.Goals.for_site() diff --git a/lib/plausible_web/controllers/api/stats_controller.ex b/lib/plausible_web/controllers/api/stats_controller.ex index 88d4819725fc..89b73d5caed6 100644 --- a/lib/plausible_web/controllers/api/stats_controller.ex +++ b/lib/plausible_web/controllers/api/stats_controller.ex @@ -951,7 +951,7 @@ defmodule PlausibleWeb.Api.StatsController do query |> struct!(order_by: []) |> Query.remove_top_level_filters(["visit:exit_page"]) - |> Query.add_filter([:is, "event:page", pages, %{}]) + |> Query.add_filter([:is, "event:page", pages]) |> Query.set(dimensions: ["event:page"]) total_pageviews = diff --git a/test/plausible/google/search_console/filters_test.exs b/test/plausible/google/search_console/filters_test.exs index a37c60f06ff9..d056dfb2e670 100644 --- a/test/plausible/google/search_console/filters_test.exs +++ b/test/plausible/google/search_console/filters_test.exs @@ -4,7 +4,7 @@ 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, "") @@ -24,7 +24,7 @@ defmodule Plausible.Google.SearchConsole.FiltersTest do test "transforms matches_wildcard page filter" do filters = [ - [:matches_wildcard, "visit:entry_page", ["*page*"], %{}] + [:matches_wildcard, "visit:entry_page", ["*page*"]] ] {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters, "") @@ -44,7 +44,7 @@ defmodule Plausible.Google.SearchConsole.FiltersTest do test "transforms is page filter" do filters = [ - [:is, "visit:entry_page", ["/pageA", "/pageB"], %{}] + [:is, "visit:entry_page", ["/pageA", "/pageB"]] ] {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters, "") @@ -64,7 +64,7 @@ defmodule Plausible.Google.SearchConsole.FiltersTest do test "transforms matches multiple page filter" do filters = [ - [:matches_wildcard, "visit:entry_page", ["/pageA*", "/pageB*"], %{}] + [:matches_wildcard, "visit:entry_page", ["/pageA*", "/pageB*"]] ] {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters, "") @@ -84,7 +84,7 @@ defmodule Plausible.Google.SearchConsole.FiltersTest do test "transforms event:page exactly like visit:entry_page" do filters = [ - [:matches_wildcard, "event:page", ["/pageA*", "/pageB*"], %{}] + [:matches_wildcard, "event:page", ["/pageA*", "/pageB*"]] ] {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters, "") @@ -104,7 +104,7 @@ 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, "") @@ -120,7 +120,7 @@ defmodule Plausible.Google.SearchConsole.FiltersTest do test "transforms is visit:screen filter" do filters = [ - [:is, "visit:screen", ["Mobile", "Tablet"], %{}] + [:is, "visit:screen", ["Mobile", "Tablet"]] ] {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters, "") @@ -136,7 +136,7 @@ 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, "") @@ -148,7 +148,7 @@ defmodule Plausible.Google.SearchConsole.FiltersTest do test "transforms member visit:country filter" do filters = [ - [:is, "visit:country", ["EE", "PL"], %{}] + [:is, "visit:country", ["EE", "PL"]] ] {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters, "") @@ -164,9 +164,9 @@ defmodule Plausible.Google.SearchConsole.FiltersTest do test "filters can be combined" do filters = [ - [:is, "visit:country", ["EE", "PL"], %{}], - [:matches_wildcard, "visit:entry_page", ["*web-analytics*"], %{}], - [:is, "visit:screen", ["Desktop"], %{}] + [:is, "visit:country", ["EE", "PL"]], + [:matches_wildcard, "visit:entry_page", ["*web-analytics*"]], + [:is, "visit:screen", ["Desktop"]] ] {:ok, transformed} = Filters.transform("sc-domain:plausible.io", filters, "") @@ -188,10 +188,10 @@ defmodule Plausible.Google.SearchConsole.FiltersTest do test "when unsupported filter is included the whole set becomes invalid" do filters = [ - [:matches_wildcard, "visit:entry_page", "*web-analytics*", %{}], - [:is, "visit:screen", "Desktop", %{}], - [:member, "visit:country", ["EE", "PL"], %{}], - [:is, "visit:utm_medium", "facebook", %{}] + [:matches_wildcard, "visit:entry_page", "*web-analytics*"], + [:is, "visit:screen", "Desktop"], + [:member, "visit:country", ["EE", "PL"]], + [:is, "visit:utm_medium", "facebook"] ] assert :unsupported_filters = Filters.transform("sc-domain:plausible.io", filters, "") diff --git a/test/plausible/stats/comparisons_test.exs b/test/plausible/stats/comparisons_test.exs index f04356727a15..f291cfd6b071 100644 --- a/test/plausible/stats/comparisons_test.exs +++ b/test/plausible/stats/comparisons_test.exs @@ -298,7 +298,7 @@ defmodule Plausible.Stats.ComparisonsTest do Comparisons.add_comparison_filters(query, [%{dimensions: ["Chrome"], metrics: [123]}]) assert result_query.filters == [ - [:ignore_in_totals_query, [:is, "visit:browser", ["Chrome"], %{}]] + [:ignore_in_totals_query, [:is, "visit:browser", ["Chrome"]]] ] end @@ -323,7 +323,7 @@ defmodule Plausible.Stats.ComparisonsTest do result_query = Comparisons.add_comparison_filters(query, main_query_results) assert result_query.filters == [ - [:is, "visit:country_name", ["Estonia"], %{}], + [:is, "visit:country_name", ["Estonia"]], [ :ignore_in_totals_query, [ @@ -332,15 +332,15 @@ defmodule Plausible.Stats.ComparisonsTest do [ :and, [ - [:is, "visit:browser", ["Chrome"], %{}], - [:is, "visit:browser_version", ["99.9"], %{}] + [:is, "visit:browser", ["Chrome"]], + [:is, "visit:browser_version", ["99.9"]] ] ], [ :and, [ - [:is, "visit:browser", ["Firefox"], %{}], - [:is, "visit:browser_version", ["12.0"], %{}] + [:is, "visit:browser", ["Firefox"]], + [:is, "visit:browser_version", ["12.0"]] ] ] ] diff --git a/test/plausible/stats/filters_test.exs b/test/plausible/stats/filters_test.exs index ec7ee04a3b37..c239bfa156d0 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_wildcard, "event:page", ["/blog/post-*"], %{}]]) + |> assert_parsed([[:matches_wildcard, "event:page", ["/blog/post-*"]]]) end test "negative wildcard" do "event:page!=/blog/post-*" - |> assert_parsed([[:matches_wildcard_not, "event:page", ["/blog/post-*"], %{}]]) + |> assert_parsed([[:matches_wildcard_not, "event:page", ["/blog/post-*"]]]) end test "custom event goal" do "event:goal==Signup" - |> assert_parsed([[:is, "event:goal", ["Signup"], %{}]]) + |> assert_parsed([[:is, "event:goal", ["Signup"]]]) end test "pageview goal" do "event:goal==Visit /blog" - |> assert_parsed([[:is, "event:goal", ["Visit /blog"], %{}]]) + |> assert_parsed([[:is, "event:goal", ["Visit /blog"]]]) end test "is" do "visit:country==FR|GB|DE" - |> assert_parsed([[:is, "visit:country", ["FR", "GB", "DE"], %{}]]) + |> assert_parsed([[:is, "visit:country", ["FR", "GB", "DE"]]]) end test "member + wildcard" do "event:page==/blog**|/newsletter|/*/" - |> assert_parsed([[:matches_wildcard, "event:page", ["/blog**|/newsletter|/*/"], %{}]]) + |> assert_parsed([[:matches_wildcard, "event:page", ["/blog**|/newsletter|/*/"]]]) end test "combined with \";\"" do "event:page==/blog**|/newsletter|/*/ ; visit:country==FR|GB|DE" |> assert_parsed([ - [:matches_wildcard, "event:page", ["/blog**|/newsletter|/*/"], %{}], - [:is, "visit:country", ["FR", "GB", "DE"], %{}] + [:matches_wildcard, "event:page", ["/blog**|/newsletter|/*/"]], + [:is, "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 "utm_campaign==campaign \\| 1|campaign \\| 2" - |> assert_parsed([[:is, "utm_campaign", ["campaign | 1", "campaign | 2"], %{}]]) + |> assert_parsed([[:is, "utm_campaign", ["campaign | 1", "campaign | 2"]]]) end test "keeps escape characters in is + wildcard filter" do "event:page==/**\\|page|/other/page" - |> assert_parsed([[:matches_wildcard, "event:page", ["/**\\|page|/other/page"], %{}]]) + |> assert_parsed([[:matches_wildcard, "event:page", ["/**\\|page|/other/page"]]]) end test "gracefully fails to parse garbage" do @@ -103,7 +103,7 @@ defmodule Plausible.Stats.FiltersTest do test "simple" do [["is", "event:name", ["pageview"]]] |> Jason.encode!() - |> assert_parsed([[:is, "event:name", ["pageview"], %{}]]) + |> 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 index e06e2b120b8c..89002036f848 100644 --- a/test/plausible/stats/legacy_dashboard_filter_parser_test.exs +++ b/test/plausible/stats/legacy_dashboard_filter_parser_test.exs @@ -9,174 +9,174 @@ defmodule Plausible.Stats.Legacy.DashboardFilterParserTest do describe "adding prefix" do test "adds appropriate prefix to filter" do %{"page" => "/"} - |> assert_parsed([[:is, "event:page", ["/"], %{}]]) + |> assert_parsed([[:is, "event:page", ["/"]]]) %{"goal" => "Signup"} - |> assert_parsed([[:is, "event:goal", ["Signup"], %{}]]) + |> assert_parsed([[:is, "event:goal", ["Signup"]]]) %{"goal" => "Visit /blog"} - |> assert_parsed([[:is, "event:goal", ["Visit /blog"], %{}]]) + |> assert_parsed([[:is, "event:goal", ["Visit /blog"]]]) %{"source" => "Google"} - |> assert_parsed([[:is, "visit:source", ["Google"], %{}]]) + |> assert_parsed([[:is, "visit:source", ["Google"]]]) %{"referrer" => "cnn.com"} - |> assert_parsed([[:is, "visit:referrer", ["cnn.com"], %{}]]) + |> assert_parsed([[:is, "visit:referrer", ["cnn.com"]]]) %{"utm_medium" => "search"} - |> assert_parsed([[:is, "visit:utm_medium", ["search"], %{}]]) + |> assert_parsed([[:is, "visit:utm_medium", ["search"]]]) %{"utm_source" => "bing"} - |> assert_parsed([[:is, "visit:utm_source", ["bing"], %{}]]) + |> assert_parsed([[:is, "visit:utm_source", ["bing"]]]) %{"utm_content" => "content"} - |> assert_parsed([[:is, "visit:utm_content", ["content"], %{}]]) + |> assert_parsed([[:is, "visit:utm_content", ["content"]]]) %{"utm_term" => "term"} - |> assert_parsed([[:is, "visit:utm_term", ["term"], %{}]]) + |> assert_parsed([[:is, "visit:utm_term", ["term"]]]) %{"screen" => "Desktop"} - |> assert_parsed([[:is, "visit:screen", ["Desktop"], %{}]]) + |> assert_parsed([[:is, "visit:screen", ["Desktop"]]]) %{"browser" => "Opera"} - |> assert_parsed([[:is, "visit:browser", ["Opera"], %{}]]) + |> assert_parsed([[:is, "visit:browser", ["Opera"]]]) %{"browser_version" => "10.1"} - |> assert_parsed([[:is, "visit:browser_version", ["10.1"], %{}]]) + |> assert_parsed([[:is, "visit:browser_version", ["10.1"]]]) %{"os" => "Linux"} - |> assert_parsed([[:is, "visit:os", ["Linux"], %{}]]) + |> assert_parsed([[:is, "visit:os", ["Linux"]]]) %{"os_version" => "13.0"} - |> assert_parsed([[:is, "visit:os_version", ["13.0"], %{}]]) + |> assert_parsed([[:is, "visit:os_version", ["13.0"]]]) %{"country" => "EE"} - |> assert_parsed([[:is, "visit:country", ["EE"], %{}]]) + |> assert_parsed([[:is, "visit:country", ["EE"]]]) %{"region" => "EE-12"} - |> assert_parsed([[:is, "visit:region", ["EE-12"], %{}]]) + |> assert_parsed([[:is, "visit:region", ["EE-12"]]]) %{"city" => "123"} - |> assert_parsed([[:is, "visit:city", ["123"], %{}]]) + |> assert_parsed([[:is, "visit:city", ["123"]]]) %{"entry_page" => "/blog"} - |> assert_parsed([[:is, "visit:entry_page", ["/blog"], %{}]]) + |> assert_parsed([[:is, "visit:entry_page", ["/blog"]]]) %{"exit_page" => "/blog"} - |> assert_parsed([[:is, "visit:exit_page", ["/blog"], %{}]]) + |> assert_parsed([[:is, "visit:exit_page", ["/blog"]]]) %{"props" => %{"cta" => "Top"}} - |> assert_parsed([[:is, "event:props:cta", ["Top"], %{}]]) + |> assert_parsed([[:is, "event:props:cta", ["Top"]]]) %{"hostname" => "dummy.site"} - |> assert_parsed([[:is, "event: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", ["Foo | Bar"], %{}]]) + |> assert_parsed([[:is, "event:goal", ["Foo | Bar"]]]) end test "in member filter" do %{"page" => ~S(/|\|)} - |> assert_parsed([[:is, "event:page", ["/", "|"], %{}]]) + |> 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", ["/"], %{}]]) + |> assert_parsed([[:is_not, "event:page", ["/"]]]) %{"props" => %{"cta" => "!Top"}} - |> assert_parsed([[:is_not, "event: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"], %{}]]) + |> assert_parsed([[:is, "event:page", ["/", "/blog"]]]) end test "escaping pipe character" do %{"page" => "/|\\|"} - |> assert_parsed([[:is, "event:page", ["/", "|"], %{}]]) + |> assert_parsed([[:is, "event:page", ["/", "|"]]]) end test "mixed goals" do %{"goal" => "Signup|Visit /thank-you"} - |> assert_parsed([[:is, "event:goal", ["Signup", "Visit /thank-you"], %{}]]) + |> assert_parsed([[:is, "event:goal", ["Signup", "Visit /thank-you"]]]) %{"goal" => "Visit /thank-you|Signup"} - |> assert_parsed([[:is, "event:goal", ["Visit /thank-you", "Signup"], %{}]]) + |> assert_parsed([[:is, "event:goal", ["Visit /thank-you", "Signup"]]]) end end describe "matches_wildcard filter type" do test "parses matches filter type" do %{"page" => "/|/blog**"} - |> assert_parsed([[:matches_wildcard, "event:page", ["/", "/blog**"], %{}]]) + |> assert_parsed([[:matches_wildcard, "event:page", ["/", "/blog**"]]]) end test "parses not_matches filter type" do %{"page" => "!/|/blog**"} - |> assert_parsed([[:matches_wildcard_not, "event:page", ["/", "/blog**"], %{}]]) + |> assert_parsed([[:matches_wildcard_not, "event:page", ["/", "/blog**"]]]) end test "single matches" do %{"page" => "~blog"} - |> assert_parsed([[:contains, "event:page", ["blog"], %{}]]) + |> assert_parsed([[:contains, "event:page", ["blog"]]]) end test "negated matches" do %{"page" => "!~articles"} - |> assert_parsed([[:matches_wildcard_not, "event:page", ["**articles**"], %{}]]) + |> assert_parsed([[:matches_wildcard_not, "event:page", ["**articles**"]]]) end test "matches member" do %{"page" => "~articles|blog"} - |> assert_parsed([[:contains, "event:page", ["articles", "blog"], %{}]]) + |> assert_parsed([[:contains, "event:page", ["articles", "blog"]]]) end test "not matches member" do %{"page" => "!~articles|blog"} - |> assert_parsed([[:matches_wildcard_not, "event:page", ["**articles**", "**blog**"], %{}]]) + |> assert_parsed([[:matches_wildcard_not, "event:page", ["**articles**", "**blog**"]]]) end test "other filters default to `is` even when wildcard is present" do %{"country" => "Germa**"} - |> assert_parsed([[:is, "visit:country", ["Germa**"], %{}]]) + |> assert_parsed([[:is, "visit:country", ["Germa**"]]]) end test "can be used with `page` filter" do %{"page" => "!/blog/post-*"} - |> assert_parsed([[:matches_wildcard_not, "event:page", ["/blog/post-*"], %{}]]) + |> assert_parsed([[:matches_wildcard_not, "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**"], %{}]]) + |> 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"], %{}]]) + |> assert_parsed([[:is_not, "event:page", ["/", "/blog"]]]) end test "mixed goals" do %{"goal" => "!Signup|Visit /thank-you"} |> assert_parsed([ - [:is_not, "event:goal", ["Signup", "Visit /thank-you"], %{}] + [:is_not, "event:goal", ["Signup", "Visit /thank-you"]] ]) %{"goal" => "!Visit /thank-you|Signup"} |> assert_parsed([ - [:is_not, "event:goal", ["Visit /thank-you", "Signup"], %{}] + [:is_not, "event:goal", ["Visit /thank-you", "Signup"]] ]) end end @@ -184,10 +184,10 @@ defmodule Plausible.Stats.Legacy.DashboardFilterParserTest do describe "contains prefix filter type" do test "can be used with any filter" do %{"page" => "~/blog/post"} - |> assert_parsed([[:contains, "event:page", ["/blog/post"], %{}]]) + |> assert_parsed([[:contains, "event:page", ["/blog/post"]]]) %{"source" => "~facebook"} - |> assert_parsed([[:contains, "visit:source", ["facebook"], %{}]]) + |> assert_parsed([[:contains, "visit:source", ["facebook"]]]) end end end diff --git a/test/plausible/stats/query_parser_test.exs b/test/plausible/stats/query_parser_test.exs index 9a40a189b423..9525b0642a02 100644 --- a/test/plausible/stats/query_parser_test.exs +++ b/test/plausible/stats/query_parser_test.exs @@ -208,7 +208,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do metrics: [:visitors], utc_time_range: @date_range_day, filters: [ - [unquote(operation), "event:name", ["foo"], %{}] + [unquote(operation), "event:name", ["foo"]] ], dimensions: [], order_by: nil, @@ -333,7 +333,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do metrics: [:visitors], utc_time_range: @date_range_day, filters: [ - [:is, "event:props:foobar", ["value"], %{}] + [:is, "event:props:foobar", ["value"]] ], dimensions: [], order_by: nil, @@ -358,7 +358,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do metrics: [:visitors], utc_time_range: @date_range_day, filters: [ - [:is, "event:#{unquote(dimension)}", ["foo"], %{}] + [:is, "event:#{unquote(dimension)}", ["foo"]] ], dimensions: [], order_by: nil, @@ -384,7 +384,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do metrics: [:visitors], utc_time_range: @date_range_day, filters: [ - [:is, "visit:#{unquote(dimension)}", ["ab"], %{}] + [:is, "visit:#{unquote(dimension)}", ["ab"]] ], dimensions: [], order_by: nil, @@ -450,7 +450,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do metrics: [:visitors], utc_time_range: @date_range_day, filters: [ - [:is, "visit:city", [123, 456], %{}] + [:is, "visit:city", [123, 456]] ], dimensions: [], order_by: nil, @@ -469,7 +469,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do metrics: [:visitors], utc_time_range: @date_range_day, filters: [ - [:is, "visit:city", ["123", "456"], %{}] + [:is, "visit:city", ["123", "456"]] ], dimensions: [], order_by: nil, @@ -523,11 +523,11 @@ defmodule Plausible.Stats.Filters.QueryParserTest do [ :and, [ - [:is, "visit:city_name", ["Tallinn"], %{}], - [:is, "visit:country_name", ["Estonia"], %{}] + [:is, "visit:city_name", ["Tallinn"]], + [:is, "visit:country_name", ["Estonia"]] ] ], - [:not, [:is, "visit:country_name", ["Estonia"], %{}]] + [:not, [:is, "visit:country_name", ["Estonia"]]] ] ] ], @@ -576,7 +576,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do metrics: [:visitors], utc_time_range: @date_range_day, filters: [ - [:is, "event:hostname", ["a.plausible.io"], %{}] + [:is, "event:hostname", ["a.plausible.io"]] ], dimensions: [], order_by: nil, @@ -641,7 +641,9 @@ defmodule Plausible.Stats.Filters.QueryParserTest do "site_id" => site.domain, "metrics" => ["visitors"], "date_range" => "all", - "filters" => [["is_not", "event:hostname", ["a.plausible.io"], %{"case_sensitive" => false}]] + "filters" => [ + ["is_not", "event:hostname", ["a.plausible.io"], %{"case_sensitive" => false}] + ] } |> check_error( site, @@ -896,7 +898,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do metrics: [:visitors], utc_time_range: @date_range_day, filters: [ - [:is, "event:goal", ["Signup", "Visit /thank-you"], %{}] + [:is, "event:goal", ["Signup", "Visit /thank-you"]] ], dimensions: [], order_by: nil, @@ -1396,7 +1398,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do |> check_success(site, %{ metrics: [:conversion_rate], utc_time_range: @date_range_day, - filters: [[:is, "event:goal", ["Signup"], %{}]], + filters: [[:is, "event:goal", ["Signup"]]], dimensions: [], order_by: nil, timezone: site.timezone, @@ -1441,7 +1443,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do metrics: [:conversion_rate, :group_conversion_rate], utc_time_range: @date_range_day, filters: [ - [:is, "event:props:foo", ["bar"], %{}] + [:is, "event:props:foo", ["bar"]] ], dimensions: ["event:goal"], order_by: nil, @@ -1506,7 +1508,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do %{ metrics: [:scroll_depth], utc_time_range: @date_range_day, - filters: [[:is, "event:page", ["/"], %{}]], + filters: [[:is, "event:page", ["/"]]], dimensions: [], order_by: nil, timezone: site.timezone, @@ -1554,7 +1556,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do |> check_success(site, %{ metrics: [:views_per_visit], utc_time_range: @date_range_day, - filters: [[:is, "event:goal", ["Signup"], %{}]], + filters: [[:is, "event:goal", ["Signup"]]], dimensions: [], order_by: nil, timezone: site.timezone, @@ -1680,7 +1682,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do %{ metrics: [:total_revenue, :average_revenue], utc_time_range: @date_range_day, - filters: [[:is, "event:goal", ["PurchaseUSD", "Signup", "Subscription"], %{}]], + filters: [[:is, "event:goal", ["PurchaseUSD", "Signup", "Subscription"]]], dimensions: [], order_by: nil, timezone: site.timezone, @@ -1711,7 +1713,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do %{ metrics: [:total_revenue, :average_revenue], utc_time_range: @date_range_day, - filters: [[:is, "event:goal", ["Purchase", "Signup", "Subscription"], %{}]], + filters: [[:is, "event:goal", ["Purchase", "Signup", "Subscription"]]], dimensions: [], order_by: nil, timezone: site.timezone, @@ -1775,7 +1777,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do %{ metrics: [:total_revenue, :average_revenue], utc_time_range: @date_range_day, - filters: [[:is, "event:goal", ["Purchase", "Signup"], %{}]], + filters: [[:is, "event:goal", ["Purchase", "Signup"]]], dimensions: ["event:goal"], order_by: nil, timezone: site.timezone, @@ -1853,7 +1855,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do |> check_success(site, %{ metrics: [:bounce_rate], utc_time_range: @date_range_day, - filters: [[:is, "event:props:foo", ["(none)"], %{}]], + filters: [[:is, "event:props:foo", ["(none)"]]], dimensions: [], order_by: nil, timezone: site.timezone, diff --git a/test/plausible/stats/query_test.exs b/test/plausible/stats/query_test.exs index 88e10959560d..8716128b7b73 100644 --- a/test/plausible/stats/query_test.exs +++ b/test/plausible/stats/query_test.exs @@ -199,14 +199,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", ["Signup"], %{}]] + assert q.filters == [[:is, "event:goal", ["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 From 4c6ac23ddfb41ebe679b6cc8cf6761ab38746ac3 Mon Sep 17 00:00:00 2001 From: Karl-Aksel Puulmann Date: Thu, 28 Nov 2024 11:39:19 +0200 Subject: [PATCH 03/15] Use preloaded_goals when determining whether imports can be included This was previously broken with conversion_rate totals metrics since it removed event:goal filters but did not update preloaded_goals --- lib/plausible/goals/filters.ex | 2 +- lib/plausible/stats/imported/base.ex | 14 +------------- lib/plausible/stats/sql/special_metrics.ex | 6 ++++-- 3 files changed, 6 insertions(+), 16 deletions(-) diff --git a/lib/plausible/goals/filters.ex b/lib/plausible/goals/filters.ex index 850575560e62..3b1a01cba349 100644 --- a/lib/plausible/goals/filters.ex +++ b/lib/plausible/goals/filters.ex @@ -46,7 +46,7 @@ defmodule Plausible.Goals.Filters do end) end - def filter_preloaded(preloaded_goals, operation, clause) when operation in [:is, :contains] do + defp filter_preloaded(preloaded_goals, operation, clause) when operation in [:is, :contains] do Enum.filter(preloaded_goals, fn goal -> matches?(goal, operation, clause) end) end diff --git a/lib/plausible/stats/imported/base.ex b/lib/plausible/stats/imported/base.ex index 9469b4b7f32c..09b6371efc5d 100644 --- a/lib/plausible/stats/imported/base.ex +++ b/lib/plausible/stats/imported/base.ex @@ -177,8 +177,7 @@ defmodule Plausible.Stats.Imported.Base do |> Enum.map(&@property_to_table_mappings[&1]) filter_goal_table_candidates = - query - |> get_filter_goals() + query.preloaded_goals |> Enum.map(&Plausible.Goal.type/1) |> Enum.map(fn :event -> "imported_custom_events" @@ -193,17 +192,6 @@ defmodule Plausible.Stats.Imported.Base do end end - defp get_filter_goals(query) do - query.filters - |> Enum.filter(fn [_, dimension | _rest] -> dimension == "event:goal" end) - |> Enum.flat_map(fn [operation, _dimension, clauses | _rest] -> - Enum.flat_map(clauses, fn clause -> - query.preloaded_goals - |> Plausible.Goals.Filters.filter_preloaded(operation, clause) - end) - end) - end - def special_goals_for("event:props:url"), do: Imported.goals_with_url() def special_goals_for("event:props:path"), do: Imported.goals_with_path() end diff --git a/lib/plausible/stats/sql/special_metrics.ex b/lib/plausible/stats/sql/special_metrics.ex index 1bf77d50b6b1..e6b3aeba9065 100644 --- a/lib/plausible/stats/sql/special_metrics.ex +++ b/lib/plausible/stats/sql/special_metrics.ex @@ -56,7 +56,8 @@ defmodule Plausible.Stats.SQL.SpecialMetrics do |> remove_filters_ignored_in_totals_query() |> Query.set( dimensions: [], - include_imported: query.include_imported + include_imported: query.include_imported, + preloaded_goals: [] ) q @@ -98,7 +99,8 @@ defmodule Plausible.Stats.SQL.SpecialMetrics do |> Query.set( metrics: [:visitors], order_by: [], - include_imported: query.include_imported + include_imported: query.include_imported, + preloaded_goals: [] ) from(e in subquery(q), From f59b8e0391cd4112a0bd96b5403cbf6e8ff4b791 Mon Sep 17 00:00:00 2001 From: Karl-Aksel Puulmann Date: Thu, 28 Nov 2024 13:18:02 +0200 Subject: [PATCH 04/15] Preload goals according to modifiers --- lib/plausible/goals/filters.ex | 36 ++++++--- lib/plausible/stats/filters/query_parser.ex | 2 +- lib/plausible/stats/imported/base.ex | 2 +- test/plausible/stats/query_parser_test.exs | 90 +++++++++++++++++++++ 4 files changed, 117 insertions(+), 13 deletions(-) diff --git a/lib/plausible/goals/filters.ex b/lib/plausible/goals/filters.ex index 3b1a01cba349..b9d7d5a74935 100644 --- a/lib/plausible/goals/filters.ex +++ b/lib/plausible/goals/filters.ex @@ -20,14 +20,14 @@ defmodule Plausible.Goals.Filters do * `imported?` - when `true`, builds conditions on the `page` db field rather than `pathname`, and also skips the `e.name == "pageview"` check. """ - def add_filter(query, [operation, "event:goal", clauses | _], opts \\ []) + def add_filter(query, [operation, "event:goal", clauses | _] = filter, opts \\ []) when operation in [:is, :contains] do imported? = Keyword.get(opts, :imported?, false) Enum.reduce(clauses, false, fn clause, dynamic_statement -> condition = query.preloaded_goals - |> filter_preloaded(operation, clause) + |> filter_preloaded(filter, clause) |> build_condition(imported?) dynamic([e], ^condition or ^dynamic_statement) @@ -38,32 +38,46 @@ defmodule Plausible.Goals.Filters do goals = Plausible.Goals.for_site(site) Enum.reduce(filters, goals, fn - [operation, "event:goal", clauses | _rest], goals -> - goals_matching_any_clause(goals, operation, clauses) + [_, "event:goal" | _] = filter, goals -> + goals_matching_any_clause(goals, filter) _filter, goals -> goals end) end - defp filter_preloaded(preloaded_goals, operation, clause) when operation in [:is, :contains] do - Enum.filter(preloaded_goals, fn goal -> matches?(goal, operation, clause) end) + defp filter_preloaded(preloaded_goals, filter, clause) do + Enum.filter(preloaded_goals, fn goal -> matches?(goal, filter, clause) end) end - defp goals_matching_any_clause(goals, operation, clauses) do + defp goals_matching_any_clause(goals, [_, _, clauses | _] = filter) do goals |> Enum.filter(fn goal -> - Enum.any?(clauses, fn clause -> matches?(goal, operation, clause) end) + Enum.any?(clauses, fn clause -> matches?(goal, filter, clause) end) end) end - defp matches?(goal, operation, clause) do + defp matches?(goal, [operation | _rest] = filter, clause) do + goal_name = + goal + |> Plausible.Goal.display_name() + |> mod(filter) + + clause = mod(clause, filter) + case operation do :is -> - Plausible.Goal.display_name(goal) == clause + goal_name == clause :contains -> - String.contains?(Plausible.Goal.display_name(goal), clause) + String.contains?(goal_name, clause) + end + end + + defp mod(str, filter) do + case filter do + [_, _, _, %{case_sensitive: false}] -> String.downcase(str) + _ -> str end end diff --git a/lib/plausible/stats/filters/query_parser.ex b/lib/plausible/stats/filters/query_parser.ex index ebd3ff9060dd..684d6e1310f4 100644 --- a/lib/plausible/stats/filters/query_parser.ex +++ b/lib/plausible/stats/filters/query_parser.ex @@ -471,7 +471,7 @@ defmodule Plausible.Stats.Filters.QueryParser do # Note: Only works since event:goal is allowed as a top level filter goal_filter_clauses = Enum.flat_map(query.filters, fn - [:is, "event:goal", clauses | _rest] -> clauses + [:is, "event:goal", clauses] -> clauses _ -> [] end) diff --git a/lib/plausible/stats/imported/base.ex b/lib/plausible/stats/imported/base.ex index 09b6371efc5d..2328a4497f65 100644 --- a/lib/plausible/stats/imported/base.ex +++ b/lib/plausible/stats/imported/base.ex @@ -144,7 +144,7 @@ defmodule Plausible.Stats.Imported.Base do defp do_decide_tables(%Query{dimensions: ["event:goal"]} = query) do filter_dimensions = dimensions_used_in_filters(query.filters) - filter_goals = get_filter_goals(query) + filter_goals = query.preloaded_goals any_event_goals? = Enum.any?(filter_goals, fn goal -> Plausible.Goal.type(goal) == :event end) diff --git a/test/plausible/stats/query_parser_test.exs b/test/plausible/stats/query_parser_test.exs index 9525b0642a02..a92bc86fa878 100644 --- a/test/plausible/stats/query_parser_test.exs +++ b/test/plausible/stats/query_parser_test.exs @@ -652,6 +652,96 @@ defmodule Plausible.Stats.Filters.QueryParserTest do end end + describe "preloading goals" do + setup %{site: site} do + insert(:goal, %{site: site, event_name: "Signup"}) + insert(:goal, %{site: site, event_name: "Purchase"}) + insert(:goal, %{site: site, event_name: "Contact"}) + + :ok + end + + test "with exact match", %{site: site} do + %{ + "site_id" => site.domain, + "metrics" => ["visitors"], + "date_range" => "all", + "filters" => [["is", "event:goal", ["Signup", "Purchase"]]] + } + |> check_success(site, %{ + metrics: [:visitors], + utc_time_range: @date_range_day, + filters: [[:is, "event:goal", ["Signup", "Purchase"]]], + dimensions: [], + order_by: nil, + timezone: site.timezone, + include: %{imports: false, time_labels: false, total_rows: false, comparisons: nil}, + pagination: %{limit: 10_000, offset: 0} + }) + |> check_goals(preloaded_goals: ["Purchase", "Signup"], revenue_currencies: %{}) + end + + test "with case insensitive match", %{site: site} do + %{ + "site_id" => site.domain, + "metrics" => ["visitors"], + "date_range" => "all", + "filters" => [["is", "event:goal", ["signup", "purchase"], %{"case_sensitive" => false}]] + } + |> check_success(site, %{ + metrics: [:visitors], + utc_time_range: @date_range_day, + filters: [[:is, "event:goal", ["signup", "purchase"], %{case_sensitive: false}]], + dimensions: [], + order_by: nil, + timezone: site.timezone, + include: %{imports: false, time_labels: false, total_rows: false, comparisons: nil}, + pagination: %{limit: 10_000, offset: 0} + }) + |> check_goals(preloaded_goals: ["Purchase", "Signup"], revenue_currencies: %{}) + end + + test "with contains match", %{site: site} do + %{ + "site_id" => site.domain, + "metrics" => ["visitors"], + "date_range" => "all", + "filters" => [["contains", "event:goal", ["Sign", "pur"]]] + } + |> check_success(site, %{ + metrics: [:visitors], + utc_time_range: @date_range_day, + filters: [[:contains, "event:goal", ["Sign", "pur"]]], + dimensions: [], + order_by: nil, + timezone: site.timezone, + include: %{imports: false, time_labels: false, total_rows: false, comparisons: nil}, + pagination: %{limit: 10_000, offset: 0} + }) + |> check_goals(preloaded_goals: ["Signup"], revenue_currencies: %{}) + end + + test "with case insensitive contains match", %{site: site} do + %{ + "site_id" => site.domain, + "metrics" => ["visitors"], + "date_range" => "all", + "filters" => [["contains", "event:goal", ["sign", "CONT"], %{"case_sensitive" => false}]] + } + |> check_success(site, %{ + metrics: [:visitors], + utc_time_range: @date_range_day, + filters: [[:contains, "event:goal", ["sign", "CONT"], %{case_sensitive: false}]], + dimensions: [], + order_by: nil, + timezone: site.timezone, + include: %{imports: false, time_labels: false, total_rows: false, comparisons: nil}, + pagination: %{limit: 10_000, offset: 0} + }) + |> check_goals(preloaded_goals: ["Contact", "Signup"], revenue_currencies: %{}) + end + end + describe "include validation" do test "setting include values", %{site: site} do %{ From b40d6c7b95db441e456b257108d7fdeb0d960373 Mon Sep 17 00:00:00 2001 From: Karl-Aksel Puulmann Date: Thu, 28 Nov 2024 14:57:54 +0200 Subject: [PATCH 05/15] Make case_sensitive: false work for is/contains operators --- lib/plausible/stats/filters/query_parser.ex | 5 + lib/plausible/stats/sql/where_builder.ex | 50 ++- .../query_imported_test.exs | 295 ++++++++++++++++++ .../external_stats_controller/query_test.exs | 226 +++++++++++++- 4 files changed, 573 insertions(+), 3 deletions(-) diff --git a/lib/plausible/stats/filters/query_parser.ex b/lib/plausible/stats/filters/query_parser.ex index 684d6e1310f4..cf9744a1e3c6 100644 --- a/lib/plausible/stats/filters/query_parser.ex +++ b/lib/plausible/stats/filters/query_parser.ex @@ -17,7 +17,12 @@ defmodule Plausible.Stats.Filters.QueryParser do offset: 0 } + @default_modifiers %{ + case_sensitive: false + } + def default_include(), do: @default_include + def default_modifiers(), do: @default_modifiers def parse(site, schema_type, params, now \\ nil) when is_map(params) do {now, date} = diff --git a/lib/plausible/stats/sql/where_builder.ex b/lib/plausible/stats/sql/where_builder.ex index df14a22485b1..821bfd82a7bf 100644 --- a/lib/plausible/stats/sql/where_builder.ex +++ b/lib/plausible/stats/sql/where_builder.ex @@ -93,7 +93,12 @@ defmodule Plausible.Stats.SQL.WhereBuilder do |> Enum.reduce(fn condition, acc -> dynamic([], ^acc or ^condition) end) end - defp add_filter(:events, _query, [:is, "event:name", clauses | _rest]) do + defp add_filter(:events, _query, [:is, "event:name", clauses, %{case_sensitive: false}]) do + clauses = Enum.map(clauses, &String.downcase/1) + dynamic([e], fragment("lower(?)", e.name) in ^clauses) + end + + defp add_filter(:events, _query, [:is, "event:name", clauses]) do dynamic([e], e.name in ^clauses) end @@ -154,6 +159,18 @@ defmodule Plausible.Stats.SQL.WhereBuilder do false end + defp filter_custom_prop(prop_name, column_name, [:is, _, clauses, %{case_sensitive: false}]) do + clauses = clauses |> Enum.map(&String.downcase/1) + none_value_included = Enum.member?(clauses, "(none)") + + dynamic( + [t], + (has_key(t, column_name, ^prop_name) and + fragment("lower(?)", get_by_key(t, column_name, ^prop_name)) in ^clauses) or + (^none_value_included and not has_key(t, column_name, ^prop_name)) + ) + end + defp filter_custom_prop(prop_name, column_name, [:is, _, clauses | _rest]) do none_value_included = Enum.member?(clauses, "(none)") @@ -218,6 +235,23 @@ defmodule Plausible.Stats.SQL.WhereBuilder do ) end + defp filter_custom_prop(prop_name, column_name, [ + :contains, + _dimension, + clauses, + %{case_sensitive: false} + ]) do + dynamic( + [t], + has_key(t, column_name, ^prop_name) and + fragment( + "multiSearchAnyCaseInsensitive(?, ?)", + get_by_key(t, column_name, ^prop_name), + ^clauses + ) + ) + end + defp filter_custom_prop(prop_name, column_name, [:contains, _dimension, clauses | _rest]) do dynamic( [t], @@ -255,6 +289,13 @@ defmodule Plausible.Stats.SQL.WhereBuilder do dynamic([], not (^filter_field(db_field, [:matches_wildcard | rest]))) end + defp filter_field(db_field, [:contains, _dimension, values, %{case_sensitive: false}]) do + dynamic( + [x], + fragment("multiSearchAnyCaseInsensitive(?, ?)", type(field(x, ^db_field), :string), ^values) + ) + end + defp filter_field(db_field, [:contains, _dimension, values | _rest]) do dynamic([x], fragment("multiSearchAny(?, ?)", type(field(x, ^db_field), :string), ^values)) end @@ -274,8 +315,13 @@ defmodule Plausible.Stats.SQL.WhereBuilder do dynamic([], not (^filter_field(db_field, [:matches | rest]))) end + defp filter_field(db_field, [:is, _dimension, clauses, %{case_sensitive: false}]) do + list = clauses |> Enum.map(&db_field_val(db_field, &1)) |> Enum.map(&String.downcase/1) + dynamic([x], fragment("lower(?)", field(x, ^db_field)) in ^list) + end + defp filter_field(db_field, [:is, _dimension, clauses | _rest]) do - list = Enum.map(clauses, &db_field_val(db_field, &1)) + list = clauses |> Enum.map(&db_field_val(db_field, &1)) dynamic([x], field(x, ^db_field) in ^list) end diff --git a/test/plausible_web/controllers/api/external_stats_controller/query_imported_test.exs b/test/plausible_web/controllers/api/external_stats_controller/query_imported_test.exs index 3c7c1a41c37f..cd887fb7418a 100644 --- a/test/plausible_web/controllers/api/external_stats_controller/query_imported_test.exs +++ b/test/plausible_web/controllers/api/external_stats_controller/query_imported_test.exs @@ -34,6 +34,301 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryImportedTest do assert json_response(conn2, 200)["meta"]["imports_included"] refute json_response(conn2, 200)["meta"]["imports_warning"] end + + test "filters correctly with 'is' operator", %{ + conn: conn, + site: site, + site_import: site_import + } do + populate_stats(site, site_import.id, [ + build(:pageview, pathname: "/blog", timestamp: ~N[2023-01-01 00:00:00]), + build(:pageview, pathname: "/blog", timestamp: ~N[2023-01-01 00:00:00]), + build(:pageview, pathname: "/blog/post/1", timestamp: ~N[2023-01-01 00:00:00]), + build(:pageview, pathname: "/about", timestamp: ~N[2023-01-01 00:00:00]), + build(:imported_pages, + page: "/blog", + pageviews: 5, + visitors: 3, + date: ~D[2023-01-01] + ), + build(:imported_pages, + page: "/blog/post/1", + pageviews: 2, + visitors: 2, + date: ~D[2023-01-01] + ) + ]) + + conn = + post(conn, "/api/v2/query", %{ + "site_id" => site.domain, + "date_range" => "all", + "metrics" => ["visitors", "pageviews"], + "filters" => [ + ["is", "event:page", ["/blog"]] + ], + "include" => %{"imports" => true} + }) + + assert json_response(conn, 200)["results"] == [ + %{"metrics" => [5, 7], "dimensions" => []} + ] + end + + test "filters correctly with 'is' operator (case insensitive)", %{ + conn: conn, + site: site, + site_import: site_import + } do + populate_stats(site, site_import.id, [ + build(:pageview, pathname: "/BLOG", timestamp: ~N[2023-01-01 00:00:00]), + build(:pageview, pathname: "/blog", timestamp: ~N[2023-01-01 00:00:00]), + build(:pageview, pathname: "/blog/post/1", timestamp: ~N[2023-01-01 00:00:00]), + build(:pageview, pathname: "/about", timestamp: ~N[2023-01-01 00:00:00]), + build(:imported_pages, + page: "/BLOG", + pageviews: 5, + visitors: 3, + date: ~D[2023-01-01] + ), + build(:imported_pages, + page: "/blog/post/1", + pageviews: 2, + visitors: 2, + date: ~D[2023-01-01] + ) + ]) + + conn = + post(conn, "/api/v2/query", %{ + "site_id" => site.domain, + "date_range" => "all", + "metrics" => ["visitors", "pageviews"], + "filters" => [ + ["is", "event:page", ["/blOG"], %{"case_sensitive" => false}] + ], + "include" => %{"imports" => true} + }) + + assert json_response(conn, 200)["results"] == [ + %{"metrics" => [5, 7], "dimensions" => []} + ] + end + + test "filters correctly with 'contains' operator", %{ + conn: conn, + site: site, + site_import: site_import + } do + populate_stats(site, site_import.id, [ + build(:pageview, pathname: "/blog", timestamp: ~N[2023-01-01 00:00:00]), + build(:pageview, pathname: "/blog/post/1", timestamp: ~N[2023-01-01 00:00:00]), + build(:pageview, pathname: "/blog/post/2", timestamp: ~N[2023-01-01 00:00:00]), + build(:pageview, pathname: "/about", timestamp: ~N[2023-01-01 00:00:00]), + build(:imported_pages, + page: "/blog", + pageviews: 5, + visitors: 3, + date: ~D[2023-01-01] + ), + build(:imported_pages, + page: "/blog/post/1", + pageviews: 2, + visitors: 2, + date: ~D[2023-01-01] + ), + build(:imported_pages, + page: "/blog/POST/2", + pageviews: 3, + visitors: 1, + date: ~D[2023-01-01] + ) + ]) + + conn = + post(conn, "/api/v2/query", %{ + "site_id" => site.domain, + "date_range" => "all", + "metrics" => ["visitors", "pageviews"], + "filters" => [ + ["contains", "event:page", ["blog/post"]] + ], + "include" => %{"imports" => true} + }) + + assert json_response(conn, 200)["results"] == [ + %{"metrics" => [4, 4], "dimensions" => []} + ] + end + + test "filters correctly with 'contains' operator (case insensitive)", %{ + conn: conn, + site: site, + site_import: site_import + } do + populate_stats(site, site_import.id, [ + build(:pageview, pathname: "/BLOG/post/1", timestamp: ~N[2023-01-01 00:00:00]), + build(:pageview, pathname: "/blog/POST/2", timestamp: ~N[2023-01-01 00:00:00]), + build(:pageview, pathname: "/about", timestamp: ~N[2023-01-01 00:00:00]), + build(:imported_pages, + page: "/BLOG/POST/1", + pageviews: 5, + visitors: 3, + date: ~D[2023-01-01] + ), + build(:imported_pages, + page: "/blog/post/2", + pageviews: 2, + visitors: 2, + date: ~D[2023-01-01] + ) + ]) + + conn = + post(conn, "/api/v2/query", %{ + "site_id" => site.domain, + "date_range" => "all", + "metrics" => ["visitors", "pageviews"], + "filters" => [ + ["contains", "event:page", ["blog/POST"], %{"case_sensitive" => false}] + ], + "include" => %{"imports" => true} + }) + + assert json_response(conn, 200)["results"] == [ + %{"metrics" => [7, 9], "dimensions" => []} + ] + end + + test "aggregates custom event goals with 'is' and 'contains' operators", %{ + conn: conn, + site: site, + site_import: site_import + } do + insert(:goal, event_name: "Purchase", site: site) + + populate_stats(site, site_import.id, [ + build(:event, + name: "Purchase", + timestamp: ~N[2023-01-01 00:00:00] + ), + build(:event, + name: "Purchase", + timestamp: ~N[2023-01-01 00:00:00] + ), + build(:event, + name: "Signup", + timestamp: ~N[2023-01-01 00:00:00] + ), + build(:imported_custom_events, + name: "Purchase", + visitors: 3, + events: 5, + date: ~D[2023-01-01] + ), + build(:imported_custom_events, + name: "Signup", + visitors: 2, + events: 2, + date: ~D[2023-01-01] + ) + ]) + + conn = + post(conn, "/api/v2/query", %{ + "site_id" => site.domain, + "date_range" => "all", + "metrics" => ["visitors", "events"], + "filters" => [ + ["is", "event:goal", ["Purchase"]] + ], + "include" => %{"imports" => true} + }) + + assert json_response(conn, 200)["results"] == [ + %{"dimensions" => [], "metrics" => [5, 7]} + ] + + conn = + post(conn, "/api/v2/query", %{ + "site_id" => site.domain, + "date_range" => "all", + "metrics" => ["visitors", "events"], + "filters" => [ + ["contains", "event:goal", ["Purch", "sign"]] + ], + "include" => %{"imports" => true} + }) + + assert json_response(conn, 200)["results"] == [ + %{"dimensions" => [], "metrics" => [5, 7]} + ] + end + + test "aggregates custom event goals with 'is' and 'contains' operators (case insensitive)", %{ + conn: conn, + site: site, + site_import: site_import + } do + insert(:goal, event_name: "Purchase", site: site) + + populate_stats(site, site_import.id, [ + build(:event, + name: "Purchase", + timestamp: ~N[2023-01-01 00:00:00] + ), + build(:event, + name: "Purchase", + timestamp: ~N[2023-01-01 00:00:00] + ), + build(:event, + name: "Signup", + timestamp: ~N[2023-01-01 00:00:00] + ), + build(:imported_custom_events, + name: "Purchase", + visitors: 3, + events: 5, + date: ~D[2023-01-01] + ), + build(:imported_custom_events, + name: "Signup", + visitors: 2, + events: 2, + date: ~D[2023-01-01] + ) + ]) + + conn = + post(conn, "/api/v2/query", %{ + "site_id" => site.domain, + "date_range" => "all", + "metrics" => ["visitors", "events"], + "filters" => [ + ["is", "event:goal", ["purchase"], %{"case_sensitive" => false}] + ], + "include" => %{"imports" => true} + }) + + assert json_response(conn, 200)["results"] == [ + %{"dimensions" => [], "metrics" => [5, 7]} + ] + + conn = + post(conn, "/api/v2/query", %{ + "site_id" => site.domain, + "date_range" => "all", + "metrics" => ["visitors", "events"], + "filters" => [ + ["contains", "event:goal", ["PURCH"], %{"case_sensitive" => false}] + ], + "include" => %{"imports" => true} + }) + + assert json_response(conn, 200)["results"] == [ + %{"dimensions" => [], "metrics" => [5, 7]} + ] + end end test "breaks down all metrics by visit:referrer with imported data", %{conn: conn, site: site} do diff --git a/test/plausible_web/controllers/api/external_stats_controller/query_test.exs b/test/plausible_web/controllers/api/external_stats_controller/query_test.exs index 087004aa72b6..e3d1df31e815 100644 --- a/test/plausible_web/controllers/api/external_stats_controller/query_test.exs +++ b/test/plausible_web/controllers/api/external_stats_controller/query_test.exs @@ -271,6 +271,34 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryTest do ] end + test "can filter by utm_medium case insensitively", %{conn: conn, site: site} do + populate_stats(site, [ + build(:pageview, + utm_medium: "Social", + user_id: @user_id, + timestamp: ~N[2021-01-01 00:00:00] + ), + build(:pageview, + utm_medium: "SOCIAL", + user_id: @user_id, + timestamp: ~N[2021-01-01 00:25:00] + ), + build(:pageview, timestamp: ~N[2021-01-01 00:00:00]) + ]) + + conn = + post(conn, "/api/v2/query", %{ + "site_id" => site.domain, + "date_range" => "all", + "metrics" => ["pageviews", "visitors", "bounce_rate", "visit_duration"], + "filters" => [["is", "visit:utm_medium", ["sociaL"], %{"case_sensitive" => false}]] + }) + + assert json_response(conn, 200)["results"] == [ + %{"metrics" => [2, 1, 0, 1500], "dimensions" => []} + ] + end + test "can filter by utm_source", %{conn: conn, site: site} do populate_stats(site, [ build(:pageview, @@ -621,6 +649,43 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryTest do assert json_response(conn, 200)["results"] == [%{"metrics" => [2, 3], "dimensions" => []}] end + test "filtering by a custom event goal (case insensitive)", %{conn: conn, site: site} do + populate_stats(site, [ + build(:event, + name: "Signup", + timestamp: ~N[2021-01-01 00:00:00] + ), + build(:event, + name: "Signup", + user_id: @user_id, + timestamp: ~N[2021-01-01 00:25:00] + ), + build(:event, + name: "Signup", + user_id: @user_id, + timestamp: ~N[2021-01-01 00:25:00] + ), + build(:event, + name: "NotConfigured", + timestamp: ~N[2021-01-01 00:25:00] + ) + ]) + + insert(:goal, %{site: site, event_name: "Signup"}) + + conn = + post(conn, "/api/v2/query", %{ + "site_id" => site.domain, + "date_range" => "all", + "metrics" => ["visitors", "events"], + "filters" => [ + ["is", "event:goal", ["signup"], %{"case_sensitive" => false}] + ] + }) + + assert json_response(conn, 200)["results"] == [%{"metrics" => [2, 3], "dimensions" => []}] + end + test "filtering by a revenue goal", %{conn: conn, site: site} do populate_stats(site, [ build(:event, @@ -819,6 +884,26 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryTest do assert json_response(conn, 200)["results"] == [%{"metrics" => [2], "dimensions" => []}] end + test "contains page filter case insensitive", %{conn: conn, site: site} do + populate_stats(site, [ + build(:pageview, pathname: "/en/page1"), + build(:pageview, pathname: "/EN/page2"), + build(:pageview, pathname: "/pl/page1") + ]) + + conn = + post(conn, "/api/v2/query", %{ + "site_id" => site.domain, + "date_range" => "all", + "metrics" => ["visitors"], + "filters" => [ + ["contains", "event:page", ["/En/"], %{"case_sensitive" => false}] + ] + }) + + assert json_response(conn, 200)["results"] == [%{"metrics" => [2], "dimensions" => []}] + end + test "contains_not page filter", %{conn: conn, site: site} do populate_stats(site, [ build(:pageview, pathname: "/en/page1"), @@ -1009,6 +1094,47 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryTest do assert json_response(conn, 200)["results"] == [%{"metrics" => [1], "dimensions" => []}] end + test "`contains` operator with custom properties case insensitive", %{ + conn: conn, + site: site + } do + populate_stats(site, [ + build(:pageview, + "meta.key": ["name"], + "meta.value": ["large-1"] + ), + build(:pageview, + "meta.key": ["name"], + "meta.value": ["Small-1"] + ), + build(:pageview, + "meta.key": ["name"], + "meta.value": ["mall-1"] + ), + build(:pageview, + "meta.key": ["name"], + "meta.value": ["SMALL-2"] + ), + build(:pageview, + "meta.key": ["name"], + "meta.value": ["small-2"] + ), + build(:pageview) + ]) + + conn = + post(conn, "/api/v2/query", %{ + "site_id" => site.domain, + "date_range" => "all", + "metrics" => ["visitors"], + "filters" => [ + ["contains", "event:props:name", ["maLL"], %{"case_sensitive" => false}] + ] + }) + + assert json_response(conn, 200)["results"] == [%{"metrics" => [4], "dimensions" => []}] + end + test "`matches` and `matches_not` operator with custom properties", %{ conn: conn, site: site @@ -2587,6 +2713,39 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryTest do ] end + test "goal contains filter for goal breakdown (case insensitive)", %{conn: conn, site: site} do + populate_stats(site, [ + build(:event, name: "Onboarding conversion: Step 1"), + build(:event, name: "Onboarding conversion: Step 1"), + build(:event, name: "Onboarding conversion: Step 2"), + build(:event, name: "Unrelated"), + build(:pageview, pathname: "/conversion") + ]) + + insert(:goal, site: site, event_name: "Onboarding conversion: Step 1") + insert(:goal, site: site, event_name: "Onboarding conversion: Step 2") + insert(:goal, site: site, event_name: "Unrelated") + insert(:goal, site: site, page_path: "/conversion") + + conn = + post(conn, "/api/v2/query", %{ + "site_id" => site.domain, + "metrics" => ["visitors"], + "date_range" => "all", + "dimensions" => ["event:goal"], + "filters" => [ + ["contains", "event:goal", ["step"], %{"case_sensitive" => false}] + ] + }) + + %{"results" => results} = json_response(conn, 200) + + assert results == [ + %{"dimensions" => ["Onboarding conversion: Step 1"], "metrics" => [2]}, + %{"dimensions" => ["Onboarding conversion: Step 2"], "metrics" => [1]} + ] + end + test "mixed multi-goal filter for breakdown by visit:country", %{conn: conn, site: site} do populate_stats(site, [ build(:pageview, country_code: "EE", pathname: "/en/register"), @@ -2813,6 +2972,43 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryTest do ] end + test "IN filter for event:name case insensitive", %{conn: conn, site: site} do + populate_stats(site, [ + build(:event, + name: "Signup", + timestamp: ~N[2021-01-01 00:00:00] + ), + build(:event, + name: "Signup", + timestamp: ~N[2021-01-01 00:00:00] + ), + build(:event, + name: "Login", + timestamp: ~N[2021-01-01 00:00:00] + ), + build(:event, + name: "Irrelevant", + timestamp: ~N[2021-01-01 00:00:00] + ) + ]) + + conn = + post(conn, "/api/v2/query", %{ + "site_id" => site.domain, + "metrics" => ["visitors"], + "date_range" => "all", + "filters" => [ + ["is", "event:name", ["signup", "LOGIN"], %{"case_sensitive" => false}] + ] + }) + + %{"results" => results} = json_response(conn, 200) + + assert results == [ + %{"dimensions" => [], "metrics" => [3]} + ] + end + test "IN filter for event:props:*", %{conn: conn, site: site} do populate_stats(site, [ build(:pageview, @@ -2895,7 +3091,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryTest do "date_range" => "all", "dimensions" => ["visit:browser"], "filters" => [ - ["is", "event:props:browser", ["Chrome", "Safari"]], + ["is", "event:props:browser", ["CHROME", "sAFari"], %{"case_sensitive" => false}], ["is", "event:props:prop", ["target_value"]] ] }) @@ -3720,4 +3916,32 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryTest do ] end end + + test "can filter by utm_medium case insensitively", %{conn: conn, site: site} do + populate_stats(site, [ + build(:pageview, + utm_medium: "Social", + user_id: @user_id, + timestamp: ~N[2021-01-01 00:00:00] + ), + build(:pageview, + utm_medium: "SOCIAL", + user_id: @user_id, + timestamp: ~N[2021-01-01 00:25:00] + ), + build(:pageview, timestamp: ~N[2021-01-01 00:00:00]) + ]) + + conn = + post(conn, "/api/v2/query", %{ + "site_id" => site.domain, + "date_range" => "all", + "metrics" => ["pageviews", "visitors", "bounce_rate", "visit_duration"], + "filters" => [["is", "visit:utm_medium", ["social"], %{"case_sensitive" => false}]] + }) + + assert json_response(conn, 200)["results"] == [ + %{"metrics" => [2, 1, 0, 1500], "dimensions" => []} + ] + end end From 8b237242c8346ac2ba4f59da044d70bc7ddbfbae Mon Sep 17 00:00:00 2001 From: Karl-Aksel Puulmann Date: Thu, 28 Nov 2024 16:37:35 +0200 Subject: [PATCH 06/15] Make modals send { case_sensitive: false } to backend for search --- assets/js/dashboard/stats/modals/conversions.js | 2 +- .../stats/modals/devices/browser-versions-modal.js | 2 +- assets/js/dashboard/stats/modals/devices/browsers-modal.js | 2 +- .../stats/modals/devices/operating-system-versions-modal.js | 2 +- .../stats/modals/devices/operating-systems-modal.js | 2 +- assets/js/dashboard/stats/modals/entry-pages.js | 2 +- assets/js/dashboard/stats/modals/exit-pages.js | 2 +- assets/js/dashboard/stats/modals/locations-modal.js | 2 +- assets/js/dashboard/stats/modals/pages.js | 6 +++--- assets/js/dashboard/stats/modals/props.js | 2 +- assets/js/dashboard/stats/modals/referrer-drilldown.js | 2 +- assets/js/dashboard/stats/modals/sources.js | 2 +- assets/js/dashboard/util/filters.js | 4 ++-- 13 files changed, 16 insertions(+), 16 deletions(-) diff --git a/assets/js/dashboard/stats/modals/conversions.js b/assets/js/dashboard/stats/modals/conversions.js index 3514bd852de8..cc1a00d94782 100644 --- a/assets/js/dashboard/stats/modals/conversions.js +++ b/assets/js/dashboard/stats/modals/conversions.js @@ -28,7 +28,7 @@ function ConversionsModal() { }, [reportInfo.dimension]) const addSearchFilter = useCallback((query, searchString) => { - return addFilter(query, ['contains', reportInfo.dimension, [searchString]]) + return addFilter(query, ['contains', reportInfo.dimension, [searchString], { case_sensitive: false }]) }, [reportInfo.dimension]) function chooseMetrics() { diff --git a/assets/js/dashboard/stats/modals/devices/browser-versions-modal.js b/assets/js/dashboard/stats/modals/devices/browser-versions-modal.js index 369b4dce9f9c..d0e11e299b97 100644 --- a/assets/js/dashboard/stats/modals/devices/browser-versions-modal.js +++ b/assets/js/dashboard/stats/modals/devices/browser-versions-modal.js @@ -29,7 +29,7 @@ function BrowserVersionsModal() { }, [reportInfo.dimension]) const addSearchFilter = useCallback((query, searchString) => { - return addFilter(query, ['contains', reportInfo.dimension, [searchString]]) + return addFilter(query, ['contains', reportInfo.dimension, [searchString], { case_sensitive: false }]) }, [reportInfo.dimension]) const renderIcon = useCallback((listItem) => browserIconFor(listItem.browser), []) diff --git a/assets/js/dashboard/stats/modals/devices/browsers-modal.js b/assets/js/dashboard/stats/modals/devices/browsers-modal.js index 1160b8ebf5f8..9b600514bd99 100644 --- a/assets/js/dashboard/stats/modals/devices/browsers-modal.js +++ b/assets/js/dashboard/stats/modals/devices/browsers-modal.js @@ -29,7 +29,7 @@ function BrowsersModal() { }, [reportInfo.dimension]) const addSearchFilter = useCallback((query, searchString) => { - return addFilter(query, ['contains', reportInfo.dimension, [searchString]]) + return addFilter(query, ['contains', reportInfo.dimension, [searchString], { case_sensitive: false }]) }, [reportInfo.dimension]) const renderIcon = useCallback((listItem) => browserIconFor(listItem.name), []) diff --git a/assets/js/dashboard/stats/modals/devices/operating-system-versions-modal.js b/assets/js/dashboard/stats/modals/devices/operating-system-versions-modal.js index 6c7d5c72cc61..9c5622b48ded 100644 --- a/assets/js/dashboard/stats/modals/devices/operating-system-versions-modal.js +++ b/assets/js/dashboard/stats/modals/devices/operating-system-versions-modal.js @@ -29,7 +29,7 @@ function OperatingSystemVersionsModal() { }, [reportInfo.dimension]) const addSearchFilter = useCallback((query, searchString) => { - return addFilter(query, ['contains', reportInfo.dimension, [searchString]]) + return addFilter(query, ['contains', reportInfo.dimension, [searchString], { case_sensitive: false }]) }, [reportInfo.dimension]) const renderIcon = useCallback((listItem) => osIconFor(listItem.os), []) diff --git a/assets/js/dashboard/stats/modals/devices/operating-systems-modal.js b/assets/js/dashboard/stats/modals/devices/operating-systems-modal.js index 2a92795648ad..9df8a6c7c7e4 100644 --- a/assets/js/dashboard/stats/modals/devices/operating-systems-modal.js +++ b/assets/js/dashboard/stats/modals/devices/operating-systems-modal.js @@ -29,7 +29,7 @@ function OperatingSystemsModal() { }, [reportInfo.dimension]) const addSearchFilter = useCallback((query, searchString) => { - return addFilter(query, ['contains', reportInfo.dimension, [searchString]]) + return addFilter(query, ['contains', reportInfo.dimension, [searchString], { case_sensitive: false }]) }, [reportInfo.dimension]) const renderIcon = useCallback((listItem) => osIconFor(listItem.name), []) diff --git a/assets/js/dashboard/stats/modals/entry-pages.js b/assets/js/dashboard/stats/modals/entry-pages.js index 6d5a72d693ad..79963046fb64 100644 --- a/assets/js/dashboard/stats/modals/entry-pages.js +++ b/assets/js/dashboard/stats/modals/entry-pages.js @@ -29,7 +29,7 @@ function EntryPagesModal() { }, [reportInfo.dimension]) const addSearchFilter = useCallback((query, searchString) => { - return addFilter(query, ['contains', reportInfo.dimension, [searchString]]) + return addFilter(query, ['contains', reportInfo.dimension, [searchString], { case_sensitive: false }]) }, [reportInfo.dimension]) function chooseMetrics() { diff --git a/assets/js/dashboard/stats/modals/exit-pages.js b/assets/js/dashboard/stats/modals/exit-pages.js index 2be05f016868..413c89056877 100644 --- a/assets/js/dashboard/stats/modals/exit-pages.js +++ b/assets/js/dashboard/stats/modals/exit-pages.js @@ -29,7 +29,7 @@ function ExitPagesModal() { }, [reportInfo.dimension]) const addSearchFilter = useCallback((query, searchString) => { - return addFilter(query, ['contains', reportInfo.dimension, [searchString]]) + return addFilter(query, ['contains', reportInfo.dimension, [searchString], { case_sensitive: false }]) }, [reportInfo.dimension]) function chooseMetrics() { diff --git a/assets/js/dashboard/stats/modals/locations-modal.js b/assets/js/dashboard/stats/modals/locations-modal.js index 615f6e9e3d42..bfdd471b9720 100644 --- a/assets/js/dashboard/stats/modals/locations-modal.js +++ b/assets/js/dashboard/stats/modals/locations-modal.js @@ -32,7 +32,7 @@ function LocationsModal({ currentView }) { }, [reportInfo.dimension]) const addSearchFilter = useCallback((query, searchString) => { - return addFilter(query, ['contains', `${reportInfo.dimension}_name`, [searchString]]) + return addFilter(query, ['contains', `${reportInfo.dimension}_name`, [searchString], { case_sensitive: false }]) }, [reportInfo.dimension]) function chooseMetrics() { diff --git a/assets/js/dashboard/stats/modals/pages.js b/assets/js/dashboard/stats/modals/pages.js index 26bf287c6cb1..e9521f9774be 100644 --- a/assets/js/dashboard/stats/modals/pages.js +++ b/assets/js/dashboard/stats/modals/pages.js @@ -29,7 +29,7 @@ function PagesModal() { }, [reportInfo.dimension]) const addSearchFilter = useCallback((query, searchString) => { - return addFilter(query, ['contains', reportInfo.dimension, [searchString]]) + return addFilter(query, ['contains', reportInfo.dimension, [searchString], { case_sensitive: false }]) }, [reportInfo.dimension]) function chooseMetrics() { @@ -46,14 +46,14 @@ function PagesModal() { metrics.createVisitors({renderLabel: (_query) => 'Current visitors', width: 'w-36'}) ] } - + const defaultMetrics = [ metrics.createVisitors({renderLabel: (_query) => "Visitors" }), metrics.createPageviews(), metrics.createBounceRate(), metrics.createTimeOnPage() ] - + return site.flags.scroll_depth ? [...defaultMetrics, metrics.createScrollDepth()] : defaultMetrics } diff --git a/assets/js/dashboard/stats/modals/props.js b/assets/js/dashboard/stats/modals/props.js index 0d691abe8d59..a131e8a43837 100644 --- a/assets/js/dashboard/stats/modals/props.js +++ b/assets/js/dashboard/stats/modals/props.js @@ -36,7 +36,7 @@ function PropsModal() { }, [propKey]) const addSearchFilter = useCallback((query, searchString) => { - return addFilter(query, ['contains', `${EVENT_PROPS_PREFIX}${propKey}`, [searchString]]) + return addFilter(query, ['contains', `${EVENT_PROPS_PREFIX}${propKey}`, [searchString], { case_sensitive: false }]) }, [propKey]) function chooseMetrics() { diff --git a/assets/js/dashboard/stats/modals/referrer-drilldown.js b/assets/js/dashboard/stats/modals/referrer-drilldown.js index 6077bb737e2b..264e1d54547b 100644 --- a/assets/js/dashboard/stats/modals/referrer-drilldown.js +++ b/assets/js/dashboard/stats/modals/referrer-drilldown.js @@ -33,7 +33,7 @@ function ReferrerDrilldownModal() { }, [reportInfo.dimension]) const addSearchFilter = useCallback((query, searchString) => { - return addFilter(query, ['contains', reportInfo.dimension, [searchString]]) + return addFilter(query, ['contains', reportInfo.dimension, [searchString], { case_sensitive: false }]) }, [reportInfo.dimension]) function chooseMetrics() { diff --git a/assets/js/dashboard/stats/modals/sources.js b/assets/js/dashboard/stats/modals/sources.js index d4be4b30b951..3fd0006c71d1 100644 --- a/assets/js/dashboard/stats/modals/sources.js +++ b/assets/js/dashboard/stats/modals/sources.js @@ -57,7 +57,7 @@ function SourcesModal({ currentView }) { }, [reportInfo.dimension]) const addSearchFilter = useCallback((query, searchString) => { - return addFilter(query, ['contains', reportInfo.dimension, [searchString]]) + return addFilter(query, ['contains', reportInfo.dimension, [searchString], { case_sensitive: false }]) }, [reportInfo.dimension]) function chooseMetrics() { diff --git a/assets/js/dashboard/util/filters.js b/assets/js/dashboard/util/filters.js index d8d13a70dab5..c136cd330697 100644 --- a/assets/js/dashboard/util/filters.js +++ b/assets/js/dashboard/util/filters.js @@ -229,7 +229,7 @@ export function cleanLabels(filters, labels, mergedFilterKey, mergedLabels) { const EVENT_FILTER_KEYS = new Set(['name', 'page', 'goal', 'hostname']) export function serializeApiFilters(filters) { - const apiFilters = filters.map(([operation, filterKey, clauses]) => { + const apiFilters = filters.map(([operation, filterKey, clauses, ...modifiers]) => { let apiFilterKey = `visit:${filterKey}` if ( filterKey.startsWith(EVENT_PROPS_PREFIX) || @@ -237,7 +237,7 @@ export function serializeApiFilters(filters) { ) { apiFilterKey = `event:${filterKey}` } - return [operation, apiFilterKey, clauses] + return [operation, apiFilterKey, clauses, ...modifiers] }) return JSON.stringify(apiFilters) From ee1949d050cac4cc09391c8e03b8f4ce5ad5b412 Mon Sep 17 00:00:00 2001 From: Karl-Aksel Puulmann Date: Thu, 28 Nov 2024 16:38:58 +0200 Subject: [PATCH 07/15] CHANGELOG.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc0f3056f8bb..00bb32b5c9a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,9 +6,13 @@ All notable changes to this project will be documented in this file. ### Added - Dashboard shows comparisons for all reports - UTM Medium report and API shows (gclid) and (msclkid) for paid searches when no explicit utm medium present. +- Support for `case_sensitive: false` modifiers in Stats API V2 filters for case-insensitive searches. ### Removed ### Changed + +- Details modal search inputs are now case-insensitive. + ### Fixed - Fix returning filter suggestions for multiple custom property values in the dashboard Filter modal From 3cd1d12c82d8cf64854b485a6ff859996e512a81 Mon Sep 17 00:00:00 2001 From: Karl-Aksel Puulmann Date: Thu, 28 Nov 2024 16:55:34 +0200 Subject: [PATCH 08/15] Typegen --- assets/js/types/query-api.d.ts | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/assets/js/types/query-api.d.ts b/assets/js/types/query-api.d.ts index 06121a8166e3..5b48a4e69f85 100644 --- a/assets/js/types/query-api.d.ts +++ b/assets/js/types/query-api.d.ts @@ -64,7 +64,7 @@ export type CustomPropertyFilterDimensions = string; export type GoalDimension = "event:goal"; export type TimeDimensions = "time" | "time:month" | "time:week" | "time:day" | "time:hour"; export type FilterTree = FilterEntry | FilterAndOr | FilterNot; -export type FilterEntry = FilterWithoutGoals | FilterWithGoals; +export type FilterEntry = FilterWithoutGoals | FilterCaseSensitive; /** * @minItems 3 * @maxItems 3 @@ -81,17 +81,22 @@ export type FilterOperationWithoutGoals = "is_not" | "contains_not" | "matches" export type Clauses = (string | number)[]; /** * @minItems 3 - * @maxItems 3 + * @maxItems 4 */ -export type FilterWithGoals = [ - FilterOperationWithGoals, - GoalDimension | SimpleFilterDimensions | CustomPropertyFilterDimensions, - Clauses -]; +export type FilterCaseSensitive = + | [FilterOperationContains, GoalDimension | SimpleFilterDimensions | CustomPropertyFilterDimensions, Clauses] + | [ + FilterOperationContains, + GoalDimension | SimpleFilterDimensions | CustomPropertyFilterDimensions, + Clauses, + { + case_sensitive?: boolean; + } + ]; /** * filter operation */ -export type FilterOperationWithGoals = "is" | "contains"; +export type FilterOperationContains = "is" | "contains"; /** * @minItems 2 * @maxItems 2 From 972d84f1185f1474419102e875c92e3ef2a98297 Mon Sep 17 00:00:00 2001 From: Karl-Aksel Puulmann Date: Thu, 28 Nov 2024 16:59:39 +0200 Subject: [PATCH 09/15] Prettier --- assets/js/dashboard/util/filters.js | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/assets/js/dashboard/util/filters.js b/assets/js/dashboard/util/filters.js index c136cd330697..12e9a60c604e 100644 --- a/assets/js/dashboard/util/filters.js +++ b/assets/js/dashboard/util/filters.js @@ -229,16 +229,18 @@ export function cleanLabels(filters, labels, mergedFilterKey, mergedLabels) { const EVENT_FILTER_KEYS = new Set(['name', 'page', 'goal', 'hostname']) export function serializeApiFilters(filters) { - const apiFilters = filters.map(([operation, filterKey, clauses, ...modifiers]) => { - let apiFilterKey = `visit:${filterKey}` - if ( - filterKey.startsWith(EVENT_PROPS_PREFIX) || - EVENT_FILTER_KEYS.has(filterKey) - ) { - apiFilterKey = `event:${filterKey}` + const apiFilters = filters.map( + ([operation, filterKey, clauses, ...modifiers]) => { + let apiFilterKey = `visit:${filterKey}` + if ( + filterKey.startsWith(EVENT_PROPS_PREFIX) || + EVENT_FILTER_KEYS.has(filterKey) + ) { + apiFilterKey = `event:${filterKey}` + } + return [operation, apiFilterKey, clauses, ...modifiers] } - return [operation, apiFilterKey, clauses, ...modifiers] - }) + ) return JSON.stringify(apiFilters) } From 1c00cade2fefc40ca0273107c1e8b045743d754c Mon Sep 17 00:00:00 2001 From: Karl-Aksel Puulmann Date: Fri, 29 Nov 2024 16:06:27 +0200 Subject: [PATCH 10/15] Refactor: more DRY where_builder for case sensitivity --- lib/plausible/stats/sql/where_builder.ex | 107 ++++++++++------------- 1 file changed, 47 insertions(+), 60 deletions(-) diff --git a/lib/plausible/stats/sql/where_builder.ex b/lib/plausible/stats/sql/where_builder.ex index 821bfd82a7bf..4da668c0e203 100644 --- a/lib/plausible/stats/sql/where_builder.ex +++ b/lib/plausible/stats/sql/where_builder.ex @@ -93,13 +93,8 @@ defmodule Plausible.Stats.SQL.WhereBuilder do |> Enum.reduce(fn condition, acc -> dynamic([], ^acc or ^condition) end) end - defp add_filter(:events, _query, [:is, "event:name", clauses, %{case_sensitive: false}]) do - clauses = Enum.map(clauses, &String.downcase/1) - dynamic([e], fragment("lower(?)", e.name) in ^clauses) - end - - defp add_filter(:events, _query, [:is, "event:name", clauses]) do - dynamic([e], e.name in ^clauses) + defp add_filter(:events, _query, [:is, "event:name" | _rest] = filter) do + is_in_clause(col_value(:name), filter) end defp add_filter(:events, query, [_, "event:goal" | _rest] = filter) do @@ -159,24 +154,13 @@ defmodule Plausible.Stats.SQL.WhereBuilder do false end - defp filter_custom_prop(prop_name, column_name, [:is, _, clauses, %{case_sensitive: false}]) do - clauses = clauses |> Enum.map(&String.downcase/1) - none_value_included = Enum.member?(clauses, "(none)") - - dynamic( - [t], - (has_key(t, column_name, ^prop_name) and - fragment("lower(?)", get_by_key(t, column_name, ^prop_name)) in ^clauses) or - (^none_value_included and not has_key(t, column_name, ^prop_name)) - ) - end - - defp filter_custom_prop(prop_name, column_name, [:is, _, clauses | _rest]) do + defp filter_custom_prop(prop_name, column_name, [:is, _, clauses | _rest] = filter) do none_value_included = Enum.member?(clauses, "(none)") + prop_value_expr = custom_prop_value(column_name, prop_name) dynamic( [t], - (has_key(t, column_name, ^prop_name) and get_by_key(t, column_name, ^prop_name) in ^clauses) or + (has_key(t, column_name, ^prop_name) and ^is_in_clause(prop_value_expr, filter)) or (^none_value_included and not has_key(t, column_name, ^prop_name)) ) end @@ -235,32 +219,11 @@ defmodule Plausible.Stats.SQL.WhereBuilder do ) end - defp filter_custom_prop(prop_name, column_name, [ - :contains, - _dimension, - clauses, - %{case_sensitive: false} - ]) do - dynamic( - [t], - has_key(t, column_name, ^prop_name) and - fragment( - "multiSearchAnyCaseInsensitive(?, ?)", - get_by_key(t, column_name, ^prop_name), - ^clauses - ) - ) - end - - defp filter_custom_prop(prop_name, column_name, [:contains, _dimension, clauses | _rest]) do + defp filter_custom_prop(prop_name, column_name, [:contains | _rest] = filter) do dynamic( [t], has_key(t, column_name, ^prop_name) and - fragment( - "multiSearchAny(?, ?)", - get_by_key(t, column_name, ^prop_name), - ^clauses - ) + ^contains_clause(custom_prop_value(column_name, prop_name), filter) ) end @@ -289,15 +252,8 @@ defmodule Plausible.Stats.SQL.WhereBuilder do dynamic([], not (^filter_field(db_field, [:matches_wildcard | rest]))) end - defp filter_field(db_field, [:contains, _dimension, values, %{case_sensitive: false}]) do - dynamic( - [x], - fragment("multiSearchAnyCaseInsensitive(?, ?)", type(field(x, ^db_field), :string), ^values) - ) - end - - defp filter_field(db_field, [:contains, _dimension, values | _rest]) do - dynamic([x], fragment("multiSearchAny(?, ?)", type(field(x, ^db_field), :string), ^values)) + defp filter_field(db_field, [:contains | _rest] = filter) do + contains_clause(col_value(db_field), filter) end defp filter_field(db_field, [:contains_not | rest]) do @@ -315,14 +271,9 @@ defmodule Plausible.Stats.SQL.WhereBuilder do dynamic([], not (^filter_field(db_field, [:matches | rest]))) end - defp filter_field(db_field, [:is, _dimension, clauses, %{case_sensitive: false}]) do - list = clauses |> Enum.map(&db_field_val(db_field, &1)) |> Enum.map(&String.downcase/1) - dynamic([x], fragment("lower(?)", field(x, ^db_field)) in ^list) - end - - defp filter_field(db_field, [:is, _dimension, clauses | _rest]) do + defp filter_field(db_field, [:is, _dimension, clauses | _rest] = filter) do list = clauses |> Enum.map(&db_field_val(db_field, &1)) - dynamic([x], field(x, ^db_field) in ^list) + is_in_clause(col_value(db_field), filter, list) end defp filter_field(db_field, [:is_not | rest]) do @@ -344,4 +295,40 @@ defmodule Plausible.Stats.SQL.WhereBuilder do defp db_field_val(:utm_term, @no_ref), do: "" defp db_field_val(_, @not_set), do: "" defp db_field_val(_, val), do: val + + def col_value(column_name) do + dynamic([t], type(field(t, ^column_name), :string)) + end + + def custom_prop_value(column_name, prop_name) do + dynamic([t], get_by_key(t, column_name, ^prop_name)) + end + + def is_in_clause(value_expression, [_, _, clauses | _] = filter, values \\ nil) do + values = values || clauses + + if case_sensitive?(filter) do + dynamic([t], ^value_expression in ^values) + else + values = values |> Enum.map(&String.downcase/1) + dynamic([t], fragment("lower(?)", ^value_expression) in ^values) + end + end + + def contains_clause(value_expression, [_, _, clauses | _] = filter) do + if case_sensitive?(filter) do + dynamic( + [x], + fragment("multiSearchAny(?, ?)", ^value_expression, ^clauses) + ) + else + dynamic( + [x], + fragment("multiSearchAnyCaseInsensitive(?, ?)", ^value_expression, ^clauses) + ) + end + end + + defp case_sensitive?([_, _, _, %{case_sensitive: false}]), do: false + defp case_sensitive?(_), do: true end From d2434f8518bd8bc077cc5d4efcdd37d3e7b31174 Mon Sep 17 00:00:00 2001 From: Karl-Aksel Puulmann Date: Fri, 29 Nov 2024 16:25:35 +0200 Subject: [PATCH 11/15] Support case_sensitive modifier for is_not/contains_not --- lib/plausible/stats/sql/where_builder.ex | 15 ++- priv/json-schemas/query-api-schema.json | 42 +++++++- test/plausible/stats/query_parser_test.exs | 36 ++++--- .../external_stats_controller/query_test.exs | 96 +++++++++++++++++++ 4 files changed, 161 insertions(+), 28 deletions(-) diff --git a/lib/plausible/stats/sql/where_builder.ex b/lib/plausible/stats/sql/where_builder.ex index 4da668c0e203..b3011f36e4b9 100644 --- a/lib/plausible/stats/sql/where_builder.ex +++ b/lib/plausible/stats/sql/where_builder.ex @@ -165,16 +165,17 @@ defmodule Plausible.Stats.SQL.WhereBuilder do ) end - defp filter_custom_prop(prop_name, column_name, [:is_not, _, clauses | _rest]) do + defp filter_custom_prop(prop_name, column_name, [:is_not, _, clauses | _rest] = filter) do none_value_included = Enum.member?(clauses, "(none)") + prop_value_expr = custom_prop_value(column_name, prop_name) dynamic( [t], (has_key(t, column_name, ^prop_name) and - get_by_key(t, column_name, ^prop_name) not in ^clauses) or + not (^is_in_clause(prop_value_expr, filter))) or (^none_value_included and has_key(t, column_name, ^prop_name) and - get_by_key(t, column_name, ^prop_name) not in ^clauses) or + not (^is_in_clause(prop_value_expr, filter))) or (not (^none_value_included) and not has_key(t, column_name, ^prop_name)) ) end @@ -227,15 +228,11 @@ defmodule Plausible.Stats.SQL.WhereBuilder do ) end - defp filter_custom_prop(prop_name, column_name, [:contains_not, _dimension, clauses | _rest]) do + defp filter_custom_prop(prop_name, column_name, [:contains_not | _] = filter) do dynamic( [t], has_key(t, column_name, ^prop_name) and - fragment( - "not(multiSearchAny(?, ?))", - get_by_key(t, column_name, ^prop_name), - ^clauses - ) + not (^contains_clause(custom_prop_value(column_name, prop_name), filter)) ) end diff --git a/priv/json-schemas/query-api-schema.json b/priv/json-schemas/query-api-schema.json index cdcc3bf03c8c..593544d1ecc7 100644 --- a/priv/json-schemas/query-api-schema.json +++ b/priv/json-schemas/query-api-schema.json @@ -341,9 +341,14 @@ "enum": ["matches_wildcard", "matches_wildcard_not"], "description": "filter operation" }, + "filter_operation_regex": { + "type": "string", + "enum": ["matches", "matches_not"], + "description": "filter operation" + }, "filter_operation_without_goals": { "type": "string", - "enum": ["is_not", "contains_not", "matches", "matches_not"], + "enum": ["is_not", "contains_not"], "description": "filter operation" }, "filter_operation_contains": { @@ -351,7 +356,7 @@ "enum": ["is", "contains"], "description": "filter operation" }, - "filter_without_goals": { + "filter_with_pattern": { "type": "array", "additionalItems": false, "minItems": 3, @@ -359,7 +364,7 @@ "items": [ { "oneOf": [ - { "$ref": "#/definitions/filter_operation_without_goals" }, + { "$ref": "#/definitions/filter_operation_regex" }, { "$ref": "#/definitions/filter_operation_wildcard", "$comment": "only :internal" @@ -375,7 +380,33 @@ { "$ref": "#/definitions/clauses" } ] }, - "filter_case_sensitive": { + "filter_without_goals": { + "type": "array", + "additionalItems": false, + "minItems": 3, + "maxItems": 4, + "items": [ + { "$ref": "#/definitions/filter_operation_without_goals" }, + { + "oneOf": [ + { "$ref": "#/definitions/simple_filter_dimensions" }, + { "$ref": "#/definitions/custom_property_filter_dimensions" } + ] + }, + { "$ref": "#/definitions/clauses" }, + { + "type": "object", + "additionalProperties": false, + "properties": { + "case_sensitive": { + "type": "boolean", + "default": true + } + } + } + ] + }, + "filter_with_goals": { "type": "array", "additionalItems": false, "minItems": 3, @@ -409,7 +440,8 @@ "filter_entry": { "oneOf": [ { "$ref": "#/definitions/filter_without_goals" }, - { "$ref": "#/definitions/filter_case_sensitive" } + { "$ref": "#/definitions/filter_with_goals" }, + { "$ref": "#/definitions/filter_with_pattern" } ] }, "filter_tree": { diff --git a/test/plausible/stats/query_parser_test.exs b/test/plausible/stats/query_parser_test.exs index a92bc86fa878..e59dd28ddfa5 100644 --- a/test/plausible/stats/query_parser_test.exs +++ b/test/plausible/stats/query_parser_test.exs @@ -599,7 +599,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do ) end - for operation <- [:is, :contains] do + for operation <- [:is, :contains, :is_not, :contains_not] do test "#{operation} allows case_sensitive modifier", %{site: site} do %{ "site_id" => site.domain, @@ -636,19 +636,27 @@ defmodule Plausible.Stats.Filters.QueryParserTest do end end - test "case_sensitive modifier is not valid for other operations", %{site: site} do - %{ - "site_id" => site.domain, - "metrics" => ["visitors"], - "date_range" => "all", - "filters" => [ - ["is_not", "event:hostname", ["a.plausible.io"], %{"case_sensitive" => false}] - ] - } - |> check_error( - site, - "#/filters/0: Invalid filter [\"is_not\", \"event:hostname\", [\"a.plausible.io\"], %{\"case_sensitive\" => false}]" - ) + for operation <- [:matches, :matches_not, :matches_wildcard, :matches_wildcard_not] do + test "case_sensitive modifier is not valid for #{operation}", %{site: site} do + %{ + "site_id" => site.domain, + "metrics" => ["visitors"], + "date_range" => "all", + "filters" => [ + [ + Atom.to_string(unquote(operation)), + "event:hostname", + ["a.plausible.io"], + %{"case_sensitive" => false} + ] + ] + } + |> check_error( + site, + "#/filters/0: Invalid filter [\"#{unquote(operation)}\", \"event:hostname\", [\"a.plausible.io\"], %{\"case_sensitive\" => false}]", + :internal + ) + end end end diff --git a/test/plausible_web/controllers/api/external_stats_controller/query_test.exs b/test/plausible_web/controllers/api/external_stats_controller/query_test.exs index e3d1df31e815..451bcf7f6b35 100644 --- a/test/plausible_web/controllers/api/external_stats_controller/query_test.exs +++ b/test/plausible_web/controllers/api/external_stats_controller/query_test.exs @@ -299,6 +299,34 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryTest do ] end + test "can filter by is_not utm_medium case insensitively", %{conn: conn, site: site} do + populate_stats(site, [ + build(:pageview, + utm_medium: "Social", + user_id: @user_id, + timestamp: ~N[2021-01-01 00:00:00] + ), + build(:pageview, + utm_medium: "SOCIAL", + user_id: @user_id, + timestamp: ~N[2021-01-01 00:25:00] + ), + build(:pageview, timestamp: ~N[2021-01-01 00:00:00]) + ]) + + conn = + post(conn, "/api/v2/query", %{ + "site_id" => site.domain, + "date_range" => "all", + "metrics" => ["pageviews"], + "filters" => [["is_not", "visit:utm_medium", ["sociaL"], %{"case_sensitive" => false}]] + }) + + assert json_response(conn, 200)["results"] == [ + %{"metrics" => [1], "dimensions" => []} + ] + end + test "can filter by utm_source", %{conn: conn, site: site} do populate_stats(site, [ build(:pageview, @@ -904,6 +932,26 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryTest do assert json_response(conn, 200)["results"] == [%{"metrics" => [2], "dimensions" => []}] end + test "contains_not page filter case insensitive", %{conn: conn, site: site} do + populate_stats(site, [ + build(:pageview, pathname: "/en/page1"), + build(:pageview, pathname: "/EN/page2"), + build(:pageview, pathname: "/pl/page1") + ]) + + conn = + post(conn, "/api/v2/query", %{ + "site_id" => site.domain, + "date_range" => "all", + "metrics" => ["visitors"], + "filters" => [ + ["contains_not", "event:page", ["/En/"], %{"case_sensitive" => false}] + ] + }) + + assert json_response(conn, 200)["results"] == [%{"metrics" => [1], "dimensions" => []}] + end + test "contains_not page filter", %{conn: conn, site: site} do populate_stats(site, [ build(:pageview, pathname: "/en/page1"), @@ -1135,6 +1183,47 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryTest do assert json_response(conn, 200)["results"] == [%{"metrics" => [4], "dimensions" => []}] end + test "`contains_not` operator with custom properties case insensitive", %{ + conn: conn, + site: site + } do + populate_stats(site, [ + build(:pageview, + "meta.key": ["name"], + "meta.value": ["large-1"] + ), + build(:pageview, + "meta.key": ["name"], + "meta.value": ["Small-1"] + ), + build(:pageview, + "meta.key": ["name"], + "meta.value": ["mall-1"] + ), + build(:pageview, + "meta.key": ["name"], + "meta.value": ["SMALL-2"] + ), + build(:pageview, + "meta.key": ["name"], + "meta.value": ["small-2"] + ), + build(:pageview) + ]) + + conn = + post(conn, "/api/v2/query", %{ + "site_id" => site.domain, + "date_range" => "all", + "metrics" => ["visitors"], + "filters" => [ + ["contains_not", "event:props:name", ["maLL"], %{"case_sensitive" => false}] + ] + }) + + assert json_response(conn, 200)["results"] == [%{"metrics" => [1], "dimensions" => []}] + end + test "`matches` and `matches_not` operator with custom properties", %{ conn: conn, site: site @@ -3076,6 +3165,12 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryTest do "meta.value": ["Safari", "target_value"], timestamp: ~N[2021-01-01 00:00:00] ), + build(:pageview, + browser: "Chrome", + "meta.key": ["browser", "prop"], + "meta.value": ["Chrome", "target_value"], + timestamp: ~N[2021-01-01 00:00:00] + ), build(:pageview, browser: "Firefox", "meta.key": ["browser", "prop"], @@ -3092,6 +3187,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryTest do "dimensions" => ["visit:browser"], "filters" => [ ["is", "event:props:browser", ["CHROME", "sAFari"], %{"case_sensitive" => false}], + ["is_not", "event:props:browser", ["Chrome"], %{"case_sensitive" => false}], ["is", "event:props:prop", ["target_value"]] ] }) From 5f38e1cf4f748a18f6a08498b2d242b838a37891 Mon Sep 17 00:00:00 2001 From: Karl-Aksel Puulmann Date: Fri, 29 Nov 2024 16:44:46 +0200 Subject: [PATCH 12/15] Cleanup --- assets/js/types/query-api.d.ts | 36 ++++++++++++++++++------ lib/plausible/stats/sql/where_builder.ex | 15 ++++++---- 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/assets/js/types/query-api.d.ts b/assets/js/types/query-api.d.ts index 5b48a4e69f85..2b77acf19216 100644 --- a/assets/js/types/query-api.d.ts +++ b/assets/js/types/query-api.d.ts @@ -64,26 +64,31 @@ export type CustomPropertyFilterDimensions = string; export type GoalDimension = "event:goal"; export type TimeDimensions = "time" | "time:month" | "time:week" | "time:day" | "time:hour"; export type FilterTree = FilterEntry | FilterAndOr | FilterNot; -export type FilterEntry = FilterWithoutGoals | FilterCaseSensitive; +export type FilterEntry = FilterWithoutGoals | FilterWithGoals | FilterWithPattern; /** * @minItems 3 - * @maxItems 3 + * @maxItems 4 */ -export type FilterWithoutGoals = [ - FilterOperationWithoutGoals | ("matches_wildcard" | "matches_wildcard_not"), - SimpleFilterDimensions | CustomPropertyFilterDimensions, - Clauses -]; +export type FilterWithoutGoals = + | [FilterOperationWithoutGoals, SimpleFilterDimensions | CustomPropertyFilterDimensions, Clauses] + | [ + FilterOperationWithoutGoals, + SimpleFilterDimensions | CustomPropertyFilterDimensions, + Clauses, + { + case_sensitive?: boolean; + } + ]; /** * filter operation */ -export type FilterOperationWithoutGoals = "is_not" | "contains_not" | "matches" | "matches_not"; +export type FilterOperationWithoutGoals = "is_not" | "contains_not"; export type Clauses = (string | number)[]; /** * @minItems 3 * @maxItems 4 */ -export type FilterCaseSensitive = +export type FilterWithGoals = | [FilterOperationContains, GoalDimension | SimpleFilterDimensions | CustomPropertyFilterDimensions, Clauses] | [ FilterOperationContains, @@ -97,6 +102,19 @@ export type FilterCaseSensitive = * filter operation */ export type FilterOperationContains = "is" | "contains"; +/** + * @minItems 3 + * @maxItems 3 + */ +export type FilterWithPattern = [ + FilterOperationRegex | ("matches_wildcard" | "matches_wildcard_not"), + SimpleFilterDimensions | CustomPropertyFilterDimensions, + Clauses +]; +/** + * filter operation + */ +export type FilterOperationRegex = "matches" | "matches_not"; /** * @minItems 2 * @maxItems 2 diff --git a/lib/plausible/stats/sql/where_builder.ex b/lib/plausible/stats/sql/where_builder.ex index b3011f36e4b9..08ad5b585c3c 100644 --- a/lib/plausible/stats/sql/where_builder.ex +++ b/lib/plausible/stats/sql/where_builder.ex @@ -250,7 +250,7 @@ defmodule Plausible.Stats.SQL.WhereBuilder do end defp filter_field(db_field, [:contains | _rest] = filter) do - contains_clause(col_value(db_field), filter) + contains_clause(col_value_string(db_field), filter) end defp filter_field(db_field, [:contains_not | rest]) do @@ -293,15 +293,20 @@ defmodule Plausible.Stats.SQL.WhereBuilder do defp db_field_val(_, @not_set), do: "" defp db_field_val(_, val), do: val - def col_value(column_name) do + defp col_value(column_name) do + dynamic([t], field(t, ^column_name)) + end + + # Needed for string functions to work properly + defp col_value_string(column_name) do dynamic([t], type(field(t, ^column_name), :string)) end - def custom_prop_value(column_name, prop_name) do + defp custom_prop_value(column_name, prop_name) do dynamic([t], get_by_key(t, column_name, ^prop_name)) end - def is_in_clause(value_expression, [_, _, clauses | _] = filter, values \\ nil) do + defp is_in_clause(value_expression, [_, _, clauses | _] = filter, values \\ nil) do values = values || clauses if case_sensitive?(filter) do @@ -312,7 +317,7 @@ defmodule Plausible.Stats.SQL.WhereBuilder do end end - def contains_clause(value_expression, [_, _, clauses | _] = filter) do + defp contains_clause(value_expression, [_, _, clauses | _] = filter) do if case_sensitive?(filter) do dynamic( [x], From 6bea1d09fa9e7ca24b9a673760005b77e4a6a8ef Mon Sep 17 00:00:00 2001 From: Karl-Aksel Puulmann Date: Fri, 29 Nov 2024 16:48:09 +0200 Subject: [PATCH 13/15] credo --- lib/plausible/stats/sql/where_builder.ex | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/plausible/stats/sql/where_builder.ex b/lib/plausible/stats/sql/where_builder.ex index 08ad5b585c3c..443983521a4e 100644 --- a/lib/plausible/stats/sql/where_builder.ex +++ b/lib/plausible/stats/sql/where_builder.ex @@ -94,7 +94,7 @@ defmodule Plausible.Stats.SQL.WhereBuilder do end defp add_filter(:events, _query, [:is, "event:name" | _rest] = filter) do - is_in_clause(col_value(:name), filter) + in_clause(col_value(:name), filter) end defp add_filter(:events, query, [_, "event:goal" | _rest] = filter) do @@ -160,7 +160,7 @@ defmodule Plausible.Stats.SQL.WhereBuilder do dynamic( [t], - (has_key(t, column_name, ^prop_name) and ^is_in_clause(prop_value_expr, filter)) or + (has_key(t, column_name, ^prop_name) and ^in_clause(prop_value_expr, filter)) or (^none_value_included and not has_key(t, column_name, ^prop_name)) ) end @@ -172,10 +172,10 @@ defmodule Plausible.Stats.SQL.WhereBuilder do dynamic( [t], (has_key(t, column_name, ^prop_name) and - not (^is_in_clause(prop_value_expr, filter))) or + not (^in_clause(prop_value_expr, filter))) or (^none_value_included and has_key(t, column_name, ^prop_name) and - not (^is_in_clause(prop_value_expr, filter))) or + not (^in_clause(prop_value_expr, filter))) or (not (^none_value_included) and not has_key(t, column_name, ^prop_name)) ) end @@ -270,7 +270,7 @@ defmodule Plausible.Stats.SQL.WhereBuilder do defp filter_field(db_field, [:is, _dimension, clauses | _rest] = filter) do list = clauses |> Enum.map(&db_field_val(db_field, &1)) - is_in_clause(col_value(db_field), filter, list) + in_clause(col_value(db_field), filter, list) end defp filter_field(db_field, [:is_not | rest]) do @@ -306,7 +306,7 @@ defmodule Plausible.Stats.SQL.WhereBuilder do dynamic([t], get_by_key(t, column_name, ^prop_name)) end - defp is_in_clause(value_expression, [_, _, clauses | _] = filter, values \\ nil) do + defp in_clause(value_expression, [_, _, clauses | _] = filter, values \\ nil) do values = values || clauses if case_sensitive?(filter) do From 2ff93f8117b60f578887cfbc064b8a52fd477cb3 Mon Sep 17 00:00:00 2001 From: Karl-Aksel Puulmann Date: Tue, 3 Dec 2024 10:53:34 +0200 Subject: [PATCH 14/15] remove defaults --- lib/plausible/stats/filters/query_parser.ex | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lib/plausible/stats/filters/query_parser.ex b/lib/plausible/stats/filters/query_parser.ex index cf9744a1e3c6..684d6e1310f4 100644 --- a/lib/plausible/stats/filters/query_parser.ex +++ b/lib/plausible/stats/filters/query_parser.ex @@ -17,12 +17,7 @@ defmodule Plausible.Stats.Filters.QueryParser do offset: 0 } - @default_modifiers %{ - case_sensitive: false - } - def default_include(), do: @default_include - def default_modifiers(), do: @default_modifiers def parse(site, schema_type, params, now \\ nil) when is_map(params) do {now, date} = From 2806e8d42f2e027c22c8ca4100b811130f295eed Mon Sep 17 00:00:00 2001 From: Karl-Aksel Puulmann Date: Tue, 3 Dec 2024 10:55:35 +0200 Subject: [PATCH 15/15] negating a previously set filter --- .../controllers/api/external_stats_controller/query_test.exs | 1 + 1 file changed, 1 insertion(+) diff --git a/test/plausible_web/controllers/api/external_stats_controller/query_test.exs b/test/plausible_web/controllers/api/external_stats_controller/query_test.exs index 451bcf7f6b35..fd06c558bdfd 100644 --- a/test/plausible_web/controllers/api/external_stats_controller/query_test.exs +++ b/test/plausible_web/controllers/api/external_stats_controller/query_test.exs @@ -3187,6 +3187,7 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryTest do "dimensions" => ["visit:browser"], "filters" => [ ["is", "event:props:browser", ["CHROME", "sAFari"], %{"case_sensitive" => false}], + # Negate a previously set filter ["is_not", "event:props:browser", ["Chrome"], %{"case_sensitive" => false}], ["is", "event:props:prop", ["target_value"]] ]