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

feat(hogql): test account filters for EventsQuery and HogQLQuery #20308

Merged
merged 19 commits into from
Feb 14, 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
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
77 changes: 77 additions & 0 deletions frontend/src/queries/nodes/DataNode/TestAccountFilters.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import { useActions, useValues } from 'kea'
import { IconSettings } from 'lib/lemon-ui/icons'
import { LemonButton } from 'lib/lemon-ui/LemonButton'
import { LemonSwitch } from 'lib/lemon-ui/LemonSwitch'
import { filterTestAccountsDefaultsLogic } from 'scenes/settings/project/filterTestAccountDefaultsLogic'
import { teamLogic } from 'scenes/teamLogic'
import { urls } from 'scenes/urls'

import { DataNode, EventsQuery, HogQLQuery } from '~/queries/schema'
import { isEventsQuery, isHogQLQuery } from '~/queries/utils'

interface TestAccountFiltersProps {
query: DataNode
setQuery?: (query: EventsQuery | HogQLQuery) => void
}
export function TestAccountFilters({ query, setQuery }: TestAccountFiltersProps): JSX.Element | null {
const { currentTeam } = useValues(teamLogic)
const hasFilters = (currentTeam?.test_account_filters || []).length > 0
const { setLocalDefault } = useActions(filterTestAccountsDefaultsLogic)

if (!isEventsQuery(query) && !isHogQLQuery(query)) {
return null
}
const checked = hasFilters
? !!(isHogQLQuery(query)
? query.filters?.filterTestAccounts
: isEventsQuery(query)
? query.filterTestAccounts
: false)
: false
const onChange = isHogQLQuery(query)
? (checked: boolean) => {
const newQuery: HogQLQuery = {
...query,
filters: {
...query.filters,
filterTestAccounts: checked,
},
}
setQuery?.(newQuery)
}
: isEventsQuery(query)
? (checked: boolean) => {
const newQuery: EventsQuery = {
...query,
filterTestAccounts: checked,
}
setQuery?.(newQuery)
}
: undefined

return (
<LemonSwitch
checked={checked}
onChange={(checked: boolean) => {
onChange?.(checked)
setLocalDefault(checked)
}}
id="test-account-filter"
bordered
label={
<div className="flex items-center">
<span>Filter out internal and test users</span>
<LemonButton
icon={<IconSettings />}
to={urls.settings('project-product-analytics', 'internal-user-filtering')}
size="small"
noPadding
className="ml-1"
/>
</div>
}
disabledReason={!hasFilters ? "You haven't set any internal and test filters" : null}
/>
)
return null
}
5 changes: 5 additions & 0 deletions frontend/src/queries/nodes/DataTable/DataTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { DateRange } from '~/queries/nodes/DataNode/DateRange'
import { ElapsedTime } from '~/queries/nodes/DataNode/ElapsedTime'
import { LoadNext } from '~/queries/nodes/DataNode/LoadNext'
import { Reload } from '~/queries/nodes/DataNode/Reload'
import { TestAccountFilters } from '~/queries/nodes/DataNode/TestAccountFilters'
import { BackToSource } from '~/queries/nodes/DataTable/BackToSource'
import { ColumnConfigurator } from '~/queries/nodes/DataTable/ColumnConfigurator/ColumnConfigurator'
import { DataTableExport } from '~/queries/nodes/DataTable/DataTableExport'
Expand Down Expand Up @@ -130,6 +131,7 @@ export function DataTable({
const {
showActions,
showDateRange,
showTestAccountFilters,
showSearch,
showEventFilter,
showPropertyFilter,
Expand Down Expand Up @@ -420,6 +422,9 @@ export function DataTable({
].filter((x) => !!x)

const firstRowRight = [
showTestAccountFilters && sourceFeatures.has(QueryFeature.testAccountFilters) ? (
<TestAccountFilters key="test-account-filters" query={query.source} setQuery={setQuerySource} />
) : null,
showSavedQueries && sourceFeatures.has(QueryFeature.savedEventsQueries) ? (
<SavedQueries key="saved-queries" query={query} setQuery={setQuery} />
) : null,
Expand Down
1 change: 1 addition & 0 deletions frontend/src/queries/nodes/DataTable/dataTableLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ export const dataTableLogic = kea<dataTableLogicType>([
showSearch: query.showSearch ?? showIfFull,
showActions: query.showActions ?? true,
showDateRange: query.showDateRange ?? showIfFull,
showTestAccountFilters: query.showTestAccountFilters ?? showIfFull,
showExport: query.showExport ?? showIfFull,
showReload: query.showReload ?? showIfFull,
showTimings: query.showTimings ?? flagQueryTimingsEnabled,
Expand Down
2 changes: 2 additions & 0 deletions frontend/src/queries/nodes/DataTable/queryFeatures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export enum QueryFeature {
selectAndOrderByColumns,
displayResponseError,
hideLoadNextButton,
testAccountFilters,
}

export function getQueryFeatures(query: Node): Set<QueryFeature> {
Expand All @@ -34,6 +35,7 @@ export function getQueryFeatures(query: Node): Set<QueryFeature> {
features.add(QueryFeature.eventPropertyFilters)
features.add(QueryFeature.resultIsArrayOfArrays)
features.add(QueryFeature.displayResponseError)
features.add(QueryFeature.testAccountFilters)
}

if (isEventsQuery(query)) {
Expand Down
15 changes: 15 additions & 0 deletions frontend/src/queries/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,10 @@
"description": "Include a free text search field (PersonsNode only)",
"type": "boolean"
},
"showTestAccountFilters": {
"description": "Show filter to exclude test accounts",
"type": "boolean"
},
"showTimings": {
"description": "Show a detailed query timing breakdown",
"type": "boolean"
Expand Down Expand Up @@ -1132,6 +1136,10 @@
"description": "Limit to events matching this string",
"type": ["string", "null"]
},
"filterTestAccounts": {
"description": "Filter test accounts",
"type": "boolean"
},
"fixedProperties": {
"description": "Fixed properties in the query, can't be edited in the interface (e.g. scoping down by person)",
"items": {
Expand Down Expand Up @@ -1962,6 +1970,9 @@
"dateRange": {
"$ref": "#/definitions/DateRange"
},
"filterTestAccounts": {
"type": "boolean"
},
"properties": {
"items": {
"$ref": "#/definitions/AnyPropertyFilter"
Expand Down Expand Up @@ -4341,6 +4352,10 @@
"showTable": {
"type": "boolean"
},
"showTestAccountFilters": {
"description": "Show filter to exclude test accounts",
"type": "boolean"
},
"showTimings": {
"description": "Show a detailed query timing breakdown",
"type": "boolean"
Expand Down
5 changes: 5 additions & 0 deletions frontend/src/queries/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ export interface HogQLQueryResponse {
export interface HogQLFilters {
properties?: AnyPropertyFilter[]
dateRange?: DateRange
filterTestAccounts?: boolean
}

export interface HogQLQuery extends DataNode {
Expand Down Expand Up @@ -409,6 +410,8 @@ export interface EventsQuery extends DataNode {
properties?: AnyPropertyFilter[]
/** Fixed properties in the query, can't be edited in the interface (e.g. scoping down by person) */
fixedProperties?: AnyPropertyFilter[]
/** Filter test accounts */
filterTestAccounts?: boolean
/** Limit to events matching this string */
event?: string | null
/**
Expand Down Expand Up @@ -508,6 +511,8 @@ interface DataTableNodeViewProps {
showSearch?: boolean
/** Include a property filter above the table */
showPropertyFilter?: boolean
/** Show filter to exclude test accounts */
showTestAccountFilters?: boolean
/** Include a HogQL query editor above HogQL tables */
showHogQLEditor?: boolean
/** Show the kebab menu at the end of the row */
Expand Down
4 changes: 0 additions & 4 deletions mypy-baseline.txt
Original file line number Diff line number Diff line change
Expand Up @@ -590,10 +590,6 @@ posthog/hogql_queries/insights/test/test_paginators.py:0: error: Argument 2 to "
posthog/hogql_queries/insights/test/test_paginators.py:0: error: Value of type "object" is not indexable [index]
posthog/hogql_queries/insights/test/test_paginators.py:0: error: Value of type "object" is not indexable [index]
posthog/hogql_queries/insights/test/test_paginators.py:0: error: Value of type "object" is not indexable [index]
posthog/hogql_queries/insights/test/test_events_query_runner.py:0: error: Item "Expr" of "Expr | None" has no attribute "exprs" [union-attr]
posthog/hogql_queries/insights/test/test_events_query_runner.py:0: error: Item "None" of "Expr | None" has no attribute "exprs" [union-attr]
posthog/hogql_queries/insights/test/test_events_query_runner.py:0: error: Item "Expr" of "Expr | None" has no attribute "exprs" [union-attr]
posthog/hogql_queries/insights/test/test_events_query_runner.py:0: error: Item "None" of "Expr | None" has no attribute "exprs" [union-attr]
posthog/hogql/test/test_timings.py:0: error: No overload variant of "__setitem__" of "list" matches argument types "int", "float" [call-overload]
posthog/hogql/test/test_timings.py:0: note: Possible overload variants:
posthog/hogql/test/test_timings.py:0: note: def __setitem__(self, SupportsIndex, int, /) -> None
Expand Down
4 changes: 4 additions & 0 deletions posthog/hogql/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ def visit_placeholder(self, node):
)
)

if self.filters.filterTestAccounts:
for prop in self.team.test_account_filters or []:
exprs.append(property_to_expr(prop, self.team))

if len(exprs) == 0:
return ast.Constant(value=True)
if len(exprs) == 1:
Expand Down
26 changes: 25 additions & 1 deletion posthog/hogql/test/test_filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def _print_ast(self, node: ast.Expr):
context=HogQLContext(team_id=self.team.pk, enable_select_queries=True),
)

def test_replace_filters(self):
def test_replace_filters_empty(self):
select = replace_filters(self._parse_select("SELECT event FROM events"), HogQLFilters(), self.team)
self.assertEqual(self._print_ast(select), "SELECT event FROM events LIMIT 10000")

Expand All @@ -42,6 +42,7 @@ def test_replace_filters(self):
)
self.assertEqual(self._print_ast(select), "SELECT event FROM events WHERE true LIMIT 10000")

def test_replace_filters_date_range(self):
select = replace_filters(
self._parse_select("SELECT event FROM events where {filters}"),
HogQLFilters(dateRange=DateRange(date_from="2020-02-02")),
Expand All @@ -62,6 +63,7 @@ def test_replace_filters(self):
"SELECT event FROM events WHERE less(timestamp, toDateTime('2020-02-02 00:00:00.000000')) LIMIT 10000",
)

def test_replace_filters_event_property(self):
select = replace_filters(
self._parse_select("SELECT event FROM events where {filters}"),
HogQLFilters(
Expand All @@ -74,6 +76,7 @@ def test_replace_filters(self):
"SELECT event FROM events WHERE equals(properties.random_uuid, '123') LIMIT 10000",
)

def test_replace_filters_person_property(self):
select = replace_filters(
self._parse_select("SELECT event FROM events where {filters}"),
HogQLFilters(
Expand All @@ -100,3 +103,24 @@ def test_replace_filters(self):
self._print_ast(select),
"SELECT event FROM events WHERE and(equals(properties.random_uuid, '123'), equals(person.properties.random_uuid, '123')) LIMIT 10000",
)

def test_replace_filters_test_accounts(self):
self.team.test_account_filters = [
{
"key": "email",
"type": "person",
"value": "posthog.com",
"operator": "not_icontains",
}
]
self.team.save()

select = replace_filters(
self._parse_select("SELECT event FROM events where {filters}"),
HogQLFilters(filterTestAccounts=True),
self.team,
)
self.assertEqual(
self._print_ast(select),
"SELECT event FROM events WHERE notILike(person.properties.email, '%posthog.com%') LIMIT 10000",
)
4 changes: 4 additions & 0 deletions posthog/hogql_queries/events_query_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,10 @@ def to_query(self) -> ast.SelectQuery:
timings=self.timings,
)
)
if self.query.filterTestAccounts:
with self.timings.measure("test_account_filters"):
for prop in self.team.test_account_filters or []:
where_exprs.append(property_to_expr(prop, self.team))

with self.timings.measure("timestamps"):
# prevent accidentally future events from being visible by default
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
from typing import Tuple, Any
from typing import Tuple, Any, cast

from freezegun import freeze_time

from posthog.hogql import ast
from posthog.hogql.ast import CompareOperationOp
from posthog.hogql_queries.events_query_runner import EventsQueryRunner
from posthog.models import Person, Team
from posthog.models.organization import Organization
Expand Down Expand Up @@ -123,9 +125,30 @@ def test_person_id_expands_to_distinct_ids(self):

# matching team
query_ast = EventsQueryRunner(query=query, team=self.team).to_query()
self.assertEqual(query_ast.where.exprs[0].right.value, ["id1", "id2"])
where_expr = cast(ast.CompareOperation, cast(ast.And, query_ast.where).exprs[0])
right_expr = cast(ast.Constant, where_expr.right)
self.assertEqual(right_expr.value, ["id1", "id2"])

# another team
another_team = Team.objects.create(organization=Organization.objects.create())
query_ast = EventsQueryRunner(query=query, team=another_team).to_query()
self.assertEqual(query_ast.where.exprs[0].right.value, [])
where_expr = cast(ast.CompareOperation, cast(ast.And, query_ast.where).exprs[0])
right_expr = cast(ast.Constant, where_expr.right)
self.assertEqual(right_expr.value, [])

def test_test_account_filters(self):
self.team.test_account_filters = [
{
"key": "email",
"type": "person",
"value": "posthog.com",
"operator": "not_icontains",
}
]
self.team.save()
query = EventsQuery(kind="EventsQuery", select=["*"], filterTestAccounts=True)
query_ast = EventsQueryRunner(query=query, team=self.team).to_query()
where_expr = cast(ast.CompareOperation, cast(ast.And, query_ast.where).exprs[0])
right_expr = cast(ast.Constant, where_expr.right)
self.assertEqual(right_expr.value, "%posthog.com%")
self.assertEqual(where_expr.op, CompareOperationOp.NotILike)
9 changes: 8 additions & 1 deletion posthog/models/test/test_insight_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ def test_dashboard_with_query_insight_and_filters(self) -> None:
{},
{
"dateRange": {"date_from": "-14d", "date_to": "-7d"},
"filterTestAccounts": None,
"properties": None,
},
),
Expand All @@ -153,6 +154,7 @@ def test_dashboard_with_query_insight_and_filters(self) -> None:
{"date_from": "-14d", "date_to": "-7d"},
{
"dateRange": {"date_from": "-14d", "date_to": "-7d"},
"filterTestAccounts": None,
"properties": None,
},
),
Expand All @@ -162,6 +164,7 @@ def test_dashboard_with_query_insight_and_filters(self) -> None:
{"date_from": "-4d", "date_to": "-3d"},
{
"dateRange": {"date_from": "-4d", "date_to": "-3d"},
"filterTestAccounts": None,
"properties": None,
},
),
Expand All @@ -171,21 +174,23 @@ def test_dashboard_with_query_insight_and_filters(self) -> None:
{"date_from": "all"},
{
"dateRange": {"date_from": "all", "date_to": None},
"filterTestAccounts": None,
"properties": None,
},
),
(
# test that if no filters are set then none are outputted
{},
{},
{"dateRange": {"date_from": None, "date_to": None}, "properties": None},
{"dateRange": {"date_from": None, "date_to": None}, "filterTestAccounts": None, "properties": None},
),
(
# test that properties from the query are used when there are no dashboard properties
{"properties": [browser_equals_firefox]},
{},
{
"dateRange": {"date_from": None, "date_to": None},
"filterTestAccounts": None,
"properties": [browser_equals_firefox],
},
),
Expand All @@ -195,6 +200,7 @@ def test_dashboard_with_query_insight_and_filters(self) -> None:
{"properties": [browser_equals_chrome]},
{
"dateRange": {"date_from": None, "date_to": None},
"filterTestAccounts": None,
"properties": [browser_equals_chrome],
},
),
Expand All @@ -204,6 +210,7 @@ def test_dashboard_with_query_insight_and_filters(self) -> None:
{"properties": [browser_equals_chrome]},
{
"dateRange": {"date_from": None, "date_to": None},
"filterTestAccounts": None,
"properties": [browser_equals_firefox, browser_equals_chrome],
},
),
Expand Down
Loading
Loading