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

fix(insights): Support dashboard filters in ActorsQuery #21820

Merged
merged 10 commits into from
May 21, 2024
20 changes: 18 additions & 2 deletions frontend/src/queries/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,16 @@
"additionalProperties": false,
"properties": {
"fixedProperties": {
"description": "Currently only person filters supported (including via HogQL), See `filter_conditions()` in actor_strategies.py.",
"items": {
"$ref": "#/definitions/AnyPropertyFilter"
"anyOf": [
{
"$ref": "#/definitions/PersonPropertyFilter"
},
{
"$ref": "#/definitions/HogQLPropertyFilter"
}
]
},
"type": "array"
},
Expand All @@ -81,8 +89,16 @@
"type": "array"
},
"properties": {
"description": "Currently only person filters supported (including via HogQL). see `filter_conditions()` in actor_strategies.py.",
"items": {
"$ref": "#/definitions/AnyPropertyFilter"
"anyOf": [
{
"$ref": "#/definitions/PersonPropertyFilter"
},
{
"$ref": "#/definitions/HogQLPropertyFilter"
}
]
},
"type": "array"
},
Expand Down
7 changes: 5 additions & 2 deletions frontend/src/queries/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
FunnelsFilterType,
GroupMathType,
HogQLMathType,
HogQLPropertyFilter,
InsightShortId,
InsightType,
IntervalType,
Expand Down Expand Up @@ -986,8 +987,10 @@ export interface ActorsQuery extends DataNode<ActorsQueryResponse> {
source?: InsightActorsQuery | FunnelsActorsQuery | FunnelCorrelationActorsQuery | HogQLQuery
select?: HogQLExpression[]
search?: string
properties?: AnyPropertyFilter[]
fixedProperties?: AnyPropertyFilter[]
/** Currently only person filters supported (including via HogQL). see `filter_conditions()` in actor_strategies.py. */
properties?: (PersonPropertyFilter | HogQLPropertyFilter)[]
/** Currently only person filters supported (including via HogQL), See `filter_conditions()` in actor_strategies.py. */
fixedProperties?: (PersonPropertyFilter | HogQLPropertyFilter)[]
orderBy?: string[]
limit?: integer
offset?: integer
Expand Down
13 changes: 4 additions & 9 deletions posthog/api/insight.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,7 @@
)
from posthog.hogql.constants import BREAKDOWN_VALUES_LIMIT
from posthog.decorators import cached_by_filters
from posthog.helpers.multi_property_breakdown import (
protect_old_clients_from_multi_property_default,
)
from posthog.helpers.multi_property_breakdown import protect_old_clients_from_multi_property_default
from posthog.hogql.errors import ExposedHogQLError
from posthog.hogql.timings import HogQLTimings
from posthog.hogql_queries.apply_dashboard_filters import WRAPPER_NODE_KINDS
Expand Down Expand Up @@ -83,10 +81,7 @@
from posthog.queries.stickiness import Stickiness
from posthog.queries.trends.trends import Trends
from posthog.queries.util import get_earliest_timestamp
from posthog.rate_limit import (
ClickHouseBurstRateThrottle,
ClickHouseSustainedRateThrottle,
)
from posthog.rate_limit import ClickHouseBurstRateThrottle, ClickHouseSustainedRateThrottle
from posthog.settings import CAPTURE_TIME_TO_SEE_DATA, SITE_URL
from prometheus_client import Counter
from posthog.user_permissions import UserPermissionsSerializerMixin
Expand Down Expand Up @@ -519,11 +514,11 @@ def to_representation(self, instance: Insight):
if hogql_insights_replace_filters(instance.team) and (
instance.query is not None or instance.query_from_filters is not None
):
from posthog.hogql_queries.apply_dashboard_filters import apply_dashboard_filters
from posthog.hogql_queries.apply_dashboard_filters import apply_dashboard_filters_to_dict

query = instance.query or instance.query_from_filters
if dashboard:
query = apply_dashboard_filters(query, dashboard.filters, instance.team)
query = apply_dashboard_filters_to_dict(query, dashboard.filters, instance.team)
representation["filters"] = {}
representation["query"] = query
else:
Expand Down
14 changes: 0 additions & 14 deletions posthog/api/test/dashboards/test_dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -1293,22 +1293,8 @@ def test_create_from_template_json_can_provide_query_tile(self) -> None:
"kind": "DataTableNode",
"columns": ["person", "id", "created_at", "person.$delete"],
"source": {
"actionId": None,
"after": None,
"before": None,
"event": None,
"filterTestAccounts": None,
"fixedProperties": None,
"kind": "EventsQuery",
"limit": None,
"modifiers": None,
"offset": None,
"orderBy": None,
"personId": None,
"properties": None,
"response": None,
"select": ["*"],
"where": None,
},
},
"result": None,
Expand Down
6 changes: 5 additions & 1 deletion posthog/hogql_queries/actors_query_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from posthog.hogql_queries.insights.insight_actors_query_runner import InsightActorsQueryRunner
from posthog.hogql_queries.insights.paginators import HogQLHasMorePaginator
from posthog.hogql_queries.query_runner import QueryRunner, get_query_runner
from posthog.schema import ActorsQuery, ActorsQueryResponse, CachedActorsQueryResponse
from posthog.schema import ActorsQuery, ActorsQueryResponse, CachedActorsQueryResponse, DashboardFilter


