diff --git a/CHANGELOG.md b/CHANGELOG.md index a4838739b3b0..6b5620f61799 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ All notable changes to this project will be documented in this file. ### Changed -- Filters appear in the search bar as ?f=is,page,/docs,/blog&f=... instead of ?filters=((is,page,(/docs,/blog)),...) for Plausible links sent on various platforms to work reliably. +- Filters appear in the search bar as ?f=is,page,/docs,/blog&f=... instead of ?filters=((is,page,(/docs,/blog)),...) for Plausible links sent on various platforms to work reliably. - Details modal search inputs are now case-insensitive. - Improved report performance in cases where site has a lot of unique pathnames diff --git a/assets/js/types/query-api.d.ts b/assets/js/types/query-api.d.ts index 51b52f505099..a52c60da4a58 100644 --- a/assets/js/types/query-api.d.ts +++ b/assets/js/types/query-api.d.ts @@ -63,7 +63,7 @@ export type SimpleFilterDimensions = 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 FilterTree = FilterEntry | FilterAndOr | FilterNot | FilterHasDone; export type FilterEntry = FilterWithoutGoals | FilterWithGoals | FilterWithPattern | FilterForSegment; /** * @minItems 3 @@ -130,6 +130,11 @@ export type FilterAndOr = ["and" | "or", [FilterTree, ...FilterTree[]]]; * @maxItems 2 */ export type FilterNot = ["not", FilterTree]; +/** + * @minItems 2 + * @maxItems 2 + */ +export type FilterHasDone = ["has_done" | "has_not_done", FilterTree]; /** * @minItems 2 * @maxItems 2 diff --git a/extra/lib/plausible/stats/goal/revenue.ex b/extra/lib/plausible/stats/goal/revenue.ex index dba19909ca9a..1be42759d667 100644 --- a/extra/lib/plausible/stats/goal/revenue.ex +++ b/extra/lib/plausible/stats/goal/revenue.ex @@ -24,11 +24,11 @@ defmodule Plausible.Stats.Goal.Revenue do The resulting data structure is attached to a `Query` and used below in `format_revenue_metric/3`. """ - def preload(site, goals, metrics, dimensions) do + def preload(site, preloaded_goals, metrics, dimensions) do cond do not requested?(metrics) -> {nil, %{}} not available?(site) -> {:revenue_goals_unavailable, %{}} - true -> preload(goals, dimensions) + true -> preload(preloaded_goals.matching_toplevel_filters, dimensions) end end diff --git a/lib/plausible/goals/filters.ex b/lib/plausible/goals/filters.ex index b9d7d5a74935..0caf5e4e5179 100644 --- a/lib/plausible/goals/filters.ex +++ b/lib/plausible/goals/filters.ex @@ -4,6 +4,31 @@ defmodule Plausible.Goals.Filters do import Ecto.Query import Plausible.Stats.Filters.Utils, only: [page_regex: 1] + alias Plausible.Stats.Filters + + @doc """ + Preloads goals data if needed for query-building and related work. + """ + def preload_needed_goals(site, dimensions, filters) do + if Enum.member?(dimensions, "event:goal") or + Filters.filtering_on_dimension?(filters, "event:goal") do + goals = Plausible.Goals.for_site(site) + + %{ + # When grouping by event:goal, later pipeline needs to know which goals match filters exactly. + # This can affect both calculations whether all goals have the same revenue currency and + # whether we should skip imports. + matching_toplevel_filters: goals_matching_toplevel_filters(goals, filters), + all: goals + } + else + %{ + all: [], + matching_toplevel_filters: [] + } + end + end + @doc """ Translates an event:goal filter into SQL. Similarly to other `add_filter` clauses in `Plausible.Stats.SQL.WhereBuilder`, returns an `Ecto.Query.dynamic` expression. @@ -26,7 +51,7 @@ defmodule Plausible.Goals.Filters do Enum.reduce(clauses, false, fn clause, dynamic_statement -> condition = - query.preloaded_goals + query.preloaded_goals.all |> filter_preloaded(filter, clause) |> build_condition(imported?) @@ -34,9 +59,11 @@ defmodule Plausible.Goals.Filters do end) end - def preload_needed_goals(site, filters) do - goals = Plausible.Goals.for_site(site) + defp filter_preloaded(goals, filter, clause) do + Enum.filter(goals, fn goal -> matches?(goal, filter, clause) end) + end + defp goals_matching_toplevel_filters(goals, filters) do Enum.reduce(filters, goals, fn [_, "event:goal" | _] = filter, goals -> goals_matching_any_clause(goals, filter) @@ -46,10 +73,6 @@ defmodule Plausible.Goals.Filters do end) 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, [_, _, clauses | _] = filter) do goals |> Enum.filter(fn goal -> diff --git a/lib/plausible/segments/filters.ex b/lib/plausible/segments/filters.ex index a18cf33e787b..132ce2bd55bc 100644 --- a/lib/plausible/segments/filters.ex +++ b/lib/plausible/segments/filters.ex @@ -22,7 +22,7 @@ defmodule Plausible.Segments.Filters do filters |> Filters.traverse() |> Enum.flat_map(fn - {[_operation, "segment", clauses], _depth} -> clauses + {[_operation, "segment", clauses], _} -> clauses _ -> [] end) diff --git a/lib/plausible/stats/base.ex b/lib/plausible/stats/base.ex index c5304eaacdb1..e9c2a48b63d3 100644 --- a/lib/plausible/stats/base.ex +++ b/lib/plausible/stats/base.ex @@ -26,8 +26,8 @@ defmodule Plausible.Stats.Base do end end - defp query_events(site, query) do - q = from(e in "events_v2", where: ^SQL.WhereBuilder.build(:events, site, query)) + defp query_events(_site, query) do + q = from(e in "events_v2", where: ^SQL.WhereBuilder.build(:events, query)) on_ee do q = Plausible.Stats.Sampling.add_query_hint(q, query) @@ -36,8 +36,8 @@ defmodule Plausible.Stats.Base do q end - def query_sessions(site, query) do - q = from(s in "sessions_v2", where: ^SQL.WhereBuilder.build(:sessions, site, query)) + def query_sessions(_site, query) do + q = from(s in "sessions_v2", where: ^SQL.WhereBuilder.build(:sessions, query)) on_ee do q = Plausible.Stats.Sampling.add_query_hint(q, query) diff --git a/lib/plausible/stats/clickhouse.ex b/lib/plausible/stats/clickhouse.ex index a201d0c94ac2..232d7d0e7f29 100644 --- a/lib/plausible/stats/clickhouse.ex +++ b/lib/plausible/stats/clickhouse.ex @@ -78,7 +78,7 @@ defmodule Plausible.Stats.Clickhouse do def top_sources_for_spike(site, query, limit, page) do offset = (page - 1) * limit - {first_datetime, last_datetime} = Plausible.Stats.Time.utc_boundaries(query, site) + {first_datetime, last_datetime} = Plausible.Stats.Time.utc_boundaries(query) referrers = from(s in "sessions_v2", diff --git a/lib/plausible/stats/filters/filters.ex b/lib/plausible/stats/filters/filters.ex index 4c828fef1e50..b693528b7654 100644 --- a/lib/plausible/stats/filters/filters.ex +++ b/lib/plausible/stats/filters/filters.ex @@ -3,6 +3,7 @@ defmodule Plausible.Stats.Filters do A module for parsing filters used in stat queries. """ + alias Plausible.Stats.Query alias Plausible.Stats.Filters.QueryParser alias Plausible.Stats.Filters.StatsAPIFilterParser @@ -89,15 +90,45 @@ defmodule Plausible.Stats.Filters do def dimensions_used_in_filters(filters, opts \\ []) do min_depth = Keyword.get(opts, :min_depth, 0) + max_depth = Keyword.get(opts, :max_depth, 999) + # :ignore or :only + behavioral_filter_option = Keyword.get(opts, :behavioral_filters, nil) filters - |> traverse() - |> Enum.filter(fn {_filter, depth} -> depth >= min_depth end) + |> traverse( + {0, false}, + fn {depth, is_behavioral_filter}, operator -> + {depth + 1, is_behavioral_filter or operator in [:has_done, :has_not_done]} + end + ) + |> Enum.filter(fn {_filter, {depth, is_behavioral_filter}} -> + matches_behavioral_filter_option? = + case behavioral_filter_option do + :ignore -> not is_behavioral_filter + :only -> is_behavioral_filter + _ -> true + end + + depth >= min_depth and depth <= max_depth and matches_behavioral_filter_option? + end) |> Enum.map(fn {[_operator, dimension | _rest], _depth} -> dimension end) end - def filtering_on_dimension?(query, dimension) do - dimension in dimensions_used_in_filters(query.filters) + def filtering_on_dimension?(query, dimension, opts \\ []) do + filters = + case query do + %Query{filters: filters} -> filters + %{filters: filters} -> filters + filters when is_list(filters) -> filters + end + + dimension in dimensions_used_in_filters(filters, opts) + end + + def all_leaf_filters(filters) do + filters + |> traverse(nil, fn _, _ -> nil end) + |> Enum.map(fn {filter, _} -> filter end) end @doc """ @@ -144,12 +175,13 @@ defmodule Plausible.Stats.Filters do defp transform_tree(filter, transformer) do case {transformer.(filter), filter} do # Transformer did not return that value - transform that subtree - {nil, [operation, child_filter]} when operation in [:not, :ignore_in_totals_query] -> + {nil, [operator, child_filter]} + when operator in [:not, :ignore_in_totals_query, :has_done, :has_not_done] -> [transformed_child] = transform_tree(child_filter, transformer) - [[operation, transformed_child]] + [[operator, transformed_child]] - {nil, [operation, filters]} when operation in [:and, :or] -> - [[operation, transform_filters(filters, transformer)]] + {nil, [operator, filters]} when operator in [:and, :or] -> + [[operator, transform_filters(filters, transformer)]] # Reached a leaf node, return existing value {nil, filter} -> @@ -161,22 +193,26 @@ defmodule Plausible.Stats.Filters do end end - def traverse(filters, depth \\ -1) do + @doc """ + Traverses a filter tree while accumulating state. + """ + def traverse(filters, state \\ nil, state_transformer \\ fn state, _ -> state end) do filters - |> Enum.flat_map(&traverse_tree(&1, depth + 1)) + |> Enum.flat_map(&traverse_tree(&1, state, state_transformer)) end - defp traverse_tree(filter, depth) do + defp traverse_tree(filter, state, state_transformer) do case filter do - [operation, child_filter] when operation in [:not, :ignore_in_totals_query] -> - traverse_tree(child_filter, depth + 1) + [operation, child_filter] + when operation in [:not, :ignore_in_totals_query, :has_done, :has_not_done] -> + traverse_tree(child_filter, state_transformer.(state, operation), state_transformer) [operation, filters] when operation in [:and, :or] -> - traverse(filters, depth + 1) + traverse(filters, state_transformer.(state, operation), state_transformer) # Leaf node _ -> - [{filter, depth}] + [{filter, state}] end end end diff --git a/lib/plausible/stats/filters/query_parser.ex b/lib/plausible/stats/filters/query_parser.ex index 0dd6a976c8ac..848478e7f504 100644 --- a/lib/plausible/stats/filters/query_parser.ex +++ b/lib/plausible/stats/filters/query_parser.ex @@ -61,6 +61,7 @@ defmodule Plausible.Stats.Filters.QueryParser do :ok <- validate_custom_props_access(site, query), :ok <- validate_toplevel_only_filter_dimension(query), :ok <- validate_special_metrics_filters(query), + :ok <- validate_behavioral_filters(query), :ok <- validate_filtered_goals_exist(query), :ok <- validate_revenue_metrics_access(site, query), :ok <- validate_metrics(query), @@ -110,16 +111,20 @@ defmodule Plausible.Stats.Filters.QueryParser do defp parse_operator(["matches_wildcard_not" | _rest]), do: {:ok, :matches_wildcard_not} defp parse_operator(["contains" | _rest]), do: {:ok, :contains} defp parse_operator(["contains_not" | _rest]), do: {:ok, :contains_not} - defp parse_operator(["not" | _rest]), do: {:ok, :not} defp parse_operator(["and" | _rest]), do: {:ok, :and} defp parse_operator(["or" | _rest]), do: {:ok, :or} + defp parse_operator(["not" | _rest]), do: {:ok, :not} + defp parse_operator(["has_done" | _rest]), do: {:ok, :has_done} + defp parse_operator(["has_not_done" | _rest]), do: {:ok, :has_not_done} defp parse_operator(filter), do: {:error, "Unknown operator for filter '#{i(filter)}'."} - def parse_filter_second(:not, [_, filter | _rest]), do: parse_filter(filter) - def parse_filter_second(operator, [_, filters | _rest]) when operator in [:and, :or], do: parse_filters(filters) + def parse_filter_second(operator, [_, filter | _rest]) + when operator in [:not, :has_done, :has_not_done], + do: parse_filter(filter) + def parse_filter_second(_operator, filter), do: parse_filter_key(filter) defp parse_filter_key([_operator, filter_key | _rest] = filter) do @@ -146,7 +151,7 @@ defmodule Plausible.Stats.Filters.QueryParser do end defp parse_filter_rest(operator, _filter) - when operator in [:not, :and, :or], + when operator in [:not, :and, :or, :has_done, :has_not_done], do: {:ok, []} defp parse_clauses_list([operator, filter_key, list | _rest] = filter) when is_list(list) do @@ -434,20 +439,14 @@ defmodule Plausible.Stats.Filters.QueryParser do end def preload_goals_and_revenue(site, metrics, filters, dimensions) do - goal_filters? = - Enum.any?(filters, fn [_, filter_key | _rest] -> filter_key == "event:goal" end) + preloaded_goals = + Plausible.Goals.Filters.preload_needed_goals(site, dimensions, filters) - goals = - if goal_filters? or Enum.member?(dimensions, "event:goal") do - Plausible.Goals.Filters.preload_needed_goals(site, filters) - else - [] - end - - {revenue_warning, revenue_currencies} = preload_revenue(site, goals, metrics, dimensions) + {revenue_warning, revenue_currencies} = + preload_revenue(site, preloaded_goals, metrics, dimensions) { - goals, + preloaded_goals, revenue_warning, revenue_currencies } @@ -455,9 +454,12 @@ defmodule Plausible.Stats.Filters.QueryParser do @only_toplevel ["event:goal", "event:hostname"] defp validate_toplevel_only_filter_dimension(query) do - not_toplevel = Filters.dimensions_used_in_filters(query.filters, min_depth: 1) + not_toplevel = + query.filters + |> Filters.dimensions_used_in_filters(min_depth: 1, behavioral_filters: :ignore) + |> Enum.filter(&(&1 in @only_toplevel)) - if Enum.any?(not_toplevel, &(&1 in @only_toplevel)) do + if Enum.count(not_toplevel) > 0 do {:error, "Invalid filters. Dimension `#{List.first(not_toplevel)}` can only be filtered at the top level."} else @@ -482,16 +484,53 @@ defmodule Plausible.Stats.Filters.QueryParser do end end + defp validate_behavioral_filters(query) do + query.filters + |> Filters.traverse(0, fn behavioral_depth, operator -> + if operator in [:has_done, :has_not_done] do + behavioral_depth + 1 + else + behavioral_depth + end + end) + |> Enum.reduce_while(:ok, fn {[_operator, dimension | _rest], behavioral_depth}, :ok -> + cond do + behavioral_depth == 0 -> + # ignore non-behavioral filters + {:cont, :ok} + + behavioral_depth > 1 -> + {:halt, + {:error, + "Invalid filters. Behavioral filters (has_done, has_not_done) cannot be nested."}} + + not String.starts_with?(dimension, "event:") -> + {:halt, + {:error, + "Invalid filters. Behavioral filters (has_done, has_not_done) can only be used with event dimension filters."}} + + true -> + {:cont, :ok} + end + end) + end + defp validate_filtered_goals_exist(query) do - # Note: Only works since event:goal is allowed as a top level filter + # Note: We don't check :contains goal filters since it's acceptable if they match nothing. goal_filter_clauses = - Enum.flat_map(query.filters, fn + query.filters + |> Filters.all_leaf_filters() + |> Enum.flat_map(fn [:is, "event:goal", clauses] -> clauses _ -> [] end) if length(goal_filter_clauses) > 0 do - validate_list(goal_filter_clauses, &validate_goal_filter(&1, query.preloaded_goals)) + configured_goal_names = + query.preloaded_goals.all + |> Enum.map(&Plausible.Goal.display_name/1) + + validate_list(goal_filter_clauses, &validate_goal_filter(&1, configured_goal_names)) else :ok end @@ -517,10 +556,7 @@ defmodule Plausible.Stats.Filters.QueryParser do defp validate_revenue_metrics_access(_site, _query), do: :ok end - defp validate_goal_filter(clause, configured_goals) do - configured_goal_names = - Enum.map(configured_goals, fn goal -> Plausible.Goal.display_name(goal) end) - + defp validate_goal_filter(clause, configured_goal_names) do if Enum.member?(configured_goal_names, clause) do :ok else @@ -562,7 +598,7 @@ defmodule Plausible.Stats.Filters.QueryParser do defp validate_metric(metric, query) when metric in [:conversion_rate, :group_conversion_rate] do if Enum.member?(query.dimensions, "event:goal") or - Filters.filtering_on_dimension?(query, "event:goal") do + Filters.filtering_on_dimension?(query, "event:goal", behavioral_filters: :ignore) do :ok else {:error, "Metric `#{metric}` can only be queried with event:goal filters or dimensions."} @@ -582,7 +618,7 @@ defmodule Plausible.Stats.Filters.QueryParser do defp validate_metric(:views_per_visit = metric, query) do cond do - Filters.filtering_on_dimension?(query, "event:page") -> + Filters.filtering_on_dimension?(query, "event:page", behavioral_filters: :ignore) -> {:error, "Metric `#{metric}` cannot be queried with a filter on `event:page`."} length(query.dimensions) > 0 -> diff --git a/lib/plausible/stats/imported/base.ex b/lib/plausible/stats/imported/base.ex index 2328a4497f65..a27d6cb6f356 100644 --- a/lib/plausible/stats/imported/base.ex +++ b/lib/plausible/stats/imported/base.ex @@ -8,7 +8,8 @@ defmodule Plausible.Stats.Imported.Base do alias Plausible.Imported alias Plausible.Stats.Query - import Plausible.Stats.Filters, only: [dimensions_used_in_filters: 1] + import Plausible.Stats.Filters, + only: [dimensions_used_in_filters: 1, dimensions_used_in_filters: 2] @property_to_table_mappings %{ "visit:source" => "imported_sources", @@ -74,10 +75,18 @@ defmodule Plausible.Stats.Imported.Base do end def decide_tables(query) do - if custom_prop_query?(query) do - do_decide_custom_prop_table(query) - else - do_decide_tables(query) + behavioral_filters = dimensions_used_in_filters(query.filters, behavioral_filters: :only) + + cond do + # Behavioral filters cannot be emulated via aggregated imported stats + length(behavioral_filters) > 0 -> + [] + + custom_prop_query?(query) -> + do_decide_custom_prop_table(query) + + true -> + do_decide_tables(query) end end @@ -124,9 +133,9 @@ defmodule Plausible.Stats.Imported.Base do |> Enum.any?(&(&1 in special_goals_for(property))) has_unsupported_filters? = - Enum.any?(query.filters, fn [_, filter_key | _] -> - filter_key not in [property, "event:name", "event:goal"] - end) + query.filters + |> dimensions_used_in_filters() + |> Enum.any?(&(&1 not in [property, "event:name", "event:goal"])) if has_required_name_filter? and not has_unsupported_filters? do ["imported_custom_events"] @@ -144,7 +153,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 = query.preloaded_goals + filter_goals = query.preloaded_goals.matching_toplevel_filters any_event_goals? = Enum.any?(filter_goals, fn goal -> Plausible.Goal.type(goal) == :event end) @@ -177,7 +186,7 @@ defmodule Plausible.Stats.Imported.Base do |> Enum.map(&@property_to_table_mappings[&1]) filter_goal_table_candidates = - query.preloaded_goals + query.preloaded_goals.matching_toplevel_filters |> Enum.map(&Plausible.Goal.type/1) |> Enum.map(fn :event -> "imported_custom_events" diff --git a/lib/plausible/stats/imported/imported.ex b/lib/plausible/stats/imported/imported.ex index 9f7b038595d7..61d99373de11 100644 --- a/lib/plausible/stats/imported/imported.ex +++ b/lib/plausible/stats/imported/imported.ex @@ -239,7 +239,8 @@ defmodule Plausible.Stats.Imported do end def merge_imported(q, site, %Query{dimensions: ["event:goal"]} = query, metrics) do - {events, page_regexes} = Filters.Utils.split_goals_query_expressions(query.preloaded_goals) + {events, page_regexes} = + Filters.Utils.split_goals_query_expressions(query.preloaded_goals.matching_toplevel_filters) Imported.Base.decide_tables(query) |> Enum.map(fn diff --git a/lib/plausible/stats/legacy/legacy_query_builder.ex b/lib/plausible/stats/legacy/legacy_query_builder.ex index 5fa518bbcffc..fe81589b3499 100644 --- a/lib/plausible/stats/legacy/legacy_query_builder.ex +++ b/lib/plausible/stats/legacy/legacy_query_builder.ex @@ -13,7 +13,12 @@ defmodule Plausible.Stats.Legacy.QueryBuilder do query = Query - |> struct!(now: now, debug_metadata: debug_metadata) + |> struct!( + now: now, + debug_metadata: debug_metadata, + site_id: site.id, + site_native_stats_start_at: site.native_stats_start_at + ) |> put_period(site, params) |> put_timezone(site) |> put_dimensions(params) diff --git a/lib/plausible/stats/query.ex b/lib/plausible/stats/query.ex index 8ee940eec6e4..76e6ad702a48 100644 --- a/lib/plausible/stats/query.ex +++ b/lib/plausible/stats/query.ex @@ -22,7 +22,9 @@ defmodule Plausible.Stats.Query do # Revenue metric specific metadata revenue_currencies: %{}, revenue_warning: nil, - remove_unavailable_revenue_metrics: false + remove_unavailable_revenue_metrics: false, + site_id: nil, + site_native_stats_start_at: nil require OpenTelemetry.Tracer, as: Tracer alias Plausible.Stats.{DateTimeRange, Filters, Imported, Legacy} @@ -34,7 +36,12 @@ defmodule Plausible.Stats.Query do query = struct!(__MODULE__, Map.to_list(query_data)) |> put_imported_opts(site, %{}) - |> struct!(now: DateTime.utc_now(:second), debug_metadata: debug_metadata) + |> struct!( + now: DateTime.utc_now(:second), + debug_metadata: debug_metadata, + site_id: site.id, + site_native_stats_start_at: site.native_stats_start_at + ) on_ee do query = Plausible.Stats.Sampling.put_threshold(query, site, params) diff --git a/lib/plausible/stats/query_runner.ex b/lib/plausible/stats/query_runner.ex index fadc683466f2..6af06f21544c 100644 --- a/lib/plausible/stats/query_runner.ex +++ b/lib/plausible/stats/query_runner.ex @@ -154,7 +154,7 @@ defmodule Plausible.Stats.QueryRunner do end defp dimension_label("event:goal", entry, query) do - {events, paths} = Filters.Utils.split_goals(query.preloaded_goals) + {events, paths} = Filters.Utils.split_goals(query.preloaded_goals.matching_toplevel_filters) goal_index = Map.get(entry, Util.shortname(query, "event:goal")) diff --git a/lib/plausible/stats/sql/query_builder.ex b/lib/plausible/stats/sql/query_builder.ex index ce340d9749f5..f0b1e2b56f6e 100644 --- a/lib/plausible/stats/sql/query_builder.ex +++ b/lib/plausible/stats/sql/query_builder.ex @@ -37,7 +37,7 @@ defmodule Plausible.Stats.SQL.QueryBuilder do q = from( e in "events_v2", - where: ^SQL.WhereBuilder.build(:events, site, events_query), + where: ^SQL.WhereBuilder.build(:events, events_query), select: ^select_event_metrics(events_query) ) @@ -46,18 +46,18 @@ defmodule Plausible.Stats.SQL.QueryBuilder do end q - |> join_sessions_if_needed(site, events_query) + |> join_sessions_if_needed(events_query) |> build_group_by(:events, events_query) |> merge_imported(site, events_query, events_query.metrics) |> SQL.SpecialMetrics.add(site, events_query) end - defp join_sessions_if_needed(q, site, query) do + defp join_sessions_if_needed(q, query) do if TableDecider.events_join_sessions?(query) do sessions_q = from( s in "sessions_v2", - where: ^SQL.WhereBuilder.build(:sessions, site, query), + where: ^SQL.WhereBuilder.build(:sessions, query), where: s.sign == 1, select: %{session_id: s.session_id}, group_by: s.session_id @@ -83,7 +83,7 @@ defmodule Plausible.Stats.SQL.QueryBuilder do q = from( e in "sessions_v2", - where: ^SQL.WhereBuilder.build(:sessions, site, sessions_query), + where: ^SQL.WhereBuilder.build(:sessions, sessions_query), select: ^select_session_metrics(sessions_query) ) @@ -92,17 +92,17 @@ defmodule Plausible.Stats.SQL.QueryBuilder do end q - |> join_events_if_needed(site, sessions_query) + |> join_events_if_needed(sessions_query) |> build_group_by(:sessions, sessions_query) |> merge_imported(site, sessions_query, sessions_query.metrics) |> SQL.SpecialMetrics.add(site, sessions_query) end - def join_events_if_needed(q, site, query) do + def join_events_if_needed(q, query) do if TableDecider.sessions_join_events?(query) do events_q = from(e in "events_v2", - where: ^SQL.WhereBuilder.build(:events, site, query), + where: ^SQL.WhereBuilder.build(:events, query), select: %{ session_id: fragment("DISTINCT ?", e.session_id), _sample_factor: fragment("_sample_factor") @@ -139,7 +139,8 @@ defmodule Plausible.Stats.SQL.QueryBuilder do end defp dimension_group_by(q, _table, query, "event:goal" = dimension) do - {events, page_regexes} = Filters.Utils.split_goals_query_expressions(query.preloaded_goals) + {events, page_regexes} = + Filters.Utils.split_goals_query_expressions(query.preloaded_goals.matching_toplevel_filters) from(e in q, join: goal in Expression.event_goal_join(events, page_regexes), diff --git a/lib/plausible/stats/sql/special_metrics.ex b/lib/plausible/stats/sql/special_metrics.ex index 15eef2bed981..fd5f10c75387 100644 --- a/lib/plausible/stats/sql/special_metrics.ex +++ b/lib/plausible/stats/sql/special_metrics.ex @@ -58,7 +58,7 @@ defmodule Plausible.Stats.SQL.SpecialMetrics do |> Query.set( dimensions: [], include_imported: query.include_imported, - preloaded_goals: [], + preloaded_goals: Map.put(query.preloaded_goals, :matching_toplevel_filters, []), pagination: nil ) @@ -102,7 +102,7 @@ defmodule Plausible.Stats.SQL.SpecialMetrics do metrics: [:visitors], order_by: [], include_imported: query.include_imported, - preloaded_goals: [], + preloaded_goals: Map.put(query.preloaded_goals, :matching_toplevel_filters, []), pagination: nil ) diff --git a/lib/plausible/stats/sql/where_builder.ex b/lib/plausible/stats/sql/where_builder.ex index 443983521a4e..e19e691bd450 100644 --- a/lib/plausible/stats/sql/where_builder.ex +++ b/lib/plausible/stats/sql/where_builder.ex @@ -4,7 +4,7 @@ defmodule Plausible.Stats.SQL.WhereBuilder do """ import Ecto.Query - import Plausible.Stats.Time, only: [utc_boundaries: 2] + import Plausible.Stats.Time, only: [utc_boundaries: 1] import Plausible.Stats.Filters.Utils, only: [page_regex: 1] use Plausible.Stats.SQL.Fragments @@ -19,8 +19,8 @@ defmodule Plausible.Stats.SQL.WhereBuilder do ] @doc "Builds WHERE clause for a given Query against sessions or events table" - def build(table, site, query) do - base_condition = filter_site_time_range(table, site, query) + def build(table, query) do + base_condition = filter_site_time_range(table, query) query.filters |> Enum.map(&add_filter(table, query, &1)) @@ -41,17 +41,18 @@ defmodule Plausible.Stats.SQL.WhereBuilder do end end - defp filter_site_time_range(:events, site, query) do - {first_datetime, last_datetime} = utc_boundaries(query, site) + defp filter_site_time_range(:events, query) do + {first_datetime, last_datetime} = utc_boundaries(query) dynamic( [e], - e.site_id == ^site.id and e.timestamp >= ^first_datetime and e.timestamp <= ^last_datetime + e.site_id == ^query.site_id and e.timestamp >= ^first_datetime and + e.timestamp <= ^last_datetime ) end - defp filter_site_time_range(:sessions, site, query) do - {first_datetime, last_datetime} = utc_boundaries(query, site) + defp filter_site_time_range(:sessions, query) do + {first_datetime, last_datetime} = utc_boundaries(query) # Counts each _active_ session in time range even if they started before dynamic( @@ -66,7 +67,7 @@ defmodule Plausible.Stats.SQL.WhereBuilder do # Without it, the sample factor would be greatly overestimated for large sites, # as query would be estimated to return _all_ rows matching other conditions # before `start == last_datetime`. - s.site_id == ^site.id and + s.site_id == ^query.site_id and s.start >= ^NaiveDateTime.add(first_datetime, -7, :day) and s.timestamp >= ^first_datetime and s.start <= ^last_datetime @@ -93,6 +94,20 @@ defmodule Plausible.Stats.SQL.WhereBuilder do |> Enum.reduce(fn condition, acc -> dynamic([], ^acc or ^condition) end) end + defp add_filter(_table, query, [:has_done, filter]) do + condition = + dynamic([], ^filter_site_time_range(:events, query) and ^add_filter(:events, query, filter)) + + dynamic( + [t], + t.session_id in subquery(from(e in "events_v2", where: ^condition, select: e.session_id)) + ) + end + + defp add_filter(table, query, [:has_not_done, filter]) do + dynamic([], not (^add_filter(table, query, [:has_done, filter]))) + end + defp add_filter(:events, _query, [:is, "event:name" | _rest] = filter) do in_clause(col_value(:name), filter) end diff --git a/lib/plausible/stats/time.ex b/lib/plausible/stats/time.ex index cee037e4b03b..1872928c9837 100644 --- a/lib/plausible/stats/time.ex +++ b/lib/plausible/stats/time.ex @@ -5,11 +5,14 @@ defmodule Plausible.Stats.Time do alias Plausible.Stats.{Query, DateTimeRange} - def utc_boundaries(%Query{utc_time_range: time_range}, site) do + def utc_boundaries(%Query{ + utc_time_range: time_range, + site_native_stats_start_at: native_stats_start_at + }) do first = time_range.first |> DateTime.to_naive() - |> beginning_of_time(site.native_stats_start_at) + |> beginning_of_time(native_stats_start_at) last = DateTime.to_naive(time_range.last) diff --git a/lib/plausible_web/controllers/api/stats_controller.ex b/lib/plausible_web/controllers/api/stats_controller.ex index 32430202e804..66448d79a8d2 100644 --- a/lib/plausible_web/controllers/api/stats_controller.ex +++ b/lib/plausible_web/controllers/api/stats_controller.ex @@ -294,7 +294,7 @@ defmodule PlausibleWeb.Api.StatsController do end defp fetch_top_stats(site, query, current_user) do - goal_filter? = Filters.filtering_on_dimension?(query, "event:goal") + goal_filter? = toplevel_goal_filter?(query) cond do query.period == "30m" && goal_filter? -> @@ -392,7 +392,9 @@ defmodule PlausibleWeb.Api.StatsController do end defp fetch_other_top_stats(site, query, current_user) do - page_filter? = Filters.filtering_on_dimension?(query, "event:page") + page_filter? = + Filters.filtering_on_dimension?(query, "event:page", behavioral_filters: :ignore) + scroll_depth_enabled? = scroll_depth_enabled?(site, current_user) metrics = [:visitors, :visits, :pageviews, :sample_percent] @@ -477,7 +479,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{source: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if toplevel_goal_filter?(query) do res |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -509,7 +511,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{channel: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if toplevel_goal_filter?(query) do res |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -564,7 +566,7 @@ defmodule PlausibleWeb.Api.StatsController do defp validate_funnel_query(query) do cond do - Filters.filtering_on_dimension?(query, "event:goal") -> + toplevel_goal_filter?(query) -> {:error, {:invalid_funnel_query, "goals"}} Filters.filtering_on_dimension?(query, "event:page") -> @@ -591,7 +593,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{utm_medium: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if toplevel_goal_filter?(query) do res |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -619,7 +621,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{utm_campaign: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if toplevel_goal_filter?(query) do res |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -647,7 +649,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{utm_content: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if toplevel_goal_filter?(query) do res |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -675,7 +677,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{utm_term: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if toplevel_goal_filter?(query) do res |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -703,7 +705,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{utm_source: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if toplevel_goal_filter?(query) do res |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -731,7 +733,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{referrer: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if toplevel_goal_filter?(query) do res |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -857,7 +859,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{page: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if toplevel_goal_filter?(query) do pages |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -890,7 +892,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{entry_page: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if toplevel_goal_filter?(query) do to_csv(entry_pages, [:name, :visitors, :conversion_rate], [ :name, :conversions, @@ -926,7 +928,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{exit_page: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if toplevel_goal_filter?(query) do to_csv(exit_pages, [:name, :visitors, :conversion_rate], [ :name, :conversions, @@ -999,7 +1001,7 @@ defmodule PlausibleWeb.Api.StatsController do Map.put(country, :name, country_info.name) end) - if Filters.filtering_on_dimension?(query, "event:goal") do + if toplevel_goal_filter?(query) do countries |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -1059,7 +1061,7 @@ defmodule PlausibleWeb.Api.StatsController do end) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if toplevel_goal_filter?(query) do regions |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -1103,7 +1105,7 @@ defmodule PlausibleWeb.Api.StatsController do end) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if toplevel_goal_filter?(query) do cities |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -1135,7 +1137,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{browser: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if toplevel_goal_filter?(query) do browsers |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -1167,7 +1169,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{browser_version: :version}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if toplevel_goal_filter?(query) do results |> transform_keys(%{browser: :name, visitors: :conversions}) |> to_csv([:name, :version, :conversions, :conversion_rate]) @@ -1208,7 +1210,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{os: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if toplevel_goal_filter?(query) do systems |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -1240,7 +1242,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{os_version: :version}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if toplevel_goal_filter?(query) do results |> transform_keys(%{os: :name, visitors: :conversions}) |> to_csv([:name, :version, :conversions, :conversion_rate]) @@ -1281,7 +1283,7 @@ defmodule PlausibleWeb.Api.StatsController do |> transform_keys(%{device: :name}) if params["csv"] do - if Filters.filtering_on_dimension?(query, "event:goal") do + if toplevel_goal_filter?(query) do sizes |> transform_keys(%{visitors: :conversions}) |> to_csv([:name, :conversions, :conversion_rate]) @@ -1371,7 +1373,7 @@ defmodule PlausibleWeb.Api.StatsController do |> Enum.concat() percent_or_cr = - if Filters.filtering_on_dimension?(query, "event:goal"), + if toplevel_goal_filter?(query), do: :conversion_rate, else: :percentage @@ -1388,7 +1390,7 @@ defmodule PlausibleWeb.Api.StatsController do query = Query.from(site, params, debug_metadata(conn)) metrics = - if Filters.filtering_on_dimension?(query, "event:goal") do + if toplevel_goal_filter?(query) do [:visitors, :events, :conversion_rate] ++ @revenue_metrics else [:visitors, :events, :percentage] ++ @revenue_metrics @@ -1559,10 +1561,12 @@ defmodule PlausibleWeb.Api.StatsController do end requires_goal_filter? = metric in [:conversion_rate, :events] - has_goal_filter? = Filters.filtering_on_dimension?(query, "event:goal") + has_goal_filter? = toplevel_goal_filter?(query) requires_page_filter? = metric == :scroll_depth - has_page_filter? = Filters.filtering_on_dimension?(query, "event:page") + + has_page_filter? = + Filters.filtering_on_dimension?(query, "event:page", behavioral_filters: :ignore) cond do requires_goal_filter? and not has_goal_filter? -> @@ -1600,7 +1604,7 @@ defmodule PlausibleWeb.Api.StatsController do end defp breakdown_metrics(query, extra_metrics \\ []) do - if Filters.filtering_on_dimension?(query, "event:goal") do + if toplevel_goal_filter?(query) do [:visitors, :conversion_rate, :total_visitors] else [:visitors] ++ extra_metrics @@ -1625,6 +1629,10 @@ defmodule PlausibleWeb.Api.StatsController do defp realtime_period_to_30m(params), do: params + defp toplevel_goal_filter?(query) do + Filters.filtering_on_dimension?(query, "event:goal", max_depth: 0) + end + def scroll_depth_enabled?(site, user) do FunWithFlags.enabled?(:scroll_depth, for: user) || FunWithFlags.enabled?(:scroll_depth, for: site) diff --git a/lib/plausible_web/controllers/stats_controller.ex b/lib/plausible_web/controllers/stats_controller.ex index 686567336fa0..336571fbd19e 100644 --- a/lib/plausible_web/controllers/stats_controller.ex +++ b/lib/plausible_web/controllers/stats_controller.ex @@ -194,10 +194,10 @@ defmodule PlausibleWeb.StatsController do include_scroll_depth? = !query.include_imported && PlausibleWeb.Api.StatsController.scroll_depth_enabled?(site, current_user) && - Filters.filtering_on_dimension?(query, "event:page") + Filters.filtering_on_dimension?(query, "event:page", behavioral_filters: :ignore) {metrics, column_headers} = - if Filters.filtering_on_dimension?(query, "event:goal") do + if Filters.filtering_on_dimension?(query, "event:goal", max_depth: 0) do { [:visitors, :events, :conversion_rate], [:date, :unique_conversions, :total_conversions, :conversion_rate] diff --git a/priv/json-schemas/query-api-schema.json b/priv/json-schemas/query-api-schema.json index 6a325c8d8e0e..0c95244ce979 100644 --- a/priv/json-schemas/query-api-schema.json +++ b/priv/json-schemas/query-api-schema.json @@ -472,7 +472,11 @@ "oneOf": [ { "$ref": "#/definitions/filter_entry" }, { "$ref": "#/definitions/filter_and_or" }, - { "$ref": "#/definitions/filter_not" } + { "$ref": "#/definitions/filter_not" }, + { + "$ref": "#/definitions/filter_has_done", + "$comment": "only :internal" + } ] }, "filter_not": { @@ -480,7 +484,13 @@ "additionalItems": false, "minItems": 2, "maxItems": 2, - "items": [{ "const": "not" }, { "$ref": "#/definitions/filter_tree" }] + "items": [ + { + "type": "string", + "enum": ["not"] + }, + { "$ref": "#/definitions/filter_tree" } + ] }, "filter_and_or": { "type": "array", @@ -499,6 +509,19 @@ } ] }, + "filter_has_done": { + "type": "array", + "additionalItems": false, + "minItems": 2, + "maxItems": 2, + "items": [ + { + "type": "string", + "enum": ["has_done", "has_not_done"] + }, + { "$ref": "#/definitions/filter_tree" } + ] + }, "order_by_entry": { "type": "array", "additionalItems": false, diff --git a/test/plausible/stats/query_parser_test.exs b/test/plausible/stats/query_parser_test.exs index 968d04fa0fcf..338e071e7922 100644 --- a/test/plausible/stats/query_parser_test.exs +++ b/test/plausible/stats/query_parser_test.exs @@ -84,16 +84,18 @@ defmodule Plausible.Stats.Filters.QueryParserTest do end def check_goals(actual, opts) do - preloaded_goal_names = - actual[:preloaded_goals] - |> Enum.map(& &1.display_name) - |> Enum.sort() + assert goal_names(actual[:preloaded_goals][:all]) == + Enum.sort(Keyword.get(opts, :preloaded_goals)[:all]) + + assert goal_names(actual[:preloaded_goals][:matching_toplevel_filters]) == + Enum.sort(Keyword.get(opts, :preloaded_goals)[:matching_toplevel_filters]) - assert preloaded_goal_names == Keyword.get(opts, :preloaded_goals) assert actual[:revenue_warning] == Keyword.get(opts, :revenue_warning) assert actual[:revenue_currencies] == Keyword.get(opts, :revenue_currencies) end + defp goal_names(goals), do: Enum.map(goals, & &1.display_name) |> Enum.sort() + test "parsing empty map fails", %{site: site} do %{} |> check_error(site, "#: Required properties site_id, metrics, date_range were not present.") @@ -265,7 +267,9 @@ defmodule Plausible.Stats.Filters.QueryParserTest do ["or", []], ["not"], ["is_not"], - ["is_not", "event:name"] + ["is_not", "event:name"], + ["has_done"], + ["has_not_done"] ] do test "errors on too short filter #{inspect(too_short_filter)}", %{ site: site @@ -542,32 +546,97 @@ defmodule Plausible.Stats.Filters.QueryParserTest do }) end - test "invalid `not` clause", %{site: site} do + test "valid has_done and has_not_done filters", %{site: site} do + insert(:goal, %{site: site, event_name: "Signup"}) + + %{ + "site_id" => site.domain, + "metrics" => ["visitors"], + "date_range" => "all", + "filters" => [ + ["has_done", ["is", "event:name", ["Signup"]]], + [ + "has_not_done", + [ + "or", + [ + ["is", "event:goal", ["Signup"]], + ["is", "event:page", ["/signup"]] + ] + ] + ] + ] + } + |> check_success( + site, + %{ + metrics: [:visitors], + utc_time_range: @date_range_day, + filters: [ + [:has_done, [:is, "event:name", ["Signup"]]], + [ + :has_not_done, + [:or, [[:is, "event:goal", ["Signup"]], [:is, "event:page", ["/signup"]]]] + ] + ], + dimensions: [], + order_by: nil, + timezone: site.timezone, + include: %{imports: false, time_labels: false, total_rows: false, comparisons: nil}, + pagination: %{limit: 10_000, offset: 0} + }, + :internal + ) + end + + test "fails when using visit filters within has_done filters", %{site: site} do %{ "site_id" => site.domain, "metrics" => ["visitors"], "date_range" => "all", - "filters" => [["not", []]] + "filters" => [ + ["has_done", ["is", "visit:browser", ["Chrome"]]] + ] } |> check_error( site, - "#/filters/0: Invalid filter [\"not\", []]" + "Invalid filters. Behavioral filters (has_done, has_not_done) can only be used with event dimension filters.", + :internal ) end - test "invalid `or` clause", %{site: site} do + test "fails when nesting behavioral filters", %{site: site} do %{ "site_id" => site.domain, "metrics" => ["visitors"], "date_range" => "all", - "filters" => [["or", []]] + "filters" => [ + ["has_done", ["has_not_done", ["is", "visit:browser", ["Chrome"]]]] + ] } |> check_error( site, - "#/filters/0: Invalid filter [\"or\", []]" + "Invalid filters. Behavioral filters (has_done, has_not_done) cannot be nested.", + :internal ) end + for operator <- ["not", "or", "has_done", "has_not_done"] do + test "invalid `#{operator}` clause", %{site: site} do + %{ + "site_id" => site.domain, + "metrics" => ["visitors"], + "date_range" => "all", + "filters" => [[unquote(operator), []]] + } + |> check_error( + site, + "#/filters/0: Invalid filter [\"#{unquote(operator)}\", []]", + :internal + ) + end + end + test "event:hostname filter", %{site: site} do %{ "site_id" => site.domain, @@ -689,7 +758,13 @@ defmodule Plausible.Stats.Filters.QueryParserTest do 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: %{}) + |> check_goals( + preloaded_goals: %{ + all: ["Contact", "Purchase", "Signup"], + matching_toplevel_filters: ["Purchase", "Signup"] + }, + revenue_currencies: %{} + ) end test "with case insensitive match", %{site: site} do @@ -709,7 +784,13 @@ defmodule Plausible.Stats.Filters.QueryParserTest do 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: %{}) + |> check_goals( + preloaded_goals: %{ + all: ["Contact", "Purchase", "Signup"], + matching_toplevel_filters: ["Purchase", "Signup"] + }, + revenue_currencies: %{} + ) end test "with contains match", %{site: site} do @@ -729,7 +810,13 @@ defmodule Plausible.Stats.Filters.QueryParserTest do include: %{imports: false, time_labels: false, total_rows: false, comparisons: nil}, pagination: %{limit: 10_000, offset: 0} }) - |> check_goals(preloaded_goals: ["Signup"], revenue_currencies: %{}) + |> check_goals( + preloaded_goals: %{ + all: ["Contact", "Purchase", "Signup"], + matching_toplevel_filters: ["Signup"] + }, + revenue_currencies: %{} + ) end test "with case insensitive contains match", %{site: site} do @@ -749,7 +836,13 @@ defmodule Plausible.Stats.Filters.QueryParserTest do 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: %{}) + |> check_goals( + preloaded_goals: %{ + all: ["Contact", "Purchase", "Signup"], + matching_toplevel_filters: ["Contact", "Signup"] + }, + revenue_currencies: %{} + ) end end @@ -983,7 +1076,7 @@ defmodule Plausible.Stats.Filters.QueryParserTest do insert(:goal, %{site: site, event_name: "Signup"}) insert(:goal, %{site: site, page_path: "/thank-you"}) - params = %{ + %{ "site_id" => site.domain, "metrics" => ["visitors"], "date_range" => "all", @@ -991,26 +1084,29 @@ defmodule Plausible.Stats.Filters.QueryParserTest do ["is", "event:goal", ["Signup", "Visit /thank-you"]] ] } - - assert {:ok, res} = parse(site, :public, params, @now) - expected_timezone = site.timezone - - assert %{ - metrics: [:visitors], - utc_time_range: @date_range_day, - filters: [ - [:is, "event:goal", ["Signup", "Visit /thank-you"]] - ], - dimensions: [], - order_by: nil, - timezone: ^expected_timezone, - include: %{imports: false, time_labels: false, total_rows: false, comparisons: nil}, - pagination: %{limit: 10_000, offset: 0}, - preloaded_goals: [ - %Plausible.Goal{page_path: "/thank-you"}, - %Plausible.Goal{event_name: "Signup"} - ] - } = res + |> check_success( + site, + %{ + metrics: [:visitors], + utc_time_range: @date_range_day, + filters: [ + [:is, "event:goal", ["Signup", "Visit /thank-you"]] + ], + 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: %{ + all: ["Signup", "Visit /thank-you"], + matching_toplevel_filters: ["Signup", "Visit /thank-you"] + }, + revenue_warning: nil, + revenue_currencies: %{} + ) end test "invalid event filter", %{site: site} do @@ -1078,6 +1174,74 @@ defmodule Plausible.Stats.Filters.QueryParserTest do "Invalid filters. Dimension `event:goal` can only be filtered at the top level." ) end + + test "allowed within behavioral filters has_done", %{site: site} do + insert(:goal, %{site: site, event_name: "Signup"}) + + %{ + "site_id" => site.domain, + "metrics" => ["visitors"], + "date_range" => "all", + "filters" => [ + [ + "has_done", + [ + "or", + [ + ["is", "event:goal", ["Signup"]], + ["is", "event:name", ["pageview"]] + ] + ] + ] + ] + } + |> check_success( + site, + %{ + metrics: [:visitors], + utc_time_range: @date_range_day, + filters: [ + [ + :has_done, + [ + :or, + [ + [:is, "event:goal", ["Signup"]], + [:is, "event:name", ["pageview"]] + ] + ] + ] + ], + dimensions: [], + order_by: nil, + timezone: site.timezone, + include: %{imports: false, time_labels: false, total_rows: false, comparisons: nil}, + pagination: %{limit: 10_000, offset: 0} + }, + :internal + ) + |> check_goals( + preloaded_goals: %{all: ["Signup"], matching_toplevel_filters: ["Signup"]}, + revenue_warning: nil, + revenue_currencies: %{} + ) + end + + test "name is checked even within behavioral filters", %{site: site} do + insert(:goal, %{site: site, event_name: "Signup"}) + + %{ + "site_id" => site.domain, + "metrics" => ["visitors"], + "date_range" => "all", + "filters" => [["has_done", ["is", "event:goal", ["Unknown"]]]] + } + |> check_error( + site, + "The goal `Unknown` is not configured for this site. Find out how to configure goals here: https://plausible.io/docs/stats-api#filtering-by-goals", + :internal + ) + end end describe "date range validation" do @@ -1506,7 +1670,13 @@ defmodule Plausible.Stats.Filters.QueryParserTest do include: %{imports: false, time_labels: false, total_rows: false, comparisons: nil}, pagination: %{limit: 10_000, offset: 0} }) - |> check_goals(preloaded_goals: ["Signup"], revenue_currencies: %{}) + |> check_goals( + preloaded_goals: %{ + all: ["Purchase", "Signup"], + matching_toplevel_filters: ["Signup"] + }, + revenue_currencies: %{} + ) end test "succeeds with event:goal dimension", %{site: site} do @@ -1529,7 +1699,13 @@ defmodule Plausible.Stats.Filters.QueryParserTest do 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: %{}) + |> check_goals( + preloaded_goals: %{ + all: ["Purchase", "Signup"], + matching_toplevel_filters: ["Purchase", "Signup"] + }, + revenue_currencies: %{} + ) end test "custom properties filter with special metric", %{site: site} do @@ -1664,7 +1840,10 @@ defmodule Plausible.Stats.Filters.QueryParserTest do include: %{imports: false, time_labels: false, total_rows: false, comparisons: nil}, pagination: %{limit: 10_000, offset: 0} }) - |> check_goals(preloaded_goals: ["Signup"], revenue_currencies: %{}) + |> check_goals( + preloaded_goals: %{all: ["Signup"], matching_toplevel_filters: ["Signup"]}, + revenue_currencies: %{} + ) end test "fails validation if event:page filter specified", %{site: site} do @@ -1722,7 +1901,10 @@ defmodule Plausible.Stats.Filters.QueryParserTest do } ) |> check_goals( - preloaded_goals: [], + preloaded_goals: %{ + all: [], + matching_toplevel_filters: [] + }, revenue_warning: :no_revenue_goals_matching, revenue_currencies: %{} ) @@ -1777,7 +1959,10 @@ defmodule Plausible.Stats.Filters.QueryParserTest do } ) |> check_goals( - preloaded_goals: ["PurchaseUSD", "Signup", "Subscription"], + preloaded_goals: %{ + all: ["PurchaseUSD", "Signup", "Subscription", "Logout"], + matching_toplevel_filters: ["PurchaseUSD", "Signup", "Subscription"] + }, revenue_warning: nil, revenue_currencies: %{default: :USD} ) @@ -1808,7 +1993,10 @@ defmodule Plausible.Stats.Filters.QueryParserTest do } ) |> check_goals( - preloaded_goals: ["Purchase", "Signup", "Subscription"], + preloaded_goals: %{ + all: ["Purchase", "Signup", "Subscription"], + matching_toplevel_filters: ["Purchase", "Signup", "Subscription"] + }, revenue_warning: :no_single_revenue_currency, revenue_currencies: %{} ) @@ -1839,7 +2027,10 @@ defmodule Plausible.Stats.Filters.QueryParserTest do } ) |> check_goals( - preloaded_goals: ["Signup"], + preloaded_goals: %{ + all: ["Purchase", "Subscription", "Signup"], + matching_toplevel_filters: ["Signup"] + }, revenue_warning: :no_revenue_goals_matching, revenue_currencies: %{} ) @@ -1870,7 +2061,10 @@ defmodule Plausible.Stats.Filters.QueryParserTest do } ) |> check_goals( - preloaded_goals: ["Donation", "Purchase", "Signup"], + preloaded_goals: %{ + all: ["Donation", "Purchase", "Signup"], + matching_toplevel_filters: ["Donation", "Purchase", "Signup"] + }, revenue_warning: nil, revenue_currencies: %{"Donation" => :EUR, "Purchase" => :USD} ) @@ -1903,7 +2097,10 @@ defmodule Plausible.Stats.Filters.QueryParserTest do } ) |> check_goals( - preloaded_goals: ["Purchase", "Signup", "Subscription"], + preloaded_goals: %{ + all: ["Logout", "Purchase", "Signup", "Subscription"], + matching_toplevel_filters: ["Purchase", "Signup", "Subscription"] + }, revenue_warning: nil, revenue_currencies: %{"Purchase" => :USD, "Subscription" => :EUR} ) @@ -1938,7 +2135,10 @@ defmodule Plausible.Stats.Filters.QueryParserTest do } ) |> check_goals( - preloaded_goals: ["Signup"], + preloaded_goals: %{ + all: ["Logout", "Signup", "Subscription", "Purchase"], + matching_toplevel_filters: ["Signup"] + }, revenue_warning: :no_revenue_goals_matching, revenue_currencies: %{} ) diff --git a/test/plausible/stats/query_test.exs b/test/plausible/stats/query_test.exs index b6928122a0b8..fc148aa7c812 100644 --- a/test/plausible/stats/query_test.exs +++ b/test/plausible/stats/query_test.exs @@ -29,11 +29,11 @@ defmodule Plausible.Stats.QueryTest do } do q1 = %{now: %DateTime{}} = Query.from(site, %{"period" => "realtime"}) q2 = %{now: %DateTime{}} = Query.from(site, %{"period" => "30m"}) - boundaries1 = Plausible.Stats.Time.utc_boundaries(q1, site) - boundaries2 = Plausible.Stats.Time.utc_boundaries(q2, site) + boundaries1 = Plausible.Stats.Time.utc_boundaries(q1) + boundaries2 = Plausible.Stats.Time.utc_boundaries(q2) :timer.sleep(1500) - assert ^boundaries1 = Plausible.Stats.Time.utc_boundaries(q1, site) - assert ^boundaries2 = Plausible.Stats.Time.utc_boundaries(q2, site) + assert ^boundaries1 = Plausible.Stats.Time.utc_boundaries(q1) + assert ^boundaries2 = Plausible.Stats.Time.utc_boundaries(q2) end test "parses day format", %{site: site} do 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 f9cac1f88704..47c2035f0214 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 @@ -1287,4 +1287,79 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryImportedTest do assert pageviews in 1..2 end end + + describe "behavioral filters" do + setup :create_site_import + + test "imports are skipped when has_done filter is used", %{ + conn: conn, + site: site, + site_import: site_import + } do + populate_stats(site, site_import.id, [ + build(:event, name: "pageview", user_id: 1, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "pageview", user_id: 2, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "Conversion", user_id: 3, timestamp: ~N[2021-01-01 00:00:00]), + build(:imported_pages, + page: "/blog", + pageviews: 5, + visitors: 3, + date: ~D[2023-01-01] + ) + ]) + + conn = + post(conn, "/api/v2/query-internal-test", %{ + "site_id" => site.domain, + "metrics" => ["visitors"], + "date_range" => "all", + "filters" => [ + ["has_done", ["is", "event:name", ["pageview"]]] + ], + "include" => %{"imports" => true} + }) + + assert json_response(conn, 200)["results"] == [%{"dimensions" => [], "metrics" => [2]}] + refute json_response(conn, 200)["meta"]["imports_included"] + + assert json_response(conn, 200)["meta"]["imports_warning"] =~ + "Imported stats are not included in the results" + end + + test "imports are skipped when has_not_done filter is used", %{ + conn: conn, + site: site, + site_import: site_import + } do + populate_stats(site, site_import.id, [ + build(:event, name: "pageview", user_id: 1, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "pageview", user_id: 2, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "Conversion", user_id: 3, timestamp: ~N[2021-01-01 00:00:00]), + build(:imported_pages, + page: "/blog", + pageviews: 5, + visitors: 3, + date: ~D[2023-01-01] + ) + ]) + + conn = + post(conn, "/api/v2/query-internal-test", %{ + "site_id" => site.domain, + "metrics" => ["visitors"], + "date_range" => "all", + "dimensions" => ["event:goal"], + "filters" => [ + ["has_not_done", ["is", "event:name", ["pageview"]]] + ], + "include" => %{"imports" => true} + }) + + assert json_response(conn, 200)["results"] == [] + refute json_response(conn, 200)["meta"]["imports_included"] + + assert json_response(conn, 200)["meta"]["imports_warning"] =~ + "Imported stats are not included in the results" + 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 4697b94c7154..33da8e25e69e 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 @@ -4578,6 +4578,296 @@ defmodule PlausibleWeb.Api.ExternalStatsController.QueryTest do end end + describe "behavioral (has_done/has_not_done) filters" do + test "has_done does counting by sessions", %{conn: conn, site: site} do + populate_stats(site, [ + # Session 1 + build(:event, name: "Conversion", user_id: 1, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "pageview", user_id: 1, timestamp: ~N[2021-01-01 00:01:00]), + # Session 2 + build(:event, name: "pageview", user_id: 1, timestamp: ~N[2021-01-02 00:00:00]), + build(:event, name: "pageview", user_id: 1, timestamp: ~N[2021-01-02 00:01:00]), + # Session 3 + build(:event, name: "Conversion", user_id: 1, timestamp: ~N[2021-01-03 00:00:00]), + build(:event, name: "pageview", user_id: 1, timestamp: ~N[2021-01-03 00:01:00]), + build(:event, name: "pageview", user_id: 1, timestamp: ~N[2021-01-03 00:02:00]), + build(:event, name: "pageview", user_id: 1, timestamp: ~N[2021-01-03 00:03:00]), + # Session 4 + build(:event, name: "Conversion", user_id: 2, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "pageview", user_id: 2, timestamp: ~N[2021-01-01 00:01:00]), + build(:event, name: "pageview", user_id: 2, timestamp: ~N[2021-01-01 00:02:00]), + build(:event, name: "pageview", user_id: 2, timestamp: ~N[2021-01-01 00:03:00]), + build(:event, name: "pageview", user_id: 2, timestamp: ~N[2021-01-01 00:04:00]) + ]) + + conn = + post(conn, "/api/v2/query-internal-test", %{ + "site_id" => site.domain, + "metrics" => ["visitors", "visits", "events", "pageviews"], + "date_range" => "all", + "filters" => [["has_done", ["is", "event:name", ["Conversion"]]]] + }) + + assert json_response(conn, 200)["results"] == [ + %{"dimensions" => [], "metrics" => [2, 3, 11, 8]} + ] + end + + test "has_done returns all events by users who match condition if no further filters are provided", + %{conn: conn, site: site} do + populate_stats(site, [ + build(:event, name: "pageview", user_id: 1, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "AddToCart", user_id: 1, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "Purchase", user_id: 1, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "pageview", user_id: 2, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "Purchase", user_id: 2, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "pageview", user_id: 3, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "pageview", user_id: 4, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "Purchase", user_id: 5, timestamp: ~N[2021-01-01 00:00:00]) + ]) + + conn = + post(conn, "/api/v2/query-internal-test", %{ + "site_id" => site.domain, + "metrics" => ["visitors", "events", "pageviews"], + "date_range" => "all", + "filters" => [["has_done", ["is", "event:name", ["Purchase"]]]] + }) + + assert json_response(conn, 200)["results"] == [ + %{"dimensions" => [], "metrics" => [3, 6, 2]} + ] + end + + test "has_done event:page filter", %{conn: conn, site: site} do + populate_stats(site, [ + build(:event, + name: "pageview", + user_id: 1, + pathname: "/blog/post/1", + timestamp: ~N[2021-01-01 00:00:00] + ), + build(:event, + name: "pageview", + user_id: 1, + pathname: "/", + timestamp: ~N[2021-01-01 00:00:00] + ), + build(:event, + name: "pageview", + user_id: 2, + pathname: "/", + timestamp: ~N[2021-01-01 00:00:00] + ), + build(:event, + name: "pageview", + user_id: 3, + pathname: "/blog/post/1", + timestamp: ~N[2021-01-01 00:00:00] + ) + ]) + + conn = + post(conn, "/api/v2/query-internal-test", %{ + "site_id" => site.domain, + "metrics" => ["visitors", "pageviews"], + "date_range" => "all", + "filters" => [["has_done", ["contains", "event:page", ["/blog/"]]]] + }) + + assert json_response(conn, 200)["results"] == [ + %{"dimensions" => [], "metrics" => [2, 3]} + ] + end + + test "has_not_done event:page filter", %{conn: conn, site: site} do + populate_stats(site, [ + build(:event, + name: "pageview", + user_id: 1, + pathname: "/blog/post/1", + timestamp: ~N[2021-01-01 00:00:00] + ), + build(:event, + name: "pageview", + user_id: 1, + pathname: "/", + timestamp: ~N[2021-01-01 00:00:00] + ), + build(:event, + name: "pageview", + user_id: 2, + pathname: "/", + timestamp: ~N[2021-01-01 00:00:00] + ), + build(:event, + name: "pageview", + user_id: 3, + pathname: "/blog/post/1", + timestamp: ~N[2021-01-01 00:00:00] + ) + ]) + + conn = + post(conn, "/api/v2/query-internal-test", %{ + "site_id" => site.domain, + "metrics" => ["visitors", "pageviews"], + "date_range" => "all", + "filters" => [["has_not_done", ["contains", "event:page", ["/blog/"]]]] + }) + + assert json_response(conn, 200)["results"] == [ + %{"dimensions" => [], "metrics" => [1, 1]} + ] + end + + test "has_done with complex event:props and event:name filters", %{conn: conn, site: site} do + populate_stats(site, [ + build(:event, name: "Purchase", user_id: 1, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "Purchase", user_id: 1, timestamp: ~N[2021-01-01 00:10:00]), + build(:event, name: "Purchase", user_id: 1, timestamp: ~N[2021-01-01 00:20:00]), + build(:event, name: "pageview", user_id: 2, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "Purchase", user_id: 2, timestamp: ~N[2021-01-01 00:10:00]), + build(:event, + name: "Signup", + user_id: 3, + "meta.key": ["paid"], + "meta.value": ["true"], + timestamp: ~N[2021-01-01 00:00:00] + ), + build(:event, name: "pageview", user_id: 3, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, + name: "Signup", + user_id: 4, + "meta.key": ["paid"], + "meta.value": ["true"], + timestamp: ~N[2021-01-01 00:00:00] + ), + build(:event, name: "pageview", user_id: 4, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, + name: "Signup", + user_id: 5, + "meta.key": ["paid"], + "meta.value": ["false"], + timestamp: ~N[2021-01-01 00:00:00] + ), + build(:event, name: "pageview", user_id: 5, timestamp: ~N[2021-01-01 00:00:00]) + ]) + + conn = + post(conn, "/api/v2/query-internal-test", %{ + "site_id" => site.domain, + "metrics" => ["visitors", "pageviews"], + "date_range" => "all", + "filters" => [ + [ + "has_done", + [ + "or", + [ + ["is", "event:name", ["Purchase"]], + [ + "and", + [ + ["is", "event:name", ["Signup"]], + ["is", "event:props:paid", ["true"]] + ] + ] + ] + ] + ] + ] + }) + + assert json_response(conn, 200)["results"] == [ + %{"dimensions" => [], "metrics" => [4, 3]} + ] + end + + test "has_done event:goal filter", %{conn: conn, site: site} do + insert(:goal, site: site, event_name: "Conversion") + + populate_stats(site, [ + build(:event, name: "Conversion", user_id: 1, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "Conversion", user_id: 1, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "pageview", user_id: 1, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "pageview", user_id: 1, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "pageview", user_id: 1, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "Conversion", user_id: 2, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "pageview", user_id: 2, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "Conversion", user_id: 3, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "pageview", user_id: 4, timestamp: ~N[2021-01-01 00:00:00]) + ]) + + conn = + post(conn, "/api/v2/query-internal-test", %{ + "site_id" => site.domain, + "metrics" => ["visitors", "pageviews"], + "date_range" => "all", + "filters" => [ + ["has_done", ["is", "event:goal", ["Conversion"]]], + ["is", "event:name", ["pageview"]] + ] + }) + + assert json_response(conn, 200)["results"] == [ + %{"dimensions" => [], "metrics" => [2, 4]} + ] + end + + test "has_not_done event:goal filter", %{conn: conn, site: site} do + insert(:goal, site: site, event_name: "Conversion") + + populate_stats(site, [ + build(:event, name: "Conversion", user_id: 1, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "Conversion", user_id: 1, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "pageview", user_id: 1, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "pageview", user_id: 1, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "pageview", user_id: 1, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "Conversion", user_id: 2, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "pageview", user_id: 2, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "Conversion", user_id: 3, timestamp: ~N[2021-01-01 00:00:00]), + build(:event, name: "pageview", user_id: 4, timestamp: ~N[2021-01-01 00:00:00]) + ]) + + conn = + post(conn, "/api/v2/query-internal-test", %{ + "site_id" => site.domain, + "metrics" => ["visitors", "pageviews"], + "date_range" => "all", + "filters" => [ + ["has_not_done", ["is", "event:goal", ["Conversion"]]], + ["is", "event:name", ["pageview"]] + ] + }) + + assert json_response(conn, 200)["results"] == [ + %{"dimensions" => [], "metrics" => [1, 1]} + ] + end + + test "visit filters are not allowed with has_done/has_not_done filters", %{ + conn: conn, + site: site + } do + conn = + post(conn, "/api/v2/query-internal-test", %{ + "site_id" => site.domain, + "metrics" => ["visitors"], + "date_range" => "all", + "dimensions" => ["visit:browser"], + "filters" => [ + ["has_done", ["is", "visit:browser", ["Chrome"]]] + ] + }) + + assert %{"error" => error} = json_response(conn, 400) + + assert error =~ + "Invalid filters. Behavioral filters (has_done, has_not_done) can only be used with event dimension filters." + end + end + describe "segment filters" do setup [:create_user, :create_site, :create_api_key, :use_api_key]