Skip to content

Commit

Permalink
fix(insights): Support dashboard filters in ActorsQuery (#21820)
Browse files Browse the repository at this point in the history
* fix(insights): Support dashboard filters in ActorsQuery

* Update `test_create_from_template_json_can_provide_query_tile`

* Update insight.py

* Make `ActorsQuery` properties typing tighter

* Update test_insight_model.py
  • Loading branch information
Twixes authored May 21, 2024
1 parent 64aff85 commit 56cf1c6
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 83 deletions.
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 @@ -989,8 +990,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 @@ -250,6 +250,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 @@ -480,7 +480,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

0 comments on commit 56cf1c6

Please sign in to comment.