Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple filters on the frontend #4174

Merged
merged 16 commits into from
Jun 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions assets/js/dashboard/stats/modals/filter-modal-group.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,17 @@ export default function FilterModalGroup({
() => Object.entries(filterState).filter(([_, filter]) => getFilterGroup(filter) == filterGroup).map(([id, filter]) => ({ id, filter })),
[filterGroup, filterState]
)

const disabledOptions = useMemo(
() => (filterGroup == 'props') ? rows.map(({ filter }) => ({ value: getPropertyKeyFromFilterKey(filter[1]) })) : null,
[filterGroup, rows]
)

const showAddRow = filterGroup == 'props'
const showAddRow = site.flags.multiple_filters ? !['goal', 'hostname'].includes(filterGroup) : filterGroup == 'props'
const showTitle = filterGroup != 'props'

return (
<>
<div className="mt-4">
<div className="mt-6">
{showTitle && (<div className="text-sm font-medium text-gray-700 dark:text-gray-300">{formattedFilters[filterGroup]}</div>)}
{rows.map(({ id, filter }) =>
filterGroup === 'props' ? (
Expand All @@ -55,7 +54,7 @@ export default function FilterModalGroup({
)}
</div>
{showAddRow && (
<div className="mt-6">
<div className="mt-2">
<a className="underline text-indigo-500 text-sm cursor-pointer" onClick={() => onAddRow(filterGroup)}>
+ Add another
</a>
Expand Down
37 changes: 17 additions & 20 deletions assets/js/dashboard/util/filters.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ export const OPERATION_PREFIX = {
[FILTER_OPERATIONS.is]: ''
};

export const BACKEND_OPERATION = {
[FILTER_OPERATIONS.is]: 'is',
[FILTER_OPERATIONS.isNot]: 'is_not',
[FILTER_OPERATIONS.contains]: 'matches'
}

export function supportsIsNot(filterName) {
return !['goal', 'prop_key'].includes(filterName)
}
Expand All @@ -53,17 +59,6 @@ try {

const ESCAPED_PIPE = '\\|'

function escapeFilterValue(value) {
return value.replaceAll(NON_ESCAPED_PIPE_REGEX, ESCAPED_PIPE)
}

function toFilterQuery(type, clauses) {
const prefix = OPERATION_PREFIX[type];
const result = clauses.map(clause => escapeFilterValue(clause.toString().trim())).join('|')
return prefix + result;
}


export function getLabel(labels, filterKey, value) {
if (['country', 'region', 'city'].includes(filterKey)) {
return labels[value]
Expand Down Expand Up @@ -141,19 +136,21 @@ export function cleanLabels(filters, labels, mergedFilterKey, mergedLabels) {
return result
}

const EVENT_FILTER_KEYS = new Set(["name", "page", "goal", "hostname"])

// :TODO: New schema for filters in the BE
export function serializeApiFilters(filters) {
const cleaned = {}
filters.forEach(([operation, filterKey, clauses]) => {
if (filterKey.startsWith(EVENT_PROPS_PREFIX)) {
cleaned.props ||= {}
cleaned.props[getPropertyKeyFromFilterKey(filterKey)] = toFilterQuery(operation, clauses)
} else {
cleaned[filterKey] = toFilterQuery(operation, clauses)
const apiFilters = filters.map(([operation, filterKey, clauses]) => {
let apiFilterKey = `visit:${filterKey}`
if (filterKey.startsWith(EVENT_PROPS_PREFIX) || EVENT_FILTER_KEYS.has(filterKey)) {
apiFilterKey = `event:${filterKey}`
}
if (operation == FILTER_OPERATIONS.contains) {
clauses = clauses.map((value) => value.includes('*') ? value : `**${value}**`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to go in the direction that matches is a first-class operation that would also be available in the API. Am I reading it right?

My thinking on what filters should be available to client:

  • is - can deal with lists like you've already done in this PR. Can also take wildcards.
  • contains - equivalent to is where value is wrapped with *${value}*

I'm not sure about exposing matches for simple wildcards. It would fit better for a full regex match filter operation in the future.

WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I completely understood. Created https://3.basecamp.com/5308029/buckets/37310803/card_tables/cards/7494308360 for this discussion so it doesn't get dropped.

}
return [BACKEND_OPERATION[operation], apiFilterKey, clauses]
})
return JSON.stringify(cleaned)

return JSON.stringify(apiFilters)
}

export function fetchSuggestions(apiPath, query, input, additionalFilter) {
Expand Down
3 changes: 1 addition & 2 deletions extra/lib/plausible/stats/goal/revenue.ex
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,7 @@ defmodule Plausible.Stats.Goal.Revenue do
def get_revenue_tracking_currency(site, query, metrics) do
goal_filters =
case Query.get_filter(query, "event:goal") do
[:is, "event:goal", {_, goal_name}] -> [goal_name]
[:member, "event:goal", list] -> Enum.map(list, fn {_, goal_name} -> goal_name end)
[:is, "event:goal", list] -> Enum.map(list, fn {_, goal_name} -> goal_name end)
_ -> []
end

Expand Down
27 changes: 5 additions & 22 deletions lib/plausible/google/search_console/filters.ex
Original file line number Diff line number Diff line change
Expand Up @@ -23,44 +23,27 @@ defmodule Plausible.Google.SearchConsole.Filters do
transform_filter(property, [op, "visit:entry_page" | rest])
end

defp transform_filter(property, [:is, "visit:entry_page", page]) when is_binary(page) do
%{dimension: "page", expression: property_url(property, page)}
end

defp transform_filter(property, [:member, "visit:entry_page", pages]) when is_list(pages) do
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, "visit:entry_page", page]) when is_binary(page) do
page = page_regex(property_url(property, page))
%{dimension: "page", operator: "includingRegex", expression: page}
end

defp transform_filter(property, [:matches_member, "visit:entry_page", pages])
defp transform_filter(property, [:matches, "visit:entry_page", pages])
when is_list(pages) do
expression =
Enum.map_join(pages, "|", fn page -> page_regex(property_url(property, page)) end)

%{dimension: "page", operator: "includingRegex", expression: expression}
end

defp transform_filter(_property, [:is, "visit:screen", device]) when is_binary(device) do
%{dimension: "device", expression: search_console_device(device)}
end

defp transform_filter(_property, [:member, "visit:screen", devices]) when is_list(devices) do
expression = devices |> Enum.join("|")
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", country]) when is_binary(country) do
%{dimension: "country", expression: search_console_country(country)}
end

defp transform_filter(_property, [:member, "visit:country", countries])
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}
Expand Down
4 changes: 2 additions & 2 deletions lib/plausible/stats/base.ex
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ defmodule Plausible.Stats.Base do

def add_percentage_metric(q, site, query, metrics) do
if :percentage in metrics do
total_query = Query.set_property(query, nil)
total_query = Query.set_dimensions(query, [])

q
|> select_merge(
Expand Down Expand Up @@ -348,7 +348,7 @@ defmodule Plausible.Stats.Base do
total_query =
query
|> Query.remove_filters(["event:goal", "event:props"])
|> Query.set_property(nil)
|> Query.set_dimensions([])

# :TRICKY: Subquery is used due to event:goal breakdown above doing an UNION ALL
subquery(q)
Expand Down
48 changes: 24 additions & 24 deletions lib/plausible/stats/breakdown.ex
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ defmodule Plausible.Stats.Breakdown do

def breakdown(
site,
%Query{property: "event:goal"} = query,
%Query{dimensions: ["event:goal"]} = query,
metrics,
pagination,
opts
Expand All @@ -39,8 +39,8 @@ defmodule Plausible.Stats.Breakdown do

event_query =
query
|> Query.put_filter([:member, "event:name", events])
|> Query.set_property("event:name")
|> Query.put_filter([:is, "event:name", events])
|> Query.set_dimensions(["event:name"])

if !Keyword.get(opts, :skip_tracing), do: Query.trace(query, metrics)

Expand Down Expand Up @@ -73,7 +73,7 @@ defmodule Plausible.Stats.Breakdown do

page_q =
if Enum.any?(pageview_goals) do
page_query = Query.set_property(query, "event:page")
page_query = Query.set_dimensions(query, ["event:page"])

page_exprs = Enum.map(pageview_goals, & &1.page_path)
page_regexes = Enum.map(page_exprs, &page_regex/1)
Expand Down Expand Up @@ -134,7 +134,7 @@ defmodule Plausible.Stats.Breakdown do

def breakdown(
site,
%Query{property: "event:props:" <> custom_prop} = query,
%Query{dimensions: ["event:props:" <> custom_prop]} = query,
metrics,
pagination,
opts
Expand All @@ -157,7 +157,7 @@ defmodule Plausible.Stats.Breakdown do
|> Enum.map(&cast_revenue_metrics_to_money(&1, currency))
end

def breakdown(site, %Query{property: "event:page"} = query, metrics, pagination, opts) do
def breakdown(site, %Query{dimensions: ["event:page"]} = query, metrics, pagination, opts) do
event_metrics =
metrics
|> Util.maybe_add_visitors_metric()
Expand All @@ -182,8 +182,8 @@ defmodule Plausible.Stats.Breakdown do
pages ->
query
|> Query.remove_filters(["event:page"])
|> Query.put_filter([:member, "visit:entry_page", Enum.map(pages, & &1[:page])])
|> Query.set_property("visit:entry_page")
|> Query.put_filter([:is, "visit:entry_page", Enum.map(pages, & &1[:page])])
|> Query.set_dimensions(["visit:entry_page"])
end

if Enum.any?(event_metrics) && Enum.empty?(event_result) do
Expand All @@ -208,7 +208,7 @@ defmodule Plausible.Stats.Breakdown do
end
end

def breakdown(site, %Query{property: "event:name"} = query, metrics, pagination, opts) do
def breakdown(site, %Query{dimensions: ["event:name"]} = query, metrics, pagination, opts) do
if !Keyword.get(opts, :skip_tracing), do: Query.trace(query, metrics)

breakdown_events(site, query, metrics)
Expand All @@ -234,7 +234,7 @@ defmodule Plausible.Stats.Breakdown do
end
end

defp maybe_update_breakdown_filters(%Query{property: visit_entry_prop} = query)
defp maybe_update_breakdown_filters(%Query{dimensions: [visit_entry_prop]} = query)
when visit_entry_prop in [
"visit:source",
"visit:entry_page",
Expand All @@ -249,7 +249,7 @@ defmodule Plausible.Stats.Breakdown do
update_hostname_filter_prop(query, "visit:entry_page_hostname")
end

defp maybe_update_breakdown_filters(%Query{property: "visit:exit_page"} = query) do
defp maybe_update_breakdown_filters(%Query{dimensions: ["visit:exit_page"]} = query) do
update_hostname_filter_prop(query, "visit:exit_page_hostname")
end

Expand All @@ -271,13 +271,13 @@ defmodule Plausible.Stats.Breakdown do
# Backwards compatibility
defp breakdown_table(%Query{experimental_reduced_joins?: false}, _), do: :session

defp breakdown_table(%Query{property: "visit:entry_page"}, _metrics), do: :session
defp breakdown_table(%Query{property: "visit:entry_page_hostname"}, _metrics), do: :session
defp breakdown_table(%Query{property: "visit:exit_page"}, _metrics), do: :session
defp breakdown_table(%Query{property: "visit:exit_page_hostname"}, _metrics), do: :session
defp breakdown_table(%Query{dimensions: ["visit:entry_page"]}, _metrics), do: :session
defp breakdown_table(%Query{dimensions: ["visit:entry_page_hostname"]}, _metrics), do: :session
defp breakdown_table(%Query{dimensions: ["visit:exit_page"]}, _metrics), do: :session
defp breakdown_table(%Query{dimensions: ["visit:exit_page_hostname"]}, _metrics), do: :session

defp breakdown_table(%Query{property: property} = query, metrics) do
{_, session_metrics, _} = TableDecider.partition_metrics(metrics, query, property)
defp breakdown_table(%Query{dimensions: [dimension]} = query, metrics) do
{_, session_metrics, _} = TableDecider.partition_metrics(metrics, query, dimension)

if not Enum.empty?(session_metrics) do
:session
Expand All @@ -304,23 +304,23 @@ defmodule Plausible.Stats.Breakdown do
|> sort_results(metrics)
end

defp breakdown_sessions(site, %Query{property: property} = query, metrics) do
defp breakdown_sessions(site, %Query{dimensions: [dimension]} = query, metrics) do
from(s in query_sessions(site, query),
order_by: [desc: fragment("uniq(?)", s.user_id)],
select: ^select_session_metrics(metrics, query)
)
|> filter_converted_sessions(site, query)
|> do_group_by(property)
|> do_group_by(dimension)
|> merge_imported(site, query, metrics)
|> add_percentage_metric(site, query, metrics)
end

defp breakdown_events(site, %Query{property: property} = query, metrics) do
defp breakdown_events(site, %Query{dimensions: [dimension]} = query, metrics) do
from(e in base_event_query(site, query),
order_by: [desc: fragment("uniq(?)", e.user_id)],
select: %{}
)
|> do_group_by(property)
|> do_group_by(dimension)
|> select_merge(^select_event_metrics(metrics))
|> merge_imported(site, query, metrics)
|> add_percentage_metric(site, query, metrics)
Expand Down Expand Up @@ -718,7 +718,7 @@ defmodule Plausible.Stats.Breakdown do
q,
breakdown_fn,
site,
%Query{property: property} = query,
%Query{dimensions: [dimension]} = query,
metrics
) do
if :conversion_rate in metrics do
Expand All @@ -730,7 +730,7 @@ defmodule Plausible.Stats.Breakdown do

from(e in subquery(q),
left_join: c in subquery(breakdown_total_visitors_q),
on: ^on_matches_group_by(group_by_field_names(property)),
on: ^on_matches_group_by(group_by_field_names(dimension)),
select_merge: %{
total_visitors: c.visitors,
conversion_rate:
Expand All @@ -742,7 +742,7 @@ defmodule Plausible.Stats.Breakdown do
)
},
order_by: [desc: e.visitors],
order_by: ^outer_order_by(group_by_field_names(property))
order_by: ^outer_order_by(group_by_field_names(dimension))
)
else
q
Expand Down
6 changes: 3 additions & 3 deletions lib/plausible/stats/email_report.ex
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,16 @@ defmodule Plausible.Stats.EmailReport do
end

defp put_top_5_pages(stats, site, query) do
query = Query.set_property(query, "event:page")
query = Query.set_dimensions(query, ["event:page"])
pages = Stats.breakdown(site, query, [:visitors], {5, 1})
Map.put(stats, :pages, pages)
end

defp put_top_5_sources(stats, site, query) do
query =
query
|> Query.put_filter([:is_not, "visit:source", "Direct / None"])
|> Query.set_property("visit:source")
|> Query.put_filter([:is_not, "visit:source", ["Direct / None"]])
|> Query.set_dimensions(["visit:source"])

sources = Stats.breakdown(site, query, [:visitors], {5, 1})

Expand Down
Loading