-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
APIv2: Behavioral filtering #4980
Conversation
d00c02d
to
4088c79
Compare
defp toplevel_goal_filter?(query) do | ||
Filters.filtering_on_dimension?(query, "event:goal", max_depth: 0) | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
General question, can be taken offline: why is this a concern, but not event:name
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the dashboard has toggles for behavior based on if there's a goal filter or not. Mostly for checking whether to query/show conversion_rate metric and revenue metrics. This needed to change since conversion_rate is not calculatable with a behavioral goal filter (e.g. has not done some goal).
event:name and other event dimensions don't have this sort of special-casing.
Note this could have been in the follow-up PR but decided to split by the FE/BE sides for completeness sake.
# 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is all
used and why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question.
I needed to change the structure of preloaded_goals
since we need to know what goals match the top-level is/contains filter for group by and other logics as it will affect what gets displayed (e.g. do all have the same currency, whether we can include imports. For this reason matching_toplevel_filters
exists.
Behavioral filters has_done
and has_not_done
do not impact these bits of business logic but we still need to know what condition(s) these goals have when building the Ecto Query. Hence the all
key. This gets used only in this same file when building the Ecto query:
Enum.reduce(clauses, false, fn clause, dynamic_statement ->
condition =
query.preloaded_goals.all
|> filter_preloaded(filter, clause)
|> build_condition(imported?)
dynamic([e], ^condition or ^dynamic_statement)
end)
This allows to do more query-building without exposing and passing `site` directly.
No event:goal support yet, no validations
Minor changes along the way: - preloaded_goals structure changes - event:goal restrictions were loosened within has_done - we don't allow nesting has_done anymore
Changes
This PR adds support for behavioral filtering by adding two new operators
has_done
andhas_not_done
to APIv2.These operators can be used to filter whether users have completed (or not completed) a given at any point in their session. This can be used to exclude goals or check whether a user has done an additional action in their session.
In the following example, we could pageviews and visitors on /pricing page, but only by users who have opened support chat but have not registered or logged in either before or after visiting the page.
Docs PR: plausible/docs#575The operators are currently internal-only.Basecamp ref: https://3.basecamp.com/5308029/buckets/26383192/card_tables/cards/7793417773
What's to be done
I'll open another PR to add support for users checking whether user has not done a goal. This turned out to be a bigger rabbit hole due to UI issues hence the split.