class ActorsQueryRunner(QueryRunner):
Expand Down Expand Up @@ -249,6 +249,10 @@ def to_query(self) -> ast.SelectQuery:
def to_actors_query(self) -> ast.SelectQuery:
return self.to_query()

def apply_dashboard_filters(self, dashboard_filter: DashboardFilter):
if self.source_query_runner:
self.source_query_runner.apply_dashboard_filters(dashboard_filter)

def _is_stale(self, cached_result_package):
return True

Expand Down
10 changes: 5 additions & 5 deletions posthog/hogql_queries/apply_dashboard_filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,18 @@


# Apply the filters from the django-style Dashboard object
def apply_dashboard_filters(query: dict, filters: dict, team: Team) -> dict:
kind = query.get("kind", None)
def apply_dashboard_filters_to_dict(query: dict, filters: dict, team: Team) -> dict:
if not filters:
return query

if kind in WRAPPER_NODE_KINDS:
source = apply_dashboard_filters(query["source"], filters, team)
if query.get("kind") in WRAPPER_NODE_KINDS:
source = apply_dashboard_filters_to_dict(query["source"], filters, team)
return {**query, "source": source}

try:
query_runner = get_query_runner(query, team)
except ValueError:
capture_exception()
return query

query_runner.apply_dashboard_filters(DashboardFilter(**filters))
return query_runner.query.model_dump()
2 changes: 1 addition & 1 deletion posthog/hogql_queries/query_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ def apply_dashboard_filters(self, dashboard_filter: DashboardFilter):
),
],
)
except Exception:
except:
# If pydantic is unhappy about the shape of data, let's ignore property filters and carry on
capture_exception()
logger.exception("Failed to apply dashboard property filters")
Expand Down
4 changes: 2 additions & 2 deletions posthog/models/insight.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,12 +181,12 @@ def dashboard_filters(self, dashboard: Optional[Dashboard] = None):
return self.filters

def get_effective_query(self, *, dashboard: Optional[Dashboard]) -> Optional[dict]:
from posthog.hogql_queries.apply_dashboard_filters import apply_dashboard_filters
from posthog.hogql_queries.apply_dashboard_filters import apply_dashboard_filters_to_dict

if not dashboard or not self.query:
return self.query

return apply_dashboard_filters(self.query, dashboard.filters, self.team)
return apply_dashboard_filters_to_dict(self.query, dashboard.filters, self.team)

@property
def url(self):
Expand Down
13 changes: 2 additions & 11 deletions posthog/models/test/test_insight_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,7 @@ def test_dashboard_with_query_insight_and_filters(self) -> None:
"dateRange": {
"date_from": "-14d",
"date_to": "-7d",
"explicitDate": None,
},
"filterTestAccounts": None,
"properties": None,
}
},
),
(
Expand Down Expand Up @@ -165,19 +162,13 @@ def test_dashboard_with_query_insight_and_filters(self) -> None:
# test that if no filters are set then none are outputted
{},
{},
{
"dateRange": None,
"filterTestAccounts": None,
"properties": None,
},
{},
),
(
# test that properties from the query are used when there are no dashboard properties
{"properties": [browser_equals_firefox]},
{},
{
"dateRange": None,
"filterTestAccounts": None,
"properties": [browser_equals_firefox],
},
),
Expand Down
44 changes: 8 additions & 36 deletions posthog/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -4036,49 +4036,21 @@ class ActorsQuery(BaseModel):
model_config = ConfigDict(
extra="forbid",
)
fixedProperties: Optional[
list[
Union[
EventPropertyFilter,
PersonPropertyFilter,
ElementPropertyFilter,
SessionPropertyFilter,
CohortPropertyFilter,
RecordingDurationFilter,
GroupPropertyFilter,
FeaturePropertyFilter,
HogQLPropertyFilter,
EmptyPropertyFilter,
DataWarehousePropertyFilter,
DataWarehousePersonPropertyFilter,
]
]
] = None
fixedProperties: Optional[list[Union[PersonPropertyFilter, HogQLPropertyFilter]]] = Field(
default=None,
description="Currently only person filters supported (including via HogQL), See `filter_conditions()` in actor_strategies.py.",
)
kind: Literal["ActorsQuery"] = "ActorsQuery"
limit: Optional[int] = None
modifiers: Optional[HogQLQueryModifiers] = Field(
default=None, description="Modifiers used when performing the query"
)
offset: Optional[int] = None
orderBy: Optional[list[str]] = None
properties: Optional[
list[
Union[
EventPropertyFilter,
PersonPropertyFilter,
ElementPropertyFilter,
SessionPropertyFilter,
CohortPropertyFilter,
RecordingDurationFilter,
GroupPropertyFilter,
FeaturePropertyFilter,
HogQLPropertyFilter,
EmptyPropertyFilter,
DataWarehousePropertyFilter,
DataWarehousePersonPropertyFilter,
]
]
] = None
properties: Optional[list[Union[PersonPropertyFilter, HogQLPropertyFilter]]] = Field(
default=None,
description="Currently only person filters supported (including via HogQL). see `filter_conditions()` in actor_strategies.py.",
)
response: Optional[ActorsQueryResponse] = None
search: Optional[str] = None
select: Optional[list[str]] = None
Expand Down
Loading